[dpdk-dev] [PATCH] ixgbe fall back to non-vector rx

[dpdk-dev] [PATCH] ixgbe fall back to non-vector rx

[Eric Kinzie]

This change addresses an issue found using the bonding driver
with ixgbe interfaces that have vector operations enabled
(CONFIG_RTE_IXGBE_INC_VECTOR=y).

Eric Kinzie (1):
  ixgbe: fall back to non-vector rx

 drivers/net/ixgbe/ixgbe_rxtx.c     |   10 +++++-----
 drivers/net/ixgbe/ixgbe_rxtx.h     |    4 ++++
 drivers/net/ixgbe/ixgbe_rxtx_vec.c |    4 ++--
 3 files changed, 11 insertions(+), 7 deletions(-)

-- 
1.7.10.4

[dpdk-dev] [PATCH] ixgbe: fall back to non-vector rx

[Eric Kinzie]

The ixgbe driver refuses to receive any packets when vector receive
is enabled and fewer than the minimum number of required mbufs (32)
are supplied.  This makes it incompatible with the bonding driver
which, during receive, may start out with enough buffers but as it
collects packets from each of the enslaved interfaces can drop below
that threshold.  Instead of just giving up when insufficient buffers are
supplied fall back to the original, non-vector, ixgbe receive function.

Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
---
 drivers/net/ixgbe/ixgbe_rxtx.c     |   10 +++++-----
 drivers/net/ixgbe/ixgbe_rxtx.h     |    4 ++++
 drivers/net/ixgbe/ixgbe_rxtx_vec.c |    4 ++--
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 4f9ab22..fbba0ab 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -1088,9 +1088,9 @@ ixgbe_rx_fill_from_stage(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 	return nb_pkts;
 }
 
