On 8/24/2023 3:03 PM, Johannes Berg wrote:
On Thu, 2023-08-24 at 10:26 +0800, Wen Gong wrote:
On 8/23/2023 2:34 PM, Johannes Berg wrote:
On Wed, 2023-08-23 at 10:28 +0800, Wen Gong wrote:
/* need to have local link addresses for MLO connections */
WARN_ON(cr.ap_mld_addr && !cr.links[link_id].addr);
makes no sense anymore. Not sure if that's the only one.
After this patch, the cr.links[link_id].addr will be a valid local link
address from
struct cfg80211_rx_assoc_resp, so I think it is not needed remove now.
You don't understand.
The issue is that it's set the line above.
cr.links[link_id].addr = data->links[link_id].addr;
/* need to have local link addresses for MLO connections */
WARN_ON(cr.ap_mld_addr && !cr.links[link_id].addr);
But look at that! What values can cr.links[link_id].addr get? Note how
it's a pointer - assigned from an array.
Oh, I know it now. the cr.links[link_id].addr will always NOT 0 because
it is pointer
to an array "u8 addr[ETH_ALEN]" of struct cfg80211_rx_assoc_resp.
Yep.
So maybe we can choose one of the 2 things to do:
1. remove the "WARN_ON(cr.ap_mld_addr && !cr.links[link_id].addr);"
2. change like this:
WARN_ON(cr.ap_mld_addr && !is_valid_ether_addr(cr.links[link_id].addr));
I think we should check that it's valid, because if you don't fill it,
it'll (hopefully) point to zeroed data somewhere.
johannes
OK. So I will change it in next version.
