Re: 44cddaa99dee - main - vtnet: move offload functions to virtio_net.h to share them

> On 21. Aug 2026, at 17:22, Timo Völker <[email protected]> wrote:
> 
> Hi Ravi,
> 
> You are right. My patch changed the file sys/dev/virtio/network/virtio_net.h and put the virtio_net_rx_csum function within a #if defined(INET) || defined(INET6). I’m sorry, I didn’t test it with LINT-NOIP.
> 
> @Michael: Can you revert the commit? I’ll work on an improved patch.
Done: https://cgit.FreeBSD.org/src/commit/?id=8bea280f4325d10858ec58ae7251db7604fa77a2 <https://cgit.freebsd.org/src/commit/?id=8bea280f4325d10858ec58ae7251db7604fa77a2>

Best regards
Michael
> 
> Timo
> 
> 
>> On 21. Aug 2026, at 16:04, Ravi Pokala <[email protected]> wrote:
>> 
>> Hi Michael, Timo,
>> 
>> | @@ -1927,7 +1936,13 @@ tunwrite_l2(struct tuntap_softc *tp, struct mbuf *m,
>> | }
>> | 
>> | if (vhdr != NULL) {
>> | - if (virtio_net_rx_csum(m, &vhdr->hdr)) {
>> | + /*
>> | + * Translate the VirtIO header flags to the corresponding
>> | + * CSUM_* flags in the mbuf.
>> | + */
>> | + if (((vhdr->hdr.flags & (VIRTIO_NET_HDR_F_NEEDS_CSUM |
>> | +      VIRTIO_NET_HDR_F_DATA_VALID)) != 0) &&
>> | +    (virtio_net_rx_csum(m, &vhdr->hdr) != 0)) {
>> | m_freem(m);
>> | return (0);
>> | }
>> 
>> This chunk appears to break the amd64.LINT-NOIP kernconf:
>> 
>> | --------------------------------------------------------------
>> | >>> stage 3.1: building everything
>> | --------------------------------------------------------------
>> | ${SRCROOT}/sys/net/if_tuntap.c:1945:8: error: call to undeclared function 'virtio_net_rx_csum'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
>> |  1945 |                     (virtio_net_rx_csum(m, &vhdr->hdr) != 0)) {
>> |       |                      ^
>> | 1 error generated.
>> | --- if_tuntap.o ---
>> | *** [if_tuntap.o] Error code 1
>> 
>> Thanks,
>> 
>> Ravi (rpokala@)
>> 
>> 
>> -----Original Message-----
>> From: <[email protected] <mailto:[email protected]>> on behalf of Michael Tuexen <[email protected] <mailto:[email protected]>>
>> Date: Tuesday, August 18, 2026 at 08:54
>> To: <[email protected] <mailto:[email protected]>>, <[email protected] <mailto:[email protected]>>, <[email protected] <mailto:[email protected]>>
>> Cc: Timo Völker <[email protected] <mailto:[email protected]>>
>> Subject: git: 44cddaa99dee - main - vtnet: move offload functions to virtio_net.h to share them
>> 
>> 
>> The branch main has been updated by tuexen:
>> 
>> 
>> URL: https://cgit.FreeBSD.org/src/commit/?id=44cddaa99dee0a634cf2713f71e799eb41397355 <https://cgit.FreeBSD.org/src/commit/?id=44cddaa99dee0a634cf2713f71e799eb41397355>
>> 
>> 
>> commit 44cddaa99dee0a634cf2713f71e799eb41397355
>> Author: Timo Völker <[email protected] <mailto:[email protected]>>
>> AuthorDate: 2026-08-18 12:49:40 +0000
>> Commit: Michael Tuexen <[email protected] <mailto:[email protected]>>
>> CommitDate: 2026-08-18 12:49:40 +0000
>> 
>> 
>> vtnet: move offload functions to virtio_net.h to share them
>> 
>> 
>> Move the functions vtnet_rxq_csum() and vtnet_txq_offload() and the
>> subfunctions they call from if_vtnet.c to virtio_net.h. This allows
>> us to call these functions from if_tuntap.c and if_ptnet.c.
>> virtio_net.h already contained a copy of these functions, but a copy
>> of an outdated version. The functions evolved in if_vtnet.c.
>> In if_vtnet.c, the copy has never been used because it increments
>> counters in their own functions.
>> This patch removes the outdated copy from virtio_net.h and moves the
>> new version of the functions from if_vtnet.c to virtio_net.h.
>> if_tuntap.c, if_ptnet.c, and if_vtnet.c just call these functions,
>> and if_vtnet.c increments its counters depending on the return value.
>> 
>> 
>> Reviewed by: tuexen
>> MFC after: 1 month
>> MFC to: stable/15
>> Differential Revision: https://reviews.freebsd.org/D57299 <https://reviews.freebsd.org/D57299>
>> ---
>> sys/dev/netmap/if_ptnet.c | 22 ++-
>> sys/dev/virtio/network/if_vtnet.c | 345 +++++-------------------------------
>> sys/dev/virtio/network/virtio_net.h | 341 +++++++++++++++++++----------------
>> sys/net/if_tuntap.c | 19 +-
>> 4 files changed, 269 insertions(+), 458 deletions(-)
>> 
>> 
>> diff --git a/sys/dev/netmap/if_ptnet.c b/sys/dev/netmap/if_ptnet.c
>> index 9c06f7fec530..660b1930aa76 100644
>> --- a/sys/dev/netmap/if_ptnet.c
>> +++ b/sys/dev/netmap/if_ptnet.c
>> @@ -1464,8 +1464,17 @@ ptnet_drain_transmit_queue(struct ptnet_queue *pq, unsigned int budget,
>> * two 8-bytes-wide writes. */
>> memset(nmbuf, 0, PTNET_HDR_SIZE);
>> if (mhead->m_pkthdr.csum_flags & PTNET_ALL_OFFLOAD) {
>> - mhead = virtio_net_tx_offload(ifp, mhead, false,
>> - vh);
>> + /*
>> + * Translate the CSUM_* flags in the mbuf to the
>> + * corresponding flags in the VirtIO header.
>> + *
>> + * ptnet does not negotiate ECN and orders the
>> + * bytes in the VirtIO header as if the VirtIO
>> + * modern mode is not used. So, pass false for
>> + * both.
>> + */
>> + virtio_net_tx_offload(ifp, &mhead, vh, false,
>> + false);
>> if (unlikely(!mhead)) {
>> /* Packet dropped because errors
>> * occurred while preparing the vnet
>> @@ -1842,7 +1851,14 @@ host_sync:
>> }
>> }
>> 
>> 
>> - if (unlikely(have_vnet_hdr && virtio_net_rx_csum(mhead, vh))) {
>> + /*
>> + * Translate the VirtIO header flags to the corresponding
>> + * CSUM_* flags in the mbuf.
>> + */
>> + if (unlikely(have_vnet_hdr &&
>> + ((vh->flags & (VIRTIO_NET_HDR_F_NEEDS_CSUM |
>> + VIRTIO_NET_HDR_F_DATA_VALID)) != 0) &&
>> + (virtio_net_rx_csum(mhead, vh) != 0))) {
>> m_freem(mhead);
>> nm_prlim(1, "Csum offload error: dropping");
>> pq->stats.iqdrops ++;
>> diff --git a/sys/dev/virtio/network/if_vtnet.c b/sys/dev/virtio/network/if_vtnet.c
>> index e9683698decb..75a0d909f126 100644
>> --- a/sys/dev/virtio/network/if_vtnet.c
>> +++ b/sys/dev/virtio/network/if_vtnet.c
>> @@ -145,14 +145,6 @@ static int vtnet_rxq_replace_lro_nomrg_buf(struct vtnet_rxq *,
>> 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 *);
>> -#if defined(INET) || defined(INET6)
>> -static void 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);
>> -static int vtnet_rxq_csum(struct vtnet_rxq *, struct mbuf *,
>> - struct virtio_net_hdr *);
>> -#endif
>> static void vtnet_rxq_discard_merged_bufs(struct vtnet_rxq *, int);
>> static void vtnet_rxq_discard_buf(struct vtnet_rxq *, struct mbuf *);
>> static int vtnet_rxq_merged_eof(struct vtnet_rxq *, struct mbuf *, int);
>> @@ -167,13 +159,6 @@ static int vtnet_txq_intr_threshold(struct vtnet_txq *);
>> static int vtnet_txq_below_threshold(struct vtnet_txq *);
>> static int vtnet_txq_notify(struct vtnet_txq *);
>> static void vtnet_txq_free_mbufs(struct vtnet_txq *);
>> -static int vtnet_txq_offload_ctx(struct vtnet_txq *, struct mbuf *,
>> - int *, int *, int *);
>> -static int vtnet_txq_offload_tso(struct vtnet_txq *, struct mbuf *, int,
>> - int, struct virtio_net_hdr *);
>> -static struct mbuf *
>> - vtnet_txq_offload(struct vtnet_txq *, struct mbuf *,
>> - struct virtio_net_hdr *);
>> static int vtnet_txq_enqueue_buf(struct vtnet_txq *, struct mbuf **,
>> struct vtnet_tx_header *);
>> static int vtnet_txq_encap(struct vtnet_txq *, struct mbuf **, int);
>> @@ -1964,124 +1949,6 @@ vtnet_rxq_new_buf(struct vtnet_rxq *rxq)
>> return (error);
>> }
>> 
>> 
>> -#if defined(INET) || defined(INET6)
>> -static void
>> -vtnet_rxq_csum_needs_csum(struct vtnet_rxq *rxq, struct mbuf *m, bool isipv6,
>> - int protocol, struct virtio_net_hdr *hdr)
>> -{
>> - /*
>> - * 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));
>> -
>> - /*
>> - * Just forward the order to compute the checksum by setting
>> - * the corresponding mbuf flag (e.g., CSUM_TCP).
>> - */
>> - 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;
>> -}
>> -
>> -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)
>> -{
>> - const struct ether_header *eh;
>> - 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);
>> -
>> - sc = rxq->vtnrx_sc;
>> -
>> - /* 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 {
>> - 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;
>> - break;
>> -#endif
>> - default:
>> - sc->vtnet_stats.rx_csum_bad_ethtype++;
>> - return (1);
>> - }
>> -
>> - /* Check whether protocol is TCP or UDP. */
>> - switch (protocol) {
>> - case IPPROTO_TCP:
>> - case IPPROTO_UDP:
>> - break;
>> - default:
>> - /*
>> - * FreeBSD does not support checksum offloading of this
>> - * protocol here.
>> - */
>> - sc->vtnet_stats.rx_csum_bad_ipproto++;
>> - return (1);
>> - }
>> -
>> - if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)
>> - vtnet_rxq_csum_needs_csum(rxq, m, isipv6, protocol, hdr);
>> - else /* VIRTIO_NET_HDR_F_DATA_VALID */
>> - vtnet_rxq_csum_data_valid(rxq, m, protocol);
>> -
>> - return (0);
>> -}
>> -#endif
>> -
>> static void
>> vtnet_rxq_discard_merged_bufs(struct vtnet_rxq *rxq, int nbufs)
>> {
>> @@ -2220,10 +2087,29 @@ vtnet_rxq_input(struct vtnet_rxq *rxq, struct mbuf *m,
>> if (hdr->flags &
>> (VIRTIO_NET_HDR_F_NEEDS_CSUM | VIRTIO_NET_HDR_F_DATA_VALID)) {
>> #if defined(INET) || defined(INET6)
>> - if (vtnet_rxq_csum(rxq, m, hdr) == 0)
>> + int ret;
>> +
>> + /*
>> + * Translate the VirtIO header flags to the corresponding
>> + * CSUM_* flags in the mbuf.
>> + */
>> + ret = virtio_net_rx_csum(m, hdr);
>> + if (ret == 0)
>> rxq->vtnrx_stats.vrxs_csum++;
>> - else
>> + else {
>> + switch (ret) {
>> + case VIRTIO_NET_RX_CSUM_INACCESSIBLE_IPPROTO:
>> + sc->vtnet_stats.rx_csum_inaccessible_ipproto++;
>> + break;
>> + case VIRTIO_NET_RX_CSUM_BAD_ETHTYPE:
>> + sc->vtnet_stats.rx_csum_bad_ethtype++;
>> + break;
>> + case VIRTIO_NET_RX_CSUM_BAD_IPPROTO:
>> + sc->vtnet_stats.rx_csum_bad_ipproto++;
>> + break;
>> + }
>> rxq->vtnrx_stats.vrxs_csum_failed++;
>> + }
>> #else
>> sc->vtnet_stats.rx_csum_bad_ethtype++;
>> rxq->vtnrx_stats.vrxs_csum_failed++;
>> @@ -2587,166 +2473,6 @@ vtnet_txq_free_mbufs(struct vtnet_txq *txq)
>> ("%s: mbufs remaining in tx queue %p", __func__, txq));
>> }
>> 
>> 
>> -/*
>> - * BMV: This can go away once we finally have offsets in the mbuf header.
>> - */
>> -static int
>> -vtnet_txq_offload_ctx(struct vtnet_txq *txq, struct mbuf *m, int *etype,
>> - int *proto, int *start)
>> -{
>> - struct vtnet_softc *sc;
>> - struct ether_vlan_header *evh;
>> -#if defined(INET) || defined(INET6)
>> - int offset;
>> -#endif
>> -
>> - sc = txq->vtntx_sc;
>> -
>> - evh = mtod(m, struct ether_vlan_header *);
>> - if (evh->evl_encap_proto == htons(ETHERTYPE_VLAN)) {
>> - /* BMV: We should handle nested VLAN tags too. */
>> - *etype = ntohs(evh->evl_proto);
>> -#if defined(INET) || defined(INET6)
>> - offset = sizeof(struct ether_vlan_header);
>> -#endif
>> - } else {
>> - *etype = ntohs(evh->evl_encap_proto);
>> -#if defined(INET) || defined(INET6)
>> - offset = sizeof(struct ether_header);
>> -#endif
>> - }
>> -
>> - switch (*etype) {
>> -#if defined(INET)
>> - case ETHERTYPE_IP: {
>> - struct ip *ip, iphdr;
>> - if (__predict_false(m->m_len < offset + sizeof(struct ip))) {
>> - m_copydata(m, offset, sizeof(struct ip),
>> - (caddr_t) &iphdr);
>> - ip = &iphdr;
>> - } else
>> - ip = (struct ip *)(m->m_data + offset);
>> - *proto = ip->ip_p;
>> - *start = offset + (ip->ip_hl << 2);
>> - break;
>> - }
>> -#endif
>> -#if defined(INET6)
>> - case ETHERTYPE_IPV6:
>> - *proto = -1;
>> - *start = ip6_lasthdr(m, offset, IPPROTO_IPV6, proto);
>> - /* Assert the network stack sent us a valid packet. */
>> - KASSERT(*start > offset,
>> - ("%s: mbuf %p start %d offset %d proto %d", __func__, m,
>> - *start, offset, *proto));
>> - break;
>> -#endif
>> - default:
>> - sc->vtnet_stats.tx_csum_unknown_ethtype++;
>> - return (EINVAL);
>> - }
>> -
>> - return (0);
>> -}
>> -
>> -static int
>> -vtnet_txq_offload_tso(struct vtnet_txq *txq, struct mbuf *m, int eth_type,
>> - int offset, struct virtio_net_hdr *hdr)
>> -{
>> - static struct timeval lastecn;
>> - static int curecn;
>> - struct vtnet_softc *sc;
>> - struct tcphdr *tcp, tcphdr;
>> -
>> - sc = txq->vtntx_sc;
>> -
>> - if (__predict_false(m->m_len < offset + sizeof(struct tcphdr))) {
>> - m_copydata(m, offset, sizeof(struct tcphdr), (caddr_t) &tcphdr);
>> - tcp = &tcphdr;
>> - } else
>> - tcp = (struct tcphdr *)(m->m_data + offset);
>> -
>> - hdr->hdr_len = vtnet_gtoh16(sc, offset + (tcp->th_off << 2));
>> - hdr->gso_size = vtnet_gtoh16(sc, m->m_pkthdr.tso_segsz);
>> - hdr->gso_type = eth_type == ETHERTYPE_IP ? VIRTIO_NET_HDR_GSO_TCPV4 :
>> - VIRTIO_NET_HDR_GSO_TCPV6;
>> -
>> - if (__predict_false(tcp_get_flags(tcp) & TH_CWR)) {
>> - /*
>> - * Drop if VIRTIO_NET_F_HOST_ECN was not negotiated. In
>> - * FreeBSD, ECN support is not on a per-interface basis,
>> - * but globally via the net.inet.tcp.ecn.enable sysctl
>> - * knob. The default is off.
>> - */
>> - if ((sc->vtnet_flags & VTNET_FLAG_TSO_ECN) == 0) {
>> - if (ppsratecheck(&lastecn, &curecn, 1))
>> - if_printf(sc->vtnet_ifp,
>> - "TSO with ECN not negotiated with host\n");
>> - return (ENOTSUP);
>> - }
>> - hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
>> - }
>> -
>> - txq->vtntx_stats.vtxs_tso++;
>> -
>> - return (0);
>> -}
>> -
>> -static struct mbuf *
>> -vtnet_txq_offload(struct vtnet_txq *txq, struct mbuf *m,
>> - struct virtio_net_hdr *hdr)
>> -{
>> - struct vtnet_softc *sc;
>> - int flags, etype, csum_start, proto, error;
>> -
>> - sc = txq->vtntx_sc;
>> - flags = m->m_pkthdr.csum_flags;
>> -
>> - error = vtnet_txq_offload_ctx(txq, m, &etype, &proto, &csum_start);
>> - if (error)
>> - goto drop;
>> -
>> - if (flags & (VTNET_CSUM_OFFLOAD | VTNET_CSUM_OFFLOAD_IPV6)) {
>> - /* Sanity check the parsed mbuf matches the offload flags. */
>> - if (__predict_false((flags & VTNET_CSUM_OFFLOAD &&
>> - etype != ETHERTYPE_IP) || (flags & VTNET_CSUM_OFFLOAD_IPV6
>> - && etype != ETHERTYPE_IPV6))) {
>> - sc->vtnet_stats.tx_csum_proto_mismatch++;
>> - goto drop;
>> - }
>> -
>> - hdr->flags |= VIRTIO_NET_HDR_F_NEEDS_CSUM;
>> - hdr->csum_start = vtnet_gtoh16(sc, csum_start);
>> - hdr->csum_offset = vtnet_gtoh16(sc, m->m_pkthdr.csum_data);
>> - txq->vtntx_stats.vtxs_csum++;
>> - }
>> -
>> - if (flags & (CSUM_IP_TSO | CSUM_IP6_TSO)) {
>> - /*
>> - * Sanity check the parsed mbuf IP protocol is TCP, and
>> - * VirtIO TSO reqires the checksum offloading above.
>> - */
>> - if (__predict_false(proto != IPPROTO_TCP)) {
>> - sc->vtnet_stats.tx_tso_not_tcp++;
>> - goto drop;
>> - } else if (__predict_false((hdr->flags &
>> - VIRTIO_NET_HDR_F_NEEDS_CSUM) == 0)) {
>> - sc->vtnet_stats.tx_tso_without_csum++;
>> - goto drop;
>> - }
>> -
>> - error = vtnet_txq_offload_tso(txq, m, etype, csum_start, hdr);
>> - if (error)
>> - goto drop;
>> - }
>> -
>> - return (m);
>> -
>> -drop:
>> - m_freem(m);
>> - return (NULL);
>> -}
>> -
>> static void
>> vtnet_txq_enqueue_callback(void *arg, bus_dma_segment_t *segs,
>> int nsegs, int error)
>> @@ -2919,11 +2645,38 @@ vtnet_txq_encap(struct vtnet_txq *txq, struct mbuf **m_head, int flags)
>> }
>> 
>> 
>> if (m->m_pkthdr.csum_flags & VTNET_CSUM_ALL_OFFLOAD) {
>> - m = vtnet_txq_offload(txq, m, hdr);
>> + int ret;
>> +
>> + /*
>> + * Translate the CSUM_* flags in the mbuf to the corresponding
>> + * flags in the VirtIO header.
>> + */
>> + ret = virtio_net_tx_offload(txq->vtntx_sc->vtnet_ifp, &m, hdr,
>> + (txq->vtntx_sc->vtnet_flags & VTNET_FLAG_TSO_ECN),
>> + vtnet_modern(txq->vtntx_sc));
>> + switch (ret) {
>> + case VIRTIO_NET_TX_OFFLOAD_UNKNOWN_ETHTYPE:
>> + txq->vtntx_sc->vtnet_stats.tx_csum_unknown_ethtype++;
>> + break;
>> + case VIRTIO_NET_TX_OFFLOAD_PROTO_MISMATCH:
>> + txq->vtntx_sc->vtnet_stats.tx_csum_proto_mismatch++;
>> + break;
>> + case VIRTIO_NET_TX_OFFLOAD_TSO_NOT_TCP:
>> + txq->vtntx_sc->vtnet_stats.tx_tso_not_tcp++;
>> + break;
>> + case VIRTIO_NET_TX_OFFLOAD_TSO_WITHOUT_CSUM:
>> + txq->vtntx_sc->vtnet_stats.tx_tso_without_csum++;
>> + break;
>> + }
>> if ((*m_head = m) == NULL) {
>> error = ENOBUFS;
>> goto fail;
>> }
>> + if (m->m_pkthdr.csum_flags &
>> + (VTNET_CSUM_OFFLOAD | VTNET_CSUM_OFFLOAD_IPV6))
>> + txq->vtntx_stats.vtxs_csum++;
>> + if (m->m_pkthdr.csum_flags & (CSUM_IP_TSO | CSUM_IP6_TSO))
>> + txq->vtntx_stats.vtxs_tso++;
>> }
>> 
>> 
>> error = vtnet_txq_enqueue_buf(txq, m_head, txhdr);
>> diff --git a/sys/dev/virtio/network/virtio_net.h b/sys/dev/virtio/network/virtio_net.h
>> index f4256664fba8..16d081b0a92a 100644
>> --- a/sys/dev/virtio/network/virtio_net.h
>> +++ b/sys/dev/virtio/network/virtio_net.h
>> @@ -31,6 +31,11 @@
>> #ifndef _VIRTIO_NET_H
>> #define _VIRTIO_NET_H
>> 
>> 
>> +#include "opt_inet.h"
>> +#include "opt_inet6.h"
>> +
>> +#include <sys/endian.h>
>> +
>> /* The feature bitmap for virtio net */
>> #define VIRTIO_NET_F_CSUM (1ULL << 0) /* Host handles pkts w/ partial csum */
>> #define VIRTIO_NET_F_GUEST_CSUM (1ULL << 1) /* Guest handles pkts w/ partial csum*/
>> @@ -260,161 +265,150 @@ struct virtio_net_ctrl_mq {
>> #define VIRTIO_NET_CTRL_GUEST_OFFLOADS 5
>> #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET 0
>> 
>> 
>> -/*
>> - * Use the checksum offset in the VirtIO header to set the
>> - * correct CSUM_* flags.
>> - */
>> -static inline int
>> -virtio_net_rx_csum_by_offset(struct mbuf *m, uint16_t eth_type, int ip_start,
>> - struct virtio_net_hdr *hdr)
>> -{
>> #if defined(INET) || defined(INET6)
>> - int offset = hdr->csum_start + hdr->csum_offset;
>> -#endif
>> +static inline void
>> +virtio_net_rx_csum_needs_csum(struct mbuf *m, bool isipv6, int protocol,
>> + struct virtio_net_hdr *hdr)
>> +{
>> + /*
>> + * 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.
>> + */
>> 
>> 
>> - /* Only do a basic sanity check on the offset. */
>> - switch (eth_type) {
>> -#if defined(INET)
>> - case ETHERTYPE_IP:
>> - if (__predict_false(offset < ip_start + sizeof(struct ip)))
>> - return (1);
>> - break;
>> -#endif
>> -#if defined(INET6)
>> - case ETHERTYPE_IPV6:
>> - if (__predict_false(offset < ip_start + sizeof(struct ip6_hdr)))
>> - return (1);
>> - break;
>> -#endif
>> - default:
>> - /* Here we should increment the rx_csum_bad_ethtype counter. */
>> - return (1);
>> - }
>> + KASSERT(protocol == IPPROTO_TCP || protocol == IPPROTO_UDP,
>> + ("%s: unsupported IP protocol %d", __func__, protocol));
>> 
>> 
>> /*
>> - * Use the offset to determine the appropriate CSUM_* flags. This is
>> - * a bit dirty, but we can get by with it since the checksum offsets
>> - * happen to be different. We assume the host host does not do IPv4
>> - * header checksum offloading.
>> + * Just forward the order to compute the checksum by setting
>> + * the corresponding mbuf flag (e.g., CSUM_TCP).
>> */
>> - switch (hdr->csum_offset) {
>> - case offsetof(struct udphdr, uh_sum):
>> - case offsetof(struct tcphdr, th_sum):
>> - m->m_pkthdr.csum_flags |= CSUM_DATA_VALID | CSUM_PSEUDO_HDR;
>> - m->m_pkthdr.csum_data = 0xFFFF;
>> + 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;
>> - default:
>> - /* Here we should increment the rx_csum_bad_offset counter. */
>> - return (1);
>> }
>> + m->m_pkthdr.csum_data = hdr->csum_offset;
>> +}
>> 
>> 
>> - return (0);
>> +static inline void
>> +virtio_net_rx_csum_data_valid(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;
>> }
>> 
>> 
>> +#define VIRTIO_NET_RX_CSUM_INACCESSIBLE_IPPROTO 1
>> +#define VIRTIO_NET_RX_CSUM_BAD_ETHTYPE 2
>> +#define VIRTIO_NET_RX_CSUM_BAD_IPPROTO 3
>> +
>> +/*
>> + * For a packet received over the VirtIO channel, it checks the given
>> + * VirtIO header and sets the appropriate CSUM_* flags in the given mbuf.
>> + *
>> + * Unfortunately, the information provided is not directly useful to us. The
>> + * VirtIO header gives the offset of the checksum, which is all Linux needs, but
>> + * this is not how FreeBSD does things. We are forced to peek inside the packet
>> + * a bit.
>> + *
>> + * It would be nice if VirtIO gave us the L4 protocol or if FreeBSD
>> + * could accept the offsets and let the stack figure it out.
>> + *
>> + * @param m mbuf of the packet where CSUM_* flags might need to be set.
>> + * @param hdr VirtIO header of the received packet that needs to be checked
>> + * with its field values stored in the byte order this machine
>> + * uses (i.e., readable without a byte swap).
>> + *
>> + * @return 0 on success, or one of the VIRTIO_NET_RX_CSUM_* error codes.
>> + */
>> static inline int
>> -virtio_net_rx_csum_by_parse(struct mbuf *m, uint16_t eth_type, int ip_start,
>> - struct virtio_net_hdr *hdr)
>> +virtio_net_rx_csum(struct mbuf *m, struct virtio_net_hdr *hdr)
>> {
>> - int offset, proto;
>> + const struct ether_header *eh;
>> + 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);
>> 
>> 
>> - switch (eth_type) {
>> + /* Check whether ethernet type is IP or IPv6, and get protocol. */
>> + switch (etype) {
>> #if defined(INET)
>> - case ETHERTYPE_IP: {
>> - struct ip *ip;
>> - if (__predict_false(m->m_len < ip_start + sizeof(struct ip)))
>> - return (1);
>> - ip = (struct ip *)(m->m_data + ip_start);
>> - proto = ip->ip_p;
>> - offset = ip_start + (ip->ip_hl << 2);
>> + case ETHERTYPE_IP:
>> + if (__predict_false(m->m_len < hoff + sizeof(struct ip))) {
>> + return (VIRTIO_NET_RX_CSUM_INACCESSIBLE_IPPROTO);
>> + } 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 < ip_start +
>> - sizeof(struct ip6_hdr)))
>> - return (1);
>> - offset = ip6_lasthdr(m, ip_start, IPPROTO_IPV6, &proto);
>> - if (__predict_false(offset < 0))
>> - return (1);
>> + if (__predict_false(m->m_len < hoff + sizeof(struct ip6_hdr))
>> + || ip6_lasthdr(m, hoff, IPPROTO_IPV6, &protocol) < 0) {
>> + return (VIRTIO_NET_RX_CSUM_INACCESSIBLE_IPPROTO);
>> + }
>> + isipv6 = true;
>> break;
>> #endif
>> default:
>> - /* Here we should increment the rx_csum_bad_ethtype counter. */
>> - return (1);
>> + return (VIRTIO_NET_RX_CSUM_BAD_ETHTYPE);
>> }
>> 
>> 
>> - switch (proto) {
>> + /* Check whether protocol is TCP or UDP. */
>> + switch (protocol) {
>> case IPPROTO_TCP:
>> - if (__predict_false(m->m_len < offset + sizeof(struct tcphdr)))
>> - return (1);
>> - m->m_pkthdr.csum_flags |= CSUM_DATA_VALID | CSUM_PSEUDO_HDR;
>> - m->m_pkthdr.csum_data = 0xFFFF;
>> - break;
>> case IPPROTO_UDP:
>> - if (__predict_false(m->m_len < offset + sizeof(struct udphdr)))
>> - return (1);
>> - m->m_pkthdr.csum_flags |= CSUM_DATA_VALID | CSUM_PSEUDO_HDR;
>> - m->m_pkthdr.csum_data = 0xFFFF;
>> break;
>> default:
>> /*
>> - * For the remaining protocols, FreeBSD does not support
>> - * checksum offloading, so the checksum will be recomputed.
>> + * FreeBSD does not support checksum offloading of this
>> + * protocol here.
>> */
>> -#if 0
>> - if_printf(ifp, "cksum offload of unsupported "
>> - "protocol eth_type=%#x proto=%d csum_start=%d "
>> - "csum_offset=%d\n", __func__, eth_type, proto,
>> - hdr->csum_start, hdr->csum_offset);
>> -#endif
>> - break;
>> + return (VIRTIO_NET_RX_CSUM_BAD_IPPROTO);
>> }
>> 
>> 
>> + if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)
>> + virtio_net_rx_csum_needs_csum(m, isipv6, protocol, hdr);
>> + else /* VIRTIO_NET_HDR_F_DATA_VALID */
>> + virtio_net_rx_csum_data_valid(m, protocol);
>> +
>> return (0);
>> }
>> +#endif
>> 
>> 
>> -/*
>> - * Set the appropriate CSUM_* flags. Unfortunately, the information
>> - * provided is not directly useful to us. The VirtIO header gives the
>> - * offset of the checksum, which is all Linux needs, but this is not
>> - * how FreeBSD does things. We are forced to peek inside the packet
>> - * a bit.
>> - *
>> - * It would be nice if VirtIO gave us the L4 protocol or if FreeBSD
>> - * could accept the offsets and let the stack figure it out.
>> - */
>> -static inline int
>> -virtio_net_rx_csum(struct mbuf *m, struct virtio_net_hdr *hdr)
>> -{
>> - struct ether_header *eh;
>> - struct ether_vlan_header *evh;
>> - uint16_t eth_type;
>> - int offset, error;
>> -
>> - if ((hdr->flags & (VIRTIO_NET_HDR_F_NEEDS_CSUM |
>> - VIRTIO_NET_HDR_F_DATA_VALID)) == 0) {
>> - return (0);
>> - }
>> +#define VIRTIO_NET_TX_OFFLOAD_UNKNOWN_ETHTYPE 1
>> +#define VIRTIO_NET_TX_OFFLOAD_PROTO_MISMATCH 2
>> +#define VIRTIO_NET_TX_OFFLOAD_TSO_NOT_TCP 3
>> +#define VIRTIO_NET_TX_OFFLOAD_TSO_WITHOUT_CSUM 4
>> +#define VIRTIO_NET_TX_OFFLOAD_TSO_ECN_UNEXPECTED 5
>> 
>> 
>> - eh = mtod(m, struct ether_header *);
>> - eth_type = ntohs(eh->ether_type);
>> - if (eth_type == ETHERTYPE_VLAN) {
>> - /* BMV: We should handle nested VLAN tags too. */
>> - evh = mtod(m, struct ether_vlan_header *);
>> - eth_type = ntohs(evh->evl_proto);
>> - offset = sizeof(struct ether_vlan_header);
>> - } else
>> - offset = sizeof(struct ether_header);
>> -
>> - if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM)
>> - error = virtio_net_rx_csum_by_offset(m, eth_type, offset, hdr);
>> - else
>> - error = virtio_net_rx_csum_by_parse(m, eth_type, offset, hdr);
>> -
>> - return (error);
>> -}
>> +#define VIRTIO_NET_TX_MODERN_LE(modern, val) (modern ? htole16(val) : val)
>> 
>> 
>> +/*
>> + * BMV: This can go away once we finally have offsets in the mbuf header.
>> + */
>> static inline int
>> virtio_net_tx_offload_ctx(struct mbuf *m, int *etype, int *proto, int *start)
>> {
>> @@ -463,16 +457,15 @@ virtio_net_tx_offload_ctx(struct mbuf *m, int *etype, int *proto, int *start)
>> break;
>> #endif
>> default:
>> - /* Here we should increment the tx_csum_bad_ethtype counter. */
>> - return (EINVAL);
>> + return (VIRTIO_NET_TX_OFFLOAD_UNKNOWN_ETHTYPE);
>> }
>> 
>> 
>> return (0);
>> }
>> 
>> 
>> static inline int
>> -virtio_net_tx_offload_tso(if_t ifp, struct mbuf *m, int eth_type,
>> - int offset, bool allow_ecn, struct virtio_net_hdr *hdr)
>> +virtio_net_tx_offload_tso(struct ifnet *ifp, struct mbuf *m, int eth_type,
>> + int offset, struct virtio_net_hdr *hdr, bool tso_ecn, bool modern)
>> {
>> static struct timeval lastecn;
>> static int curecn;
>> @@ -484,79 +477,113 @@ virtio_net_tx_offload_tso(if_t ifp, struct mbuf *m, int eth_type,
>> } else
>> tcp = (struct tcphdr *)(m->m_data + offset);
>> 
>> 
>> - hdr->hdr_len = offset + (tcp->th_off << 2);
>> - hdr->gso_size = m->m_pkthdr.tso_segsz;
>> + /*
>> + * Set VirtIO header fields with the correct byte order.
>> + * In modern mode, this is little endian (LE).
>> + * In legacy mode, this is the endianness of the guest, which means a
>> + * FreeBSD guest can use its native endianness and a host must use the
>> + * guests endianness. However, since a FreeBSD host with bhyve runs only
>> + * on LE systems and supports only LE guests, no conversion is required.
>> + */
>> + hdr->hdr_len = VIRTIO_NET_TX_MODERN_LE(modern,
>> + offset + (tcp->th_off << 2));
>> + hdr->gso_size = VIRTIO_NET_TX_MODERN_LE(modern, m->m_pkthdr.tso_segsz);
>> hdr->gso_type = eth_type == ETHERTYPE_IP ? VIRTIO_NET_HDR_GSO_TCPV4 :
>> VIRTIO_NET_HDR_GSO_TCPV6;
>> 
>> 
>> - if (tcp_get_flags(tcp) & TH_CWR) {
>> + if (__predict_false(tcp_get_flags(tcp) & TH_CWR)) {
>> /*
>> - * Drop if VIRTIO_NET_F_HOST_ECN was not negotiated. In FreeBSD,
>> - * ECN support is not on a per-interface basis, but globally via
>> - * the net.inet.tcp.ecn.enable sysctl knob. The default is off.
>> + * Drop if VIRTIO_NET_F_HOST_ECN was not negotiated. In
>> + * FreeBSD, ECN support is not on a per-interface basis,
>> + * but globally via the net.inet.tcp.ecn.enable sysctl
>> + * knob. The default is off.
>> */
>> - if (!allow_ecn) {
>> + if (!tso_ecn) {
>> if (ppsratecheck(&lastecn, &curecn, 1))
>> if_printf(ifp,
>> "TSO with ECN not negotiated with host\n");
>> - return (ENOTSUP);
>> + return (VIRTIO_NET_TX_OFFLOAD_TSO_ECN_UNEXPECTED);
>> }
>> hdr->gso_type |= VIRTIO_NET_HDR_GSO_ECN;
>> }
>> 
>> 
>> - /* Here we should increment tx_tso counter. */
>> -
>> return (0);
>> }
>> 
>> 
>> -static inline struct mbuf *
>> -virtio_net_tx_offload(if_t ifp, struct mbuf *m, bool allow_ecn,
>> - struct virtio_net_hdr *hdr)
>> +/*
>> + * For a packet to be transmitted over the VirtIO channel, it checks the
>> + * CSUM_* flags in the mbuf and sets the appropriate flags in the VirtIO header.
>> + * In case of an error, it frees the mbuf and sets the pointer referenced by mp
>> + * to NULL.
>> + *
>> + * @param ifp ifnet struct of outgoing interface.
>> + * @param mp mbuf on which the CSUM_* flags needs to be checked.
>> + * @param hdr VirtIO header to be filled for the outgoing packet.
>> + * @param tso_ecn true if ECN has been negotiated between host and guest.
>> + * @param modern true if VirtIO modern mode is used.
>> + *
>> + * @return 0 on success, or one of the VIRTIO_NET_TX_OFFLOAD_* error codes.
>> + */
>> +static inline int
>> +virtio_net_tx_offload(struct ifnet *ifp, struct mbuf **mp,
>> + struct virtio_net_hdr *hdr, bool tso_ecn, bool modern)
>> {
>> int flags, etype, csum_start, proto, error;
>> + struct mbuf *m;
>> 
>> 
>> + m = *mp;
>> flags = m->m_pkthdr.csum_flags;
>> 
>> 
>> error = virtio_net_tx_offload_ctx(m, &etype, &proto, &csum_start);
>> - if (error)
>> + if (error != 0)
>> goto drop;
>> 
>> 
>> - if ((etype == ETHERTYPE_IP && (flags & (CSUM_TCP | CSUM_UDP))) ||
>> - (etype == ETHERTYPE_IPV6 &&
>> - (flags & (CSUM_TCP_IPV6 | CSUM_UDP_IPV6)))) {
>> + if (flags & (CSUM_TCP | CSUM_UDP | CSUM_TCP_IPV6 | CSUM_UDP_IPV6)) {
>> + /* Sanity check the parsed mbuf matches the offload flags. */
>> + if (__predict_false((flags & (CSUM_TCP | CSUM_UDP) &&
>> + etype != ETHERTYPE_IP) ||
>> + (flags & (CSUM_TCP_IPV6 | CSUM_UDP_IPV6) &&
>> + etype != ETHERTYPE_IPV6))) {
>> + error = VIRTIO_NET_TX_OFFLOAD_PROTO_MISMATCH;
>> + goto drop;
>> + }
>> +
>> /*
>> - * We could compare the IP protocol vs the CSUM_ flag too,
>> - * but that really should not be necessary.
>> + * Set VirtIO header fields with the correct byte order.
>> + * See comment in virtio_net_tx_offload_tso()
>> */
>> hdr->flags |= VIRTIO_NET_HDR_F_NEEDS_CSUM;
>> - hdr->csum_start = csum_start;
>> - hdr->csum_offset = m->m_pkthdr.csum_data;
>> - /* Here we should increment the tx_csum counter. */
>> + hdr->csum_start = VIRTIO_NET_TX_MODERN_LE(modern, csum_start);
>> + hdr->csum_offset = VIRTIO_NET_TX_MODERN_LE(modern,
>> + m->m_pkthdr.csum_data);
>> }
>> 
>> 
>> - if (flags & CSUM_TSO) {
>> + if (flags & (CSUM_IP_TSO | CSUM_IP6_TSO)) {
>> + /*
>> + * Sanity check the parsed mbuf IP protocol is TCP, and
>> + * VirtIO TSO reqires the checksum offloading above.
>> + */
>> if (__predict_false(proto != IPPROTO_TCP)) {
>> - /* Likely failed to correctly parse the mbuf.
>> - * Here we should increment the tx_tso_not_tcp
>> - * counter. */
>> + error = VIRTIO_NET_TX_OFFLOAD_TSO_NOT_TCP;
>> + goto drop;
>> + } else if (__predict_false((hdr->flags &
>> + VIRTIO_NET_HDR_F_NEEDS_CSUM) == 0)) {
>> + error = VIRTIO_NET_TX_OFFLOAD_TSO_WITHOUT_CSUM;
>> goto drop;
>> }
>> 
>> 
>> - KASSERT(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM,
>> - ("%s: mbuf %p TSO without checksum offload %#x",
>> - __func__, m, flags));
>> -
>> error = virtio_net_tx_offload_tso(ifp, m, etype, csum_start,
>> - allow_ecn, hdr);
>> - if (error)
>> + hdr, tso_ecn, modern);
>> + if (error != 0)
>> goto drop;
>> }
>> 
>> 
>> - return (m);
>> + return (error);
>> 
>> 
>> drop:
>> m_freem(m);
>> - return (NULL);
>> + *mp = NULL;
>> + return (error);
>> }
>> 
>> 
>> #endif /* _VIRTIO_NET_H */
>> diff --git a/sys/net/if_tuntap.c b/sys/net/if_tuntap.c
>> index a1ef2541ffba..069cf8b5f6c1 100644
>> --- a/sys/net/if_tuntap.c
>> +++ b/sys/net/if_tuntap.c
>> @@ -1882,7 +1882,16 @@ tunread(struct cdev *dev, struct uio *uio, int flag)
>> 
>> 
>> bzero(&vhdr, sizeof(vhdr));
>> if (m->m_pkthdr.csum_flags & TAP_ALL_OFFLOAD) {
>> - m = virtio_net_tx_offload(ifp, m, false, &vhdr.hdr);
>> + /*
>> + * Translate the CSUM_* flags in the mbuf to the
>> + * corresponding flags in the VirtIO header.
>> + *
>> + * Always indicate that ECN has not been negotiated
>> + * and VirtIO modern mode is not used because bhyve
>> + * does not do this.
>> + */
>> + virtio_net_tx_offload(ifp, &m, &vhdr.hdr, false,
>> + false);
>> }
>> 
>> 
>> TUNDEBUG(ifp, "txvhdr: f %u, gt %u, hl %u, "
>> @@ -1927,7 +1936,13 @@ tunwrite_l2(struct tuntap_softc *tp, struct mbuf *m,
>> }
>> 
>> 
>> if (vhdr != NULL) {
>> - if (virtio_net_rx_csum(m, &vhdr->hdr)) {
>> + /*
>> + * Translate the VirtIO header flags to the corresponding
>> + * CSUM_* flags in the mbuf.
>> + */
>> + if (((vhdr->hdr.flags & (VIRTIO_NET_HDR_F_NEEDS_CSUM |
>> + VIRTIO_NET_HDR_F_DATA_VALID)) != 0) &&
>> + (virtio_net_rx_csum(m, &vhdr->hdr) != 0)) {
>> m_freem(m);
>> return (0);
>> }
>> 
>> 
>> 
>> 
>> 
>> 
>