RE: [RFC] mac80211: take reserved vif into account when calculating the min_def

2014-11-30 Thread Grumbach, Emmanuel 
 
 On 27 November 2014 at 13:30, Emmanuel Grumbach
 emmanuel.grumb...@intel.com wrote:
  When we want to calculate the minimal bandwidth needed for
  a channel context, we need to take into account vifs that
  have reserved the channel context.
  I hit an issue with iwlwifi and channel switch as a client.
 
  We would allocate a virgin channel context and reserve it.
  At that stage, the min_def was 20MHz.
  Then we would use it after CSA, and start transmitting, but
  the channel context was still 20MHz even if the GO was in
  40MHz. This made the firmware unhappy.
 
  Signed-off-by: Emmanuel Grumbach emmanuel.grumb...@intel.com
  ---
   net/mac80211/chan.c | 6 +-
   1 file changed, 5 insertions(+), 1 deletion(-)
 
  diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
  index 4c74e8d..769e0c5 100644
  --- a/net/mac80211/chan.c
  +++ b/net/mac80211/chan.c
  @@ -256,7 +256,8 @@ ieee80211_get_chanctx_max_required_bw(struct
 ieee80211_local *local,
  if (!ieee80211_sdata_running(sdata))
  continue;
 
  -   if (rcu_access_pointer(sdata-vif.chanctx_conf) != conf)
  +   if (rcu_access_pointer(sdata-vif.chanctx_conf) != conf 
  +   sdata-reserved_chanctx-conf != conf)
  continue;
 
  switch (vif-type) {
  @@ -271,6 +272,7 @@ ieee80211_get_chanctx_max_required_bw(struct
 ieee80211_local *local,
  case NL80211_IFTYPE_WDS:
  case NL80211_IFTYPE_MESH_POINT:
  width = vif-bss_conf.chandef.width;
  +   width = max(width, sdata-reserved_chandef.width);
 
 Not really sure why this is needed in this patch?
 

Hmm... You are right - I think I got confused here :)
I guess I need to verify that removing this hunk still solves my bug.
In general though, what option would you prefer?

Internally, we had different opinions here.

 
  break;
  case NL80211_IFTYPE_UNSPECIFIED:
  case NUM_NL80211_IFTYPES:
  @@ -899,6 +901,8 @@ int ieee80211_vif_reserve_chanctx(struct
 ieee80211_sub_if_data *sdata,
  sdata-reserved_radar_required = radar_required;
  sdata-reserved_ready = false;
 
  +   ieee80211_recalc_chanctx_min_def(local, new_ctx);
  +
 
 Hmm.. Wouldn't it make sense to recalc this in
 ieee80211_vif_unreserve_ chanctx() as well?

Probably - I need to add this.

 
 
 Michał

RE: [RFC] mac80211: take reserved vif into account when calculating the min_def

2014-11-30 Thread Grumbach, Emmanuel 
 
  On 27 November 2014 at 13:30, Emmanuel Grumbach
  emmanuel.grumb...@intel.com wrote:
   When we want to calculate the minimal bandwidth needed for a channel
   context, we need to take into account vifs that have reserved the
   channel context.
   I hit an issue with iwlwifi and channel switch as a client.
  
   We would allocate a virgin channel context and reserve it.
   At that stage, the min_def was 20MHz.
   Then we would use it after CSA, and start transmitting, but the
   channel context was still 20MHz even if the GO was in 40MHz. This
   made the firmware unhappy.
  
   Signed-off-by: Emmanuel Grumbach emmanuel.grumb...@intel.com

So we discussed it internally - and it seems that the other patch is better 
(https://patchwork.kernel.org/patch/5396351/).
Let's drop this one.

   ---
net/mac80211/chan.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
  
   diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c index
   4c74e8d..769e0c5 100644
   --- a/net/mac80211/chan.c
   +++ b/net/mac80211/chan.c
   @@ -256,7 +256,8 @@ ieee80211_get_chanctx_max_required_bw(struct
  ieee80211_local *local,
   if (!ieee80211_sdata_running(sdata))
   continue;
  
   -   if (rcu_access_pointer(sdata-vif.chanctx_conf) != conf)
   +   if (rcu_access_pointer(sdata-vif.chanctx_conf) != conf 
   +   sdata-reserved_chanctx-conf != conf)
   continue;
  
   switch (vif-type) { @@ -271,6 +272,7 @@
   ieee80211_get_chanctx_max_required_bw(struct
  ieee80211_local *local,
   case NL80211_IFTYPE_WDS:
   case NL80211_IFTYPE_MESH_POINT:
   width = vif-bss_conf.chandef.width;
   +   width = max(width,
   + sdata-reserved_chandef.width);
 
  Not really sure why this is needed in this patch?
 
 
 Hmm... You are right - I think I got confused here :) I guess I need to verify
 that removing this hunk still solves my bug.
 In general though, what option would you prefer?
 
 Internally, we had different opinions here.
 
 
   break;
   case NL80211_IFTYPE_UNSPECIFIED:
   case NUM_NL80211_IFTYPES:
   @@ -899,6 +901,8 @@ int ieee80211_vif_reserve_chanctx(struct
  ieee80211_sub_if_data *sdata,
   sdata-reserved_radar_required = radar_required;
   sdata-reserved_ready = false;
  
   +   ieee80211_recalc_chanctx_min_def(local, new_ctx);
   +
 
  Hmm.. Wouldn't it make sense to recalc this in
  ieee80211_vif_unreserve_ chanctx() as well?
 
 Probably - I need to add this.
 
 
 
  Michał
N�r��yb�X��ǧv�^�)޺{.n�+{��*ޕ�,�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj��!�i

[RFC] mac80211: take reserved vif into account when calculating the min_def

2014-11-27 Thread Emmanuel Grumbach 
When we want to calculate the minimal bandwidth needed for
a channel context, we need to take into account vifs that
have reserved the channel context.
I hit an issue with iwlwifi and channel switch as a client.

We would allocate a virgin channel context and reserve it.
At that stage, the min_def was 20MHz.
Then we would use it after CSA, and start transmitting, but
the channel context was still 20MHz even if the GO was in
40MHz. This made the firmware unhappy.

Signed-off-by: Emmanuel Grumbach emmanuel.grumb...@intel.com
---
 net/mac80211/chan.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 4c74e8d..769e0c5 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -256,7 +256,8 @@ ieee80211_get_chanctx_max_required_bw(struct 
ieee80211_local *local,
if (!ieee80211_sdata_running(sdata))
continue;
 
-   if (rcu_access_pointer(sdata-vif.chanctx_conf) != conf)
+   if (rcu_access_pointer(sdata-vif.chanctx_conf) != conf 
+   sdata-reserved_chanctx-conf != conf)
continue;
 
switch (vif-type) {
@@ -271,6 +272,7 @@ ieee80211_get_chanctx_max_required_bw(struct 
ieee80211_local *local,
case NL80211_IFTYPE_WDS:
case NL80211_IFTYPE_MESH_POINT:
width = vif-bss_conf.chandef.width;
+   width = max(width, sdata-reserved_chandef.width);
break;
case NL80211_IFTYPE_UNSPECIFIED:
case NUM_NL80211_IFTYPES:
@@ -899,6 +901,8 @@ int ieee80211_vif_reserve_chanctx(struct 
ieee80211_sub_if_data *sdata,
sdata-reserved_radar_required = radar_required;
sdata-reserved_ready = false;
 
+   ieee80211_recalc_chanctx_min_def(local, new_ctx);
+
return 0;
 }
 
-- 
1.9.1

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

Re: [RFC] mac80211: take reserved vif into account when calculating the min_def

2014-11-27 Thread Michal Kazior 
On 27 November 2014 at 13:30, Emmanuel Grumbach
emmanuel.grumb...@intel.com wrote:
 When we want to calculate the minimal bandwidth needed for
 a channel context, we need to take into account vifs that
 have reserved the channel context.
 I hit an issue with iwlwifi and channel switch as a client.

 We would allocate a virgin channel context and reserve it.
 At that stage, the min_def was 20MHz.
 Then we would use it after CSA, and start transmitting, but
 the channel context was still 20MHz even if the GO was in
 40MHz. This made the firmware unhappy.

 Signed-off-by: Emmanuel Grumbach emmanuel.grumb...@intel.com
 ---
  net/mac80211/chan.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

 diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
 index 4c74e8d..769e0c5 100644
 --- a/net/mac80211/chan.c
 +++ b/net/mac80211/chan.c
 @@ -256,7 +256,8 @@ ieee80211_get_chanctx_max_required_bw(struct 
 ieee80211_local *local,
 if (!ieee80211_sdata_running(sdata))
 continue;

 -   if (rcu_access_pointer(sdata-vif.chanctx_conf) != conf)
 +   if (rcu_access_pointer(sdata-vif.chanctx_conf) != conf 
 +   sdata-reserved_chanctx-conf != conf)
 continue;

 switch (vif-type) {
 @@ -271,6 +272,7 @@ ieee80211_get_chanctx_max_required_bw(struct 
 ieee80211_local *local,
 case NL80211_IFTYPE_WDS:
 case NL80211_IFTYPE_MESH_POINT:
 width = vif-bss_conf.chandef.width;
 +   width = max(width, sdata-reserved_chandef.width);

Not really sure why this is needed in this patch?


 break;
 case NL80211_IFTYPE_UNSPECIFIED:
 case NUM_NL80211_IFTYPES:
 @@ -899,6 +901,8 @@ int ieee80211_vif_reserve_chanctx(struct 
 ieee80211_sub_if_data *sdata,
 sdata-reserved_radar_required = radar_required;
 sdata-reserved_ready = false;

 +   ieee80211_recalc_chanctx_min_def(local, new_ctx);
 +

Hmm.. Wouldn't it make sense to recalc this in
ieee80211_vif_unreserve_ chanctx() as well?


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