Re: [PATCH v5 25/29] ndp: use iov_tail rather than pool

On Thu, 17 Apr 2025 18:51:32 +0200 Laurent Vivier wrote:

Signed-off-by: Laurent Vivier --- ndp.c | 16 ++++++++-------- ndp.h | 4 ++-- tap.c | 10 +++------- 3 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/ndp.c b/ndp.c index b09a1a133f56..95cb1d983f12 100644 --- a/ndp.c +++ b/ndp.c @@ -336,13 +336,17 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst) * @c: Execution context * @ih: ICMPv6 header * @saddr: Source IPv6 address - * @p: Packet pool + * @data: Single packet with ICMPv6 header * * Return: 0 if not handled here, 1 if handled, -1 on failure */ -int ndp(const struct ctx *c, const struct icmp6hdr *ih, - const struct in6_addr *saddr, const struct pool *p) +int ndp(const struct ctx *c, const struct in6_addr *saddr, + struct iov_tail *data) { + struct icmp6hdr ihc; + const struct icmp6hdr *ih;

Nit: these two should be swapped. Usual comment about the 'c' (here it's particularly unclear, I think, but also the 'nsc' usage below is rather obscure).

+ + ih = IOV_PEEK_HEADER(data, ihc); if (ih->icmp6_type < RS || ih->icmp6_type > NA) return 0;

I had started splitting warnings reported by Coverity per patch but actually it's all about cases like this one, so here it is as a single comment: /home/sbrivio/passt/ndp.c:351:2: Type: Dereference null return value (NULL_RETURNS) /home/sbrivio/passt/ndp.c:350:2: Call to null-returning function 1. returned_null: "iov_peek_header_" returns "NULL" (checked 18 out of 20 times). /home/sbrivio/passt/iov.c:315:2: Call to null-returning function 1.1. path: Condition "p", taking false branch. /home/sbrivio/passt/iov.c:319:2: 1.2. path: Condition "l != len", taking true branch. /home/sbrivio/passt/iov.c:320:3: 1.3. return_null: Explicitly returning null. /home/sbrivio/passt/ndp.c:350:2: 2. var_assigned: Assigning: "ih" = "NULL" return value from "iov_peek_header_". /home/sbrivio/passt/ndp.c:351:2: 3. dereference: Dereferencing "ih", which is known to be "NULL". /home/sbrivio/passt/dhcp.c:320:2: Examples where return value was checked for null 4. example_assign: Example 1: Assigning: "iph" = return value from "iov_peek_header_(data, &iphc, 20UL, 4UL)". /home/sbrivio/passt/dhcp.c:321:2: 5. example_checked: Example 1 (cont.): "iph" has its value checked in "iph". /home/sbrivio/passt/dhcpv6.c:296:2: Examples where return value was checked for null 6. example_checked: Example 2: "iov_peek_header_(data, &oc, 4UL, 1UL)" has its value checked in "o = (struct opt_hdr *)iov_peek_header_(data, &oc, 4UL, 1UL)". /home/sbrivio/passt/iov.c:343:2: Examples where return value was checked for null 7. example_assign: Example 3: Assigning: "p" = return value from "iov_peek_header_(tail, v, len, align)". /home/sbrivio/passt/iov.c:345:2: 8. example_checked: Example 3 (cont.): "p" has its value checked in "p". /home/sbrivio/passt/ip.c:50:2: Examples where return value was checked for null 9. example_checked: Example 4: "iov_peek_header_(data, &oc, 2UL, 1UL)" has its value checked in "o = (struct ipv6_opt_hdr *)iov_peek_header_(data, &oc, 2UL, 1UL)". /home/sbrivio/passt/tap.c:776:3: Examples where return value was checked for null 10. example_assign: Example 5: Assigning: "uh" = return value from "iov_peek_header_(&data, &uhc, 8UL, 2UL)". /home/sbrivio/passt/tap.c:777:3: 11. example_checked: Example 5 (cont.): "uh" has its value checked in "uh". ...and I think I would actually be reasonable to add a check on 'ih'. Same for (with this whole series applied): /home/sbrivio/passt/dhcpv6.c:349:5: Type: Dereference null return value (NULL_RETURNS) /home/sbrivio/passt/tap.c:1006:4: Type: Dereference null return value (NULL_RETURNS) /home/sbrivio/passt/tap.c:1015:4: Type: Dereference null return value (NULL_RETURNS) -- Stefano