[10/11] net/hns3: fix Rx/Tx queue offload capability

Message ID 20200825115305.58490-11-huwei013@chinasoftinc.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series updates for hns3 PMD driver |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Wei Hu (Xavier) Aug. 25, 2020, 11:53 a.m. UTC
  From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>

According to rte_eth_rx_queue_setup and rte_eth_tx_queue_setup API
function, rx_queue_offload_capa and rx_offload_capa, tx_queue_offload_capa
and tx_offload_capa must be mutually exclusive in the '.dev_infos_get' ops
implementation function. Otherwise, rte_eth_rx_queue_setup or
rte_eth_tx_queue_setup will fail, if user uses rx_offload_capa and
tx_offload_capa obtained by calling the rte_eth_dev_info_get API function.

Currently, offload capabilities are enabled for all Rx/Tx queues in hns3
PF and VF PMD driver, and offload capability only applied in a Rx/Tx
queue is not supported. This patch fixes Rx/Tx queue offload capability.

Fixes: 1f5ca0b460cd67 ("net/hns3: support some device operations")
Fixes: a5475d61fa34b8 ("net/hns3: support VF")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c    | 5 +++--
 drivers/net/hns3/hns3_ethdev_vf.c | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)
  

Comments

Ferruh Yigit Sept. 4, 2020, 10:34 a.m. UTC | #1
On 8/25/2020 12:53 PM, Wei Hu (Xavier) wrote:
> From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>
> 
> According to rte_eth_rx_queue_setup and rte_eth_tx_queue_setup API
> function, rx_queue_offload_capa and rx_offload_capa, tx_queue_offload_capa
> and tx_offload_capa must be mutually exclusive in the '.dev_infos_get' ops
> implementation function. Otherwise, rte_eth_rx_queue_setup or
> rte_eth_tx_queue_setup will fail, if user uses rx_offload_capa and
> tx_offload_capa obtained by calling the rte_eth_dev_info_get API function.

Can you please clarify what is fixed here?

If the PMD doesn't support 'DEV_TX_OFFLOAD_MBUF_FAST_FREE' to be configured per
queue, it makes sense the update the capability reporting to match it.

But having an offload as queue offload shouldn't cause any error on setting it
on port wise (to all queues). I am asking because if you are getting error
'rte_eth_rx_queue_setup()' / 'rte_eth_tx_queue_setup()' the reason can be
something else.
Also what do you mean by "'tx_queue_offload_capa' and 'tx_offload_capa' must be
mutually exclusive"? All queue offloads should be present in the port offload,
because of an offload can be applied to any specific queue, this means it can be
applied to all queues which means it can be applied port wise.

> 
> Currently, offload capabilities are enabled for all Rx/Tx queues in hns3
> PF and VF PMD driver, and offload capability only applied in a Rx/Tx
> queue is not supported. This patch fixes Rx/Tx queue offload capability.
> 
> Fixes: 1f5ca0b460cd67 ("net/hns3: support some device operations")
> Fixes: a5475d61fa34b8 ("net/hns3: support VF")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> ---
>  drivers/net/hns3/hns3_ethdev.c    | 5 +++--
>  drivers/net/hns3/hns3_ethdev_vf.c | 5 +++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
> index 14e4b9e35..281d8b928 100644
> --- a/drivers/net/hns3/hns3_ethdev.c
> +++ b/drivers/net/hns3/hns3_ethdev.c
> @@ -2459,6 +2459,7 @@ hns3_dev_infos_get(struct rte_eth_dev *eth_dev, struct rte_eth_dev_info *info)
>  	info->max_mac_addrs = HNS3_UC_MACADDR_NUM;
>  	info->max_mtu = info->max_rx_pktlen - HNS3_ETH_OVERHEAD;
>  	info->max_lro_pkt_size = HNS3_MAX_LRO_SIZE;
> +	info->rx_queue_offload_capa = 0;

No need to set 'rx_queue_offload_capa' or 'tx_queue_offload_capa' to zero since
zero is their default value.
  
