On Sun, Mar 24, 2024 at 4:32 PM Akihiko Odaki <akihiko.od...@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. Set the > VIRTIO_NET_HDR_F_DATA_VALID bit instead to tell the checksum is valid. > > 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. > > The VIRTIO_NET_HDR_F_NEEDS_CSUM bit also tells to skip checksum > validation. Even if checksum offloading is disabled, it is desirable to > skip checksum validation because the checksum is always correct. Use the > VIRTIO_NET_HDR_F_DATA_VALID bit to claim the validity of the checksum. > > Fixes: ffbd2dbd8e64 ("e1000e: Perform software segmentation for loopback") > Buglink: https://issues.redhat.com/browse/RHEL-23067 > Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com> > --- > hw/net/net_tx_pkt.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c > index 2e5f58b3c9cc..c225cf706513 100644 > --- a/hw/net/net_tx_pkt.c > +++ b/hw/net/net_tx_pkt.c > @@ -833,6 +833,9 @@ 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 = > + (pkt->virt_hdr.flags & ~VIRTIO_NET_HDR_F_NEEDS_CSUM) | > + VIRTIO_NET_HDR_F_DATA_VALID;
Why VIRTIO_NET_HDR_F_DATA_VALID is used in TX path? Thanks > 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.od...@daynix.com> >