[PATCH 0/2] net/bonding: fix mbuf fast free for bond3 and bond4

[PATCH 0/2] net/bonding: fix mbuf fast free for bond3 and bond4

[Huisong Li]

This patchset fixes mbuf fast free offload for bond3 and bond4.

Huisong Li (2):
  net/bonding: fix bond3 and bond4 process mbuf fast free
  app/testpmd: remove fast free offload for bond3 and bond4

 app/test-pmd/testpmd.c                 | 8 ++++++++
 drivers/net/bonding/rte_eth_bond_pmd.c | 6 ++----
 2 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.33.0

[PATCH 1/2] net/bonding: fix bond3 and bond4 process mbuf fast free

[Huisong Li]

The RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE offload can't be used in bonding
mode Broadcast and mode 8023AD. Currently, bonding driver forcibly removes
from the dev->data->dev_conf.txmode.offloads and processes as success in
bond_ethdev_configure(). But this still cause that rte_eth_dev_configure()
fails to execute because of the failure of validating Tx offload in the
eth_dev_validate_offloads(). So bonding driver shouldn't modify offloads
in dev->data->dev_conf.txmode and should return error in this case.

Fixes: 18c41457cbae ("net/bonding: fix mbuf fast free usage")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 864e073db8..b5e832276a 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -3712,10 +3712,8 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
 	if ((offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) &&
 			(internals->mode == BONDING_MODE_8023AD ||
 			internals->mode == BONDING_MODE_BROADCAST)) {
-		RTE_BOND_LOG(WARNING,
-			"bond mode broadcast & 8023AD don't support MBUF_FAST_FREE offload, force disable it.");
-		offloads &= ~RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
-		dev->data->dev_conf.txmode.offloads = offloads;
+		RTE_BOND_LOG(ERR, "bond mode broadcast & 8023AD don't support MBUF_FAST_FREE offload.");
+		return -EINVAL;
 	}
 
 	link_speeds = dev->data->dev_conf.link_speeds;
-- 
2.33.0

[PATCH 2/2] app/testpmd: remove fast free offload for bond3 and bond4

[Huisong Li]

The port configurations of bonding device need to be updated when add a
slave device. But mbuf fast free offload can't be used in bonding mode
Broadcast and mode 8023AD. So remove this offload for both mode in case
of failing to initialize bonding device.

Fixes: 76376bd9cd49 ("app/testpmd: update bond port configurations when add slave")

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 app/test-pmd/testpmd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7381dfd9e5..4063ffe0f2 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2812,6 +2812,7 @@ update_bonding_port_dev_conf(portid_t bond_pid)
 {
 #ifdef RTE_NET_BOND
 	struct rte_port *port = &ports[bond_pid];
+	uint8_t bonding_mode;
 	uint16_t i;
 	int ret;
 
@@ -2825,6 +2826,13 @@ update_bonding_port_dev_conf(portid_t bond_pid)
 	if (port->dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE)
 		port->dev_conf.txmode.offloads |=
 				RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
+	/* Bond mode broadcast & 8023AD don't support MBUF_FAST_FREE offload */
+	bonding_mode = rte_eth_bond_mode_get(bond_pid);
+	if (bonding_mode == BONDING_MODE_8023AD ||
+	    bonding_mode == BONDING_MODE_BROADCAST)
+		port->dev_conf.txmode.offloads &=
+				~RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
+
 	/* Apply Tx offloads configuration */
 	for (i = 0; i < port->dev_info.max_tx_queues; i++)
 		port->txq[i].conf.offloads = port->dev_conf.txmode.offloads;
-- 
2.33.0

Re: [PATCH 2/2] app/testpmd: remove fast free offload for bond3 and bond4

[Stephen Hemminger]

On Tue, 8 Nov 2022 16:40:38 +0800
Huisong Li <lihuisong@huawei.com> wrote:

> The port configurations of bonding device need to be updated when add a
> slave device. But mbuf fast free offload can't be used in bonding mode
> Broadcast and mode 8023AD. So remove this offload for both mode in case
> of failing to initialize bonding device.
> 
> Fixes: 76376bd9cd49 ("app/testpmd: update bond port configurations when add slave")
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
	int ret;
>  
> @@ -2825,6 +2826,13 @@ update_bonding_port_dev_conf(portid_t bond_pid)
>  	if (port->dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE)
>  		port->dev_conf.txmode.offloads |=
>  				RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
> +	/* Bond mode broadcast & 8023AD don't support MBUF_FAST_FREE offload */
> +	bonding_mode = rte_eth_bond_mode_get(bond_pid);
> +	if (bonding_mode == BONDING_MODE_8023AD ||
> +	    bonding_mode == BONDING_MODE_BROADCAST)
> +		port->dev_conf.txmode.offloads &=
> +				~RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
> +

This would be better handled in the bonding driver.
It should return the correct txmode offloads based on bonding mode.
That way it would work for any application not just testpmd.

Re: [PATCH 2/2] app/testpmd: remove fast free offload for bond3 and bond4

[lihuisong (C)]

在 2022/11/9 2:01, Stephen Hemminger 写道:
> On Tue, 8 Nov 2022 16:40:38 +0800
> Huisong Li <lihuisong@huawei.com> wrote:
>
>> The port configurations of bonding device need to be updated when add a
>> slave device. But mbuf fast free offload can't be used in bonding mode
>> Broadcast and mode 8023AD. So remove this offload for both mode in case
>> of failing to initialize bonding device.
>>
>> Fixes: 76376bd9cd49 ("app/testpmd: update bond port configurations when add slave")
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> 	int ret;
>>   
>> @@ -2825,6 +2826,13 @@ update_bonding_port_dev_conf(portid_t bond_pid)
>>   	if (port->dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE)
>>   		port->dev_conf.txmode.offloads |=
>>   				RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
>> +	/* Bond mode broadcast & 8023AD don't support MBUF_FAST_FREE offload */
>> +	bonding_mode = rte_eth_bond_mode_get(bond_pid);
>> +	if (bonding_mode == BONDING_MODE_8023AD ||
>> +	    bonding_mode == BONDING_MODE_BROADCAST)
>> +		port->dev_conf.txmode.offloads &=
>> +				~RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
>> +
> This would be better handled in the bonding driver.
> It should return the correct txmode offloads based on bonding mode.
> That way it would work for any application not just testpmd.
Good idea. I will fix it in v2.
> .

[PATCH V2] net/bonding: fix bond3 and bond4 process mbuf fast free

[Huisong Li]

The RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE offload can't be used in bonding
mode Broadcast and mode 8023AD. Currently, bonding driver forcibly removes
from the dev->data->dev_conf.txmode.offloads and processes as success in
bond_ethdev_configure(). But this still cause that rte_eth_dev_configure()
fails to execute because of the failure of validating Tx offload in the
eth_dev_validate_offloads(). So this patch moves the modification of txmode
offlaods to the stage of adding slave device to report the correct txmode
offloads.

Fixes: 18c41457cbae ("net/bonding: fix mbuf fast free usage")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 -v2: report the correct txmode offloads based on bonding mode in the
      bonding driver, and not in testpmd.
---
 drivers/net/bonding/rte_eth_bond_api.c |  5 +++++
 drivers/net/bonding/rte_eth_bond_pmd.c | 11 -----------
 2 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index 694fe86115..c0178369b4 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -544,6 +544,11 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
 			return ret;
 	}
 
