Received-SPF: pass (google.com: domain of linux-kernel+bounces-90098-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <1709118356-133960-1-git-send-email-wangyunjian@huawei.com>
In-Reply-To: <1709118356-133960-1-git-send-email-wangyunjian@huawei.com>
From: Jason Wang <jasowang@redhat.com>
Date: Mon, 4 Mar 2024 14:55:53 +0800
Message-ID: <CACGkMEv+=k+RvbN2kWp85f9NWPYOPQtqkThdjvOrf5mWonBqvw@mail.gmail.com>
Subject: Re: [PATCH net-next v2 3/3] tun: AF_XDP Tx zero-copy support
To: Yunjian Wang <wangyunjian@huawei.com>
Cc: mst@redhat.com, willemdebruijn.kernel@gmail.com, kuba@kernel.org, 
	bjorn@kernel.org, magnus.karlsson@intel.com, maciej.fijalkowski@intel.com, 
	jonathan.lemon@gmail.com, davem@davemloft.net, bpf@vger.kernel.org, 
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, 
	virtualization@lists.linux.dev, xudingke@huawei.com, liwei395@huawei.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Feb 28, 2024 at 7:06=E2=80=AFPM Yunjian Wang <wangyunjian@huawei.co=
m> wrote:
>
> This patch set allows TUN to support the AF_XDP Tx zero-copy feature,
> which can significantly reduce CPU utilization for XDP programs.
>
> Since commit fc72d1d54dd9 ("tuntap: XDP transmission"), the pointer
> ring has been utilized to queue different types of pointers by encoding
> the type into the lower bits. Therefore, we introduce a new flag,
> TUN_XDP_DESC_FLAG(0x2UL), which allows us to enqueue XDP descriptors
> and differentiate them from XDP buffers and sk_buffs. Additionally, a
> spin lock is added for enabling and disabling operations on the xsk pool.
>
> The performance testing was performed on a Intel E5-2620 2.40GHz machine.
> Traffic were generated/send through TUN(testpmd txonly with AF_XDP)
> to VM (testpmd rxonly in guest).
>
> +------+---------+---------+---------+
> |      |   copy  |zero-copy| speedup |
> +------+---------+---------+---------+
> | UDP  |   Mpps  |   Mpps  |    %    |
> | 64   |   2.5   |   4.0   |   60%   |
> | 512  |   2.1   |   3.6   |   71%   |
> | 1024 |   1.9   |   3.3   |   73%   |
> +------+---------+---------+---------+
>
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> ---
>  drivers/net/tun.c      | 177 +++++++++++++++++++++++++++++++++++++++--
>  drivers/vhost/net.c    |   4 +
>  include/linux/if_tun.h |  32 ++++++++
>  3 files changed, 208 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index bc80fc1d576e..7f4ff50b532c 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -63,6 +63,7 @@
>  #include <net/rtnetlink.h>
>  #include <net/sock.h>
>  #include <net/xdp.h>
> +#include <net/xdp_sock_drv.h>
>  #include <net/ip_tunnels.h>
>  #include <linux/seq_file.h>
>  #include <linux/uio.h>
> @@ -86,6 +87,7 @@ static void tun_default_link_ksettings(struct net_devic=
e *dev,
>                                        struct ethtool_link_ksettings *cmd=
);
>
>  #define TUN_RX_PAD (NET_IP_ALIGN + NET_SKB_PAD)
> +#define TUN_XDP_BATCH 64
>
>  /* TUN device flags */
>
> @@ -146,6 +148,9 @@ struct tun_file {
>         struct tun_struct *detached;
>         struct ptr_ring tx_ring;
>         struct xdp_rxq_info xdp_rxq;
> +       struct xsk_buff_pool *xsk_pool;
> +       spinlock_t pool_lock;   /* Protects xsk pool enable/disable */
> +       u32 nb_descs;
>  };
>
>  struct tun_page {
> @@ -614,6 +619,8 @@ void tun_ptr_free(void *ptr)
>                 struct xdp_frame *xdpf =3D tun_ptr_to_xdp(ptr);
>
>                 xdp_return_frame(xdpf);
> +       } else if (tun_is_xdp_desc_frame(ptr)) {
> +               return;
>         } else {
>                 __skb_array_destroy_skb(ptr);
>         }
> @@ -631,6 +638,37 @@ static void tun_queue_purge(struct tun_file *tfile)
>         skb_queue_purge(&tfile->sk.sk_error_queue);
>  }
>
> +static void tun_set_xsk_pool(struct tun_file *tfile, struct xsk_buff_poo=
l *pool)
> +{
> +       if (!pool)
> +               return;
> +
> +       spin_lock(&tfile->pool_lock);
> +       xsk_pool_set_rxq_info(pool, &tfile->xdp_rxq);
> +       tfile->xsk_pool =3D pool;
> +       spin_unlock(&tfile->pool_lock);
> +}
> +
> +static void tun_clean_xsk_pool(struct tun_file *tfile)
> +{
> +       spin_lock(&tfile->pool_lock);
> +       if (tfile->xsk_pool) {
> +               void *ptr;
> +
> +               while ((ptr =3D ptr_ring_consume(&tfile->tx_ring)) !=3D N=
ULL)
> +                       tun_ptr_free(ptr);
> +
> +               if (tfile->nb_descs) {
> +                       xsk_tx_completed(tfile->xsk_pool, tfile->nb_descs=
);
> +                       if (xsk_uses_need_wakeup(tfile->xsk_pool))
> +                               xsk_set_tx_need_wakeup(tfile->xsk_pool);
> +                       tfile->nb_descs =3D 0;
> +               }
> +               tfile->xsk_pool =3D NULL;
> +       }
> +       spin_unlock(&tfile->pool_lock);
> +}
> +
>  static void __tun_detach(struct tun_file *tfile, bool clean)
>  {
>         struct tun_file *ntfile;
> @@ -648,6 +686,11 @@ static void __tun_detach(struct tun_file *tfile, boo=
l clean)
>                 u16 index =3D tfile->queue_index;
>                 BUG_ON(index >=3D tun->numqueues);
>
> +               ntfile =3D rtnl_dereference(tun->tfiles[tun->numqueues - =
1]);
> +               /* Stop xsk zc xmit */
> +               tun_clean_xsk_pool(tfile);
> +               tun_clean_xsk_pool(ntfile);
> +
>                 rcu_assign_pointer(tun->tfiles[index],
>                                    tun->tfiles[tun->numqueues - 1]);
>                 ntfile =3D rtnl_dereference(tun->tfiles[index]);
> @@ -668,6 +711,7 @@ static void __tun_detach(struct tun_file *tfile, bool=
 clean)
>                 tun_flow_delete_by_queue(tun, tun->numqueues + 1);
>                 /* Drop read queue */
>                 tun_queue_purge(tfile);
> +               tun_set_xsk_pool(ntfile, xsk_get_pool_from_qid(tun->dev, =
index));
>                 tun_set_real_num_queues(tun);
>         } else if (tfile->detached && clean) {
>                 tun =3D tun_enable_queue(tfile);
> @@ -801,6 +845,7 @@ static int tun_attach(struct tun_struct *tun, struct =
file *file,
>
>                 if (tfile->xdp_rxq.queue_index    !=3D tfile->queue_index=
)
>                         tfile->xdp_rxq.queue_index =3D tfile->queue_index=
;
> +               tun_set_xsk_pool(tfile, xsk_get_pool_from_qid(dev, tfile-=
>queue_index));
>         } else {
>                 /* Setup XDP RX-queue info, for new tfile getting attache=
d */
>                 err =3D xdp_rxq_info_reg(&tfile->xdp_rxq,
> @@ -1221,11 +1266,50 @@ static int tun_xdp_set(struct net_device *dev, st=
ruct bpf_prog *prog,
>         return 0;
>  }
>
> +static int tun_xsk_pool_enable(struct net_device *netdev,
> +                              struct xsk_buff_pool *pool,
> +                              u16 qid)
> +{
> +       struct tun_struct *tun =3D netdev_priv(netdev);
> +       struct tun_file *tfile;
> +
> +       if (qid >=3D tun->numqueues)
> +               return -EINVAL;
> +
> +       tfile =3D rtnl_dereference(tun->tfiles[qid]);
> +       tun_set_xsk_pool(tfile, pool);
> +
> +       return 0;
> +}
> +
> +static int tun_xsk_pool_disable(struct net_device *netdev, u16 qid)
> +{
> +       struct tun_struct *tun =3D netdev_priv(netdev);
> +       struct tun_file *tfile;
> +
> +       if (qid >=3D MAX_TAP_QUEUES)
> +               return -EINVAL;
> +
> +       tfile =3D rtnl_dereference(tun->tfiles[qid]);
> +       if (tfile)
> +               tun_clean_xsk_pool(tfile);
> +       return 0;
> +}
> +
> +static int tun_xsk_pool_setup(struct net_device *dev, struct xsk_buff_po=
ol *pool,
> +                             u16 qid)
> +{
> +       return pool ? tun_xsk_pool_enable(dev, pool, qid) :
> +               tun_xsk_pool_disable(dev, qid);
> +}
> +
>  static int tun_xdp(struct net_device *dev, struct netdev_bpf *xdp)
>  {
>         switch (xdp->command) {
>         case XDP_SETUP_PROG:
>                 return tun_xdp_set(dev, xdp->prog, xdp->extack);
> +       case XDP_SETUP_XSK_POOL:
> +               return tun_xsk_pool_setup(dev, xdp->xsk.pool, xdp->xsk.qu=
eue_id);
>         default:
>                 return -EINVAL;
>         }
> @@ -1330,6 +1414,19 @@ static int tun_xdp_tx(struct net_device *dev, stru=
ct xdp_buff *xdp)
>         return nxmit;
>  }
>
> +static int tun_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags)
> +{
> +       struct tun_struct *tun =3D netdev_priv(dev);
> +       struct tun_file *tfile;
> +
> +       rcu_read_lock();
> +       tfile =3D rcu_dereference(tun->tfiles[qid]);
> +       if (tfile)
> +               __tun_xdp_flush_tfile(tfile);
> +       rcu_read_unlock();
> +       return 0;
> +}

I may miss something but why not simply queue xdp frames into ptr ring
then we don't need tricks for peek?

Thanks