[dpdk-dev] [PATCH] net/bonding: switch to new offloading flags

[dpdk-dev] [PATCH] net/bonding: switch to new offloading flags

[Ferruh Yigit]

Switch from using deprecated bitfields in rxmode to offloads variable.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 drivers/net/bonding/rte_eth_bond_api.c | 3 ++-
 drivers/net/bonding/rte_eth_bond_pmd.c | 9 +++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index f854b7375..669004fec 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -194,7 +194,8 @@ slave_vlan_filter_set(uint16_t bonded_port_id, uint16_t slave_port_id)
 	uint16_t first;
 
 	bonded_eth_dev = &rte_eth_devices[bonded_port_id];
-	if (bonded_eth_dev->data->dev_conf.rxmode.hw_vlan_filter == 0)
+	if ((bonded_eth_dev->data->dev_conf.rxmode.offloads &
+			DEV_RX_OFFLOAD_VLAN_FILTER) == 0)
 		return 0;
 
 	internals = bonded_eth_dev->data->dev_private;
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index c34c3251f..c18aca222 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1818,8 +1818,13 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
 				bonded_eth_dev->data->dev_conf.rxmode.mq_mode;
 	}
 
-	slave_eth_dev->data->dev_conf.rxmode.hw_vlan_filter =
-			bonded_eth_dev->data->dev_conf.rxmode.hw_vlan_filter;
+	if (bonded_eth_dev->data->dev_conf.rxmode.offloads &
+			DEV_RX_OFFLOAD_VLAN_FILTER)
+		slave_eth_dev->data->dev_conf.rxmode.offloads |=
+				DEV_RX_OFFLOAD_VLAN_FILTER;
+	else
+		slave_eth_dev->data->dev_conf.rxmode.offloads &=
+				~DEV_RX_OFFLOAD_VLAN_FILTER;
 
 	nb_rx_queues = bonded_eth_dev->data->nb_rx_queues;
 	nb_tx_queues = bonded_eth_dev->data->nb_tx_queues;
-- 
2.13.6

Re: [dpdk-dev] [PATCH] net/bonding: switch to new offloading flags

[Radu Nicolau]

On 3/13/2018 12:24 PM, Ferruh Yigit wrote:
> Switch from using deprecated bitfields in rxmode to offloads variable.
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>
Acked-by: Radu Nicolau <radu.nicolau@intel.com>

Re: [dpdk-dev] [PATCH] net/bonding: switch to new offloading flags

[Matan Azrad]

Hi Ferruh

From: Ferruh Yigit, Sent: Tuesday, March 13, 2018 2:25 PM
> Switch from using deprecated bitfields in rxmode to offloads variable.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
>  drivers/net/bonding/rte_eth_bond_api.c | 3 ++-
> drivers/net/bonding/rte_eth_bond_pmd.c | 9 +++++++--
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_api.c
> b/drivers/net/bonding/rte_eth_bond_api.c
> index f854b7375..669004fec 100644
> --- a/drivers/net/bonding/rte_eth_bond_api.c
> +++ b/drivers/net/bonding/rte_eth_bond_api.c
> @@ -194,7 +194,8 @@ slave_vlan_filter_set(uint16_t bonded_port_id,
> uint16_t slave_port_id)
>  	uint16_t first;
> 
>  	bonded_eth_dev = &rte_eth_devices[bonded_port_id];
> -	if (bonded_eth_dev->data->dev_conf.rxmode.hw_vlan_filter == 0)
> +	if ((bonded_eth_dev->data->dev_conf.rxmode.offloads &
> +			DEV_RX_OFFLOAD_VLAN_FILTER) == 0)
>  		return 0;
> 
>  	internals = bonded_eth_dev->data->dev_private;
> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
> b/drivers/net/bonding/rte_eth_bond_pmd.c
> index c34c3251f..c18aca222 100644
> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
> @@ -1818,8 +1818,13 @@ slave_configure(struct rte_eth_dev
> *bonded_eth_dev,
>  				bonded_eth_dev->data-
> >dev_conf.rxmode.mq_mode;
>  	}
> 
> -	slave_eth_dev->data->dev_conf.rxmode.hw_vlan_filter =
> -			bonded_eth_dev->data-
> >dev_conf.rxmode.hw_vlan_filter;
> +	if (bonded_eth_dev->data->dev_conf.rxmode.offloads &
> +			DEV_RX_OFFLOAD_VLAN_FILTER)
> +		slave_eth_dev->data->dev_conf.rxmode.offloads |=
> +				DEV_RX_OFFLOAD_VLAN_FILTER;
> +	else
> +		slave_eth_dev->data->dev_conf.rxmode.offloads &=
> +				~DEV_RX_OFFLOAD_VLAN_FILTER;
> 
>  	nb_rx_queues = bonded_eth_dev->data->nb_rx_queues;
>  	nb_tx_queues = bonded_eth_dev->data->nb_tx_queues;
> --
> 2.13.6

