Re: [dpdk-dev] [PATCH] net/virtio: include host features supported in guest

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

Hi,

On 5/14/20 1:56 PM, Gowrishankar Muthukrishnan wrote:
> Virtio pmd driver can not benefit from tso and csum offload
> as they are not included in negotiation check with host. Add
> them in virtio dev init and let negotiation decide the fate.
> 
> Signed-off-by: Gowrishankar Muthukrishnan <gmuthukr@redhat.com>


I think you don't need any patch to achieve that.
Disabling TSO and csum offload by default is done on-purpose, as it is
dependent on the application that drivers the Virtio PMD to support it.

To enable offloads with Virtio PMD, your application has to enable by
setting the proper offloads flags in device's ethdev config. Doing that,
a re-negotiation will happen with the proper Virtio features added:

static int
virtio_dev_configure(struct rte_eth_dev *dev)
{
	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
	const struct rte_eth_txmode *txmode = &dev->data->dev_conf.txmode;
...
	uint64_t rx_offloads = rxmode->offloads;
	uint64_t tx_offloads = txmode->offloads;
...

	if (rx_offloads & (DEV_RX_OFFLOAD_UDP_CKSUM |
			   DEV_RX_OFFLOAD_TCP_CKSUM))
		req_features |= (1ULL << VIRTIO_NET_F_GUEST_CSUM);

	if (rx_offloads & DEV_RX_OFFLOAD_TCP_LRO)
		req_features |=
			(1ULL << VIRTIO_NET_F_GUEST_TSO4) |
			(1ULL << VIRTIO_NET_F_GUEST_TSO6);

	if (tx_offloads & (DEV_TX_OFFLOAD_UDP_CKSUM |
			   DEV_TX_OFFLOAD_TCP_CKSUM))
		req_features |= (1ULL << VIRTIO_NET_F_CSUM);

	if (tx_offloads & DEV_TX_OFFLOAD_TCP_TSO)
		req_features |=
			(1ULL << VIRTIO_NET_F_HOST_TSO4) |
			(1ULL << VIRTIO_NET_F_HOST_TSO6);

	/* if request features changed, reinit the device */
	if (req_features != hw->req_guest_features) {
		ret = virtio_init_device(dev, req_features);
		if (ret < 0)
			return ret;
	}
...



> --
> This patch has been tested with TSO tests in OVS-DPDK:
> 
> https://patchwork.ozlabs.org/project/openvswitch/list/?series=176886
> 
> ## ------------------------------- ##
> ## openvswitch 2.13.90 test suite. ##
> ## ------------------------------- ##
> 
> OVS-DPDK unit tests
> 
>   1: OVS-DPDK - EAL init                             ok
>   2: OVS-DPDK - add standard DPDK port               ok
>   3: OVS-DPDK - add vhost-user-client port           ok
>   4: OVS-DPDK - ping vhost-user ports                ok
>   5: OVS-DPDK - ping vhost-user-client ports         ok
>   6: OVS-DPDK - validate tso negotiation             ok
> 
> ## ------------- ##
> ## Test results. ##
> ## ------------- ##
> 
> All 6 tests were successful.
> 
> ---
>  drivers/net/virtio/virtio_ethdev.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 044eb10..91f6f16 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1914,7 +1914,7 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev,
>  	}
>  
>  	/* reset device and negotiate default features */
> -	ret = virtio_init_device(eth_dev, VIRTIO_PMD_DEFAULT_GUEST_FEATURES);
> +	ret = virtio_init_device(eth_dev, VIRTIO_PMD_SUPPORTED_GUEST_FEATURES);
>  	if (ret < 0)
>  		goto err_virtio_init;
>  
> @@ -2064,7 +2064,7 @@ static int eth_virtio_pci_remove(struct rte_pci_device *pci_dev)
>  	int ret;
>  
>  	PMD_INIT_LOG(DEBUG, "configure");
> -	req_features = VIRTIO_PMD_DEFAULT_GUEST_FEATURES;
> +	req_features = VIRTIO_PMD_SUPPORTED_GUEST_FEATURES;
>  
>  	if (rxmode->mq_mode != ETH_MQ_RX_NONE) {
>  		PMD_DRV_LOG(ERR,
>