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

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 23 May 2019 19:19:40 +0000
Saeed Mahameed <saeedm@mellanox.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@resnulli.us>
> > Fixes: d445516966dc ("net: xdp: support xdp generic on virtual
> > devices")
> > Signed-off-by: Stephen Hemminger <sthemmin@microsoft.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)