> -----Original Message----- > From: Morten Brørup <m...@smartsharesystems.com> > Sent: Monday, July 27, 2020 5:42 PM > To: Zhao1, Wei <wei.zh...@intel.com>; Guo, Jia <jia....@intel.com>; Xing, > Beilei <beilei.x...@intel.com>; Zhang, Qi Z <qi.z.zh...@intel.com>; Wang, > Xiao W <xiao.w.w...@intel.com>; Wu, Jingjing <jingjing...@intel.com>; Yang, > Qiming <qiming.y...@intel.com>; Xu, Rosen <rosen...@intel.com> > Cc: dev@dpdk.org; Thomas Monjalon <tho...@monjalon.net>; Yigit, Ferruh > <ferruh.yi...@intel.com>; Andrew Rybchenko <arybche...@solarflare.com> > Subject: RE: [dpdk-dev] ixgbe vector rx does not conform to rte_eth_rx_burst() > API > > > 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.
Yes > > 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. > Ok. > > > > > > > > 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); > > > }