The bonding PMD is using internal variables to save the offload capabilities (Actually holds the offloads intersection set of all the bond slaves).
I think you are missing next:
	You should change the next variable types to uint64_t to support the new offload flags:
		internals->rx_offload_capa
		internals->tx_offload_capa

	You should add the new per queue offload variables to save the intersection set of it too:
		rx_queue_offload_capa
		tx_queue_offload_capa

Questions:
Have you an idea why bonding PMD doesn't adjust the slaves port configurations to the bonding port configuration like he does for slave queue configuration?
Is the responsibility to fill the slave port configuration structure for the application?

What do you think about next configuration checks (both per port and per queue)?
	Validate the actual bonding offloads with the bonding capability.
	Validate that the queue offloads includes all the port configured offloads.

Matan.

Re: [dpdk-dev] [PATCH] net/bonding: switch to new offloading flags

[Ferruh Yigit]

On 3/14/2018 12:50 PM, Matan Azrad wrote:
> Hi Ferruh
> 
> From: Ferruh Yigit, Sent: Tuesday, March 13, 2018 2:25 PM
>> Switch from using deprecated bitfields in rxmode to offloads variable.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>>  drivers/net/bonding/rte_eth_bond_api.c | 3 ++-
>> drivers/net/bonding/rte_eth_bond_pmd.c | 9 +++++++--
>>  2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/bonding/rte_eth_bond_api.c
>> b/drivers/net/bonding/rte_eth_bond_api.c
>> index f854b7375..669004fec 100644
>> --- a/drivers/net/bonding/rte_eth_bond_api.c
>> +++ b/drivers/net/bonding/rte_eth_bond_api.c
>> @@ -194,7 +194,8 @@ slave_vlan_filter_set(uint16_t bonded_port_id,
>> uint16_t slave_port_id)
>>  	uint16_t first;
>>
>>  	bonded_eth_dev = &rte_eth_devices[bonded_port_id];
>> -	if (bonded_eth_dev->data->dev_conf.rxmode.hw_vlan_filter == 0)
>> +	if ((bonded_eth_dev->data->dev_conf.rxmode.offloads &
>> +			DEV_RX_OFFLOAD_VLAN_FILTER) == 0)
>>  		return 0;
>>
>>  	internals = bonded_eth_dev->data->dev_private;
>> diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c
>> b/drivers/net/bonding/rte_eth_bond_pmd.c
>> index c34c3251f..c18aca222 100644
>> --- a/drivers/net/bonding/rte_eth_bond_pmd.c
>> +++ b/drivers/net/bonding/rte_eth_bond_pmd.c
>> @@ -1818,8 +1818,13 @@ slave_configure(struct rte_eth_dev
>> *bonded_eth_dev,
>>  				bonded_eth_dev->data-
>>> dev_conf.rxmode.mq_mode;
>>  	}
>>
>> -	slave_eth_dev->data->dev_conf.rxmode.hw_vlan_filter =
>> -			bonded_eth_dev->data-
>>> dev_conf.rxmode.hw_vlan_filter;
>> +	if (bonded_eth_dev->data->dev_conf.rxmode.offloads &
>> +			DEV_RX_OFFLOAD_VLAN_FILTER)
>> +		slave_eth_dev->data->dev_conf.rxmode.offloads |=
>> +				DEV_RX_OFFLOAD_VLAN_FILTER;
>> +	else
>> +		slave_eth_dev->data->dev_conf.rxmode.offloads &=
>> +				~DEV_RX_OFFLOAD_VLAN_FILTER;
>>
>>  	nb_rx_queues = bonded_eth_dev->data->nb_rx_queues;
>>  	nb_tx_queues = bonded_eth_dev->data->nb_tx_queues;
>> --
>> 2.13.6
> 
> The bonding PMD is using internal variables to save the offload capabilities (Actually holds the offloads intersection set of all the bond slaves).
> I think you are missing next:
> 	You should change the next variable types to uint64_t to support the new offload flags:
> 		internals->rx_offload_capa
> 		internals->tx_offload_capa
> 
> 	You should add the new per queue offload variables to save the intersection set of it too:
> 		rx_queue_offload_capa
> 		tx_queue_offload_capa

Thanks, I will update this v2.

> 
> Questions:
> Have you an idea why bonding PMD doesn't adjust the slaves port configurations to the bonding port configuration like he does for slave queue configuration?
> Is the responsibility to fill the slave port configuration structure for the application?
> 
> What do you think about next configuration checks (both per port and per queue)?
> 	Validate the actual bonding offloads with the bonding capability.
> 	Validate that the queue offloads includes all the port configured offloads.
> 
> Matan.
> 

