Hi,
I'm using the WILC1000 wifi and with NetworkManager [1] I see issues
in certain situations I see problems.
I was able to reduce the problem and have now found out that the cause
is that the interface has the HW MAC address is 00:00:00:00:00 after
startup. Only when the interface is startup (ip link set dev wlan0
up), the driver sets a "valid" address.
Is this a valid behavior and shouldn't the address already be set
after loading the driver?
[1] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1200
--
Heiko
Hi Heiko,
On 2/8/23 07:24, Heiko Thiery wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi,
>
> I'm using the WILC1000 wifi and with NetworkManager [1] I see issues
> in certain situations I see problems.
>
> I was able to reduce the problem and have now found out that the cause
> is that the interface has the HW MAC address is 00:00:00:00:00 after
> startup. Only when the interface is startup (ip link set dev wlan0
> up), the driver sets a "valid" address.
>
IIUC network manager(NM) is trying to read the MAC address and write the
same back to wilc1000 module without making the wlan0 interface up. right?
Not sure about the requirement but if NM has a valid MAC address to
assign to the wlan0 interface, it can be configured without making
interface up("wlan0 up"). "ip link set dev wlan0 address XX:XX:XX:XX:XX"
command should allow to set the mac address without making the interface
up.
Once the mac address is set, the wilc1000 will use that mac address [1]
instead of the one from wilc1000 NV memory until reboot. However, after
a reboot, if no MAC address is configured from application then wilc1000
will use the address from its NV memory.
> Is this a valid behavior and shouldn't the address already be set
> after loading the driver?
>
Only when the interface is up(ifconfig wlan0 up), driver loads the
firmware to wilc1000 module and after that the WID commands which allows
to set/get the mac address from the wilc1000 works.
Regards,
Ajay
1.
https://github.com/torvalds/linux/blob/master/drivers/net/wireless/microchip/wilc1000/netdev.c#L600
On Thu, 9 Feb 2023 17:15:38 +0000 [email protected] wrote:
> IIUC network manager(NM) is trying to read the MAC address and write the
> same back to wilc1000 module without making the wlan0 interface up. right?
>
> Not sure about the requirement but if NM has a valid MAC address to
> assign to the wlan0 interface, it can be configured without making
> interface up("wlan0 up"). "ip link set dev wlan0 address XX:XX:XX:XX:XX"
> command should allow to set the mac address without making the interface
> up.
> Once the mac address is set, the wilc1000 will use that mac address [1]
> instead of the one from wilc1000 NV memory until reboot. However, after
> a reboot, if no MAC address is configured from application then wilc1000
> will use the address from its NV memory.
netdev should be created with a valid lladdr, is there something
wifi-specific here that'd prevalent that? The canonical flow is
to this before registering the netdev:
err = read_mac_from_nv();
if (err || !is_valid_ether_addr())
eth_hw_addr_random()
Hi,
Am Do., 9. Feb. 2023 um 18:15 Uhr schrieb <[email protected]>:
>
> Hi Heiko,
>
> On 2/8/23 07:24, Heiko Thiery wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Hi,
> >
> > I'm using the WILC1000 wifi and with NetworkManager [1] I see issues
> > in certain situations I see problems.
> >
> > I was able to reduce the problem and have now found out that the cause
> > is that the interface has the HW MAC address is 00:00:00:00:00 after
> > startup. Only when the interface is startup (ip link set dev wlan0
> > up), the driver sets a "valid" address.
> >
>
> IIUC network manager(NM) is trying to read the MAC address and write the
> same back to wilc1000 module without making the wlan0 interface up. right?
As far as I understand, network-manager will read the "real" HW
address. Then it sets a random
generated HW for scanning and after that switches back to the "real" HW address.
There seems to be circumstances where the wrong HW address
(00:00:00:00:00:00) is read and stored for
later reset after the scanning.
> Not sure about the requirement but if NM has a valid MAC address to
> assign to the wlan0 interface, it can be configured without making
> interface up("wlan0 up"). "ip link set dev wlan0 address XX:XX:XX:XX:XX"
> command should allow to set the mac address without making the interface
> up.
> Once the mac address is set, the wilc1000 will use that mac address [1]
> instead of the one from wilc1000 NV memory until reboot. However, after
> a reboot, if no MAC address is configured from application then wilc1000
> will use the address from its NV memory.
>
> > Is this a valid behavior and shouldn't the address already be set
> > after loading the driver?
> >
>
> Only when the interface is up(ifconfig wlan0 up), driver loads the
> firmware to wilc1000 module and after that the WID commands which allows
> to set/get the mac address from the wilc1000 works.
Is there a hard technical reason not to load the firmware and set the
HW address when
the driver is initialized and not only when its opened.
--
Heiko
>
> Regards,
> Ajay
>
> 1.
> https://github.com/torvalds/linux/blob/master/drivers/net/wireless/microchip/wilc1000/netdev.c#L600
On 2/9/23 10:48, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Thu, 9 Feb 2023 17:15:38 +0000 [email protected] wrote:
>> IIUC network manager(NM) is trying to read the MAC address and write the
>> same back to wilc1000 module without making the wlan0 interface up. right?
>>
>> Not sure about the requirement but if NM has a valid MAC address to
>> assign to the wlan0 interface, it can be configured without making
>> interface up("wlan0 up"). "ip link set dev wlan0 address XX:XX:XX:XX:XX"
>> command should allow to set the mac address without making the interface
>> up.
>> Once the mac address is set, the wilc1000 will use that mac address [1]
>> instead of the one from wilc1000 NV memory until reboot. However, after
>> a reboot, if no MAC address is configured from application then wilc1000
>> will use the address from its NV memory.
>
> netdev should be created with a valid lladdr, is there something
> wifi-specific here that'd prevalent that? The canonical flow is
> to this before registering the netdev:
>
Here it's the timing in wilc1000 by when the MAC address is available to
read from NV. NV read is available in "mac_open" net_device_ops instead
of bus probe function. I think, mostly the operations on netdev which
make use of mac address are performed after the "mac_open" (I may be
missing something).
Does it make sense to assign a random address in probe and later read
back from NV in mac_open to make use of stored value?
probe()
eth_hw_addr_random();
mac_open()
if (is_random_address())
read_mac_from_nv();
eth_hw_addr_set(ndev, addr);
Regards,
Ajay
On 2/9/23 11:40, Heiko Thiery wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi,
>
> Am Do., 9. Feb. 2023 um 18:15 Uhr schrieb <[email protected]>:
>>
>> Hi Heiko,
>>
>> On 2/8/23 07:24, Heiko Thiery wrote:
>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>
>>> Hi,
>>>
>>> I'm using the WILC1000 wifi and with NetworkManager [1] I see issues
>>> in certain situations I see problems.
>>>
>>> I was able to reduce the problem and have now found out that the cause
>>> is that the interface has the HW MAC address is 00:00:00:00:00 after
>>> startup. Only when the interface is startup (ip link set dev wlan0
>>> up), the driver sets a "valid" address.
>>>
>>
>> IIUC network manager(NM) is trying to read the MAC address and write the
>> same back to wilc1000 module without making the wlan0 interface up. right?
>
> As far as I understand, network-manager will read the "real" HW
> address. Then it sets a random
> generated HW for scanning and after that switches back to the "real" HW address.
>
> There seems to be circumstances where the wrong HW address
> (00:00:00:00:00:00) is read and stored for
> later reset after the scanning.
>
Actually, the scan operation is allowed only after the interface is up.
Probably the address was stored before any wlan operation.
>> Not sure about the requirement but if NM has a valid MAC address to
>> assign to the wlan0 interface, it can be configured without making
>> interface up("wlan0 up"). "ip link set dev wlan0 address XX:XX:XX:XX:XX"
>> command should allow to set the mac address without making the interface
>> up.
>> Once the mac address is set, the wilc1000 will use that mac address [1]
>> instead of the one from wilc1000 NV memory until reboot. However, after
>> a reboot, if no MAC address is configured from application then wilc1000
>> will use the address from its NV memory.
>>
>>> Is this a valid behavior and shouldn't the address already be set
>>> after loading the driver?
>>>
>>
>> Only when the interface is up(ifconfig wlan0 up), driver loads the
>> firmware to wilc1000 module and after that the WID commands which allows
>> to set/get the mac address from the wilc1000 works.
>
> Is there a hard technical reason not to load the firmware and set the
> HW address when
> the driver is initialized and not only when its opened.
>
AFAIK, wilc1000 flow is designed that way to load the firmware start
when the interface is up (before wlan operation are performed).
Similarly when the interface is down it will not execute to save power.
Regards,
Ajay
On Thu, 9 Feb 2023 18:51:58 +0000 [email protected] wrote:
> > netdev should be created with a valid lladdr, is there something
> > wifi-specific here that'd prevalent that? The canonical flow is
> > to this before registering the netdev:
>
> Here it's the timing in wilc1000 by when the MAC address is available to
> read from NV. NV read is available in "mac_open" net_device_ops instead
> of bus probe function. I think, mostly the operations on netdev which
> make use of mac address are performed after the "mac_open" (I may be
> missing something).
>
> Does it make sense to assign a random address in probe and later read
> back from NV in mac_open to make use of stored value?
Hard to say, I'd suspect that may be even more confusing than
starting with zeroes. There aren't any hard rules around the
addresses AFAIK, but addrs are visible to user space. So user
space will likely make assumptions based on the most commonly
observed sequence (reading real addr at probe).
Am 2023-02-09 22:07, schrieb Jakub Kicinski:
> On Thu, 9 Feb 2023 18:51:58 +0000 [email protected] wrote:
>> > netdev should be created with a valid lladdr, is there something
>> > wifi-specific here that'd prevalent that? The canonical flow is
>> > to this before registering the netdev:
>>
>> Here it's the timing in wilc1000 by when the MAC address is available
>> to
>> read from NV. NV read is available in "mac_open" net_device_ops
>> instead
>> of bus probe function. I think, mostly the operations on netdev which
>> make use of mac address are performed after the "mac_open" (I may be
>> missing something).
>>
>> Does it make sense to assign a random address in probe and later read
>> back from NV in mac_open to make use of stored value?
>
> Hard to say, I'd suspect that may be even more confusing than
> starting with zeroes. There aren't any hard rules around the
> addresses AFAIK, but addrs are visible to user space. So user
> space will likely make assumptions based on the most commonly
> observed sequence (reading real addr at probe).
Maybe we should also ask the NetworkManager guys. IMHO random
MAC address sounds bogus.
I don't understand the "we load the firmware when the interface
is brought up" thing. Esp. with network manager scanning in the
background, the firmware gets loaded so many times.
-michael
HI,
Am Do., 9. Feb. 2023 um 22:19 Uhr schrieb Michael Walle <[email protected]>:
>
> Am 2023-02-09 22:07, schrieb Jakub Kicinski:
> > On Thu, 9 Feb 2023 18:51:58 +0000 [email protected] wrote:
> >> > netdev should be created with a valid lladdr, is there something
> >> > wifi-specific here that'd prevalent that? The canonical flow is
> >> > to this before registering the netdev:
> >>
> >> Here it's the timing in wilc1000 by when the MAC address is available
> >> to
> >> read from NV. NV read is available in "mac_open" net_device_ops
> >> instead
> >> of bus probe function. I think, mostly the operations on netdev which
> >> make use of mac address are performed after the "mac_open" (I may be
> >> missing something).
> >>
> >> Does it make sense to assign a random address in probe and later read
> >> back from NV in mac_open to make use of stored value?
> >
> > Hard to say, I'd suspect that may be even more confusing than
> > starting with zeroes. There aren't any hard rules around the
> > addresses AFAIK, but addrs are visible to user space. So user
> > space will likely make assumptions based on the most commonly
> > observed sequence (reading real addr at probe).
>
> Maybe we should also ask the NetworkManager guys. IMHO random
> MAC address sounds bogus.
Maybe it would be a "workaround" with loading the firmware while
probing the device to set the real hw address.
probe()
load_fw()
read_hw_addr_from_nv()
eth_hw_addr_set(ndev, addr)
unload_fw()
mac_open()
load_fw()
mac_close()
unload_fw()
> I don't understand the "we load the firmware when the interface
> is brought up" thing. Esp. with network manager scanning in the
> background, the firmware gets loaded so many times.
Yes this is also an additional issue here.
I added Thomas and Beniamino as I hope one of them can help regarding
the network-manager questions.
>
> -michael
Heiko Thiery <[email protected]> writes:
> HI,
>
> Am Do., 9. Feb. 2023 um 22:19 Uhr schrieb Michael Walle <[email protected]>:
>>
>> Am 2023-02-09 22:07, schrieb Jakub Kicinski:
>> > On Thu, 9 Feb 2023 18:51:58 +0000 [email protected] wrote:
>> >> > netdev should be created with a valid lladdr, is there something
>> >> > wifi-specific here that'd prevalent that? The canonical flow is
>> >> > to this before registering the netdev:
>> >>
>> >> Here it's the timing in wilc1000 by when the MAC address is available
>> >> to
>> >> read from NV. NV read is available in "mac_open" net_device_ops
>> >> instead
>> >> of bus probe function. I think, mostly the operations on netdev which
>> >> make use of mac address are performed after the "mac_open" (I may be
>> >> missing something).
>> >>
>> >> Does it make sense to assign a random address in probe and later read
>> >> back from NV in mac_open to make use of stored value?
>> >
>> > Hard to say, I'd suspect that may be even more confusing than
>> > starting with zeroes. There aren't any hard rules around the
>> > addresses AFAIK, but addrs are visible to user space. So user
>> > space will likely make assumptions based on the most commonly
>> > observed sequence (reading real addr at probe).
>>
>> Maybe we should also ask the NetworkManager guys. IMHO random
>> MAC address sounds bogus.
>
> Maybe it would be a "workaround" with loading the firmware while
> probing the device to set the real hw address.
>
> probe()
> load_fw()
> read_hw_addr_from_nv()
> eth_hw_addr_set(ndev, addr)
> unload_fw()
>
> mac_open()
> load_fw()
>
> mac_close()
> unload_fw()
This is exactly what many wireless drivers already do and I recommend
that wilc1000 would do the same.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Hi Kalle,
On 2/10/23 02:25, Kalle Valo wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Heiko Thiery <[email protected]> writes:
>
>> HI,
>>
>> Am Do., 9. Feb. 2023 um 22:19 Uhr schrieb Michael Walle <[email protected]>:
>>>
>>> Am 2023-02-09 22:07, schrieb Jakub Kicinski:
>>>> On Thu, 9 Feb 2023 18:51:58 +0000 [email protected] wrote:
>>>>>> netdev should be created with a valid lladdr, is there something
>>>>>> wifi-specific here that'd prevalent that? The canonical flow is
>>>>>> to this before registering the netdev:
>>>>>
>>>>> Here it's the timing in wilc1000 by when the MAC address is available
>>>>> to
>>>>> read from NV. NV read is available in "mac_open" net_device_ops
>>>>> instead
>>>>> of bus probe function. I think, mostly the operations on netdev which
>>>>> make use of mac address are performed after the "mac_open" (I may be
>>>>> missing something).
>>>>>
>>>>> Does it make sense to assign a random address in probe and later read
>>>>> back from NV in mac_open to make use of stored value?
>>>>
>>>> Hard to say, I'd suspect that may be even more confusing than
>>>> starting with zeroes. There aren't any hard rules around the
>>>> addresses AFAIK, but addrs are visible to user space. So user
>>>> space will likely make assumptions based on the most commonly
>>>> observed sequence (reading real addr at probe).
>>>
>>> Maybe we should also ask the NetworkManager guys. IMHO random
>>> MAC address sounds bogus.
>>
>> Maybe it would be a "workaround" with loading the firmware while
>> probing the device to set the real hw address.
>>
>> probe()
>> load_fw()
>> read_hw_addr_from_nv()
>> eth_hw_addr_set(ndev, addr)
>> unload_fw()
>>
>> mac_open()
>> load_fw()
>>
>> mac_close()
>> unload_fw()
>
> This is exactly what many wireless drivers already do and I recommend
> that wilc1000 would do the same.
>
In wilc1000, the bus interface is up in probe but we don't have access
to mac address via register until the driver starts the wilc firmware
because of design limitation. This information is only available after
the MAC layer is initialized.
Regards,
Ajay
On Fri, 2023-02-10 at 19:12 +0000, [email protected] wrote:
> Hi Kalle,
>
> On 2/10/23 02:25, Kalle Valo wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> >
> > Heiko Thiery <[email protected]> writes:
> >
> > > HI,
> > >
> > > Am Do., 9. Feb. 2023 um 22:19 Uhr schrieb Michael Walle
> > > <[email protected]>:
> > > >
> > > > Am 2023-02-09 22:07, schrieb Jakub Kicinski:
> > > > > On Thu, 9 Feb 2023 18:51:58 +0000
> > > > > [email protected] wrote:
> > > > > > > netdev should be created with a valid lladdr, is there
> > > > > > > something
> > > > > > > wifi-specific here that'd prevalent that? The canonical
> > > > > > > flow is
> > > > > > > to this before registering the netdev:
> > > > > >
> > > > > > Here it's the timing in wilc1000 by when the MAC address is
> > > > > > available
> > > > > > to
> > > > > > read from NV. NV read is available in "mac_open"
> > > > > > net_device_ops
> > > > > > instead
> > > > > > of bus probe function. I think, mostly the operations on
> > > > > > netdev which
> > > > > > make use of mac address are performed after the "mac_open"
> > > > > > (I may be
> > > > > > missing something).
> > > > > >
> > > > > > Does it make sense to assign a random address in probe and
> > > > > > later read
> > > > > > back from NV in mac_open to make use of stored value?
> > > > >
> > > > > Hard to say, I'd suspect that may be even more confusing than
> > > > > starting with zeroes. There aren't any hard rules around the
> > > > > addresses AFAIK, but addrs are visible to user space. So user
> > > > > space will likely make assumptions based on the most commonly
> > > > > observed sequence (reading real addr at probe).
> > > >
> > > > Maybe we should also ask the NetworkManager guys. IMHO random
> > > > MAC address sounds bogus.
> > >
> > > Maybe it would be a "workaround" with loading the firmware while
> > > probing the device to set the real hw address.
> > >
> > > probe()
> > > load_fw()
> > > read_hw_addr_from_nv()
> > > eth_hw_addr_set(ndev, addr)
> > > unload_fw()
> > >
> > > mac_open()
> > > load_fw()
> > >
> > > mac_close()
> > > unload_fw()
> >
> > This is exactly what many wireless drivers already do and I
> > recommend
> > that wilc1000 would do the same.
> >
>
> In wilc1000, the bus interface is up in probe but we don't have
> access
> to mac address via register until the driver starts the wilc firmware
> because of design limitation. This information is only available
> after
> the MAC layer is initialized.
So... initialize the MAC layer and read the address, then stop the card
until dev open which reloads and reinits? That's what eg Atmel does
(though it has a special "read the MAC only" firmware to do that):
/* No stored firmware so load a small stub which just
tells us the MAC address */
int i;
priv->card_type = CARD_TYPE_EEPROM;
atmel_write16(dev, BSR, BSS_IRAM);
atmel_copy_to_card(dev, 0, mac_reader, sizeof(mac_reader));
atmel_set_gcr(dev, GCR_REMAP);
atmel_clear_gcr(priv->dev, 0x0040);
atmel_write16(dev, BSR, BSS_SRAM);
for (i = LOOP_RETRY_LIMIT; i; i--)
if (atmel_read16(dev, MR3) & MAC_BOOT_COMPLETE)
break;
if (i == 0) {
printk(KERN_ALERT "%s: MAC failed to boot MAC address reader.\n", dev->name);
} else {
atmel_copy_to_host(dev, addr, atmel_read16(dev, MR2), 6);
eth_hw_addr_set(dev, addr);
/* got address, now squash it again until the network
interface is opened */
if (priv->bus_type == BUS_TYPE_PCCARD)
atmel_write16(dev, GCR, 0x0060);
atmel_write16(dev, GCR, 0x0040);
rc = 1;
}
Dan
>
>
> Regards,
> Ajay
On Fri, 2023-02-10 at 15:28 -0600, Dan Williams wrote:
> >
> > In wilc1000, the bus interface is up in probe but we don't have
> > access
> > to mac address via register until the driver starts the wilc firmware
> > because of design limitation. This information is only available
> > after
> > the MAC layer is initialized.
>
> So... initialize the MAC layer and read the address, then stop the card
> until dev open which reloads and reinits? That's what eg Atmel does
For a more modern example, iwlwifi also ;-)
You should also load the firmware async, so it becomes:
probe
-> load firmware
firmware success callback
- boot device
- read information
- register with mac80211
- shut down device
mac80211 start callback
- boot device again
- etc.
johannes
Hi Ajay,
Am Mi., 15. Feb. 2023 um 21:14 Uhr schrieb Johannes Berg
<[email protected]>:
>
> On Fri, 2023-02-10 at 15:28 -0600, Dan Williams wrote:
> > >
> > > In wilc1000, the bus interface is up in probe but we don't have
> > > access
> > > to mac address via register until the driver starts the wilc firmware
> > > because of design limitation. This information is only available
> > > after
> > > the MAC layer is initialized.
> >
> > So... initialize the MAC layer and read the address, then stop the card
> > until dev open which reloads and reinits? That's what eg Atmel does
>
> For a more modern example, iwlwifi also ;-)
>
> You should also load the firmware async, so it becomes:
>
> probe
> -> load firmware
>
> firmware success callback
> - boot device
> - read information
> - register with mac80211
> - shut down device
>
> mac80211 start callback
> - boot device again
> - etc.
Do you have a meaning about that? That sounds like a viable solution.
What do you think?
--
Heiko
> johannes
Hi Heiko,
On 2/23/23 03:44, Heiko Thiery wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi Ajay,
>
> Am Mi., 15. Feb. 2023 um 21:14 Uhr schrieb Johannes Berg
> <[email protected]>:
>>
>> On Fri, 2023-02-10 at 15:28 -0600, Dan Williams wrote:
>>>>
>>>> In wilc1000, the bus interface is up in probe but we don't have
>>>> access
>>>> to mac address via register until the driver starts the wilc firmware
>>>> because of design limitation. This information is only available
>>>> after
>>>> the MAC layer is initialized.
>>>
>>> So... initialize the MAC layer and read the address, then stop the card
>>> until dev open which reloads and reinits? That's what eg Atmel does
>>
>> For a more modern example, iwlwifi also ;-)
>>
>> You should also load the firmware async, so it becomes:
>>
>> probe
>> -> load firmware
>>
>> firmware success callback
>> - boot device
>> - read information
>> - register with mac80211
>> - shut down device
>>
>> mac80211 start callback
>> - boot device again
>> - etc.
>
> Do you have a meaning about that? That sounds like a viable solution.
> What do you think?
>
Yeah, loading the firmware async approach looks good but wilc
init/deinit states are coupled with mac_open/mac_close so it would need
some restructuring.
Firstly, I am checking on reading the MAC address register by just
bringing the SDIO bus interface up so, there is no need to fetch it
using WID's command, which is only available from the firmware. I think
it is possible but I need to check more.
If the first approach didn't work, then I am going to try loading the
firmware in probe as suggested before.
Regards,
Ajay