fix: ethernet frame header is 14 not 18

[Easyoakland]

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

• I have updated existing examples or added new ones (if applicable).
• I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
• My changes were added to the CHANGELOG.md in the proper section.
• I have added necessary changes to user code to the Migration Guide.
• My changes are in accordance to the esp-rs API guidelines

Extra:

• I have read the CONTRIBUTING.md guide and followed its instructions.

Pull Request Details 📖

Description

ETHERNET_HEADER_FRAME_SIZE = 18 in the following code

esp-hal/esp-wifi/src/wifi/mod.rs

Line 76 in f247b40

const ETHERNET_FRAME_HEADER_SIZE: usize = 18;

is only used here

esp-hal/esp-wifi/src/wifi/mod.rs

Line 950 in f247b40

const DATA_FRAME_SIZE: usize = MTU + ETHERNET_FRAME_HEADER_SIZE;

which is only used here

esp-hal/esp-wifi/src/wifi/mod.rs

Line 2715 in f247b40

static mut BUFFER: [u8; DATA_FRAME_SIZE] = [0u8; DATA_FRAME_SIZE];

I thought maybe the extra 4 bytes were for the fcs at the end but the buffer is passed with the length given by caller here

esp-hal/esp-wifi/src/wifi/mod.rs

Lines 2717 to 2721 in f247b40

	let buffer = unsafe { &mut BUFFER[..len] };
	let res = f(buffer);
	esp_wifi_send_data(self.mode.interface(), buffer);

so that can't be it.

The only reason I noticed this is because smoltcp doesn't support ipv6 fragmentation and ignores if the given MTU is too large (see here) and I tried to send a max MTU UDP packet. It panicked indexing [..len] so I calculated what the max non-fragmenting size would be like so:

    pub const UDP_FORWARD_HEADER_LEN: usize = {
        let out = smoltcp::wire::IPV6_HEADER_LEN
            + smoltcp::wire::UDP_HEADER_LEN
            + size_of::<PacketHeader>(); // repr(C) custom protocol header in the udp payload
        assert!(out == 56);
        out
    };
    pub const MTU: usize = 1492 // matching ESP32 ESP_WIFI_CONFIG_MTU
    // minus the headers
    - UDP_FORWARD_HEADER_LEN;

but was surprised to find the calculated MTU seemed to be 4 bytes less than the panic message implied was valid. I believe I tracked down the source of this discrepancy to the changed line in the PR. As seen here and here an ethernet header is 14 bytes not 18.

I don't know if this would be considered public facing enough to warrant a change log entry, but I'm pretty sure it's a bug.

If my analysis is wrong and the fcs is included in that sizing, then there should at least be an assert that the passed length matches the configured MTU and not config.MTU+4 as it does now. The index check of [..len] should optimize out if the assert occurs before it.

Sidenote: It's a good thing the buffer index was checked and panicked in the esp-wifi code or I probably would never have figured this out. Also, I'll file an issue with smoltcp to at least warn when a packet which is too large and can't be fragmented needs to be sent.

Testing

See above.