Re: [dpdk-dev] [PATCH] net/bonding: switch to new offloading flags

[Ferruh Yigit]

On 3/14/2018 12:50 PM, Matan Azrad wrote:
> Questions:
> Have you an idea why bonding PMD doesn't adjust the slaves port configurations to the bonding port configuration like he does for slave queue configuration?
> Is the responsibility to fill the slave port configuration structure for the application?
> 
> What do you think about next configuration checks (both per port and per queue)?
> 	Validate the actual bonding offloads with the bonding capability.
> 	Validate that the queue offloads includes all the port configured offloads.

I don't have answers for these, perhaps Declan or Radu can help.

Re: [dpdk-dev] [PATCH] net/bonding: switch to new offloading flags

[Matan Azrad]

Hi Declan, Radu

Please try to answer for the below questions.

From: Ferruh Yigit, Sent: Thursday, March 22, 2018 8:14 PM
> On 3/14/2018 12:50 PM, Matan Azrad wrote:
> > Questions:
> > Have you an idea why bonding PMD doesn't adjust the slaves port
> configurations to the bonding port configuration like it does for slave queue
> configuration?
> > Is the responsibility to fill the slave port configuration structure for the
> application?
> >
> > What do you think about next configuration checks (both per port and per
> queue)?
> > 	Validate the actual bonding offloads with the bonding capability.
> > 	Validate that the queue offloads includes all the port configured
> offloads.
> 
> I don't have answers for these, perhaps Declan or Radu can help.

[dpdk-dev] [PATCH v2] net/bonding: switch to new offloading API

[Ferruh Yigit]

Switch to new ethdev offloading API.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Radu Nicolau <radu.nicolau@intel.com>
Cc: Matan Azrad <matan@mellanox.com>

v2:
* added [rt]x_queue_offload_capa to driver internal struct
* increased [rt]x_offload_capa size to 64bit as same as ethdev
---
 drivers/net/bonding/rte_eth_bond_api.c     |  9 ++++++++-
 drivers/net/bonding/rte_eth_bond_pmd.c     | 13 +++++++++++--
 drivers/net/bonding/rte_eth_bond_private.h |  6 ++++--
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_api.c b/drivers/net/bonding/rte_eth_bond_api.c
index f854b7375..6d34f6038 100644
--- a/drivers/net/bonding/rte_eth_bond_api.c
+++ b/drivers/net/bonding/rte_eth_bond_api.c
@@ -194,7 +194,8 @@ slave_vlan_filter_set(uint16_t bonded_port_id, uint16_t slave_port_id)
 	uint16_t first;
 
 	bonded_eth_dev = &rte_eth_devices[bonded_port_id];
-	if (bonded_eth_dev->data->dev_conf.rxmode.hw_vlan_filter == 0)
+	if ((bonded_eth_dev->data->dev_conf.rxmode.offloads &
+			DEV_RX_OFFLOAD_VLAN_FILTER) == 0)
 		return 0;
 
 	internals = bonded_eth_dev->data->dev_private;
@@ -284,6 +285,8 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
 		/* Take the first dev's offload capabilities */
 		internals->rx_offload_capa = dev_info.rx_offload_capa;
 		internals->tx_offload_capa = dev_info.tx_offload_capa;
+		internals->rx_queue_offload_capa = dev_info.rx_queue_offload_capa;
+		internals->tx_queue_offload_capa = dev_info.tx_queue_offload_capa;
 		internals->flow_type_rss_offloads = dev_info.flow_type_rss_offloads;
 
 		/* Inherit first slave's max rx packet size */
