Re: [PATCH v8 18/30] ip: Use iov_tail in ipv6_l4hdr()

On Tue, Aug 05, 2025 at 05:46:16PM +0200, Laurent Vivier wrote:

Use packet_data() and extract headers using IOV_REMOVE_HEADER() and IOV_PEEK_HEADER() rather than packet_get().

Signed-off-by: Laurent Vivier --- ip.c | 32 +++++++++++++++----------------- ip.h | 3 +-- packet.c | 1 + tap.c | 4 +++- 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/ip.c b/ip.c index 2cc7f6548aff..50bd69a70596 100644 --- a/ip.c +++ b/ip.c @@ -23,50 +23,48 @@

/** * ipv6_l4hdr() - Find pointer to L4 header in IPv6 packet and extract protocol - * @p: Packet pool, packet number @idx has IPv6 header at @offset - * @idx: Index of packet in pool - * @offset: Pre-calculated IPv6 header offset + * @data: IPv6 packet * @proto: Filled with L4 protocol number * @dlen: Data length (payload excluding header extensions), set on return * - * Return: pointer to L4 header, NULL if not found + * Return: true if the L4 header is found and @data, @proto, @dlen are set, + * false on error. Outputs are indeterminate on failure. */ -char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto, - size_t *dlen) +bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen) { + struct ipv6_opt_hdr o_storage; const struct ipv6_opt_hdr *o; + struct ipv6hdr ip6h_storage; const struct ipv6hdr *ip6h; - char *base; int hdrlen; uint8_t nh;

- base = packet_get(p, idx, 0, 0, NULL); - ip6h = packet_get(p, idx, offset, sizeof(*ip6h), dlen); + ip6h = IOV_REMOVE_HEADER(data, ip6h_storage); if (!ip6h) - return NULL; - - offset += sizeof(*ip6h); + return false; + *dlen = iov_tail_size(data);

nh = ip6h->nexthdr; if (!IPV6_NH_OPT(nh)) goto found;

- while ((o = packet_get_try(p, idx, offset, sizeof(*o), dlen))) { + while ((o = IOV_PEEK_HEADER(data, o_storage))) { + *dlen = iov_tail_size(data) - sizeof(*o);

I don't think this is quite right. This removes the option header from the total, but not the option body (if any). It will be corrected on the next loop, via iova_tail_size() - at the cost of rescanning the IOV. However, I think that means the body of the last option will be incorrectly included in dlen. You could update *dlen incrementally, but AFAICT you don't need it internally, so you could just compute from iov_tail_size() after the found label.

nh = o->nexthdr; hdrlen = (o->hdrlen + 1) * 8;

if (IPV6_NH_OPT(nh)) - offset += hdrlen; + iov_tail_drop(data, hdrlen); else goto found; }

- return NULL; + return false;

found: if (nh == 59)

Pre-existing: it'd be nice to have a name for that bare 59.

- return NULL; + return false;

*proto = nh; - return base + offset; + return true; } diff --git a/ip.h b/ip.h index 24509d9c11cd..5830b92302e2 100644 --- a/ip.h +++ b/ip.h @@ -115,8 +115,7 @@ static inline uint32_t ip6_get_flow_lbl(const struct ipv6hdr *ip6h) ip6h->flow_lbl[2]; }

-char *ipv6_l4hdr(const struct pool *p, int idx, size_t offset, uint8_t *proto, - size_t *dlen); +bool ipv6_l4hdr(struct iov_tail *data, uint8_t *proto, size_t *dlen);

/* IPv6 link-local all-nodes multicast address, ff02::1 */ static const struct in6_addr in6addr_ll_all_nodes = { diff --git a/packet.c b/packet.c index 34b1722b9a03..014b353cdf8b 100644 --- a/packet.c +++ b/packet.c @@ -133,6 +133,7 @@ void packet_add_do(struct pool *p, struct iov_tail *data, * * Return: pointer to start of data range, NULL on invalid range or descriptor */ +/* cppcheck-suppress [staticFunction] */ void *packet_get_try_do(const struct pool *p, size_t idx, size_t offset, size_t len, size_t *left, const char *func, int line) { diff --git a/tap.c b/tap.c index 8d2b118152f1..d7852fad6069 100644 --- a/tap.c +++ b/tap.c @@ -911,8 +911,10 @@ resume: if (plen != check) continue;

- if (!(l4h = ipv6_l4hdr(in, i, sizeof(*eh), &proto, &l4len))) + data = IOV_TAIL_FROM_BUF(ip6h, sizeof(*ip6h) + check, 0); + if (!ipv6_l4hdr(&data, &proto, &l4len)) continue; + l4h = (char *)data.iov[0].iov_base + data.off;

if (IN6_IS_ADDR_LOOPBACK(saddr) || IN6_IS_ADDR_LOOPBACK(daddr)) { char sstr[INET6_ADDRSTRLEN], dstr[INET6_ADDRSTRLEN];

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson