Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IPv4 multicast datagrams can't be received for mimxrt1064_evk board (missing ethernet API) #26585

Closed
elsinkior opened this issue Jul 1, 2020 · 9 comments · Fixed by #38501
Closed
Assignees
Labels
area: Networking Enhancement Changes/Updates/Additions to existing features platform: NXP NXP priority: medium Medium impact/importance bug

Comments

@elsinkior
Copy link

elsinkior commented Jul 1, 2020

Describe the bug
IPv4 multicast datagrams can't be received for mimxrt1064_evk board (missing ethernet API).
In order to support IPv4 multicast datagrams, it is necessary to modify the mimxrt1064_evk ethernet driver API to join a multicast group.
For mimxrt1064, it is the file eth_mcux.c, which could provide the configuration function set_config to add or remove a multicast source adress filter to the interface.

  1. Open the file $ ZEPHYR_BASE / zephyr / drivers / ethernet / eth_mcux.c
  2. the following static function could be defined
static int set_config (struct device * dev,
                           enum ethernet_config_type type,
                           const struct ethernet_config * config)
{
    int result = -1;
    switch (type)
    {
    ETHERNET_CONFIG_TYPE_FILTER box:
    {
        switch (config-> filter.type)
        {
        ETHERNET_FILTER_TYPE_SRC_MAC_ADDRESS box:
        {
            / * Adds or leaves the ENET device to a multicast group. * /
            uint8_t multicastMacAddr [6];
            for (uint32_t i = 0; i <6; i ++)
            {
                multicastMacAddr [i] = config-> filter.mac_address.addr [i];
                if (config-> filter.set)
                {
                    printk ("\ r \ nENET_AddMulticastGroup \ r \ n");
                    ENET_AddMulticastGroup (ENET, multicastMacAddr);
                }
                else
                {
                    printk ("\ r \ nENET_LeaveMulticastGroup \ r \ n");
                    ENET_LeaveMulticastGroup (ENET, multicastMacAddr);
                }
                result = 0;
            }
        }
        break;
        default:
        {
            // Nothing
        }
        break;
        }
        break;
        default:
        {
            // Nothing
        }
        break;
    }
    }
    return result;
}
  1. ... and added to the api by modifying the api_funcs structure:
static const struct ethernet_api api_funcs = {
    .iface_api.init = eth_iface_init,
    .get_capabilities = eth_mcux_get_capabilities,
    .send = eth_tx,
    .set_config = set_config, / * Adding the set config function to the API * /
};

To Reproduce
Try to receive UDP ipv4 multicast datagrams.

Environment (please complete the following information):

  • OS: linux
  • Toolchain: zephyr sdk v0.1.13
  • board: mimxrt1064_evk
  • SHA: 979e124
@elsinkior elsinkior added the bug The issue is a bug, or the PR is fixing a bug label Jul 1, 2020
@MaureenHelm MaureenHelm changed the title IPv4 multicast datagrams can't be received for mimxrt1046_evk board (missing ethernet API) IPv4 multicast datagrams can't be received for mimxrt1064_evk board (missing ethernet API) Jul 7, 2020
@MaureenHelm MaureenHelm added the priority: medium Medium impact/importance bug label Jul 7, 2020
@jukkar
Copy link
Member

jukkar commented Jul 10, 2020

For IPv6 there is net_if_mcast_mon_register() that can be used to register a function that will be called if a multicast address is added/removed to the network interface. That function should have IPv4 address support and then we could enhance the IPv4 support in the driver side. I am not convinced that we should have this implemented via set_config().

@agansari
Copy link
Collaborator

agansari commented Jul 10, 2020

@jukkar i've looked at the driver and adding the driver part is not difficult we have to re-purpose net_if_mcast_cb and remove ipv6 configuration, also re-purposing net_if_mcast_mon_register() is not difficult.
However I need to ask you don't we need to add some new ipv4 functions/events/handles that handle joining/leaving ethernet multicast groups? I.e. networking stack support.

@jukkar
Copy link
Member

jukkar commented Jul 20, 2020

However I need to ask you don't we need to add some new ipv4 functions/events/handles that handle joining/leaving ethernet multicast groups? I.e. networking stack support.

Yes, we need to join/leave multicast group when IPv4 address is added/removed to/from network interface. Sorry for not being more clear in my words above. So this would need to be implemented, basically the functionality should be similar as with IPv6.

@dleach02
Copy link
Member

dleach02 commented Aug 13, 2020

On the issue of adding join/leave API, is this something that is generically needed across all drivers?

Also, if we add the .set_config would that be enough to close this ticket?

@jukkar
Copy link
Member

jukkar commented Aug 14, 2020

On the issue of adding join/leave API, is this something that is generically needed across all drivers?

Probably yes, typically (at least for IPv6 mcast msg) the driver needs to program the Ethernet controller so that it can receive the multicast packets.

Also, if we add the .set_config would that be enough to close this ticket?

We have an API where a driver can register a function to be called when net stack joins/leaves a multicast group. Right now that API supports only IPv6 addresses. That API would need to be extended to support also IPv4 addresses. See for example drivers/ethernet/eth_mcux.cand search for net_if_mcast_mon_register() to see how it is used.

But this is only part of the story as also network stack would need to support IPv4 multicast handling similar way as what have been done for IPv6. Right now there is an API to add a IPv4 multicast address to the network interface net_if_ipv4_maddr_add(() but it does not trigger a call to multicast monitoring which would inform the driver that the multicast group needs to be joined.
Changes to the code to support this are not very big, it is just that someone needs to join the dots to make this work and test it properly.

@dleach02
Copy link
Member

@jukkar, If I'm understanding you then it seems like this isn't a platform specific bug?

@jukkar
Copy link
Member

jukkar commented Aug 17, 2020

@dleach02 First of all, this is not a bug, we are just missing a feature. And second, this is not a platform specific issue.

@dleach02
Copy link
Member

Cool... so @jukkar we can convert this to an enhancement request then?

@dleach02 dleach02 removed the platform: NXP NXP label Aug 17, 2020
@jukkar jukkar added Enhancement Changes/Updates/Additions to existing features and removed bug The issue is a bug, or the PR is fixing a bug labels Aug 18, 2020
@jukkar
Copy link
Member

jukkar commented Aug 18, 2020

we can convert this to an enhancement request then?

Yep, done.

@pfalcon pfalcon added the platform: NXP NXP label Oct 5, 2020
mrfuchs added a commit to mrfuchs/zephyr that referenced this issue Sep 19, 2021
Make multicast group join/leave monitor support both IPv6 and IPv4
addresses.

Fixes zephyrproject-rtos#26585

Signed-off-by: Markus Fuchs <markus.fuchs@ch.sauter-bc.com>
cfriedt pushed a commit that referenced this issue Sep 29, 2021
Make multicast group join/leave monitor support both IPv6 and IPv4
addresses.

Fixes #26585

Signed-off-by: Markus Fuchs <markus.fuchs@ch.sauter-bc.com>
Rushybrook pushed a commit to Rushybrook/zephyr that referenced this issue Oct 21, 2021
Make multicast group join/leave monitor support both IPv6 and IPv4
addresses.

Fixes zephyrproject-rtos#26585

Signed-off-by: Markus Fuchs <markus.fuchs@ch.sauter-bc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking Enhancement Changes/Updates/Additions to existing features platform: NXP NXP priority: medium Medium impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants