From owner-dev-commits-src-main@freebsd.org Tue Jan 19 05:08:39 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id C84114E54C6; Tue, 19 Jan 2021 05:08:39 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (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 "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DKcBH1Mf8z4XRg; Tue, 19 Jan 2021 05:08:39 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (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 did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 9632F20B41; Tue, 19 Jan 2021 05:08:34 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 10J58YkL086180; Tue, 19 Jan 2021 05:08:34 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 10J58YIQ086179; Tue, 19 Jan 2021 05:08:34 GMT (envelope-from git) Date: Tue, 19 Jan 2021 05:08:34 GMT Message-Id: <202101190508.10J58YIQ086179@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Bryan Venteicher Subject: git: 475a60aec7e3 - main - if_vtnet: Misc Tx path cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: bryanv X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 475a60aec7e3126f7a8b93a9f0fdf23360804080 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Jan 2021 05:08:41 -0000 The branch main has been updated by bryanv: URL: https://cgit.FreeBSD.org/src/commit/?id=475a60aec7e3126f7a8b93a9f0fdf23360804080 commit 475a60aec7e3126f7a8b93a9f0fdf23360804080 Author: Bryan Venteicher AuthorDate: 2021-01-19 04:55:26 +0000 Commit: Bryan Venteicher CommitDate: 2021-01-19 04:55:26 +0000 if_vtnet: Misc Tx path cleanup - Add and fix a few error path counters - Improve sysctl descriptions - Use flags consistently to determine IPv4 vs IPv6 Reviewed by: grehan (mentor) Differential Revision: https://reviews.freebsd.org/D27926 --- sys/dev/virtio/network/if_vtnet.c | 65 +++++++++++++++++++++--------------- sys/dev/virtio/network/if_vtnetvar.h | 5 +-- 2 files changed, 41 insertions(+), 29 deletions(-) diff --git a/sys/dev/virtio/network/if_vtnet.c b/sys/dev/virtio/network/if_vtnet.c index 3da945baab63..1ed149c3d1eb 100644 --- a/sys/dev/virtio/network/if_vtnet.c +++ b/sys/dev/virtio/network/if_vtnet.c @@ -2375,7 +2375,7 @@ vtnet_txq_offload_ctx(struct vtnet_txq *txq, struct mbuf *m, int *etype, break; #endif default: - sc->vtnet_stats.tx_csum_bad_ethtype++; + sc->vtnet_stats.tx_csum_unknown_ethtype++; return (EINVAL); } @@ -2383,7 +2383,7 @@ vtnet_txq_offload_ctx(struct vtnet_txq *txq, struct mbuf *m, int *etype, } static int -vtnet_txq_offload_tso(struct vtnet_txq *txq, struct mbuf *m, int eth_type, +vtnet_txq_offload_tso(struct vtnet_txq *txq, struct mbuf *m, int flags, int offset, struct virtio_net_hdr *hdr) { static struct timeval lastecn; @@ -2401,14 +2401,15 @@ vtnet_txq_offload_tso(struct vtnet_txq *txq, struct mbuf *m, int eth_type, 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; + hdr->gso_type = (flags & CSUM_IP_TSO) ? + VIRTIO_NET_HDR_GSO_TCPV4 : VIRTIO_NET_HDR_GSO_TCPV6; - if (tcp->th_flags & TH_CWR) { + if (__predict_false(tcp->th_flags & 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 ((sc->vtnet_flags & VTNET_FLAG_TSO_ECN) == 0) { if (ppsratecheck(&lastecn, &curecn, 1)) @@ -2438,30 +2439,36 @@ vtnet_txq_offload(struct vtnet_txq *txq, struct mbuf *m, if (error) goto drop; - if ((etype == ETHERTYPE_IP && flags & VTNET_CSUM_OFFLOAD) || - (etype == ETHERTYPE_IPV6 && flags & VTNET_CSUM_OFFLOAD_IPV6)) { - /* - * We could compare the IP protocol vs the CSUM_ flag too, - * but that really should not be necessary. - */ + 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_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. */ 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; } - KASSERT(hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM, - ("%s: mbuf %p TSO without checksum offload %#x", - __func__, m, flags)); - - error = vtnet_txq_offload_tso(txq, m, etype, csum_start, hdr); + error = vtnet_txq_offload_tso(txq, m, flags, csum_start, hdr); if (error) goto drop; } @@ -4097,7 +4104,7 @@ vtnet_setup_txq_sysctl(struct sysctl_ctx_list *ctx, SYSCTL_ADD_UQUAD(ctx, list, OID_AUTO, "csum", CTLFLAG_RD, &stats->vtxs_csum, "Transmit checksum offloaded"); SYSCTL_ADD_UQUAD(ctx, list, OID_AUTO, "tso", CTLFLAG_RD, - &stats->vtxs_tso, "Transmit segmentation offloaded"); + &stats->vtxs_tso, "Transmit TCP segmentation offloaded"); SYSCTL_ADD_UQUAD(ctx, list, OID_AUTO, "rescheduled", CTLFLAG_RD, &stats->vtxs_rescheduled, "Transmit interrupt handler rescheduled"); @@ -4177,16 +4184,20 @@ vtnet_setup_stat_sysctl(struct sysctl_ctx_list *ctx, CTLFLAG_RD, &stats->rx_task_rescheduled, "Times the receive interrupt task rescheduled itself"); - SYSCTL_ADD_UQUAD(ctx, child, OID_AUTO, "tx_csum_bad_ethtype", - CTLFLAG_RD, &stats->tx_csum_bad_ethtype, + SYSCTL_ADD_UQUAD(ctx, child, OID_AUTO, "tx_csum_unknown_ethtype", + CTLFLAG_RD, &stats->tx_csum_unknown_ethtype, "Aborted transmit of checksum offloaded buffer with unknown " "Ethernet type"); - SYSCTL_ADD_UQUAD(ctx, child, OID_AUTO, "tx_tso_bad_ethtype", - CTLFLAG_RD, &stats->tx_tso_bad_ethtype, - "Aborted transmit of TSO buffer with unknown Ethernet type"); + SYSCTL_ADD_UQUAD(ctx, child, OID_AUTO, "tx_csum_proto_mismatch", + CTLFLAG_RD, &stats->tx_csum_proto_mismatch, + "Aborted transmit of checksum offloaded buffer because mismatched " + "protocols"); SYSCTL_ADD_UQUAD(ctx, child, OID_AUTO, "tx_tso_not_tcp", CTLFLAG_RD, &stats->tx_tso_not_tcp, "Aborted transmit of TSO buffer with non TCP protocol"); + SYSCTL_ADD_UQUAD(ctx, child, OID_AUTO, "tx_tso_without_csum", + CTLFLAG_RD, &stats->tx_tso_without_csum, + "Aborted transmit of TSO buffer without TCP checksum offload"); SYSCTL_ADD_UQUAD(ctx, child, OID_AUTO, "tx_defragged", CTLFLAG_RD, &stats->tx_defragged, "Transmit mbufs defragged"); diff --git a/sys/dev/virtio/network/if_vtnetvar.h b/sys/dev/virtio/network/if_vtnetvar.h index c9403c0dbb0e..f928b9bd9b38 100644 --- a/sys/dev/virtio/network/if_vtnetvar.h +++ b/sys/dev/virtio/network/if_vtnetvar.h @@ -43,9 +43,10 @@ struct vtnet_statistics { uint64_t rx_csum_bad_ipproto; uint64_t rx_csum_bad_offset; uint64_t rx_csum_bad_proto; - uint64_t tx_csum_bad_ethtype; - uint64_t tx_tso_bad_ethtype; + uint64_t tx_csum_unknown_ethtype; + uint64_t tx_csum_proto_mismatch; uint64_t tx_tso_not_tcp; + uint64_t tx_tso_without_csum; uint64_t tx_defragged; uint64_t tx_defrag_failed;