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

[Maxime Coquelin <maxime.coquelin@redhat.com>]

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
>