alpha and em mtu

[Sten Spans]

On Tue, 23 Nov 2004, Sten Spans wrote:

>> doesn't seem to print anything, but ...
>> 
>> if_em.c
>>   2442
>>   2443         /*if (ifp->if_mtu <= ETHERMTU) { */
>>   2444                 m_adj(mp, ETHER_ALIGN);
>>   2445         /*} */
>>   2446
>> 
>> does seem to fix the crash, also trashes the performance,
>> but that's another matter. It looks like mbuf alignment is
>> needed, if_bge seems to provide reasonable examples.
>
> And looking at netbsd/openbsd clarifies the whole issue,
>
> #ifdef __STRICT_ALIGNMENT
> 			/*
> 			 * The ethernet payload is not 32-bit aligned when
> 			 * Jumbo packets are enabled, so on architectures 
> with
> 			 * strict alignment we need to shift the entire 
> packet
> 			 * ETHER_ALIGN bytes. Ugh.
> 			 */
>
>
> This diff probably should be merged.
> http://www.openbsd.org/cgi-bin/cvsweb/src/sys/dev/pci/if_em.c.diff?r1=1.22&r2=1.23
>
> Although I don't know wether freebsd has the STRICT_ALIGNMENT define.

This is an initial patch based on the openbsd code,
which solves the if_em issue, and seems to give ok performance
( note to self: turn off debugging when testing network performance ).

Comments welcome.

--- if_em.c.orig	Tue Nov 23 03:03:23 2004
+++ if_em.c	Tue Nov 23 03:20:15 2004
@@ -2784,8 +2784,57 @@
  			/* Assign correct length to the current fragment */
  			mp->m_len = len;

+#ifndef __i386__
+			/*
+			 * The ethernet payload is not 32-bit aligned when
+			 * Jumbo packets are enabled, so on architectures with
+			 * strict alignment we need to shift the entire packet
+			 * ETHER_ALIGN bytes. Ugh.
+			 */
+			if (ifp->if_mtu > ETHERMTU) {
+				unsigned char tmp_align_buf[ETHER_ALIGN];
+				int tmp_align_buf_len = 0;
+
+				if (prev_len_adj > adapter->align_buf_len)
+					prev_len_adj -= adapter->align_buf_len;
+				else
+					prev_len_adj = 0;
+
+				if (mp->m_len > MCLBYTES - ETHER_ALIGN) {
+					bcopy(mp->m_data +
+					    (MCLBYTES - ETHER_ALIGN),
+					    &tmp_align_buf,
+					    ETHER_ALIGN);
+					tmp_align_buf_len = mp->m_len -
+					    (MCLBYTES - ETHER_ALIGN);
+					mp->m_len -= ETHER_ALIGN;
+				} 
+
+				if (mp->m_len) {
+					bcopy(mp->m_data,
+					    mp->m_data + ETHER_ALIGN,
+					    mp->m_len);
+					if (!adapter->align_buf_len)
+						mp->m_data += ETHER_ALIGN;
+				}
+
+				if (adapter->align_buf_len) {
+					mp->m_len += adapter->align_buf_len;
+					bcopy(&adapter->align_buf,
+					    mp->m_data,
+					    adapter->align_buf_len);
+				}
+
+				if (tmp_align_buf_len) 
+					bcopy(&tmp_align_buf,
+					    &adapter->align_buf,
+					    tmp_align_buf_len);
+				adapter->align_buf_len = tmp_align_buf_len;
+			}
+#endif /* __386__ */
+
  			if (adapter->fmp == NULL) {
-				mp->m_pkthdr.len = len;
+				mp->m_pkthdr.len = mp->m_len;
  				adapter->fmp = mp;	 /* Store the first mbuf */
  				adapter->lmp = mp;
  			} else {
@@ -2801,7 +2850,7 @@
  				}
  				adapter->lmp->m_next = mp;
  				adapter->lmp = adapter->lmp->m_next;
-				adapter->fmp->m_pkthdr.len += len;
+				adapter->fmp->m_pkthdr.len += mp->m_len;
  			}

                          if (eop) {
--- if_em.h.orig	Wed Nov 10 03:18:52 2004
+++ if_em.h	Tue Nov 23 03:33:01 2004
@@ -347,6 +347,12 @@
  	u_int8_t        unit;
  	struct mtx	mtx;

+#ifndef __i386__
+	/* Used for carrying forward alignment adjustments */
+	unsigned char	align_buf[ETHER_ALIGN];	/* tail of unaligned packet */
+	u_int8_t	align_buf_len;		/* bytes in tail */
+#endif /* __i386__ */
+
  	/* Info about the board itself */
  	u_int32_t       part_num;
  	u_int8_t        link_active;

-- 
Sten Spans

"There is a crack in everything, that's how the light gets in."
Leonard Cohen - Anthem