Skip to contentSkip to conversation

ath9k, multiple stations, and AMPDUs

ath9k-devel Ben Greear 3 participants 20 messages Sep 21, 2010 – Sep 23, 2010

Subsystems (2)
lore
Share & tools
lore

Ben Greear

author
#0 lore raw

I think I have narrowed down some problems I see when I create two STA interfaces on the same radio and associate with the same AP.

I'm using wireless-testing plus some patches to the rx logic I posted earlier.

It appears that the ath9k NIC does not transport AMPDUs properly, and after a few seconds, it's backlog is exhausted and the queues are stopped in ath_tx_start.

debugfs seems to agree:

[root at atom ~]# cat /debug/ath9k/phy0/xmit
BE BK VI VO

MPDUs Queued:                2          0         0        85
MPDUs Completed:             2          0         0        85
Aggregates:                  0          0         0         0
AMPDUs Queued:             144          0         0         0
AMPDUs Completed:           20          0         0         0
AMPDUs Retried:             18          0         0         0
AMPDUs XRetried:             0          0         0         0
FIFO Underrun:               0          0         0         0
TXOP Exceeded:               0          0         0         0
TXTIMER Expiry:              0          0         0         0
DESC CFG Error:              0          0         0         0
DATA Underrun:               0          0         0         0
DELIM Underrun:              0          0         0         0

[root at atom ~]#

I can still receive packets on the interface, but nothing is transmitted.

I tested against an a/b/g AP (instead of N), and it ran clean for many minutes.

Any ideas how to debug this further?

Thanks,
Ben

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#1 lore raw
Quoted 10 lines · Ben Greear

I think I have narrowed down some problems I see when I create two STA interfaces on the same radio and associate with the same AP.

I'm using wireless-testing plus some patches to the rx logic I posted earlier.

It appears that the ath9k NIC does not transport AMPDUs properly, and after a few seconds, it's backlog is exhausted and the queues are stopped in ath_tx_start.

ath9k currently looks up the sta by hw and not by vif on tx completion. That completely breaks aggregation in such a setup. Unfortunately I don't know an easy way to fix this yet.

This will be fixed by my sw aggregation rewrite, but I don't know when I'll get that completed yet.

- Felix

author
#2 lore raw
Quoted 11 lines · Felix Fietkau
Quoted 10 lines · Ben Greear

I think I have narrowed down some problems I see when I create two STA interfaces on the same radio and associate with the same AP.

I'm using wireless-testing plus some patches to the rx logic I posted earlier.

It appears that the ath9k NIC does not transport AMPDUs properly, and after a few seconds, it's backlog is exhausted and the queues are stopped in ath_tx_start.

Quoted 6 lines

ath9k currently looks up the sta by hw and not by vif on tx completion. That completely breaks aggregation in such a setup. Unfortunately I don't know an easy way to fix this yet.

This will be fixed by my sw aggregation rewrite, but I don't know when I'll get that completed yet.

If you have any more details on this, please let me know. I'm going to attempt to fix it...I certainly have a good test case :)

Ben

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#3 lore raw
Quoted 16 lines · Ben Greear
Quoted 15 lines · Felix Fietkau
Quoted 10 lines · Ben Greear

I think I have narrowed down some problems I see when I create two STA interfaces on the same radio and associate with the same AP.

I'm using wireless-testing plus some patches to the rx logic I posted earlier.

It appears that the ath9k NIC does not transport AMPDUs properly, and after a few seconds, it's backlog is exhausted and the queues are stopped in ath_tx_start.

ath9k currently looks up the sta by hw and not by vif on tx completion. That completely breaks aggregation in such a setup. Unfortunately I don't know an easy way to fix this yet.

Quoted 5 lines
Quoted 2 lines

This will be fixed by my sw aggregation rewrite, but I don't know when I'll get that completed yet.

If you have any more details on this, please let me know. I'm going to attempt to fix it...I certainly have a good test case :)

ath_tx_complete_aggr completes an A-MPDU frame, which typically triggers the release of the next A-MPDU to the hw queue.
To keep track of the Block ACK window, it needs to look up the TID, for which it needs a STA pointer. At that level, the driver typically doesn't have access to the vif.

