From d3fc3985806a97b15e363276c33c19bc50a10f24 Mon Sep 17 00:00:00 2001 From: David van Moolenbroek Date: Mon, 27 Feb 2017 14:43:58 +0000 Subject: [PATCH] arp/ndp: allow overriding the decision to copy pbufs lwIP aims to support zero-copy TX, and thus, must internally handle all cases that pbufs are referenced rather than copied upon low-level output. However, in the current situation, the arp/ndp packet queuing routines conservatively copy entire packets, even when unnecessary in cases where lwIP is used in a zero-copy compliant manner. This patch moves the decision whether to copy into a centralized macro, allowing zero-copy compliant applications to override the macro to avoid the unnecessary copies. The macro defaults to the safe behavior, though. --- src/core/ipv4/etharp.c | 7 +++---- src/core/ipv6/nd6.c | 7 +++---- src/include/lwip/pbuf.h | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/core/ipv4/etharp.c b/src/core/ipv4/etharp.c index efafcea2..d76b7f2b 100644 --- a/src/core/ipv4/etharp.c +++ b/src/core/ipv4/etharp.c @@ -996,13 +996,12 @@ etharp_query(struct netif *netif, const ip4_addr_t *ipaddr, struct pbuf *q) /* entry is still pending, queue the given packet 'q' */ struct pbuf *p; int copy_needed = 0; - /* IF q includes a PBUF_REF, PBUF_POOL or PBUF_RAM, we have no choice but - * to copy the whole queue into a new PBUF_RAM (see bug #11400) - * PBUF_ROMs can be left as they are, since ROM must not get changed. */ + /* IF q includes a pbuf that must be copied, copy the whole chain into a + * new PBUF_RAM. See the definition of PBUF_NEEDS_COPY for details. */ p = q; while (p) { LWIP_ASSERT("no packet queues allowed!", (p->len != p->tot_len) || (p->next == 0)); - if (p->type != PBUF_ROM) { + if (PBUF_NEEDS_COPY(p)) { copy_needed = 1; break; } diff --git a/src/core/ipv6/nd6.c b/src/core/ipv6/nd6.c index df57df12..18b6b755 100644 --- a/src/core/ipv6/nd6.c +++ b/src/core/ipv6/nd6.c @@ -2034,12 +2034,11 @@ nd6_queue_packet(s8_t neighbor_index, struct pbuf *q) return ERR_ARG; } - /* IF q includes a PBUF_REF, PBUF_POOL or PBUF_RAM, we have no choice but - * to copy the whole queue into a new PBUF_RAM (see bug #11400) - * PBUF_ROMs can be left as they are, since ROM must not get changed. */ + /* IF q includes a pbuf that must be copied, we have to copy the whole chain + * into a new PBUF_RAM. See the definition of PBUF_NEEDS_COPY for details. */ p = q; while (p) { - if (p->type != PBUF_ROM) { + if (PBUF_NEEDS_COPY(p)) { copy_needed = 1; break; } diff --git a/src/include/lwip/pbuf.h b/src/include/lwip/pbuf.h index 3ce8449d..c2238028 100644 --- a/src/include/lwip/pbuf.h +++ b/src/include/lwip/pbuf.h @@ -55,6 +55,22 @@ extern "C" { #define LWIP_SUPPORT_CUSTOM_PBUF ((IP_FRAG && !LWIP_NETIF_TX_SINGLE_PBUF) || (LWIP_IPV6 && LWIP_IPV6_FRAG)) #endif +/** PBUF_NEEDS_COPY(p): return a boolean value indicating whether the given + * pbuf needs to be copied in order to be kept around beyond the current call + * stack without risking being corrupted. The default setting provides safety: + * it will make a copy iof any pbuf chain that does not consist entirely of + * PBUF_ROM type pbufs. For setups with zero-copy support, it may be redefined + * to evaluate to true in all cases, for example. However, doing so also has an + * effect on the application side: any buffers that are *not* copied must also + * *not* be reused by the application after passing them to lwIP. For example, + * when setting PBUF_NEEDS_COPY to (0), after using udp_send() with a PBUF_RAM + * pbuf, the application must free the pbuf immediately, rather than reusing it + * for other purposes. For more background information on this, see tasks #6735 + * and #7896, and bugs #11400 and #49914. */ +#ifndef PBUF_NEEDS_COPY +#define PBUF_NEEDS_COPY(p) ((p)->type != PBUF_ROM) +#endif /* PBUF_NEEDS_COPY */ + /* @todo: We need a mechanism to prevent wasting memory in every pbuf (TCP vs. UDP, IPv4 vs. IPv6: UDP/IPv4 packets may waste up to 28 bytes) */