Ping requests only responded to after two requests have arrived about stabilizer HOT 17 CLOSED

If there is other traffic (e.g. streaming the samples out of port 1234) the ping response is prompt.

$ ping 10.255.6.1
PING 10.255.6.1 (10.255.6.1) 56(84) bytes of data.
64 bytes from 10.255.6.1: icmp_seq=1 ttl=64 time=0.298 ms
64 bytes from 10.255.6.1: icmp_seq=2 ttl=64 time=0.240 ms
64 bytes from 10.255.6.1: icmp_seq=3 ttl=64 time=0.353 ms

from stabilizer.

I had also seen it and tried to debug it in different ways but didn't figure it out. TCP speed or latency is not affected at all by this. It's only ICMP.

@whitequark @astro ideas?

from stabilizer.

Can you grab the debug log?

from stabilizer.

@whitequark here is a debug log
and the associated packet dump

The output from ping:

$ ping 10.255.6.54
PING 10.255.6.54 (10.255.6.54) 56(84) bytes of data.
64 bytes from 10.255.6.54: icmp_seq=1 ttl=64 time=1011 ms
64 bytes from 10.255.6.54: icmp_seq=2 ttl=64 time=0.396 ms
64 bytes from 10.255.6.54: icmp_seq=3 ttl=64 time=1020 ms
64 bytes from 10.255.6.54: icmp_seq=4 ttl=64 time=0.387 ms
64 bytes from 10.255.6.54: icmp_seq=5 ttl=64 time=928 ms
64 bytes from 10.255.6.54: icmp_seq=6 ttl=64 time=1016 ms
64 bytes from 10.255.6.54: icmp_seq=7 ttl=64 time=0.395 ms
^C
--- 10.255.6.54 ping statistics ---
7 packets transmitted, 7 received, 0% packet loss, time 6095ms
--

My read of this is that this is a problem on the MAC transmit.
The timestamps of the received packets match what is on the wire, e.g. the ping requests for seq 6 and 7 have a time difference on the wire of 1.016s, and the smoltcp timestamps differ by 1.017s.
However the timestamps of the transmitted packets do not match. E.g. the response to seq 6 occurs at the same smoltcp timestamp as the request, but does not appear on the wire for over 1s, until the response to 7 has also been queued.

from stabilizer.

Yes, looks like a PHY driver problem.

from stabilizer.

This is a cache coherency issue: the OWN flag in the tx descriptor is not written-back by the time the ethernet DMA reads the descriptor, meaning that the packet is only sent when a second one is queued.
Adding a data cache clean after writing the descriptor OWN flag solves /this/ problem.

This probably also explains a lot of other weird crashes / glitches we see on the ethernet interface, as the current interface is not cache friendly. I am not sure what cache we are hitting though, as the CPU L1 cache is definitely disabled...

from stabilizer.

Are we doing all these things? https://community.st.com/s/article/FAQ-Ethernet-not-working-on-STM32H7x3

from stabilizer.

Notes:

• We are putting the buffers into .sram3.eth, which should be fine: SRAM3 (rwx) : ORIGIN = 0x30040000, LENGTH = 32K.
• Is there an ordering/atomicity issue even with the dcache disabled?
• Did we check that the CPU actually comes up with the dcache disabled/there isn't some piece of code actually enabling it?

from stabilizer.

More notes:

• If we ever do enable the dcache, I think we need to align the descriptors to the cache line size (32b). They might end up like this currently by accident, but making this clear in the source would help (iirc the u32 array will have an align size of 4).

from stabilizer.

Note aside: How are we making sure that the tx buffer content stores aren't reordered across the DMA tail pointer store? The latter is (I hope) a ptr::write_volatile, but aren't volatile writes only ordered w.r.t. other volatile operations in Rust as well?

from stabilizer.

Correct. That code should use an atomic access, or an explicit memory barrier.

from stabilizer.

Something like atomic::compiler_fence(Ordering::SeqCst), right?

from stabilizer.

(I haven't checked what atomics map to on that target, but a compiler fence is all that should be required.)

from stabilizer.

Sounds correct.

from stabilizer.

Turns out a compiler fence is not enough; possibly because of the CPU store buffer.

Chris has a patch brewing, I think. (Edit: See #49).

from stabilizer.

possibly because of the CPU store buffer.

I've actually thought of recommending using a proper atomic store instead of a fence because of the store buffer, but foolishly assumed that it must be strictly in-order and so that wouldn't matter. Looks like it isn't even on M-class CPUs.

from stabilizer.

Looks like it isn't even on M-class CPUs.

I was somewhat taken by surprise on this as well. Apparently M0-M4 are still in-order, but M7 isn't.

from stabilizer.