Re: [PATCH v2] hw/net/net_tx_pkt: Fix virtio header without checksum offloading

[Jason Wang]

On Wed, Mar 27, 2024 at 4:43 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> It is incorrect to have the VIRTIO_NET_HDR_F_NEEDS_CSUM set when
> checksum offloading is disabled so clear the bit.
>
> TCP/UDP checksum is usually offloaded when the peer requires virtio
> headers because they can instruct the peer to compute checksum. However,
> igb disables TX checksum offloading when a VF is enabled whether the
> peer requires virtio headers because a transmitted packet can be routed
> to it and it expects the packet has a proper checksum. Therefore, it
> is necessary to have a correct virtio header even when checksum
> offloading is disabled.
>
> A real TCP/UDP checksum will be computed and saved in the buffer when
> checksum offloading is disabled. The virtio specification requires to
> set the packet checksum stored in the buffer to the TCP/UDP pseudo
> header when the VIRTIO_NET_HDR_F_NEEDS_CSUM bit is set so the bit must
> be cleared in that case.
>
> Fixes: ffbd2dbd8e64 ("e1000e: Perform software segmentation for loopback")
> Buglink: https://issues.redhat.com/browse/RHEL-23067
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> Changes in v2:
> - Dropped VIRTIO_NET_HDR_F_DATA_VALID. (Jason Wang)
> - Link to v1: 
> 20240324-tx-v1-1-a3b4135749ec@daynix.com">https://lore.kernel.org/r/20240324-tx-v1-1-a3b4135749ec@daynix.com

Queued.

Thanks

> ---
>  hw/net/net_tx_pkt.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
> index 2e5f58b3c9cc..2134a18c4c90 100644
> --- a/hw/net/net_tx_pkt.c
> +++ b/hw/net/net_tx_pkt.c
> @@ -833,6 +833,7 @@ bool net_tx_pkt_send_custom(struct NetTxPkt *pkt, bool 
> offload,
>
>      if (offload || gso_type == VIRTIO_NET_HDR_GSO_NONE) {
>          if (!offload && pkt->virt_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
> +            pkt->virt_hdr.flags &= ~VIRTIO_NET_HDR_F_NEEDS_CSUM;
>              net_tx_pkt_do_sw_csum(pkt, &pkt->vec[NET_TX_PKT_L2HDR_FRAG],
>                                    pkt->payload_frags + 
> NET_TX_PKT_PL_START_FRAG - 1,
>                                    pkt->payload_len);
>
> ---
> base-commit: ba49d760eb04630e7b15f423ebecf6c871b8f77b
> change-id: 20240324-tx-c57d3c22ad73
>
> Best regards,
> --
> Akihiko Odaki <akihiko.odaki@daynix.com>
>