| 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
| ||
|
Date: Fri, 21 Dec 2018 16:04:10 -0800
From: William Tu <u9012063@...il.com>
To: Björn Töpel <bjorn.topel@...il.com>
Cc: Magnus Karlsson <magnus.karlsson@...il.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Netdev <netdev@...r.kernel.org>, makita.toshiaki@....ntt.co.jp,
Yi-Hung Wei <yihung.wei@...il.com>,
"Karlsson, Magnus" <magnus.karlsson@...el.com>
Subject: Re: [bpf-next RFCv2 2/3] veth: support AF_XDP.
On Fri, Dec 21, 2018 at 12:38 AM Björn Töpel <bjorn.topel@...il.com> wrote:
>
> Den ons 19 dec. 2018 kl 01:55 skrev William Tu <u9012063@...il.com>:
> >
> > The patch adds support for AF_XDP async xmit. Users can use
> > AF_XDP on both side of the veth and get better performance, with
> > the cost of ksoftirqd doing the xmit. The veth_xsk_async_xmit
> > simply kicks the napi function, veth_poll, to run, and the transmit
> > logic is implemented there.
> >
> > Tested using two namespaces, one runs xdpsock and the other runs
> > xdp_rxq_info. A simple script comparing the performance with/without
> > AF_XDP shows improvement from 724Kpps to 1.1Mpps.
> >
> > ip netns add at_ns0
> > ip link add p0 type veth peer name p1
> > ip link set p0 netns at_ns0
> > ip link set dev p1 up
> > ip netns exec at_ns0 ip link set dev p0 up
> >
> > # receiver
> > ip netns exec at_ns0 xdp_rxq_info --dev p0 --action XDP_DROP
> >
> > # sender
> > xdpsock -i p1 -t -N -z
> > or
> > xdpsock -i p1 -t -S
> >
> > Signed-off-by: William Tu <u9012063@...il.com>
> > ---
> > drivers/net/veth.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 197 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> > index f412ea1cef18..0ce89820ce70 100644
> > --- a/drivers/net/veth.c
> > +++ b/drivers/net/veth.c
> > @@ -25,6 +25,10 @@
> > #include <linux/ptr_ring.h>
> > #include <linux/bpf_trace.h>
> > #include <linux/net_tstamp.h>
> > +#include <net/xdp_sock.h>
> > +#include <linux/mm.h>
> > +#include <linux/slab.h>
> > +#include <net/page_pool.h>
> >
> > #define DRV_NAME "veth"
> > #define DRV_VERSION "1.0"
> > @@ -53,6 +57,8 @@ struct veth_rq {
> > bool rx_notify_masked;
> > struct ptr_ring xdp_ring;
> > struct xdp_rxq_info xdp_rxq;
> > + struct xdp_umem *xsk_umem;
> > + u16 qid;
> > };
> >
> > struct veth_priv {
> > @@ -737,15 +743,108 @@ static int veth_xdp_rcv(struct veth_rq *rq, int budget, unsigned int *xdp_xmit)
> > return done;
> > }
> >
> > +static int veth_xsk_send(struct napi_struct *napi, int budget)
> > +{
> > + struct veth_rq *rq =
> > + container_of(napi, struct veth_rq, xdp_napi);
> > + int done = 0;
> > +
> > + /* tx: use netif_tx_napi_add? */
> > + while (rq->xsk_umem && budget--) {
> > + struct veth_priv *priv, *peer_priv;
> > + struct net_device *dev, *peer_dev;
> > + unsigned int inner_xdp_xmit = 0;
> > + unsigned int metasize = 0;
> > + struct veth_rq *peer_rq;
> > + struct xdp_frame *xdpf;
> > + bool dropped = false;
> > + struct sk_buff *skb;
> > + struct page *page;
> > + void *vaddr;
> > + void *addr;
> > + u32 len;
> > +
> > + if (!xsk_umem_consume_tx_virtual(rq->xsk_umem, &vaddr, &len))
> > + break;
> > +
> > + page = dev_alloc_page();
> > + if (!page) {
> > + pr_warn("veth: page allocation fails\n");
> > + xsk_umem_complete_tx(rq->xsk_umem, 1);
> > + xsk_umem_consume_tx_done(rq->xsk_umem);
> > + return -ENOMEM;
> > + }
> > +
> > + addr = page_to_virt(page);
> > + xdpf = addr;
> > + memset(xdpf, 0, sizeof(*xdpf));
> > +
> > + addr += sizeof(*xdpf);
> > + memcpy(addr, vaddr, len);
> > +
> > + xdpf->data = addr + metasize;
> > + xdpf->len = len;
> > + xdpf->headroom = 0;
> > + xdpf->metasize = metasize;
> > + xdpf->mem.type = MEM_TYPE_PAGE_SHARED;
> > +
> > + /* Invoke peer rq to rcv */
> > + dev = rq->dev;
> > + priv = netdev_priv(dev);
> > + peer_dev = priv->peer;
> > + peer_priv = netdev_priv(peer_dev);
> > + peer_rq = &peer_priv->rq[rq->qid];
> > +
> > + /* put into peer rq */
> > + skb = veth_xdp_rcv_one(peer_rq, xdpf, &inner_xdp_xmit);
> > + if (!skb) {
> > + /* Peer side has XDP program attached */
> > + if (inner_xdp_xmit & VETH_XDP_TX) {
> > + /* Not supported */
> > + pr_warn("veth: peer XDP_TX not supported\n");
> > + xdp_return_frame(xdpf);
> > + dropped = true;
> > + goto skip_tx;
> > + } else if (inner_xdp_xmit & VETH_XDP_REDIR) {
> > + xdp_do_flush_map();
> > + } else {
> > + dropped = true;
> > + }
> > + } else {
> > + /* Peer side has no XDP attached */
> > + napi_gro_receive(&peer_rq->xdp_napi, skb);
> > + }
> > +skip_tx:
> > + xsk_umem_complete_tx(rq->xsk_umem, 1);
> > + xsk_umem_consume_tx_done(rq->xsk_umem);
> > +
> > + /* update peer stats */
> > + u64_stats_update_begin(&peer_rq->stats.syncp);
> > + peer_rq->stats.xdp_packets++;
> > + peer_rq->stats.xdp_bytes += len;
> > + if (dropped)
> > + rq->stats.xdp_drops++;
> > + u64_stats_update_end(&peer_rq->stats.syncp);
> > + done++;
> > + }
> > +
> > + return done;
> > +}
> > +
> > static int veth_poll(struct napi_struct *napi, int budget)
> > {
> > struct veth_rq *rq =
> > container_of(napi, struct veth_rq, xdp_napi);
> > unsigned int xdp_xmit = 0;
> > - int done;
> > + int done = 0;
> > + int tx_done;
> > +
> > + tx_done = veth_xsk_send(napi, budget);
> > + if (tx_done > 0)
> > + done += tx_done;
> >
> > xdp_set_return_frame_no_direct();
> > - done = veth_xdp_rcv(rq, budget, &xdp_xmit);
> > + done += veth_xdp_rcv(rq, budget, &xdp_xmit);
> >
> > if (done < budget && napi_complete_done(napi, done)) {
> > /* Write rx_notify_masked before reading ptr_ring */
> > @@ -776,6 +875,7 @@ static int veth_napi_add(struct net_device *dev)
> > err = ptr_ring_init(&rq->xdp_ring, VETH_RING_SIZE, GFP_KERNEL);
> > if (err)
> > goto err_xdp_ring;
> > + rq->qid = i;
> > }
> >
> > for (i = 0; i < dev->real_num_rx_queues; i++) {
> > @@ -812,6 +912,7 @@ static void veth_napi_del(struct net_device *dev)
> > netif_napi_del(&rq->xdp_napi);
> > rq->rx_notify_masked = false;
> > ptr_ring_cleanup(&rq->xdp_ring, veth_ptr_free);
> > + rq->qid = -1;
> > }
> > }
> >
> > @@ -836,6 +937,7 @@ static int veth_enable_xdp(struct net_device *dev)
> >
> > /* Save original mem info as it can be overwritten */
> > rq->xdp_mem = rq->xdp_rxq.mem;
> > + rq->qid = i;
> > }
> >
> > err = veth_napi_add(dev);
> > @@ -1115,6 +1217,78 @@ static u32 veth_xdp_query(struct net_device *dev)
> > return 0;
> > }
> >
> > +int veth_xsk_umem_query(struct net_device *dev, struct xdp_umem **umem,
> > + u16 qid)
> > +{
> > + struct xdp_umem *queried_umem;
> > +
> > + queried_umem = xdp_get_umem_from_qid(dev, qid);
> > +
> > + if (!queried_umem)
> > + return -EINVAL;
> > +
> > + *umem = queried_umem;
> > + return 0;
> > +}
> > +
> > +static int veth_xsk_umem_enable(struct net_device *dev,
> > + struct xdp_umem *umem,
> > + u16 qid)
> > +{
> > + struct veth_priv *priv = netdev_priv(dev);
> > + struct xdp_umem_fq_reuse *reuseq;
> > + int err = 0;
> > +
> > + if (qid >= dev->real_num_rx_queues)
> > + return -EINVAL;
> > +
> > + reuseq = xsk_reuseq_prepare(priv->rq[0].xdp_ring.size);
> > + if (!reuseq)
> > + return -ENOMEM;
> > +
> > + xsk_reuseq_free(xsk_reuseq_swap(umem, reuseq));
> > +
> > + priv->rq[qid].xsk_umem = umem;
> > + return err;
> > +}
> > +
> > +static int veth_xsk_umem_disable(struct net_device *dev,
> > + u16 qid)
> > +{
> > + struct veth_priv *priv = netdev_priv(dev);
> > + struct xdp_umem *umem;
> > +
> > + umem = xdp_get_umem_from_qid(dev, qid);
> > + if (!umem)
> > + return -EINVAL;
> > +
> > + priv->rq[qid].xsk_umem = NULL;
> > + return 0;
> > +}
> > +
> > +int veth_xsk_umem_setup(struct net_device *dev, struct xdp_umem *umem,
> > + u16 qid)
> > +{
> > + return umem ? veth_xsk_umem_enable(dev, umem, qid) :
> > + veth_xsk_umem_disable(dev, qid);
> > +}
> > +
> > +int veth_xsk_async_xmit(struct net_device *dev, u32 qid)
> > +{
> > + struct veth_priv *priv = netdev_priv(dev);
> > + struct veth_rq *rq;
> > +
> > + rq = &priv->rq[qid];
> > +
> > + if (qid >= dev->real_num_rx_queues)
> > + return -ENXIO;
> > +
> > + if (!napi_if_scheduled_mark_missed(&rq->xdp_napi))
> > + napi_schedule(&rq->xdp_napi);
> > +
> > + return 0;
> > +}
> > +
> > static int veth_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> > {
> > switch (xdp->command) {
> > @@ -1123,6 +1297,26 @@ static int veth_xdp(struct net_device *dev, struct netdev_bpf *xdp)
> > case XDP_QUERY_PROG:
> > xdp->prog_id = veth_xdp_query(dev);
> > return 0;
> > + case XDP_QUERY_XSK_UMEM:
> > + return veth_xsk_umem_query(dev, &xdp->xsk.umem,
> > + xdp->xsk.queue_id);
> > + case XDP_SETUP_XSK_UMEM: {
> > + struct veth_priv *priv;
> > + int err;
> > +
> > + /* Enable XDP on both sides */
> > + err = veth_enable_xdp(dev);
> > + if (err)
> > + return err;
> > +
> > + priv = netdev_priv(dev);
> > + err = veth_enable_xdp(priv->peer);
>
> Hmm, setting the umem on one peer, enables XDP on both ends? Why?
>
> I'm think there's some inconsistency with this patch -- again I might
> be missing something. To get some clarity, here are a couple of
> questions/statements/thoughts:
>
> For a veth pair A and B, B is XDP enabled. Transmissions from A or
> XDP_REDIRECT to A, will put the xdpf/skb onto B's Rx ring, and
> eventually schedule B napi context (from flush). B's napi poll will
> execute, draining the queue and execute the XDP program.
>
> In this patch, if A runs AF_XDP xmit, you schedule A's napi context,
> and execute B's receive path in that context. This is racy, right?
Yes, I think you're right.
>
> What you'd like is that, if B is running in a napi-mode, the sendmsg
> schedules *B's* napi, and drain A's AF_XDP Tx ring in B's receive
OK, it makes sense to use B's napi to to receive. Then, for the sync xmit,
it will kick B's napi instead of A. I will make the change in next patch.
> path. What would be the method to drain A's AF_XDP Tx ring if B is
> *not* running in napi (i.e. in XDP mode)? Schedule A's napi, build skb
> and pass it to the peer B? Another approach is that when a A has
> AF_XDP enabled, enable napi B to mode, and drain from B's napi poll.
> Is this why you "enable XDP on both sides", even if one peer hasn't an
> XDP program set (enabling XDP -> enable napi for that peer)?
Right, I want to make sure both side have NAPI, so I enable xdp on both.
Thanks for the review.
William
>
>
> Björn
>
>
>
> > + if (err)
> > + return err;
> > +
> > + return veth_xsk_umem_setup(dev, xdp->xsk.umem,
> > + xdp->xsk.queue_id);
> > + }
> > default:
> > return -EINVAL;
> > }
> > @@ -1145,6 +1339,7 @@ static const struct net_device_ops veth_netdev_ops = {
> > .ndo_set_rx_headroom = veth_set_rx_headroom,
> > .ndo_bpf = veth_xdp,
> > .ndo_xdp_xmit = veth_xdp_xmit,
> > + .ndo_xsk_async_xmit = veth_xsk_async_xmit,
> > };
> >
> > #define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HW_CSUM | \
> > --
> > 2.7.4
> >
Powered by blists - more mailing lists