Re: [PATCH 15/18] dhcp: Convert to iov_tail

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

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

On Wed, Apr 02, 2025 at 07:23:40PM +0200, Laurent Vivier wrote:
> Use packet_base() and extract headers using IOV_REMOVE_HEADER()
> and IOV_PEEK_HEADER() rather than packet_get().
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  dhcp.c | 38 ++++++++++++++++++++++----------------
>  iov.c  |  1 -
>  2 files changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/dhcp.c b/dhcp.c
> index b0de04be6f27..f3167beb18b7 100644
> --- a/dhcp.c
> +++ b/dhcp.c
> @@ -302,27 +302,32 @@ static void opt_set_dns_search(const struct ctx *c, size_t max_len)
>   */
>  int dhcp(const struct ctx *c, const struct pool *p)
>  {
> -	size_t mlen, dlen, offset = 0, opt_len, opt_off = 0;
> +	size_t mlen, dlen, opt_len, opt_off = 0;
>  	char macstr[ETH_ADDRSTRLEN];
>  	struct in_addr mask, dst;
>  	const struct ethhdr *eh;
>  	const struct iphdr *iph;
>  	const struct udphdr *uh;
> +	struct iov_tail data;
>  	struct msg const *m;
> +	struct msg mc;
> +	struct ethhdr ehc;
> +	struct iphdr iphc;
> +	struct udphdr uhc;
>  	struct msg reply;
>  	unsigned int i;
>  
> -	eh  = packet_get(p, 0, offset, sizeof(*eh),  NULL);
> -	offset += sizeof(*eh);
> +	if (!packet_base(p, 0, &data))
> +		return -1;
>  
> -	iph = packet_get(p, 0, offset, sizeof(*iph), NULL);
> +	eh = IOV_REMOVE_HEADER(&data, ehc);
> +	iph = IOV_PEEK_HEADER(&data, iphc);
>  	if (!eh || !iph)
>  		return -1;
>  
> -	offset += iph->ihl * 4UL;
> -	uh  = packet_get(p, 0, offset, sizeof(*uh),  &mlen);
> -	offset += sizeof(*uh);
> +	data.off += iph->ihl * 4UL;

iov_drop().

> +	uh = IOV_REMOVE_HEADER(&data, uhc);
>  	if (!uh)
>  		return -1;
>  
> @@ -332,7 +337,9 @@ int dhcp(const struct ctx *c, const struct pool *p)
>  	if (c->no_dhcp)
>  		return 1;
>  
> -	m   = packet_get(p, 0, offset, offsetof(struct msg, o), &opt_len);
> +	mlen = iov_tail_size(&data);
> +	m = (struct msg const *)iov_remove_header_(&data, &mc, offsetof(struct msg, o),
> +						   __alignof__(struct msg));
>  	if (!m						||
>  	    mlen  != ntohs(uh->len) - sizeof(*uh)	||
>  	    mlen  <  offsetof(struct msg, o)		||
> @@ -355,25 +362,24 @@ int dhcp(const struct ctx *c, const struct pool *p)
>  	memset(&reply.file,	0,		sizeof(reply.file));
>  	reply.magic		= m->magic;
>  
> -	offset += offsetof(struct msg, o);
> -
>  	for (i = 0; i < ARRAY_SIZE(opts); i++)
>  		opts[i].clen = -1;
>  
> +	opt_len = iov_tail_size(&data);
>  	while (opt_off + 2 < opt_len) {
> -		const uint8_t *olen, *val;
> +		const uint8_t *olen;
>  		uint8_t *type;
>  
> -		type = packet_get(p, 0, offset + opt_off,	1,	NULL);
> -		olen = packet_get(p, 0, offset + opt_off + 1,	1,	NULL);
> +		type = iov_tail_ptr(&data, opt_off, 1);
> +		olen = iov_tail_ptr(&data, opt_off + 1, 1);

I think you can and should use remove_header() for this as well.

>  		if (!type || !olen)
>  			return -1;
>  
> -		val =  packet_get(p, 0, offset + opt_off + 2,	*olen,	NULL);
> -		if (!val)
> +		if (iov_tail_size(&data) < opt_off + 2 + *olen)
>  			return -1;
>  
> -		memcpy(&opts[*type].c, val, *olen);
> +		iov_to_buf(&data.iov[0], data.cnt, data.off + opt_off + 2,
> +			   &opts[*type].c, *olen);
>  		opts[*type].clen = *olen;
>  		opt_off += *olen + 2;
>  	}
> diff --git a/iov.c b/iov.c
> index 0c69379316aa..66f15585a61a 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -253,7 +253,6 @@ bool iov_drop(struct iov_tail *tail, size_t len)
>   * Returns:	pointer to the data in the tail, NULL if the
>   *		tail doesn't contains @len bytes.
>   */
> -/* cppcheck-suppress unusedFunction */
>  void *iov_tail_ptr(struct iov_tail *tail, size_t off, size_t len)
>  {
>  	const struct iovec *iov;

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