net/virtio: include ipv4 cksum to support cksum offload capability
PATCH 1/1 net/virtio: include ipv4 cksum to support cksum offload capability
Device cksum offload capability usually include ipv4 cksum, tcp and udp cksum offload capability. The application such as OVS usually negotiate with the drive like this to enable cksum offload.
--- drivers/net/virtio/virtio_ethdev.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/net/virtio/virtio_ethdev.c +2 -0
@@ -3041,6 +3041,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) dev_info->rx_offload_capa |=RTE_ETH_RX_OFFLOAD_SCATTER ;if (host_features & (1ULL <<VIRTIO_NET_F_GUEST_CSUM )) { dev_info->rx_offload_capa |= +RTE_ETH_RX_OFFLOAD_IPV4_CKSUM |RTE_ETH_RX_OFFLOAD_TCP_CKSUM |RTE_ETH_RX_OFFLOAD_UDP_CKSUM ; }
@@ -3055,6 +3056,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)RTE_ETH_TX_OFFLOAD_VLAN_INSERT ;if (host_features & (1ULL <<VIRTIO_NET_F_CSUM )) { dev_info->tx_offload_capa |= +RTE_ETH_TX_OFFLOAD_IPV4_CKSUM |RTE_ETH_TX_OFFLOAD_UDP_CKSUM |RTE_ETH_TX_OFFLOAD_TCP_CKSUM ; }
Signature · 1 line
2.27.0
Hi Harold,
Quoted 22 lines · Harold Huang
Device cksum offload capability usually include ipv4 cksum, tcp and udp cksum offload capability. The application such as OVS usually negotiate with the drive like this to enable cksum offload.
Signed-off-by Harold Huang <[email protected]>--- drivers/net/virtio/virtio_ethdev.c | 2 ++ 1 file changed, 2 insertions(+)drivers/net/virtio/virtio_ethdev.c +1
-0@@ -3041,6 +3041,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) dev_info->rx_offload_capa |=RTE_ETH_RX_OFFLOAD_SCATTER ;if (host_features & (1ULL <<VIRTIO_NET_F_GUEST_CSUM )) { dev_info->rx_offload_capa |= +RTE_ETH_RX_OFFLOAD_IPV4_CKSUM |RTE_ETH_RX_OFFLOAD_TCP_CKSUM |RTE_ETH_RX_OFFLOAD_UDP_CKSUM ; }
drivers/net/virtio/virtio_ethdev.c virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) · line 3062
Quoted 7 lines
drivers/net/virtio/virtio_ethdev.c +1
-0@@ -3055,6 +3056,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)RTE_ETH_TX_OFFLOAD_VLAN_INSERT ;if (host_features & (1ULL <<VIRTIO_NET_F_CSUM )) { dev_info->tx_offload_capa |= +RTE_ETH_TX_OFFLOAD_IPV4_CKSUM |RTE_ETH_TX_OFFLOAD_UDP_CKSUM |RTE_ETH_TX_OFFLOAD_TCP_CKSUM ; }
I'm not sure to understand why this is needed, as Vhost lib will always ensure the IP csum has been calculated. Could you please elaborate?
Thanks,
Maxime
Hi, Maxime,
Quoted 28 lines · Maxime Coquelin <[email protected]> 于2022年1月31日周一 18:04写道：
Hi Harold,
Quoted 24 lines · Harold Huang
Device cksum offload capability usually include ipv4 cksum, tcp and udp cksum offload capability. The application such as OVS usually negotiate with the drive like this to enable cksum offload.
Signed-off-by Harold Huang <[email protected]>--- drivers/net/virtio/virtio_ethdev.c | 2 ++ 1 file changed, 2 insertions(+)drivers/net/virtio/virtio_ethdev.c +1
-0@@ -3041,6 +3041,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) dev_info->rx_offload_capa |=RTE_ETH_RX_OFFLOAD_SCATTER ;if (host_features & (1ULL <<VIRTIO_NET_F_GUEST_CSUM )) { dev_info->rx_offload_capa |= +RTE_ETH_RX_OFFLOAD_IPV4_CKSUM |RTE_ETH_RX_OFFLOAD_TCP_CKSUM |RTE_ETH_RX_OFFLOAD_UDP_CKSUM ; }@@ -3055,6 +3056,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)RTE_ETH_TX_OFFLOAD_VLAN_INSERT ;if (host_features & (1ULL <<VIRTIO_NET_F_CSUM )) {
Quoted 8 lines
drivers/net/virtio/virtio_ethdev.c virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) · line 3062
Quoted 5 lines
drivers/net/virtio/virtio_ethdev.c +1
-0dev_info->tx_offload_capa |= +RTE_ETH_TX_OFFLOAD_IPV4_CKSUM |RTE_ETH_TX_OFFLOAD_UDP_CKSUM |RTE_ETH_TX_OFFLOAD_TCP_CKSUM ; }I'm not sure to understand why this is needed, as Vhost lib will always ensure the IP csum has been calculated. Could you please elaborate?
Thanks for your comments. Previously I want to enable tx checksum offload for the tap device when I use DPDK virtio-user driver with OVS. OVS assume checksum offload capability includes tcp, udp and ipv4 checksum offload:
https://github.com/openvswitch/ovs/blob/master/lib/netdev-dpdk.c#L1097.
But AFAIK, ipv4 checksum has always been calculated in the kernel. And according to the virito-spec
(https://docs.oasis-open.org/virtio/virtio/v1.1/csprd01/virtio-v1.1-csprd01.html),VIRTIO_NET_F_GUEST_CSUM feature bit may only indicate TCP or UDP checksum offload, right? Maybe I need to change OVS to adapt to virtio-user driver to enable checksum offload.
Quoted 4 lines
Thanks,
Maxime
Thanks,
Harold
Adding Olivier who, iirc, worked on those offloads a long time ago.
Quoted 4 lines · Harold Huang
Device cksum offload capability usually include ipv4 cksum, tcp and udp cksum offload capability. The application such as OVS usually negotiate with the drive like this to enable cksum offload.
OVS checksum offloading is still being worked on.
By this, I mean neither that it is fully functional nor completely broken. But important to keep in mind that it is still a work in progress.
Quoted 11 lines
Signed-off-by Harold Huang <[email protected]>--- drivers/net/virtio/virtio_ethdev.c | 2 ++ 1 file changed, 2 insertions(+)drivers/net/virtio/virtio_ethdev.c +0
-0
drivers/net/virtio/virtio_ethdev.c virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) · line 3047
Quoted 7 lines
drivers/net/virtio/virtio_ethdev.c +1
-0@@ -3041,6 +3041,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) dev_info->rx_offload_capa |=RTE_ETH_RX_OFFLOAD_SCATTER ;if (host_features & (1ULL <<VIRTIO_NET_F_GUEST_CSUM )) { dev_info->rx_offload_capa |= +RTE_ETH_RX_OFFLOAD_IPV4_CKSUM |RTE_ETH_RX_OFFLOAD_TCP_CKSUM |RTE_ETH_RX_OFFLOAD_UDP_CKSUM ; }
Adding this capability alone probably has no effect visible to a dpdk application.
For L3, virtio rx handlers never sets anything but RTE_MBUF_F_RX_IP_CKSUM_UNKNOWN.
https://git.dpdk.org/dpdk/tree/drivers/net/virtio/virtio_rxtx.c#n920
As a consequence, the application can't tell if the ip checksum is correct and the application must validate the checksum of packets on its own.
Maybe we could add some RTE_MBUF_F_RX_IP_CKSUM_GOOD in case of TCP/UDP (?)
https://git.dpdk.org/dpdk/tree/drivers/net/virtio/virtio_rxtx.c#n932
But simply reporting the capability looks wrong to me.
drivers/net/virtio/virtio_ethdev.c virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) · line 3062
Quoted 8 lines
drivers/net/virtio/virtio_ethdev.c +1
-0@@ -3055,6 +3056,7 @@ virtio_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)RTE_ETH_TX_OFFLOAD_VLAN_INSERT ;if (host_features & (1ULL <<VIRTIO_NET_F_CSUM )) { dev_info->tx_offload_capa |= +RTE_ETH_TX_OFFLOAD_IPV4_CKSUM |RTE_ETH_TX_OFFLOAD_UDP_CKSUM |RTE_ETH_TX_OFFLOAD_TCP_CKSUM ; }
For the tx part, I am a bit unsure, but here is what I understand.
With such a capability, the application may think it can send the packet with no checksum, neither ip, nor tcp.
The virtio specification writes that the driver must make sure the ip checksum is correct if it wants to ask for L4 checksum.
"""
If the VIRTIO_NET_F_CSUM feature has been negotiated, the driver MAY set the VIRTIO_NET_HDR_F_NEEDS_CSUM bit in flags, if so:
the driver MUST validate the packet checksum at offset csum_offset from csum_start as well as all preceding offsets;
the driver MUST set the packet checksum stored in the buffer to the TCP/UDP pseudo header;
the driver MUST set csum_start and csum_offset such that calculating a ones’ complement checksum from csum_start up until the end of the packet and storing the result at offset csum_offset from csum_start will result in a fully checksummed packet;
"""
But, nothing in the virtio tx handlers deals with ip cheksum offloading.
https://git.dpdk.org/dpdk/tree/drivers/net/virtio/virtqueue.h#n664
So here too, I think it is wrong to report such a capability.
Signature · 1 line
David Marchand