[dpdk-dev,2/4] net/vhost: improve Tx path selection

Message ID 20180601124758.22652-3-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers

Checks

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

Commit Message

Maxime Coquelin June 1, 2018, 12:47 p.m. UTC
  This patch improves the Tx path selection depending on
whether the application request for offloads, and on whether
offload features have been negotiated.

When the application doesn't request for Tx offload features,
the corresponding features bits aren't negotiated.

When Tx offload virtio features have been negotiated, ensure
the simple Tx path isn't selected.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 20 ++++++++++++++++++--
 drivers/net/virtio/virtio_ethdev.h |  3 ---
 2 files changed, 18 insertions(+), 5 deletions(-)
  

Comments

Tiwei Bie June 4, 2018, 12:25 p.m. UTC | #1
On Fri, Jun 01, 2018 at 02:47:56PM +0200, Maxime Coquelin wrote:
> This patch improves the Tx path selection depending on
> whether the application request for offloads, and on whether
> offload features have been negotiated.
> 
> When the application doesn't request for Tx offload features,
> the corresponding features bits aren't negotiated.
> 
> When Tx offload virtio features have been negotiated, ensure
> the simple Tx path isn't selected.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/net/virtio/virtio_ethdev.c | 20 ++++++++++++++++++--
>  drivers/net/virtio/virtio_ethdev.h |  3 ---
>  2 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index e68e9d067..5730620ed 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1798,8 +1798,10 @@ 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;
>  	struct virtio_hw *hw = dev->data->dev_private;
>  	uint64_t rx_offloads = rxmode->offloads;
> +	uint64_t tx_offloads = txmode->offloads;
>  	uint64_t req_features;
>  	int ret;
>  
> @@ -1821,6 +1823,15 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>  			(1ULL << VIRTIO_NET_F_GUEST_TSO4) |
>  			(1ULL << VIRTIO_NET_F_GUEST_TSO6);
>  
> +	if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
> +				DEV_TX_OFFLOAD_UDP_CKSUM))

I think it's better to keep DEV_TX_OFFLOAD_TCP/UDP_CKSUM
aligned, something like:

	if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
			   DEV_TX_OFFLOAD_UDP_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);
> @@ -1885,6 +1896,11 @@ virtio_dev_configure(struct rte_eth_dev *dev)
>  			   DEV_RX_OFFLOAD_TCP_CKSUM))
>  		hw->use_simple_rx = 0;
>  
> +	if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
> +				DEV_TX_OFFLOAD_UDP_CKSUM |
> +				DEV_TX_OFFLOAD_TCP_TSO))

Ditto. I think it's better to keep them aligned,
something like:

	if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
			   DEV_TX_OFFLOAD_UDP_CKSUM |
			   DEV_TX_OFFLOAD_TCP_TSO))

Besides, we also need to consider not using simple Tx
when DEV_TX_OFFLOAD_VLAN_INSERT is requested.

Best regards,
Tiwei Bie

> +		hw->use_simple_tx = 0;
> +
>  	return 0;
>  }
>  
> @@ -2138,14 +2154,14 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>  
>  	dev_info->tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS |
>  				    DEV_TX_OFFLOAD_VLAN_INSERT;
> -	if (hw->guest_features & (1ULL << VIRTIO_NET_F_CSUM)) {
> +	if (host_features & (1ULL << VIRTIO_NET_F_CSUM)) {
>  		dev_info->tx_offload_capa |=
>  			DEV_TX_OFFLOAD_UDP_CKSUM |
>  			DEV_TX_OFFLOAD_TCP_CKSUM;
>  	}
>  	tso_mask = (1ULL << VIRTIO_NET_F_HOST_TSO4) |
>  		(1ULL << VIRTIO_NET_F_HOST_TSO6);
> -	if ((hw->guest_features & tso_mask) == tso_mask)
> +	if ((host_features & tso_mask) == tso_mask)
>  		dev_info->tx_offload_capa |= DEV_TX_OFFLOAD_TCP_TSO;
>  }
>  
> diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
> index bb40064ea..b603665c7 100644
> --- a/drivers/net/virtio/virtio_ethdev.h
> +++ b/drivers/net/virtio/virtio_ethdev.h
> @@ -28,9 +28,6 @@
>  	 1u << VIRTIO_NET_F_CTRL_VQ	  |	\
>  	 1u << VIRTIO_NET_F_CTRL_RX	  |	\
>  	 1u << VIRTIO_NET_F_CTRL_VLAN	  |	\
> -	 1u << VIRTIO_NET_F_CSUM	  |	\
> -	 1u << VIRTIO_NET_F_HOST_TSO4	  |	\
> -	 1u << VIRTIO_NET_F_HOST_TSO6	  |	\
>  	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
>  	 1u << VIRTIO_NET_F_MTU	| \
>  	 1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE |	\
> -- 
> 2.14.3
>
  

