stm32-rs / stm32-eth

Embedded Ethernet driver in Rust
Apache License 2.0
147 stars 47 forks source link

Add checksum insertion and offloading #48

Closed dsgruss closed 2 years ago

dsgruss commented 2 years ago

The stm32 has a built-in module to automatically handle checksums inside IP headers and TCP, UDP, and ICMP payloads, on both the transmit and receive paths. For reference, the stm4 manual covers transmitting on pages 1141 and 1172, and receiving on pages 1145 and 1184.

For transmission, this involves setting two CIC bits in the transmit descriptor so that checksums are calculated and inserted. On the receive side, we set a bit in the maccr register, and the error is grouped into the general error summary flag on the receive descriptor.

In my (very rough) profiling, this change resulted in about a 30% speedup at 75 Mbps Tx over doing the checksums in software inside smoltpc.

Looking through some other manuals, this feature appears to be supported in all MCUs that have a hardware ETH controller. For example:

Let me know if this would be better done some other way, such as config setting, feature, ...

datdenkikniet commented 2 years ago

I think this sounds like a good addition, and it would make sense to enable unconditionally given that it appears that all parts support this feature, and it almost exclusively comes with upsides.

All appropriate bits seem to be set:

The CI seems to agree with your observation about it being supported on all parts, so that's very nice too.

~@korken89 @thalesfragoso would appreciate another look~ Not yet, I guess :P

datdenkikniet commented 2 years ago

Correction: I thought we had already enabled enhanced descriptors (edfe in ETHDMABMR), but it seems like I have made a mistake there. On stm32f4xx (and most likely stm32f7xx parts), enhanced descriptors must be enabled for offload support. See page 1186 of the f4 RM, under Enhanced Rx DMA descriptors format with Ieee1588 time stamp (gee, thanks for the obvious location of that description ST...).

This bit is also required for timestamping, so you can steal/find the code for how to do it in #46. The main (and AFAIK only) differences are that the edfe bit must be set, and that a descriptor is now 8 words wide, instead of 4.

dsgruss commented 2 years ago

Interesting! I had missed that part of the manual. I'm on an stm32f429 and, as far as I could tell, packets with malformed headers were still dropped even without enabling enhanced descriptors and correct packet data came through unaltered...

Is it alright to set the descriptor width to 8 words even on the f1 parts, or should there be a config check there as well?

datdenkikniet commented 2 years ago

Aha, that's interesting indeed! Wonder how that works...

Hmm, I'm not sure. We're running the descriptors in "chain" mode (aka singly-linked-list mode), so it should be fine since the descriptors will point to the next entry by address. However, I don't have access to an f1 so have no way to test it.

I think it'd be best if we avoid using 8-word wide descriptors on f1 for now, simply because it's not 100% certain that it'll work without issues. Luckily, it's quite easily achievable by changing the descriptor code to the following:

#[cfg(not(feature = "stm32f107"))]
const DESC_SIZE: usize = 8;

#[cfg(feature = "stm32f107")]
const DESC_SIZE: usize = 4;

#[repr(C)]
pub struct Descriptor {
    desc: Aligned<A8, [u32; DESC_SIZE]>,
}

...
impl Descriptor {
    pub const fn new() -> Self {
        Self {
            desc: Aligned([0; DESC_SIZE]),
        }
    }
...
}
datdenkikniet commented 2 years ago

LGTM! Thank you for your contribution :)