ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.67k stars 2.97k forks source link

STM32_EMAC has no-op multicast operations / NUCLEO_743ZI2 multicast pass all filter opt disabled after initialization #13233

Closed tschiemer closed 2 years ago

tschiemer commented 4 years ago

Description of defect

No reception of multicast ethernet packets on STM32 board(s) w/o low-level intervention.

Reception of multicast messages is disabled on startup (MACPFR PM bit) and is never activated when joining a multicast group even though the EMAC's methods are called by the upper layers. As the set_all_multicast method is a no-op it also can not be used as a simple, quick-and-dirty workaround to allow all multicast messages.

The relevant functions of STM32_EMAC are no-op functions :


void STM32_EMAC::add_multicast_group(const uint8_t *addr)
{
    /* No-op at this stage */
}

void STM32_EMAC::remove_multicast_group(const uint8_t *addr)
{
    /* No-op at this stage */
}

void STM32_EMAC::set_all_multicast(bool all)
{
    /* No-op at this stage */
}

Target(s) affected by this defect ?

NUCLEO_H743ZI2 Possibly other STM32 products using the STM32_EMAC and HAL implementations

Toolchain(s) (name and version) displaying this defect ?

mbed Studio 1.0.0.6 macOS 10.15.5

What version of Mbed-os are you using (tag or sha) ?

mbed-os-6.0.0

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

according to mbed studio: arm compiler 6.13 clangd 8.0.1 mbed library cache 1.5.0 python tools 0.5.2

How is this defect reproduced ?

See comment https://github.com/ARMmbed/mbed-os/issues/13233#issuecomment-656032525

ciarmcom commented 4 years ago

@tschiemer thank you for raising this issue.Please take a look at the following comments:

We cannot automatically identify a release based on the version of Mbed OS that you have provided. Please provide either a single valid sha of the form #abcde12 or #3b8265d70af32261311a06e423ca33434d8d80de or a single valid release tag of the form mbed-os-x.y.z . E.g. 'https://github.com/ARMmbed/mbed-os.git#165be79392ae7b1bee4388d2bc8ed8281978f07c' has not been matched as a valid tag or sha.It would help if you could also specify the versions of any tools you are using? How can we reproduce your issue?

NOTE: If there are fields which are not applicable then please just add 'n/a' or 'None'.This indicates to us that at least all the fields have been considered. Please update the issue header with the missing information, the issue will not be mirroredto our internal defect tracking system or investigated until this has been fully resolved.

jeromecoutant commented 4 years ago

Hi @tschiemer Do you think you could propose a patch ? Regards,

tschiemer commented 4 years ago

Hi!

Well, maybe...

..enabling all multicast messages is rather simple, such as:


void STM32_EMAC::set_all_multicast(bool all)
{
    STM32_EMAC &emac = STM32_EMAC::get_instance();

    ETH_MACFilterConfigTypeDef pFilterConfig;

    if (HAL_ETH_GetMACFilterConfig(&emac.EthHandle, &pFilterConfig) != HAL_OK){
        return;
    }

    pFilterConfig.PassAllMulticast = all ? ENABLE : DISABLE;

    HAL_ETH_SetMACFilterConfig(&emac.EthHandle, &pFilterConfig);
}

Hmm.. is that actually the intended behaviour of set_all_multicast(...) ?

With respect to the filtering of specific multicast addresses I see the problem of the available number of MAC addresses, according reference manual of H743ZI2:

The MAC supports 4 MAC addresses for unicast perfect filtering. If perfect filtering is selected (HUC bit of Packet filtering control register (ETH_MACPFR) is reset), the MAC compares all 48 bits of received unicast address with the programmed MAC address for any match. The default MacAddr0 is always enabled.

The MacAddr1 to MacAddr3 addresses are selected with an individual enable bit. You can mask each byte during comparison with corresponding received DA byte by setting the cor- responding Mask Byte Control bit in the register. This enables group address filtering for the DA.

In Hash filtering mode (when HUC bit is set), the MAC performs imperfect filtering for uni- cast addresses using a 64-bit Hash table. For Hash filtering, the MAC uses the upper 6 bits CRC of the received destination address to index the content of the Hash table. A value of 00000 selects bit 0 of selected register, and a value of 11111 selects bit 63 of Hash Table register. If the corresponding bit (indicated by the 6-bit CRC) is set to 1, the unicast packet is considered to have passed the Hash filter; otherwise, the packet is considered to have failed the Hash filter.

So in perfect filtering mode there can be at most three additional multicast addresses (on H743ZI2s). Wanting to use more than three multicast addresses can lead to desired packets not arriving - which has to be hunted down to the emac abstraction. (is there some sort of warning mechanism, that could warn about this situation?)

The hash filtering mode is imperfect and has the potential of bugs that take a moment to hunt down aswell (when a multicast group is left and miraculously some other group fails to receive further messages aswell), even if it would allow for upto 64 different (hashes of) MAC addresses (as I understand).

