For example, let's consider the case of a 4-addr client connecting to
an AP for which AP_VLAN interface is also created, let the initial
value for tailroom_needed on the AP be 1.

* 4-addr client connects to the AP (AP: tailroom_needed = 1)
* AP will clear old keys, delay decrement of tailroom_needed count
* AP_VLAN is created, it takes the tailroom count from master
   (AP_VLAN: tailroom_needed = 1, AP: tailroom_needed = 1)
* Install new key for the station, assume key is plumbed in the HW,
   there won't be any change in tailroom_needed count on AP iface
* Delayed decrement of tailroom_needed count on AP
   (AP: tailroom_needed = 0, AP_VLAN: tailroom_needed = 1)

Because of the delayed decrement on AP iface, tailroom_needed count goes
out of sync between AP(master iface) and AP_VLAN(slave iface) and
there would be unnecessary tailroom created for the packets going
through AP_VLAN iface.
This describes a scenario where there's *unnecessary* work done, but not
really one where we have something that's *incorrect*?


To me at least doing unnecessary things is incorrect :-D, may be we can change the statement.

Are you saying that you found a problem with this? Did this show up in
some sort of measurements?

Not really, I have observed in my testing that there were warnings whenever a AP_VLAN was brought down; this is done in ieee80211_free_keys. Warnings show up every time we bring down the AP_VLAN interface(using ifconfig); warnings apart but I thought there would be unnecessary overhead in the tailroom expansion though the overhead may be trivial.

+++ b/net/mac80211/key.c
@@ -583,7 +583,8 @@ static void __ieee80211_key_destroy(struct ieee80211_key 
*key,
ieee80211_debugfs_key_remove(key); - if (delay_tailroom) {
+               if (delay_tailroom &&
+                   sdata->vif.type == NL80211_IFTYPE_STATION) {
                        /* see ieee80211_delayed_tailroom_dec */
                        sdata->crypto_tx_tailroom_pending_dec++;
                        schedule_delayed_work(&sdata->dec_tailroom_needed_wk,
I think you'd better change the caller, i.e. ieee80211_del_key(), and a
add a comment there that explains what's going on.

Not really sure what you were trying to tell here.

Manikanta

Reply via email to