-static inline uint16_t
-rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
-	     uint16_t nb_pkts)
+uint16_t
+ixgbe_rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
+		   uint16_t nb_pkts)
 {
 	struct ixgbe_rx_queue *rxq = (struct ixgbe_rx_queue *)rx_queue;
 	uint16_t nb_rx = 0;
@@ -1158,14 +1158,14 @@ ixgbe_recv_pkts_bulk_alloc(void *rx_queue, struct rte_mbuf **rx_pkts,
 		return 0;
 
 	if (likely(nb_pkts <= RTE_PMD_IXGBE_RX_MAX_BURST))
-		return rx_recv_pkts(rx_queue, rx_pkts, nb_pkts);
+		return ixgbe_rx_recv_pkts(rx_queue, rx_pkts, nb_pkts);
 
 	/* request is relatively large, chunk it up */
 	nb_rx = 0;
 	while (nb_pkts) {
 		uint16_t ret, n;
 		n = (uint16_t)RTE_MIN(nb_pkts, RTE_PMD_IXGBE_RX_MAX_BURST);
-		ret = rx_recv_pkts(rx_queue, &rx_pkts[nb_rx], n);
+		ret = ixgbe_rx_recv_pkts(rx_queue, &rx_pkts[nb_rx], n);
 		nb_rx = (uint16_t)(nb_rx + ret);
 		nb_pkts = (uint16_t)(nb_pkts - ret);
 		if (ret < n)
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index af36438..811e514 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -283,6 +283,10 @@ uint16_t ixgbe_recv_scattered_pkts_vec(void *rx_queue,
 int ixgbe_rx_vec_dev_conf_condition_check(struct rte_eth_dev *dev);
 int ixgbe_rxq_vec_setup(struct ixgbe_rx_queue *rxq);
 
+uint16_t ixgbe_rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
+		uint16_t nb_pkts);
+
+
 #ifdef RTE_IXGBE_INC_VECTOR
 
 uint16_t ixgbe_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts,
diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
index abd10f6..d27424c 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
@@ -181,7 +181,7 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
  * in one loop
  *
  * Notice:
- * - nb_pkts < RTE_IXGBE_VPMD_RX_BURST, just return no packet
+ * - nb_pkts < RTE_IXGBE_VPMD_RX_BURST, fall back to non-vector receive
  * - nb_pkts > RTE_IXGBE_VPMD_RX_BURST, only scan RTE_IXGBE_VPMD_RX_BURST
  *   numbers of DD bit
  * - don't support ol_flags for rss and csum err
@@ -206,7 +206,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 	__m128i dd_check, eop_check;
 
 	if (unlikely(nb_pkts < RTE_IXGBE_VPMD_RX_BURST))
-		return 0;
+		return ixgbe_rx_recv_pkts(rxq, rx_pkts, nb_pkts);
 
 	/* Just the act of getting into the function from the application is
 	 * going to cost about 7 cycles */
-- 
1.7.10.4

Re: [dpdk-dev] [PATCH] ixgbe: fall back to non-vector rx

[Ananyev, Konstantin]

Hi Eric,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Eric Kinzie
> Sent: Tuesday, May 26, 2015 4:52 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH] ixgbe: fall back to non-vector rx
> 
> The ixgbe driver refuses to receive any packets when vector receive
> is enabled and fewer than the minimum number of required mbufs (32)
> are supplied.  This makes it incompatible with the bonding driver
> which, during receive, may start out with enough buffers but as it
> collects packets from each of the enslaved interfaces can drop below
> that threshold.  Instead of just giving up when insufficient buffers are
> supplied fall back to the original, non-vector, ixgbe receive function.

Right now,  vector and bulk_alloc RX methods are not interchangeable.
Once you setup your RX queue, you can't mix them.
It would be good to make vector RX method to work with arbitrary number of packets,
but I don't think your method would work properly.
In meanwhile, wonder can this problem be handled on the bonding device level?
Something like prevent vector RX be enabled at setup stage, or something?
Konstantin

> 
> Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
> ---
>  drivers/net/ixgbe/ixgbe_rxtx.c     |   10 +++++-----
>  drivers/net/ixgbe/ixgbe_rxtx.h     |    4 ++++
>  drivers/net/ixgbe/ixgbe_rxtx_vec.c |    4 ++--
>  3 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 4f9ab22..fbba0ab 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -1088,9 +1088,9 @@ ixgbe_rx_fill_from_stage(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
>  	return nb_pkts;
>  }
> 
> -static inline uint16_t
> -rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
> -	     uint16_t nb_pkts)
> +uint16_t
> +ixgbe_rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
> +		   uint16_t nb_pkts)
>  {
>  	struct ixgbe_rx_queue *rxq = (struct ixgbe_rx_queue *)rx_queue;
>  	uint16_t nb_rx = 0;
> @@ -1158,14 +1158,14 @@ ixgbe_recv_pkts_bulk_alloc(void *rx_queue, struct rte_mbuf **rx_pkts,
>  		return 0;
> 
>  	if (likely(nb_pkts <= RTE_PMD_IXGBE_RX_MAX_BURST))
> -		return rx_recv_pkts(rx_queue, rx_pkts, nb_pkts);
> +		return ixgbe_rx_recv_pkts(rx_queue, rx_pkts, nb_pkts);
> 
>  	/* request is relatively large, chunk it up */
>  	nb_rx = 0;
>  	while (nb_pkts) {
>  		uint16_t ret, n;
>  		n = (uint16_t)RTE_MIN(nb_pkts, RTE_PMD_IXGBE_RX_MAX_BURST);
> -		ret = rx_recv_pkts(rx_queue, &rx_pkts[nb_rx], n);
> +		ret = ixgbe_rx_recv_pkts(rx_queue, &rx_pkts[nb_rx], n);
>  		nb_rx = (uint16_t)(nb_rx + ret);
>  		nb_pkts = (uint16_t)(nb_pkts - ret);
>  		if (ret < n)
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
> index af36438..811e514 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.h
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.h
> @@ -283,6 +283,10 @@ uint16_t ixgbe_recv_scattered_pkts_vec(void *rx_queue,
>  int ixgbe_rx_vec_dev_conf_condition_check(struct rte_eth_dev *dev);
>  int ixgbe_rxq_vec_setup(struct ixgbe_rx_queue *rxq);
> 
> +uint16_t ixgbe_rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
> +		uint16_t nb_pkts);
> +
> +
>  #ifdef RTE_IXGBE_INC_VECTOR
> 
>  uint16_t ixgbe_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts,
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> index abd10f6..d27424c 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> @@ -181,7 +181,7 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
>   * in one loop
>   *
>   * Notice:
> - * - nb_pkts < RTE_IXGBE_VPMD_RX_BURST, just return no packet
> + * - nb_pkts < RTE_IXGBE_VPMD_RX_BURST, fall back to non-vector receive
>   * - nb_pkts > RTE_IXGBE_VPMD_RX_BURST, only scan RTE_IXGBE_VPMD_RX_BURST
>   *   numbers of DD bit
>   * - don't support ol_flags for rss and csum err
> @@ -206,7 +206,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
>  	__m128i dd_check, eop_check;
> 
>  	if (unlikely(nb_pkts < RTE_IXGBE_VPMD_RX_BURST))
> -		return 0;
> +		return ixgbe_rx_recv_pkts(rxq, rx_pkts, nb_pkts);
> 
>  	/* Just the act of getting into the function from the application is
>  	 * going to cost about 7 cycles */
> --
> 1.7.10.4

Re: [dpdk-dev] [PATCH] ixgbe: fall back to non-vector rx

[Ananyev, Konstantin]

Hi Eric,

> -----Original Message-----
> From: Eric Kinzie [mailto:ekinzie@brocade.com]
> Sent: Wednesday, May 27, 2015 7:20 PM
> To: Ananyev, Konstantin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] ixgbe: fall back to non-vector rx
> 
> On Wed May 27 10:20:39 +0000 2015, Ananyev, Konstantin wrote:
> > Hi Eric,
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Eric Kinzie
> > > Sent: Tuesday, May 26, 2015 4:52 PM
> > > To: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH] ixgbe: fall back to non-vector rx
> > >
> > > The ixgbe driver refuses to receive any packets when vector receive
> > > is enabled and fewer than the minimum number of required mbufs (32)
> > > are supplied.  This makes it incompatible with the bonding driver
> > > which, during receive, may start out with enough buffers but as it
> > > collects packets from each of the enslaved interfaces can drop below
> > > that threshold.  Instead of just giving up when insufficient buffers are
> > > supplied fall back to the original, non-vector, ixgbe receive function.
> >
> > Right now,  vector and bulk_alloc RX methods are not interchangeable.
> > Once you setup your RX queue, you can't mix them.
> > It would be good to make vector RX method to work with arbitrary number of packets,
> > but I don't think your method would work properly.
> > In meanwhile, wonder can this problem be handled on the bonding device level?
> > Something like prevent vector RX be enabled at setup stage, or something?
> > Konstantin
> 
> 
> Konstantin, thanks for reviewing this -- I'll look for some other way to
> address the problem.
> 
> Regardless of how this is dealt with, is it acceptable to make
> _recv_raw_pkts_vec() return an error when nb_pkts is too small?  Or will
> this cause problems elsewhere?

I am afraid it would.
Right now rte_eth_rx_burst() function does not provide any error information,
it just returns number of packets.
Changing that would have a massive impact on both DPDK libraries and external applications.
Konstantin

> 
> Eric
> 
> 
> > >
> > > Signed-off-by: Eric Kinzie <ehkinzie@gmail.com>
> > > ---
> > >  drivers/net/ixgbe/ixgbe_rxtx.c     |   10 +++++-----
> > >  drivers/net/ixgbe/ixgbe_rxtx.h     |    4 ++++
> > >  drivers/net/ixgbe/ixgbe_rxtx_vec.c |    4 ++--
> > >  3 files changed, 11 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> > > index 4f9ab22..fbba0ab 100644
> > > --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> > > @@ -1088,9 +1088,9 @@ ixgbe_rx_fill_from_stage(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
> > >  	return nb_pkts;
> > >  }
> > >
> > > -static inline uint16_t
> > > -rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
> > > -	     uint16_t nb_pkts)
> > > +uint16_t
> > > +ixgbe_rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
> > > +		   uint16_t nb_pkts)
> > >  {
> > >  	struct ixgbe_rx_queue *rxq = (struct ixgbe_rx_queue *)rx_queue;
> > >  	uint16_t nb_rx = 0;
> > > @@ -1158,14 +1158,14 @@ ixgbe_recv_pkts_bulk_alloc(void *rx_queue, struct rte_mbuf **rx_pkts,
> > >  		return 0;
> > >
> > >  	if (likely(nb_pkts <= RTE_PMD_IXGBE_RX_MAX_BURST))
> > > -		return rx_recv_pkts(rx_queue, rx_pkts, nb_pkts);
> > > +		return ixgbe_rx_recv_pkts(rx_queue, rx_pkts, nb_pkts);
> > >
> > >  	/* request is relatively large, chunk it up */
> > >  	nb_rx = 0;
> > >  	while (nb_pkts) {
> > >  		uint16_t ret, n;
> > >  		n = (uint16_t)RTE_MIN(nb_pkts, RTE_PMD_IXGBE_RX_MAX_BURST);
> > > -		ret = rx_recv_pkts(rx_queue, &rx_pkts[nb_rx], n);
> > > +		ret = ixgbe_rx_recv_pkts(rx_queue, &rx_pkts[nb_rx], n);
> > >  		nb_rx = (uint16_t)(nb_rx + ret);
> > >  		nb_pkts = (uint16_t)(nb_pkts - ret);
> > >  		if (ret < n)
> > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
> > > index af36438..811e514 100644
> > > --- a/drivers/net/ixgbe/ixgbe_rxtx.h
> > > +++ b/drivers/net/ixgbe/ixgbe_rxtx.h
> > > @@ -283,6 +283,10 @@ uint16_t ixgbe_recv_scattered_pkts_vec(void *rx_queue,
> > >  int ixgbe_rx_vec_dev_conf_condition_check(struct rte_eth_dev *dev);
> > >  int ixgbe_rxq_vec_setup(struct ixgbe_rx_queue *rxq);
> > >
> > > +uint16_t ixgbe_rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
> > > +		uint16_t nb_pkts);
> > > +
> > > +
> > >  #ifdef RTE_IXGBE_INC_VECTOR
> > >
> > >  uint16_t ixgbe_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts,
> > > diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec.c b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> > > index abd10f6..d27424c 100644
> > > --- a/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> > > +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec.c
> > > @@ -181,7 +181,7 @@ desc_to_olflags_v(__m128i descs[4], struct rte_mbuf **rx_pkts)
> > >   * in one loop
> > >   *
> > >   * Notice:
> > > - * - nb_pkts < RTE_IXGBE_VPMD_RX_BURST, just return no packet
> > > + * - nb_pkts < RTE_IXGBE_VPMD_RX_BURST, fall back to non-vector receive
> > >   * - nb_pkts > RTE_IXGBE_VPMD_RX_BURST, only scan RTE_IXGBE_VPMD_RX_BURST
> > >   *   numbers of DD bit
> > >   * - don't support ol_flags for rss and csum err
> > > @@ -206,7 +206,7 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
> > >  	__m128i dd_check, eop_check;
> > >
> > >  	if (unlikely(nb_pkts < RTE_IXGBE_VPMD_RX_BURST))
> > > -		return 0;
> > > +		return ixgbe_rx_recv_pkts(rxq, rx_pkts, nb_pkts);
> > >
> > >  	/* Just the act of getting into the function from the application is
> > >  	 * going to cost about 7 cycles */
> > > --
> > > 1.7.10.4
> >