Re: [PATCH RFC net-next 2/4] xdp: return status from generic_xdp_tx()

["Björn Töpel" <bjorn@kernel.org>]

Maciej Fijalkowski <maciej.fijalkowski@intel.com> writes:

> Once veth will start to use __do_xdp_generic(), this will be helpful for
> correct stats values being incremented - otherwise we would not have a
> knowledge whether XDP_TX succeeded or not. With this being implemented we
> will only bump xdp_tx stat when actual transmission happened.
>
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
>  include/linux/netdevice.h |  2 +-
>  kernel/bpf/devmap.c       |  2 +-
>  net/core/dev.c            | 11 +++++++++--
>  net/core/filter.c         |  2 +-
>  4 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 47417b2d48a4..473b18b0bb63 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -4255,7 +4255,7 @@ static inline void dev_consume_skb_any(struct sk_buff *skb)
>  
>  u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
>  			     const struct bpf_prog *xdp_prog);
> -void generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog);
> +int generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog);
>  int do_xdp_generic(const struct bpf_prog *xdp_prog, struct sk_buff **pskb);
>  int netif_rx(struct sk_buff *skb);
>  int __netif_rx(struct sk_buff *skb);
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index 3d619d01088e..b7a4a0266345 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -714,7 +714,7 @@ int dev_map_generic_redirect(struct bpf_dtab_netdev *dst, struct sk_buff *skb,
>  		return 0;
>  
>  	skb->dev = dst->dev;
> -	generic_xdp_tx(skb, xdp_prog);
> +	(void)generic_xdp_tx(skb, xdp_prog);

Why not propagate the error now that we can?

>  
>  	return 0;
>  }
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6cc2a5bed20f..09012cdea376 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5628,11 +5628,12 @@ static u32 netif_receive_generic_xdp(struct sk_buff **pskb,
>   * and DDOS attacks will be more effective. In-driver-XDP use dedicated TX
>   * queues, so they do not have this starvation issue.
>   */
> -void generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog)
> +int generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog)
>  {
>  	struct net_device *dev = skb->dev;
>  	struct netdev_queue *txq;
>  	bool free_skb = true;
> +	int err = -ENETDOWN;
>  	int cpu, rc;
>  
>  	txq = netdev_core_pick_tx(dev, skb, NULL);
> @@ -5640,8 +5641,12 @@ void generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog)
>  	HARD_TX_LOCK(dev, txq, cpu);
>  	if (!netif_xmit_frozen_or_drv_stopped(txq)) {
>  		rc = netdev_start_xmit(skb, dev, txq, 0);
> -		if (dev_xmit_complete(rc))
> +		if (dev_xmit_complete(rc)) {
>  			free_skb = false;
> +			err = 0;
> +		} else {
> +			err = -EBUSY;
> +		}
>  	}
>  	HARD_TX_UNLOCK(dev, txq);
>  	if (free_skb) {
> @@ -5649,6 +5654,8 @@ void generic_xdp_tx(struct sk_buff *skb, const struct bpf_prog *xdp_prog)
>  		dev_core_stats_tx_dropped_inc(dev);
>  		kfree_skb(skb);
>  	}
> +
> +	return err;
>  }
>  
>  static DEFINE_STATIC_KEY_FALSE(generic_xdp_needed_key);
> diff --git a/net/core/filter.c b/net/core/filter.c
> index bf9c37b27646..d6d14e0a1e35 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4619,7 +4619,7 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
>  
>  		skb->dev = fwd;
>  		_trace_xdp_redirect(dev, xdp_prog, ri->tgt_index);
> -		generic_xdp_tx(skb, xdp_prog);
> +		(void)generic_xdp_tx(skb, xdp_prog);


Dito.


Björn