Re: [PATCH v8 07/30] arp: Convert to iov_tail

7 Aug 2025

On 06/08/2025 04:17, David Gibson wrote:
...
On Tue, Aug 05, 2025 at 05:46:05PM +0200, Laurent Vivier wrote:
...
Use packet_data() and extract headers using IOV_REMOVE_HEADER()
rather than packet_get().
Signed-off-by: Laurent Vivier 
Reviewed-by: David Gibson 
Still R-b, but making an observation below that's perhaps more
relevant to the previous patch.
...
---
  arp.c    | 12 +++++++++---
  packet.c |  1 -
  2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arp.c b/arp.c
index 9f1fedeafec0..b3ac42082841 100644
--- a/arp.c
+++ b/arp.c
@@ -74,14 +74,20 @@ int arp(const struct ctx *c, const struct pool *p)
      struct arphdr ah;
      struct arpmsg am;
  } __attribute__((__packed__)) resp;
+	struct arphdr ah_storage;
+	struct ethhdr eh_storage;
+	struct arpmsg am_storage;
  const struct ethhdr *eh;
  const struct arphdr *ah;
  const struct arpmsg *am;
+	struct iov_tail data;
-	eh = packet_get(p, 0, 0,			 sizeof(*eh), NULL);
-	ah = packet_get(p, 0, sizeof(*eh),		 sizeof(*ah), NULL);
-	am = packet_get(p, 0, sizeof(*eh) + sizeof(*ah), sizeof(*am), NULL);
+	if (!packet_data(p, 0, &data))
+		return -1;
The only case where packet_data() will return false is if you give it
a bad packet index.  That should never happen, by construction.  So
I'm wondering if that should be an ASSSERT() in packet_data() rather
than a return value.
Stefano, why do you think of this idea?

Thanks,
Laurent

    