net/vhost: get csum offload capabilities of vhost backend

Message ID 20220217151628.441165-1-wenwux.ma@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Maxime Coquelin
Headers
Series net/vhost: get csum offload capabilities of vhost backend |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS

Commit Message

Ma, WenwuX Feb. 17, 2022, 3:16 p.m. UTC
  The current vhost backend lacks csum offloads information,
which will cause testpmd command such as "csum set tcp hw
<port_id>" to fail. This patch adds the information according
to the device features.

Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
  

Comments

Hu, Jiayu March 15, 2022, 2:56 a.m. UTC | #1
Hi Wenwu,

> -----Original Message-----
> From: Ma, WenwuX <wenwux.ma@intel.com>
> Sent: Thursday, February 17, 2022 11:16 PM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Wang, Yinan
> <yinan.wang@intel.com>; He, Xingguang <xingguang.he@intel.com>; Ma,
> WenwuX <wenwux.ma@intel.com>
> Subject: [PATCH] net/vhost: get csum offload capabilities of vhost backend
> 
> The current vhost backend lacks csum offloads information, which will cause
> testpmd command such as "csum set tcp hw <port_id>" to fail. This patch
> adds the information according to the device features.

Vhost-pmd cannot report correct checksum offloading capabilities, since
feature negotiation may happen after vhost-pmd device creation. It's good
to see there is a solution for it, IMO.

> 
> Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
> ---
>  drivers/net/vhost/rte_eth_vhost.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/net/vhost/rte_eth_vhost.c
> b/drivers/net/vhost/rte_eth_vhost.c
> index 070f0e6dfd..7593d5a9ae 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -1281,6 +1281,24 @@ eth_dev_info(struct rte_eth_dev *dev,
>  				RTE_ETH_TX_OFFLOAD_VLAN_INSERT;
>  	dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_VLAN_STRIP;
> 
> +	if (internal->vid != -1) {
> +		uint64_t features = 0;
> +		if (rte_vhost_get_negotiated_features(internal->vid,
> &features) != 0)
> +			return 0;
> +
> +		if (features & (1ULL << VIRTIO_NET_F_CSUM)) {
> +			dev_info->tx_offload_capa |=
> RTE_ETH_TX_OFFLOAD_TCP_CKSUM |

The spec says "VIRTIO_NET_F_CSUM: host handles pkts w/ partial csum".
If this feature is negotiated, it means vhost pmd supports to receive packets
with partial checksum, right? So VIRTIO_NET_F_CSUM should represent Rx
offloading capabilities for vhost pmd.

> +
> 	RTE_ETH_TX_OFFLOAD_UDP_CKSUM |
> +
> 	RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
> +		}
> +
> +		if (features & (1ULL << VIRTIO_NET_F_GUEST_CSUM)) {
> +			dev_info->rx_offload_capa |=

Same as above.

Thanks,
Jiayu
  
Maxime Coquelin May 5, 2022, 2:15 p.m. UTC | #2
Hi Wenwu,

On 2/17/22 16:16, Wenwu Ma wrote:
> The current vhost backend lacks csum offloads information,
> which will cause testpmd command such as "csum set tcp hw
> <port_id>" to fail. This patch adds the information according
> to the device features.
> 
> Signed-off-by: Wenwu Ma <wenwux.ma@intel.com>
> ---
>   drivers/net/vhost/rte_eth_vhost.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
> index 070f0e6dfd..7593d5a9ae 100644
> --- a/drivers/net/vhost/rte_eth_vhost.c
> +++ b/drivers/net/vhost/rte_eth_vhost.c
> @@ -1281,6 +1281,24 @@ eth_dev_info(struct rte_eth_dev *dev,
>   				RTE_ETH_TX_OFFLOAD_VLAN_INSERT;
>   	dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_VLAN_STRIP;
>   
> +	if (internal->vid != -1) {
> +		uint64_t features = 0;
> +		if (rte_vhost_get_negotiated_features(internal->vid, &features) != 0)
> +			return 0;
> +
> +		if (features & (1ULL << VIRTIO_NET_F_CSUM)) {
> +			dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_TCP_CKSUM |
> +						RTE_ETH_TX_OFFLOAD_UDP_CKSUM |
> +						RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
> +		}
> +
> +		if (features & (1ULL << VIRTIO_NET_F_GUEST_CSUM)) {
> +			dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
> +						RTE_ETH_RX_OFFLOAD_UDP_CKSUM |
> +						RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
> +		}
> +	}
> +
>   	return 0;
>   }
>   
> 

This patch has lots of gaps, since the negotiated Virtio features can
change if the guest driver decides so, so the exposed ethdev offload
capabilities may not represent what is really supported by the guest
driver.

I have done a series that handles this issue by implementing SW
fallbacks in case of misalignment between host application and guest
application:

http://patches.dpdk.org/project/dpdk/cover/20220505102729.821075-1-maxime.coquelin@redhat.com/

Please help reviewing & testing the series if possible.

Thanks,
Maxime
  

Patch

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 070f0e6dfd..7593d5a9ae 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -1281,6 +1281,24 @@  eth_dev_info(struct rte_eth_dev *dev,
 				RTE_ETH_TX_OFFLOAD_VLAN_INSERT;
 	dev_info->rx_offload_capa = RTE_ETH_RX_OFFLOAD_VLAN_STRIP;
 
+	if (internal->vid != -1) {
+		uint64_t features = 0;
+		if (rte_vhost_get_negotiated_features(internal->vid, &features) != 0)
+			return 0;
+
+		if (features & (1ULL << VIRTIO_NET_F_CSUM)) {
+			dev_info->tx_offload_capa |= RTE_ETH_TX_OFFLOAD_TCP_CKSUM |
+						RTE_ETH_TX_OFFLOAD_UDP_CKSUM |
+						RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
+		}
+
+		if (features & (1ULL << VIRTIO_NET_F_GUEST_CSUM)) {
+			dev_info->rx_offload_capa |= RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
+						RTE_ETH_RX_OFFLOAD_UDP_CKSUM |
+						RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+		}
+	}
+
 	return 0;
 }