Date: Sun, 18 Apr 2021 13:39:24 GMT From: Vincenzo Maffione <vmaffione@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: f4a54f4333c5 - main - netmap: use safer defaults for hwbuf_len Message-ID: <202104181339.13IDdO3I028371@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by vmaffione: URL: https://cgit.FreeBSD.org/src/commit/?id=f4a54f4333c5c0f61d8ddddbb75e9c18af8c1aaa commit f4a54f4333c5c0f61d8ddddbb75e9c18af8c1aaa Author: Vincenzo Maffione <vmaffione@FreeBSD.org> AuthorDate: 2021-04-18 13:36:05 +0000 Commit: Vincenzo Maffione <vmaffione@FreeBSD.org> 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;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202104181339.13IDdO3I028371>