| Message ID | 20230104194132.24637-10-gerhard@engleder-embedded.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | tsnep: XDP support | expand |
From: Gerhard Engleder <gerhard@engleder-embedded.com> Date: Wed Jan 04 2023 20:41:32 GMT+0100 > If BPF program is set up, then run BPF program for every received frame > and execute the selected action. > > Test results with A53 1.2GHz: > > XDP_DROP (samples/bpf/xdp1) > proto 17: 883878 pkt/s > > XDP_TX (samples/bpf/xdp2) > proto 17: 255693 pkt/s > > XDP_REDIRECT (samples/bpf/xdpsock) > sock0@eth2:0 rxdrop xdp-drv > pps pkts 1.00 > rx 855,582 5,404,523 > tx 0 0 > > XDP_REDIRECT (samples/bpf/xdp_redirect) > eth2->eth1 613,267 rx/s 0 err,drop/s 613,272 xmit/s > > Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com> > --- > drivers/net/ethernet/engleder/tsnep_main.c | 129 ++++++++++++++++++++- > 1 file changed, 127 insertions(+), 2 deletions(-) [...] > @@ -624,6 +628,34 @@ static void tsnep_xdp_xmit_flush(struct tsnep_tx *tx) > iowrite32(TSNEP_CONTROL_TX_ENABLE, tx->addr + TSNEP_CONTROL); > } > > +static bool tsnep_xdp_xmit_back(struct tsnep_adapter *adapter, > + struct xdp_buff *xdp) > +{ > + struct xdp_frame *xdpf = xdp_convert_buff_to_frame(xdp); > + int cpu = smp_processor_id(); > + struct netdev_queue *nq; > + int queue; Squash with @cpu above (or make @cpu u32)? > + bool xmit; > + > + if (unlikely(!xdpf)) > + return -EFAULT; > + > + queue = cpu % adapter->num_tx_queues; > + nq = netdev_get_tx_queue(adapter->netdev, queue); > + > + __netif_tx_lock(nq, cpu); [...] > @@ -788,6 +820,11 @@ static unsigned int tsnep_rx_offset(struct tsnep_rx *rx) > return TSNEP_SKB_PAD; > } > > +static unsigned int tsnep_rx_offset_xdp(void) > +{ > + return XDP_PACKET_HEADROOM; > +} The reason for creating a function to always return a constant? > + > static void tsnep_rx_ring_cleanup(struct tsnep_rx *rx) > { > struct device *dmadev = rx->adapter->dmadev; [...] > +static void tsnep_finalize_xdp(struct tsnep_adapter *adapter, int status) > +{ > + int cpu = smp_processor_id(); > + struct netdev_queue *nq; > + int queue; (same re squashing) > + > + if (status & TSNEP_XDP_TX) { > + queue = cpu % adapter->num_tx_queues; > + nq = netdev_get_tx_queue(adapter->netdev, queue); > + > + __netif_tx_lock(nq, cpu); > + tsnep_xdp_xmit_flush(&adapter->tx[queue]); > + __netif_tx_unlock(nq); > + } This can be optimized. Given that one NAPI cycle is always being run on one CPU, you can get both @queue and @nq once at the beginning of a polling cycle and then pass it to perform %XDP_TX and this flush. Alternatively, if you don't want to do that not knowing in advance if you'll need it at all during the cycle, you can obtain them at the first tsnep_xdp_xmit_back() invocation. > + > + if (status & TSNEP_XDP_REDIRECT) > + xdp_do_flush(); > +} > + > static struct sk_buff *tsnep_build_skb(struct tsnep_rx *rx, struct page *page, > int length) > { [...] > @@ -1087,6 +1189,26 @@ static int tsnep_rx_poll(struct tsnep_rx *rx, struct napi_struct *napi, > rx->read = (rx->read + 1) % TSNEP_RING_SIZE; > desc_available++; > > + if (prog) { > + bool consume; > + > + xdp_init_buff(&xdp, PAGE_SIZE, &rx->xdp_rxq); xdp_init_buff() is designed to be called once per NAPI cycle, at the beginning. You don't need to reinit it given that the values you pass are always the same. > + xdp_prepare_buff(&xdp, page_address(entry->page), > + tsnep_rx_offset_xdp() + TSNEP_RX_INLINE_METADATA_SIZE, > + length, false); > + > + consume = tsnep_xdp_run_prog(rx, prog, &xdp, > + &xdp_status); [...] Thanks, Olek
On 05.01.23 18:52, Alexander Lobakin wrote: > From: Gerhard Engleder <gerhard@engleder-embedded.com> > Date: Wed Jan 04 2023 20:41:32 GMT+0100 > >> If BPF program is set up, then run BPF program for every received frame >> and execute the selected action. >> >> Test results with A53 1.2GHz: >> >> XDP_DROP (samples/bpf/xdp1) >> proto 17: 883878 pkt/s >> >> XDP_TX (samples/bpf/xdp2) >> proto 17: 255693 pkt/s >> >> XDP_REDIRECT (samples/bpf/xdpsock) >> sock0@eth2:0 rxdrop xdp-drv >> pps pkts 1.00 >> rx 855,582 5,404,523 >> tx 0 0 >> >> XDP_REDIRECT (samples/bpf/xdp_redirect) >> eth2->eth1 613,267 rx/s 0 err,drop/s 613,272 xmit/s >> >> Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com> >> --- >> drivers/net/ethernet/engleder/tsnep_main.c | 129 ++++++++++++++++++++- >> 1 file changed, 127 insertions(+), 2 deletions(-) > > [...] > >> @@ -624,6 +628,34 @@ static void tsnep_xdp_xmit_flush(struct tsnep_tx *tx) >> iowrite32(TSNEP_CONTROL_TX_ENABLE, tx->addr + TSNEP_CONTROL); >> } >> >> +static bool tsnep_xdp_xmit_back(struct tsnep_adapter *adapter, >> + struct xdp_buff *xdp) >> +{ >> + struct xdp_frame *xdpf = xdp_convert_buff_to_frame(xdp); >> + int cpu = smp_processor_id(); >> + struct netdev_queue *nq; >> + int queue; > > Squash with @cpu above (or make @cpu u32)? Will change to u32. >> + bool xmit; >> + >> + if (unlikely(!xdpf)) >> + return -EFAULT; >> + >> + queue = cpu % adapter->num_tx_queues; >> + nq = netdev_get_tx_queue(adapter->netdev, queue); >> + >> + __netif_tx_lock(nq, cpu); > > [...] > >> @@ -788,6 +820,11 @@ static unsigned int tsnep_rx_offset(struct tsnep_rx *rx) >> return TSNEP_SKB_PAD; >> } >> >> +static unsigned int tsnep_rx_offset_xdp(void) >> +{ >> + return XDP_PACKET_HEADROOM; >> +} > > The reason for creating a function to always return a constant? It is a variant of tsnep_rx_offset() for the XDP path to prevent unneeded calls of tsnep_xdp_is_enabled(). With this function I keep the RX offset local. But yes, it provides actually no functionality. I will add a comment. What about always using XDP_PACKET_HEADROOM as offset in the RX buffer? NET_IP_ALIGN would not be considered then, but it is zero anyway on the main platforms x86 and arm64. This would simplify the code. >> + >> static void tsnep_rx_ring_cleanup(struct tsnep_rx *rx) >> { >> struct device *dmadev = rx->adapter->dmadev; > > [...] > >> +static void tsnep_finalize_xdp(struct tsnep_adapter *adapter, int status) >> +{ >> + int cpu = smp_processor_id(); >> + struct netdev_queue *nq; >> + int queue; > > (same re squashing) u32. >> + >> + if (status & TSNEP_XDP_TX) { >> + queue = cpu % adapter->num_tx_queues; >> + nq = netdev_get_tx_queue(adapter->netdev, queue); >> + >> + __netif_tx_lock(nq, cpu); >> + tsnep_xdp_xmit_flush(&adapter->tx[queue]); >> + __netif_tx_unlock(nq); >> + } > > This can be optimized. Given that one NAPI cycle is always being run on > one CPU, you can get both @queue and @nq once at the beginning of a > polling cycle and then pass it to perform %XDP_TX and this flush. > Alternatively, if you don't want to do that not knowing in advance if > you'll need it at all during the cycle, you can obtain them at the first > tsnep_xdp_xmit_back() invocation. I will give it a try. >> + >> + if (status & TSNEP_XDP_REDIRECT) >> + xdp_do_flush(); >> +} >> + >> static struct sk_buff *tsnep_build_skb(struct tsnep_rx *rx, struct page *page, >> int length) >> { > > [...] > >> @@ -1087,6 +1189,26 @@ static int tsnep_rx_poll(struct tsnep_rx *rx, struct napi_struct *napi, >> rx->read = (rx->read + 1) % TSNEP_RING_SIZE; >> desc_available++; >> >> + if (prog) { >> + bool consume; >> + >> + xdp_init_buff(&xdp, PAGE_SIZE, &rx->xdp_rxq); > > xdp_init_buff() is designed to be called once per NAPI cycle, at the > beginning. You don't need to reinit it given that the values you pass > are always the same. Will be done. Thanks for the review! Gerhard
Hi Gerhard,
url: https://github.com/intel-lab-lkp/linux/commits/Gerhard-Engleder/tsnep-Use-spin_lock_bh-for-TX/20230105-034351
patch link: https://lore.kernel.org/r/20230104194132.24637-10-gerhard%40engleder-embedded.com
patch subject: [PATCH net-next v3 9/9] tsnep: Add XDP RX support
config: arc-randconfig-m041-20230106
compiler: arc-elf-gcc (GCC) 12.1.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
New smatch warnings:
drivers/net/ethernet/engleder/tsnep_main.c:641 tsnep_xdp_xmit_back() warn: signedness bug returning '(-14)'
vim +641 drivers/net/ethernet/engleder/tsnep_main.c
dd23b834a352b5 Gerhard Engleder 2023-01-04 631 static bool tsnep_xdp_xmit_back(struct tsnep_adapter *adapter,
^^^^
dd23b834a352b5 Gerhard Engleder 2023-01-04 632 struct xdp_buff *xdp)
dd23b834a352b5 Gerhard Engleder 2023-01-04 633 {
dd23b834a352b5 Gerhard Engleder 2023-01-04 634 struct xdp_frame *xdpf = xdp_convert_buff_to_frame(xdp);
dd23b834a352b5 Gerhard Engleder 2023-01-04 635 int cpu = smp_processor_id();
dd23b834a352b5 Gerhard Engleder 2023-01-04 636 struct netdev_queue *nq;
dd23b834a352b5 Gerhard Engleder 2023-01-04 637 int queue;
dd23b834a352b5 Gerhard Engleder 2023-01-04 638 bool xmit;
dd23b834a352b5 Gerhard Engleder 2023-01-04 639
dd23b834a352b5 Gerhard Engleder 2023-01-04 640 if (unlikely(!xdpf))
dd23b834a352b5 Gerhard Engleder 2023-01-04 @641 return -EFAULT;
return false?
dd23b834a352b5 Gerhard Engleder 2023-01-04 642
dd23b834a352b5 Gerhard Engleder 2023-01-04 643 queue = cpu % adapter->num_tx_queues;
dd23b834a352b5 Gerhard Engleder 2023-01-04 644 nq = netdev_get_tx_queue(adapter->netdev, queue);
dd23b834a352b5 Gerhard Engleder 2023-01-04 645
dd23b834a352b5 Gerhard Engleder 2023-01-04 646 __netif_tx_lock(nq, cpu);
dd23b834a352b5 Gerhard Engleder 2023-01-04 647
dd23b834a352b5 Gerhard Engleder 2023-01-04 648 /* Avoid transmit queue timeout since we share it with the slow path */
dd23b834a352b5 Gerhard Engleder 2023-01-04 649 txq_trans_cond_update(nq);
dd23b834a352b5 Gerhard Engleder 2023-01-04 650
dd23b834a352b5 Gerhard Engleder 2023-01-04 651 xmit = tsnep_xdp_xmit_frame_ring(xdpf, &adapter->tx[queue],
dd23b834a352b5 Gerhard Engleder 2023-01-04 652 TSNEP_TX_TYPE_XDP_TX);
dd23b834a352b5 Gerhard Engleder 2023-01-04 653
dd23b834a352b5 Gerhard Engleder 2023-01-04 654 __netif_tx_unlock(nq);
dd23b834a352b5 Gerhard Engleder 2023-01-04 655
dd23b834a352b5 Gerhard Engleder 2023-01-04 656 return xmit;
dd23b834a352b5 Gerhard Engleder 2023-01-04 657 }
On 07.01.23 09:46, Dan Carpenter wrote: > dd23b834a352b5 Gerhard Engleder 2023-01-04 631 static bool tsnep_xdp_xmit_back(struct tsnep_adapter *adapter, > ^^^^ > dd23b834a352b5 Gerhard Engleder 2023-01-04 632 struct xdp_buff *xdp) > dd23b834a352b5 Gerhard Engleder 2023-01-04 633 { > dd23b834a352b5 Gerhard Engleder 2023-01-04 634 struct xdp_frame *xdpf = xdp_convert_buff_to_frame(xdp); > dd23b834a352b5 Gerhard Engleder 2023-01-04 635 int cpu = smp_processor_id(); > dd23b834a352b5 Gerhard Engleder 2023-01-04 636 struct netdev_queue *nq; > dd23b834a352b5 Gerhard Engleder 2023-01-04 637 int queue; > dd23b834a352b5 Gerhard Engleder 2023-01-04 638 bool xmit; > dd23b834a352b5 Gerhard Engleder 2023-01-04 639 > dd23b834a352b5 Gerhard Engleder 2023-01-04 640 if (unlikely(!xdpf)) > dd23b834a352b5 Gerhard Engleder 2023-01-04 @641 return -EFAULT; > > return false? Of course. Will be fixed. Thanks! Gerhard
diff --git a/drivers/net/ethernet/engleder/tsnep_main.c b/drivers/net/ethernet/engleder/tsnep_main.c index 6a30f3bf73a6..beafaa60a35d 100644 --- a/drivers/net/ethernet/engleder/tsnep_main.c +++ b/drivers/net/ethernet/engleder/tsnep_main.c @@ -27,6 +27,7 @@ #include <linux/phy.h> #include <linux/iopoll.h> #include <linux/bpf.h> +#include <linux/bpf_trace.h> #define TSNEP_SKB_PAD (NET_SKB_PAD + NET_IP_ALIGN) #define TSNEP_HEADROOM ALIGN(max(TSNEP_SKB_PAD, XDP_PACKET_HEADROOM), 4) @@ -44,6 +45,9 @@ #define TSNEP_COALESCE_USECS_MAX ((ECM_INT_DELAY_MASK >> ECM_INT_DELAY_SHIFT) * \ ECM_INT_DELAY_BASE_US + ECM_INT_DELAY_BASE_US - 1) +#define TSNEP_XDP_TX BIT(0) +#define TSNEP_XDP_REDIRECT BIT(1) + enum { __TSNEP_DOWN, }; @@ -624,6 +628,34 @@ static void tsnep_xdp_xmit_flush(struct tsnep_tx *tx) iowrite32(TSNEP_CONTROL_TX_ENABLE, tx->addr + TSNEP_CONTROL); } +static bool tsnep_xdp_xmit_back(struct tsnep_adapter *adapter, + struct xdp_buff *xdp) +{ + struct xdp_frame *xdpf = xdp_convert_buff_to_frame(xdp); + int cpu = smp_processor_id(); + struct netdev_queue *nq; + int queue; + bool xmit; + + if (unlikely(!xdpf)) + return -EFAULT; + + queue = cpu % adapter->num_tx_queues; + nq = netdev_get_tx_queue(adapter->netdev, queue); + + __netif_tx_lock(nq, cpu); + + /* Avoid transmit queue timeout since we share it with the slow path */ + txq_trans_cond_update(nq); + + xmit = tsnep_xdp_xmit_frame_ring(xdpf, &adapter->tx[queue], + TSNEP_TX_TYPE_XDP_TX); + + __netif_tx_unlock(nq); + + return xmit; +} + static bool tsnep_tx_poll(struct tsnep_tx *tx, int napi_budget) { struct tsnep_tx_entry *entry; @@ -788,6 +820,11 @@ static unsigned int tsnep_rx_offset(struct tsnep_rx *rx) return TSNEP_SKB_PAD; } +static unsigned int tsnep_rx_offset_xdp(void) +{ + return XDP_PACKET_HEADROOM; +} + static void tsnep_rx_ring_cleanup(struct tsnep_rx *rx) { struct device *dmadev = rx->adapter->dmadev; @@ -993,6 +1030,67 @@ static int tsnep_rx_refill(struct tsnep_rx *rx, int count, bool reuse) return i; } +static bool tsnep_xdp_run_prog(struct tsnep_rx *rx, struct bpf_prog *prog, + struct xdp_buff *xdp, int *status) +{ + unsigned int length; + unsigned int sync; + u32 act; + + length = xdp->data_end - xdp->data_hard_start - tsnep_rx_offset_xdp(); + + act = bpf_prog_run_xdp(prog, xdp); + + /* Due xdp_adjust_tail: DMA sync for_device cover max len CPU touch */ + sync = xdp->data_end - xdp->data_hard_start - tsnep_rx_offset_xdp(); + sync = max(sync, length); + + switch (act) { + case XDP_PASS: + return false; + case XDP_TX: + if (!tsnep_xdp_xmit_back(rx->adapter, xdp)) + goto out_failure; + *status |= TSNEP_XDP_TX; + return true; + case XDP_REDIRECT: + if (xdp_do_redirect(rx->adapter->netdev, xdp, prog) < 0) + goto out_failure; + *status |= TSNEP_XDP_REDIRECT; + return true; + default: + bpf_warn_invalid_xdp_action(rx->adapter->netdev, prog, act); + fallthrough; + case XDP_ABORTED: +out_failure: + trace_xdp_exception(rx->adapter->netdev, prog, act); + fallthrough; + case XDP_DROP: + page_pool_put_page(rx->page_pool, virt_to_head_page(xdp->data), + sync, true); + return true; + } +} + +static void tsnep_finalize_xdp(struct tsnep_adapter *adapter, int status) +{ + int cpu = smp_processor_id(); + struct netdev_queue *nq; + int queue; + + if (status & TSNEP_XDP_TX) { + queue = cpu % adapter->num_tx_queues; + nq = netdev_get_tx_queue(adapter->netdev, queue); + + __netif_tx_lock(nq, cpu); + tsnep_xdp_xmit_flush(&adapter->tx[queue]); + __netif_tx_unlock(nq); + } + + if (status & TSNEP_XDP_REDIRECT) + xdp_do_flush(); +} + static struct sk_buff *tsnep_build_skb(struct tsnep_rx *rx, struct page *page, int length) { @@ -1028,15 +1126,19 @@ static int tsnep_rx_poll(struct tsnep_rx *rx, struct napi_struct *napi, int budget) { struct device *dmadev = rx->adapter->dmadev; - int desc_available; - int done = 0; enum dma_data_direction dma_dir; struct tsnep_rx_entry *entry; + struct bpf_prog *prog; + struct xdp_buff xdp; struct sk_buff *skb; + int desc_available; + int xdp_status = 0; + int done = 0; int length; desc_available = tsnep_rx_desc_available(rx); dma_dir = page_pool_get_dma_dir(rx->page_pool); + prog = READ_ONCE(rx->adapter->xdp_prog); while (likely(done < budget) && (rx->read != rx->write)) { entry = &rx->entry[rx->read]; @@ -1087,6 +1189,26 @@ static int tsnep_rx_poll(struct tsnep_rx *rx, struct napi_struct *napi, rx->read = (rx->read + 1) % TSNEP_RING_SIZE; desc_available++; + if (prog) { + bool consume; + + xdp_init_buff(&xdp, PAGE_SIZE, &rx->xdp_rxq); + xdp_prepare_buff(&xdp, page_address(entry->page), + tsnep_rx_offset_xdp() + TSNEP_RX_INLINE_METADATA_SIZE, + length, false); + + consume = tsnep_xdp_run_prog(rx, prog, &xdp, + &xdp_status); + if (consume) { + rx->packets++; + rx->bytes += length; + + entry->page = NULL; + + continue; + } + } + skb = tsnep_build_skb(rx, entry->page, length); if (skb) { page_pool_release_page(rx->page_pool, entry->page); @@ -1105,6 +1227,9 @@ static int tsnep_rx_poll(struct tsnep_rx *rx, struct napi_struct *napi, entry->page = NULL; } + if (xdp_status) + tsnep_finalize_xdp(rx->adapter, xdp_status); + if (desc_available) tsnep_rx_refill(rx, desc_available, false);
If BPF program is set up, then run BPF program for every received frame and execute the selected action. Test results with A53 1.2GHz: XDP_DROP (samples/bpf/xdp1) proto 17: 883878 pkt/s XDP_TX (samples/bpf/xdp2) proto 17: 255693 pkt/s XDP_REDIRECT (samples/bpf/xdpsock) sock0@eth2:0 rxdrop xdp-drv pps pkts 1.00 rx 855,582 5,404,523 tx 0 0 XDP_REDIRECT (samples/bpf/xdp_redirect) eth2->eth1 613,267 rx/s 0 err,drop/s 613,272 xmit/s Signed-off-by: Gerhard Engleder <gerhard@engleder-embedded.com> --- drivers/net/ethernet/engleder/tsnep_main.c | 129 ++++++++++++++++++++- 1 file changed, 127 insertions(+), 2 deletions(-)