Wei Hu (Xavier) Sept. 8, 2020, 11:48 a.m. UTC | #2
Hi, Ferruh Yigit

On 2020/9/4 18:34, Ferruh Yigit wrote:
> On 8/25/2020 12:53 PM, Wei Hu (Xavier) wrote:
>> From: "Wei Hu (Xavier)" <xavier.huwei@huawei.com>
>>
>> According to rte_eth_rx_queue_setup and rte_eth_tx_queue_setup API
>> function, rx_queue_offload_capa and rx_offload_capa, tx_queue_offload_capa
>> and tx_offload_capa must be mutually exclusive in the '.dev_infos_get' ops
>> implementation function. Otherwise, rte_eth_rx_queue_setup or
>> rte_eth_tx_queue_setup will fail, if user uses rx_offload_capa and
>> tx_offload_capa obtained by calling the rte_eth_dev_info_get API function.
> Can you please clarify what is fixed here?
>
> If the PMD doesn't support 'DEV_TX_OFFLOAD_MBUF_FAST_FREE' to be configured per
> queue, it makes sense the update the capability reporting to match it.
>
> But having an offload as queue offload shouldn't cause any error on setting it
> on port wise (to all queues). I am asking because if you are getting error
> 'rte_eth_rx_queue_setup()' / 'rte_eth_tx_queue_setup()' the reason can be
> something else.
> Also what do you mean by "'tx_queue_offload_capa' and 'tx_offload_capa' must be
> mutually exclusive"? All queue offloads should be present in the port offload,
> because of an offload can be applied to any specific queue, this means it can be
> applied to all queues which means it can be applied port wise.

"rx_queue_offload_capa and rx_offload_capa, tx_queue_offload_capa

  and tx_offload_capa must be mutually exclusive" -- It's wrong, we 
misunderstood

the process of rte_eth_rx_queue_setup and rte_eth_tx_queue_setup.

Thanks :-)


We will update the commit log as below:

Currently, offload capabilities are only enabled for all Rx/Tx queues in hns3
PF and VF PMD driver, and offload capability only applied in a Rx/Tx
queue is not supported. So this patch moves 'DEV_TX_OFFLOAD_MBUF_FAST_FREE'
from tx_queue_offload_capa to tx_offload_capa.


>> Currently, offload capabilities are enabled for all Rx/Tx queues in hns3
>> PF and VF PMD driver, and offload capability only applied in a Rx/Tx
>> queue is not supported. This patch fixes Rx/Tx queue offload capability.
>>
>> Fixes: 1f5ca0b460cd67 ("net/hns3: support some device operations")
>> Fixes: a5475d61fa34b8 ("net/hns3: support VF")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> ---
>>   drivers/net/hns3/hns3_ethdev.c    | 5 +++--
>>   drivers/net/hns3/hns3_ethdev_vf.c | 5 +++--
>>   2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
>> index 14e4b9e35..281d8b928 100644
>> --- a/drivers/net/hns3/hns3_ethdev.c
>> +++ b/drivers/net/hns3/hns3_ethdev.c
>> @@ -2459,6 +2459,7 @@ hns3_dev_infos_get(struct rte_eth_dev *eth_dev, struct rte_eth_dev_info *info)
>>   	info->max_mac_addrs = HNS3_UC_MACADDR_NUM;
>>   	info->max_mtu = info->max_rx_pktlen - HNS3_ETH_OVERHEAD;
>>   	info->max_lro_pkt_size = HNS3_MAX_LRO_SIZE;
>> +	info->rx_queue_offload_capa = 0;
> No need to set 'rx_queue_offload_capa' or 'tx_queue_offload_capa' to zero since
> zero is their default value.

Ok, I  will fix it in V2.

Thanks


Regards

Xavier
  

Patch

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 14e4b9e35..281d8b928 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2459,6 +2459,7 @@  hns3_dev_infos_get(struct rte_eth_dev *eth_dev, struct rte_eth_dev_info *info)
 	info->max_mac_addrs = HNS3_UC_MACADDR_NUM;
 	info->max_mtu = info->max_rx_pktlen - HNS3_ETH_OVERHEAD;
 	info->max_lro_pkt_size = HNS3_MAX_LRO_SIZE;
