[PATCH rtems-libbsd 2/2] microblaze: Finish AXI Ethernet support

[Alex White]

On Thu, Jan 20, 2022 at 12:12 AM Sebastian Huber <sebastian.huber at embedded-brains.de> wrote:
>
> On 20/01/2022 04:54, Alex White wrote:
> > diff --git a/freebsd/sys/netinet/tcp_input.c b/freebsd/sys/netinet/tcp_input.c
> > index fc111d9c..60f9123a 100644
> > --- a/freebsd/sys/netinet/tcp_input.c
> > +++ b/freebsd/sys/netinet/tcp_input.c
> > @@ -721,6 +721,15 @@ tcp_input(struct mbuf **mp, int *offp, int proto)
> >       }
> >   #endif /* INET */
> >  
> > +#ifdef __rtems__
> > +#ifdef __MICROBLAZE__
> > +     /* Ensure that the TCP header is properly aligned in memory. */
> > +     struct tcphdr aligned_hdr;
> > +     memcpy(&aligned_hdr, th, sizeof(struct tcphdr));
> > +     th = &aligned_hdr;
> > +#endif
> > +#endif
> > +
> >       /*
> >        * Check that TCP offset makes sense,
> >        * pull out TCP options and adjust length.              XXX
>
> Changes like this should not be hidden in a driver-specific patch. The
> alignment changes are hacks. Why don't you fix the Ethernet frame
> alignment in the network interface driver?
>

I have made the realization while testing fixes for this that if I
change the definition of struct tcphdr in netinet/tcp.h to include
__attribute__((packed)), GCC generates code that avoids the unaligned
memory access. This removes the need for any code to handle memory
alignment of the network headers.

I then noticed that you had some notes on what I think is this same
phenomenon at freebsd/contrib/libpcap/extract.h line 40.

Since, for example, the ip struct in netinet/ip.h is defined as packed,
shouldn't the tcphdr struct be packed, too?

In addition, struct tcphdr is being used to represent raw bytes on the
wire so it makes sense to declare it as packed so no padding is
included.

The initial solution I came up with was to change
newlib/libc/sys/rtems/include/netinet/tcp.h in newlib to add
__attribute__((packed)) to struct tcphdr . This could be added inside
an #ifdef __microblaze__ to minimize impact elsewhere. This would, of
course, diverge from FreeBSD's netinet/tcp.h.

Does that sound like a reasonable solution?

Thanks,

Alex