Hi Lorenzo,
On Sun, 28 Jan 2024 at 16:22, Lorenzo Bianconi <lorenzo@xxxxxxxxxx> wrote:
>
> Rely on skb pointer reference instead of the skb pointer in do_xdp_generic and
> netif_receive_generic_xdp routine signatures. This is a preliminary patch to add
> multi-buff support for xdp running in generic mode.
The patch looks fine, but can we tweak the commit message explaining
in more detail why this is needed?
Thanks
/Ilias
>
> Acked-by: Jesper Dangaard Brouer <hawk@xxxxxxxxxx>
> Signed-off-by: Lorenzo Bianconi <lorenzo@xxxxxxxxxx>
> ---
> drivers/net/tun.c | 4 ++--
> include/linux/netdevice.h | 2 +-
> net/core/dev.c | 16 +++++++++-------
> 3 files changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 4a4f8c8e79fa..5bd98bdaddf2 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1927,7 +1927,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> rcu_read_lock();
> xdp_prog = rcu_dereference(tun->xdp_prog);
> if (xdp_prog) {
> - ret = do_xdp_generic(xdp_prog, skb);
> + ret = do_xdp_generic(xdp_prog, &skb);
> if (ret != XDP_PASS) {
> rcu_read_unlock();
> local_bh_enable();
> @@ -2517,7 +2517,7 @@ static int tun_xdp_one(struct tun_struct *tun,
> skb_record_rx_queue(skb, tfile->queue_index);
>
> if (skb_xdp) {
> - ret = do_xdp_generic(xdp_prog, skb);
> + ret = do_xdp_generic(xdp_prog, &skb);
> if (ret != XDP_PASS) {
> ret = 0;
> goto out;
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 118c40258d07..7eee99a58200 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -3958,7 +3958,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,
> struct bpf_prog *xdp_prog);
> void generic_xdp_tx(struct sk_buff *skb, struct bpf_prog *xdp_prog);
> -int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff *skb);
> +int do_xdp_generic(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/net/core/dev.c b/net/core/dev.c
> index bf9ec740b09a..960f39ac5e33 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4924,10 +4924,11 @@ u32 bpf_prog_run_generic_xdp(struct sk_buff *skb, struct xdp_buff *xdp,
> return act;
> }
>
> -static u32 netif_receive_generic_xdp(struct sk_buff *skb,
> +static u32 netif_receive_generic_xdp(struct sk_buff **pskb,
> struct xdp_buff *xdp,
> struct bpf_prog *xdp_prog)
> {
> + struct sk_buff *skb = *pskb;
> u32 act = XDP_DROP;
>
> /* Reinjected packets coming from act_mirred or similar should
> @@ -5008,24 +5009,24 @@ void generic_xdp_tx(struct sk_buff *skb, struct bpf_prog *xdp_prog)
>
> static DEFINE_STATIC_KEY_FALSE(generic_xdp_needed_key);
>
> -int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff *skb)
> +int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff **pskb)
> {
> if (xdp_prog) {
> struct xdp_buff xdp;
> u32 act;
> int err;
>
> - act = netif_receive_generic_xdp(skb, &xdp, xdp_prog);
> + act = netif_receive_generic_xdp(pskb, &xdp, xdp_prog);
> if (act != XDP_PASS) {
> switch (act) {
> case XDP_REDIRECT:
> - err = xdp_do_generic_redirect(skb->dev, skb,
> + err = xdp_do_generic_redirect((*pskb)->dev, *pskb,
> &xdp, xdp_prog);
> if (err)
> goto out_redir;
> break;
> case XDP_TX:
> - generic_xdp_tx(skb, xdp_prog);
> + generic_xdp_tx(*pskb, xdp_prog);
> break;
> }
> return XDP_DROP;
> @@ -5033,7 +5034,7 @@ int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff *skb)
> }
> return XDP_PASS;
> out_redir:
> - kfree_skb_reason(skb, SKB_DROP_REASON_XDP);
> + kfree_skb_reason(*pskb, SKB_DROP_REASON_XDP);
> return XDP_DROP;
> }
> EXPORT_SYMBOL_GPL(do_xdp_generic);
> @@ -5356,7 +5357,8 @@ static int __netif_receive_skb_core(struct sk_buff **pskb, bool pfmemalloc,
> int ret2;
>
> migrate_disable();
> - ret2 = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog), skb);
> + ret2 = do_xdp_generic(rcu_dereference(skb->dev->xdp_prog),
> + &skb);
> migrate_enable();
>
> if (ret2 != XDP_PASS) {
> --
> 2.43.0
>
