| Message ID | 20181221132223.14012-1-domagoj.pintaric@sartura.hr |
|---|---|
| State | Deferred, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2] net: mvneta: add basic XDP_DROP support | expand |
From: Domagoj Pintaric <domagoj.pintaric@sartura.hr> Date: Fri, 21 Dec 2018 14:22:23 +0100 > Add initial mvneta XDP support for hardware buffer management enabled > devices only. > > Signed-off-by: Domagoj Pintaric <domagoj.pintaric@sartura.hr> > CC: Luka Perkov <luka.perkov@sartura.hr> > CC: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > CC: David S. Miller <davem@davemloft.net> > --- > v2: > - fixed unused value warning > - added additional hardware buffer management check Since the net-next tree is closed, this new feature will have to wait until the net-next tree opens back up again. So please resubmit this at that time. Meanwhile you can add support for the rest of the XDP features and do more testing :-) Thank you.
On Fri, 21 Dec 2018 14:22:23 +0100 Domagoj Pintaric <domagoj.pintaric@sartura.hr> wrote: > Add initial mvneta XDP support for hardware buffer management enabled > devices only. Hi Domagoj, I would really appreciate if we could coordinate our work on the mvneta driver. Ilias (Cc'ed) and I are also working on adding XDP support for this driver, although this is the software-buffer side of the driver we have functioning now. You can directly follow our progress here: [1][2] [1] https://github.com/xdp-project/xdp-project/blob/master/areas/arm64/board_espressobin04_bench_xdp.org [2] https://github.com/apalos/bpf-next/commits/mvneta_04_page_pool_recycle_xdp You XDP-setup function is actually more correct that ours[3], as you handle BPF per queue (which we were planning to fix before upstreaming). That said, adding XDP_DROP is easy, but I want to see more of the XDP features/actions added, as those require a lot more work. I always worry that a driver will stop at just XDP_DROP, so what are your plans for adding the harder features? Even-thought XDP_DROP looks easy in this patch, then you are actually doing some wrong, as XDP can also modify frames before doing XDP_PASS, and (1) you have non-standard-head room (MVNETA_MH_SIZE + NET_SKB_PAD), (2) and you don't handle if XDP changed the xdp.data header pointer, and (3) you backing memory either comes from page-fragments or kmalloc which also goes against the XDP memory requirements. [3] https://github.com/apalos/bpf-next/commit/4b567e74552d3cdf55
Hi Domagoj, > > Add initial mvneta XDP support for hardware buffer management enabled > > devices only. > > Hi Domagoj, > > I would really appreciate if we could coordinate our work on the mvneta > driver. Ilias (Cc'ed) and I are also working on adding XDP support for > this driver, although this is the software-buffer side of the driver we > have functioning now. Yes please, Jesper and i didn't have access to hardware with BM. Let's sync up and try to provide XDP for both. > > You can directly follow our progress here: [1][2] > [1] https://github.com/xdp-project/xdp-project/blob/master/areas/arm64/board_espressobin04_bench_xdp.org > [2] https://github.com/apalos/bpf-next/commits/mvneta_04_page_pool_recycle_xdp > > You XDP-setup function is actually more correct that ours[3], as you > handle BPF per queue (which we were planning to fix before upstreaming). > > That said, adding XDP_DROP is easy, but I want to see more of the XDP > features/actions added, as those require a lot more work. I always > worry that a driver will stop at just XDP_DROP, so what are your plans > for adding the harder features? > > Even-thought XDP_DROP looks easy in this patch, then you are actually > doing some wrong, as XDP can also modify frames before doing XDP_PASS, > and (1) you have non-standard-head room (MVNETA_MH_SIZE + NET_SKB_PAD), > (2) and you don't handle if XDP changed the xdp.data header pointer, > and (3) you backing memory either comes from page-fragments or kmalloc > which also goes against the XDP memory requirements. > [3] https://github.com/apalos/bpf-next/commit/4b567e74552d3cdf55 Our patches use the page_pool API for this (the mvneta SWBM part of the driver was already allocating pages for backing up descriptors) Thanks! /Ilias
Hi Jesper, On Sat, Dec 22, 2018 at 11:52:54AM +0100, Jesper Dangaard Brouer wrote: > On Fri, 21 Dec 2018 14:22:23 +0100 > Domagoj Pintaric <domagoj.pintaric@sartura.hr> wrote: > > > Add initial mvneta XDP support for hardware buffer management enabled > > devices only. > > Hi Domagoj, > > I would really appreciate if we could coordinate our work on the mvneta > driver. Ilias (Cc'ed) and I are also working on adding XDP support for > this driver, although this is the software-buffer side of the driver we > have functioning now. > > You can directly follow our progress here: [1][2] > [1] https://github.com/xdp-project/xdp-project/blob/master/areas/arm64/board_espressobin04_bench_xdp.org > [2] https://github.com/apalos/bpf-next/commits/mvneta_04_page_pool_recycle_xdp > > You XDP-setup function is actually more correct that ours[3], as you > handle BPF per queue (which we were planning to fix before upstreaming). > > That said, adding XDP_DROP is easy, but I want to see more of the XDP > features/actions added, as those require a lot more work. I always > worry that a driver will stop at just XDP_DROP, so what are your plans > for adding the harder features? Thank you for point out the you already made so much progress as we were not aware of it. Generally speaking, in Sartura our goal was to leverage all the development done in eBPF and XDP mainly for datacenter use-cases and use the technology in embedded systems where our primary focus is. That said, in OpenWrt summit two months ago Damir gave a talk about these technologies [1]. You are correct that we have sent a patch only for the easy part. But that is only what we have implemented so far :) As this was low-priority project on our side we didn't have concrete timelines for other features. Looking at what you have done and what we have I'd be happy to see if we can somehow divide the efforts in most optimal way. Except the mvneta driver (and we have a lot of boards utilizing that driver) we would also like to collaborate in mainline XDP support for mvpp2 which I have seen you have in plans as well. Thanks, Luka [1] https://openwrtsummit.files.wordpress.com/2018/11/20181029-state-of-fast-path-networking-in-linux.pdf > Even-thought XDP_DROP looks easy in this patch, then you are actually > doing some wrong, as XDP can also modify frames before doing XDP_PASS, > and (1) you have non-standard-head room (MVNETA_MH_SIZE + NET_SKB_PAD), > (2) and you don't handle if XDP changed the xdp.data header pointer, > and (3) you backing memory either comes from page-fragments or kmalloc > which also goes against the XDP memory requirements. > > [3] https://github.com/apalos/bpf-next/commit/4b567e74552d3cdf55 > -- > Best regards, > Jesper Dangaard Brouer > MSc.CS, Principal Kernel Engineer at Red Hat > LinkedIn: http://www.linkedin.com/in/brouer
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c index 61b23497f836..00c890bdf31e 100644 --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -31,6 +31,7 @@ #include <linux/phylink.h> #include <linux/platform_device.h> #include <linux/skbuff.h> +#include <linux/bpf.h> #include <net/hwbm.h> #include "mvneta_bm.h" #include <net/ip.h> @@ -454,6 +455,8 @@ struct mvneta_port { bool neta_armada3700; u16 rx_offset_correction; const struct mbus_dram_target_info *dram_target_info; + + struct bpf_prog *xdp_prog; }; /* The mvneta_tx_desc and mvneta_rx_desc structures describe the @@ -626,6 +629,8 @@ struct mvneta_rx_queue { /* error counters */ u32 skb_alloc_err; u32 refill_err; + + struct bpf_prog *xdp_prog; }; static enum cpuhp_state online_hpstate; @@ -2099,6 +2104,7 @@ static int mvneta_rx_hwbm(struct napi_struct *napi, int rx_done; u32 rcvd_pkts = 0; u32 rcvd_bytes = 0; + struct bpf_prog *xdp_prog; /* Get number of received packets */ rx_done = mvneta_rxq_busy_desc_num_get(pp, rxq); @@ -2140,6 +2146,29 @@ static int mvneta_rx_hwbm(struct napi_struct *napi, continue; } + xdp_prog = READ_ONCE(rxq->xdp_prog); + if (xdp_prog) { + struct xdp_buff xdp; + enum xdp_action act; + + xdp.data = data + MVNETA_MH_SIZE + NET_SKB_PAD; + xdp.data_end = xdp.data + rx_bytes; + + act = bpf_prog_run_xdp(xdp_prog, &xdp); + switch (act) { + case XDP_PASS: + break; + default: + bpf_warn_invalid_xdp_action(act); + /* fall through */ + case XDP_DROP: + /* Return the buffer to the pool */ + mvneta_bm_pool_put_bp(pp->bm_priv, bm_pool, + rx_desc->buf_phys_addr); + continue; + } + } + if (rx_bytes <= rx_copybreak) { /* better copy a small frame and not unmap the DMA region */ skb = netdev_alloc_skb_ip_align(dev, rx_bytes); @@ -2935,6 +2964,8 @@ static int mvneta_rxq_init(struct mvneta_port *pp, mvneta_rxq_hw_init(pp, rxq); + rxq->xdp_prog = pp->xdp_prog; + return 0; } @@ -2961,6 +2992,7 @@ static void mvneta_rxq_deinit(struct mvneta_port *pp, rxq->refill_num = 0; rxq->skb = NULL; rxq->left_size = 0; + rxq->xdp_prog = NULL; } static int mvneta_txq_sw_init(struct mvneta_port *pp, @@ -3878,6 +3910,43 @@ static int mvneta_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd) return phylink_mii_ioctl(pp->phylink, ifr, cmd); } +static int mvneta_xdp_set(struct net_device *dev, struct bpf_prog *xdp_prog) +{ + struct mvneta_port *pp = netdev_priv(dev); + struct bpf_prog *xdp_prog_old; + int queue; + + if (!pp->bm_priv) + return -EINVAL; + + xdp_prog_old = xchg(&pp->xdp_prog, xdp_prog); + + for (queue = 0; queue < rxq_number; queue++) { + struct mvneta_rx_queue *rxq = &pp->rxqs[queue]; + (void)xchg(&rxq->xdp_prog, pp->xdp_prog); + } + + if (xdp_prog_old) + bpf_prog_put(xdp_prog_old); + + return 0; +} + +static int mvneta_bpf(struct net_device *dev, struct netdev_bpf *bpf) +{ + struct mvneta_port *pp = netdev_priv(dev); + + switch (bpf->command) { + case XDP_SETUP_PROG: + return mvneta_xdp_set(dev, bpf->prog); + case XDP_QUERY_PROG: + bpf->prog_id = pp->xdp_prog ? pp->xdp_prog->aux->id : 0; + return 0; + default: + return -EINVAL; + } +} + /* Ethtool methods */ /* Set link ksettings (phy address, speed) for ethtools */ @@ -4275,6 +4344,7 @@ static const struct net_device_ops mvneta_netdev_ops = { .ndo_fix_features = mvneta_fix_features, .ndo_get_stats64 = mvneta_get_stats64, .ndo_do_ioctl = mvneta_ioctl, + .ndo_bpf = mvneta_bpf, }; static const struct ethtool_ops mvneta_eth_tool_ops = {
Add initial mvneta XDP support for hardware buffer management enabled devices only. Signed-off-by: Domagoj Pintaric <domagoj.pintaric@sartura.hr> CC: Luka Perkov <luka.perkov@sartura.hr> CC: Thomas Petazzoni <thomas.petazzoni@bootlin.com> CC: David S. Miller <davem@davemloft.net> --- v2: - fixed unused value warning - added additional hardware buffer management check --- drivers/net/ethernet/marvell/mvneta.c | 70 +++++++++++++++++++++++++++ 1 file changed, 70 insertions(+)