It might be possible to fix this by adding another sta lookup helper function in mac80211 that takes another address argument for the BSSID, so that it can get the sta entry for the correct vif. I don't know if Johannes wants something like that though.

- Felix

maintainer
#4 lore raw
Quoted 8 lines · Felix Fietkau

To keep track of the Block ACK window, it needs to look up the TID, for which it needs a STA pointer. At that level, the driver typically doesn't have access to the vif.

It might be possible to fix this by adding another sta lookup helper function in mac80211 that takes another address argument for the BSSID, so that it can get the sta entry for the correct vif. I don't know if Johannes wants something like that though.

Seems simple enough, and if it fixes things ... for iwlwifi I fixed the driver to keep track of the vif properly, though more work it does make a lot more things better than just this.

johannes

maintainer
#5 lore raw
In reply to Felix Fietkau
Quoted 4 lines · Felix Fietkau

It might be possible to fix this by adding another sta lookup helper function in mac80211 that takes another address argument for the BSSID, so that it can get the sta entry for the correct vif. I don't know if Johannes wants something like that though.

Actually, you want the RA, not the BSSID.

johannes

author
#6 lore raw
In reply to Felix Fietkau
Quoted 10 lines · Felix Fietkau
Quoted 2 lines · Ben Greear

If you have any more details on this, please let me know. I'm going to attempt to fix it...I certainly have a good test case :)

ath_tx_complete_aggr completes an A-MPDU frame, which typically triggers the release of the next A-MPDU to the hw queue.
To keep track of the Block ACK window, it needs to look up the TID, for which it needs a STA pointer. At that level, the driver typically doesn't have access to the vif.

Quoted 4 lines

It might be possible to fix this by adding another sta lookup helper function in mac80211 that takes another address argument for the BSSID, so that it can get the sta entry for the correct vif. I don't know if Johannes wants something like that though.

Could we just poke a pointer to the STA into the ath_buf structure?

Thanks,
Ben

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#7 lore raw
Quoted 11 lines · Ben Greear
Quoted 10 lines · Felix Fietkau
Quoted 2 lines · Ben Greear

If you have any more details on this, please let me know. I'm going to attempt to fix it...I certainly have a good test case :)

ath_tx_complete_aggr completes an A-MPDU frame, which typically triggers the release of the next A-MPDU to the hw queue.
To keep track of the Block ACK window, it needs to look up the TID, for which it needs a STA pointer. At that level, the driver typically doesn't have access to the vif.

Quoted 6 lines
Quoted 4 lines

It might be possible to fix this by adding another sta lookup helper function in mac80211 that takes another address argument for the BSSID, so that it can get the sta entry for the correct vif. I don't know if Johannes wants something like that though.

Could we just poke a pointer to the STA into the ath_buf structure?

No, that doesn't work because of RCU.

- Felix

author
#8 lore raw
Quoted 12 lines · Felix Fietkau
Quoted 11 lines · Ben Greear
Quoted 10 lines · Felix Fietkau
Quoted 2 lines · Ben Greear

If you have any more details on this, please let me know. I'm going to attempt to fix it...I certainly have a good test case :)

ath_tx_complete_aggr completes an A-MPDU frame, which typically triggers the release of the next A-MPDU to the hw queue.
To keep track of the Block ACK window, it needs to look up the TID, for which it needs a STA pointer. At that level, the driver typically doesn't have access to the vif.

Quoted 7 lines
Quoted 6 lines
Quoted 4 lines

It might be possible to fix this by adding another sta lookup helper function in mac80211 that takes another address argument for the BSSID, so that it can get the sta entry for the correct vif. I don't know if Johannes wants something like that though.

Could we just poke a pointer to the STA into the ath_buf structure?

No, that doesn't work because of RCU.

Would it also be bad to use skb->dev to find an STA?

Thanks,
Ben

Quoted 6 lines

- Felix
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#9 lore raw
Quoted 18 lines · Ben Greear
Quoted 17 lines · Felix Fietkau
Quoted 16 lines · Ben Greear
Quoted 14 lines · Felix Fietkau
Quoted 2 lines · Ben Greear

If you have any more details on this, please let me know. I'm going to attempt to fix it...I certainly have a good test case :)

