diff mbox

[dpdk-dev,v2] net/vhost: convert to new Rx/Tx offload API

Message ID 20180522122211.26627-1-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Maxime Coquelin May 22, 2018, 12:22 p.m. UTC 
  Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
Changes since v2:
- Announce supported Rx & Tx offload features (Tiwei)

 drivers/net/vhost/rte_eth_vhost.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Tiwei Bie May 22, 2018, 12:34 p.m. UTC | #1 
On Tue, May 22, 2018 at 02:22:11PM +0200, Maxime Coquelin wrote:
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> Changes since v2:
> - Announce supported Rx & Tx offload features (Tiwei)
> 
>  drivers/net/vhost/rte_eth_vhost.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
> index f473bbbb3..229a5ba8d 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -491,9 +491,9 @@ eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
>  	struct pmd_internal *internal = dev->data->dev_private;
>  	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
>  
> -	internal->vlan_strip = rxmode->hw_vlan_strip;
> +	internal->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
>  
> -	if (rxmode->hw_vlan_filter)
> +	if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
>  		VHOST_LOG(WARNING,
>  			"vhost(%s): vlan filtering not available\n",
>  			internal->dev_name);

I'm not quite sure whether we still need to log this warning.

> @@ -1072,6 +1072,10 @@ eth_dev_info(struct rte_eth_dev *dev,
>  	dev_info->max_rx_queues = internal->max_queues;
>  	dev_info->max_tx_queues = internal->max_queues;
>  	dev_info->min_rx_bufsize = 0;
> +
> +	dev_info->tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS |
> +				DEV_TX_OFFLOAD_VLAN_INSERT;
> +	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
>  }

Other than that, it looks good to me. Thanks!

Best regards,
Tiwei Bie
Maxime Coquelin May 22, 2018, 12:37 p.m. UTC | #2 
On 05/22/2018 02:34 PM, Tiwei Bie wrote:
> On Tue, May 22, 2018 at 02:22:11PM +0200, Maxime Coquelin wrote:
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>> Changes since v2:
>> - Announce supported Rx & Tx offload features (Tiwei)
>>
>>   drivers/net/vhost/rte_eth_vhost.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
>> index f473bbbb3..229a5ba8d 100644
>> --- a/drivers/net/vhost/rte_eth_vhost.c
>> +++ b/drivers/net/vhost/rte_eth_vhost.c
>> @@ -491,9 +491,9 @@ eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
>>   	struct pmd_internal *internal = dev->data->dev_private;
>>   	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
>>   
>> -	internal->vlan_strip = rxmode->hw_vlan_strip;
>> +	internal->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
>>   
>> -	if (rxmode->hw_vlan_filter)
>> +	if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
>>   		VHOST_LOG(WARNING,
>>   			"vhost(%s): vlan filtering not available\n",
>>   			internal->dev_name);
> 
> I'm not quite sure whether we still need to log this warning.

In theory we should not. But currently, my understanding is that no
error is returned in the current release (see 
lib/librte_ethdev/rte_ethdev.c:1174).

I think we can remove it in next release.

>> @@ -1072,6 +1072,10 @@ eth_dev_info(struct rte_eth_dev *dev,
>>   	dev_info->max_rx_queues = internal->max_queues;
>>   	dev_info->max_tx_queues = internal->max_queues;
>>   	dev_info->min_rx_bufsize = 0;
>> +
>> +	dev_info->tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS |
>> +				DEV_TX_OFFLOAD_VLAN_INSERT;
>> +	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
>>   }
> 
> Other than that, it looks good to me. Thanks!


Thanks for the review.
Maxime

> Best regards,
> Tiwei Bie
>
Ferruh Yigit May 22, 2018, 12:38 p.m. UTC | #3 
On 5/22/2018 1:34 PM, Tiwei Bie wrote:
> On Tue, May 22, 2018 at 02:22:11PM +0200, Maxime Coquelin wrote:
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>> Changes since v2:
>> - Announce supported Rx & Tx offload features (Tiwei)
>>
>>  drivers/net/vhost/rte_eth_vhost.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
>> index f473bbbb3..229a5ba8d 100644
>> --- a/drivers/net/vhost/rte_eth_vhost.c
>> +++ b/drivers/net/vhost/rte_eth_vhost.c
>> @@ -491,9 +491,9 @@ eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
>>  	struct pmd_internal *internal = dev->data->dev_private;
>>  	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
>>  
>> -	internal->vlan_strip = rxmode->hw_vlan_strip;
>> +	internal->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
>>  
>> -	if (rxmode->hw_vlan_filter)
>> +	if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
>>  		VHOST_LOG(WARNING,
>>  			"vhost(%s): vlan filtering not available\n",
>>  			internal->dev_name);
> 
> I'm not quite sure whether we still need to log this warning.

