diff mbox

nl80211: Add association id to station response

Message ID CAP3TkT3VE6FWBXm0K4=-pWLrmsCk4a7wuqc24KdcgYNBRQRyoA@mail.gmail.com (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show

Commit Message

Kirill Obukhov May 21, 2017, 8:45 a.m. UTC
There are unique association id attribute must be passed in station add request.
But we have no way to retrieve ids already in use. To solve issue
association id attribute added to station response.
---

Comments

Johannes Berg May 22, 2017, 6:17 a.m. UTC | #1
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.
> But we have no way to retrieve ids already in use. To solve issue
> association id attribute added to station response.

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
Arend Van Spriel May 22, 2017, 9:27 a.m. UTC | #2
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
>
diff mbox

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index b083e6c..3c270a5 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1098,6 +1098,7 @@  struct cfg80211_tid_stats {
  *
  * @filled: bitflag of flags using the bits of &enum nl80211_sta_info to
  *     indicate the relevant values in this struct for them
+ * @aid: station association id
  * @connected_time: time(in secs) since a station is last connected
  * @inactive_time: time since last station activity (tx/rx) in milliseconds
  * @rx_bytes: bytes (size of MPDUs) received from this station
@@ -1146,6 +1147,7 @@  struct cfg80211_tid_stats {
  */
 struct station_info {
        u64 filled;
+       u16 aid;
        u32 connected_time;
        u32 inactive_time;
        u64 rx_bytes;
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 7cdf7a8..ce41299 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -2091,6 +2091,7 @@  void sta_set_sinfo(struct sta_info *sta, struct
station_info *sinfo)
                sinfo->filled |= BIT(NL80211_STA_INFO_BEACON_LOSS);
        }

+       sinfo->aid = sta->sta.aid;
        sinfo->connected_time = ktime_get_seconds() - sta->last_connected;
        sinfo->inactive_time =
                jiffies_to_msecs(jiffies - ieee80211_sta_last_active(sta));
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index c3bc9da..b08df75 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4303,6 +4303,7 @@  static int nl80211_send_station(struct sk_buff
*msg, u32 cmd, u32 portid,

        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) ||
            nla_put_u32(msg, NL80211_ATTR_GENERATION, sinfo->generation))
                goto nla_put_failure;