Re: [PATCH] tap: fix seq->p.count limit

[Stefano Brivio <sbrivio@redhat.com>]

On Sun, 13 Aug 2023 15:17:37 +0200
Stefano Brivio <sbrivio@redhat.com> wrote:

> On Wed,  9 Aug 2023 11:23:42 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
> > The number of items in pool_l4_t is defined to UIO_MAXIOV,
> > not TAP_SEQS. TAP_SEQS is the number of the messages.  
> 
> ...sequences of packets (within the same connection), rather than
> "messages" (which might sound like packets).
> 
> > Fix the value used to compare seq->p.count with.
> > 
> > Fix: bb708111833e ("treewide: Packet abstraction with mandatory boundary checks")
> > Signed-off-by: Laurent Vivier <lvivier@redhat.com>  
> 
> I was wondering why throughput tests on the tap path, namespace to
> host, started failing miserably with this (0.4 Gbps instead of 7 Gbps
> with small packets).
> 
> It turned out that yes, the pool has UIO_MAXIOV items, but (also by
> mistake in some sense) we initialised only the first TAP_SEQS ones. This
> fixes it:
> 
> diff --git a/tap.c b/tap.c
> index 7d5dd6a..a6f8692 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -1258,8 +1258,8 @@ void tap_sock_init(struct ctx *c)
>         pool_tap6_storage = PACKET_INIT(pool_tap6, TAP_MSGS, pkt_buf, sz);
>  
>         for (i = 0; i < TAP_SEQS; i++) {
> -               tap4_l4[i].p = PACKET_INIT(pool_l4, TAP_SEQS, pkt_buf, sz);
> -               tap6_l4[i].p = PACKET_INIT(pool_l4, TAP_SEQS, pkt_buf, sz);
> +               tap4_l4[i].p = PACKET_INIT(pool_l4, UIO_MAXIOV, pkt_buf, sz);
> +               tap6_l4[i].p = PACKET_INIT(pool_l4, UIO_MAXIOV, pkt_buf, sz);
>         }
>  
>         if (c->fd_tap != -1) { /* Passed as --fd */
> 
> ...I would simply apply it on top.

Applied (with this change), thanks!

-- 
Stefano