| Message ID | 20220405110631.404427-4-maciej.fijalkowski@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | xsk: stop softirq processing on full XSK Rx queue | expand |
From: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Date: Tue, 5 Apr 2022 13:06:24 +0200 > Correlate -ENOBUFS that was returned from xdp_do_redirect() with a XSK > Rx queue being full. In such case, terminate the softirq processing and > let the user space to consume descriptors from XSK Rx queue so that > there is room that driver can use later on. > > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_txrx.h | 1 + > drivers/net/ethernet/intel/ice/ice_xsk.c | 25 +++++++++++++++-------- > 2 files changed, 17 insertions(+), 9 deletions(-) --- 8< --- > @@ -551,15 +552,15 @@ ice_run_xdp_zc(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, > if (result == ICE_XDP_CONSUMED) > goto out_failure; > break; > + case XDP_DROP: > + result = ICE_XDP_CONSUMED; > + break; > default: > bpf_warn_invalid_xdp_action(rx_ring->netdev, xdp_prog, act); > fallthrough; > case XDP_ABORTED: > out_failure: > trace_xdp_exception(rx_ring->netdev, xdp_prog, act); > - fallthrough; > - case XDP_DROP: > - result = ICE_XDP_CONSUMED; > break; So the result for %XDP_ABORTED will be %ICE_XDP_PASS now? Or I'm missing something :s > } > > @@ -628,10 +629,16 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, int budget) --- 8< --- > -- > 2.33.1 Thanks, Al
On Tue, Apr 05, 2022 at 01:34:03PM +0200, Alexander Lobakin wrote: > From: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > Date: Tue, 5 Apr 2022 13:06:24 +0200 > > > Correlate -ENOBUFS that was returned from xdp_do_redirect() with a XSK > > Rx queue being full. In such case, terminate the softirq processing and > > let the user space to consume descriptors from XSK Rx queue so that > > there is room that driver can use later on. > > > > Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> > > --- > > drivers/net/ethernet/intel/ice/ice_txrx.h | 1 + > > drivers/net/ethernet/intel/ice/ice_xsk.c | 25 +++++++++++++++-------- > > 2 files changed, 17 insertions(+), 9 deletions(-) > > --- 8< --- > > > @@ -551,15 +552,15 @@ ice_run_xdp_zc(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, > > if (result == ICE_XDP_CONSUMED) > > goto out_failure; > > break; > > + case XDP_DROP: > > + result = ICE_XDP_CONSUMED; > > + break; > > default: > > bpf_warn_invalid_xdp_action(rx_ring->netdev, xdp_prog, act); > > fallthrough; > > case XDP_ABORTED: > > out_failure: > > trace_xdp_exception(rx_ring->netdev, xdp_prog, act); > > - fallthrough; > > - case XDP_DROP: > > - result = ICE_XDP_CONSUMED; > > break; > > So the result for %XDP_ABORTED will be %ICE_XDP_PASS now? Or I'm > missing something :s Yikes! I generally wanted to avoid the overwrite of result but still go through the exception path. Below should be fine if we add it to the current patch, but i'll double check after the dinner. Good catch as always, Alex :) diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c index 143f6b6937bd..16c282b7050b 100644 --- a/drivers/net/ethernet/intel/ice/ice_xsk.c +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c @@ -559,6 +559,7 @@ ice_run_xdp_zc(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, bpf_warn_invalid_xdp_action(rx_ring->netdev, xdp_prog, act); fallthrough; case XDP_ABORTED: + result = ICE_XDP_CONSUMED; out_failure: trace_xdp_exception(rx_ring->netdev, xdp_prog, act); break; > > > } > > > > @@ -628,10 +629,16 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, int budget) > > --- 8< --- > > > -- > > 2.33.1 > > Thanks, > Al
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h b/drivers/net/ethernet/intel/ice/ice_txrx.h index cead3eb149bd..f5a906c03669 100644 --- a/drivers/net/ethernet/intel/ice/ice_txrx.h +++ b/drivers/net/ethernet/intel/ice/ice_txrx.h @@ -133,6 +133,7 @@ static inline int ice_skb_pad(void) #define ICE_XDP_CONSUMED BIT(0) #define ICE_XDP_TX BIT(1) #define ICE_XDP_REDIR BIT(2) +#define ICE_XDP_EXIT BIT(3) #define ICE_RX_DMA_ATTR \ (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING) diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c index dfbcaf08520e..2439141cfa9d 100644 --- a/drivers/net/ethernet/intel/ice/ice_xsk.c +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c @@ -538,9 +538,10 @@ ice_run_xdp_zc(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, if (likely(act == XDP_REDIRECT)) { err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog); - if (err) - goto out_failure; - return ICE_XDP_REDIR; + if (!err) + return ICE_XDP_REDIR; + result = (err == -ENOBUFS) ? ICE_XDP_EXIT : ICE_XDP_CONSUMED; + goto out_failure; } switch (act) { @@ -551,15 +552,15 @@ ice_run_xdp_zc(struct ice_rx_ring *rx_ring, struct xdp_buff *xdp, if (result == ICE_XDP_CONSUMED) goto out_failure; break; + case XDP_DROP: + result = ICE_XDP_CONSUMED; + break; default: bpf_warn_invalid_xdp_action(rx_ring->netdev, xdp_prog, act); fallthrough; case XDP_ABORTED: out_failure: trace_xdp_exception(rx_ring->netdev, xdp_prog, act); - fallthrough; - case XDP_DROP: - result = ICE_XDP_CONSUMED; break; } @@ -628,10 +629,16 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, int budget) xdp_res = ice_run_xdp_zc(rx_ring, xdp, xdp_prog, xdp_ring); if (xdp_res) { - if (xdp_res & (ICE_XDP_TX | ICE_XDP_REDIR)) + if (xdp_res == ICE_XDP_EXIT) { + failure = true; + xsk_buff_free(xdp); + ice_bump_ntc(rx_ring); + break; + } else if (xdp_res & (ICE_XDP_TX | ICE_XDP_REDIR)) { xdp_xmit |= xdp_res; - else + } else { xsk_buff_free(xdp); + } total_rx_bytes += size; total_rx_packets++; @@ -666,7 +673,7 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring, int budget) ice_receive_skb(rx_ring, skb, vlan_tag); } - failure = !ice_alloc_rx_bufs_zc(rx_ring, ICE_DESC_UNUSED(rx_ring)); + failure |= !ice_alloc_rx_bufs_zc(rx_ring, ICE_DESC_UNUSED(rx_ring)); ice_finalize_xdp_rx(xdp_ring, xdp_xmit); ice_update_rx_ring_stats(rx_ring, total_rx_packets, total_rx_bytes);
Correlate -ENOBUFS that was returned from xdp_do_redirect() with a XSK Rx queue being full. In such case, terminate the softirq processing and let the user space to consume descriptors from XSK Rx queue so that there is room that driver can use later on. Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> --- drivers/net/ethernet/intel/ice/ice_txrx.h | 1 + drivers/net/ethernet/intel/ice/ice_xsk.c | 25 +++++++++++++++-------- 2 files changed, 17 insertions(+), 9 deletions(-)