On 20-9-2016 13:45, Beker, Ayala wrote: > -----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?
What is common is that these are both non-netdev interface types. >> 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. Maybe better to avoid such disclaimers when you are posting on community mailing lists. Regards, Arend
