RFC: Allow multiple STA connected to same AP.

RFC: Allow multiple STA connected to same AP.

[Ben Greear]

With the patch below against latest wireless-testing, I can create two
STA on the same ath9k phy0 and have them send traffic to each other.
I tested only un-encrypted STAs at this point.

It seems the tx logic hangs after a bit, so there are still issues, but
I'm not sure if that is something introduced by my patch or some
existing ath9k bug.

Please let me know if this is a viable approach.

Thanks,
Ben


diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 4935b84..21401c9 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -44,6 +44,8 @@ void ieee80211_configure_filter(struct ieee80211_local *local)
         u64 mc;
         unsigned int changed_flags;
         unsigned int new_flags = 0;
+       int avifs = 0;
+       struct ieee80211_sub_if_data *sdata;

         if (atomic_read(&local->iff_promiscs))
                 new_flags |= FIF_PROMISC_IN_BSS;
@@ -78,6 +80,22 @@ void ieee80211_configure_filter(struct ieee80211_local *local)
         /* be a bit nasty */
         new_flags |= (1<<31);

+       /* If we have more than one virtual station, turn on PROMISC_IN_BSS
+        *  --Ben
+        */
+       list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+               if (!sdata->dev || !netif_running(sdata->dev))
+                       continue;
+
+               if (sdata->vif.type == NL80211_IFTYPE_STATION) {
+                       avifs++;
+                       if (avifs > 1)
+                               break;
+               }
+       }
+       if (avifs > 1)
+               new_flags |= FIF_PROMISC_IN_BSS;
+
         drv_configure_filter(local, changed_flags, &new_flags, mc);

         WARN_ON(new_flags & (1<<31));
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 687077e..db751f1 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -440,7 +440,7 @@ int sta_info_insert_rcu(struct sta_info *sta) __acquires(RCU)

         spin_lock_irqsave(&local->sta_lock, flags);
         /* check if STA exists already */
