From db2f3bd3dde359a8e2a84fea2a76694a023f6ee6 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Wed, 13 Nov 2024 14:37:32 +0100 Subject: [PATCH 1/4] sys/net/gnrc/pkt: use uint8_t for user count It's very unlikely that a pkt snip will have more than 255 users. Use a uint8_t here to save 4 bytes per snip as this now fits into the ununsed struct padding. --- sys/include/net/gnrc/pkt.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/sys/include/net/gnrc/pkt.h b/sys/include/net/gnrc/pkt.h index 7467f0bdc5..c6eddef01e 100644 --- a/sys/include/net/gnrc/pkt.h +++ b/sys/include/net/gnrc/pkt.h @@ -110,17 +110,18 @@ typedef struct gnrc_pktsnip { struct gnrc_pktsnip *next; /**< next snip in the packet */ void *data; /**< pointer to the data of the snip */ size_t size; /**< the length of the snip in byte */ + /* end of iolist_t */ +#ifdef MODULE_GNRC_NETERR + kernel_pid_t err_sub; /**< subscriber to errors related to this + * packet snip */ +#endif + gnrc_nettype_t type; /**< protocol of the packet snip */ /** * @brief Counter of threads currently having control over this packet. * * @internal */ - unsigned int users; - gnrc_nettype_t type; /**< protocol of the packet snip */ -#ifdef MODULE_GNRC_NETERR - kernel_pid_t err_sub; /**< subscriber to errors related to this - * packet snip */ -#endif + uint8_t users; } gnrc_pktsnip_t; /** From e8b3b4d3e42d32f0a27bdd22fa4a800f103c909b Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Fri, 15 Nov 2024 16:41:42 +0100 Subject: [PATCH 2/4] gnrc_pktbuf: assert that user count does not exceed 255 --- sys/net/gnrc/pktbuf_malloc/gnrc_pktbuf_malloc.c | 1 + sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c | 1 + 2 files changed, 2 insertions(+) diff --git a/sys/net/gnrc/pktbuf_malloc/gnrc_pktbuf_malloc.c b/sys/net/gnrc/pktbuf_malloc/gnrc_pktbuf_malloc.c index 683044adc0..9eb6335155 100644 --- a/sys/net/gnrc/pktbuf_malloc/gnrc_pktbuf_malloc.c +++ b/sys/net/gnrc/pktbuf_malloc/gnrc_pktbuf_malloc.c @@ -210,6 +210,7 @@ void gnrc_pktbuf_hold(gnrc_pktsnip_t *pkt, unsigned int num) { mutex_lock(&gnrc_pktbuf_mutex); while (pkt) { + assert(pkt->users + num <= 0xff); pkt->users += num; pkt = pkt->next; } diff --git a/sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c b/sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c index 3c98e8a61d..f1c15279c8 100644 --- a/sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c +++ b/sys/net/gnrc/pktbuf_static/gnrc_pktbuf_static.c @@ -213,6 +213,7 @@ void gnrc_pktbuf_hold(gnrc_pktsnip_t *pkt, unsigned int num) { mutex_lock(&gnrc_pktbuf_mutex); while (pkt) { + assert(pkt->users + num <= 0xff); pkt->users += num; pkt = pkt->next; } From 300a936e42ca3073b2ae99da94395909a410089e Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Fri, 15 Nov 2024 16:59:08 +0100 Subject: [PATCH 3/4] gnrc: add comment about gnrc_pktbuf_hold() use --- sys/net/gnrc/netapi/gnrc_netapi.c | 1 + sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/sys/net/gnrc/netapi/gnrc_netapi.c b/sys/net/gnrc/netapi/gnrc_netapi.c index 266d0839f3..a8cecb285b 100644 --- a/sys/net/gnrc/netapi/gnrc_netapi.c +++ b/sys/net/gnrc/netapi/gnrc_netapi.c @@ -92,6 +92,7 @@ int gnrc_netapi_dispatch(gnrc_nettype_t type, uint32_t demux_ctx, if (numof != 0) { gnrc_netreg_entry_t *sendto = gnrc_netreg_lookup(type, demux_ctx); + /* the packet is replicated over all interfaces that is's being sent on */ gnrc_pktbuf_hold(pkt, numof - 1); while (sendto) { diff --git a/sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c b/sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c index fea9c412b6..5fbabfb625 100644 --- a/sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c +++ b/sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c @@ -576,7 +576,7 @@ static void _send_multicast(gnrc_pktsnip_t *pkt, bool prep_hdr, if (!gnrc_netif_highlander()) { /* interface not given: send over all interfaces */ if (netif == NULL) { - /* send packet to link layer */ + /* the packet is replicated over all interfaces that is's being sent on */ gnrc_pktbuf_hold(pkt, ifnum - 1); while ((netif = gnrc_netif_iter(netif))) { From cb554fd342b4322970f1cfbd1776056a5c235b43 Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Fri, 15 Nov 2024 17:06:47 +0100 Subject: [PATCH 4/4] tests/unittests: use proper initializers for gnrc_pktsnip_t --- tests/unittests/tests-pktbuf/tests-pktbuf.c | 5 ++++- tests/unittests/tests-pktqueue/tests-pktqueue.c | 4 ++-- .../tests-priority_pktqueue/tests-priority_pktqueue.c | 4 ++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/unittests/tests-pktbuf/tests-pktbuf.c b/tests/unittests/tests-pktbuf/tests-pktbuf.c index a78fd16c84..cb9b30479f 100644 --- a/tests/unittests/tests-pktbuf/tests-pktbuf.c +++ b/tests/unittests/tests-pktbuf/tests-pktbuf.c @@ -325,7 +325,10 @@ static void test_pktbuf_mark__pkt_NOT_NULL__size_greater_than_pkt_size(void) static void test_pktbuf_mark__pkt_NOT_NULL__pkt_data_NULL(void) { - gnrc_pktsnip_t pkt = { NULL, NULL, sizeof(TEST_STRING16), 1, GNRC_NETTYPE_TEST }; + gnrc_pktsnip_t pkt = { .size = sizeof(TEST_STRING16), + .type = GNRC_NETTYPE_TEST, + .users = 1, + }; TEST_ASSERT_NULL(gnrc_pktbuf_mark(&pkt, sizeof(TEST_STRING16) - 1, GNRC_NETTYPE_TEST)); diff --git a/tests/unittests/tests-pktqueue/tests-pktqueue.c b/tests/unittests/tests-pktqueue/tests-pktqueue.c index 858d9005d4..84795307c6 100644 --- a/tests/unittests/tests-pktqueue/tests-pktqueue.c +++ b/tests/unittests/tests-pktqueue/tests-pktqueue.c @@ -21,8 +21,8 @@ #include "unittests-constants.h" #include "tests-pktqueue.h" -#define PKT_INIT_ELEM(len, data, next) \ - { (next), (void *)(data), (len), 1, GNRC_NETTYPE_UNDEF } +#define PKT_INIT_ELEM(len, ptr, nxt) \ + { .next = (nxt), .data = (void *)(ptr), .size = (len), .users = 1, .type = GNRC_NETTYPE_UNDEF } #define PKT_INIT_ELEM_STATIC_DATA(data, next) PKT_INIT_ELEM(sizeof(data), (void *)(data), (next)) #define PKTQUEUE_INIT_ELEM(pkt) { NULL, pkt } diff --git a/tests/unittests/tests-priority_pktqueue/tests-priority_pktqueue.c b/tests/unittests/tests-priority_pktqueue/tests-priority_pktqueue.c index faed65340a..cc5b8ca7ba 100644 --- a/tests/unittests/tests-priority_pktqueue/tests-priority_pktqueue.c +++ b/tests/unittests/tests-priority_pktqueue/tests-priority_pktqueue.c @@ -22,8 +22,8 @@ #include "unittests-constants.h" #include "tests-priority_pktqueue.h" -#define PKT_INIT_ELEM(len, data, next) \ - { (next), (void *)(data), (len), 1, GNRC_NETTYPE_UNDEF } +#define PKT_INIT_ELEM(len, ptr, nxt) \ + { .next = (nxt), .data = (void *)(ptr), .size = (len), .users = 1, .type = GNRC_NETTYPE_UNDEF } #define PKT_INIT_ELEM_STATIC_DATA(data, next) PKT_INIT_ELEM(sizeof(data), (void *)(data), (next)) #define PKTQUEUE_INIT_ELEM(pkt) { NULL, pkt }