[PATCH] app/testpmd: update bond port configurations when add slave

[PATCH] app/testpmd: update bond port configurations when add slave

[Huisong Li]

Some capabilities (like, rx_offload_capa and tx_offload_capa) of
bonding device in dev_info is zero when no slave is added. And its
capability will be updated when add a new slave device.

The capability to update dynamically can introduce some probelms if
not handled properly. For example, the reconfig() is called to initialize
bonding port configurations when create a bonding device. The global
tx_mode is assigned to dev_conf.txmode. The DEV_TX_OFFLOAD_MBUF_FAST_FREE
which is the default value of global tx_mode.offloads in testpmd is removed
from bonding device configuration because of zero rx_offload_capa.
As a result, this offload isn't set to bonding device.

Generally, port configurations of bonding device must be within the
intersection of the capability of all slave devices. If use original port
configurations, the removed capabilities because of adding a new slave may
cause failure when re-initialize bonding device.

So port configurations of bonding device also need to be updated because of
the added and removed capabilities. In addition, this also helps to ensure
consistency between testpmd and bonding device.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 app/test-pmd/testpmd.c                | 40 +++++++++++++++++++++++++++
 app/test-pmd/testpmd.h                |  3 +-
 drivers/net/bonding/bonding_testpmd.c |  2 ++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 97adafacd0..7324b8865c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2805,6 +2805,41 @@ fill_xstats_display_info(void)
 		fill_xstats_display_info_for_port(pi);
 }
 