Patch

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index e68e9d067..5730620ed 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1798,8 +1798,10 @@  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;
 	struct virtio_hw *hw = dev->data->dev_private;
 	uint64_t rx_offloads = rxmode->offloads;
+	uint64_t tx_offloads = txmode->offloads;
 	uint64_t req_features;
 	int ret;
 
@@ -1821,6 +1823,15 @@  virtio_dev_configure(struct rte_eth_dev *dev)
 			(1ULL << VIRTIO_NET_F_GUEST_TSO4) |
 			(1ULL << VIRTIO_NET_F_GUEST_TSO6);
 
+	if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
+				DEV_TX_OFFLOAD_UDP_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);
@@ -1885,6 +1896,11 @@  virtio_dev_configure(struct rte_eth_dev *dev)
 			   DEV_RX_OFFLOAD_TCP_CKSUM))
 		hw->use_simple_rx = 0;
 
+	if (tx_offloads & (DEV_TX_OFFLOAD_TCP_CKSUM |
+				DEV_TX_OFFLOAD_UDP_CKSUM |
+				DEV_TX_OFFLOAD_TCP_TSO))
+		hw->use_simple_tx = 0;
+
 	return 0;
 }
 
@@ -2138,14 +2154,14 @@  virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 
 	dev_info->tx_offload_capa = DEV_TX_OFFLOAD_MULTI_SEGS |
 				    DEV_TX_OFFLOAD_VLAN_INSERT;
-	if (hw->guest_features & (1ULL << VIRTIO_NET_F_CSUM)) {
+	if (host_features & (1ULL << VIRTIO_NET_F_CSUM)) {
 		dev_info->tx_offload_capa |=
 			DEV_TX_OFFLOAD_UDP_CKSUM |
 			DEV_TX_OFFLOAD_TCP_CKSUM;
 	}
 	tso_mask = (1ULL << VIRTIO_NET_F_HOST_TSO4) |
 		(1ULL << VIRTIO_NET_F_HOST_TSO6);
-	if ((hw->guest_features & tso_mask) == tso_mask)
+	if ((host_features & tso_mask) == tso_mask)
 		dev_info->tx_offload_capa |= DEV_TX_OFFLOAD_TCP_TSO;
 }
 
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index bb40064ea..b603665c7 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -28,9 +28,6 @@ 
 	 1u << VIRTIO_NET_F_CTRL_VQ	  |	\
 	 1u << VIRTIO_NET_F_CTRL_RX	  |	\
 	 1u << VIRTIO_NET_F_CTRL_VLAN	  |	\
-	 1u << VIRTIO_NET_F_CSUM	  |	\
-	 1u << VIRTIO_NET_F_HOST_TSO4	  |	\
-	 1u << VIRTIO_NET_F_HOST_TSO6	  |	\
 	 1u << VIRTIO_NET_F_MRG_RXBUF	  |	\
 	 1u << VIRTIO_NET_F_MTU	| \
 	 1ULL << VIRTIO_NET_F_GUEST_ANNOUNCE |	\