ath_tx_complete_aggr completes an A-MPDU frame, which typically triggers the release of the next A-MPDU to the hw queue.
To keep track of the Block ACK window, it needs to look up the TID, for which it needs a STA pointer. At that level, the driver typically doesn't have access to the vif.

It might be possible to fix this by adding another sta lookup helper function in mac80211 that takes another address argument for the BSSID, so that it can get the sta entry for the correct vif. I don't know if Johannes wants something like that though.

Quoted 4 lines
Quoted 2 lines
Quoted 1 line

Could we just poke a pointer to the STA into the ath_buf structure?

No, that doesn't work because of RCU.

Would it also be bad to use skb->dev to find an STA?

It would be bad to keep a STA pointer around anywhere in the skb or the ath_buf. You can only use it within a rcu_read_lock()/rcu_read_unlock() pair.

- Felix

maintainer
#10 lore raw
In reply to Felix Fietkau
Quoted 3 lines · Felix Fietkau
Quoted 1 line

Could we just poke a pointer to the STA into the ath_buf structure?

No, that doesn't work because of RCU.

Well, it could work, if you walk all the structures upon sta_notify and remove now stale pointers (or just drop the frames or something).

johannes

#11 lore raw
Quoted 8 lines · Johannes Berg
Quoted 3 lines · Felix Fietkau
Quoted 1 line

Could we just poke a pointer to the STA into the ath_buf structure?

No, that doesn't work because of RCU.

Well, it could work, if you walk all the structures upon sta_notify and remove now stale pointers (or just drop the frames or something).

I think it would be much better to just add the helper function that checks the RA on STA lookup. Keeps things simple, especially since nothing else in the tx path needs the vif.

- Felix

author
#12 lore raw
Quoted 5 lines · Felix Fietkau
Quoted 4 lines · Johannes Berg
Quoted 1 line · Felix Fietkau
Quoted 1 line

Could we just poke a pointer to the STA into the ath_buf structure?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

No, that doesn't work because of RCU.

Well, it could work, if you walk all the structures upon sta_notify and remove now stale pointers (or just drop the frames or something).

I think it would be much better to just add the helper function that checks the RA on STA lookup. Keeps things simple, especially since nothing else in the tx path needs the vif.

How about this. Seems to do the trick on my system:

drivers/net/wireless/ath/ath9k/xmit.c +1 -2
index 85a7323..09815a1 100644
@@ -328,8 +328,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
 
         rcu_read_lock();
 
