From nobody Fri Aug 21 14:04:39 2026 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4hRMWG1sZYz6qRF0; Fri, 21 Aug 2026 14:04:42 +0000 (UTC) (envelope-from rpokala@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "YR2" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 4hRMWG1F1lz3Ws0; Fri, 21 Aug 2026 14:04:42 +0000 (UTC) (envelope-from rpokala@freebsd.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1787321082; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8y95UJjOCgImhSLIg27hVMiKGc/v9RqpKESlLBurEh4=; b=BsD7oIGrvy0JrRpVQKdf7ghLzhoGgmK9ZsVfPWVrXr+ioN4ux+MFZx/ZBybqO343T+WJc/ ktsyOsb1Az5CUjue6g7dzyVRe+8RD5BN1/0RK970Kejlce7cXDbDk2MHqI4hTNKJW1H5WM SpF1A9C+28oB4rk7HPBFe/L3d+F9FyEKdQ48N4CJTKsq3ii7wz//M/AKlCY6PZj+kqfsLt +4Vm8bPoV8p3+iLoXgThgqJQcDGvBub0rOIXYABXI1w59pEHu3jq1yK9dIQ5s1Lxp3o4RI +M/ZX1cjlggfQ2OFC52AVLq/crxDeMabVEvJQP3RlDorkV83pSEC/jD0dmUm1w== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1787321082; a=rsa-sha256; cv=none; b=WMEEu+zVcH4FLgTTdwGzyJ2qWOW6hmKJI7QYCVE+0ZgDfGN1dsrpH9L7YP6XeHlWZuz1Ri ZDVctARGblM4B8HmSqfUlg0MaUD5v3XoiWCa3CZPf2hmaI25psBtuZapzHTvFzj2UxPiVK 4PXSxNYBRt78QrZak02ZI3CGtNyU5q4IyV8tK9/4Djwnkq/zk+/vRB6WnXZDq6AhvshTPw YBIxk9W8ny0OQWb0vmVEw+sEQgDj5NOPLqKByR33vhSBFY1wrqjl9jlf9jz43ANeI90jWz Tz+S4DGk4EAQRP2Qi5vIEy1HdtHsjhRLaTVo260wP03RCkJga8c6EfsDg7oJ2A== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1787321082; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=8y95UJjOCgImhSLIg27hVMiKGc/v9RqpKESlLBurEh4=; b=ZqX+P0HF6KVHtU2K/FxNHEYx2NKk6CDO7bw1SPyzxlB6zbsPdAJbCMYa9aa5yTMmeO/6r0 MfUvEOs5nvmM8TCwSWeDu7RpiB2kd5bvMcPY/XJw/O1gjYKWHy/WMPa7ShmdJQqaiW7MMC KUfXQBKY4ATxvDj/gDz6iZMIY5rFJaEWJPkLXTb4GKGW+vUTAdngnYm9OFSEQNlxO/y/la xgujAYTZJHPDzSqWd5+0nnEwhWOhIC7J8Wdo/+D6d8jLEmtFxXycSZ7vGMAx2KMDEl4CXK 7288WFzPzRYhcHOfzPU3rV1xEyddETQbGr9JqIrbI9k+NI48yIMwu3FN9VG9iw== Received: from [192.168.1.10] (c-73-40-187-156.hsd1.pa.comcast.net [73.40.187.156]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) (Authenticated sender: rpokala) by smtp.freebsd.org (Postfix) with ESMTPSA id 4hRMWF44kyzs4h; Fri, 21 Aug 2026 14:04:41 +0000 (UTC) (envelope-from rpokala@freebsd.org) User-Agent: Microsoft-MacOutlook/16.112.26081720 Date: Fri, 21 Aug 2026 10:04:39 -0400 Subject: Re: 44cddaa99dee - main - vtnet: move offload functions to virtio_net.h to share them From: Ravi Pokala To: Michael Tuexen , Timo =?UTF-8?B?VsO2bGtlcg==?= , , , Message-ID: <292C4DF8-EF30-4FAE-AED5-5097B31F87A4@panasas.com> Thread-Topic: 44cddaa99dee - main - vtnet: move offload functions to virtio_net.h to share them References: <6a845614.42262.1972156d@gitrepo.freebsd.org> In-Reply-To: <6a845614.42262.1972156d@gitrepo.freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list Mime-version: 1.0 Content-type: text/plain; charset="UTF-8" Content-transfer-encoding: quoted-printable Hi Michael, Timo, | @@ -1927,7 +1936,13 @@ tunwrite_l2(struct tuntap_softc *tp, struct mbuf *= m, | } |=20 | if (vhdr !=3D 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)) !=3D 0) && | + (virtio_net_rx_csum(m, &vhdr->hdr) !=3D 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 de= clarations [-Werror,-Wimplicit-function-declaration] | 1945 | (virtio_net_rx_csum(m, &vhdr->hdr) !=3D 0)) { | | ^ | 1 error generated. | --- if_tuntap.o --- | *** [if_tuntap.o] Error code 1 Thanks, Ravi (rpokala@) =EF=BB=BF-----Original Message----- From: > on behalf of Michael Tuexen > Date: Tuesday, August 18, 2026 at 08:54 To: >, >, > Cc: Timo V=C3=B6lker > 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=3D44cddaa99dee0a634cf2713f71e799= eb41397355 commit 44cddaa99dee0a634cf2713f71e799eb41397355 Author: Timo V=C3=B6lker > AuthorDate: 2026-08-18 12:49:40 +0000 Commit: Michael Tuexen > 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 --- 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, u= nsigned int budget, * two 8-bytes-wide writes. */ memset(nmbuf, 0, PTNET_HDR_SIZE); if (mhead->m_pkthdr.csum_flags & PTNET_ALL_OFFLOAD) { - mhead =3D 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)) !=3D 0) && + (virtio_net_rx_csum(mhead, vh) !=3D 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 vtne= t_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 isip= v6, - 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 =3D=3D IPPROTO_TCP || protocol =3D=3D 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 |=3D (isipv6 ? CSUM_TCP_IPV6 : CSUM_TCP); - break; - case IPPROTO_UDP: - m->m_pkthdr.csum_flags |=3D (isipv6 ? CSUM_UDP_IPV6 : CSUM_UDP); - break; - } - m->m_pkthdr.csum_data =3D hdr->csum_offset; -} - -static void -vtnet_rxq_csum_data_valid(struct vtnet_rxq *rxq, struct mbuf *m, int proto= col) -{ - KASSERT(protocol =3D=3D IPPROTO_TCP || protocol =3D=3D IPPROTO_UDP, - ("%s: unsupported IP protocol %d", __func__, protocol)); - - m->m_pkthdr.csum_flags |=3D CSUM_DATA_VALID | CSUM_PSEUDO_HDR; - m->m_pkthdr.csum_data =3D 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 =3D mtod(m, const struct ether_header *); - etype =3D ntohs(eh->ether_type); - if (etype =3D=3D ETHERTYPE_VLAN) { - /* TODO BMV: Handle QinQ. */ - const struct ether_vlan_header *evh =3D - mtod(m, const struct ether_vlan_header *); - etype =3D ntohs(evh->evl_proto); - hoff =3D sizeof(struct ether_vlan_header); - } else - hoff =3D sizeof(struct ether_header); - - sc =3D 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 =3D (struct ip *)(m->m_data + hoff); - protocol =3D ip->ip_p; - } - isipv6 =3D 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 =3D 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) =3D=3D 0) + int ret; + + /* + * Translate the VirtIO header flags to the corresponding + * CSUM_* flags in the mbuf. + */ + ret =3D virtio_net_rx_csum(m, hdr); + if (ret =3D=3D 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 =3D txq->vtntx_sc; - - evh =3D mtod(m, struct ether_vlan_header *); - if (evh->evl_encap_proto =3D=3D htons(ETHERTYPE_VLAN)) { - /* BMV: We should handle nested VLAN tags too. */ - *etype =3D ntohs(evh->evl_proto); -#if defined(INET) || defined(INET6) - offset =3D sizeof(struct ether_vlan_header); -#endif - } else { - *etype =3D ntohs(evh->evl_encap_proto); -#if defined(INET) || defined(INET6) - offset =3D 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 =3D &iphdr; - } else - ip =3D (struct ip *)(m->m_data + offset); - *proto =3D ip->ip_p; - *start =3D offset + (ip->ip_hl << 2); - break; - } -#endif -#if defined(INET6) - case ETHERTYPE_IPV6: - *proto =3D -1; - *start =3D 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 =3D txq->vtntx_sc; - - if (__predict_false(m->m_len < offset + sizeof(struct tcphdr))) { - m_copydata(m, offset, sizeof(struct tcphdr), (caddr_t) &tcphdr); - tcp =3D &tcphdr; - } else - tcp =3D (struct tcphdr *)(m->m_data + offset); - - hdr->hdr_len =3D vtnet_gtoh16(sc, offset + (tcp->th_off << 2)); - hdr->gso_size =3D vtnet_gtoh16(sc, m->m_pkthdr.tso_segsz); - hdr->gso_type =3D eth_type =3D=3D 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) =3D=3D 0) { - if (ppsratecheck(&lastecn, &curecn, 1)) - if_printf(sc->vtnet_ifp, - "TSO with ECN not negotiated with host\n"); - return (ENOTSUP); - } - hdr->gso_type |=3D 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 =3D txq->vtntx_sc; - flags =3D m->m_pkthdr.csum_flags; - - error =3D 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 !=3D ETHERTYPE_IP) || (flags & VTNET_CSUM_OFFLOAD_IPV6 - && etype !=3D ETHERTYPE_IPV6))) { - sc->vtnet_stats.tx_csum_proto_mismatch++; - goto drop; - } - - hdr->flags |=3D VIRTIO_NET_HDR_F_NEEDS_CSUM; - hdr->csum_start =3D vtnet_gtoh16(sc, csum_start); - hdr->csum_offset =3D 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 !=3D IPPROTO_TCP)) { - sc->vtnet_stats.tx_tso_not_tcp++; - goto drop; - } else if (__predict_false((hdr->flags & - VIRTIO_NET_HDR_F_NEEDS_CSUM) =3D=3D 0)) { - sc->vtnet_stats.tx_tso_without_csum++; - goto drop; - } - - error =3D 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 =3D vtnet_txq_offload(txq, m, hdr); + int ret; + + /* + * Translate the CSUM_* flags in the mbuf to the corresponding + * flags in the VirtIO header. + */ + ret =3D 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 =3D m) =3D=3D NULL) { error =3D 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 =3D vtnet_txq_enqueue_buf(txq, m_head, txhdr); diff --git a/sys/dev/virtio/network/virtio_net.h b/sys/dev/virtio/network/v= irtio_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 + /* 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/ partia= l 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_sta= rt, - struct virtio_net_hdr *hdr) -{ #if defined(INET) || defined(INET6) - int offset =3D 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 =3D=3D IPPROTO_TCP || protocol =3D=3D 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 |=3D CSUM_DATA_VALID | CSUM_PSEUDO_HDR; - m->m_pkthdr.csum_data =3D 0xFFFF; + switch (protocol) { + case IPPROTO_TCP: + m->m_pkthdr.csum_flags |=3D (isipv6 ? CSUM_TCP_IPV6 : CSUM_TCP); + break; + case IPPROTO_UDP: + m->m_pkthdr.csum_flags |=3D (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 =3D hdr->csum_offset; +} - return (0); +static inline void +virtio_net_rx_csum_data_valid(struct mbuf *m, int protocol) +{ + KASSERT(protocol =3D=3D IPPROTO_TCP || protocol =3D=3D IPPROTO_UDP, + ("%s: unsupported IP protocol %d", __func__, protocol)); + + m->m_pkthdr.csum_flags |=3D CSUM_DATA_VALID | CSUM_PSEUDO_HDR; + m->m_pkthdr.csum_data =3D 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. T= he + * VirtIO header gives the offset of the checksum, which is all Linux need= s, but + * this is not how FreeBSD does things. We are forced to peek inside the p= acket + * 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 checke= d + * 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_star= t, - 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 =3D mtod(m, const struct ether_header *); + etype =3D ntohs(eh->ether_type); + if (etype =3D=3D ETHERTYPE_VLAN) { + /* TODO BMV: Handle QinQ. */ + const struct ether_vlan_header *evh =3D + mtod(m, const struct ether_vlan_header *); + etype =3D ntohs(evh->evl_proto); + hoff =3D sizeof(struct ether_vlan_header); + } else + hoff =3D 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 =3D (struct ip *)(m->m_data + ip_start); - proto =3D ip->ip_p; - offset =3D 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 =3D (struct ip *)(m->m_data + hoff); + protocol =3D ip->ip_p; + } + isipv6 =3D false; break; - } #endif #if defined(INET6) case ETHERTYPE_IPV6: - if (__predict_false(m->m_len < ip_start + - sizeof(struct ip6_hdr))) - return (1); - offset =3D 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 =3D 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 |=3D CSUM_DATA_VALID | CSUM_PSEUDO_HDR; - m->m_pkthdr.csum_data =3D 0xFFFF; - break; case IPPROTO_UDP: - if (__predict_false(m->m_len < offset + sizeof(struct udphdr))) - return (1); - m->m_pkthdr.csum_flags |=3D CSUM_DATA_VALID | CSUM_PSEUDO_HDR; - m->m_pkthdr.csum_data =3D 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=3D%#x proto=3D%d csum_start=3D%d " - "csum_offset=3D%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)) =3D=3D 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 =3D mtod(m, struct ether_header *); - eth_type =3D ntohs(eh->ether_type); - if (eth_type =3D=3D ETHERTYPE_VLAN) { - /* BMV: We should handle nested VLAN tags too. */ - evh =3D mtod(m, struct ether_vlan_header *); - eth_type =3D ntohs(evh->evl_proto); - offset =3D sizeof(struct ether_vlan_header); - } else - offset =3D sizeof(struct ether_header); - - if (hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) - error =3D virtio_net_rx_csum_by_offset(m, eth_type, offset, hdr); - else - error =3D 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 *star= t) { @@ -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 =3D (struct tcphdr *)(m->m_data + offset); - hdr->hdr_len =3D offset + (tcp->th_off << 2); - hdr->gso_size =3D 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 =3D VIRTIO_NET_TX_MODERN_LE(modern, + offset + (tcp->th_off << 2)); + hdr->gso_size =3D VIRTIO_NET_TX_MODERN_LE(modern, m->m_pkthdr.tso_segsz); hdr->gso_type =3D eth_type =3D=3D 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 |=3D 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 h= eader. + * 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 =3D *mp; flags =3D m->m_pkthdr.csum_flags; error =3D virtio_net_tx_offload_ctx(m, &etype, &proto, &csum_start); - if (error) + if (error !=3D 0) goto drop; - if ((etype =3D=3D ETHERTYPE_IP && (flags & (CSUM_TCP | CSUM_UDP))) || - (etype =3D=3D 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 !=3D ETHERTYPE_IP) || + (flags & (CSUM_TCP_IPV6 | CSUM_UDP_IPV6) && + etype !=3D ETHERTYPE_IPV6))) { + error =3D 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 |=3D VIRTIO_NET_HDR_F_NEEDS_CSUM; - hdr->csum_start =3D csum_start; - hdr->csum_offset =3D m->m_pkthdr.csum_data; - /* Here we should increment the tx_csum counter. */ + hdr->csum_start =3D VIRTIO_NET_TX_MODERN_LE(modern, csum_start); + hdr->csum_offset =3D 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 !=3D IPPROTO_TCP)) { - /* Likely failed to correctly parse the mbuf. - * Here we should increment the tx_tso_not_tcp - * counter. */ + error =3D VIRTIO_NET_TX_OFFLOAD_TSO_NOT_TCP; + goto drop; + } else if (__predict_false((hdr->flags & + VIRTIO_NET_HDR_F_NEEDS_CSUM) =3D=3D 0)) { + error =3D 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 =3D virtio_net_tx_offload_tso(ifp, m, etype, csum_start, - allow_ecn, hdr); - if (error) + hdr, tso_ecn, modern); + if (error !=3D 0) goto drop; } - return (m); + return (error); drop: m_freem(m); - return (NULL); + *mp =3D 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 =3D 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 !=3D 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)) !=3D 0) && + (virtio_net_rx_csum(m, &vhdr->hdr) !=3D 0)) { m_freem(m); return (0); }