@@ -292,6 +295,8 @@ __eth_bond_slave_add_lock_free(uint16_t bonded_port_id, uint16_t slave_port_id)
 	} else {
 		internals->rx_offload_capa &= dev_info.rx_offload_capa;
 		internals->tx_offload_capa &= dev_info.tx_offload_capa;
+		internals->rx_queue_offload_capa &= dev_info.rx_queue_offload_capa;
+		internals->tx_queue_offload_capa &= dev_info.tx_queue_offload_capa;
 		internals->flow_type_rss_offloads &= dev_info.flow_type_rss_offloads;
 
 		if (link_properties_valid(bonded_eth_dev,
@@ -458,6 +463,8 @@ __eth_bond_slave_remove_lock_free(uint16_t bonded_port_id,
 	if (internals->slave_count == 0) {
 		internals->rx_offload_capa = 0;
 		internals->tx_offload_capa = 0;
+		internals->rx_queue_offload_capa = 0;
+		internals->tx_queue_offload_capa = 0;
 		internals->flow_type_rss_offloads = ETH_RSS_PROTO_MASK;
 		internals->reta_size = 0;
 		internals->candidate_max_rx_pktlen = 0;
diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index b59ba9f7c..74ad3d297 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -1818,8 +1818,13 @@ slave_configure(struct rte_eth_dev *bonded_eth_dev,
 				bonded_eth_dev->data->dev_conf.rxmode.mq_mode;
 	}
 
-	slave_eth_dev->data->dev_conf.rxmode.hw_vlan_filter =
-			bonded_eth_dev->data->dev_conf.rxmode.hw_vlan_filter;
+	if (bonded_eth_dev->data->dev_conf.rxmode.offloads &
+			DEV_RX_OFFLOAD_VLAN_FILTER)
+		slave_eth_dev->data->dev_conf.rxmode.offloads |=
+				DEV_RX_OFFLOAD_VLAN_FILTER;
+	else
+		slave_eth_dev->data->dev_conf.rxmode.offloads &=
+				~DEV_RX_OFFLOAD_VLAN_FILTER;
 
 	nb_rx_queues = bonded_eth_dev->data->nb_rx_queues;
 	nb_tx_queues = bonded_eth_dev->data->nb_tx_queues;
@@ -2252,6 +2257,8 @@ bond_ethdev_info(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 
 	dev_info->rx_offload_capa = internals->rx_offload_capa;
 	dev_info->tx_offload_capa = internals->tx_offload_capa;
+	dev_info->rx_queue_offload_capa = internals->rx_queue_offload_capa;
+	dev_info->tx_queue_offload_capa = internals->tx_queue_offload_capa;
 	dev_info->flow_type_rss_offloads = internals->flow_type_rss_offloads;
 
 	dev_info->reta_size = internals->reta_size;
@@ -2944,6 +2951,8 @@ bond_alloc(struct rte_vdev_device *dev, uint8_t mode)
 	internals->active_slave_count = 0;
 	internals->rx_offload_capa = 0;
 	internals->tx_offload_capa = 0;
+	internals->rx_queue_offload_capa = 0;
+	internals->tx_queue_offload_capa = 0;
 	internals->candidate_max_rx_pktlen = 0;
 	internals->max_rx_pktlen = 0;
 
diff --git a/drivers/net/bonding/rte_eth_bond_private.h b/drivers/net/bonding/rte_eth_bond_private.h
index 92e15f8cc..94eca8838 100644
--- a/drivers/net/bonding/rte_eth_bond_private.h
+++ b/drivers/net/bonding/rte_eth_bond_private.h
@@ -128,8 +128,10 @@ struct bond_dev_private {
 	/**< TLB active slaves send order */
 	struct mode_alb_private mode6;
 
-	uint32_t rx_offload_capa;            /** Rx offload capability */
-	uint32_t tx_offload_capa;            /** Tx offload capability */
+	uint64_t rx_offload_capa;       /** Rx offload capability */
+	uint64_t tx_offload_capa;       /** Tx offload capability */
+	uint64_t rx_queue_offload_capa; /** per queue Rx offload capability */
+	uint64_t tx_queue_offload_capa; /** per queue Tx offload capability */
 
 	/** Bit mask of RSS offloads, the bit offset also means flow type */
 	uint64_t flow_type_rss_offloads;
-- 
2.13.6

Re: [dpdk-dev] [PATCH v2] net/bonding: switch to new offloading API

[Radu Nicolau]

On 3/22/2018 6:13 PM, Ferruh Yigit wrote:
> Switch to new ethdev offloading API.
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: Radu Nicolau <radu.nicolau@intel.com>
> Cc: Matan Azrad <matan@mellanox.com>
>
> v2:
> * added [rt]x_queue_offload_capa to driver internal struct
> * increased [rt]x_offload_capa size to 64bit as same as ethdev
> ---
>   
Acked-by: Radu Nicolau <radu.nicolau@intel.com> 
<mailto:radu.nicolau@intel.com>

Re: [dpdk-dev] [PATCH v2] net/bonding: switch to new offloading API

[Ferruh Yigit]

On 4/6/2018 10:57 AM, Radu Nicolau wrote:
> 
> On 3/22/2018 6:13 PM, Ferruh Yigit wrote:
>> Switch to new ethdev offloading API.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> Cc: Radu Nicolau <radu.nicolau@intel.com>
>> Cc: Matan Azrad <matan@mellanox.com>
>>
>> v2:
>> * added [rt]x_queue_offload_capa to driver internal struct
>> * increased [rt]x_offload_capa size to 64bit as same as ethdev
>> ---
>>  
> Acked-by: Radu Nicolau <radu.nicolau@intel.com
Applied to dpdk-next-net/master, thanks.