-       /* XXX: use ieee80211_find_sta! */
-       sta = ieee80211_find_sta_by_hw(hw, hdr->addr1);
+       sta = tx_info->control.sta;
         if (!sta) {
                 rcu_read_unlock();
 

Thanks,
Ben

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#13 lore raw
Quoted 22 lines · Ben Greear
Quoted 5 lines · Felix Fietkau
Quoted 4 lines · Johannes Berg
Quoted 1 line · Felix Fietkau
Quoted 1 line

Could we just poke a pointer to the STA into the ath_buf structure?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

No, that doesn't work because of RCU.

Well, it could work, if you walk all the structures upon sta_notify and remove now stale pointers (or just drop the frames or something).

I think it would be much better to just add the helper function that checks the RA on STA lookup. Keeps things simple, especially since nothing else in the tx path needs the vif.

How about this. Seems to do the trick on my system:

drivers/net/wireless/ath/ath9k/xmit.c +0 -0
index 85a7323..09815a1 100644

drivers/net/wireless/ath/ath9k/xmit.c static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq, · line 331

Quoted 6 lines
drivers/net/wireless/ath/ath9k/xmit.c +1 -2
@@ -328,8 +328,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
 
         rcu_read_lock();
 
-       /* XXX: use ieee80211_find_sta! */
-       sta = ieee80211_find_sta_by_hw(hw, hdr->addr1);
+       sta = tx_info->control.sta;

As I mentioned in another email: at the time we get the tx status report, we have to consider the sta pointer stale. It may or may not still be valid.

- Felix

author
#14 lore raw
Quoted 24 lines · Felix Fietkau
Quoted 23 lines · Ben Greear
Quoted 5 lines · Felix Fietkau
Quoted 4 lines · Johannes Berg
Quoted 1 line · Felix Fietkau
Quoted 1 line

Could we just poke a pointer to the STA into the ath_buf structure?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

No, that doesn't work because of RCU.

Well, it could work, if you walk all the structures upon sta_notify and remove now stale pointers (or just drop the frames or something).

I think it would be much better to just add the helper function that checks the RA on STA lookup. Keeps things simple, especially since nothing else in the tx path needs the vif.

How about this. Seems to do the trick on my system:

drivers/net/wireless/ath/ath9k/xmit.c +0 -0
index 85a7323..09815a1 100644
@@ -328,8 +328,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
 
Quoted 8 lines

drivers/net/wireless/ath/ath9k/xmit.c static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq, · line 331

Quoted 5 lines
drivers/net/wireless/ath/ath9k/xmit.c +1 -2
          rcu_read_lock();
 
-       /* XXX: use ieee80211_find_sta! */
-       sta = ieee80211_find_sta_by_hw(hw, hdr->addr1);
+       sta = tx_info->control.sta;

As I mentioned in another email: at the time we get the tx status report, we have to consider the sta pointer stale. It may or may not still be valid.

How about this one. I think it ensures that the sta will never be stale, since it flushes the tx queue on vif removal. Minimal testing shows it working, but of course I might be missing something.

drivers/net/wireless/ath/ath9k/main.c +6 -0
index 8b327bc..8485729 100644
@@ -1459,6 +1459,12 @@ static void ath9k_remove_interface(struct ieee80211_hw *hw,
 
         mutex_lock(&sc->mutex);
 
+       /* Make sure we have no outstanding packets that might reference
+        * the vif.  This way, we can always reference tx_info->control.sta
+        * in the tx_complete logic.
+        */
+       ath_drain_all_txq(sc, false);
+
         /* Stop ANI */
         sc->sc_flags &= ~SC_OP_ANI_RUN;
         del_timer_sync(&common->ani.timer);
drivers/net/wireless/ath/ath9k/xmit.c +1 -2
index 85a7323..09815a1 100644
@@ -328,8 +328,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
 
         rcu_read_lock();
 
-       /* XXX: use ieee80211_find_sta! */
-       sta = ieee80211_find_sta_by_hw(hw, hdr->addr1);
+       sta = tx_info->control.sta;
         if (!sta) {
                 rcu_read_unlock();
 

Thanks,
Ben

Quoted 2 lines

- Felix

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
#15 lore raw
Quoted 27 lines · Ben Greear
Quoted 26 lines · Felix Fietkau
Quoted 25 lines · Ben Greear
Quoted 5 lines · Felix Fietkau
Quoted 4 lines · Johannes Berg
Quoted 1 line · Felix Fietkau
Quoted 1 line

Could we just poke a pointer to the STA into the ath_buf structure?

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

No, that doesn't work because of RCU.

Well, it could work, if you walk all the structures upon sta_notify and remove now stale pointers (or just drop the frames or something).

I think it would be much better to just add the helper function that checks the RA on STA lookup. Keeps things simple, especially since nothing else in the tx path needs the vif.

How about this. Seems to do the trick on my system:

drivers/net/wireless/ath/ath9k/xmit.c +0 -0
index 85a7323..09815a1 100644
@@ -328,8 +328,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
 
          rcu_read_lock();
 
Quoted 8 lines
Quoted 6 lines

drivers/net/wireless/ath/ath9k/xmit.c static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq, · line 331

Quoted 3 lines
drivers/net/wireless/ath/ath9k/xmit.c +1 -2
-       /* XXX: use ieee80211_find_sta! */
-       sta = ieee80211_find_sta_by_hw(hw, hdr->addr1);
+       sta = tx_info->control.sta;

As I mentioned in another email: at the time we get the tx status report, we have to consider the sta pointer stale. It may or may not still be valid.

How about this one. I think it ensures that the sta will never be stale,

No, it doesn't. At least not in AP mode.

Quoted 2 lines

since it flushes the tx queue on vif removal. Minimal testing shows it working, but of course I might be missing something.

In AP mode, a vif has multiple sta. And draining the queue when a sta gets removed is not a good idea.

- Felix

author
#16 lore raw
Quoted 7 lines · Felix Fietkau
Quoted 6 lines · Ben Greear
Quoted 3 lines · Felix Fietkau

As I mentioned in another email: at the time we get the tx status report, we have to consider the sta pointer stale. It may or may not still be valid.

Quoted 7 lines
Quoted 1 line

How about this one. I think it ensures that the sta will never be stale,

No, it doesn't. At least not in AP mode.

Quoted 2 lines

since it flushes the tx queue on vif removal. Minimal testing shows it working, but of course I might be missing something.

In AP mode, a vif has multiple sta. And draining the queue when a sta gets removed is not a good idea.

Ok, here's another try. It does a lookup each time a tx is completed, so it's not exactly efficient..

I think it might fail the lookup if NIC manages to change it's MAC while pkts are in the queue, but at least it mostly works.

drivers/net/wireless/ath/ath9k/recv.c +5 -1
index c5e7af4..9165ac8 100644
@@ -977,7 +977,11 @@ static void ath9k_process_rssi(struct ath_common *common,
          * at least one sdata of a wiphy on mac80211 but with ath9k virtual
          * wiphy you'd have to iterate over every wiphy and each sdata.
          */
-       sta = ieee80211_find_sta_by_hw(hw, hdr->addr2);
+       if (is_multicast_ether_addr(hdr->addr1))
+               sta = ieee80211_find_sta_by_hw(hw, hdr->addr2, NULL);
+       else
+               sta = ieee80211_find_sta_by_hw(hw, hdr->addr2, hdr->addr1);
+
         if (sta) {
                 an = (struct ath_node *) sta->drv_priv;
                 if (rx_stats->rs_rssi != ATH9K_RSSI_BAD &&
drivers/net/wireless/ath/ath9k/xmit.c +1 -1
index 85a7323..6543828 100644
@@ -329,7 +329,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
         rcu_read_lock();
 
         /* XXX: use ieee80211_find_sta! */
-       sta = ieee80211_find_sta_by_hw(hw, hdr->addr1);
+       sta = ieee80211_find_sta_by_hw(hw, hdr->addr1, hdr->addr2);
         if (!sta) {
                 rcu_read_unlock();
 
include/net/mac80211.h +4 -2
index 12a49f0..1b2840f 100644
@@ -2419,7 +2419,8 @@ struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_vif *vif,
   * ieee80211_find_sta_by_hw - find a station on hardware
   *
   * @hw: pointer as obtained from ieee80211_alloc_hw()
- * @addr: station's address
+ * @addr: remote station's address
+ * @addr2: local address (vif->sdata->dev->dev_addr).  Can be NULL for 'any'.
   *
   * This function must be called under RCU lock and the
   * resulting pointer is only valid under RCU lock as well.
@@ -2434,7 +2435,8 @@ struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_vif *vif,
   * DO NOT USE THIS FUNCTION.
   */
  struct ieee80211_sta *ieee80211_find_sta_by_hw(struct ieee80211_hw *hw,
-                                              const u8 *addr);
+                                              const u8 *addr,
+                                              const u8 *localaddr);
 
  /**
   * ieee80211_sta_block_awake - block station from waking up
net/mac80211/sta_info.c +8 -2
index 44e10a9..1ba56a8 100644
@@ -839,12 +839,18 @@ void ieee80211_sta_expire(struct ieee80211_sub_if_data *sdata,
  }
 
  struct ieee80211_sta *ieee80211_find_sta_by_hw(struct ieee80211_hw *hw,
-                                              const u8 *addr)
+                                              const u8 *addr,
+                                              const u8 *localaddr)
  {
         struct sta_info *sta, *nxt;
 
-       /* Just return a random station ... first in list ... */
+       /* Just return a random station if localaddr is NULL
+        * ... first in list.
+        */
         for_each_sta_info(hw_to_local(hw), addr, sta, nxt) {
+               if (localaddr &&
+                   memcmp(sta->sdata->dev->dev_addr, localaddr, ETH_ALEN) != 0)
+                       continue;
                 if (!sta->uploaded)
                         return NULL;
                 return &sta->sta;
Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
maintainer
#17 lore raw

include/net/mac80211.h struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_vif *vif, · line 2439

Quoted 11 lines · Ben Greear

+ * @addr2: local address (vif->sdata->dev->dev_addr). Can be NULL for 'any'. *
* This function must be called under RCU lock and the
* resulting pointer is only valid under RCU lock as well.

include/net/mac80211.h +2 -1
@@ -2434,7 +2435,8 @@ struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_vif *vif,
   * DO NOT USE THIS FUNCTION.
   */
  struct ieee80211_sta *ieee80211_find_sta_by_hw(struct ieee80211_hw *hw,
-                                              const u8 *addr);
+                                              const u8 *addr,
+                                              const u8 *localaddr);

That's not going to generate proper documentation. Also, it now probably shouldn't be called "by_hw" any more.

net/mac80211/sta_info.c void ieee80211_sta_expire(struct ieee80211_sub_if_data *sdata, · line 849

Quoted 4 lines
net/mac80211/sta_info.c +3 -1
-       /* Just return a random station ... first in list ... */
+       /* Just return a random station if localaddr is NULL
+        * ... first in list.
+        */

Please use the comment style like this:

	/*
	 * Just return a random station ...
	 */

I know davem personally doesn't like it, but it's the regular style in this code.

net/mac80211/sta_info.c void ieee80211_sta_expire(struct ieee80211_sub_if_data *sdata, · line 853

Quoted 4 lines
net/mac80211/sta_info.c +3 -0
         for_each_sta_info(hw_to_local(hw), addr, sta, nxt) {
+               if (localaddr &&
+                   memcmp(sta->sdata->dev->dev_addr, localaddr, ETH_ALEN) != 0)
+                       continue;

This should use compare_ether_addr() (if it's not aligned on a two-byte boundary something is seriously wrong), and also sta->sdata->vif.addr instead of dev->dev_addr.

johannes

author
#18 lore raw
Quoted 17 lines · Johannes Berg

include/net/mac80211.h struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_vif *vif, · line 2439

Quoted 11 lines · Ben Greear

+ * @addr2: local address (vif->sdata->dev->dev_addr). Can be NULL for 'any'. *
* This function must be called under RCU lock and the
* resulting pointer is only valid under RCU lock as well.

include/net/mac80211.h +2 -1
@@ -2434,7 +2435,8 @@ struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_vif *vif,
    * DO NOT USE THIS FUNCTION.
    */
   struct ieee80211_sta *ieee80211_find_sta_by_hw(struct ieee80211_hw *hw,
-                                              const u8 *addr);
+                                              const u8 *addr,
+                                              const u8 *localaddr);

That's not going to generate proper documentation. Also, it now probably shouldn't be called "by_hw" any more.

I can't think of a better name, and it still does take the hw struct, but if you have a suggestion you like better, I will use it.

I re-posted with the rest of the changes you suggested. Seems to be working fine.

Thanks,
Ben

Signature · 2 lines
Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com
maintainer
#19 lore raw
Quoted 19 lines · Ben Greear
Quoted 17 lines · Johannes Berg

include/net/mac80211.h struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_vif *vif, · line 2439

Quoted 11 lines · Ben Greear

+ * @addr2: local address (vif->sdata->dev->dev_addr). Can be NULL for 'any'. *
* This function must be called under RCU lock and the
* resulting pointer is only valid under RCU lock as well.

include/net/mac80211.h +2 -1
@@ -2434,7 +2435,8 @@ struct ieee80211_sta *ieee80211_find_sta(struct ieee80211_vif *vif,
    * DO NOT USE THIS FUNCTION.
    */
   struct ieee80211_sta *ieee80211_find_sta_by_hw(struct ieee80211_hw *hw,
-                                              const u8 *addr);
+                                              const u8 *addr,
+                                              const u8 *localaddr);

That's not going to generate proper documentation. Also, it now probably shouldn't be called "by_hw" any more.

Quoted 2 lines

I can't think of a better name, and it still does take the hw struct, but if you have a suggestion you like better, I will use it.

Yeah, but it doesn't use the hw struct for matching (well, not necessarily anyway) ... so something like _by_ifaddr() would be better?

johannes