From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: Eric Kinzie <ekinzie@brocade.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] ixgbe: fall back to non-vector rx
Date: Thu, 28 May 2015 09:59:24 +0000 [thread overview]
Message-ID: <2601191342CEEE43887BDE71AB977258214333D4@irsmsx105.ger.corp.intel.com> (raw)
In-Reply-To: <20150527182000.GB27368@zaphod>
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
> >
prev parent reply other threads:[~2015-05-28 9:59 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-26 15:52 [dpdk-dev] [PATCH] ixgbe " Eric Kinzie
2015-05-26 15:52 ` [dpdk-dev] [PATCH] ixgbe: " Eric Kinzie
2015-05-27 10:20 ` Ananyev, Konstantin
[not found] ` <20150527182000.GB27368@zaphod>
2015-05-28 9:59 ` Ananyev, Konstantin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2601191342CEEE43887BDE71AB977258214333D4@irsmsx105.ger.corp.intel.com \
--to=konstantin.ananyev@intel.com \
--cc=dev@dpdk.org \
--cc=ekinzie@brocade.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).