Subject: Small Ethernet packet padding
To: None <tech-net@NetBSD.org>
From: None <cube@cubidou.net>
List: tech-net
Date: 10/31/2004 12:18:29
--+QahgC5+KEYLbs62
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi,

Yesterday while working on some different issue, I could see
that gem(4) did not pad small Ethernet packets.  That means
you get data from the kernel on the wire at the trailing of
those packets.

When that issue was covered last year (I think it was last
year, at least), Manuel Bouyer made changes to the affected
drivers at that time.

However, I was suggested yesterday by Matt Thomas to take a
more general approach and handle that issue at the Ethernet
layer of the kernel.

With the help of Emmanuel Dreyfuss I've come up with the
attached patch.  It works fairly well, even though it had
some limited testing, but I'm still wondering if there are
any other (and possibly better) ways to manipulate the
mbuf chain.

From our tests, it appears that having space already inside
the mbuf is clearly not a given.  At least freshly after
the boot, there is about never enough space, except for
a certain length, because the size is 32-bits aligned.  I
guess we don't get space 'other than from alignment) when
the mbuf storage is external.  I haven't actually checked
that.

As my knowledge in the mbuf area is limited so there might
be ways of making the m_clrhdr/m_cat step simpler.  I'm
open to suggestions.

Also, I wonder what is the gain of making the padding at
the driver level.  I'd like some input there too.  If
there is no actual advantage to doing that inside the
hardware driver, I think it would be best to convert all
affected drivers to such a scheme as the low-level padding
adds quite some complexity to the driver.

Quentin Garnier.

--+QahgC5+KEYLbs62
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff

Index: dev/ic/gem.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/gem.c,v
retrieving revision 1.32
diff -u -r1.32 gem.c
--- dev/ic/gem.c	30 Oct 2004 18:08:36 -0000	1.32
+++ dev/ic/gem.c	31 Oct 2004 10:53:49 -0000
@@ -341,6 +341,9 @@
 	/* claim 802.1q capability */
 	sc->sc_ethercom.ec_capabilities |= ETHERCAP_VLAN_MTU;
 
+	/* claim the auto-padding un-capability */
+	sc->sc_ethercom.ec_capabilities |= ETHERCAP_NOPADDING;
+
 	/* Attach the interface. */
 	if_attach(ifp);
 	ether_ifattach(ifp, enaddr);
Index: net/if_ether.h
===================================================================
RCS file: /cvsroot/src/sys/net/if_ether.h,v
retrieving revision 1.34
diff -u -r1.34 if_ether.h
--- net/if_ether.h	7 Aug 2003 16:32:51 -0000	1.34
+++ net/if_ether.h	31 Oct 2004 10:53:49 -0000
@@ -166,6 +166,7 @@
 #define	ETHERCAP_VLAN_MTU	0x00000001	/* VLAN-compatible MTU */
 #define	ETHERCAP_VLAN_HWTAGGING	0x00000002	/* hardware VLAN tag support */
 #define	ETHERCAP_JUMBO_MTU	0x00000004	/* 9000 byte MTU supported */
+#define ETHERCAP_NOPADDING	0x00000008	/* doesn't autopad packets */
 
 #ifdef	_KERNEL
 extern u_int8_t etherbroadcastaddr[ETHER_ADDR_LEN];
Index: net/if_ethersubr.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_ethersubr.c,v
retrieving revision 1.116
diff -u -r1.116 if_ethersubr.c
--- net/if_ethersubr.c	24 Jun 2004 04:15:51 -0000	1.116
+++ net/if_ethersubr.c	31 Oct 2004 10:53:49 -0000
@@ -510,6 +510,27 @@
 		return (0);
 #endif
 
+	/*
+	 * Handle padding
+	 */
+	if (m->m_pkthdr.len < (ETHERMIN + sizeof(struct ether_header)) &&
+	    (((struct ethercom *)ifp)->ec_capabilities & ETHERCAP_NOPADDING)) {
+		size_t pad = ETHERMIN + sizeof(struct ether_header) -
+		    m->m_pkthdr.len;
+		if (m->m_next == NULL && M_TRAILINGSPACE(m) >= (ETHERMIN +
+		    sizeof(struct ether_header) - m->m_pkthdr.len)) {
+			caddr_t pkt = mtod(m, caddr_t) + m->m_pkthdr.len;
+			memset(pkt, 0, pad);
+			m->m_len += pad;
+		} else {
+			struct mbuf *n = m_getclr(M_DONTWAIT, MT_DATA);
+			if (n == NULL)
+				goto bad;
+			n->m_len = pad;
+			m_cat(m, n);
+		}
+	}
+
 #if NBRIDGE > 0
 	/*
 	 * Bridges require special output handling.

--+QahgC5+KEYLbs62--