+	/* Bond mode Broadcast & 8023AD don't support MBUF_FAST_FREE offload. */
+	if (internals->mode == BONDING_MODE_8023AD ||
+	    internals->mode == BONDING_MODE_BROADCAST)
+		internals->tx_offload_capa &= ~RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
+
 	bonded_eth_dev->data->dev_conf.rx_adv_conf.rss_conf.rss_hf &=
 			internals->flow_type_rss_offloads;
 
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 864e073db8..2efaad1e8e 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -3643,7 +3643,6 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
 	const char *name = dev->device->name;
 	struct bond_dev_private *internals = dev->data->dev_private;
 	struct rte_kvargs *kvlist = internals->kvlist;
-	uint64_t offloads;
 	int arg_count;
 	uint16_t port_id = dev - rte_eth_devices;
 	uint32_t link_speeds;
@@ -3708,16 +3707,6 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
 		}
 	}
 
-	offloads = dev->data->dev_conf.txmode.offloads;
-	if ((offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) &&
-			(internals->mode == BONDING_MODE_8023AD ||
-			internals->mode == BONDING_MODE_BROADCAST)) {
-		RTE_BOND_LOG(WARNING,
-			"bond mode broadcast & 8023AD don't support MBUF_FAST_FREE offload, force disable it.");
-		offloads &= ~RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
-		dev->data->dev_conf.txmode.offloads = offloads;
-	}
-
 	link_speeds = dev->data->dev_conf.link_speeds;
 	/*
 	 * The default value of 'link_speeds' is zero. From its definition,
-- 
2.33.0

Re: [PATCH V2] net/bonding: fix bond3 and bond4 process mbuf fast free

[Stephen Hemminger]

On Wed, 9 Nov 2022 10:22:37 +0800
Huisong Li <lihuisong@huawei.com> wrote:

> The RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE offload can't be used in bonding
> mode Broadcast and mode 8023AD. Currently, bonding driver forcibly removes
> from the dev->data->dev_conf.txmode.offloads and processes as success in
> bond_ethdev_configure(). But this still cause that rte_eth_dev_configure()
> fails to execute because of the failure of validating Tx offload in the
> eth_dev_validate_offloads(). So this patch moves the modification of txmode
> offlaods to the stage of adding slave device to report the correct txmode
> offloads.
> 
> Fixes: 18c41457cbae ("net/bonding: fix mbuf fast free usage")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>

Looks good thanks.
Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [PATCH V2] net/bonding: fix bond3 and bond4 process mbuf fast free

[Andrew Rybchenko]

On 11/9/22 06:24, Stephen Hemminger wrote:
> On Wed, 9 Nov 2022 10:22:37 +0800
> Huisong Li <lihuisong@huawei.com> wrote:
> 
>> The RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE offload can't be used in bonding
>> mode Broadcast and mode 8023AD. Currently, bonding driver forcibly removes
>> from the dev->data->dev_conf.txmode.offloads and processes as success in
>> bond_ethdev_configure(). But this still cause that rte_eth_dev_configure()
>> fails to execute because of the failure of validating Tx offload in the
>> eth_dev_validate_offloads(). So this patch moves the modification of txmode
>> offlaods to the stage of adding slave device to report the correct txmode
>> offloads.
>>
>> Fixes: 18c41457cbae ("net/bonding: fix mbuf fast free usage")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> 
> Looks good thanks.
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Applied to dpdk-next-net/main, thanks.