On 20-9-2016 13:45, Beker, Ayala wrote: > -----Original Message----- > From: Arend Van Spriel [mailto:arend.vanspriel@xxxxxxxxxxxx] > Sent: Sunday, September 18, 2016 22:01 > To: Otcheretianski, Andrei <andrei.otcheretianski@xxxxxxxxx>; Luca Coelho <luca@xxxxxxxxx>; johannes@xxxxxxxxxxxxxxxx > Cc: linux-wireless@xxxxxxxxxxxxxxx; Beker, Ayala <ayala.beker@xxxxxxxxx>; Grumbach, Emmanuel <emmanuel.grumbach@xxxxxxxxx>; Coelho, Luciano <luciano.coelho@xxxxxxxxx> > 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:arend.vanspriel@xxxxxxxxxxxx] >>> Sent: Friday, September 16, 2016 14:09 >>> To: Luca Coelho <luca@xxxxxxxxx>; johannes@xxxxxxxxxxxxxxxx >>> Cc: linux-wireless@xxxxxxxxxxxxxxx; Beker, Ayala >>> <ayala.beker@xxxxxxxxx>; Otcheretianski, Andrei >>> <andrei.otcheretianski@xxxxxxxxx>; Grumbach, Emmanuel >>> <emmanuel.grumbach@xxxxxxxxx>; Coelho, Luciano >>> <luciano.coelho@xxxxxxxxx> >>> 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 <ayala.beker@xxxxxxxxx> >>>> >>>> This code doesn't do much besides allowing to start and stop the vif. >>>> >>>> Signed-off-by: Andrei Otcheretianski >>>> <andrei.otcheretianski@xxxxxxxxx> >>>> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx> >>>> Signed-off-by: Ayala Beker <ayala.beker@xxxxxxxxx> >>>> Signed-off-by: Luca Coelho <luciano.coelho@xxxxxxxxx> >>>> --- >>>> 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
