Return-path: Received: from mga06.intel.com ([134.134.136.31]:7086 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751540AbcITLsn (ORCPT ); Tue, 20 Sep 2016 07:48:43 -0400 From: "Beker, Ayala" To: Arend Van Spriel , "Otcheretianski, Andrei" , Luca Coelho , "johannes@sipsolutions.net" CC: "linux-wireless@vger.kernel.org" , "Grumbach, Emmanuel" , "Coelho, Luciano" Subject: RE: [PATCH v2 2/9] mac80211: add boilerplate code for start / stop NAN Date: Tue, 20 Sep 2016 11:45:34 +0000 Message-ID: <6E2DD73D17E17047917F6A81399A8ED3972503A6@hasmsx109.ger.corp.intel.com> (sfid-20160920_134847_280755_56708C61) References: <20160916083321.5840-1-luca@coelho.fi> <20160916083321.5840-3-luca@coelho.fi> In-Reply-To: Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org List-ID: -----Original Message----- From: Arend Van Spriel [mailto:arend.vanspriel@broadcom.com] Sent: Sunday, September 18, 2016 22:01 To: Otcheretianski, Andrei ; Luca Coelho ; johannes@sipsolutions.net Cc: linux-wireless@vger.kernel.org; Beker, Ayala ; Grumbach, Emmanuel ; Coelho, Luciano 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@broadcom.com] >> Sent: Friday, September 16, 2016 14:09 >> To: Luca Coelho ; johannes@sipsolutions.net >> Cc: linux-wireless@vger.kernel.org; Beker, Ayala >> ; Otcheretianski, Andrei >> ; Grumbach, Emmanuel >> ; Coelho, Luciano >> >> 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 >>> >>> This code doesn't do much besides allowing to start and stop the vif. >>> >>> Signed-off-by: Andrei Otcheretianski >>> >>> Signed-off-by: Emmanuel Grumbach >>> Signed-off-by: Ayala Beker >>> Signed-off-by: Luca Coelho >>> --- >>> 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.