|Subject:||Re: [lwip-devel] in-place overwriting of payload via static "tcphdr" pointer.|
|Date:||Mon, 8 Jan 2018 11:18:14 +0100|
|User-agent:||Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0|
At first I also suspected the Xilinx driver to be at fault. But after tracing the problem I think
that Xilinx can't really correct the problem without a significant performance impact.
I mean, what is the Xilinx port driver (or any other driver for that matter) supposed to do with
received packets from the EMACs DMA ring buffer?
- Copy the all packets to a different memory location, just so LWIP can patch some fields in the TCP header?
- Completely disable the memory cache for all RX DMA buffers?
Both options would have a significant performance impact.
There is no real reason for writing into received packets anyways. You could byte-flip the TCP header fields on-the-fly when needed.
Please note that LWIP's UDP stack does not modify RX packets -- so actually it *is* possible to leave rx packets unmodified.
Honestly, I do not fully understand how this is not a problem for every lwip port driver, not just Xilinx.
Many systems issue an RX interrupt and use (non-cache-coherent) DMA to transfer data from EMAC to memory.
Since memory is not unlimited, new rx packets will at some point overwrite old rx packets (ring buffer).
As response to your post:
> How would you ensure no application writes into the application part of an rx pbuf?
- either by documenting that you are not allowed to modify rx pbufs.
- or by handing out 'const' pointers, (and documenting that you shouldn't cast away the 'const').
> Remember that rx pbufs are not 'const' for lwIP.
But they should be const.
Normally an application does not need to write into the received packets. I can't think of a good reason
for that, especially not for TCP, where you normally have to assemble/copy the data stream from multiple
pbuf payloads anyways! If you need to patch rx data at all, then why not patch the assembled stream data?
Patching rx data is also not good programming practice: multiple entities (EMAC and application/lwip) would write to the same memory.
This makes code less readable and can lead to race conditions.
Actually I tried to fix this issue by making all pointers to rx packet buffers 'const' in tcp_in.c, but I resigned after realizing the amount of changes that would require.
My current solution is to just patch lwip's tcp_input function and place a cache flush for the TCP header at each exit point.
This is no nice solution either, since it adds dependence on Xilinx API to LWIP - but it's the smaller patch.
but you have to understand that lwIP is working for many many people like it is now.That is what I don't fully understand, how it is working for those many people. My guesses:
- either their driver just copies the RX packets after DMA (performance hit).
- or they have some form of cache-coherent DMA (but this requires special DMA hardware which most systems don't have).
- or, since it's a very rare race condition, it might accidentally not happen with their application's memory access pattern - but it could happen in the future.
- or it does happen, but they don't care if they lose a packet about once every hour, since TCP recovers from that loss automatically.
So the issue you have is an issue in your driver or port, not in lwIP.I tried that two years ago, they don't react. Even If they did, the best solution would be to just copy the packets before handing them to lwip.
Am 08.01.2018 um 11:10 schrieb Bernhart Pelger:
-- -- Bernhart Pelger-Alzner Entwicklungsingenieur Software Tel: +49 89 201804-261 Fax: +49 89 201804-100 E-Mail: address@hidden ASTYX GmbH Lise-Meitner-Str. 2a, 85521 Ottobrunn, Germany Postfach 1248, 85504 Ottobrunn, www.astyx.de Reg.Nr.: München HRB 116 461, VAT: DE 186 893 572 CEO: Dipl.-Ing. G. Trummer
|[Prev in Thread]||Current Thread||[Next in Thread]|