Re: xdp generic default option

[Jesper Dangaard Brouer <brouer@redhat.com>]

On Wed, 19 Aug 2020 13:57:51 -0700
Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:

> On Wed, Aug 19, 2020 at 2:29 AM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >
> > Hi Andrii,
> >
> > working on xdp multi-buff I figured out now xdp generic is the default choice
> > if not specified by userspace. In particular after commit 7f0a838254bd
> > ("bpf, xdp: Maintain info on attached XDP BPF programs in net_device"), running
> > the command below, XDP will run in generic mode even if the underlay driver
> > support XDP in native mode:
> >
> > $ip link set dev eth0 xdp obj prog.o
> > $ip link show dev eth0
> > 2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdpgeneric qdisc mq state UP mode DEFAULT
> >    group default qlen 1024
> >    link/ether f0:ad:4e:09:6b:57 brd ff:ff:ff:ff:ff:ff
> >    prog/xdp id 1 tag 3b185187f1855c4c jited
> >
> > Is it better to use xdpdrv as default choice if not specified by userspace?
> > doing something like:
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index a00aa737ce29..1f85880ee412 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -8747,9 +8747,9 @@ static enum bpf_xdp_mode dev_xdp_mode(u32 flags)
> >  {
> >         if (flags & XDP_FLAGS_HW_MODE)
> >                 return XDP_MODE_HW;
> > -       if (flags & XDP_FLAGS_DRV_MODE)
> > -               return XDP_MODE_DRV;
> > -       return XDP_MODE_SKB;
> > +       if (flags & XDP_FLAGS_SKB_MODE)
> > +               return XDP_MODE_SKB;
> > +       return XDP_MODE_DRV;
> >  }
> >  
> 
> I think the better way would be to choose XDP_MODE_DRV if ndo_bpf !=
> NULL and XDP_MODE_SKB otherwise. That seems to be matching original
> behavior, no?

Yes, but this silent fallback to XDP_MODE_SKB (generic-XDP) have
cause a lot of support issues in the past.  I wish we could change it.
We already changed all the samples/bpf/ to ask for XDP_FLAGS_DRV_MODE,
so they behave this way.

d50ecc46d18f ("samples/bpf: Attach XDP programs in driver mode by default")
 https://git.kernel.org/torvalds/c/d50ecc46d18fa

> It was not my intent to change the behavior, sorry about that. I'll
> post patch a bit later today.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer