alpha and em mtu

[Sten Spans]

On Tue, 23 Nov 2004, Sten Spans wrote:

> On Mon, 22 Nov 2004, John Baldwin wrote:
>
>> On Monday 22 November 2004 04:15 pm, Sten Spans wrote:
>>> 
>>> db> trace
>>> tcp_input() at tcp_input+0x3a4
>>> ip_input() at ip_input+0x9fc
>>> netisr_processqueue() at netisr_processqueue+0xac
>>> swi_net() at swi_net+0xf0
>>> ithread_loop() at ithread_loop+0x1d4
>>> fork_exit() at fork_exit+0x100
>>> exception_return() at exception_return
>>> --- root of call graph ---
>>> 
>>> (gdb) l *tcp_input+0x3a4
>>> 0xfffffc00004cd054 is in tcp_input (/usr/src/sys/netinet/tcp_input.c:554).
>>> 549
>>> 550             /*
>>> 551              * Check that TCP offset makes sense,
>>> 552              * pull out TCP options and adjust length.
>>> XXX
>>> 553              */
>>> 554             off = th->th_off << 2;
>>> 555             if (off < sizeof (struct tcphdr) || off > tlen) {
>>> 556                     tcpstat.tcps_rcvbadoff++;
>>> 557                     goto drop;
>>> 558             }
>>> (gdb) l *ip_input+0x9fc
>>> 0xfffffc00004c355c is in ip_input (/usr/src/sys/netinet/ip_input.c:739).
>>> 734             /*
>>> 735              * Switch out to protocol's input routine.
>>> 736              */
>>> 737             ipstat.ips_delivered++;
>>> 738
>>> 739             (*inetsw[ip_protox[ip->ip_p]].pr_input)(m, hlen);
>>> 740             return;
>>> 741     bad:
>>> 742             m_freem(m);
>>> 743     }
>>> (gdb) l *netisr_processqueue+0xac
>>> 0xfffffc00004ad45c is in netisr_processqueue
>>> (/usr/src/sys/net/netisr.c:233).
>>> 228
>>> 229             for (;;) {
>>> 230                     IF_DEQUEUE(ni->ni_queue, m);
>>> 231                     if (m == NULL)
>>> 232                             break;
>>> 233                     ni->ni_handler(m);
>>> 234             }
>>> 235     }
>> 
>> Hmm, so can you check here to see if the 'm' pointer in this routine is
>> misaligned?  If so, then this may be a driver bug.
>> 
>
> --- netisr.c.orig       Tue Nov 23 00:21:38 2004
> +++ netisr.c    Tue Nov 23 00:21:45 2004
> @@ -230,6 +230,8 @@
>                IF_DEQUEUE(ni->ni_queue, m);
>                if (m == NULL)
>                        break;
> +                if (((uintptr_t)m & 0x3) != 0)
> +                        printf("%s: mbuf %p is misaligned\n", __func__, m);
>                ni->ni_handler(m);
>        }
> }
>
> 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.

-- 
Sten Spans

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