+/*
+ * Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
+ * device in dev_info is zero when no slave is added. And its capability of
+ * will be updated when add a new slave device. So adding a device slave need
+ * to update the port configurations of bonding device.
+ */
+static void
+update_bonding_port_dev_conf(portid_t bond_pid)
+{
+#ifdef RTE_NET_BOND
+	struct rte_port *port = &ports[bond_pid];
+	uint16_t i;
+	int ret;
+
+	ret = eth_dev_info_get_print_err(bond_pid, &port->dev_info);
+	if (ret != 0) {
+		fprintf(stderr, "Failed to get dev info for port = %u\n",
+			bond_pid);
+		return;
+	}
+
+	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;
+	/* 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;
+
+	port->dev_conf.rx_adv_conf.rss_conf.rss_hf &=
+				port->dev_info.flow_type_rss_offloads;
+#else
+	RTE_SET_USED(bond_pid);
+#endif
+}
+
 int
 start_port(portid_t pid)
 {
@@ -2869,6 +2904,11 @@ start_port(portid_t pid)
 				return -1;
 			}
 
+			if (port->bond_flag == 1 && port->update_conf == 1) {
+				update_bonding_port_dev_conf(pi);
+				port->update_conf = 0;
+			}
+
 			/* configure port */
 			diag = eth_dev_configure_mp(pi, nb_rxq + nb_hairpinq,
 						     nb_txq + nb_hairpinq,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7fef96f9b1..82714119e8 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -316,7 +316,8 @@ struct rte_port {
 	queueid_t               queue_nb; /**< nb. of queues for flow rules */
 	uint32_t                queue_sz; /**< size of a queue for flow rules */
 	uint8_t                 slave_flag : 1, /**< bonding slave port */
-				bond_flag : 1; /**< port is bond device */
+				bond_flag : 1, /**< port is bond device */
+				update_conf : 1; /**< need to update bonding device configuration */
 	struct port_template    *pattern_templ_list; /**< Pattern templates. */
 	struct port_template    *actions_templ_list; /**< Actions templates. */
 	struct port_table       *table_list; /**< Flow tables. */
diff --git a/drivers/net/bonding/bonding_testpmd.c b/drivers/net/bonding/bonding_testpmd.c
index 3941f4cf23..9529e16fb6 100644
--- a/drivers/net/bonding/bonding_testpmd.c
+++ b/drivers/net/bonding/bonding_testpmd.c
@@ -625,6 +625,7 @@ static void cmd_add_bonding_slave_parsed(void *parsed_result,
 			slave_port_id, master_port_id);
 		return;
 	}
+	ports[master_port_id].update_conf = 1;
 	init_port_config();
 	set_port_slave_flag(slave_port_id);
 }
@@ -762,6 +763,7 @@ static void cmd_create_bonded_device_parsed(void *parsed_result,
 		fprintf(stderr, "Failed to enable promiscuous mode for port %u: %s - ignore\n",
 			port_id, rte_strerror(-ret));
 
+	ports[port_id].update_conf = 1;
 	ports[port_id].bond_flag = 1;
 	ports[port_id].need_setup = 0;
 	ports[port_id].port_status = RTE_PORT_STOPPED;
-- 
2.33.0

[PATCH V2] app/testpmd: update bond port configurations when add slave

[Huisong Li]

Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
device in dev_info is zero when no slave is added. And its capability will
be updated when add a new slave device.

The capability to update dynamically may introduce some problems if not
handled properly. For example, the reconfig() is called to initialize
bonding port configurations when create a bonding device. The global
tx_mode is assigned to dev_conf.txmode. The DEV_TX_OFFLOAD_MBUF_FAST_FREE
which is the default value of global tx_mode.offloads in testpmd is removed
from bonding device configuration because of zero rx_offload_capa.
As a result, this offload isn't set to bonding device.

Generally, port configurations of bonding device must be within the
intersection of the capability of all slave devices. If use original port
configurations, the removed capabilities because of adding a new slave may
cause failure when re-initialize bonding device.

So port configurations of bonding device also need to be updated because of
the added and removed capabilities. In addition, this also helps to ensure
consistency between testpmd and bonding device.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 - v2: fix a spelling error in commit log

---
 app/test-pmd/testpmd.c                | 40 +++++++++++++++++++++++++++
 app/test-pmd/testpmd.h                |  3 +-
 drivers/net/bonding/bonding_testpmd.c |  2 ++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 97adafacd0..7324b8865c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2805,6 +2805,41 @@ fill_xstats_display_info(void)
 		fill_xstats_display_info_for_port(pi);
 }
 
+/*
+ * Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
+ * device in dev_info is zero when no slave is added. And its capability of
+ * will be updated when add a new slave device. So adding a device slave need
+ * to update the port configurations of bonding device.
+ */
+static void
+update_bonding_port_dev_conf(portid_t bond_pid)
+{
+#ifdef RTE_NET_BOND
+	struct rte_port *port = &ports[bond_pid];
+	uint16_t i;
+	int ret;
+
+	ret = eth_dev_info_get_print_err(bond_pid, &port->dev_info);
+	if (ret != 0) {
+		fprintf(stderr, "Failed to get dev info for port = %u\n",
+			bond_pid);
+		return;
+	}
+
+	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;
+	/* 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;
+
+	port->dev_conf.rx_adv_conf.rss_conf.rss_hf &=
+				port->dev_info.flow_type_rss_offloads;
+#else
+	RTE_SET_USED(bond_pid);
+#endif
+}
+
 int
 start_port(portid_t pid)
 {
@@ -2869,6 +2904,11 @@ start_port(portid_t pid)
 				return -1;
 			}
 
+			if (port->bond_flag == 1 && port->update_conf == 1) {
+				update_bonding_port_dev_conf(pi);
+				port->update_conf = 0;
+			}
+
 			/* configure port */
 			diag = eth_dev_configure_mp(pi, nb_rxq + nb_hairpinq,
 						     nb_txq + nb_hairpinq,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7fef96f9b1..82714119e8 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -316,7 +316,8 @@ struct rte_port {
 	queueid_t               queue_nb; /**< nb. of queues for flow rules */
 	uint32_t                queue_sz; /**< size of a queue for flow rules */
 	uint8_t                 slave_flag : 1, /**< bonding slave port */
-				bond_flag : 1; /**< port is bond device */
+				bond_flag : 1, /**< port is bond device */
+				update_conf : 1; /**< need to update bonding device configuration */
 	struct port_template    *pattern_templ_list; /**< Pattern templates. */
 	struct port_template    *actions_templ_list; /**< Actions templates. */
 	struct port_table       *table_list; /**< Flow tables. */
diff --git a/drivers/net/bonding/bonding_testpmd.c b/drivers/net/bonding/bonding_testpmd.c
index 3941f4cf23..9529e16fb6 100644
--- a/drivers/net/bonding/bonding_testpmd.c
+++ b/drivers/net/bonding/bonding_testpmd.c
@@ -625,6 +625,7 @@ static void cmd_add_bonding_slave_parsed(void *parsed_result,
 			slave_port_id, master_port_id);
 		return;
 	}
+	ports[master_port_id].update_conf = 1;
 	init_port_config();
 	set_port_slave_flag(slave_port_id);
 }
@@ -762,6 +763,7 @@ static void cmd_create_bonded_device_parsed(void *parsed_result,
 		fprintf(stderr, "Failed to enable promiscuous mode for port %u: %s - ignore\n",
 			port_id, rte_strerror(-ret));
 
+	ports[port_id].update_conf = 1;
 	ports[port_id].bond_flag = 1;
 	ports[port_id].need_setup = 0;
 	ports[port_id].port_status = RTE_PORT_STOPPED;
-- 
2.33.0

Re: [PATCH V2] app/testpmd: update bond port configurations when add slave

[humin (Q)]

在 2022/10/28 14:20, Huisong Li 写道:
> Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
> device in dev_info is zero when no slave is added. And its capability will
> be updated when add a new slave device.
>
> The capability to update dynamically may introduce some problems if not
> handled properly. For example, the reconfig() is called to initialize
> bonding port configurations when create a bonding device. The global
> tx_mode is assigned to dev_conf.txmode. The DEV_TX_OFFLOAD_MBUF_FAST_FREE
> which is the default value of global tx_mode.offloads in testpmd is removed
> from bonding device configuration because of zero rx_offload_capa.
> As a result, this offload isn't set to bonding device.
>
> Generally, port configurations of bonding device must be within the
> intersection of the capability of all slave devices. If use original port
> configurations, the removed capabilities because of adding a new slave may
> cause failure when re-initialize bonding device.
>
> So port configurations of bonding device also need to be updated because of
> the added and removed capabilities. In addition, this also helps to ensure
> consistency between testpmd and bonding device.
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>   - v2: fix a spelling error in commit log
>
> ---
>   app/test-pmd/testpmd.c                | 40 +++++++++++++++++++++++++++
>   app/test-pmd/testpmd.h                |  3 +-
>   drivers/net/bonding/bonding_testpmd.c |  2 ++
>   3 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 97adafacd0..7324b8865c 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2805,6 +2805,41 @@ fill_xstats_display_info(void)
>   		fill_xstats_display_info_for_port(pi);
>   }
>   
> +/*
> + * Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
> + * device in dev_info is zero when no slave is added. And its capability of

“And its capability of will be ”, what does this mean ?

> + * will be updated when add a new slave device. So adding a device slave need
> + * to update the port configurations of bonding device.
> + */
> +static void
> +update_bonding_port_dev_conf(portid_t bond_pid)
> +{
> +#ifdef RTE_NET_BOND
> +	struct rte_port *port = &ports[bond_pid];
> +	uint16_t i;
> +	int ret;
> +
> +	ret = eth_dev_info_get_print_err(bond_pid, &port->dev_info);
> +	if (ret != 0) {
> +		fprintf(stderr, "Failed to get dev info for port = %u\n",
> +			bond_pid);
> +		return;
> +	}
> +
> +	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;
> +	/* 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;
> +
> +	port->dev_conf.rx_adv_conf.rss_conf.rss_hf &=
> +				port->dev_info.flow_type_rss_offloads;
> +#else
> +	RTE_SET_USED(bond_pid);
> +#endif
> +}
> +
>   int
>   start_port(portid_t pid)
>   {
> @@ -2869,6 +2904,11 @@ start_port(portid_t pid)
>   				return -1;
>   			}
>   
> +			if (port->bond_flag == 1 && port->update_conf == 1) {
> +				update_bonding_port_dev_conf(pi);
> +				port->update_conf = 0;
> +			}
> +
>   			/* configure port */
>   			diag = eth_dev_configure_mp(pi, nb_rxq + nb_hairpinq,
>   						     nb_txq + nb_hairpinq,
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 7fef96f9b1..82714119e8 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -316,7 +316,8 @@ struct rte_port {
>   	queueid_t               queue_nb; /**< nb. of queues for flow rules */
>   	uint32_t                queue_sz; /**< size of a queue for flow rules */
>   	uint8_t                 slave_flag : 1, /**< bonding slave port */
> -				bond_flag : 1; /**< port is bond device */
> +				bond_flag : 1, /**< port is bond device */
> +				update_conf : 1; /**< need to update bonding device configuration */
>   	struct port_template    *pattern_templ_list; /**< Pattern templates. */
>   	struct port_template    *actions_templ_list; /**< Actions templates. */
>   	struct port_table       *table_list; /**< Flow tables. */
> diff --git a/drivers/net/bonding/bonding_testpmd.c b/drivers/net/bonding/bonding_testpmd.c
> index 3941f4cf23..9529e16fb6 100644
> --- a/drivers/net/bonding/bonding_testpmd.c
> +++ b/drivers/net/bonding/bonding_testpmd.c
> @@ -625,6 +625,7 @@ static void cmd_add_bonding_slave_parsed(void *parsed_result,
>   			slave_port_id, master_port_id);
>   		return;
>   	}
> +	ports[master_port_id].update_conf = 1;
>   	init_port_config();
>   	set_port_slave_flag(slave_port_id);
>   }
> @@ -762,6 +763,7 @@ static void cmd_create_bonded_device_parsed(void *parsed_result,
>   		fprintf(stderr, "Failed to enable promiscuous mode for port %u: %s - ignore\n",
>   			port_id, rte_strerror(-ret));
>   
> +	ports[port_id].update_conf = 1;
>   	ports[port_id].bond_flag = 1;
>   	ports[port_id].need_setup = 0;
>   	ports[port_id].port_status = RTE_PORT_STOPPED;

Re: [PATCH V2] app/testpmd: update bond port configurations when add slave

[lihuisong (C)]

在 2022/10/28 18:59, humin (Q) 写道:
>
> 在 2022/10/28 14:20, Huisong Li 写道:
>> Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
>> device in dev_info is zero when no slave is added. And its capability 
>> will
>> be updated when add a new slave device.
>>
>> The capability to update dynamically may introduce some problems if not
>> handled properly. For example, the reconfig() is called to initialize
>> bonding port configurations when create a bonding device. The global
>> tx_mode is assigned to dev_conf.txmode. The 
>> DEV_TX_OFFLOAD_MBUF_FAST_FREE
>> which is the default value of global tx_mode.offloads in testpmd is 
>> removed
>> from bonding device configuration because of zero rx_offload_capa.
>> As a result, this offload isn't set to bonding device.
>>
>> Generally, port configurations of bonding device must be within the
>> intersection of the capability of all slave devices. If use original 
>> port
>> configurations, the removed capabilities because of adding a new 
>> slave may
>> cause failure when re-initialize bonding device.
>>
>> So port configurations of bonding device also need to be updated 
>> because of
>> the added and removed capabilities. In addition, this also helps to 
>> ensure
>> consistency between testpmd and bonding device.
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   - v2: fix a spelling error in commit log
>>
>> ---
>>   app/test-pmd/testpmd.c                | 40 +++++++++++++++++++++++++++
>>   app/test-pmd/testpmd.h                |  3 +-
>>   drivers/net/bonding/bonding_testpmd.c |  2 ++
>>   3 files changed, 44 insertions(+), 1 deletion(-)
>>
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index 97adafacd0..7324b8865c 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -2805,6 +2805,41 @@ fill_xstats_display_info(void)
>>           fill_xstats_display_info_for_port(pi);
>>   }
>>   +/*
>> + * Some capabilities (like, rx_offload_capa and tx_offload_capa) of 
>> bonding
>> + * device in dev_info is zero when no slave is added. And its 
>> capability of
>
> “And its capability of will be ”, what does this mean ?
will fix in next version.
>
>> + * will be updated when add a new slave device. So adding a device 
>> slave need
>> + * to update the port configurations of bonding device.
>> + */
>> +static void
>> +update_bonding_port_dev_conf(portid_t bond_pid)
>> +{
>> +#ifdef RTE_NET_BOND
>> +    struct rte_port *port = &ports[bond_pid];
>> +    uint16_t i;
>> +    int ret;
>> +
>> +    ret = eth_dev_info_get_print_err(bond_pid, &port->dev_info);
>> +    if (ret != 0) {
>> +        fprintf(stderr, "Failed to get dev info for port = %u\n",
>> +            bond_pid);
>> +        return;
>> +    }
>> +
>> +    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;
>> +    /* 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;
>> +
>> +    port->dev_conf.rx_adv_conf.rss_conf.rss_hf &=
>> +                port->dev_info.flow_type_rss_offloads;
>> +#else
>> +    RTE_SET_USED(bond_pid);
>> +#endif
>> +}
>> +
>>   int
>>   start_port(portid_t pid)
>>   {
>> @@ -2869,6 +2904,11 @@ start_port(portid_t pid)
>>                   return -1;
>>               }
>>   +            if (port->bond_flag == 1 && port->update_conf == 1) {
>> +                update_bonding_port_dev_conf(pi);
>> +                port->update_conf = 0;
>> +            }
>> +
>>               /* configure port */
>>               diag = eth_dev_configure_mp(pi, nb_rxq + nb_hairpinq,
>>                                nb_txq + nb_hairpinq,
>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>> index 7fef96f9b1..82714119e8 100644
>> --- a/app/test-pmd/testpmd.h
>> +++ b/app/test-pmd/testpmd.h
>> @@ -316,7 +316,8 @@ struct rte_port {
>>       queueid_t               queue_nb; /**< nb. of queues for flow 
>> rules */
>>       uint32_t                queue_sz; /**< size of a queue for flow 
>> rules */
>>       uint8_t                 slave_flag : 1, /**< bonding slave port */
>> -                bond_flag : 1; /**< port is bond device */
>> +                bond_flag : 1, /**< port is bond device */
>> +                update_conf : 1; /**< need to update bonding device 
>> configuration */
>>       struct port_template    *pattern_templ_list; /**< Pattern 
>> templates. */
>>       struct port_template    *actions_templ_list; /**< Actions 
>> templates. */
>>       struct port_table       *table_list; /**< Flow tables. */
>> diff --git a/drivers/net/bonding/bonding_testpmd.c 
>> b/drivers/net/bonding/bonding_testpmd.c
>> index 3941f4cf23..9529e16fb6 100644
>> --- a/drivers/net/bonding/bonding_testpmd.c
>> +++ b/drivers/net/bonding/bonding_testpmd.c
>> @@ -625,6 +625,7 @@ static void cmd_add_bonding_slave_parsed(void 
>> *parsed_result,
>>               slave_port_id, master_port_id);
>>           return;
>>       }
>> +    ports[master_port_id].update_conf = 1;
>>       init_port_config();
>>       set_port_slave_flag(slave_port_id);
>>   }
>> @@ -762,6 +763,7 @@ static void cmd_create_bonded_device_parsed(void 
>> *parsed_result,
>>           fprintf(stderr, "Failed to enable promiscuous mode for port 
>> %u: %s - ignore\n",
>>               port_id, rte_strerror(-ret));
>>   +    ports[port_id].update_conf = 1;
>>       ports[port_id].bond_flag = 1;
>>       ports[port_id].need_setup = 0;
>>       ports[port_id].port_status = RTE_PORT_STOPPED;
> .

[PATCH V3] app/testpmd: update bond port configurations when add slave

[Huisong Li]

Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
device in dev_info is zero when no slave is added. And its capability will
be updated when add a new slave device.

The capability to update dynamically may introduce some problems if not
handled properly. For example, the reconfig() is called to initialize
bonding port configurations when create a bonding device. The global
tx_mode is assigned to dev_conf.txmode. The DEV_TX_OFFLOAD_MBUF_FAST_FREE
which is the default value of global tx_mode.offloads in testpmd is removed
from bonding device configuration because of zero rx_offload_capa.
As a result, this offload isn't set to bonding device.

Generally, port configurations of bonding device must be within the
intersection of the capability of all slave devices. If use original port
configurations, the removed capabilities because of adding a new slave may
cause failure when re-initialize bonding device.

So port configurations of bonding device also need to be updated because of
the added and removed capabilities. In addition, this also helps to ensure
consistency between testpmd and bonding device.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 - v3: fix code comment
 - v2: fix a spelling error in commit log
---
 app/test-pmd/testpmd.c                | 40 +++++++++++++++++++++++++++
 app/test-pmd/testpmd.h                |  3 +-
 drivers/net/bonding/bonding_testpmd.c |  2 ++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 97adafacd0..7c9de07367 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2805,6 +2805,41 @@ fill_xstats_display_info(void)
 		fill_xstats_display_info_for_port(pi);
 }
 
+/*
+ * Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
+ * device in dev_info is zero when no slave is added. And its capability
+ * will be updated when add a new slave device. So adding a device slave need
+ * to update the port configurations of bonding device.
+ */
+static void
+update_bonding_port_dev_conf(portid_t bond_pid)
+{
+#ifdef RTE_NET_BOND
+	struct rte_port *port = &ports[bond_pid];
+	uint16_t i;
+	int ret;
+
+	ret = eth_dev_info_get_print_err(bond_pid, &port->dev_info);
+	if (ret != 0) {
+		fprintf(stderr, "Failed to get dev info for port = %u\n",
+			bond_pid);
+		return;
+	}
+
+	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;
+	/* 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;
+
+	port->dev_conf.rx_adv_conf.rss_conf.rss_hf &=
+				port->dev_info.flow_type_rss_offloads;
+#else
+	RTE_SET_USED(bond_pid);
+#endif
+}
+
 int
 start_port(portid_t pid)
 {
@@ -2869,6 +2904,11 @@ start_port(portid_t pid)
 				return -1;
 			}
 
+			if (port->bond_flag == 1 && port->update_conf == 1) {
+				update_bonding_port_dev_conf(pi);
+				port->update_conf = 0;
+			}
+
 			/* configure port */
 			diag = eth_dev_configure_mp(pi, nb_rxq + nb_hairpinq,
 						     nb_txq + nb_hairpinq,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 7fef96f9b1..82714119e8 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -316,7 +316,8 @@ struct rte_port {
 	queueid_t               queue_nb; /**< nb. of queues for flow rules */
 	uint32_t                queue_sz; /**< size of a queue for flow rules */
 	uint8_t                 slave_flag : 1, /**< bonding slave port */
-				bond_flag : 1; /**< port is bond device */
+				bond_flag : 1, /**< port is bond device */
+				update_conf : 1; /**< need to update bonding device configuration */
 	struct port_template    *pattern_templ_list; /**< Pattern templates. */
 	struct port_template    *actions_templ_list; /**< Actions templates. */
 	struct port_table       *table_list; /**< Flow tables. */
diff --git a/drivers/net/bonding/bonding_testpmd.c b/drivers/net/bonding/bonding_testpmd.c
index 3941f4cf23..9529e16fb6 100644
--- a/drivers/net/bonding/bonding_testpmd.c
+++ b/drivers/net/bonding/bonding_testpmd.c
@@ -625,6 +625,7 @@ static void cmd_add_bonding_slave_parsed(void *parsed_result,
 			slave_port_id, master_port_id);
 		return;
 	}
+	ports[master_port_id].update_conf = 1;
 	init_port_config();
 	set_port_slave_flag(slave_port_id);
 }
@@ -762,6 +763,7 @@ static void cmd_create_bonded_device_parsed(void *parsed_result,
 		fprintf(stderr, "Failed to enable promiscuous mode for port %u: %s - ignore\n",
 			port_id, rte_strerror(-ret));
 
+	ports[port_id].update_conf = 1;
 	ports[port_id].bond_flag = 1;
 	ports[port_id].need_setup = 0;
 	ports[port_id].port_status = RTE_PORT_STOPPED;
-- 
2.33.0

Re: [PATCH V3] app/testpmd: update bond port configurations when add slave

[humin (Q)]

Reviewed-by: Min Hu (Connor) <humin29huawei.com>

在 2022/10/29 11:50, Huisong Li 写道:
> Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
> device in dev_info is zero when no slave is added. And its capability will
> be updated when add a new slave device.
>
> The capability to update dynamically may introduce some problems if not
> handled properly. For example, the reconfig() is called to initialize
> bonding port configurations when create a bonding device. The global
> tx_mode is assigned to dev_conf.txmode. The DEV_TX_OFFLOAD_MBUF_FAST_FREE
> which is the default value of global tx_mode.offloads in testpmd is removed
> from bonding device configuration because of zero rx_offload_capa.
> As a result, this offload isn't set to bonding device.
>
> Generally, port configurations of bonding device must be within the
> intersection of the capability of all slave devices. If use original port
> configurations, the removed capabilities because of adding a new slave may
> cause failure when re-initialize bonding device.
>
> So port configurations of bonding device also need to be updated because of
> the added and removed capabilities. In addition, this also helps to ensure
> consistency between testpmd and bonding device.
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>   - v3: fix code comment
>   - v2: fix a spelling error in commit log
> ---
>   app/test-pmd/testpmd.c                | 40 +++++++++++++++++++++++++++
>   app/test-pmd/testpmd.h                |  3 +-
>   drivers/net/bonding/bonding_testpmd.c |  2 ++
>   3 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 97adafacd0..7c9de07367 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -2805,6 +2805,41 @@ fill_xstats_display_info(void)
>   		fill_xstats_display_info_for_port(pi);
>   }
>   
> +/*
> + * Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
> + * device in dev_info is zero when no slave is added. And its capability
> + * will be updated when add a new slave device. So adding a device slave need
> + * to update the port configurations of bonding device.
> + */
> +static void
> +update_bonding_port_dev_conf(portid_t bond_pid)
> +{
> +#ifdef RTE_NET_BOND
> +	struct rte_port *port = &ports[bond_pid];
> +	uint16_t i;
> +	int ret;
> +
> +	ret = eth_dev_info_get_print_err(bond_pid, &port->dev_info);
> +	if (ret != 0) {
> +		fprintf(stderr, "Failed to get dev info for port = %u\n",
> +			bond_pid);
> +		return;
> +	}
> +
> +	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;
> +	/* 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;
> +
> +	port->dev_conf.rx_adv_conf.rss_conf.rss_hf &=
> +				port->dev_info.flow_type_rss_offloads;
> +#else
> +	RTE_SET_USED(bond_pid);
> +#endif
> +}
> +
>   int
>   start_port(portid_t pid)
>   {
> @@ -2869,6 +2904,11 @@ start_port(portid_t pid)
>   				return -1;
>   			}
>   
> +			if (port->bond_flag == 1 && port->update_conf == 1) {
> +				update_bonding_port_dev_conf(pi);
> +				port->update_conf = 0;
> +			}
> +
>   			/* configure port */
>   			diag = eth_dev_configure_mp(pi, nb_rxq + nb_hairpinq,
>   						     nb_txq + nb_hairpinq,
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 7fef96f9b1..82714119e8 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -316,7 +316,8 @@ struct rte_port {
>   	queueid_t               queue_nb; /**< nb. of queues for flow rules */
>   	uint32_t                queue_sz; /**< size of a queue for flow rules */
>   	uint8_t                 slave_flag : 1, /**< bonding slave port */
> -				bond_flag : 1; /**< port is bond device */
> +				bond_flag : 1, /**< port is bond device */
> +				update_conf : 1; /**< need to update bonding device configuration */
>   	struct port_template    *pattern_templ_list; /**< Pattern templates. */
>   	struct port_template    *actions_templ_list; /**< Actions templates. */
>   	struct port_table       *table_list; /**< Flow tables. */
> diff --git a/drivers/net/bonding/bonding_testpmd.c b/drivers/net/bonding/bonding_testpmd.c
> index 3941f4cf23..9529e16fb6 100644
> --- a/drivers/net/bonding/bonding_testpmd.c
> +++ b/drivers/net/bonding/bonding_testpmd.c
> @@ -625,6 +625,7 @@ static void cmd_add_bonding_slave_parsed(void *parsed_result,
>   			slave_port_id, master_port_id);
>   		return;
>   	}
> +	ports[master_port_id].update_conf = 1;
>   	init_port_config();
>   	set_port_slave_flag(slave_port_id);
>   }
> @@ -762,6 +763,7 @@ static void cmd_create_bonded_device_parsed(void *parsed_result,
>   		fprintf(stderr, "Failed to enable promiscuous mode for port %u: %s - ignore\n",
>   			port_id, rte_strerror(-ret));
>   
> +	ports[port_id].update_conf = 1;
>   	ports[port_id].bond_flag = 1;
>   	ports[port_id].need_setup = 0;
>   	ports[port_id].port_status = RTE_PORT_STOPPED;

Re: [PATCH V3] app/testpmd: update bond port configurations when add slave

[Andrew Rybchenko]

On 11/1/22 06:41, humin (Q) wrote:
> Reviewed-by: Min Hu (Connor) <humin29huawei.com>
> 
> 在 2022/10/29 11:50, Huisong Li 写道:
>> Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
>> device in dev_info is zero when no slave is added. And its capability 
>> will
>> be updated when add a new slave device.
>>
>> The capability to update dynamically may introduce some problems if not
>> handled properly. For example, the reconfig() is called to initialize
>> bonding port configurations when create a bonding device. The global
>> tx_mode is assigned to dev_conf.txmode. The DEV_TX_OFFLOAD_MBUF_FAST_FREE
>> which is the default value of global tx_mode.offloads in testpmd is 
>> removed
>> from bonding device configuration because of zero rx_offload_capa.
>> As a result, this offload isn't set to bonding device.
>>
>> Generally, port configurations of bonding device must be within the
>> intersection of the capability of all slave devices. If use original port
>> configurations, the removed capabilities because of adding a new slave 
>> may
>> cause failure when re-initialize bonding device.
>>
>> So port configurations of bonding device also need to be updated 
>> because of
>> the added and removed capabilities. In addition, this also helps to 
>> ensure
>> consistency between testpmd and bonding device.
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   - v3: fix code comment
>>   - v2: fix a spelling error in commit log

It fails to apply on dpdk-next-net/main. Could you rebase it,
please, and send v4. Thanks.

[PATCH V4] app/testpmd: update bond port configurations when add slave

[Huisong Li]

Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
device in dev_info is zero when no slave is added. And its capability will
be updated when add a new slave device.

The capability to update dynamically may introduce some problems if not
handled properly. For example, the reconfig() is called to initialize
bonding port configurations when create a bonding device. The global
tx_mode is assigned to dev_conf.txmode. The DEV_TX_OFFLOAD_MBUF_FAST_FREE
which is the default value of global tx_mode.offloads in testpmd is removed
from bonding device configuration because of zero rx_offload_capa.
As a result, this offload isn't set to bonding device.

Generally, port configurations of bonding device must be within the
intersection of the capability of all slave devices. If use original port
configurations, the removed capabilities because of adding a new slave may
cause failure when re-initialize bonding device.

So port configurations of bonding device need to be updated because of the
added and removed capabilities. In addition, this also helps to ensure
consistency between testpmd and bonding device.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Reviewed-by: Min Hu (Connor) <humin29huawei.com>
---
 - v4: resolve applying failure on dpdk-next-net/main
 - v3: fix code comment
 - v2: fix a spelling error in commit log
---
 app/test-pmd/testpmd.c                | 40 +++++++++++++++++++++++++++
 app/test-pmd/testpmd.h                |  3 +-
 drivers/net/bonding/bonding_testpmd.c |  2 ++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index bf589c4e8d..7381dfd9e5 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2801,6 +2801,41 @@ fill_xstats_display_info(void)
 		fill_xstats_display_info_for_port(pi);
 }
 
+/*
+ * Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
+ * device in dev_info is zero when no slave is added. And its capability
+ * will be updated when add a new slave device. So adding a slave device need
+ * to update the port configurations of bonding device.
+ */
+static void
+update_bonding_port_dev_conf(portid_t bond_pid)
+{
+#ifdef RTE_NET_BOND
+	struct rte_port *port = &ports[bond_pid];
+	uint16_t i;
+	int ret;
+
+	ret = eth_dev_info_get_print_err(bond_pid, &port->dev_info);
+	if (ret != 0) {
+		fprintf(stderr, "Failed to get dev info for port = %u\n",
+			bond_pid);
+		return;
+	}
+
+	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;
+	/* 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;
+
+	port->dev_conf.rx_adv_conf.rss_conf.rss_hf &=
+				port->dev_info.flow_type_rss_offloads;
+#else
+	RTE_SET_USED(bond_pid);
+#endif
+}
+
 int
 start_port(portid_t pid)
 {
@@ -2865,6 +2900,11 @@ start_port(portid_t pid)
 				return -1;
 			}
 
+			if (port->bond_flag == 1 && port->update_conf == 1) {
+				update_bonding_port_dev_conf(pi);
+				port->update_conf = 0;
+			}
+
 			/* configure port */
 			diag = eth_dev_configure_mp(pi, nb_rxq + nb_hairpinq,
 						     nb_txq + nb_hairpinq,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 93fdb9d331..8b5a3cdb30 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -317,7 +317,8 @@ struct rte_port {
 	uint32_t                queue_sz; /**< size of a queue for flow rules */
 	uint8_t                 slave_flag : 1, /**< bonding slave port */
 				bond_flag : 1, /**< port is bond device */
-				fwd_mac_swap : 1; /**< swap packet MAC before forward */
+				fwd_mac_swap : 1, /**< swap packet MAC before forward */
+				update_conf : 1; /**< need to update bonding device configuration */
 	struct port_template    *pattern_templ_list; /**< Pattern templates. */
 	struct port_template    *actions_templ_list; /**< Actions templates. */
 	struct port_table       *table_list; /**< Flow tables. */
diff --git a/drivers/net/bonding/bonding_testpmd.c b/drivers/net/bonding/bonding_testpmd.c
index 3941f4cf23..9529e16fb6 100644
--- a/drivers/net/bonding/bonding_testpmd.c
+++ b/drivers/net/bonding/bonding_testpmd.c
@@ -625,6 +625,7 @@ static void cmd_add_bonding_slave_parsed(void *parsed_result,
 			slave_port_id, master_port_id);
 		return;
 	}
+	ports[master_port_id].update_conf = 1;
 	init_port_config();
 	set_port_slave_flag(slave_port_id);
 }
@@ -762,6 +763,7 @@ static void cmd_create_bonded_device_parsed(void *parsed_result,
 		fprintf(stderr, "Failed to enable promiscuous mode for port %u: %s - ignore\n",
 			port_id, rte_strerror(-ret));
 
+	ports[port_id].update_conf = 1;
 	ports[port_id].bond_flag = 1;
 	ports[port_id].need_setup = 0;
 	ports[port_id].port_status = RTE_PORT_STOPPED;
-- 
2.33.0

[PATCH V5] app/testpmd: update bond port configurations when add slave

[Huisong Li]

Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
device in dev_info is zero when no slave is added. And its capability will
be updated when add a new slave device.

The capability to update dynamically may introduce some problems if not
handled properly. For example, the reconfig() is called to initialize
bonding port configurations when create a bonding device. The global
tx_mode is assigned to dev_conf.txmode. The DEV_TX_OFFLOAD_MBUF_FAST_FREE
which is the default value of global tx_mode.offloads in testpmd is removed
from bonding device configuration because of zero rx_offload_capa.
As a result, this offload isn't set to bonding device.

Generally, port configurations of bonding device must be within the
intersection of the capability of all slave devices. If use original port
configurations, the removed capabilities because of adding a new slave may
cause failure when re-initialize bonding device.

So port configurations of bonding device need to be updated because of the
added and removed capabilities. In addition, this also helps to ensure
consistency between testpmd and bonding device.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Reviewed-by: Min Hu (Connor) <humin29@huawei.com>
---
 - v5: fix unrecognized email address
 - v4: resolve applying failure on dpdk-next-net/main
 - v3: fix code comment
 - v2: fix a spelling error in commit log
---
 app/test-pmd/testpmd.c                | 40 +++++++++++++++++++++++++++
 app/test-pmd/testpmd.h                |  3 +-
 drivers/net/bonding/bonding_testpmd.c |  2 ++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index bf589c4e8d..7381dfd9e5 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2801,6 +2801,41 @@ fill_xstats_display_info(void)
 		fill_xstats_display_info_for_port(pi);
 }
 
+/*
+ * Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
+ * device in dev_info is zero when no slave is added. And its capability
+ * will be updated when add a new slave device. So adding a slave device need
+ * to update the port configurations of bonding device.
+ */
+static void
+update_bonding_port_dev_conf(portid_t bond_pid)
+{
+#ifdef RTE_NET_BOND
+	struct rte_port *port = &ports[bond_pid];
+	uint16_t i;
+	int ret;
+
+	ret = eth_dev_info_get_print_err(bond_pid, &port->dev_info);
+	if (ret != 0) {
+		fprintf(stderr, "Failed to get dev info for port = %u\n",
+			bond_pid);
+		return;
+	}
+
+	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;
+	/* 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;
+
+	port->dev_conf.rx_adv_conf.rss_conf.rss_hf &=
+				port->dev_info.flow_type_rss_offloads;
+#else
+	RTE_SET_USED(bond_pid);
+#endif
+}
+
 int
 start_port(portid_t pid)
 {
@@ -2865,6 +2900,11 @@ start_port(portid_t pid)
 				return -1;
 			}
 
+			if (port->bond_flag == 1 && port->update_conf == 1) {
+				update_bonding_port_dev_conf(pi);
+				port->update_conf = 0;
+			}
+
 			/* configure port */
 			diag = eth_dev_configure_mp(pi, nb_rxq + nb_hairpinq,
 						     nb_txq + nb_hairpinq,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 93fdb9d331..8b5a3cdb30 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -317,7 +317,8 @@ struct rte_port {
 	uint32_t                queue_sz; /**< size of a queue for flow rules */
 	uint8_t                 slave_flag : 1, /**< bonding slave port */
 				bond_flag : 1, /**< port is bond device */
-				fwd_mac_swap : 1; /**< swap packet MAC before forward */
+				fwd_mac_swap : 1, /**< swap packet MAC before forward */
+				update_conf : 1; /**< need to update bonding device configuration */
 	struct port_template    *pattern_templ_list; /**< Pattern templates. */
 	struct port_template    *actions_templ_list; /**< Actions templates. */
 	struct port_table       *table_list; /**< Flow tables. */
diff --git a/drivers/net/bonding/bonding_testpmd.c b/drivers/net/bonding/bonding_testpmd.c
index 3941f4cf23..9529e16fb6 100644
--- a/drivers/net/bonding/bonding_testpmd.c
+++ b/drivers/net/bonding/bonding_testpmd.c
@@ -625,6 +625,7 @@ static void cmd_add_bonding_slave_parsed(void *parsed_result,
 			slave_port_id, master_port_id);
 		return;
 	}
+	ports[master_port_id].update_conf = 1;
 	init_port_config();
 	set_port_slave_flag(slave_port_id);
 }
@@ -762,6 +763,7 @@ static void cmd_create_bonded_device_parsed(void *parsed_result,
 		fprintf(stderr, "Failed to enable promiscuous mode for port %u: %s - ignore\n",
 			port_id, rte_strerror(-ret));
 
+	ports[port_id].update_conf = 1;
 	ports[port_id].bond_flag = 1;
 	ports[port_id].need_setup = 0;
 	ports[port_id].port_status = RTE_PORT_STOPPED;
-- 
2.33.0

Re: [PATCH V5] app/testpmd: update bond port configurations when add slave

[Andrew Rybchenko]

On 11/7/22 05:18, Huisong Li wrote:
> Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
> device in dev_info is zero when no slave is added. And its capability will
> be updated when add a new slave device.
> 
> The capability to update dynamically may introduce some problems if not
> handled properly. For example, the reconfig() is called to initialize
> bonding port configurations when create a bonding device. The global
> tx_mode is assigned to dev_conf.txmode. The DEV_TX_OFFLOAD_MBUF_FAST_FREE
> which is the default value of global tx_mode.offloads in testpmd is removed
> from bonding device configuration because of zero rx_offload_capa.
> As a result, this offload isn't set to bonding device.
> 
> Generally, port configurations of bonding device must be within the
> intersection of the capability of all slave devices. If use original port
> configurations, the removed capabilities because of adding a new slave may
> cause failure when re-initialize bonding device.
> 
> So port configurations of bonding device need to be updated because of the
> added and removed capabilities. In addition, this also helps to ensure
> consistency between testpmd and bonding device.
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Reviewed-by: Min Hu (Connor) <humin29@huawei.com>

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