netdev - Re: Co-existing XDP generic and native mode? (Re: [PATCH bpf-next v5 5/8] xdp: Provide extack messages when prog attachment failed)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 1 Feb 2019 10:47:38 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     daniel@...earbox.net, ast@...nel.org,
        David Miller <davem@...emloft.net>,
        Maciej Fijalkowski <maciejromanfijalkowski@...il.com>,
        netdev@...r.kernel.org, john.fastabend@...il.com,
        David Ahern <dsahern@...il.com>,
        Saeed Mahameed <saeedm@...lanox.com>
Subject: Re: Co-existing XDP generic and native mode? (Re: [PATCH bpf-next
 v5 5/8] xdp: Provide extack messages when prog attachment failed)

On Fri, 1 Feb 2019 08:02:36 +0100, Jesper Dangaard Brouer wrote:
> On Thu, 31 Jan 2019 19:11:01 -0800
> Jakub Kicinski <jakub.kicinski@...ronome.com> wrote:
> 
> > On Fri,  1 Feb 2019 01:19:51 +0100, Maciej Fijalkowski wrote:  
> > >  		if (__dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG) ||
> > > -		    __dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG_HW))
> > > +		    __dev_xdp_query(dev, bpf_chk, XDP_QUERY_PROG_HW)) {
> > > +			NL_SET_ERR_MSG(extack, "native and generic XDP can't be active at the same time");
> > >  			return -EEXIST;
> > > +		}    
> > 
> > This reminds me, since we allowed native/driver and offloaded XDP
> > programs to coexist in a25717d2b604 ("xdp: support simultaneous 
> > driver and hw XDP attachment") I got an internal feature request 
> > to also allow generic and native mode.  Would anyone object to that?  
> 
> Well, I will object ;-)
> 
> I have two refactor ideas [1] and [2], that depend on not allowing
> XDP-native and XDP-generic to co-exist.   The general idea is to let
> XDP-native use the same fields in net_device->rx[] as XDP-generic given
> they (currently) cannot co-exist. 
>  The goal is (1) to move stuff out of driver code, and (2) hopefully
> make it easier to implement per RXq XDP progs.

You mean you'd use one pointer to keep the prog in the RXQ structure?
Then some from from of an extra flag will be necessary to distinguish?
I.e.:
 if (rxq->prog && rxq->is_native)
	/* got_prog */

rather than:
 if (rxq->native_prog)
	/* got_prog */
 
The cost of this reuse would be a read-only cache line per-q when XDP is
not enabled.  Right now drivers have the ability to pack the XDP prog
into a structure which is in cache already, and don't need to bring the
entire RXQ structure out (which is cache line aligned so driver authors
can't do anything to place it cleverly).

No doubt, thought, that if we allow both to be enabled we will have to
bloat the data structures.

> These are only refactor ideas, so if you can argue why your internal
> feature request for simultaneous generic and native make more sense,
> then I'm open for allowing this ?

The request was actually to enable xdpoffload and xdpgeneric at the
same time.  I'm happy to have that as another HW offload exclusive
for now :)

> [1] https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp_per_rxq01.org#refactor-idea-move-xdp_rxq_info-to-net_devicenetdev_rx_queue
> 
> [2] https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp_per_rxq01.org#refactor-idea-xdpbpf_prog-into-netdev_rx_queuenet_device
> 
> > Apart from a touch up to test_offload.py I don't think anything 
> > would care.  netlink can already carry multiple IDs, iproute2
> > understands it, too..  
> 
> And we did notice you added support for HW+native:
>  [3] https://github.com/xdp-project/xdp-project/blob/master/areas/core/xdp_per_rxq01.org