[En-Nut-Discussion] TCP/IP problems with httpd

[Bernd Walter]

On Tue, Jan 11, 2011 at 04:21:25PM -0200, Thiago A. Corrêa wrote:
> Hi,
> 
> On Tue, Jan 11, 2011 at 3:56 PM, Ole Reinhardt
> <ole.reinhardt at embedded-it.de> wrote:
> 
> > The changes of r3170 look a little strange to me, I have to
> > verify/understand this patch tomorrow.
> >
> > But currently I don't see a problem with alignment, as (hopefully) the
> > compiler should fix this and just generate correctly alligned code.
> >
> 
> Well, I can't say I have very good understanding of alignment issues,
> but before that patch, the code did byte access to the memory, and
> it's always ok. Then it was changed to cast to ushort a position in
> memory that's inside the network buffer. Since network buffer is a
> byte array, there is nothing that really enforces the alignment. The
> buffer might be aligned at it's start, but not at that field's start
> address inside the buffer.

That's basicly a problem whenever you cast something to a more
stricter alignment.
The compiler won't fix alignment problems, since accessing unaligned
memory is always much slower than aligned, so it trusts the promise
a variable make, but it can warn you whenever you do something, which
might be unsave.
It is not always unsave.
E.g. malloc is required to return memory in the strictest alignment
an architecture has, so casting the void* to uint32_t* shouldn't be a
problem unless malloc is broken.
Therefor compilers won't warn you about casting void pointers, so be
carefull when doing this when receiving void* from unknown sources.
If you receive bytes from network it is a byte array, but if it was
malloced then it's not a problem, since e.g. on ARM it was aligned
at least to an 4 byte address and on AVR at least to a 1 byte address.
Ethernet packets can be tricky with alignment because of MAC addresses.
You have a receive buffer, which on ARM is malloced to 4 byte.
Then you receive an IP packet into this buffer.
Now you have 6 bytes target MAC.
The IP header contains 32 bit fields for source and target address
and is 20 Bytes long (5 * 4 bytes).
The problem is that you can't access the addresses by 32 bit, because
although they are properly aligned within the IP header the Header
itself is not because of the MAC address.
The same goes with every header below, such as TCP, UDP, ...
It is not a problem to access 16 bit, because this is still OK.
Under BSD a packet normaly gets realigned after receiving from NIC.
If the packet is not correctly aligned to IP header it is copyed.
Und BSD this is rarely a problem because with most drivers received
buffers are intentianaly missaligned by two bytes to compensate the
requirement, but some hardware (e.g. the already mentioned RM9200 EMAC)
can't use missaligned buffers.
If you want to trust alignment on network buffers you need to go the
BSD way or always handle data at byte level - 2 byte sounds Ok with
ethernet, but other link types like slip might even invalidate this
assumtion.

But completely beside alignment R3170 changes behavour.
R3169 reads byte 2 as high order and byte 3 as low order to form a
16 bit word.
R3170 reads byte 2 and byte 3 in CPU byte order.
Since ntohs is there to reorder the bytes to convert a 16 bit word from
network (high byte first) to host order (CPU depending) R3169 couldn't
be correct, since the 16 bit word passed to ntohs was already converted
to host order.
Either use byte access as in R3169, but without ntohs or trust the 16 bit
alignment.
The log comment isn't absolutely correct, since it is only true on little
endian CPUs.
What puzzles me most is that also ping is said to fail working.
ICMP echo handling is implemented in a very (maybe too) simple way.

> As I said, I haven't hit that as an issue, but it looks like it could
> be an issue, perhaps at other platforms that have different alignment
> boundary requirements or perhaps I just had happy coincidences.
> If the AVR32 code were to reach an alighnment exception, I would have
> seen it in the debugger, as the execution is trapped inside an
> infinite loop, and stopping the debugger drops me there.

I don't think that 16 bit alignment assumption fail for ethernet.

> Also, there is something that bother's me with that code + description.
> Unlike the description, both versions of the code swaps correctly and
> nothing should be advertised wrongly as far as I can see.
> If you ignore the alignment issues, it would only make a difference in
> LE platforms, as the cast incorrectly interprets that memory as a LE
> short, but then ntohs isn't a nop in LE and should bswap it as
> expected.

It is not incorrectly interpreted, since ntohs explicitly wants it in
network order.

> Am I assuming something wrong?

-- 
B.Walter <bernd at bwct.de> http://www.bwct.de
Modbus/TCP Ethernet I/O Baugruppen, ARM basierte FreeBSD Rechner uvm.