[2/6] vhost: fix missing enqueue pseudo-header calculation

Message ID 20220505102729.821075-3-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series Vhost checksum offload improvements |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Maxime Coquelin May 5, 2022, 10:27 a.m. UTC
  The Virtio specification requires that in case of checksum
offloading, the pseudo-header checksum must be set in the
L4 header.

When received from another Vhost-user port, the packet
checksum might already contain the pseudo-header checksum
but we have no way to know it. So we have no other choice
than doing the pseudo-header checksum systematically.

This patch handles this using the rte_net_intel_cksum_prepare()
helper.

Fixes: 859b480d5afd ("vhost: add guest offload setting")
Cc: stable@dpdk.org

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/vhost/virtio_net.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
  

Comments

Chenbo Xia May 16, 2022, 1:24 p.m. UTC | #1
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, May 5, 2022 6:27 PM
> To: dev@dpdk.org; jasowang@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>;
> david.marchand@redhat.com; olivier.matz@6wind.com
> Cc: stable@dpdk.org; Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH 2/6] vhost: fix missing enqueue pseudo-header calculation
> 
> The Virtio specification requires that in case of checksum
> offloading, the pseudo-header checksum must be set in the
> L4 header.
> 
> When received from another Vhost-user port, the packet
> checksum might already contain the pseudo-header checksum
> but we have no way to know it. So we have no other choice
> than doing the pseudo-header checksum systematically.
> 
> This patch handles this using the rte_net_intel_cksum_prepare()
> helper.
> 
> Fixes: 859b480d5afd ("vhost: add guest offload setting")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/vhost/virtio_net.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
> index 5f432b0d77..c0ff3357a8 100644
> --- a/lib/vhost/virtio_net.c
> +++ b/lib/vhost/virtio_net.c
> @@ -548,6 +548,16 @@ virtio_enqueue_offload(struct rte_mbuf *m_buf, struct
> virtio_net_hdr *net_hdr)
>  		csum_l4 |= RTE_MBUF_F_TX_TCP_CKSUM;
> 
>  	if (csum_l4) {
> +		/*
> +		 * Pseudo-header checksum must be set as per Virtio spec.
> +		 *
> +		 * Note: We don't propagate rte_net_intel_cksum_prepare()
> +		 * errors, as it would have an impact on performance, and an
> +		 * error would mean the packet is dropped by the guest instead
> +		 * of being dropped here.
> +		 */
> +		rte_net_intel_cksum_prepare(m_buf);
> +
>  		net_hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
>  		net_hdr->csum_start = m_buf->l2_len + m_buf->l3_len;
> 
> --
> 2.35.1

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
  

Patch

diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 5f432b0d77..c0ff3357a8 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -548,6 +548,16 @@  virtio_enqueue_offload(struct rte_mbuf *m_buf, struct virtio_net_hdr *net_hdr)
 		csum_l4 |= RTE_MBUF_F_TX_TCP_CKSUM;
 
 	if (csum_l4) {
+		/*
+		 * Pseudo-header checksum must be set as per Virtio spec.
+		 *
+		 * Note: We don't propagate rte_net_intel_cksum_prepare()
+		 * errors, as it would have an impact on performance, and an
+		 * error would mean the packet is dropped by the guest instead
+		 * of being dropped here.
+		 */
+		rte_net_intel_cksum_prepare(m_buf);
+
 		net_hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
 		net_hdr->csum_start = m_buf->l2_len + m_buf->l3_len;