Subject: Re: [PATCH] net/xen-netback: Break build if netback slots > max_skbs + 1

From: Jakub Kicinski <[email-masked]>
Date: Thu, 5 Oct 2023 09:03:28 -0700

On Thu, 5 Oct 2023 18:39:51 +0300 David Kahurani wrote:
> > MAX_SKB_FRAGS can now be set via Kconfig, this allows us to create
> > larger super-packets. Can XEN_NETBK_LEGACY_SLOTS_MAX be made relative
> > to MAX_SKB_FRAGS, or does the number have to match between guest and
> > host?  
> 
> Historically, netback driver allows for a maximum of 18 fragments.
> With recent changes, it also relies on the assumption that the
> difference between MAX_SKB_FRAGS and XEN_NETBK_LEGACY_SLOTS_MAX is one
> and MAX_SKB_FRAGS is the lesser value.
> 
> Now, look at Ubuntu kernel for instance( a change has been made and,
> presumably, with good reason so we have reason to assume that the
> change will persist in future releases).
> 
> /* To allow 64K frame to be packed as single skb without frag_list we
>  * require 64K/PAGE_SIZE pages plus 1 additional page to allow for
>  * buffers which do not start on a page boundary.
>  *
>  * Since GRO uses frags we allocate at least 16 regardless of page
>  * size.
>  */
> #if (65536/PAGE_SIZE + 1) < 16
> #define MAX_SKB_FRAGS 16UL
> #else
> #define MAX_SKB_FRAGS (65536/PAGE_SIZE + 1)
> #endif
> 
> So, MAX_SKB_FRAGS can sometimes be 16. This is exactly what we're
> trying to avoid with this patch. I host running with this change is
> vulnerable to attack by the guest(though, this will only happen when
> PAGE_SIZE > 4096).

My bad, you're protecting from the inverse condition than I thought.

But to be clear the code you're quoting (the defines for MAX_SKB_FRAGS)
are what has been there upstream forever until 3948b059 was merged.
Not 100% sure why 3948b059 switched the min from 16 to 17, I think it
was just to keep consistency between builds.

If this change gets backported to 6.1 stable it will break ppc build
of stable, right? Since ppc has 64k pages.