git: ca0ba818cfd7 - stable/15 - Revert "vtnet: improve checksum offloading"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 08 Sep 2025 20:30:23 UTC
The branch stable/15 has been updated by tuexen:
URL: https://cgit.FreeBSD.org/src/commit/?id=ca0ba818cfd70a73c12bf25ece9951ae8f1fe3b8
commit ca0ba818cfd70a73c12bf25ece9951ae8f1fe3b8
Author: Michael Tuexen <tuexen@FreeBSD.org>
AuthorDate: 2025-09-08 20:27:52 +0000
Commit: Michael Tuexen <tuexen@FreeBSD.org>
CommitDate: 2025-09-08 20:29:59 +0000
Revert "vtnet: improve checksum offloading"
This reverts commit 1c23d8f9f39870951c1d0dfbb112fc4e53237737.
Will be committed again with correct authorship.
(cherry picked from commit f217bc7651a4126a6819da1af03a64e81a551005)
---
share/man/man4/vtnet.4 | 28 ++---
sys/dev/virtio/network/if_vtnet.c | 220 +++++++++++++++++------------------
sys/dev/virtio/network/if_vtnetvar.h | 2 +-
3 files changed, 118 insertions(+), 132 deletions(-)
diff --git a/share/man/man4/vtnet.4 b/share/man/man4/vtnet.4
index 67a835050422..073383df11ff 100644
--- a/share/man/man4/vtnet.4
+++ b/share/man/man4/vtnet.4
@@ -22,7 +22,7 @@
.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
.\" SUCH DAMAGE.
.\"
-.Dd September 4, 2025
+.Dd September 3, 2025
.Dt VTNET 4
.Os
.Sh NAME
@@ -153,14 +153,7 @@ The number of times the receive interrupt handler was rescheduled.
.It Va dev.vtnet. Ns Ar X Ns Va .rxq Ns Ar Y Ns Va .host_lro
The number of times TCP large receive offload was performed.
.It Va dev.vtnet. Ns Ar X Ns Va .rxq Ns Ar Y Ns Va .csum_failed
-The number of times a packet with a request for receive or transmit checksum
-offloading was received and this request failed.
-The different reasons for the failure are counted by
-.Va dev.vtnet. Ns Ar X Ns Va .rx_csum_inaccessible_ipproto ,
-.Va dev.vtnet. Ns Ar X Ns Va .rx_csum_bad_ipproto ,
-.Va dev.vtnet. Ns Ar X Ns Va .rx_csum_bad_ethtype ,
-and
-.Va dev.vtnet. Ns Ar X Ns Va .rx_csum_bad_offset .
+Currently not used.
.It Va dev.vtnet. Ns Ar X Ns Va .rxq Ns Ar Y Ns Va .csum
The number of times receive checksum offloading for UDP or TCP was performed.
.It Va dev.vtnet. Ns Ar X Ns Va .rxq Ns Ar Y Ns Va .ierrors
@@ -220,21 +213,18 @@ over all receive queues of the interface.
The sum of
.Va dev.vtnet. Ns Ar X Ns Va .rxq Ns Ar Y Ns Va .csum_failed
over all receive queues of the interface.
-.It Va dev.vtnet. Ns Ar X Ns Va .rx_csum_inaccessible_ipproto
-The number of times a packet with a request for receive or transmit checksum
-offloading was received where the IP protocol was not accessible.
+.It Va dev.vtnet. Ns Ar X Ns Va .rx_csum_bad_proto
+Currently unused.
.It Va dev.vtnet. Ns Ar X Ns Va .rx_csum_bad_offset
+Currently unused.
+.It Va dev.vtnet. Ns Ar X Ns Va .rx_csum_bad_ipproto
+Currently unused.
+.It Va dev.vtnet. Ns Ar X Ns Va .rx_csum_bad_ethtype
The number of times fixing the checksum required by
.Va hw.vtnet.fixup_needs_csum
or
.Va hw.vtnet. Ns Ar X Ns Va .fixup_needs_csum
-was attempted for a packet where the csum is not located in the first mbuf.
-.It Va dev.vtnet. Ns Ar X Ns Va .rx_csum_bad_ipproto
-The number of times a packet with a request for receive or transmit checksum
-offloading was received where the IP protocol was neither TCP nor UDP.
-.It Va dev.vtnet. Ns Ar X Ns Va .rx_csum_bad_ethtype
-The number of times a packet with a request for receive or transmit checksum
-offloading was received where the EtherType was neither IPv4 nor IPv6.
+was attempted for a packet with an EtherType other than IPv4 or IPv6.
.It Va dev.vtnet. Ns Ar X Ns Va .rx_mergeable_failed
The number of times receiving a mergable buffer failed.
.It Va dev.vtnet. Ns Ar X Ns Va .rx_enq_replacement_failed
diff --git a/sys/dev/virtio/network/if_vtnet.c b/sys/dev/virtio/network/if_vtnet.c
index 73f27ac147ff..afda5157a624 100644
--- a/sys/dev/virtio/network/if_vtnet.c
+++ b/sys/dev/virtio/network/if_vtnet.c
@@ -134,9 +134,9 @@ static int vtnet_rxq_replace_buf(struct vtnet_rxq *, struct mbuf *, int);
static int vtnet_rxq_enqueue_buf(struct vtnet_rxq *, struct mbuf *);
static int vtnet_rxq_new_buf(struct vtnet_rxq *);
static int vtnet_rxq_csum_needs_csum(struct vtnet_rxq *, struct mbuf *,
- bool, int, struct virtio_net_hdr *);
-static void vtnet_rxq_csum_data_valid(struct vtnet_rxq *, struct mbuf *,
- int);
+ uint16_t, int, struct virtio_net_hdr *);
+static int vtnet_rxq_csum_data_valid(struct vtnet_rxq *, struct mbuf *,
+ uint16_t, int, struct virtio_net_hdr *);
static int vtnet_rxq_csum(struct vtnet_rxq *, struct mbuf *,
struct virtio_net_hdr *);
static void vtnet_rxq_discard_merged_bufs(struct vtnet_rxq *, int);
@@ -1762,161 +1762,162 @@ vtnet_rxq_new_buf(struct vtnet_rxq *rxq)
}
static int
-vtnet_rxq_csum_needs_csum(struct vtnet_rxq *rxq, struct mbuf *m, bool isipv6,
- int protocol, struct virtio_net_hdr *hdr)
+vtnet_rxq_csum_needs_csum(struct vtnet_rxq *rxq, struct mbuf *m, uint16_t etype,
+ int hoff, struct virtio_net_hdr *hdr)
{
struct vtnet_softc *sc;
+ int error;
- /*
- * The packet is likely from another VM on the same host or from the
- * host that itself performed checksum offloading so Tx/Rx is basically
- * a memcpy and the checksum has little value so far.
- */
-
- KASSERT(protocol == IPPROTO_TCP || protocol == IPPROTO_UDP,
- ("%s: unsupported IP protocol %d", __func__, protocol));
+ sc = rxq->vtnrx_sc;
/*
- * If the user don't want us to fix it up here by computing the
- * checksum, just forward the order to compute the checksum by setting
- * the corresponding mbuf flag (e.g., CSUM_TCP).
+ * NEEDS_CSUM corresponds to Linux's CHECKSUM_PARTIAL, but FreeBSD does
+ * not have an analogous CSUM flag. The checksum has been validated,
+ * but is incomplete (TCP/UDP pseudo header).
+ *
+ * The packet is likely from another VM on the same host that itself
+ * performed checksum offloading so Tx/Rx is basically a memcpy and
+ * the checksum has little value.
+ *
+ * Default to receiving the packet as-is for performance reasons, but
+ * this can cause issues if the packet is to be forwarded because it
+ * does not contain a valid checksum. This patch may be helpful:
+ * https://reviews.freebsd.org/D6611. In the meantime, have the driver
+ * compute the checksum if requested.
+ *
+ * BMV: Need to add an CSUM_PARTIAL flag?
*/
- sc = rxq->vtnrx_sc;
if ((sc->vtnet_flags & VTNET_FLAG_FIXUP_NEEDS_CSUM) == 0) {
- switch (protocol) {
- case IPPROTO_TCP:
- m->m_pkthdr.csum_flags |=
- (isipv6 ? CSUM_TCP_IPV6 : CSUM_TCP);
- break;
- case IPPROTO_UDP:
- m->m_pkthdr.csum_flags |=
- (isipv6 ? CSUM_UDP_IPV6 : CSUM_UDP);
- break;
- }
- m->m_pkthdr.csum_data = hdr->csum_offset;
- return (0);
+ error = vtnet_rxq_csum_data_valid(rxq, m, etype, hoff, hdr);
+ return (error);
}
/*
* Compute the checksum in the driver so the packet will contain a
* valid checksum. The checksum is at csum_offset from csum_start.
*/
- int csum_off, csum_end;
- uint16_t csum;
+ switch (etype) {
+#if defined(INET) || defined(INET6)
+ case ETHERTYPE_IP:
+ case ETHERTYPE_IPV6: {
+ int csum_off, csum_end;
+ uint16_t csum;
- csum_off = hdr->csum_start + hdr->csum_offset;
- csum_end = csum_off + sizeof(uint16_t);
+ csum_off = hdr->csum_start + hdr->csum_offset;
+ csum_end = csum_off + sizeof(uint16_t);
- /* Assume checksum will be in the first mbuf. */
- if (m->m_len < csum_end || m->m_pkthdr.len < csum_end) {
- sc->vtnet_stats.rx_csum_bad_offset++;
+ /* Assume checksum will be in the first mbuf. */
+ if (m->m_len < csum_end || m->m_pkthdr.len < csum_end)
+ return (1);
+
+ /*
+ * Like in_delayed_cksum()/in6_delayed_cksum(), compute the
+ * checksum and write it at the specified offset. We could
+ * try to verify the packet: csum_start should probably
+ * correspond to the start of the TCP/UDP header.
+ *
+ * BMV: Need to properly handle UDP with zero checksum. Is
+ * the IPv4 header checksum implicitly validated?
+ */
+ csum = in_cksum_skip(m, m->m_pkthdr.len, hdr->csum_start);
+ *(uint16_t *)(mtodo(m, csum_off)) = csum;
+ m->m_pkthdr.csum_flags |= CSUM_DATA_VALID | CSUM_PSEUDO_HDR;
+ m->m_pkthdr.csum_data = 0xFFFF;
+ break;
+ }
+#endif
+ default:
+ sc->vtnet_stats.rx_csum_bad_ethtype++;
return (1);
}
- /*
- * Like in_delayed_cksum()/in6_delayed_cksum(), compute the
- * checksum and write it at the specified offset. We could
- * try to verify the packet: csum_start should probably
- * correspond to the start of the TCP/UDP header.
- *
- * BMV: Need to properly handle UDP with zero checksum. Is
- * the IPv4 header checksum implicitly validated?
- */
- csum = in_cksum_skip(m, m->m_pkthdr.len, hdr->csum_start);
- *(uint16_t *)(mtodo(m, csum_off)) = csum;
- m->m_pkthdr.csum_flags |= CSUM_DATA_VALID | CSUM_PSEUDO_HDR;
- m->m_pkthdr.csum_data = 0xFFFF;
-
return (0);
}
-static void
-vtnet_rxq_csum_data_valid(struct vtnet_rxq *rxq, struct mbuf *m, int protocol)
-{
- KASSERT(protocol == IPPROTO_TCP || protocol == IPPROTO_UDP,
- ("%s: unsupported IP protocol %d", __func__, protocol));
-
- m->m_pkthdr.csum_flags |= CSUM_DATA_VALID | CSUM_PSEUDO_HDR;
- m->m_pkthdr.csum_data = 0xFFFF;
-}
-
static int
-vtnet_rxq_csum(struct vtnet_rxq *rxq, struct mbuf *m,
- struct virtio_net_hdr *hdr)
+vtnet_rxq_csum_data_valid(struct vtnet_rxq *rxq, struct mbuf *m,
+ uint16_t etype, int hoff, struct virtio_net_hdr *hdr __unused)
{
- const struct ether_header *eh;
+#if 0
struct vtnet_softc *sc;
- int hoff, protocol;
- uint16_t etype;
- bool isipv6;
-
- KASSERT(hdr->flags &
- (VIRTIO_NET_HDR_F_NEEDS_CSUM | VIRTIO_NET_HDR_F_DATA_VALID),
- ("%s: missing checksum offloading flag %x", __func__, hdr->flags));
-
- eh = mtod(m, const struct ether_header *);
- etype = ntohs(eh->ether_type);
- if (etype == ETHERTYPE_VLAN) {
- /* TODO BMV: Handle QinQ. */
- const struct ether_vlan_header *evh =
- mtod(m, const struct ether_vlan_header *);
- etype = ntohs(evh->evl_proto);
- hoff = sizeof(struct ether_vlan_header);
- } else
- hoff = sizeof(struct ether_header);
+#endif
+ int protocol;
+#if 0
sc = rxq->vtnrx_sc;
+#endif
- /* Check whether ethernet type is IP or IPv6, and get protocol. */
switch (etype) {
#if defined(INET)
case ETHERTYPE_IP:
- if (__predict_false(m->m_len < hoff + sizeof(struct ip))) {
- sc->vtnet_stats.rx_csum_inaccessible_ipproto++;
- return (1);
- } else {
+ if (__predict_false(m->m_len < hoff + sizeof(struct ip)))
+ protocol = IPPROTO_DONE;
+ else {
struct ip *ip = (struct ip *)(m->m_data + hoff);
protocol = ip->ip_p;
}
- isipv6 = false;
break;
#endif
#if defined(INET6)
case ETHERTYPE_IPV6:
if (__predict_false(m->m_len < hoff + sizeof(struct ip6_hdr))
- || ip6_lasthdr(m, hoff, IPPROTO_IPV6, &protocol) < 0) {
- sc->vtnet_stats.rx_csum_inaccessible_ipproto++;
- return (1);
- }
- isipv6 = true;
+ || ip6_lasthdr(m, hoff, IPPROTO_IPV6, &protocol) < 0)
+ protocol = IPPROTO_DONE;
break;
#endif
default:
- sc->vtnet_stats.rx_csum_bad_ethtype++;
- return (1);
+ protocol = IPPROTO_DONE;
+ break;
}
- /* Check whether protocol is TCP or UDP. */
switch (protocol) {
case IPPROTO_TCP:
case IPPROTO_UDP:
+ m->m_pkthdr.csum_flags |= CSUM_DATA_VALID | CSUM_PSEUDO_HDR;
+ m->m_pkthdr.csum_data = 0xFFFF;
break;
default:
/*
* FreeBSD does not support checksum offloading of this
- * protocol here.
+ * protocol. Let the stack re-verify the checksum later
+ * if the protocol is supported.
*/
- sc->vtnet_stats.rx_csum_bad_ipproto++;
- return (1);
+#if 0
+ if_printf(sc->vtnet_ifp,
+ "%s: checksum offload of unsupported protocol "
+ "etype=%#x protocol=%d csum_start=%d csum_offset=%d\n",
+ __func__, etype, protocol, hdr->csum_start,
+ hdr->csum_offset);
+#endif
+ break;
}
+ return (0);
+}
+
+static int
+vtnet_rxq_csum(struct vtnet_rxq *rxq, struct mbuf *m,
+ struct virtio_net_hdr *hdr)
+{
+ const struct ether_header *eh;
+ int hoff;
+ uint16_t etype;
+
+ eh = mtod(m, const struct ether_header *);
+ etype = ntohs(eh->ether_type);
+ if (etype == ETHERTYPE_VLAN) {
+ /* TODO BMV: Handle QinQ. */
+ const struct ether_vlan_header *evh =
+ mtod(m, const struct ether_vlan_header *);
+ etype = ntohs(evh->evl_proto);
+ hoff = sizeof(struct ether_vlan_header);
+ } else
+ hoff = sizeof(struct ether_header);
+
if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)
- return (vtnet_rxq_csum_needs_csum(rxq, m, isipv6, protocol,
- hdr));
+ return (vtnet_rxq_csum_needs_csum(rxq, m, etype, hoff, hdr));
else /* VIRTIO_NET_HDR_F_DATA_VALID */
- vtnet_rxq_csum_data_valid(rxq, m, protocol);
-
- return (0);
+ return (vtnet_rxq_csum_data_valid(rxq, m, etype, hoff, hdr));
}
static void
@@ -2497,10 +2498,6 @@ vtnet_txq_offload(struct vtnet_txq *txq, struct mbuf *m,
hdr->csum_start = vtnet_gtoh16(sc, csum_start);
hdr->csum_offset = vtnet_gtoh16(sc, m->m_pkthdr.csum_data);
txq->vtntx_stats.vtxs_csum++;
- } else if ((flags & (CSUM_DATA_VALID | CSUM_PSEUDO_HDR)) &&
- (proto == IPPROTO_TCP || proto == IPPROTO_UDP) &&
- (m->m_pkthdr.csum_data == 0xFFFF)) {
- hdr->flags |= VIRTIO_NET_HDR_F_DATA_VALID;
}
if (flags & (CSUM_IP_TSO | CSUM_IP6_TSO)) {
@@ -2614,8 +2611,7 @@ vtnet_txq_encap(struct vtnet_txq *txq, struct mbuf **m_head, int flags)
m->m_flags &= ~M_VLANTAG;
}
- if (m->m_pkthdr.csum_flags &
- (VTNET_CSUM_ALL_OFFLOAD | CSUM_DATA_VALID)) {
+ if (m->m_pkthdr.csum_flags & VTNET_CSUM_ALL_OFFLOAD) {
m = vtnet_txq_offload(txq, m, hdr);
if ((*m_head = m) == NULL) {
error = ENOBUFS;
@@ -4325,9 +4321,9 @@ vtnet_setup_stat_sysctl(struct sysctl_ctx_list *ctx,
SYSCTL_ADD_UQUAD(ctx, child, OID_AUTO, "rx_csum_bad_offset",
CTLFLAG_RD | CTLFLAG_STATS, &stats->rx_csum_bad_offset,
"Received checksum offloaded buffer with incorrect offset");
- SYSCTL_ADD_UQUAD(ctx, child, OID_AUTO, "rx_csum_inaccessible_ipproto",
- CTLFLAG_RD | CTLFLAG_STATS, &stats->rx_csum_inaccessible_ipproto,
- "Received checksum offloaded buffer with inaccessible IP protocol");
+ SYSCTL_ADD_UQUAD(ctx, child, OID_AUTO, "rx_csum_bad_proto",
+ CTLFLAG_RD | CTLFLAG_STATS, &stats->rx_csum_bad_proto,
+ "Received checksum offloaded buffer with incorrect protocol");
SYSCTL_ADD_PROC(ctx, child, OID_AUTO, "rx_csum_failed",
CTLTYPE_U64 | CTLFLAG_RD | CTLFLAG_STATS,
sc, 0, vtnet_sysctl_rx_csum_failed, "QU",
diff --git a/sys/dev/virtio/network/if_vtnetvar.h b/sys/dev/virtio/network/if_vtnetvar.h
index cab7ced639a7..0144b0f3232d 100644
--- a/sys/dev/virtio/network/if_vtnetvar.h
+++ b/sys/dev/virtio/network/if_vtnetvar.h
@@ -46,7 +46,7 @@ struct vtnet_statistics {
uint64_t rx_csum_bad_ethtype;
uint64_t rx_csum_bad_ipproto;
uint64_t rx_csum_bad_offset;
- uint64_t rx_csum_inaccessible_ipproto;
+ uint64_t rx_csum_bad_proto;
uint64_t tx_csum_unknown_ethtype;
uint64_t tx_csum_proto_mismatch;
uint64_t tx_tso_not_tcp;