-       if (sta_info_get_bss(sdata, sta->sta.addr)) {
+       if (sta_info_get(sdata, sta->sta.addr)) {
                 spin_unlock_irqrestore(&local->sta_lock, flags);
                 mutex_unlock(&local->sta_mtx);
                 rcu_read_lock();

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: RFC: Allow multiple STA connected to same AP.

[Johannes Berg]

On Thu, 09 Sep 2010 15:03:40 -0700, Ben Greear <greearb@candelatech.com>
wrote:
> With the patch below against latest wireless-testing, I can create two
> STA on the same ath9k phy0 and have them send traffic to each other.
> I tested only un-encrypted STAs at this point.

After connecting them to the same AP, presumably?

> It seems the tx logic hangs after a bit, so there are still issues, but
> I'm not sure if that is something introduced by my patch or some
> existing ath9k bug.
> 
> Please let me know if this is a viable approach.

> +       /* If we have more than one virtual station, turn on
PROMISC_IN_BSS
> +        *  --Ben
> +        */
> +       list_for_each_entry_rcu(sdata, &local->interfaces, list) {
> +               if (!sdata->dev || !netif_running(sdata->dev))
> +                       continue;
> +
> +               if (sdata->vif.type == NL80211_IFTYPE_STATION) {
> +                       avifs++;
> +                       if (avifs > 1)
> +                               break;
> +               }
> +       }
> +       if (avifs > 1)
> +               new_flags |= FIF_PROMISC_IN_BSS;
> +

This seems weird. Why do you need to be *promisc* within a given
BSS if you are multiple stations? You already have two MAC addrs
that you need to accept, so that should be fine, i.e. you don't
need to be promisc in the BSS. If ath9k needs to, it should be in
the driver. So NAK on this change for sure.

> @@ -440,7 +440,7 @@ int sta_info_insert_rcu(struct sta_info *sta)
> __acquires(RCU)
> 
>          spin_lock_irqsave(&local->sta_lock, flags);
>          /* check if STA exists already */
> -       if (sta_info_get_bss(sdata, sta->sta.addr)) {
> +       if (sta_info_get(sdata, sta->sta.addr)) {

I don't understand this change. These functions are the same
if sdata is in managed mode.

johannes

Re: RFC: Allow multiple STA connected to same AP.

[Ben Greear]

On 09/09/2010 03:51 PM, Johannes Berg wrote:
>
> On Thu, 09 Sep 2010 15:03:40 -0700, Ben Greear<greearb@candelatech.com>
> wrote:
>> With the patch below against latest wireless-testing, I can create two
>> STA on the same ath9k phy0 and have them send traffic to each other.
>> I tested only un-encrypted STAs at this point.
>
> After connecting them to the same AP, presumably?

Yes, though hopefully it would also work connecting to different
APs so long as the APs can communicate properly.

I've also been testing WPA..and that definitely doesn't work
yet.  Seems they fail to authenticate with the AP, though they
do talk and send EAPOL pkts back and forth.

>
>> It seems the tx logic hangs after a bit, so there are still issues, but
>> I'm not sure if that is something introduced by my patch or some
>> existing ath9k bug.
>>
>> Please let me know if this is a viable approach.
>
>> +       /* If we have more than one virtual station, turn on
> PROMISC_IN_BSS
>> +        *  --Ben
>> +        */
>> +       list_for_each_entry_rcu(sdata,&local->interfaces, list) {
>> +               if (!sdata->dev || !netif_running(sdata->dev))
>> +                       continue;
>> +
>> +               if (sdata->vif.type == NL80211_IFTYPE_STATION) {
>> +                       avifs++;
>> +                       if (avifs>  1)
>> +                               break;
>> +               }
>> +       }
>> +       if (avifs>  1)
>> +               new_flags |= FIF_PROMISC_IN_BSS;
>> +
>
> This seems weird. Why do you need to be *promisc* within a given
> BSS if you are multiple stations? You already have two MAC addrs
> that you need to accept, so that should be fine, i.e. you don't
> need to be promisc in the BSS. If ath9k needs to, it should be in
> the driver. So NAK on this change for sure.

This seems to let the driver receive pkts for the 'real' mac on
wlan0 and also the arbitrary mac on the second STA interface.

If there is supposed to be some other way for the driver to
figure out it's supposed to go promisc, please let me know and
I'll try to debug it.

>> @@ -440,7 +440,7 @@ int sta_info_insert_rcu(struct sta_info *sta)
>> __acquires(RCU)
>>
>>           spin_lock_irqsave(&local->sta_lock, flags);
>>           /* check if STA exists already */
>> -       if (sta_info_get_bss(sdata, sta->sta.addr)) {
>> +       if (sta_info_get(sdata, sta->sta.addr)) {
>
> I don't understand this change. These functions are the same
> if sdata is in managed mode.

The _bss method returns anything in same bss or with same
sdata.  The sta_info_get only returns if MAC and sdata match.

W/out this change, the second STA fails this check because it
has same BSS as the first one.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: RFC: Allow multiple STA connected to same AP.

[Johannes Berg]

On Thu, 09 Sep 2010 16:01:27 -0700, Ben Greear <greearb@candelatech.com>
wrote:
>>> +       /* If we have more than one virtual station, turn on
>> PROMISC_IN_BSS
>>> +        *  --Ben
>>> +        */
>>> +       list_for_each_entry_rcu(sdata,&local->interfaces, list) {
>>> +               if (!sdata->dev || !netif_running(sdata->dev))
>>> +                       continue;
>>> +
>>> +               if (sdata->vif.type == NL80211_IFTYPE_STATION) {
>>> +                       avifs++;
>>> +                       if (avifs>  1)
>>> +                               break;
>>> +               }
>>> +       }
>>> +       if (avifs>  1)
>>> +               new_flags |= FIF_PROMISC_IN_BSS;
>>> +
>>
>> This seems weird. Why do you need to be *promisc* within a given
>> BSS if you are multiple stations? You already have two MAC addrs
>> that you need to accept, so that should be fine, i.e. you don't
>> need to be promisc in the BSS. If ath9k needs to, it should be in
>> the driver. So NAK on this change for sure.
> 
> This seems to let the driver receive pkts for the 'real' mac on
> wlan0 and also the arbitrary mac on the second STA interface.
> 
> If there is supposed to be some other way for the driver to
> figure out it's supposed to go promisc, please let me know and
> I'll try to debug it.

But you can do the exact same thing in the driver. I just think
it doesn't belong into mac80211, since mac80211 doesn't need you
to be promisc, it just needs you to receive frames for those *two*
addresses that you've added. And you already know about those,
since you were notified about the interfaces going up and down
and associating etc.

>>> @@ -440,7 +440,7 @@ int sta_info_insert_rcu(struct sta_info *sta)
>>> __acquires(RCU)
>>>
>>>           spin_lock_irqsave(&local->sta_lock, flags);
>>>           /* check if STA exists already */
>>> -       if (sta_info_get_bss(sdata, sta->sta.addr)) {
>>> +       if (sta_info_get(sdata, sta->sta.addr)) {
>>
>> I don't understand this change. These functions are the same
>> if sdata is in managed mode.
> 
> The _bss method returns anything in same bss or with same
> sdata.  The sta_info_get only returns if MAC and sdata match.
> 
> W/out this change, the second STA fails this check because it
> has same BSS as the first one.

I'll have to look at this, I may misremember the way this works
internally, but this change will certainly break some things
since it'd allow adding the same STA to multiple VLANs that sit
on top of the same BSS.

johannes

Re: RFC: Allow multiple STA connected to same AP.

[Ben Greear]

On 09/09/2010 04:08 PM, Johannes Berg wrote:
>
> On Thu, 09 Sep 2010 16:01:27 -0700, Ben Greear<greearb@candelatech.com>
> wrote:
>>>> +       /* If we have more than one virtual station, turn on
>>> PROMISC_IN_BSS
>>>> +        *  --Ben
>>>> +        */
>>>> +       list_for_each_entry_rcu(sdata,&local->interfaces, list) {
>>>> +               if (!sdata->dev || !netif_running(sdata->dev))
>>>> +                       continue;
>>>> +
>>>> +               if (sdata->vif.type == NL80211_IFTYPE_STATION) {
>>>> +                       avifs++;
>>>> +                       if (avifs>   1)
>>>> +                               break;
>>>> +               }
>>>> +       }
>>>> +       if (avifs>   1)
>>>> +               new_flags |= FIF_PROMISC_IN_BSS;
>>>> +
>>>
>>> This seems weird. Why do you need to be *promisc* within a given
>>> BSS if you are multiple stations? You already have two MAC addrs
>>> that you need to accept, so that should be fine, i.e. you don't
>>> need to be promisc in the BSS. If ath9k needs to, it should be in
>>> the driver. So NAK on this change for sure.
>>
>> This seems to let the driver receive pkts for the 'real' mac on
>> wlan0 and also the arbitrary mac on the second STA interface.
>>
>> If there is supposed to be some other way for the driver to
>> figure out it's supposed to go promisc, please let me know and
>> I'll try to debug it.
>
> But you can do the exact same thing in the driver. I just think
> it doesn't belong into mac80211, since mac80211 doesn't need you
> to be promisc, it just needs you to receive frames for those *two*
> addresses that you've added. And you already know about those,
> since you were notified about the interfaces going up and down
> and associating etc.

After some more debugging, it seems at least part of the problem is that
ath9k uses ieee80211_iterate_active_interfaces_atomic to determine which
MACs to add to the BSSID mask.  But, before things are associated properly,
sdata isn't 'running', so ath9k doesn't calc the mask properly.

Looks a bit like chicken-and-egg issue, and for some reason, it doesn't
seem to happen with WPA (but then, WPA doesn't authenticate properly either).

Maybe a new iterator is needed to get interfaces that want to go active
(maybe by determining if sdata->dev is 'UP') ?

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: RFC: Allow multiple STA connected to same AP.

[Johannes Berg]

On Thu, 09 Sep 2010 21:52:31 -0700, Ben Greear <greearb@candelatech.com>
wrote:

> After some more debugging, it seems at least part of the problem is that
> ath9k uses ieee80211_iterate_active_interfaces_atomic to determine which
> MACs to add to the BSSID mask.  But, before things are associated
properly,
> sdata isn't 'running', so ath9k doesn't calc the mask properly.
> 
> Looks a bit like chicken-and-egg issue, and for some reason, it doesn't
> seem to happen with WPA (but then, WPA doesn't authenticate properly
> either).
> 
> Maybe a new iterator is needed to get interfaces that want to go active
> (maybe by determining if sdata->dev is 'UP') ?

Err, that's exactly what the active interfaces iterator does. sdata is
in fact 'running' as soon as the interface is UP.

johannes

Re: RFC: Allow multiple STA connected to same AP.

[Ben Greear]

On 09/10/2010 09:37 AM, Johannes Berg wrote:
>
> On Thu, 09 Sep 2010 21:52:31 -0700, Ben Greear<greearb@candelatech.com>
> wrote:
>
>> After some more debugging, it seems at least part of the problem is that
>> ath9k uses ieee80211_iterate_active_interfaces_atomic to determine which
>> MACs to add to the BSSID mask.  But, before things are associated
> properly,
>> sdata isn't 'running', so ath9k doesn't calc the mask properly.
>>
>> Looks a bit like chicken-and-egg issue, and for some reason, it doesn't
>> seem to happen with WPA (but then, WPA doesn't authenticate properly
>> either).
>>
>> Maybe a new iterator is needed to get interfaces that want to go active
>> (maybe by determining if sdata->dev is 'UP') ?
>
> Err, that's exactly what the active interfaces iterator does. sdata is
> in fact 'running' as soon as the interface is UP.

Maybe that bssidmask logic runs before sdata is actually brought up
then.  I'll keep poking at things.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com