drivers/wifi/nrfwifi: Unable to change mac address by net_mgmt

[hongquan-prog]

Describe the bug When I change the mac address of nrf7002 via net_mgmt, the function returns success, after I try to connect to the AP, the address on the gateway remains unchanged.

To Reproduce Add the following code to the wifi sample and run it.

static int mac_init(void)
{
    uint8_t mac[6] = {0xfa, 0xd5, 0x92, 0x57, 0xfe, 0x6f};
    struct ethernet_req_params params = {0};
    struct net_if             *iface  = net_if_get_default();

    memcpy(params.mac_address.addr, mac, sizeof(mac));
    net_mgmt(NET_REQUEST_ETHERNET_SET_MAC_ADDRESS, iface, &params, sizeof(struct ethernet_req_params));

    return 0;
}

Expected behavior The address shown on the AP is the new address.

Impact Unable to change mac address via net_mgmt.

Environment (please complete the following information):

• OS: MacOS
• Toolchain: zephyr-sdk-0.16.4
• v4.0-branch

[jukkar]

Please make sure that the network interface is down before trying to change the MAC address.

[hongquan-prog]

Please make sure that the network interface is down before trying to change the MAC address.

I confirmed through breakpoint debugging that it does enter nrf_wifi_if_set_config_zep, the ethernet_set_config function also checks the state of the network interface and returns an error code if it is up.

[hongquan-prog]

Please make sure that the network interface is down before trying to change the MAC address.

I can change the MAC address by setting the link address via net_if_set_link_addr before the network stack initialisation

[krish2718]

NET_REQUEST_ETHERNET_SET_MAC_ADDRESS

I just tried on v4.0-branch and it seems to work, right after the boot, I did net iface 1 and I could see the MAC address Link addr : FA:D5:92:57:FE:6F.

[hongquan-prog]

NET_REQUEST_ETHERNET_SET_MAC_ADDRESS

I just tried on v4.0-branch and it seems to work, right after the boot, I did net iface 1 and I could see the MAC address Link addr : FA:D5:92:57:FE:6F.

Thanks for your help, I found something from zephyr, I always thought that net_if_is_up returning false meant that the interface was not up yet, when I looked at the function implementation through debugging, I found that the interface was up but not running yet，so I always failed to set the mac address.

By default NRF_WIFI_IF_AUTO_START has a value of 1. In order to use net_mgmt to change the mac address I need to turn this option off and call net_if_up(iface) after setting the mac address.

static int mac_init(void)
{
    uint8_t mac[6] = {0xfa, 0xd5, 0x92, 0x57, 0xfe, 0x00};
    struct ethernet_req_params params = {0};
    struct net_if             *iface  = net_if_get_default();

    memcpy(params.mac_address.addr, mac, sizeof(mac));
    net_mgmt(NET_REQUEST_ETHERNET_SET_MAC_ADDRESS, iface, &params, sizeof(struct ethernet_req_params));
    net_if_up(iface);

    return 0;
}

I sincerely hope you can modify the following check on the status of the network interface.

static int ethernet_set_config(uint32_t mgmt_request,
                   struct net_if *iface,
                   void *data, size_t len)
{
    struct ethernet_req_params *params = (struct ethernet_req_params *)data;
    const struct device *dev = net_if_get_device(iface);
    const struct ethernet_api *api = dev->api;
    struct ethernet_config config = { 0 };
    enum ethernet_config_type type;

    ......

    if (mgmt_request == NET_REQUEST_ETHERNET_SET_AUTO_NEGOTIATION) {
        if (!is_hw_caps_supported(dev,
                      ETHERNET_AUTO_NEGOTIATION_SET)) {
            return -ENOTSUP;
        }

        config.auto_negotiation = params->auto_negotiation;
        type = ETHERNET_CONFIG_TYPE_AUTO_NEG;
    } 

        ......

    } else if (mgmt_request == NET_REQUEST_ETHERNET_SET_MAC_ADDRESS) {
        if (net_if_is_up(iface)) {
            return -EACCES;
        }

                 /* We need to remove the old IPv6 link layer address, that is
         * generated from old MAC address, from network interface if
         * needed.
         */
        if (IS_ENABLED(CONFIG_NET_NATIVE_IPV6)) {
            struct in6_addr iid;

            net_ipv6_addr_create_iid(&iid,
                         net_if_get_link_addr(iface));

            /* No need to check the return value in this case. It
             * is not an error if the address is not found atm.
             */
            (void)net_if_ipv6_addr_rm(iface, &iid);
        }

        memcpy(&config.mac_address, &params->mac_address,
               sizeof(struct net_eth_addr));
        type = ETHERNET_CONFIG_TYPE_MAC_ADDRESS;
    }
}

Replace net_if_is_up with the following code：

if (net_if_flag_is_set(iface, NET_IF_UP) ||
    net_if_flag_is_set(iface, NET_IF_RUNNING)) {
    return -EACCES;
}

[krish2718]

A network interface in Zephyr has two active states ADMIN_UP -> administratively UP i.e., the driver is up and running, UP -> operationally UP i.e., the link can be used. For 802.11 UP is only after a successful Wi-Fi connection.

So, now to set MAC address the device shouldn't be administratively up, so, you are right about the bug, but the fix should be like below:

diff --git a/subsys/net/l2/ethernet/ethernet_mgmt.c b/subsys/net/l2/ethernet/ethernet_mgmt.c
index 703dbbccb6d..d7a49022fca 100644
--- a/subsys/net/l2/ethernet/ethernet_mgmt.c
+++ b/subsys/net/l2/ethernet/ethernet_mgmt.c
@@ -90,7 +90,7 @@ static int ethernet_set_config(uint32_t mgmt_request,
                config.full_duplex = params->full_duplex;
                type = ETHERNET_CONFIG_TYPE_DUPLEX;
        } else if (mgmt_request == NET_REQUEST_ETHERNET_SET_MAC_ADDRESS) {
-               if (net_if_is_up(iface)) {
+               if (net_if_is_admin_up(iface)) {
                        return -EACCES;
                }

And with the above patch, you can do something like https://github.com/nrfconnect/sdk-nrf/blob/main/samples/wifi/scan/src/main.c#L336.

I can submit a PR with the above fix, @jukkar @rlubos WDYT?

[rlubos]

I can submit a PR with the above fix, @jukkar @rlubos WDYT?

Sounds reasonable.

[jukkar]

Makes sense, just submit a proper PR. Note that your example commit has a typo in the subject (s/ehternet/ethernet/)