Re: [PATCH v8 23/30] dhcp: use iov_tail rather than pool

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4742 bytes --]

On Tue, Aug 05, 2025 at 05:46:21PM +0200, Laurent Vivier wrote:
> This patch refactors the dhcp() function to accept `struct iov_tail *data`
> directly as its packet input, replacing the previous `const struct pool *p`
> parameter. Consequently, dhcp() no longer fetches packet data internally
> using packet_data().
> 
> This change simplifies callers, such as tap4_handler(), which now pass
> the iov_tail representing the L2 frame directly to dhcp(). This removes
> the need for intermediate packet pool handling for DHCP processing.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  dhcp.c | 32 ++++++++++++++------------------
>  dhcp.h |  2 +-
>  tap.c  |  5 +----
>  3 files changed, 16 insertions(+), 23 deletions(-)
> 
> diff --git a/dhcp.c b/dhcp.c
> index 47317f334945..cad1d037cee1 100644
> --- a/dhcp.c
> +++ b/dhcp.c
> @@ -296,11 +296,11 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len)
>  /**
>   * dhcp() - Check if this is a DHCP message, reply as needed
>   * @c:		Execution context
> - * @p:		Packet pool, single packet with Ethernet buffer
> + * @data:	Single packet with Ethernet buffer
>   *
>   * Return: 0 if it's not a DHCP message, 1 if handled, -1 on failure
>   */
> -int dhcp(const struct ctx *c, const struct pool *p)
> +int dhcp(const struct ctx *c, struct iov_tail *data)
>  {
>  	char macstr[ETH_ADDRSTRLEN];
>  	size_t mlen, dlen, opt_len;
> @@ -311,24 +311,20 @@ int dhcp(const struct ctx *c, const struct pool *p)
>  	const struct ethhdr *eh;
>  	const struct iphdr *iph;
>  	const struct udphdr *uh;
> -	struct iov_tail data;
>  	struct msg const *m;
>  	struct msg reply;
>  	unsigned int i;
>  	struct msg m_storage;
>  
> -	if (!packet_get(p, 0, &data))
> -		return -1;
> -
> -	eh = IOV_REMOVE_HEADER(&data, eh_storage);
> -	iph = IOV_PEEK_HEADER(&data, iph_storage);
> +	eh = IOV_REMOVE_HEADER(data, eh_storage);
> +	iph = IOV_PEEK_HEADER(data, iph_storage);
>  	if (!eh || !iph)
>  		return -1;
>  
> -	if (!iov_tail_drop(&data, iph->ihl * 4UL))
> +	if (!iov_tail_drop(data, iph->ihl * 4UL))
>  		return -1;
>  
> -	uh = IOV_REMOVE_HEADER(&data, uh_storage);
> +	uh = IOV_REMOVE_HEADER(data, uh_storage);
>  	if (!uh)
>  		return -1;
>  
> @@ -338,8 +334,8 @@ int dhcp(const struct ctx *c, const struct pool *p)
>  	if (c->no_dhcp)
>  		return 1;
>  
> -	mlen = iov_tail_size(&data);
> -	m = (struct msg const *)iov_remove_header_(&data, &m_storage,
> +	mlen = iov_tail_size(data);
> +	m = (struct msg const *)iov_remove_header_(data, &m_storage,
>  						   offsetof(struct msg, o),
>  						   __alignof__(struct msg));
>  	if (!m						||
> @@ -367,24 +363,24 @@ int dhcp(const struct ctx *c, const struct pool *p)
>  	for (i = 0; i < ARRAY_SIZE(opts); i++)
>  		opts[i].clen = -1;
>  
> -	opt_len = iov_tail_size(&data);
> +	opt_len = iov_tail_size(data);
>  	while (opt_len >= 2) {
>  		uint8_t olen_storage, type_storage;
>  		const uint8_t *olen;
>  		uint8_t *type;
>  
> -		type = IOV_REMOVE_HEADER(&data, type_storage);
> -		olen = IOV_REMOVE_HEADER(&data, olen_storage);
> +		type = IOV_REMOVE_HEADER(data, type_storage);
> +		olen = IOV_REMOVE_HEADER(data, olen_storage);
>  		if (!type || !olen)
>  			return -1;
>  
> -		opt_len = iov_tail_size(&data);
> +		opt_len = iov_tail_size(data);
>  		if (opt_len < *olen)
>  			return -1;
>  
> -		iov_to_buf(&data.iov[0], data.cnt, data.off, &opts[*type].c, *olen);
> +		iov_to_buf(&data->iov[0], data->cnt, data->off, &opts[*type].c, *olen);
>  		opts[*type].clen = *olen;
> -		iov_tail_drop(&data, *olen);
> +		iov_tail_drop(data, *olen);
>  		opt_len -= *olen;
>  	}
>  
> diff --git a/dhcp.h b/dhcp.h
> index 87aeecd8dec8..cd50c99b8856 100644
> --- a/dhcp.h
> +++ b/dhcp.h
> @@ -6,7 +6,7 @@
>  #ifndef DHCP_H
>  #define DHCP_H
>  
> -int dhcp(const struct ctx *c, const struct pool *p);
> +int dhcp(const struct ctx *c, struct iov_tail *data);
>  void dhcp_init(void);
>  
>  #endif /* DHCP_H */
> diff --git a/tap.c b/tap.c
> index ace735cfc136..7d7e89304723 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -785,11 +785,8 @@ resume:
>  		if (iph->protocol == IPPROTO_UDP) {
>  			struct iov_tail eh_data;
>  
> -			PACKET_POOL_P(pkt, 1, in->buf, in->buf_size);
> -
>  			packet_get(in, i, &eh_data);
> -			packet_add(pkt, &eh_data);
> -			if (dhcp(c, pkt))
> +			if (dhcp(c, &eh_data))
>  				continue;
>  		}
>  

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]