* [PATCH v2 1/2] i40e: tweak page counting for XDP_REDIRECT @ 2018-03-22 9:03 ` =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= 0 siblings, 0 replies; 10+ messages in thread From: Björn Töpel @ 2018-03-22 9:03 UTC (permalink / raw) To: jeffrey.t.kirsher, intel-wired-lan Cc: Björn Töpel, magnus.karlsson, netdev, alexander.h.duyck, alexander.duyck From: Björn Töpel <bjorn.topel@intel.com> This commit tweaks the page counting for XDP_REDIRECT to function properly. XDP_REDIRECT support will be added in a future commit. The current page counting scheme assumes that the reference count cannot decrease until the received frame is sent to the upper layers of the networking stack. This assumption does not hold for the XDP_REDIRECT action, since a page (pointed out by xdp_buff) can have its reference count decreased via the xdp_do_redirect call. To work around that, we now start off by a large page count and then don't allow a refcount less than two. Signed-off-by: Björn Töpel <bjorn.topel@intel.com> --- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index e8eef9a56b6b..2f817d1466eb 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -1588,9 +1588,8 @@ static bool i40e_alloc_mapped_page(struct i40e_ring *rx_ring, bi->dma = dma; bi->page = page; bi->page_offset = i40e_rx_offset(rx_ring); - - /* initialize pagecnt_bias to 1 representing we fully own page */ - bi->pagecnt_bias = 1; + page_ref_add(page, USHRT_MAX - 1); + bi->pagecnt_bias = USHRT_MAX; return true; } @@ -1956,8 +1955,8 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer) * the pagecnt_bias and page count so that we fully restock the * number of references the driver holds. */ - if (unlikely(!pagecnt_bias)) { - page_ref_add(page, USHRT_MAX); + if (unlikely(pagecnt_bias == 1)) { + page_ref_add(page, USHRT_MAX - 1); rx_buffer->pagecnt_bias = USHRT_MAX; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Intel-wired-lan] [PATCH v2 1/2] i40e: tweak page counting for XDP_REDIRECT @ 2018-03-22 9:03 ` =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= 0 siblings, 0 replies; 10+ messages in thread From: =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= @ 2018-03-22 9:03 UTC (permalink / raw) To: intel-wired-lan From: Bj?rn T?pel <bjorn.topel@intel.com> This commit tweaks the page counting for XDP_REDIRECT to function properly. XDP_REDIRECT support will be added in a future commit. The current page counting scheme assumes that the reference count cannot decrease until the received frame is sent to the upper layers of the networking stack. This assumption does not hold for the XDP_REDIRECT action, since a page (pointed out by xdp_buff) can have its reference count decreased via the xdp_do_redirect call. To work around that, we now start off by a large page count and then don't allow a refcount less than two. Signed-off-by: Bj?rn T?pel <bjorn.topel@intel.com> --- drivers/net/ethernet/intel/i40e/i40e_txrx.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index e8eef9a56b6b..2f817d1466eb 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -1588,9 +1588,8 @@ static bool i40e_alloc_mapped_page(struct i40e_ring *rx_ring, bi->dma = dma; bi->page = page; bi->page_offset = i40e_rx_offset(rx_ring); - - /* initialize pagecnt_bias to 1 representing we fully own page */ - bi->pagecnt_bias = 1; + page_ref_add(page, USHRT_MAX - 1); + bi->pagecnt_bias = USHRT_MAX; return true; } @@ -1956,8 +1955,8 @@ static bool i40e_can_reuse_rx_page(struct i40e_rx_buffer *rx_buffer) * the pagecnt_bias and page count so that we fully restock the * number of references the driver holds. */ - if (unlikely(!pagecnt_bias)) { - page_ref_add(page, USHRT_MAX); + if (unlikely(pagecnt_bias == 1)) { + page_ref_add(page, USHRT_MAX - 1); rx_buffer->pagecnt_bias = USHRT_MAX; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] i40e: add support for XDP_REDIRECT 2018-03-22 9:03 ` [Intel-wired-lan] " =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= @ 2018-03-22 9:03 ` =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= -1 siblings, 0 replies; 10+ messages in thread From: Björn Töpel @ 2018-03-22 9:03 UTC (permalink / raw) To: jeffrey.t.kirsher, intel-wired-lan Cc: Björn Töpel, magnus.karlsson, netdev, alexander.h.duyck, alexander.duyck From: Björn Töpel <bjorn.topel@intel.com> The driver now acts upon the XDP_REDIRECT return action. Two new ndos are implemented, ndo_xdp_xmit and ndo_xdp_flush. XDP_REDIRECT action enables XDP program to redirect frames to other netdevs. Signed-off-by: Björn Töpel <bjorn.topel@intel.com> --- drivers/net/ethernet/intel/i40e/i40e_main.c | 2 + drivers/net/ethernet/intel/i40e/i40e_txrx.c | 74 +++++++++++++++++++++++++---- drivers/net/ethernet/intel/i40e/i40e_txrx.h | 2 + 3 files changed, 68 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 79ab52276d12..2fb4261b4fd9 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -11815,6 +11815,8 @@ static const struct net_device_ops i40e_netdev_ops = { .ndo_bridge_getlink = i40e_ndo_bridge_getlink, .ndo_bridge_setlink = i40e_ndo_bridge_setlink, .ndo_bpf = i40e_xdp, + .ndo_xdp_xmit = i40e_xdp_xmit, + .ndo_xdp_flush = i40e_xdp_flush, }; /** diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index 2f817d1466eb..0168611312df 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -2214,7 +2214,7 @@ static int i40e_xmit_xdp_ring(struct xdp_buff *xdp, static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring, struct xdp_buff *xdp) { - int result = I40E_XDP_PASS; + int err, result = I40E_XDP_PASS; struct i40e_ring *xdp_ring; struct bpf_prog *xdp_prog; u32 act; @@ -2233,6 +2233,10 @@ static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring, xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index]; result = i40e_xmit_xdp_ring(xdp, xdp_ring); break; + case XDP_REDIRECT: + err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog); + result = !err ? I40E_XDP_TX : I40E_XDP_CONSUMED; + break; default: bpf_warn_invalid_xdp_action(act); case XDP_ABORTED: @@ -2268,6 +2272,15 @@ static void i40e_rx_buffer_flip(struct i40e_ring *rx_ring, #endif } +static inline void i40e_xdp_ring_update_tail(struct i40e_ring *xdp_ring) +{ + /* Force memory writes to complete before letting h/w + * know there are new descriptors to fetch. + */ + wmb(); + writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail); +} + /** * i40e_clean_rx_irq - Clean completed descriptors from Rx ring - bounce buf * @rx_ring: rx descriptor ring to transact packets on @@ -2402,16 +2415,11 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget) } if (xdp_xmit) { - struct i40e_ring *xdp_ring; + struct i40e_ring *xdp_ring = + rx_ring->vsi->xdp_rings[rx_ring->queue_index]; - xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index]; - - /* Force memory writes to complete before letting h/w - * know there are new descriptors to fetch. - */ - wmb(); - - writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail); + i40e_xdp_ring_update_tail(xdp_ring); + xdp_do_flush_map(); } rx_ring->skb = skb; @@ -3659,3 +3667,49 @@ netdev_tx_t i40e_lan_xmit_frame(struct sk_buff *skb, struct net_device *netdev) return i40e_xmit_frame_ring(skb, tx_ring); } + +/** + * i40e_xdp_xmit - Implements ndo_xdp_xmit + * @dev: netdev + * @xdp: XDP buffer + * + * Returns Zero if sent, else an error code + **/ +int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp) +{ + struct i40e_netdev_priv *np = netdev_priv(dev); + unsigned int queue_index = smp_processor_id(); + struct i40e_vsi *vsi = np->vsi; + int err; + + if (test_bit(__I40E_VSI_DOWN, vsi->state)) + return -EINVAL; + + if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs) + return -EINVAL; + + err = i40e_xmit_xdp_ring(xdp, vsi->xdp_rings[queue_index]); + if (err != I40E_XDP_TX) + return -ENOMEM; + + return 0; +} + +/** + * i40e_xdp_flush - Implements ndo_xdp_flush + * @dev: netdev + **/ +void i40e_xdp_flush(struct net_device *dev) +{ + struct i40e_netdev_priv *np = netdev_priv(dev); + unsigned int queue_index = smp_processor_id(); + struct i40e_vsi *vsi = np->vsi; + + if (test_bit(__I40E_VSI_DOWN, vsi->state)) + return; + + if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs) + return; + + i40e_xdp_ring_update_tail(vsi->xdp_rings[queue_index]); +} diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h index 2444f338bb0c..a97e59721393 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h @@ -510,6 +510,8 @@ u32 i40e_get_tx_pending(struct i40e_ring *ring, bool in_sw); void i40e_detect_recover_hung(struct i40e_vsi *vsi); int __i40e_maybe_stop_tx(struct i40e_ring *tx_ring, int size); bool __i40e_chk_linearize(struct sk_buff *skb); +int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp); +void i40e_xdp_flush(struct net_device *dev); /** * i40e_get_head - Retrieve head from head writeback -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Intel-wired-lan] [PATCH v2 2/2] i40e: add support for XDP_REDIRECT @ 2018-03-22 9:03 ` =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= 0 siblings, 0 replies; 10+ messages in thread From: =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= @ 2018-03-22 9:03 UTC (permalink / raw) To: intel-wired-lan From: Bj?rn T?pel <bjorn.topel@intel.com> The driver now acts upon the XDP_REDIRECT return action. Two new ndos are implemented, ndo_xdp_xmit and ndo_xdp_flush. XDP_REDIRECT action enables XDP program to redirect frames to other netdevs. Signed-off-by: Bj?rn T?pel <bjorn.topel@intel.com> --- drivers/net/ethernet/intel/i40e/i40e_main.c | 2 + drivers/net/ethernet/intel/i40e/i40e_txrx.c | 74 +++++++++++++++++++++++++---- drivers/net/ethernet/intel/i40e/i40e_txrx.h | 2 + 3 files changed, 68 insertions(+), 10 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 79ab52276d12..2fb4261b4fd9 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -11815,6 +11815,8 @@ static const struct net_device_ops i40e_netdev_ops = { .ndo_bridge_getlink = i40e_ndo_bridge_getlink, .ndo_bridge_setlink = i40e_ndo_bridge_setlink, .ndo_bpf = i40e_xdp, + .ndo_xdp_xmit = i40e_xdp_xmit, + .ndo_xdp_flush = i40e_xdp_flush, }; /** diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index 2f817d1466eb..0168611312df 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -2214,7 +2214,7 @@ static int i40e_xmit_xdp_ring(struct xdp_buff *xdp, static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring, struct xdp_buff *xdp) { - int result = I40E_XDP_PASS; + int err, result = I40E_XDP_PASS; struct i40e_ring *xdp_ring; struct bpf_prog *xdp_prog; u32 act; @@ -2233,6 +2233,10 @@ static struct sk_buff *i40e_run_xdp(struct i40e_ring *rx_ring, xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index]; result = i40e_xmit_xdp_ring(xdp, xdp_ring); break; + case XDP_REDIRECT: + err = xdp_do_redirect(rx_ring->netdev, xdp, xdp_prog); + result = !err ? I40E_XDP_TX : I40E_XDP_CONSUMED; + break; default: bpf_warn_invalid_xdp_action(act); case XDP_ABORTED: @@ -2268,6 +2272,15 @@ static void i40e_rx_buffer_flip(struct i40e_ring *rx_ring, #endif } +static inline void i40e_xdp_ring_update_tail(struct i40e_ring *xdp_ring) +{ + /* Force memory writes to complete before letting h/w + * know there are new descriptors to fetch. + */ + wmb(); + writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail); +} + /** * i40e_clean_rx_irq - Clean completed descriptors from Rx ring - bounce buf * @rx_ring: rx descriptor ring to transact packets on @@ -2402,16 +2415,11 @@ static int i40e_clean_rx_irq(struct i40e_ring *rx_ring, int budget) } if (xdp_xmit) { - struct i40e_ring *xdp_ring; + struct i40e_ring *xdp_ring = + rx_ring->vsi->xdp_rings[rx_ring->queue_index]; - xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index]; - - /* Force memory writes to complete before letting h/w - * know there are new descriptors to fetch. - */ - wmb(); - - writel_relaxed(xdp_ring->next_to_use, xdp_ring->tail); + i40e_xdp_ring_update_tail(xdp_ring); + xdp_do_flush_map(); } rx_ring->skb = skb; @@ -3659,3 +3667,49 @@ netdev_tx_t i40e_lan_xmit_frame(struct sk_buff *skb, struct net_device *netdev) return i40e_xmit_frame_ring(skb, tx_ring); } + +/** + * i40e_xdp_xmit - Implements ndo_xdp_xmit + * @dev: netdev + * @xdp: XDP buffer + * + * Returns Zero if sent, else an error code + **/ +int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp) +{ + struct i40e_netdev_priv *np = netdev_priv(dev); + unsigned int queue_index = smp_processor_id(); + struct i40e_vsi *vsi = np->vsi; + int err; + + if (test_bit(__I40E_VSI_DOWN, vsi->state)) + return -EINVAL; + + if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs) + return -EINVAL; + + err = i40e_xmit_xdp_ring(xdp, vsi->xdp_rings[queue_index]); + if (err != I40E_XDP_TX) + return -ENOMEM; + + return 0; +} + +/** + * i40e_xdp_flush - Implements ndo_xdp_flush + * @dev: netdev + **/ +void i40e_xdp_flush(struct net_device *dev) +{ + struct i40e_netdev_priv *np = netdev_priv(dev); + unsigned int queue_index = smp_processor_id(); + struct i40e_vsi *vsi = np->vsi; + + if (test_bit(__I40E_VSI_DOWN, vsi->state)) + return; + + if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs) + return; + + i40e_xdp_ring_update_tail(vsi->xdp_rings[queue_index]); +} diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h index 2444f338bb0c..a97e59721393 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h @@ -510,6 +510,8 @@ u32 i40e_get_tx_pending(struct i40e_ring *ring, bool in_sw); void i40e_detect_recover_hung(struct i40e_vsi *vsi); int __i40e_maybe_stop_tx(struct i40e_ring *tx_ring, int size); bool __i40e_chk_linearize(struct sk_buff *skb); +int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp); +void i40e_xdp_flush(struct net_device *dev); /** * i40e_get_head - Retrieve head from head writeback -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] i40e: add support for XDP_REDIRECT 2018-03-22 9:03 ` [Intel-wired-lan] " =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= @ 2018-03-22 11:58 ` Jesper Dangaard Brouer -1 siblings, 0 replies; 10+ messages in thread From: Jesper Dangaard Brouer @ 2018-03-22 11:58 UTC (permalink / raw) To: Björn Töpel Cc: brouer, jeffrey.t.kirsher, intel-wired-lan, Björn Töpel, magnus.karlsson, netdev, alexander.h.duyck, alexander.duyck On Thu, 22 Mar 2018 10:03:07 +0100 Björn Töpel <bjorn.topel@gmail.com> wrote: > +/** > + * i40e_xdp_xmit - Implements ndo_xdp_xmit > + * @dev: netdev > + * @xdp: XDP buffer > + * > + * Returns Zero if sent, else an error code > + **/ > +int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp) > +{ The return code is used by the XDP redirect tracepoint... this is the only way we have to debug/troubleshoot runtime issues with XDP. Thus, these need to be consistent across drives and distinguishable. > + struct i40e_netdev_priv *np = netdev_priv(dev); > + unsigned int queue_index = smp_processor_id(); > + struct i40e_vsi *vsi = np->vsi; > + int err; > + > + if (test_bit(__I40E_VSI_DOWN, vsi->state)) > + return -EINVAL; Should be: -ENETDOWN > + > + if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs) > + return -EINVAL; Should be: -ENXIO > + err = i40e_xmit_xdp_ring(xdp, vsi->xdp_rings[queue_index]); > + if (err != I40E_XDP_TX) > + return -ENOMEM; Should be: -ENOSPC The ENOSPC return code is important, as this can be used as a feedback to a XDP_REDIRECT load-balancer facility. -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-wired-lan] [PATCH v2 2/2] i40e: add support for XDP_REDIRECT @ 2018-03-22 11:58 ` Jesper Dangaard Brouer 0 siblings, 0 replies; 10+ messages in thread From: Jesper Dangaard Brouer @ 2018-03-22 11:58 UTC (permalink / raw) To: intel-wired-lan On Thu, 22 Mar 2018 10:03:07 +0100 Bj?rn T?pel <bjorn.topel@gmail.com> wrote: > +/** > + * i40e_xdp_xmit - Implements ndo_xdp_xmit > + * @dev: netdev > + * @xdp: XDP buffer > + * > + * Returns Zero if sent, else an error code > + **/ > +int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp) > +{ The return code is used by the XDP redirect tracepoint... this is the only way we have to debug/troubleshoot runtime issues with XDP. Thus, these need to be consistent across drives and distinguishable. > + struct i40e_netdev_priv *np = netdev_priv(dev); > + unsigned int queue_index = smp_processor_id(); > + struct i40e_vsi *vsi = np->vsi; > + int err; > + > + if (test_bit(__I40E_VSI_DOWN, vsi->state)) > + return -EINVAL; Should be: -ENETDOWN > + > + if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs) > + return -EINVAL; Should be: -ENXIO > + err = i40e_xmit_xdp_ring(xdp, vsi->xdp_rings[queue_index]); > + if (err != I40E_XDP_TX) > + return -ENOMEM; Should be: -ENOSPC The ENOSPC return code is important, as this can be used as a feedback to a XDP_REDIRECT load-balancer facility. -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] i40e: add support for XDP_REDIRECT 2018-03-22 11:58 ` [Intel-wired-lan] " Jesper Dangaard Brouer @ 2018-03-22 12:20 ` =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= -1 siblings, 0 replies; 10+ messages in thread From: Björn Töpel @ 2018-03-22 12:20 UTC (permalink / raw) To: Jesper Dangaard Brouer Cc: Jeff Kirsher, intel-wired-lan, Björn Töpel, Karlsson, Magnus, Netdev, Duyck, Alexander H, Alexander Duyck 2018-03-22 12:58 GMT+01:00 Jesper Dangaard Brouer <brouer@redhat.com>: > > On Thu, 22 Mar 2018 10:03:07 +0100 Björn Töpel <bjorn.topel@gmail.com> wrote: > >> +/** >> + * i40e_xdp_xmit - Implements ndo_xdp_xmit >> + * @dev: netdev >> + * @xdp: XDP buffer >> + * >> + * Returns Zero if sent, else an error code >> + **/ >> +int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp) >> +{ > > The return code is used by the XDP redirect tracepoint... this is the > only way we have to debug/troubleshoot runtime issues with XDP. Thus, > these need to be consistent across drives and distinguishable. > Thanks for pointing this out! I'll address all your comments and do a respin (but I'll wait for Alex' comments, if any). Björn >> + struct i40e_netdev_priv *np = netdev_priv(dev); >> + unsigned int queue_index = smp_processor_id(); >> + struct i40e_vsi *vsi = np->vsi; >> + int err; >> + >> + if (test_bit(__I40E_VSI_DOWN, vsi->state)) >> + return -EINVAL; > > Should be: -ENETDOWN > >> + >> + if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs) >> + return -EINVAL; > > Should be: -ENXIO > >> + err = i40e_xmit_xdp_ring(xdp, vsi->xdp_rings[queue_index]); >> + if (err != I40E_XDP_TX) >> + return -ENOMEM; > > Should be: -ENOSPC > > The ENOSPC return code is important, as this can be used as a feedback > to a XDP_REDIRECT load-balancer facility. > > -- > Best regards, > Jesper Dangaard Brouer > MSc.CS, Principal Kernel Engineer at Red Hat > LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-wired-lan] [PATCH v2 2/2] i40e: add support for XDP_REDIRECT @ 2018-03-22 12:20 ` =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= 0 siblings, 0 replies; 10+ messages in thread From: =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= @ 2018-03-22 12:20 UTC (permalink / raw) To: intel-wired-lan 2018-03-22 12:58 GMT+01:00 Jesper Dangaard Brouer <brouer@redhat.com>: > > On Thu, 22 Mar 2018 10:03:07 +0100 Bj?rn T?pel <bjorn.topel@gmail.com> wrote: > >> +/** >> + * i40e_xdp_xmit - Implements ndo_xdp_xmit >> + * @dev: netdev >> + * @xdp: XDP buffer >> + * >> + * Returns Zero if sent, else an error code >> + **/ >> +int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp) >> +{ > > The return code is used by the XDP redirect tracepoint... this is the > only way we have to debug/troubleshoot runtime issues with XDP. Thus, > these need to be consistent across drives and distinguishable. > Thanks for pointing this out! I'll address all your comments and do a respin (but I'll wait for Alex' comments, if any). Bj?rn >> + struct i40e_netdev_priv *np = netdev_priv(dev); >> + unsigned int queue_index = smp_processor_id(); >> + struct i40e_vsi *vsi = np->vsi; >> + int err; >> + >> + if (test_bit(__I40E_VSI_DOWN, vsi->state)) >> + return -EINVAL; > > Should be: -ENETDOWN > >> + >> + if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs) >> + return -EINVAL; > > Should be: -ENXIO > >> + err = i40e_xmit_xdp_ring(xdp, vsi->xdp_rings[queue_index]); >> + if (err != I40E_XDP_TX) >> + return -ENOMEM; > > Should be: -ENOSPC > > The ENOSPC return code is important, as this can be used as a feedback > to a XDP_REDIRECT load-balancer facility. > > -- > Best regards, > Jesper Dangaard Brouer > MSc.CS, Principal Kernel Engineer at Red Hat > LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] i40e: add support for XDP_REDIRECT 2018-03-22 12:20 ` [Intel-wired-lan] " =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= @ 2018-03-22 14:52 ` Alexander Duyck -1 siblings, 0 replies; 10+ messages in thread From: Alexander Duyck @ 2018-03-22 14:52 UTC (permalink / raw) To: Björn Töpel Cc: Jesper Dangaard Brouer, Jeff Kirsher, intel-wired-lan, Björn Töpel, Karlsson, Magnus, Netdev, Duyck, Alexander H On Thu, Mar 22, 2018 at 5:20 AM, Björn Töpel <bjorn.topel@gmail.com> wrote: > 2018-03-22 12:58 GMT+01:00 Jesper Dangaard Brouer <brouer@redhat.com>: >> >> On Thu, 22 Mar 2018 10:03:07 +0100 Björn Töpel <bjorn.topel@gmail.com> wrote: >> >>> +/** >>> + * i40e_xdp_xmit - Implements ndo_xdp_xmit >>> + * @dev: netdev >>> + * @xdp: XDP buffer >>> + * >>> + * Returns Zero if sent, else an error code >>> + **/ >>> +int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp) >>> +{ >> >> The return code is used by the XDP redirect tracepoint... this is the >> only way we have to debug/troubleshoot runtime issues with XDP. Thus, >> these need to be consistent across drives and distinguishable. >> > > Thanks for pointing this out! I'll address all your comments and do a > respin (but I'll wait for Alex' comments, if any). > > > Björn > The patch mostly looks okay to me. Maybe a bit of reverse xmas tree formatting needs to be addressed for the variable declarations in your two new functions but that is about it in terms of what I see. - Alex ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-wired-lan] [PATCH v2 2/2] i40e: add support for XDP_REDIRECT @ 2018-03-22 14:52 ` Alexander Duyck 0 siblings, 0 replies; 10+ messages in thread From: Alexander Duyck @ 2018-03-22 14:52 UTC (permalink / raw) To: intel-wired-lan On Thu, Mar 22, 2018 at 5:20 AM, Bj?rn T?pel <bjorn.topel@gmail.com> wrote: > 2018-03-22 12:58 GMT+01:00 Jesper Dangaard Brouer <brouer@redhat.com>: >> >> On Thu, 22 Mar 2018 10:03:07 +0100 Bj?rn T?pel <bjorn.topel@gmail.com> wrote: >> >>> +/** >>> + * i40e_xdp_xmit - Implements ndo_xdp_xmit >>> + * @dev: netdev >>> + * @xdp: XDP buffer >>> + * >>> + * Returns Zero if sent, else an error code >>> + **/ >>> +int i40e_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp) >>> +{ >> >> The return code is used by the XDP redirect tracepoint... this is the >> only way we have to debug/troubleshoot runtime issues with XDP. Thus, >> these need to be consistent across drives and distinguishable. >> > > Thanks for pointing this out! I'll address all your comments and do a > respin (but I'll wait for Alex' comments, if any). > > > Bj?rn > The patch mostly looks okay to me. Maybe a bit of reverse xmas tree formatting needs to be addressed for the variable declarations in your two new functions but that is about it in terms of what I see. - Alex ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-03-22 14:52 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-22 9:03 [PATCH v2 1/2] i40e: tweak page counting for XDP_REDIRECT Björn Töpel 2018-03-22 9:03 ` [Intel-wired-lan] " =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= 2018-03-22 9:03 ` [PATCH v2 2/2] i40e: add support " Björn Töpel 2018-03-22 9:03 ` [Intel-wired-lan] " =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= 2018-03-22 11:58 ` Jesper Dangaard Brouer 2018-03-22 11:58 ` [Intel-wired-lan] " Jesper Dangaard Brouer 2018-03-22 12:20 ` Björn Töpel 2018-03-22 12:20 ` [Intel-wired-lan] " =?unknown-8bit?q?Bj=C3=B6rn_T=C3=B6pel?= 2018-03-22 14:52 ` Alexander Duyck 2018-03-22 14:52 ` [Intel-wired-lan] " Alexander Duyck
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.