From owner-dev-commits-src-all@freebsd.org Sun Apr 18 13:39:24 2021 Return-Path: Delivered-To: dev-commits-src-all@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 E596A5E9D2E; Sun, 18 Apr 2021 13:39:24 +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 4FNWJX627lz3RJn; Sun, 18 Apr 2021 13:39:24 +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 C24ED49CA; Sun, 18 Apr 2021 13:39:24 +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 13IDdO28028372; Sun, 18 Apr 2021 13:39:24 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 13IDdO3I028371; Sun, 18 Apr 2021 13:39:24 GMT (envelope-from git) Date: Sun, 18 Apr 2021 13:39:24 GMT Message-Id: <202104181339.13IDdO3I028371@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Vincenzo Maffione Subject: git: f4a54f4333c5 - main - netmap: use safer defaults for hwbuf_len MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: vmaffione X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: f4a54f4333c5c0f61d8ddddbb75e9c18af8c1aaa Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Apr 2021 13:39:25 -0000 The branch main has been updated by vmaffione: URL: https://cgit.FreeBSD.org/src/commit/?id=f4a54f4333c5c0f61d8ddddbb75e9c18af8c1aaa commit f4a54f4333c5c0f61d8ddddbb75e9c18af8c1aaa Author: Vincenzo Maffione AuthorDate: 2021-04-18 13:36:05 +0000 Commit: Vincenzo Maffione CommitDate: 2021-04-18 13:39:15 +0000 netmap: use safer defaults for hwbuf_len We must make sure that incoming packets will never overflow the netmap buffers, even when the user is using the offset feature. In the typical scenario, the netmap buffer is 2KiB and, with an MTU of 1500, there are ~500 bytes available for user offsets. Unfortunately, some NICs accept incoming packets even when they are larger then the MTU. This means that the only way to stop DMA from overflowing the netmap buffers, when offsets are allowed, is to choose a hardware buffer length which is smaller than the netmap buffer length. For most NICs and for 2KiB netmap buffers, this means 1024 bytes, which is unconveniently small. The current code will select the small hardware buf size even when offsets are not in use. The main purpose of this change is to fix this bug by returning to the normal behavior for the no-offsets case. At the same time, the patch pushes the handling of the offset case to the lower level driver code, so that it can be made NIC-specific (in future patches). --- sys/dev/netmap/netmap.c | 46 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/sys/dev/netmap/netmap.c b/sys/dev/netmap/netmap.c index 4835c47d2785..9f1edb246cae 100644 --- a/sys/dev/netmap/netmap.c +++ b/sys/dev/netmap/netmap.c @@ -2381,6 +2381,12 @@ out: } + +/* set the hardware buffer length in each one of the newly opened rings + * (hwbuf_len field in the kring struct). The purpose it to select + * the maximum supported input buffer lenght that will not cause writes + * outside of the available space, even when offsets are in use. + */ static int netmap_compute_buf_len(struct netmap_priv_d *priv) { @@ -2390,32 +2396,44 @@ netmap_compute_buf_len(struct netmap_priv_d *priv) int error = 0; unsigned mtu = 0; struct netmap_adapter *na = priv->np_na; - uint64_t target, maxframe; - - if (na->ifp != NULL) - mtu = nm_os_ifnet_mtu(na->ifp); + uint64_t target; foreach_selected_ring(priv, t, i, kring) { - + /* rings that are already active have their hwbuf_len + * already set and we cannot change it. + */ if (kring->users > 1) continue; + /* For netmap buffers which are not shared among several ring + * slots (the normal case), the available space is the buf size + * minus the max offset declared by the user at open time. If + * the user plans to have several slots pointing to different + * offsets into the same large buffer, she must also declare a + * "minimum gap" between two such consecutive offsets. In this + * case the user-declared 'offset_gap' is taken as the + * available space and offset_max is ignored. + */ + + /* start with the normal case (unshared buffers) */ target = NETMAP_BUF_SIZE(kring->na) - kring->offset_max; + /* if offset_gap is zero, the user does not intend to use + * shared buffers. In this case the minimum gap between + * two consective offsets into the same buffer can be + * assumed to be equal to the buffer size. In this way + * offset_gap always contains the available space ignoring + * offset_max. This may be used by drivers of NICs that + * are guaranteed to never write more than MTU bytes, even + * if the input buffer is larger: if the MTU is less + * than the target they can set hwbuf_len to offset_gap. + */ if (!kring->offset_gap) kring->offset_gap = NETMAP_BUF_SIZE(kring->na); + if (kring->offset_gap < target) target = kring->offset_gap; - - if (mtu) { - maxframe = mtu + ETH_HLEN + - ETH_FCS_LEN + VLAN_HLEN; - if (maxframe < target) { - target = maxframe; - } - } - error = kring->nm_bufcfg(kring, target); if (error) goto out;