-----Original Message-----
From: Arend Van Spriel [mailto:[email protected]] 
Sent: Sunday, September 18, 2016 22:01
To: Otcheretianski, Andrei <[email protected]>; Luca Coelho 
<[email protected]>; [email protected]
Cc: [email protected]; Beker, Ayala <[email protected]>; 
Grumbach, Emmanuel <[email protected]>; Coelho, Luciano 
<[email protected]>
Subject: Re: [PATCH v2 2/9] mac80211: add boilerplate code for start / stop NAN

On 18-9-2016 9:59, Otcheretianski, Andrei wrote:
>> -----Original Message-----
>> From: Arend Van Spriel [mailto:[email protected]]
>> Sent: Friday, September 16, 2016 14:09
>> To: Luca Coelho <[email protected]>; [email protected]
>> Cc: [email protected]; Beker, Ayala 
>> <[email protected]>; Otcheretianski, Andrei 
>> <[email protected]>; Grumbach, Emmanuel 
>> <[email protected]>; Coelho, Luciano 
>> <[email protected]>
>> Subject: Re: [PATCH v2 2/9] mac80211: add boilerplate code for start 
>> / stop NAN
>>
>> On 16-9-2016 10:33, Luca Coelho wrote:
>>> From: Ayala Beker <[email protected]>
>>>
>>> This code doesn't do much besides allowing to start and stop the vif.
>>>
>>> Signed-off-by: Andrei Otcheretianski 
>>> <[email protected]>
>>> Signed-off-by: Emmanuel Grumbach <[email protected]>
>>> Signed-off-by: Ayala Beker <[email protected]>
>>> Signed-off-by: Luca Coelho <[email protected]>
>>> ---
>>>  include/net/mac80211.h    |  9 +++++++++
>>>  net/mac80211/cfg.c        | 36 ++++++++++++++++++++++++++++++++++
>>>  net/mac80211/chan.c       |  3 +++
>>>  net/mac80211/driver-ops.h | 29 ++++++++++++++++++++++++++-
>>>  net/mac80211/iface.c      |  8 ++++++--
>>>  net/mac80211/main.c       |  5 +++++
>>>  net/mac80211/offchannel.c |  3 ++-
>>>  net/mac80211/trace.h      | 50
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>>  net/mac80211/util.c       |  3 ++-
>>>  9 files changed, 141 insertions(+), 5 deletions(-)
>>
>> [...]
>>
>>> diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h 
>>> index fe35a1c..67b42c8 100644
>>> --- a/net/mac80211/driver-ops.h
>>> +++ b/net/mac80211/driver-ops.h
>>> @@ -163,7 +163,8 @@ static inline void drv_bss_info_changed(struct 
>>> ieee80211_local *local,
>>>
>>>     if (WARN_ON_ONCE(sdata->vif.type ==
>> NL80211_IFTYPE_P2P_DEVICE ||
>>>                      (sdata->vif.type == NL80211_IFTYPE_MONITOR &&
>>> -                     !sdata->vif.mu_mimo_owner)))
>>> +                     !sdata->vif.mu_mimo_owner) ||
>>> +                    sdata->vif.type == NL80211_IFTYPE_NAN))
>>
>> Might be more clear to move this up right after P2P_DEVICE check.
> 
> Why? It's a completely separate new condition - so it goes to the end.

> I would say readability. Both P2P_DEVICE and NAN checks are single 
> comparisons as opposed to the MONITOR check.

>>
>>>             return;
>>>
>>>     if (!check_sdata_in_driver(sdata)) @@ -1165,4 +1166,30 @@ static 
>>> inline void drv_wake_tx_queue(struct
>> ieee80211_local *local,
>>>     local->ops->wake_tx_queue(&local->hw, &txq->txq);  }
>>>
>>> +static inline int drv_start_nan(struct ieee80211_local *local,
>>> +                           struct ieee80211_sub_if_data *sdata,
>>> +                           struct cfg80211_nan_conf *conf) {
>>> +   int ret;
>>> +
>>> +   might_sleep();
>>> +   check_sdata_in_driver(sdata);
>>> +
>>> +   trace_drv_start_nan(local, sdata, conf);
>>> +   ret = local->ops->start_nan(&local->hw, &sdata->vif, conf);
>>> +   trace_drv_return_int(local, ret);
>>> +   return ret;
>>> +}
>>> +
>>> +static inline void drv_stop_nan(struct ieee80211_local *local,
>>> +                           struct ieee80211_sub_if_data *sdata) {
>>> +   might_sleep();
>>> +   check_sdata_in_driver(sdata);
>>> +
>>> +   trace_drv_stop_nan(local, sdata);
>>> +   local->ops->stop_nan(&local->hw, &sdata->vif);
>>> +   trace_drv_return_void(local);
>>> +}
>>> +
>>>  #endif /* __MAC80211_DRIVER_OPS */
>>> diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c index 
>>> e694ca2..507f46a 100644
>>> --- a/net/mac80211/iface.c
>>> +++ b/net/mac80211/iface.c
>>> @@ -327,6 +327,9 @@ static int ieee80211_check_queues(struct
>> ieee80211_sub_if_data *sdata,
>>>     int n_queues = sdata->local->hw.queues;
>>>     int i;
>>>
>>> +   if (iftype == NL80211_IFTYPE_NAN)
>>> +           return 0;
>>> +
>>>     if (iftype != NL80211_IFTYPE_P2P_DEVICE) {
>>>             for (i = 0; i < IEEE80211_NUM_ACS; i++) {
>>>                     if (WARN_ON_ONCE(sdata->vif.hw_queue[i] == @@
>> -647,7 +650,8 @@ int
>>> ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
>>>                     local->fif_probe_req++;
>>>             }
>>>
>>> -           if (sdata->vif.type != NL80211_IFTYPE_P2P_DEVICE)
>>> +           if (sdata->vif.type != NL80211_IFTYPE_P2P_DEVICE &&
>>> +               sdata->vif.type != NL80211_IFTYPE_NAN)
>>
>> similar check keeps reoccuring in various places so maybe we can 
>> create a helper function for it.
> 
> Right, but not sure that it deserves a function.

> If similar new iftypes are anticipated it would make sense as it would mean 
> adding it in one place.

I don't think there is something in common to those interface types that can 
fit such a function semantically, so I decided not to change it.
Do you have any idea?

> Regards,
> Arend

---------------------------------------------------------------------
A member of the Intel Corporation group of companies

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Reply via email to