Hi everyone, > > On 2021/4/20 9:26, Ferruh Yigit wrote: > > On 4/16/2021 12:04 PM, Chengchang Tang wrote: > >> This patch add Tx prepare for bonding device. > >> > >> Currently, the bonding driver has not implemented the callback of > >> rte_eth_tx_prepare function. Therefore, the TX prepare function of the > >> slave devices will never be invoked. When hardware offloading such as > >> CKSUM and TSO are enabled for some drivers, tx_prepare needs to be used > >> to adjust packets (for example, set correct pseudo packet headers). > >> Otherwise, related offloading fails and even packets are sent > >> incorrectly. Due to this limitation, the bonded device cannot use these > >> HW offloading in the Tx direction. > >> > >> Because packet sending algorithms are numerous and complex in bond PMD, > >> it is hard to design the callback for rte_eth_tx_prepare. In this patch, > >> the tx_prepare callback of bonding PMD is not implemented. Instead, > >> rte_eth_tx_prepare has been called in tx_burst callback. And a global > >> variable is introduced to control whether the bonded device need call > >> the rte_eth_tx_prepare. If upper-layer users need to use some TX > >> offloading that depend on tx_prepare , they should enable the preparation > >> function. In this way, the bonded device will call the rte_eth_tx_prepare > >> for the fast path packets in the tx_burst callback.
I admit that I didn't look at the implementation yet, but it sounds like overcomplication to me. Can't we just have a new TX function for bonding PMD when TX offloads are enabled? And inside that function we will do: tx_prepare(); tx_burst(); for selected device. We can select this function at setup stage analysing requested by user TX offloads. > >> > > > > What do you think to add a devarg to bonding PMD to control the tx_prepare? > > It won't be as dynamic as API, since it can be possible to change the > > behavior after application is started with API, but do we really need > this? > > If an API is not added, unnecessary constraints may be introduced. If the > bonding device is created through the rte_eth_bond_create interface instead > devarg "vdev", this function cannot be used because devargs does not take > effect > in this case. But from an ease-of-use perspective, adding a devarg is a good > idea. I will add related implementations in the later official patches. I am also against introducing new devarg to control tx_prepare() invocation. I think at dev_config/queue_setup phase PMD will have enough information to decide. > > If I understand correctly, the current community does not want to introduce > more private APIs for PMDs. However, the absence of an API on this issue would > introduce some unnecessary constraints, and from that point of view, I think > adding an API seems necessary. > > > >> Chengchang Tang (2): > >> net/bonding: add Tx prepare for bonding > >> app/testpmd: add cmd for bonding Tx prepare > >> > >> app/test-pmd/cmdline.c | 66 > >> +++++++++++++++++++++++++++++ > >> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 9 ++++ > >> drivers/net/bonding/eth_bond_private.h | 1 + > >> drivers/net/bonding/rte_eth_bond.h | 29 +++++++++++++ > >> drivers/net/bonding/rte_eth_bond_api.c | 28 ++++++++++++ > >> drivers/net/bonding/rte_eth_bond_pmd.c | 33 +++++++++++++-- > >> drivers/net/bonding/version.map | 5 +++ > >> 7 files changed, 167 insertions(+), 4 deletions(-) > >> > >> -- > >> 2.7.4 > >> > > > > > > . > >
