netdev - Re: [PATCH v3 2/2] net: core: support XDP generic on stacked devices.

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]

Message-ID: <20190523131544.6d8a28f7@hermes.lan>
Date:   Thu, 23 May 2019 13:15:44 -0700
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Saeed Mahameed <saeedm@...lanox.com>
Cc:     "jiri@...nulli.us" <jiri@...nulli.us>,
        "jesper.brouer@...il.com" <jesper.brouer@...il.com>,
        "sthemmin@...rosoft.com" <sthemmin@...rosoft.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH v3 2/2] net: core: support XDP generic on stacked
 devices.

On Thu, 23 May 2019 19:19:40 +0000
Saeed Mahameed <saeedm@...lanox.com> wrote:

> On Thu, 2019-05-23 at 10:54 -0700, Stephen Hemminger wrote:
> > When a device is stacked like (team, bonding, failsafe or netvsc) the
> > XDP generic program for the parent device was not called.
> > 
> > Move the call to XDP generic inside __netif_receive_skb_core where
> > it can be done multiple times for stacked case.
> > 
> > Suggested-by: Jiri Pirko <jiri@...nulli.us>
> > Fixes: d445516966dc ("net: xdp: support xdp generic on virtual
> > devices")
> > Signed-off-by: Stephen Hemminger <sthemmin@...rosoft.com>
> > ---
> > v1 - call xdp_generic in netvsc handler
> > v2 - do xdp_generic in generic rx handler processing
> > v3 - move xdp_generic call inside the another pass loop
> > 
> >  net/core/dev.c | 56 ++++++++++------------------------------------
> > ----
> >  1 file changed, 11 insertions(+), 45 deletions(-)
> > 
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index b6b8505cfb3e..696776e14d00 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -4502,23 +4502,6 @@ static int netif_rx_internal(struct sk_buff
> > *skb)
> >  
> >  	trace_netif_rx(skb);
> >  
> > -	if (static_branch_unlikely(&generic_xdp_needed_key)) {
> > -		int ret;
> > -
> > -		preempt_disable();
> > -		rcu_read_lock();
> > -		ret = do_xdp_generic(rcu_dereference(skb->dev-  
> > >xdp_prog), skb);  
> > -		rcu_read_unlock();
> > -		preempt_enable();
> > -
> > -		/* Consider XDP consuming the packet a success from
> > -		 * the netdev point of view we do not want to count
> > -		 * this as an error.
> > -		 */
> > -		if (ret != XDP_PASS)
> > -			return NET_RX_SUCCESS;
> > -	}
> > -  
> 
> Adding Jesper, 
> 
> There is a small behavioral change due to this patch, 
> the XDP program after this patch will run on the RPS CPU, if
> configured, which could cause some behavioral changes in
> xdp_redirect_cpu: bpf_redirect_map(cpu_map).
> 
> Maybe this is acceptable, but it should be documented, as the current
> assumption dictates: XDP program runs on the core where the XDP
> frame/SKB was first seen.

Or maybe XDP should just force off RPS (like it does gro)