> -----Original Message-----
> From: Arend Van Spriel [mailto:arend.vanspr...@broadcom.com]
> Sent: Friday, September 16, 2016 13:59
> To: Luca Coelho <l...@coelho.fi>; johan...@sipsolutions.net
> Cc: linux-wireless@vger.kernel.org; Beker, Ayala <ayala.be...@intel.com>;
> Otcheretianski, Andrei <andrei.otcheretian...@intel.com>; Grumbach,
> Emmanuel <emmanuel.grumb...@intel.com>; Coelho, Luciano
> <luciano.coe...@intel.com>
> Subject: Re: [PATCH v2 1/9] cfg80211: add start / stop NAN commands
> 
> On 16-9-2016 10:33, Luca Coelho wrote:
> > From: Ayala Beker <ayala.be...@intel.com>
> >
> > This allows user space to start/stop NAN interface.
> > A NAN interface is like P2P device in a few aspects: it doesn't have a
> > netdev associated to it.
> > Add the new interface type and prevent operations that can't be
> > executed on NAN interface like scan.
> >
> > Define several attributes that may be configured by user space when
> > starting NAN functionality (master preference and dual band operation)
> >
> > Signed-off-by: Andrei Otcheretianski <andrei.otcheretian...@intel.com>
> > Signed-off-by: Emmanuel Grumbach <emmanuel.grumb...@intel.com>
> > Signed-off-by: Luca Coelho <luciano.coe...@intel.com>
> > ---
> >  include/net/cfg80211.h       | 21 +++++++++-
> >  include/uapi/linux/nl80211.h | 52 +++++++++++++++++++++++++
> >  net/mac80211/cfg.c           |  2 +
> >  net/mac80211/chan.c          |  3 ++
> >  net/mac80211/iface.c         |  4 ++
> >  net/mac80211/offchannel.c    |  1 +
> >  net/mac80211/rx.c            |  3 ++
> >  net/mac80211/util.c          |  1 +
> >  net/wireless/chan.c          |  2 +
> >  net/wireless/core.c          | 34 ++++++++++++++++
> >  net/wireless/core.h          |  3 ++
> >  net/wireless/mlme.c          |  1 +
> >  net/wireless/nl80211.c       | 93
> ++++++++++++++++++++++++++++++++++++++++++--
> >  net/wireless/rdev-ops.h      | 20 ++++++++++
> >  net/wireless/trace.h         | 27 +++++++++++++
> >  net/wireless/util.c          |  6 ++-
> >  16 files changed, 267 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index
> > d5e7f69..ca64d69 100644
> > --- a/include/net/cfg80211.h
> > +++ b/include/net/cfg80211.h
> > @@ -2293,6 +2293,19 @@ struct cfg80211_qos_map {  };
> 
> [...]
> 
> > +/**
> > + * enum nl80211_nan_dual_band_conf - NAN dual band configuration
> > + *
> > + * Defines the NAN dual band mode of operation
> 
> Does it make sense to have such a notion of bands in use. And what does
> 5.2GHz mean. Is this a subband within 5G channels? Probably I should read
> the NAN spec to understand what is meant here. I would consider 5.2G as
> lower 5G, ie. discovery on channel 44 but not sure if that is meant here.
>

The NAN spec defines single and dual band modes of operation. The channels are 
fixed for each band.
On 2.4Ghz is channel 6 and 5GHz is either 44 or 149. Regarding 5.2 - it's just 
a typo. It should be 5G - no deeper meaning.
 
> > + * @NL80211_NAN_BAND_DEFAULT: device default mode
> > + * @NL80211_NAN_BAND_SINGLE: 2.4GHz only mode
> > + * @NL80211_NAN_BAND_DUAL: 2.4GHz and 5.2GHz mode
> > +  */
> > +enum nl80211_nan_dual_band_conf {
> > +   NL80211_NAN_BAND_DEFAULT,
> > +   NL80211_NAN_BAND_SINGLE,
> > +   NL80211_NAN_BAND_DUAL,
> > +
> > +   /* keep last */
> > +   __NL80211_NAN_BAND_AFTER_LAST,
> > +   NL80211_NAN_BAND_MAX =
> > +   __NL80211_NAN_BAND_AFTER_LAST - 1,
> > +};
> > +
> >  #endif /* __LINUX_NL80211_H */
> > diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index
> > e29ff57..a74027f 100644
> > --- a/net/mac80211/cfg.c
> > +++ b/net/mac80211/cfg.c
> > @@ -257,6 +257,7 @@ static int ieee80211_add_key(struct wiphy *wiphy,
> struct net_device *dev,
> >     case NL80211_IFTYPE_WDS:
> >     case NL80211_IFTYPE_MONITOR:
> >     case NL80211_IFTYPE_P2P_DEVICE:
> > +   case NL80211_IFTYPE_NAN:
> >     case NL80211_IFTYPE_UNSPECIFIED:
> >     case NUM_NL80211_IFTYPES:
> 
> Huh? What is this doing here? Not yours but weird still.
> 
> >     case NL80211_IFTYPE_P2P_CLIENT:
> > @@ -2036,6 +2037,7 @@ static int ieee80211_scan(struct wiphy *wiphy,
> >                  !(req->flags & NL80211_SCAN_FLAG_AP)))
> >                     return -EOPNOTSUPP;
> >             break;
> > +   case NL80211_IFTYPE_NAN:
> >     default:
> >             return -EOPNOTSUPP;
> >     }
> > diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c index
> > 74142d0..acb50f8 100644
> > --- a/net/mac80211/chan.c
> > +++ b/net/mac80211/chan.c
> > @@ -274,6 +274,7 @@ ieee80211_get_chanctx_max_required_bw(struct
> ieee80211_local *local,
> >                                 ieee80211_get_max_required_bw(sdata));
> >                     break;
> >             case NL80211_IFTYPE_P2P_DEVICE:
> > +           case NL80211_IFTYPE_NAN:
> >                     continue;
> >             case NL80211_IFTYPE_ADHOC:
> >             case NL80211_IFTYPE_WDS:
> > @@ -718,6 +719,7 @@ void ieee80211_recalc_smps_chanctx(struct
> > ieee80211_local *local,
> >
> >             switch (sdata->vif.type) {
> >             case NL80211_IFTYPE_P2P_DEVICE:
> > +           case NL80211_IFTYPE_NAN:
> >                     continue;
> >             case NL80211_IFTYPE_STATION:
> >                     if (!sdata->u.mgd.associated)
> > @@ -981,6 +983,7 @@
> ieee80211_vif_chanctx_reservation_complete(struct ieee80211_sub_if_data
> *sdata)
> >     case NL80211_IFTYPE_P2P_GO:
> >     case NL80211_IFTYPE_P2P_DEVICE:
> >     case NUM_NL80211_IFTYPES:
> 
> I think NUM_NL80211_IFTYPES should not be in the switch. If it must I would
> leave it as last one here.

Agree that it should go last

> 
> > +   case NL80211_IFTYPE_NAN:
> >             WARN_ON(1);
> >             break;
> >     }
> 
> Regards,
> Arend

Reply via email to