git: f4a54f4333c5 - main - netmap: use safer defaults for hwbuf_len

[Vincenzo Maffione]

The branch main has been updated by vmaffione:

URL: https://cgit.FreeBSD.org/src/commit/?id=f4a54f4333c5c0f61d8ddddbb75e9c18af8c1aaa

commit f4a54f4333c5c0f61d8ddddbb75e9c18af8c1aaa
Author:     Vincenzo Maffione <vmaffione at FreeBSD.org>
AuthorDate: 2021-04-18 13:36:05 +0000
Commit:     Vincenzo Maffione <vmaffione at 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;