Re: [PATCH 1/3] tap: specify the packet pool

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

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

On Tue, Apr 01, 2025 at 01:38:07PM +0200, Eugenio Pérez wrote:
> In vhost-kernel we need to allocate a storage for rx so kernel can
> write the buffers async. We need to tell tap_add_packet where to find
> them.
> 
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>

As discussed on our call, I don't think we need this.  We should be
able to re-use pool_tap[46] for vhost-net.

> ---
>  tap.c       | 34 ++++++++++++++++++++++++----------
>  tap.h       |  4 ++--
>  vu_common.c |  7 ++++---
>  3 files changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/tap.c b/tap.c
> index 182a115..ce859ba 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -1031,10 +1031,16 @@ void tap_flush_pools(void)
>   * @c:		Execution context
>   * @now:	Current timestamp
>   */
> -void tap_handler(struct ctx *c, const struct timespec *now)
> +void tap_handler(struct ctx *c, const struct timespec *now, struct pool *pool4,
> +		struct pool *pool6)
>  {
> -	tap4_handler(c, pool_tap4, now);
> -	tap6_handler(c, pool_tap6, now);
> +	if (!pool4)
> +		pool4 = pool_tap4;
> +	if (!pool6)
> +		pool6 = pool_tap6;
> +
> +	tap4_handler(c, pool4, now);
> +	tap6_handler(c, pool6, now);
>  }
>  
>  /**
> @@ -1042,11 +1048,19 @@ void tap_handler(struct ctx *c, const struct timespec *now)
>   * @c:		Execution context
>   * @l2len:	Total L2 packet length
>   * @p:		Packet buffer
> + * @pool4	Pool for tap ipv4 packets. If NULL, is pool_tap4
> + * @pool6	Pool for tap ipv6 packets. If NULL, is pool_tap6
>   */
> -void tap_add_packet(struct ctx *c, ssize_t l2len, char *p)
> +void tap_add_packet(struct ctx *c, ssize_t l2len, char *p,
> +					struct pool *pool4, struct pool *pool6)
>  {
>  	const struct ethhdr *eh;
>  
> +	if (!pool4)
> +		pool4 = pool_tap4;
> +	if (!pool6)
> +		pool6 = pool_tap6;
> +
>  	pcap(p, l2len);
>  
>  	eh = (struct ethhdr *)p;
> @@ -1059,10 +1073,10 @@ void tap_add_packet(struct ctx *c, ssize_t l2len, char *p)
>  	switch (ntohs(eh->h_proto)) {
>  	case ETH_P_ARP:
>  	case ETH_P_IP:
> -		packet_add(pool_tap4, l2len, p);
> +		packet_add(pool4, l2len, p);
>  		break;
>  	case ETH_P_IPV6:
> -		packet_add(pool_tap6, l2len, p);
> +		packet_add(pool6, l2len, p);
>  		break;
>  	default:
>  		break;
> @@ -1142,7 +1156,7 @@ static void tap_passt_input(struct ctx *c, const struct timespec *now)
>  		p += sizeof(uint32_t);
>  		n -= sizeof(uint32_t);
>  
> -		tap_add_packet(c, l2len, p);
> +		tap_add_packet(c, l2len, p, pool_tap4, pool_tap6);
>  
>  		p += l2len;
>  		n -= l2len;
> @@ -1151,7 +1165,7 @@ static void tap_passt_input(struct ctx *c, const struct timespec *now)
>  	partial_len = n;
>  	partial_frame = p;
>  
> -	tap_handler(c, now);
> +	tap_handler(c, now, NULL, NULL);
>  }
>  
>  /**
> @@ -1207,10 +1221,10 @@ static void tap_pasta_input(struct ctx *c, const struct timespec *now)
>  		    len > (ssize_t)L2_MAX_LEN_PASTA)
>  			continue;
>  
> -		tap_add_packet(c, len, pkt_buf + n);
> +		tap_add_packet(c, len, pkt_buf + n, pool_tap4, pool_tap6);
>  	}
>  
> -	tap_handler(c, now);
> +	tap_handler(c, now, NULL, NULL);
>  }
>  
>  /**
> diff --git a/tap.h b/tap.h
> index dd39fd8..0b5ad17 100644
> --- a/tap.h
> +++ b/tap.h
> @@ -118,7 +118,7 @@ void tap_sock_reset(struct ctx *c);
>  void tap_sock_update_pool(void *base, size_t size);
>  void tap_backend_init(struct ctx *c);
>  void tap_flush_pools(void);
> -void tap_handler(struct ctx *c, const struct timespec *now);
> -void tap_add_packet(struct ctx *c, ssize_t l2len, char *p);
> +void tap_handler(struct ctx *c, const struct timespec *now, struct pool *pool4, struct pool *pool6);
> +void tap_add_packet(struct ctx *c, ssize_t l2len, char *p, struct pool *pool4, struct pool *pool6);
>  
>  #endif /* TAP_H */
> diff --git a/vu_common.c b/vu_common.c
> index 686a09b..4fe982f 100644
> --- a/vu_common.c
> +++ b/vu_common.c
> @@ -191,7 +191,7 @@ static void vu_handle_tx(struct vu_dev *vdev, int index,
>  			tap_add_packet(vdev->context,
>  				       elem[count].out_sg[0].iov_len - hdrlen,
>  				       (char *)elem[count].out_sg[0].iov_base +
> -				        hdrlen);
> +				        hdrlen, NULL, NULL);
>  		} else {
>  			/* vnet header can be in a separate iovec */
>  			if (elem[count].out_num != 2) {
> @@ -203,13 +203,14 @@ static void vu_handle_tx(struct vu_dev *vdev, int index,
>  			} else {
>  				tap_add_packet(vdev->context,
>  					       elem[count].out_sg[1].iov_len,
> -					       (char *)elem[count].out_sg[1].iov_base);
> +					       (char *)elem[count].out_sg[1].iov_base,
> +						   NULL, NULL);
>  			}
>  		}
>  
>  		count++;
>  	}
> -	tap_handler(vdev->context, now);
> +	tap_handler(vdev->context, now, NULL, NULL);
>  
>  	if (count) {
>  		int i;

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