Re: [PATCH] nl80211: Add association id to station response

[Arend van Spriel <arend.vanspriel@broadcom.com>]

On 5/22/2017 8:17 AM, Johannes Berg wrote:
> Hi Kirill,
> 
> First of all, please read
> 
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> 
> particularly
> 
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
> 
> is important
> 
> On Sun, 2017-05-21 at 11:45 +0300, Kirill Obukhov wrote:
>> There are unique association id attribute must be passed in station
>> add request.

This sentence is a bit quirky. It is sort of clear what is meant, but 
maybe rephrase:

When adding a station user-space using CMD_NEW_STATION user-space needs 
to provide a unique association ID. However, we have no way...

>> But we have no way to retrieve ids already in use. To solve issue
>> association id attribute added to station response.

It would also be nice to give an example when you want to add a new 
station (maybe it is just me being curious ;-) ).

Regards,
Arend

> 
> This sound fine.
> 
> However, it'd be good to mention that you're also adding it to
> mac80211, or preferably even split the patch into two, one to add the
> feature and one to make use of it in mac80211.
> 
> Once you do that, you'll quite likely also notice that with just the
> first of the patches, the thing is broken and always reports a 0 AID.
> 
>>          if (nla_put_u32(msg, NL80211_ATTR_IFINDEX, dev->ifindex) ||
>>              nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, mac_addr) ||
>> +           nla_put_u16(msg, NL80211_ATTR_STA_AID, sinfo->aid) ||
> 
> That's because you need to make this conditional on the right filled
> flag.
> 
> johannes
>