Similar warning exists in ethdev layer, when an offload requested that is not
reported in capability. So I agree to remove this.

And in next release that warning will be converted into error, so that PMD will
not get those values at all.

> 
>> @@ -1072,6 +1072,10 @@ eth_dev_info(struct rte_eth_dev *dev,
>>  	dev_info->max_rx_queues = internal->max_queues;
>>  	dev_info->max_tx_queues = internal->max_queues;
>>  	dev_info->min_rx_bufsize = 0;
>> +
>> +	dev_info->tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS |
>> +				DEV_TX_OFFLOAD_VLAN_INSERT;
>> +	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
>>  }
> 
> Other than that, it looks good to me. Thanks!
> 
> Best regards,
> Tiwei Bie
>
Maxime Coquelin May 22, 2018, 12:48 p.m. UTC | #4 
On 05/22/2018 02:38 PM, Ferruh Yigit wrote:
> On 5/22/2018 1:34 PM, Tiwei Bie wrote:
>> On Tue, May 22, 2018 at 02:22:11PM +0200, Maxime Coquelin wrote:
>>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>>> ---
>>> Changes since v2:
>>> - Announce supported Rx & Tx offload features (Tiwei)
>>>
>>>   drivers/net/vhost/rte_eth_vhost.c | 8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
>>> index f473bbbb3..229a5ba8d 100644
>>> --- a/drivers/net/vhost/rte_eth_vhost.c
>>> +++ b/drivers/net/vhost/rte_eth_vhost.c
>>> @@ -491,9 +491,9 @@ eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
>>>   	struct pmd_internal *internal = dev->data->dev_private;
>>>   	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
>>>   
>>> -	internal->vlan_strip = rxmode->hw_vlan_strip;
>>> +	internal->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
>>>   
>>> -	if (rxmode->hw_vlan_filter)
>>> +	if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
>>>   		VHOST_LOG(WARNING,
>>>   			"vhost(%s): vlan filtering not available\n",
>>>   			internal->dev_name);
>>
>> I'm not quite sure whether we still need to log this warning.
> 
> Similar warning exists in ethdev layer, when an offload requested that is not
> reported in capability. So I agree to remove this.
> 
> And in next release that warning will be converted into error, so that PMD will
> not get those values at all.

Ok, I'll post a v3 removing the check.

Thanks,
Maxime

>>
>>> @@ -1072,6 +1072,10 @@ eth_dev_info(struct rte_eth_dev *dev,
>>>   	dev_info->max_rx_queues = internal->max_queues;
>>>   	dev_info->max_tx_queues = internal->max_queues;
>>>   	dev_info->min_rx_bufsize = 0;
>>> +
>>> +	dev_info->tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS |
>>> +				DEV_TX_OFFLOAD_VLAN_INSERT;
>>> +	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
>>>   }
>>
>> Other than that, it looks good to me. Thanks!
>>
>> Best regards,
>> Tiwei Bie
>>
>
diff mbox

Patch

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index f473bbbb3..229a5ba8d 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -491,9 +491,9 @@  eth_dev_configure(struct rte_eth_dev *dev __rte_unused)
 	struct pmd_internal *internal = dev->data->dev_private;
 	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 
-	internal->vlan_strip = rxmode->hw_vlan_strip;
+	internal->vlan_strip = !!(rxmode->offloads & DEV_RX_OFFLOAD_VLAN_STRIP);
 
-	if (rxmode->hw_vlan_filter)
+	if (rxmode->offloads & DEV_RX_OFFLOAD_VLAN_FILTER)
 		VHOST_LOG(WARNING,
 			"vhost(%s): vlan filtering not available\n",
 			internal->dev_name);
@@ -1072,6 +1072,10 @@  eth_dev_info(struct rte_eth_dev *dev,
 	dev_info->max_rx_queues = internal->max_queues;
 	dev_info->max_tx_queues = internal->max_queues;
 	dev_info->min_rx_bufsize = 0;
+
+	dev_info->tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS |
+				DEV_TX_OFFLOAD_VLAN_INSERT;
+	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_VLAN_STRIP;
 }
 
 static int