> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Zhao1, Wei
> Sent: Saturday, July 18, 2020 5:45 AM
>
> > From: dev <dev-boun...@dpdk.org> On Behalf Of Zhao1, Wei
> > Sent: Saturday, July 18, 2020 11:32 AM
> > To: Morten Brørup <m...@smartsharesystems.com>; Guo, Jia
> > <jia....@intel.com>
> >
> > HI,
> >
> > > From: Morten Brørup <m...@smartsharesystems.com>
> > > Sent: Thursday, July 16, 2020 5:09 PM
> > >
> > > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Zhao1, Wei
> > > > Sent: Thursday, July 16, 2020 10:50 AM
> > > >
> > > > Hi,
> > > >
> > > > > From: Morten Brørup <m...@smartsharesystems.com>
> > > > > Sent: Thursday, July 16, 2020 12:03 AM
> > > > >
> > > > > Wei, Jeff,
> > > > >
> > > > > For the ixgbe driver using vector functions, i.e.
> > > > ixgbe_recv_pkts_vec(), calling
> > > > > rte_eth_rx_burst() with nb_pkts > RTE_IXGBE_MAX_RX_BURST only
> > > > > returns RTE_IXGBE_MAX_RX_BURST packets. E.g. calling
> > > > > rte_eth_rx_burst() with
> > > > > nb_pkts=64 only returns 32 packets.
> > > > >
> > > > >
> > > > > The API description of rte_eth_rx_burst() says:
> > > > >
> > > > > <quote>
> > > > > The rte_eth_rx_burst() function returns the number of packets
> > > > actually
> > > > > retrieved, which is the number of rte_mbuf data structures
> > > > effectively supplied
> > > > > into the rx_pkts array. A return value equal to nb_pkts
> indicates
> > > > that the RX
> > > > > queue contained at least rx_pkts packets, and this is likely to
> > > > signify that other
> > > > > received packets remain in the input queue. Applications
> > > > > implementing
> > > > a
> > > > > "retrieve as much received packets as possible" policy can
> check
> > > > > this
> > > > specific
> > > > > case and keep invoking the rte_eth_rx_burst() function until a
> > > > > value
> > > > less than
> > > > > nb_pkts is returned.
> > > > > </quote>
> > > > >
> > > > > The driver implementation does not conform to the documented
> > > > > behavior
> > > > for
> > > > > "retrieve as much received packets as possible" applications.
> > > >
> > > > It seems not an issue, this function has comment bellow, it is
> > > > design work in that way.
> > > >
> > > >
> > > > /*
> > > > * vPMD receive routine, only accept(nb_pkts >=
> > > > RTE_IXGBE_DESCS_PER_LOOP)
> > > > *
> > > > * Notice:
> > > > * - nb_pkts < RTE_IXGBE_DESCS_PER_LOOP, just return no packet
> > > > * - nb_pkts > RTE_IXGBE_MAX_RX_BURST, only scan
> > > RTE_IXGBE_MAX_RX_BURST
> > > > * numbers of DD bit
> > > > * - floor align nb_pkts to a RTE_IXGBE_DESC_PER_LOOP power-of-
> two
> > > > */
> > > >
> > >
> > > I noticed this already. And yes, ixgbe_recv_pkts_vec() does what
> its
> > > comments says.
> > >
> > > However, when ixgbe_recv_pkts_vec() is used as the driver's
> > > implementation of the rte_eth_rx_burst() function call, the
> > > rte_eth_rx_burst() function does not do what is expected of the
> > rte_eth_rx_burst() function.
> > >
> > > The implementation must conform to the API that it implements.
> > >
> > > If you don't want to update the ixgbe_recv_pkts_vec() function, I
> > > propose that you add a wrapper function that calls
> > > ixgbe_recv_pkts_vec() repeatedly, and use the wrapper function as
> the
> > > implementation of the rte_eth_rx_burst() function.
>
> A code review will be do for that change, it is need because that is a
> important change.
I looked at it, and it seems that most Intel Ethernet drivers suffer from this
bug.
Some of them do not seem simple to fix, e.g. the fm40k behavior depends on the
configured rx_free_thresh, which defaults to 32.
I think we can all agree that it is too risky for me to attempt to fix drivers
for hardware which I do not have available for testing, so I have created a bug
in Bugzilla, and am thus passing the baton to the Intel PMD developer team.
>
> >
> > Get your point, I know what you need, but is there any risk for
> > ixgbe_recv_pkts_vec? I am not sure.
> > Maybe you can have a try first, if it work well, you can submit a
> patch.
> > What you need is this:
> >
> > uint16_t
> > i40e_recv_scattered_pkts_vec_avx2(void *rx_queue, struct rte_mbuf
> > **rx_pkts,
> > uint16_t nb_pkts)
> > {
> > uint16_t retval = 0;
> > while (nb_pkts > RTE_I40E_VPMD_RX_BURST) {
> > uint16_t burst =
> i40e_recv_scattered_burst_vec_avx2(rx_queue,
> > rx_pkts + retval, RTE_I40E_VPMD_RX_BURST);
> > retval += burst;
> > nb_pkts -= burst;
> > if (burst < RTE_I40E_VPMD_RX_BURST)
> > return retval;
> > }
> > return retval + i40e_recv_scattered_burst_vec_avx2(rx_queue,
> > rx_pkts + retval, nb_pkts);
> > }