Not using either perfect filtering and/or hash mode but instead just enabling all multicast messages (say by maintaining an internal multicast counter) should never lead to a situation where problematic behaviour has to be hunted down but it has the disadvantage of consuming more higher-layer resources (and - I don't know (lwIP?) - might thus cause problems) if indeed multicast messages are used. Although, this would interfer with the all multicast option...

So, I don't know if there is a good general approach to take here or if this should instead be configurable through some options... if the options are listed somewhere people can at least look it up..

using a silent escalation-strategy (use perfect match until more addresses are used, after that, just pass all multicast) might be a good generic compromise aswell, although might also lead to strange effects (system behaviour changes after more than three address filters).

Also using either the perfect or hash filter options reduces the options otherwise available - so if any of these are used by default, it must be possible to override it without editing the library file.

So how about:

?

edit: just for reference according to (wikipedia)[https://en.wikipedia.org/wiki/Multicast_address#Ethernet] 01-00-5E-00-00-00 through 01-00-5E-7F-FF-FF : IPv4 Multicast (RFC 1112), insert the low 23 bits of the multicast IPv4 address into the Ethernet address 33-33-00-00-00-00 through 33-33-FF-FF-FF-FF: IPv6 Multicast (RFC 2464), insert the low 32 Bits of the multicast IPv6 Address into the Ethernet Address

so when using ipv4 and ipv6 in parallel, the number of available filters are quite limited. Just having an mDNS service alone would consume two filters...

0xc0170 commented 4 years ago

cc @ARMmbed/mbed-os-ipcore

kjbracey commented 4 years ago

If the specific multicast filter functions aren't implemented, then the driver must have multicast fully enabled always. Just implementing "set_all_multicast" isn't sufficient. (That's only used by users who aren't going to bother to use specific filters, and just want to ask for everything). If you're saying that they're not fully enabled on some STM platforms, that is a bug - IPv6 won't be working at all.

Simplest real implementation is to implement hash-based filtering for add, and forget remove, which I think is what Freescale/NXP devices do.

If you want to try specific filtering (I'm not sure how useful it is), then you need to fall-back to hash-based as soon as you have too many. That should be default behaviour. You could have a config option to force specific filtering and error if going above 3, but not sure how useful that is.

Just having an mDNS service alone would consume two filters...

IPv4 and IPv6 each implicitly register their "all-hosts" addresses too - 224.0.0.1 and ff02::1.

pea-pod commented 4 years ago

I wrote something (that I cannot share) a while back that responds to a multicast packet for the Nucleo F429ZI. It even worked! That was a number of years ago. Did the EMAC changes that hit in 5.9 cause the application I write a while back to break? Otherwise, could you not dig up what was going on there to get it to work?

kjbracey commented 4 years ago

As far as I'm aware IPv6 and hence must be working on major platforms like F429ZI - it's being used as a 6LoWPAN border router in quite a few tests. So the original report, if accurate, must apply to some minority of STM platforms:

Low level initialization of the board's ethernet mac disables reception multicast messages and never activates it

OP says NUCLEO_H743ZI2 specifically.

If the generic EMAC driver never touches multicast settings, then it would be reliant on board init code, and it shouldn't be. That would explain the syymptoms. It should be explicitly asking for all multicast from the HAL if not implementing the controls.

tschiemer commented 4 years ago

Ok, so I made a partially wrong analysis: the PM Bit is seemingly not set to begin with - as opposed to my previous statement it's not set before the low level init (mistakingly relied on an uninitialized handle there..)

Having another look at it right now.

tschiemer commented 4 years ago

So using this basic code on an NUCLEO_H743ZI2 I see that:

Now, being new to mbed-os, I might be overlooking something or misinterpreting something here, but I think I don't ?

ciarmcom commented 4 years ago

@tschiemer it has been 5 days since the last reminder. Could you please update the issue header as previously requested?

jeromecoutant commented 4 years ago

@tschiemer any update ? Will you propose some patch soon ? Thx

tschiemer commented 4 years ago

hej @jeromecoutant

sorry, at the moment I don't enough capacity to look into it and I'm not sure I have enough insight into the overall system to make a proposition (and have no means to check it on other systems).

But my suggestion would be to add something like the code from my previous comment to the STM32_EMAC::add_multicast_group method thus enabling all multicast messages when at least one multicast group is joined; I would ignore leaving any groups.

If you think this'd be fine I can propose a patch the next few days?

jeromecoutant commented 4 years ago

If you think that your proposition is helping community, I will approve the patch! Thx

ciarmcom commented 3 years ago

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers. Internal Jira reference: https://jira.arm.com/browse/IOTOSM-2423

ciarmcom commented 2 years ago

We closed this issue because it has been inactive for quite some time and we believe it to be low priority. If you think that the priority should be higher, then please reopen with your justification for increasing the priority.