+	info->rx_queue_offload_capa = 0;
 	info->rx_offload_capa = (DEV_RX_OFFLOAD_IPV4_CKSUM |
 				 DEV_RX_OFFLOAD_TCP_CKSUM |
 				 DEV_RX_OFFLOAD_UDP_CKSUM |
@@ -2472,7 +2473,7 @@  hns3_dev_infos_get(struct rte_eth_dev *eth_dev, struct rte_eth_dev_info *info)
 				 DEV_RX_OFFLOAD_JUMBO_FRAME |
 				 DEV_RX_OFFLOAD_RSS_HASH |
 				 DEV_RX_OFFLOAD_TCP_LRO);
-	info->tx_queue_offload_capa = DEV_TX_OFFLOAD_MBUF_FAST_FREE;
+	info->tx_queue_offload_capa = 0;
 	info->tx_offload_capa = (DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM |
 				 DEV_TX_OFFLOAD_IPV4_CKSUM |
 				 DEV_TX_OFFLOAD_TCP_CKSUM |
@@ -2483,7 +2484,7 @@  hns3_dev_infos_get(struct rte_eth_dev *eth_dev, struct rte_eth_dev_info *info)
 				 DEV_TX_OFFLOAD_VXLAN_TNL_TSO |
 				 DEV_TX_OFFLOAD_GRE_TNL_TSO |
 				 DEV_TX_OFFLOAD_GENEVE_TNL_TSO |
-				 info->tx_queue_offload_capa |
+				 DEV_TX_OFFLOAD_MBUF_FAST_FREE |
 				 hns3_txvlan_cap_get(hw));
 
 	info->rx_desc_lim = (struct rte_eth_desc_lim) {
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 7fd0e6a43..2f7a96826 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -903,6 +903,7 @@  hns3vf_dev_infos_get(struct rte_eth_dev *eth_dev, struct rte_eth_dev_info *info)
 	info->max_mtu = info->max_rx_pktlen - HNS3_ETH_OVERHEAD;
 	info->max_lro_pkt_size = HNS3_MAX_LRO_SIZE;
 
+	info->rx_queue_offload_capa = 0;
 	info->rx_offload_capa = (DEV_RX_OFFLOAD_IPV4_CKSUM |
 				 DEV_RX_OFFLOAD_UDP_CKSUM |
 				 DEV_RX_OFFLOAD_TCP_CKSUM |
@@ -915,7 +916,7 @@  hns3vf_dev_infos_get(struct rte_eth_dev *eth_dev, struct rte_eth_dev_info *info)
 				 DEV_RX_OFFLOAD_JUMBO_FRAME |
 				 DEV_RX_OFFLOAD_RSS_HASH |
 				 DEV_RX_OFFLOAD_TCP_LRO);
-	info->tx_queue_offload_capa = DEV_TX_OFFLOAD_MBUF_FAST_FREE;
+	info->tx_queue_offload_capa = 0;
 	info->tx_offload_capa = (DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM |
 				 DEV_TX_OFFLOAD_IPV4_CKSUM |
 				 DEV_TX_OFFLOAD_TCP_CKSUM |
@@ -926,7 +927,7 @@  hns3vf_dev_infos_get(struct rte_eth_dev *eth_dev, struct rte_eth_dev_info *info)
 				 DEV_TX_OFFLOAD_VXLAN_TNL_TSO |
 				 DEV_TX_OFFLOAD_GRE_TNL_TSO |
 				 DEV_TX_OFFLOAD_GENEVE_TNL_TSO |
-				 info->tx_queue_offload_capa |
+				 DEV_TX_OFFLOAD_MBUF_FAST_FREE |
 				 hns3_txvlan_cap_get(hw));
 
 	info->rx_desc_lim = (struct rte_eth_desc_lim) {