Re: [lwip-devel] in-place overwriting of payload via static "tcphdr" pointer.

[address@hidden]

Bernhart Pelger wrote:
> [..]
> The Xilinx driver does not use custom_pbufs (as suggested in ZeroCopyRx.c).
> Therefore it cannot know when a pbuf is not needed any more.
> The Xilinx driver immediately frees the DMA buffer at RX interrupt (yes,
> before LWIP has processed it).
>
> I know this sounds fundamentally broken, because other arriving packets
> might be overwriting the free'd DMA buffers before LWIP or the
> application had a chance to process it.

This does not *sound* fundamentally broken, it *is*!
You don't need custom pbufs to achieve what you want. Just allocate one
PBUF_POOL per RX packet and if the pool is empty, re-enqueue a freshly
allocated PBUF_POOL once a pbuf gets freed again.

> However, with 256 DMA buffers there is some time for 256 packets to
> arrive before a wraparound-overwrite occurs.
> And, If you permanently process packets slower than they arrive then you
> will lose packets anyways - at one place or the other.

You risk all kinds of errors if a pbuf you are in the process of parsing
is just overwritten by the DMA engine. You won't notice lost packets but
just some strange kind of overwritten data at *any* point in an RX packet!

> [..]
> What do you mean by "this (byte-flipping?) can be delayed"?
> Do you mean "this (byte-flipping) can cause a delay"?

I meant because of handling out-of-sequence packets at a later point, the
header fields are touched again. Then, they have to be converted again.

> [..]
> Ok, leaving the application programmer more freedom is fine.
> But what about the freedom for the driver programmer, to use single
> cache-invalidates for some performance gain?

That's what I meant. If someone would make a patch changin TCP to not
write back into the pbuf, I'd probably push it. I just don't want to do
the work ;-)

> [..] That's directly in the RX ISR before the pbuf is prepared for LWIP.
> There's already a cache invalidation there. Double-invalidating there
> would have no effect.

That's because your driver is broken, yes :-)

> [..] But that would still leave me invalidating the same DMA buffer two
> times: after packet reception and
> when the pbuf is freed (after lwip and application processing).

Yes. Going back to the Linux example, I don't think they would do this if
it wasn't required. It hurts there, too. Trust me, I've been there
measuring the performance gain of a Linux netdev only invalidating once.
It's just not stable with all possible workloads.

But you might be right, when changing TCP, we might improve the situation
for some very specialized setups. Do keep in mind though that this doesn't
result in a generic driver on your side! Your driver (or the Xilinx driver?)
is still fundamentally broken! :-/

Simon