Re: CVS commit: src/sys

[Roy Marples]

On 13/02/2021 21:34, David Young wrote:

On Tue, Feb 09, 2021 at 07:02:32AM +, Roy Marples wrote:

Hi David

On 03/02/2021 21:45, David Young wrote:


This change looks a little hasty to me.

It looks to me like some of these structs were __packed so that
they could be read/written directly from/to any offset in a packet
chain using mtod(), which does not pay any mind to the alignment
of `*t`:

#define mtod(m, t)  ((t)((m)->m_data))

I see gre_h is accessed in that way, just for one example.


Looking at the other places where this is handled, does this patch to gre_h
address your concerns?
I tested this on aarch64 which does not define __NO_STRICT_ALIGNMENT and it
passes the KASSERT.


It is possible to simplify your patch a lot.  The GRE header is only 4
bytes long.  On the receive side, just perform the m_pullup like the
old code did and then memcpy to a `struct gre_h` on the stack.  On the
send side, construct the header on the stack and then memcpy it into the
mbuf.

The same general approach, of copying headers between mbufs the stack,
is probably plenty fast for virtually any size of header used in the
network stack.


Done

Re: CVS commit: src/sys/net

[Roy Marples]

On 13/02/2021 14:19, Jonathan A. Kollasch wrote:

On Sat, Feb 13, 2021 at 07:28:05AM +, Roy Marples wrote:

Module Name:src
Committed By:   roy
Date:   Sat Feb 13 07:28:05 UTC 2021

Modified Files:
src/sys/net: if_ether.h if_ethersubr.c

Log Message:
if_ether: Ensure that ether_header is aligned


To generate a diff of this commit:
cvs rdiff -u -r1.84 -r1.85 src/sys/net/if_ether.h
cvs rdiff -u -r1.289 -r1.290 src/sys/net/if_ethersubr.c


This appears to ensure the Ethernet header is naturally aligned on a
32-bit boundary.  The 16-bit ether_type field is the only thing in
ether_header that is wider than a uint8_t.

Many drivers will place the start of the receive buffer at an
ETHER_ALIGN (which is 2) offset to ensure the layer three header
is 32-bit aligned after the 14-byte Ethernet header.  Thus this
will result in always calling m_copyup() in ether_input() on strict
alignment platforms.


Reverted