[PATCH] net/vhost: get csum offload capabilities of vhost backend

[PATCH] net/vhost: get csum offload capabilities of vhost backend

[Wenwu Ma]

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;
 }
 
-- 
2.25.1

RE: [PATCH] net/vhost: get csum offload capabilities of vhost backend

[Hu, Jiayu]

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

Re: [PATCH] net/vhost: get csum offload capabilities of vhost backend

[Maxime Coquelin]

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