>
>
> On 2021/6/8 17:49, Andrew Rybchenko wrote:
> > "for bonding" is redundant in the summary since it is already
> > "net/bonding"
> >
> > On 4/23/21 12:46 PM, Chengchang Tang wrote:
> >> Currently, the TX offloading of the bonding device will not take effect by
> >
> > TX -> Tx
> >
> >> using dev_configure. Because the related configuration will not be
> >> delivered to the slave devices in this way.
> >
> > I think it is a major problem that Tx offloads are actually
> > ignored. It should be a patches with "Fixes:" which addresses
> > it.
> >
> >> The Tx offloading capability of the bonding device is the intersection of
> >> the capability of all slave devices. Based on this, the following functions
> >> are added to the bonding driver:
> >> 1. If a Tx offloading is within the capability of the bonding device (i.e.
> >> all the slave devices support this Tx offloading), the enabling status of
> >> the offloading of all slave devices depends on the configuration of the
> >> bonding device.
> >>
> >> 2. For the Tx offloading that is not within the Tx offloading capability
> >> of the bonding device, the enabling status of the offloading on the slave
> >> devices is irrelevant to the bonding device configuration. And it depends
> >> on the original configuration of the slave devices.
> >>
> >> Signed-off-by: Chengchang Tang <tangchengch...@huawei.com>
> >> ---
> >> drivers/net/bonding/rte_eth_bond_pmd.c | 13 +++++++++++++
> >> 1 file changed, 13 insertions(+)
> >>
> >> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
> >> b/drivers/net/bonding/rte_eth_bond_pmd.c
> >> index 84af348..9922657 100644
> >> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> >> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> >> @@ -1712,6 +1712,8 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
> >> struct rte_flow_error flow_error;
> >>
> >> struct bond_dev_private *internals = bonded_eth_dev->data->dev_private;
> >> + uint64_t tx_offload_cap = internals->tx_offload_capa;
> >> + uint64_t tx_offload;
> >>
> >> /* Stop slave */
> >> errval = rte_eth_dev_stop(slave_eth_dev->data->port_id);
> >> @@ -1759,6 +1761,17 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
> >> slave_eth_dev->data->dev_conf.rxmode.offloads &=
> >> ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> >>
> >> + while (tx_offload_cap != 0) {
> >> + tx_offload = 1ULL << __builtin_ctzll(tx_offload_cap);
> >> + if (bonded_eth_dev->data->dev_conf.txmode.offloads & tx_offload)
> >> + slave_eth_dev->data->dev_conf.txmode.offloads |=
> >> + tx_offload;
> >> + else
> >> + slave_eth_dev->data->dev_conf.txmode.offloads &=
> >> + ~tx_offload;
> >> + tx_offload_cap &= ~tx_offload;
> >> + }
> >> +
> >
> > Frankly speaking I don't understand why it is that complicated.
> > ethdev rejects of unsupported Tx offloads. So, can't we simply:
> > slave_eth_dev->data->dev_conf.txmode.offloads =
> > bonded_eth_dev->data->dev_conf.txmode.offloads;
> >
>
> Using such a complicated method is to increase the flexibility of the slave
> devices,
> allowing the Tx offloading of the slave devices to be incompletely consistent
> with
> the bond device. If some offloading can be turned on without bond device
> awareness,
> they can be retained in this case.
Not sure how that can that happen...
From my understanding tx_offload for bond device has to be intersection of
tx_offloads
of all slaves, no? Otherwise bond device might be misconfigured.
Anyway for that code snippet above, wouldn't the same be achived by:
slave_eth_dev->data->dev_conf.txmode.offloads &= internals->tx_offload_capa &
bonded_eth_dev->data->dev_conf.txmode.offloads;
?
