2013-02-27 12:24:15

by Vladimir Kondratiev

[permalink] [raw]
Subject: [RFC] P2P find offload

Intro:

P2P scan consists of scan and find phases.
Scan is just usual scan.
Find phase consists of search and listen states.
Currently, 'search' state implemented as 'scan'
and 'listen' state implemented as 'remain on channel'

Recently spec for 60GHz band added, defining new procedures.
For 60GHz devices, spec defined search state in the way that can't be
simulated by the scan. On the 60GHz band, device in the 'search' state
transmits specially crafted DMG beacon frames, while in scan state it does
not transmit DMG beacon frames.

GO resolution for 60GHz devices performed based on 'PCP factor' calculated
by certain bits of probe request/response frames. GO negotiation frames are
sent, but used to negotiate the rest of parameters like operational channel.

Proposal:

Enable p2p find phase offload to the driver.
Add methods for the struct cfg80211_ops, like

int (*start_p2p_find)(struct wiphy *wiphy,
struct cfg80211_p2p_find_params *params);
void (*stop_p2p_find)(struct wiphy *wiphy);

where struct cfg80211_p2p_find_params includes info elements
to be added for probe request and probe response frames;
social channels etc.

wpa_supplicant will call these methods through nl80211.

Driver responsible for toggling between search and listen states,
reporting probe request/response frames to the user space.

Driver/firmware may answer to the probe request frames on itself,
in this case probe requests are still reported.

To satisfy requirements for 60GHz band, additional attribute 'pcp_resolution'
shall be added to the reported frames, indicating result of the PCP resolution.
This attribute carries result of PCP factor comparison between probe request
and response, as defined in the spec for 60GHz. It is enum having values
'undefined', 'win' and 'lose'.
For the 2.4GHz band devices, PCP resolution is not used and pcp_resolution
attribute set to 'undefined'

Rationale:

- For all devices, this allows for better time utilization since
there is no need to 'close' each phase and return back to the idle state.

- Option to answer probe request in the driver/firmware allows to use firmware
capabilities.

- Enabler for 60GHz devices.

Comments?

Thanks, Vladimir.



2013-03-04 15:48:13

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC] P2P find offload

On Wed, 2013-02-27 at 14:24 +0200, Vladimir Kondratiev wrote:

> Enable p2p find phase offload to the driver.
> Add methods for the struct cfg80211_ops, like
>
> int (*start_p2p_find)(struct wiphy *wiphy,
> struct cfg80211_p2p_find_params *params);
> void (*stop_p2p_find)(struct wiphy *wiphy);
>
> where struct cfg80211_p2p_find_params includes info elements
> to be added for probe request and probe response frames;
> social channels etc.

We had something like this before. I think you need to specify not the
wiphy but a netdev though, to know what MAC address to use. In fact, not
a netdev but a wdev, so it can work on P2P_Device type wdevs.

Also, timings? Or is that left to the driver?

> wpa_supplicant will call these methods through nl80211.
>
> Driver responsible for toggling between search and listen states,
> reporting probe request/response frames to the user space.
>
> Driver/firmware may answer to the probe request frames on itself,
> in this case probe requests are still reported.

wpa_s will need to know whether or not it responded, unless you mandate
that it must respond by itself -- not sure if you should do that though?

> To satisfy requirements for 60GHz band, additional attribute 'pcp_resolution'
> shall be added to the reported frames, indicating result of the PCP resolution.
> This attribute carries result of PCP factor comparison between probe request
> and response, as defined in the spec for 60GHz. It is enum having values
> 'undefined', 'win' and 'lose'.

?

> For the 2.4GHz band devices, PCP resolution is not used and pcp_resolution
> attribute set to 'undefined'

No ... you'd just leave out the attribute instead of having a special
undefined value.

johannes


2013-03-07 10:01:47

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC] P2P find offload

On Thu, 2013-03-07 at 09:10 +0200, Vladimir Kondratiev wrote:

> > We had something like this before. I think you need to specify not the
> > wiphy but a netdev though, to know what MAC address to use. In fact, not
> > a netdev but a wdev, so it can work on P2P_Device type wdevs.

> Why do we need wdev here? It should be similar to scan, as p2p scan composed
> of legacy scan and p2p find. Scan have only wiphy.
> It's no problem to add wdev as well, of course. I'll add it.

No, scan also has the device? It is needed for the MAC address, of
course.

> > Also, timings? Or is that left to the driver?

> Yes, timing is up to the driver. Point is, firmware may have its reasons
> when switch search/listen phases.

Some hints from userspace might be helpful though?

> > wpa_s will need to know whether or not it responded, unless you mandate
> > that it must respond by itself -- not sure if you should do that though?

> Same way as today wpa_s know whether driver/firmware answer probes -
> WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD. Or, should I add one more flag, like
> WIPHY_FLAG_P2P_PROBE_RESP_OFFLOAD?

AP_PROBE_RESP_OFFLOAD doesn't seem appropriate? just read out the name
aloud, it starts with "AP" ;-)

johannes


2013-03-07 10:11:13

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC] P2P find offload

Also ...

I really don't think this should be supported on IBSS/AP/... netdevs.
Seems like the only reasonable ones are P2P_DEVICE and STATION, although
it would probably be good to have feature advertising for both, or
document that if P2P_DEVICE is supported at all then this doesn't have
to be supported on STATION interfaces, or so.

And then ... should this really be allowed to be concurrent with
scanning/remain-on-channel? You haven't done any checking or
documentation, so users and driver authors are left to guess.

johannes


2013-03-07 14:11:03

by Vladimir Kondratiev

[permalink] [raw]
Subject: Re: [RFC] P2P find offload

On Thursday, March 07, 2013 11:08:58 AM Johannes Berg wrote:
> > + int n_channels;
> > + /* Up to 4 social channels: 3 in 2.4GHz band + 1 in 60GHz band */
> > + struct ieee80211_channel *channels[4];
>
> Hmm, is 4 really a good limit? If you wanted to implement progressive
> search as part of the find, for example, you'd need more? It seems like
> a pretty arbitrary limit -- should it be advertised to userspace?
It is P2P spec. P2P find operates on social channels; there are 3 social
channels on 2.4 band and 1 on 60. No social channels on 5.2.

I can rework to do dynamic allocation, but does it worth the effort?

>
> > + * start_p2p_find: start P2P find phase
> > + * Parameters include IEs for probe/probe resp and channels to operate on
>
> The parameters?
>
> you could spell out "resp" ...
>
> Also needs a period (".") at the end or so, if the sentence runs on in
> the next line it doesn't make sense.
Yes, I'll fix this
>
> > + * Parameters are not retained after call, driver need to copy data if
> > + * it need it later.
> > + * stop_p2p_find: stop P2P find phase
> > + *
>
> don't need that blank line
Sure
>
> > + i = 0;
> > + if (attr_freq) {
>
> i can be inside the if()?
Moved to initializer

>
> > + /* user specified, bail out if channel not found */
> > + nla_for_each_nested(attr, attr_freq, tmp) {
> > + struct ieee80211_channel *chan;
> > +
> > + chan = ieee80211_get_channel(wiphy, nla_get_u32(attr));
> > +
> > + if (!chan)
> > + return -EINVAL;
> > +
> > + /* ignore disabled channels */
> > + if (chan->flags & IEEE80211_CHAN_DISABLED)
> > + continue;
>
> I think you should reject them, also if they have passive scan or
> various other flags set, I think?
Passive channel may be used as social for the listening; and may be
actively scanned if beacon found on the channel. Scan do not filter
based on these flags, I suppose same reason apply here.

>
> > + params.channels[i] = chan;
> > + i++;
> > + }
> > + if (!i)
> > + return -EINVAL;
> > + }
> > +
> > + params.n_channels = i;
>
> if you also move that assignment
>
> > + i = 0;
>
> Not needed.
Sure
>
> > + attr = info->attrs[NL80211_ATTR_IE];
> > + if (attr) {
>
> This is not typically done in nl80211, I'd prefer not having the
> variable I think.
It is to reasonable fit code into 80 columns. Let me know if this
argument don't play for you, I'll revert to variables.

>
> > + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> > + if (!msg)
> > + return -ENOMEM;
> > +
> > + hdr = nl80211hdr_put(msg, info->snd_portid, info->snd_seq, 0,
> > + NL80211_CMD_START_P2P_FIND);
>
> Err? What's the value of sending a reply back here? It would seem maybe
> appropriate to send one when it *actually* started, but you haven't
> implemented that.
>
> > + hdr = nl80211hdr_put(msg, info->snd_portid, info->snd_seq, 0,
> > + NL80211_CMD_STOP_P2P_FIND);
>
> same here ...
Yes, agree. I'll add indications, like ROC do.

>
> You're also entirely missing feature advertising, so userspace can only
> guess whether it's supported or not ...
Will do

>Also ...
>
> I really don't think this should be supported on IBSS/AP/... netdevs.
> Seems like the only reasonable ones are P2P_DEVICE and STATION, although
> it would probably be good to have feature advertising for both, or
> document that if P2P_DEVICE is supported at all then this doesn't have
> to be supported on STATION interfaces, or so.
Yes, sure. will add checks
>
> And then ... should this really be allowed to be concurrent with
> scanning/remain-on-channel? You haven't done any checking or
> documentation, so users and driver authors are left to guess.
I check for scan, but not for ROC. Btw, scan also don't check for ROC.
And, ROC don't check for scan.
I'll document that driver should do all checks for ROC/scan.
>
> johannes
>

Thanks, Vladimir


2013-03-07 10:09:06

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC] P2P find offload

On Thu, 2013-03-07 at 11:31 +0200, Vladimir Kondratiev wrote:

> /**
> + * struct cfg80211_p2p_find_params - parameters for P2P find
> + * @probe_ie: IE's for probe frames
> + * @probe_ie_len: length, bytes, of @probe_ie
> + * @probe_resp_ie: IE's for probe response frames
> + * @probe_resp_ie_len: length, bytes, of @probe_resp_ie
> + * @n_channels: number of channels to operate on
> + * @channels: channels to operate on
> + */
> +struct cfg80211_p2p_find_params {
> + const u8 *probe_ie;
> + size_t probe_ie_len;
> + const u8 *probe_resp_ie;
> + size_t probe_resp_ie_len;
> +
> + int n_channels;
> + /* Up to 4 social channels: 3 in 2.4GHz band + 1 in 60GHz band */
> + struct ieee80211_channel *channels[4];

Hmm, is 4 really a good limit? If you wanted to implement progressive
search as part of the find, for example, you'd need more? It seems like
a pretty arbitrary limit -- should it be advertised to userspace?

> + * start_p2p_find: start P2P find phase
> + * Parameters include IEs for probe/probe resp and channels to operate on

The parameters?

you could spell out "resp" ...

Also needs a period (".") at the end or so, if the sentence runs on in
the next line it doesn't make sense.

> + * Parameters are not retained after call, driver need to copy data if
> + * it need it later.
> + * stop_p2p_find: stop P2P find phase
> + *

don't need that blank line

> + i = 0;
> + if (attr_freq) {

i can be inside the if()?

> + /* user specified, bail out if channel not found */
> + nla_for_each_nested(attr, attr_freq, tmp) {
> + struct ieee80211_channel *chan;
> +
> + chan = ieee80211_get_channel(wiphy, nla_get_u32(attr));
> +
> + if (!chan)
> + return -EINVAL;
> +
> + /* ignore disabled channels */
> + if (chan->flags & IEEE80211_CHAN_DISABLED)
> + continue;

I think you should reject them, also if they have passive scan or
various other flags set, I think?

> + params.channels[i] = chan;
> + i++;
> + }
> + if (!i)
> + return -EINVAL;
> + }
> +
> + params.n_channels = i;

if you also move that assignment

> + i = 0;

Not needed.

> + attr = info->attrs[NL80211_ATTR_IE];
> + if (attr) {

This is not typically done in nl80211, I'd prefer not having the
variable I think.

> + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;
> +
> + hdr = nl80211hdr_put(msg, info->snd_portid, info->snd_seq, 0,
> + NL80211_CMD_START_P2P_FIND);

Err? What's the value of sending a reply back here? It would seem maybe
appropriate to send one when it *actually* started, but you haven't
implemented that.

> + hdr = nl80211hdr_put(msg, info->snd_portid, info->snd_seq, 0,
> + NL80211_CMD_STOP_P2P_FIND);

same here ...

You're also entirely missing feature advertising, so userspace can only
guess whether it's supported or not ...

johannes


2013-03-10 15:43:59

by Vladimir Kondratiev

[permalink] [raw]
Subject: Re: [RFC] P2P find offload

On Thursday, March 07, 2013 04:10:37 PM Vladimir Kondratiev wrote:
> On Thursday, March 07, 2013 11:08:58 AM Johannes Berg wrote:
> > > + int n_channels;
> > > + /* Up to 4 social channels: 3 in 2.4GHz band + 1 in 60GHz band */
> > > + struct ieee80211_channel *channels[4];
> >
> > Hmm, is 4 really a good limit?
Allocated dynamically

> > > + * start_p2p_find: start P2P find phase
> > > + * Parameters include IEs for probe/probe resp and channels to operate on
> >
> > The parameters?
Documented

> > > + i = 0;
> > > + if (attr_freq) {
> >
> > i can be inside the if()?
Moved to initializer
>
> >
> > > + /* user specified, bail out if channel not found */
> > > + nla_for_each_nested(attr, attr_freq, tmp) {
> > > + struct ieee80211_channel *chan;
> > > +
> > > + chan = ieee80211_get_channel(wiphy, nla_get_u32(attr));
> > > +
> > > + if (!chan)
> > > + return -EINVAL;
> > > +
> > > + /* ignore disabled channels */
> > > + if (chan->flags & IEEE80211_CHAN_DISABLED)
> > > + continue;
> >
> > I think you should reject them, also if they have passive scan or
> > various other flags set, I think?
> Passive channel may be used as social for the listening; and may be
> actively scanned if beacon found on the channel. Scan do not filter
> based on these flags, I suppose same reason apply here.
>
> >
> > > + params.channels[i] = chan;
> > > + i++;
> > > + }
> > > + if (!i)
> > > + return -EINVAL;
> > > + }
> > > +
> > > + params.n_channels = i;
> >
> > if you also move that assignment
> >
> > > + i = 0;
> >
> > Not needed.
Sure, done
> >
> > > + attr = info->attrs[NL80211_ATTR_IE];
> > > + if (attr) {
> >
> > This is not typically done in nl80211, I'd prefer not having the
> > variable I think.
> It is to reasonable fit code into 80 columns. Let me know if this
> argument don't play for you, I'll revert to variables.
>
> >
> > > + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> > > + if (!msg)
> > > + return -ENOMEM;
> > > +
> > > + hdr = nl80211hdr_put(msg, info->snd_portid, info->snd_seq, 0,
> > > + NL80211_CMD_START_P2P_FIND);
> >
> > Err? What's the value of sending a reply back here? It would seem maybe
> > appropriate to send one when it *actually* started, but you haven't
> > implemented that.
> >
> > > + hdr = nl80211hdr_put(msg, info->snd_portid, info->snd_seq, 0,
> > > + NL80211_CMD_STOP_P2P_FIND);
> >
> > same here ...
Removed; notification function added
>
> >
> > You're also entirely missing feature advertising, so userspace can only
> > guess whether it's supported or not ...
Done
>
> >Also ...
> >
> > I really don't think this should be supported on IBSS/AP/... netdevs.
> > Seems like the only reasonable ones are P2P_DEVICE and STATION, although
> > it would probably be good to have feature advertising for both, or
> > document that if P2P_DEVICE is supported at all then this doesn't have
> > to be supported on STATION interfaces, or so.
Done
> >
> > And then ... should this really be allowed to be concurrent with
> > scanning/remain-on-channel? You haven't done any checking or
> > documentation, so users and driver authors are left to guess.
Documented

I added relevant parameters for the find phase timing, accordingly to the spec.

And finally, rebased to recent wireless-testing.

>From c7e67726b042f6ed490858b02a6d1961be22bfa4 Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev <[email protected]>
Date: Sun, 10 Mar 2013 15:52:37 +0200
Subject: [PATCH v2] cfg80211: P2P find phase offload

Allow to implement P2P find phase in the driver/firmware.

Offload scheme designed as follows:

- Driver provide methods start_p2p_find and stop_p2p_find in the cfg80211_ops;
indicates support for p2p find offload by setting feature
NL80211_FEATURE_P2P_FIND_OFFLOAD
- Driver indicate firmware or driver responds to the probe requests by setting
feature NL80211_FEATURE_P2P_PROBE_RESP_OFFLOAD
- wpa_supplicant analyses feature flags to discover p2p find offload support;
to perform p2p scan, wpa_supplicant:
- perform legacy scan, through driver's cfg80211_ops 'scan' method
- configure rx management filter to get probe-request and probe-response frames
- start p2p find via driver's cfg80211_ops start_p2p_find method
- driver start p2p find with hardware and notify wpa_supplicant with
cfg80211_p2p_find_notify(NL80211_CMD_START_P2P_FIND)
- driver/firmware toggle search/listen states. Received probe-request and
probe-response frames passed to the wpa_supplicant via cfg80211_rx_mgmt
- when wpa_supplicant wants to stop p2p find, it calls driver's cfg80211_ops stop_p2p_find
method. Alternatively, driver/firmware may decide to stop p2p find. In all cases,
driver notifies wpa_supplicant using
cfg80211_p2p_find_notify(NL80211_CMD_STOP_P2P_FIND)

All driver to user space communication done through nl80211 layer.

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
include/net/cfg80211.h | 53 ++++++++++++++
include/uapi/linux/nl80211.h | 16 +++++
net/wireless/nl80211.c | 157 ++++++++++++++++++++++++++++++++++++++++++
net/wireless/rdev-ops.h | 19 +++++
net/wireless/trace.h | 35 ++++++++++
5 files changed, 280 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index bdba9b6..ef276f6 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1780,6 +1780,30 @@ struct cfg80211_update_ft_ies_params {
};

/**
+ * struct cfg80211_p2p_find_params - parameters for P2P find
+ * @probe_ie: IE's for probe frames
+ * @probe_ie_len: length, bytes, of @probe_ie
+ * @probe_resp_ie: IE's for probe response frames
+ * @probe_resp_ie_len: length, bytes, of @probe_resp_ie
+ * @min_discoverable_interval and
+ * @max_discoverable_interval: min/max for random multiplier of 100TU's
+ * for the listen state duration
+ * @n_channels: number of channels to operate on
+ * @channels: channels to operate on
+ */
+struct cfg80211_p2p_find_params {
+ const u8 *probe_ie;
+ size_t probe_ie_len;
+ const u8 *probe_resp_ie;
+ size_t probe_resp_ie_len;
+ u32 min_discoverable_interval;
+ u32 max_discoverable_interval;
+
+ int n_channels;
+ struct ieee80211_channel **channels;
+};
+
+/**
* struct cfg80211_ops - backend description for wireless configuration
*
* This struct is registered by fullmac card drivers and/or wireless stacks
@@ -1998,6 +2022,15 @@ struct cfg80211_update_ft_ies_params {
* advertise the support for MAC based ACL have to implement this callback.
*
* @start_radar_detection: Start radar detection in the driver.
+ *
+ * start_p2p_find: start P2P find phase
+ * Parameters include IEs for probe/probe-resp frames;
+ * and channels to operate on.
+ * Parameters are not retained after call, driver need to copy data if
+ * it need it later.
+ * P2P find can't run concurrently with ROC or scan, and driver should
+ * check this.
+ * stop_p2p_find: stop P2P find phase
*/
struct cfg80211_ops {
int (*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -2227,6 +2260,12 @@ struct cfg80211_ops {
struct cfg80211_chan_def *chandef);
int (*update_ft_ies)(struct wiphy *wiphy, struct net_device *dev,
struct cfg80211_update_ft_ies_params *ftie);
+
+ int (*start_p2p_find)(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ struct cfg80211_p2p_find_params *params);
+ void (*stop_p2p_find)(struct wiphy *wiphy,
+ struct wireless_dev *wdev);
};

/*
@@ -4122,6 +4161,20 @@ void cfg80211_report_wowlan_wakeup(struct wireless_dev *wdev,
struct cfg80211_wowlan_wakeup *wakeup,
gfp_t gfp);

+/**
+ * cfg80211_p2p_find_notify - report p2p find state change
+ * @wdev: the wireless device reporting the event
+ * @cmd: the event, NL80211_CMD_START_P2P_FIND or NL80211_CMD_STOP_P2P_FIND
+ * @gfp: allocation flags
+ *
+ * This function reports p2p find state chenge for the given device
+ * When hardware actually starts p2p find, NL80211_CMD_START_P2P_FIND
+ * should be reported
+ * When p2p find finished, either unsolicited or in response to
+ * @ops->p2p_stop_find, NL80211_CMD_STOP_P2P_FIND should be reported
+ */
+void cfg80211_p2p_find_notify(struct wireless_dev *wdev, int cmd, gfp_t gfp);
+
/* Logging, debugging and troubleshooting/diagnostic helpers. */

/* wiphy_printk helpers, similar to dev_printk */
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 79da871..8c87815 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -798,6 +798,9 @@ enum nl80211_commands {
NL80211_CMD_UPDATE_FT_IES,
NL80211_CMD_FT_EVENT,

+ NL80211_CMD_START_P2P_FIND,
+ NL80211_CMD_STOP_P2P_FIND,
+
/* add new commands above here */

/* used to define NL80211_CMD_MAX below */
@@ -1414,6 +1417,10 @@ enum nl80211_commands {
* @NL80211_ATTR_IE_RIC: Resource Information Container Information
* Element
*
+ * @NL80211_ATTR_MIN_DISCOVERABLE_INTERVAL,
+ * @NL80211_ATTR_MAX_DISCOVERABLE_INTERVAL: min/max discoverable interval
+ * for the p2p find, represented as u32
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1709,6 +1716,9 @@ enum nl80211_attrs {
NL80211_ATTR_MDID,
NL80211_ATTR_IE_RIC,

+ NL80211_ATTR_MIN_DISCOVERABLE_INTERVAL,
+ NL80211_ATTR_MAX_DISCOVERABLE_INTERVAL,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
@@ -3538,6 +3548,10 @@ enum nl80211_ap_sme_features {
* Peering Management entity which may be implemented by registering for
* beacons or NL80211_CMD_NEW_PEER_CANDIDATE events. The mesh beacon is
* still generated by the driver.
+ * @NL80211_FEATURE_P2P_FIND_OFFLOAD: The driver supports P2P find phase
+ * offload.
+ * @NL80211_FEATURE_P2P_PROBE_RESP_OFFLOAD: When in P2P find phase,
+ * the device responds to probe-requests in hardware.
*/
enum nl80211_feature_flags {
NL80211_FEATURE_SK_TX_STATUS = 1 << 0,
@@ -3557,6 +3571,8 @@ enum nl80211_feature_flags {
NL80211_FEATURE_ADVERTISE_CHAN_LIMITS = 1 << 14,
NL80211_FEATURE_FULL_AP_CLIENT_STATE = 1 << 15,
NL80211_FEATURE_USERSPACE_MPM = 1 << 16,
+ NL80211_FEATURE_P2P_FIND_OFFLOAD = 1 << 17,
+ NL80211_FEATURE_P2P_PROBE_RESP_OFFLOAD = 1 << 18,
};

/**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index f924d45..d51aa2c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -378,6 +378,8 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_MDID] = { .type = NLA_U16 },
[NL80211_ATTR_IE_RIC] = { .type = NLA_BINARY,
.len = IEEE80211_MAX_DATA_LEN },
+ [NL80211_ATTR_MIN_DISCOVERABLE_INTERVAL] = { .type = NLA_U32 },
+ [NL80211_ATTR_MAX_DISCOVERABLE_INTERVAL] = { .type = NLA_U32 },
};

/* policy for the key attributes */
@@ -1417,6 +1419,8 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
}
CMD(start_p2p_device, START_P2P_DEVICE);
CMD(set_mcast_rate, SET_MCAST_RATE);
+ CMD(start_p2p_find, START_P2P_FIND);
+ CMD(stop_p2p_find, STOP_P2P_FIND);

#ifdef CONFIG_NL80211_TESTMODE
CMD(testmode_cmd, TESTMODE);
@@ -8196,6 +8200,115 @@ static int nl80211_update_ft_ies(struct sk_buff *skb, struct genl_info *info)
return rdev_update_ft_ies(rdev, dev, &ft_params);
}

+static int nl80211_start_p2p_find(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct wireless_dev *wdev = info->user_ptr[1];
+ struct wiphy *wiphy = &rdev->wiphy;
+ struct cfg80211_p2p_find_params params = {};
+ struct nlattr *attr_freq = info->attrs[NL80211_ATTR_SCAN_FREQUENCIES];
+ struct nlattr *attr;
+ int err, tmp, n_channels, i = 0;
+ struct ieee80211_channel **channels = NULL;
+
+ switch (wdev->iftype) {
+ case NL80211_IFTYPE_P2P_DEVICE:
+ case NL80211_IFTYPE_STATION:
+ break;
+ default:
+ return -EOPNOTSUPP;
+
+ }
+
+ if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE]))
+ return -EINVAL;
+
+ if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE_PROBE_RESP]))
+ return -EINVAL;
+
+ if (!(wiphy->features & NL80211_FEATURE_P2P_FIND_OFFLOAD) ||
+ !rdev->ops->start_p2p_find || !rdev->ops->stop_p2p_find)
+ return -EOPNOTSUPP;
+
+ if (rdev->scan_req)
+ return -EBUSY;
+
+ if (attr_freq) {
+ n_channels = validate_scan_freqs(attr_freq);
+ if (!n_channels)
+ return -EINVAL;
+
+ channels = kzalloc(n_channels * sizeof(*channels), GFP_KERNEL);
+ if (!channels)
+ return -ENOMEM;
+
+ /* user specified, bail out if channel not found */
+ nla_for_each_nested(attr, attr_freq, tmp) {
+ struct ieee80211_channel *chan;
+
+ chan = ieee80211_get_channel(wiphy, nla_get_u32(attr));
+
+ if (!chan)
+ return -EINVAL;
+
+ /* ignore disabled channels */
+ if (chan->flags & IEEE80211_CHAN_DISABLED)
+ continue;
+
+ params.channels[i] = chan;
+ i++;
+ }
+ if (!i) {
+ err = -EINVAL;
+ goto out_free;
+ }
+
+ params.n_channels = i;
+ params.channels = channels;
+ }
+
+
+ attr = info->attrs[NL80211_ATTR_IE];
+ if (attr) {
+ params.probe_ie_len = nla_len(attr);
+ params.probe_ie = nla_data(attr);
+ }
+
+ attr = info->attrs[NL80211_ATTR_IE_PROBE_RESP];
+ if (attr) {
+ params.probe_resp_ie_len = nla_len(attr);
+ params.probe_resp_ie = nla_data(attr);
+ }
+
+ attr = info->attrs[NL80211_ATTR_MIN_DISCOVERABLE_INTERVAL];
+ if (attr)
+ params.min_discoverable_interval = nla_get_u32(attr);
+
+ attr = info->attrs[NL80211_ATTR_MAX_DISCOVERABLE_INTERVAL];
+ if (attr)
+ params.max_discoverable_interval = nla_get_u32(attr);
+
+ err = rdev_start_p2p_find(rdev, wdev, &params);
+
+out_free:
+ kfree(channels);
+
+ return err;
+}
+
+static int nl80211_stop_p2p_find(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct wireless_dev *wdev = info->user_ptr[1];
+
+ if (!rdev->ops->start_p2p_find || !rdev->ops->stop_p2p_find)
+ return -EOPNOTSUPP;
+
+ rdev_stop_p2p_find(rdev, wdev);
+
+ return 0;
+}
+
#define NL80211_FLAG_NEED_WIPHY 0x01
#define NL80211_FLAG_NEED_NETDEV 0x02
#define NL80211_FLAG_NEED_RTNL 0x04
@@ -8873,6 +8986,22 @@ static struct genl_ops nl80211_ops[] = {
NL80211_FLAG_NEED_RTNL,
},
{
+ .cmd = NL80211_CMD_START_P2P_FIND,
+ .doit = nl80211_start_p2p_find,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
+ .cmd = NL80211_CMD_STOP_P2P_FIND,
+ .doit = nl80211_stop_p2p_find,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
.cmd = NL80211_CMD_GET_PROTOCOL_FEATURES,
.doit = nl80211_get_protocol_features,
.policy = nl80211_policy,
@@ -10547,6 +10676,34 @@ void cfg80211_tdls_oper_request(struct net_device *dev, const u8 *peer,
}
EXPORT_SYMBOL(cfg80211_tdls_oper_request);

+void cfg80211_p2p_find_notify(struct wireless_dev *wdev, int cmd, gfp_t gfp)
+{
+ struct wiphy *wiphy = wdev->wiphy;
+ struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
+ struct sk_buff *msg;
+ void *hdr;
+
+ trace_cfg80211_p2p_find_notify(wdev, cmd);
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return;
+
+ hdr = nl80211hdr_put(msg, 0, 0, 0, cmd);
+ if (!hdr) {
+ nlmsg_free(msg);
+ return;
+ }
+
+ if (genlmsg_end(msg, hdr) < 0) {
+ nlmsg_free(msg);
+ return;
+ }
+
+ genlmsg_multicast_netns(wiphy_net(&rdev->wiphy), msg, 0,
+ nl80211_scan_mcgrp.id, GFP_KERNEL);
+}
+EXPORT_SYMBOL(cfg80211_p2p_find_notify);
+
static int nl80211_netlink_notify(struct notifier_block * nb,
unsigned long state,
void *_notify)
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index d77e1c1..5e43d50 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -901,4 +901,23 @@ static inline int rdev_update_ft_ies(struct cfg80211_registered_device *rdev,
return ret;
}

+static inline int rdev_start_p2p_find(struct cfg80211_registered_device *rdev,
+ struct wireless_dev *wdev,
+ struct cfg80211_p2p_find_params *params)
+{
+ int ret;
+ trace_rdev_start_p2p_find(&rdev->wiphy, wdev, params);
+ ret = rdev->ops->start_p2p_find(&rdev->wiphy, wdev, params);
+ trace_rdev_return_int(&rdev->wiphy, ret);
+ return ret;
+}
+
+static inline void rdev_stop_p2p_find(struct cfg80211_registered_device *rdev,
+ struct wireless_dev *wdev)
+{
+ trace_rdev_stop_p2p_find(&rdev->wiphy, wdev);
+ rdev->ops->stop_p2p_find(&rdev->wiphy, wdev);
+ trace_rdev_return_void(&rdev->wiphy);
+}
+
#endif /* __CFG80211_RDEV_OPS */
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 23e063b..850dd3b 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -1805,6 +1805,26 @@ TRACE_EVENT(rdev_update_ft_ies,
WIPHY_PR_ARG, NETDEV_PR_ARG, __entry->md)
);

+TRACE_EVENT(rdev_start_p2p_find,
+ TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev,
+ struct cfg80211_p2p_find_params *params),
+ TP_ARGS(wiphy, wdev, params),
+ TP_STRUCT__entry(
+ WIPHY_ENTRY
+ WDEV_ENTRY
+ ),
+ TP_fast_assign(
+ WIPHY_ASSIGN;
+ WDEV_ASSIGN;
+ ),
+ TP_printk(WIPHY_PR_FMT ", " WDEV_PR_FMT, WIPHY_PR_ARG, WDEV_PR_ARG)
+);
+
+DEFINE_EVENT(wiphy_wdev_evt, rdev_stop_p2p_find,
+ TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev),
+ TP_ARGS(wiphy, wdev)
+);
+
/*************************************************************
* cfg80211 exported functions traces *
*************************************************************/
@@ -2459,6 +2479,21 @@ TRACE_EVENT(cfg80211_ft_event,
WIPHY_PR_ARG, NETDEV_PR_ARG, MAC_PR_ARG(target_ap))
);

+TRACE_EVENT(cfg80211_p2p_find_notify,
+ TP_PROTO(struct wireless_dev *wdev, int cmd),
+ TP_ARGS(wdev, cmd),
+ TP_STRUCT__entry(
+ WDEV_ENTRY
+ __field(int, cmd)
+ ),
+ TP_fast_assign(
+ WDEV_ASSIGN;
+ __entry->cmd = cmd;
+ ),
+ TP_printk(WDEV_PR_FMT ", cmd: %d", WDEV_PR_ARG,
+ __entry->cmd)
+);
+
#endif /* !__RDEV_OPS_TRACE || TRACE_HEADER_MULTI_READ */

#undef TRACE_INCLUDE_PATH
--
1.7.10.4



2013-03-07 09:31:50

by Vladimir Kondratiev

[permalink] [raw]
Subject: Re: [RFC] P2P find offload

Something like the following for method. Attribute for PCP resolution not included yet.

>From 44b47e039b767cdc68d9299889d12b7e008638d5 Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev <[email protected]>
Date: Mon, 4 Mar 2013 13:51:48 +0200
Subject: [PATCH] cfg80211: P2P find phase offload

Allow to implement P2P find phase in the driver/firmware.

Driver may provide methods start_p2p_find/stop_p2p_find in the cfg80211_ops;
to be called by supplicant through nl80211 layer.

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
include/net/cfg80211.h | 33 ++++++++++
include/uapi/linux/nl80211.h | 3 +
net/wireless/nl80211.c | 143 ++++++++++++++++++++++++++++++++++++++++++
net/wireless/rdev-ops.h | 20 ++++++
net/wireless/trace.h | 20 ++++++
5 files changed, 219 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index fa26129..8882310 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1722,6 +1722,26 @@ struct cfg80211_gtk_rekey_data {
};

/**
+ * struct cfg80211_p2p_find_params - parameters for P2P find
+ * @probe_ie: IE's for probe frames
+ * @probe_ie_len: length, bytes, of @probe_ie
+ * @probe_resp_ie: IE's for probe response frames
+ * @probe_resp_ie_len: length, bytes, of @probe_resp_ie
+ * @n_channels: number of channels to operate on
+ * @channels: channels to operate on
+ */
+struct cfg80211_p2p_find_params {
+ const u8 *probe_ie;
+ size_t probe_ie_len;
+ const u8 *probe_resp_ie;
+ size_t probe_resp_ie_len;
+
+ int n_channels;
+ /* Up to 4 social channels: 3 in 2.4GHz band + 1 in 60GHz band */
+ struct ieee80211_channel *channels[4];
+};
+
+/**
* struct cfg80211_ops - backend description for wireless configuration
*
* This struct is registered by fullmac card drivers and/or wireless stacks
@@ -1941,6 +1961,13 @@ struct cfg80211_gtk_rekey_data {
* advertise the support for MAC based ACL have to implement this callback.
*
* @start_radar_detection: Start radar detection in the driver.
+ *
+ * start_p2p_find: start P2P find phase
+ * Parameters include IEs for probe/probe resp and channels to operate on
+ * Parameters are not retained after call, driver need to copy data if
+ * it need it later.
+ * stop_p2p_find: stop P2P find phase
+ *
*/
struct cfg80211_ops {
int (*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -2168,6 +2195,12 @@ struct cfg80211_ops {
int (*start_radar_detection)(struct wiphy *wiphy,
struct net_device *dev,
struct cfg80211_chan_def *chandef);
+
+ int (*start_p2p_find)(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ struct cfg80211_p2p_find_params *params);
+ void (*stop_p2p_find)(struct wiphy *wiphy,
+ struct wireless_dev *wdev);
};

/*
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index c46bb01..6a0c263 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -765,6 +765,9 @@ enum nl80211_commands {

NL80211_CMD_RADAR_DETECT,

+ NL80211_CMD_START_P2P_FIND,
+ NL80211_CMD_STOP_P2P_FIND,
+
/* add new commands above here */

/* used to define NL80211_CMD_MAX below */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 35545cc..c52e4b9 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1207,6 +1207,8 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 portid, u32 seq, int flag
}
CMD(start_p2p_device, START_P2P_DEVICE);
CMD(set_mcast_rate, SET_MCAST_RATE);
+ CMD(start_p2p_find, START_P2P_FIND);
+ CMD(stop_p2p_find, STOP_P2P_FIND);

#ifdef CONFIG_NL80211_TESTMODE
CMD(testmode_cmd, TESTMODE);
@@ -7823,6 +7825,131 @@ static int nl80211_stop_p2p_device(struct sk_buff *skb, struct genl_info *info)
return 0;
}

+static int nl80211_start_p2p_find(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct wireless_dev *wdev = info->user_ptr[1];
+ struct wiphy *wiphy = &rdev->wiphy;
+ struct cfg80211_p2p_find_params params = {};
+ struct nlattr *attr_freq = info->attrs[NL80211_ATTR_SCAN_FREQUENCIES];
+ struct nlattr *attr;
+ struct sk_buff *msg = NULL;
+ void *hdr = NULL;
+ int err, tmp, n_channels, i;
+
+ if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE]))
+ return -EINVAL;
+
+ if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE_PROBE_RESP]))
+ return -EINVAL;
+
+ if (!rdev->ops->start_p2p_find || !rdev->ops->stop_p2p_find)
+ return -EOPNOTSUPP;
+
+ if (rdev->scan_req)
+ return -EBUSY;
+
+ if (attr_freq) {
+ n_channels = validate_scan_freqs(attr_freq);
+ if (!n_channels || n_channels > ARRAY_SIZE(params.channels))
+ return -EINVAL;
+ }
+
+ i = 0;
+ if (attr_freq) {
+ /* user specified, bail out if channel not found */
+ nla_for_each_nested(attr, attr_freq, tmp) {
+ struct ieee80211_channel *chan;
+
+ chan = ieee80211_get_channel(wiphy, nla_get_u32(attr));
+
+ if (!chan)
+ return -EINVAL;
+
+ /* ignore disabled channels */
+ if (chan->flags & IEEE80211_CHAN_DISABLED)
+ continue;
+
+ params.channels[i] = chan;
+ i++;
+ }
+ if (!i)
+ return -EINVAL;
+ }
+
+ params.n_channels = i;
+
+ i = 0;
+
+ attr = info->attrs[NL80211_ATTR_IE];
+ if (attr) {
+ params.probe_ie_len = nla_len(attr);
+ params.probe_ie = nla_data(attr);
+ }
+
+ attr = info->attrs[NL80211_ATTR_IE_PROBE_RESP];
+ if (attr) {
+ params.probe_resp_ie_len = nla_len(attr);
+ params.probe_resp_ie = nla_data(attr);
+ }
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ hdr = nl80211hdr_put(msg, info->snd_portid, info->snd_seq, 0,
+ NL80211_CMD_START_P2P_FIND);
+
+ if (IS_ERR(hdr)) {
+ err = PTR_ERR(hdr);
+ goto free_msg;
+ }
+
+ err = rdev_start_p2p_find(rdev, wdev, &params);
+ if (err)
+ goto free_msg;
+
+ genlmsg_end(msg, hdr);
+ return genlmsg_reply(msg, info);
+
+ free_msg:
+ nlmsg_free(msg);
+ return err;
+}
+
+static int nl80211_stop_p2p_find(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct wireless_dev *wdev = info->user_ptr[1];
+ struct sk_buff *msg = NULL;
+ void *hdr = NULL;
+ int err;
+
+ if (!rdev->ops->start_p2p_find || !rdev->ops->stop_p2p_find)
+ return -EOPNOTSUPP;
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ hdr = nl80211hdr_put(msg, info->snd_portid, info->snd_seq, 0,
+ NL80211_CMD_STOP_P2P_FIND);
+
+ if (IS_ERR(hdr)) {
+ err = PTR_ERR(hdr);
+ goto free_msg;
+ }
+
+ rdev_stop_p2p_find(rdev, wdev);
+
+ genlmsg_end(msg, hdr);
+ return genlmsg_reply(msg, info);
+
+ free_msg:
+ nlmsg_free(msg);
+ return err;
+}
+
#define NL80211_FLAG_NEED_WIPHY 0x01
#define NL80211_FLAG_NEED_NETDEV 0x02
#define NL80211_FLAG_NEED_RTNL 0x04
@@ -8499,6 +8626,22 @@ static struct genl_ops nl80211_ops[] = {
.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
NL80211_FLAG_NEED_RTNL,
},
+ {
+ .cmd = NL80211_CMD_START_P2P_FIND,
+ .doit = nl80211_start_p2p_find,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
+ .cmd = NL80211_CMD_STOP_P2P_FIND,
+ .doit = nl80211_stop_p2p_find,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
};

static struct genl_multicast_group nl80211_mlme_mcgrp = {
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 422d382..eea01aa 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -887,4 +887,24 @@ static inline int rdev_set_mac_acl(struct cfg80211_registered_device *rdev,
trace_rdev_return_int(&rdev->wiphy, ret);
return ret;
}
+
+static inline int rdev_start_p2p_find(struct cfg80211_registered_device *rdev,
+ struct wireless_dev *wdev,
+ struct cfg80211_p2p_find_params *params)
+{
+ int ret;
+ trace_rdev_start_p2p_find(&rdev->wiphy, wdev, params);
+ ret = rdev->ops->start_p2p_find(&rdev->wiphy, wdev, params);
+ trace_rdev_return_int(&rdev->wiphy, ret);
+ return ret;
+}
+
+static inline void rdev_stop_p2p_find(struct cfg80211_registered_device *rdev,
+ struct wireless_dev *wdev)
+{
+ trace_rdev_stop_p2p_find(&rdev->wiphy, wdev);
+ rdev->ops->stop_p2p_find(&rdev->wiphy, wdev);
+ trace_rdev_return_void(&rdev->wiphy);
+}
+
#endif /* __CFG80211_RDEV_OPS */
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index b7a5313..20ba224 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -1785,6 +1785,26 @@ TRACE_EVENT(rdev_set_mac_acl,
WIPHY_PR_ARG, NETDEV_PR_ARG, __entry->acl_policy)
);

+TRACE_EVENT(rdev_start_p2p_find,
+ TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev,
+ struct cfg80211_p2p_find_params *params),
+ TP_ARGS(wiphy, wdev, params),
+ TP_STRUCT__entry(
+ WIPHY_ENTRY
+ WDEV_ENTRY
+ ),
+ TP_fast_assign(
+ WIPHY_ASSIGN;
+ WDEV_ASSIGN;
+ ),
+ TP_printk(WIPHY_PR_FMT ", " WDEV_PR_FMT, WIPHY_PR_ARG, WDEV_PR_ARG)
+);
+
+DEFINE_EVENT(wiphy_wdev_evt, rdev_stop_p2p_find,
+ TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev),
+ TP_ARGS(wiphy, wdev)
+);
+
/*************************************************************
* cfg80211 exported functions traces *
*************************************************************/
--
1.7.10.4



2013-03-16 12:57:10

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC] P2P find offload

On Thu, 2013-03-07 at 16:10 +0200, Vladimir Kondratiev wrote:

> > > + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> > > + if (!msg)
> > > + return -ENOMEM;
> > > +
> > > + hdr = nl80211hdr_put(msg, info->snd_portid, info->snd_seq, 0,
> > > + NL80211_CMD_START_P2P_FIND);
> >
> > Err? What's the value of sending a reply back here? It would seem maybe
> > appropriate to send one when it *actually* started, but you haven't
> > implemented that.
> >
> > > + hdr = nl80211hdr_put(msg, info->snd_portid, info->snd_seq, 0,
> > > + NL80211_CMD_STOP_P2P_FIND);
> >
> > same here ...
> Yes, agree. I'll add indications, like ROC do.

Is there really much value in indications, rather than having it start
right away? Similarly, in particular if sending probe responses is
offloaded, is there really any reason for userspace to care? OTOH, we
also send an indication when a scan starts, so maybe it's useful that
way. Just seems that the code requesting this would assume that if it
didn't get an error, it will be started, and not really care about when
it *really* started operating.

johannes


2013-03-16 12:57:10

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC] P2P find offload


> Allow to implement P2P find phase in the driver/firmware.
>
> Offload scheme designed as follows:
>
> - Driver provide methods start_p2p_find and stop_p2p_find in the cfg80211_ops;
> indicates support for p2p find offload by setting feature
> NL80211_FEATURE_P2P_FIND_OFFLOAD

Why not just indicate the command presence rather than a feature flag?

> - perform legacy scan, through driver's cfg80211_ops 'scan' method

Why the legacy scan? btw this is what I was worried about -- this API
robs us of the ability to do "progressive" p2p scan.

> - configure rx management filter to get probe-request and probe-response frames

Interesting, so this is pretty much bypassing the scan BSS table
mechanism. I guess it makes sense though.

> - driver start p2p find with hardware and notify wpa_supplicant with
> cfg80211_p2p_find_notify(NL80211_CMD_START_P2P_FIND)

See other mail -- is that really useful?

> cfg80211_p2p_find_notify(NL80211_CMD_STOP_P2P_FIND)

btw, if both are needed I'd prefer not to give a command number here,
it's useless to give anything else and this is a bit confusing.

> /**
> + * struct cfg80211_p2p_find_params - parameters for P2P find
> + * @probe_ie: IE's for probe frames

probe request, presumably?

Also, this is probably *additional* IEs, i.e. supported rates etc. are
expected to be created by the driver?

> + * @probe_resp_ie: IE's for probe response frames

similarly here?

> + * @probe_resp_ie_len: length, bytes, of @probe_resp_ie
> + * @min_discoverable_interval and

need : otherwise it's not valid kernel-doc

> + * @NL80211_ATTR_MIN_DISCOVERABLE_INTERVAL,

same here

> + genlmsg_multicast_netns(wiphy_net(&rdev->wiphy), msg, 0,
> + nl80211_scan_mcgrp.id, GFP_KERNEL);

indentation


> +static inline int rdev_start_p2p_find(struct cfg80211_registered_device *rdev,
> + struct wireless_dev *wdev,
> + struct cfg80211_p2p_find_params *params)

indentation


> +static inline void rdev_stop_p2p_find(struct cfg80211_registered_device *rdev,
> + struct wireless_dev *wdev)

ditto

> + TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev,
> + struct cfg80211_p2p_find_params *params),

ditto

> + TP_printk(WDEV_PR_FMT ", cmd: %d", WDEV_PR_ARG,
> + __entry->cmd)

ditto

johannes


2013-03-07 07:10:32

by Vladimir Kondratiev

[permalink] [raw]
Subject: Re: [RFC] P2P find offload

On Monday, March 04, 2013 04:47:59 PM Johannes Berg wrote:
> On Wed, 2013-02-27 at 14:24 +0200, Vladimir Kondratiev wrote:
>
> > Enable p2p find phase offload to the driver.
> > Add methods for the struct cfg80211_ops, like
> >
> > int (*start_p2p_find)(struct wiphy *wiphy,
> > struct cfg80211_p2p_find_params *params);
> > void (*stop_p2p_find)(struct wiphy *wiphy);
> >
> > where struct cfg80211_p2p_find_params includes info elements
> > to be added for probe request and probe response frames;
> > social channels etc.
>
> We had something like this before. I think you need to specify not the
> wiphy but a netdev though, to know what MAC address to use. In fact, not
> a netdev but a wdev, so it can work on P2P_Device type wdevs.
Why do we need wdev here? It should be similar to scan, as p2p scan composed
of legacy scan and p2p find. Scan have only wiphy.
It's no problem to add wdev as well, of course. I'll add it.

>
> Also, timings? Or is that left to the driver?
Yes, timing is up to the driver. Point is, firmware may have its reasons
when switch search/listen phases.

>
> > wpa_supplicant will call these methods through nl80211.
> >
> > Driver responsible for toggling between search and listen states,
> > reporting probe request/response frames to the user space.
> >
> > Driver/firmware may answer to the probe request frames on itself,
> > in this case probe requests are still reported.
>
> wpa_s will need to know whether or not it responded, unless you mandate
> that it must respond by itself -- not sure if you should do that though?
Same way as today wpa_s know whether driver/firmware answer probes -
WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD. Or, should I add one more flag, like
WIPHY_FLAG_P2P_PROBE_RESP_OFFLOAD?

>
> > To satisfy requirements for 60GHz band, additional attribute 'pcp_resolution'
> > shall be added to the reported frames, indicating result of the PCP resolution.
> > This attribute carries result of PCP factor comparison between probe request
> > and response, as defined in the spec for 60GHz. It is enum having values
> > 'undefined', 'win' and 'lose'.
>
> ?
>
> > For the 2.4GHz band devices, PCP resolution is not used and pcp_resolution
> > attribute set to 'undefined'
>
> No ... you'd just leave out the attribute instead of having a special
> undefined value.
I think I was not clear here. On nl80211 - yes, just leave out attribute if it is
'undefined', I meant this 'undefined' to be used on driver API level.
>
> johannes
>

2013-03-17 08:56:17

by Vladimir Kondratiev

[permalink] [raw]
Subject: Re: [RFC] P2P find offload

On Friday, March 15, 2013 07:40:23 PM Johannes Berg wrote:
>
> > Allow to implement P2P find phase in the driver/firmware.
> >
> > Offload scheme designed as follows:
> >
> > - Driver provide methods start_p2p_find and stop_p2p_find in the cfg80211_ops;
> > indicates support for p2p find offload by setting feature
> > NL80211_FEATURE_P2P_FIND_OFFLOAD
>
> Why not just indicate the command presence rather than a feature flag?

I also thought this way, it was my impression you was asking for feature flag
earlier:
--cut--
> You're also entirely missing feature advertising, so userspace can only
> guess whether it's supported or not ...
--cut--

But I should be mis-interpreting, let's remove feature flag.

> > - perform legacy scan, through driver's cfg80211_ops 'scan' method
>
> Why the legacy scan? btw this is what I was worried about -- this API
> robs us of the ability to do "progressive" p2p scan.

It is how P2P spec defines device discovery (P2P specification v1.3_D4):

3.1.2.1 Basic mechanisms of Device Discovery
The objective of P2P Device Discovery is to find P2P Devices and quickly
determine the P2P Device to which a connection will be attempted.
P2P Device Discovery consists of two major phases: Scan and Find, which are
described in detail in the following sections.
<skip>
3.1.2.1.2 Scan Phase
The Scan Phase uses the scanning process defined in IEEE Std 802.11-2007 [1]
for a non-DMG network and defined in [15][16] for a DMG network. It may be
used by a P2P Device to find P2P Devices or P2P Groups and to locate the best
potential Operating Channel to establish a P2P Group. In the Scan Phase,
devices collect information about surrounding devices or networks by scanning
all supported channels.


So it is spec that says P2P scan includes legacy scan. Actually, prpposed API
do not mandate it, in makes possible to use any scan variations. Comment text
outlines typical scan flow.

For "progressive" P2P scan - do you mean normal scan or find phase? For normal
scan, nothing changes and one can use any scan optimisations. For find phase -
spec don't let you do so, it pretty much mandates what and when to do.

> > - configure rx management filter to get probe-request and probe-response frames
>
> Interesting, so this is pretty much bypassing the scan BSS table
> mechanism. I guess it makes sense though.

Yes, indeed. Probe requests it is how 'listening' peer discovers the
'searching' one. And, I don't want P2P stuff to mix with BSS table, as
it likely to confuse legacy scan logic.

> > - driver start p2p find with hardware and notify wpa_supplicant with
> > cfg80211_p2p_find_notify(NL80211_CMD_START_P2P_FIND)
>
> See other mail -- is that really useful?

I copied it here:

> Is there really much value in indications, rather than having it start
> right away? Similarly, in particular if sending probe responses is
> offloaded, is there really any reason for userspace to care? OTOH, we
> also send an indication when a scan starts, so maybe it's useful that
> way. Just seems that the code requesting this would assume that if it
> didn't get an error, it will be started, and not really care about when
> it really started operating.

There are 2 points:
1) timing. For hardware, it may take time to really start find phase.
For card I am working on now, 60G one, it tooks some 150ms.
I suppose, upper layers may be confused with this delay.
2) hardware may decide to end find phase on its own, in this case 'end'
notification is a must; it seems more consistent to provide both 'start'
and 'end'.

> > cfg80211_p2p_find_notify(NL80211_CMD_STOP_P2P_FIND)
>
> btw, if both are needed I'd prefer not to give a command number here,
> it's useless to give anything else and this is a bit confusing.

OK, will do it. Split this to 2 functions,
cfg80211_p2p_find_notify_start() and cfg80211_p2p_find_notify_end(),
yes?

Open question: can supplicant be interested in more details, like when
'search' started, and when 'listen' started for particular channel?

> > /**
> > + * struct cfg80211_p2p_find_params - parameters for P2P find
> > + * @probe_ie: IE's for probe frames
>
> probe request, presumably?

Yes, but spec call it 'probe', it does not use 'probe request' term.

>
> Also, this is probably *additional* IEs, i.e. supported rates etc. are
> expected to be created by the driver?

In params for start_ap, it is called 'extra' IE's;
I will call it the same here for consistency.

>
> > + * @probe_resp_ie: IE's for probe response frames
>
> similarly here?
>
> > + * @probe_resp_ie_len: length, bytes, of @probe_resp_ie
> > + * @min_discoverable_interval and
>
> need : otherwise it's not valid kernel-doc

OK

>
> > + * @NL80211_ATTR_MIN_DISCOVERABLE_INTERVAL,
>
> same here

OK

>
> > + genlmsg_multicast_netns(wiphy_net(&rdev->wiphy), msg, 0,
> > + nl80211_scan_mcgrp.id, GFP_KERNEL);
>
> indentation
I double checked, identation seems OK. Checkpatch also fine.

For trace.h, whole file uses one-tab identation after TRACE_EVENT
or DEFINE_EVENT; yes it is not what checkpatch likes.
But, should I follow the file style or properly indent my additions
in non-consistent with the rest of file style?
I left it with the same identation as the rest of file.


> johannes

Patch follows

Thanks, Vladimir

>From fa8c2a98b61ad8cbdd42ac41385f5839b49dba6a Mon Sep 17 00:00:00 2001
From: Vladimir Kondratiev <[email protected]>
Date: Sun, 10 Mar 2013 15:52:37 +0200
Subject: [PATCH v3] cfg80211: P2P find phase offload

Allow to implement P2P find phase in the driver/firmware.

Offload scheme designed as follows:

- Driver provide methods start_p2p_find and stop_p2p_find in the cfg80211_ops;
- Driver indicate firmware or driver responds to the probe requests by setting
feature NL80211_FEATURE_P2P_PROBE_RESP_OFFLOAD
- wpa_supplicant analyses methods supported to discover p2p offload support;
- wpa_supplicant analyses feature flags to discover p2p probe response
offload support;
to perform p2p scan, wpa_supplicant:
- perform legacy scan, through driver's cfg80211_ops 'scan' method
- configure rx management filter to get probe-request and probe-response frames
- start p2p find via driver's cfg80211_ops start_p2p_find method
- driver start p2p find with hardware and notify wpa_supplicant with
cfg80211_p2p_find_notify_start()
- driver/firmware toggle search/listen states. Received probe-request and
probe-response frames passed to the wpa_supplicant via cfg80211_rx_mgmt
- when wpa_supplicant wants to stop p2p find, it calls driver's
cfg80211_ops stop_p2p_find method. Alternatively, driver/firmware may decide
to stop p2p find. In all cases, driver notifies wpa_supplicant using
cfg80211_p2p_find_notify_end()

All driver to user space communication done through nl80211 layer.

Signed-off-by: Vladimir Kondratiev <[email protected]>
---
include/net/cfg80211.h | 56 ++++++++++++++
include/uapi/linux/nl80211.h | 13 ++++
net/wireless/nl80211.c | 167 ++++++++++++++++++++++++++++++++++++++++++
net/wireless/rdev-ops.h | 19 +++++
net/wireless/trace.h | 34 +++++++++
5 files changed, 289 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index bdba9b6..e2fdbb8 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1780,6 +1780,30 @@ struct cfg80211_update_ft_ies_params {
};

/**
+ * struct cfg80211_p2p_find_params - parameters for P2P find
+ * @probe_ie: extra IE's for probe frames
+ * @probe_ie_len: length, bytes, of @probe_ie
+ * @probe_resp_ie: extra IE's for probe response frames
+ * @probe_resp_ie_len: length, bytes, of @probe_resp_ie
+ * @min_discoverable_interval: and
+ * @max_discoverable_interval: min/max for random multiplier of 100TU's
+ * for the listen state duration
+ * @n_channels: number of channels to operate on
+ * @channels: channels to operate on
+ */
+struct cfg80211_p2p_find_params {
+ const u8 *probe_ie;
+ size_t probe_ie_len;
+ const u8 *probe_resp_ie;
+ size_t probe_resp_ie_len;
+ u32 min_discoverable_interval;
+ u32 max_discoverable_interval;
+
+ int n_channels;
+ struct ieee80211_channel **channels;
+};
+
+/**
* struct cfg80211_ops - backend description for wireless configuration
*
* This struct is registered by fullmac card drivers and/or wireless stacks
@@ -1998,6 +2022,15 @@ struct cfg80211_update_ft_ies_params {
* advertise the support for MAC based ACL have to implement this callback.
*
* @start_radar_detection: Start radar detection in the driver.
+ *
+ * start_p2p_find: start P2P find phase
+ * Parameters include IEs for probe/probe-resp frames;
+ * and channels to operate on.
+ * Parameters are not retained after call, driver need to copy data if
+ * it need it later.
+ * P2P find can't run concurrently with ROC or scan, and driver should
+ * check this.
+ * stop_p2p_find: stop P2P find phase
*/
struct cfg80211_ops {
int (*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -2227,6 +2260,12 @@ struct cfg80211_ops {
struct cfg80211_chan_def *chandef);
int (*update_ft_ies)(struct wiphy *wiphy, struct net_device *dev,
struct cfg80211_update_ft_ies_params *ftie);
+
+ int (*start_p2p_find)(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ struct cfg80211_p2p_find_params *params);
+ void (*stop_p2p_find)(struct wiphy *wiphy,
+ struct wireless_dev *wdev);
};

/*
@@ -4122,6 +4161,23 @@ void cfg80211_report_wowlan_wakeup(struct wireless_dev *wdev,
struct cfg80211_wowlan_wakeup *wakeup,
gfp_t gfp);

+/**
+ * cfg80211_p2p_find_notify_start - report p2p find state started
+ * @wdev: the wireless device reporting the event
+ * @gfp: allocation flags
+ */
+void cfg80211_p2p_find_notify_start(struct wireless_dev *wdev, gfp_t gfp);
+
+/**
+ * cfg80211_p2p_find_notify_end - report p2p find state ended
+ * @wdev: the wireless device reporting the event
+ * @gfp: allocation flags
+ *
+ * p2p find state may be ended either unsolicited or in response to
+ * @ops->p2p_stop_find
+ */
+void cfg80211_p2p_find_notify_end(struct wireless_dev *wdev, gfp_t gfp);
+
/* Logging, debugging and troubleshooting/diagnostic helpers. */

/* wiphy_printk helpers, similar to dev_printk */
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 79da871..c8a8fe0 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -798,6 +798,9 @@ enum nl80211_commands {
NL80211_CMD_UPDATE_FT_IES,
NL80211_CMD_FT_EVENT,

+ NL80211_CMD_START_P2P_FIND,
+ NL80211_CMD_STOP_P2P_FIND,
+
/* add new commands above here */

/* used to define NL80211_CMD_MAX below */
@@ -1414,6 +1417,10 @@ enum nl80211_commands {
* @NL80211_ATTR_IE_RIC: Resource Information Container Information
* Element
*
+ * @NL80211_ATTR_MIN_DISCOVERABLE_INTERVAL:
+ * @NL80211_ATTR_MAX_DISCOVERABLE_INTERVAL: min/max discoverable interval
+ * for the p2p find, represented as u32
+ *
* @NL80211_ATTR_MAX: highest attribute number currently defined
* @__NL80211_ATTR_AFTER_LAST: internal use
*/
@@ -1709,6 +1716,9 @@ enum nl80211_attrs {
NL80211_ATTR_MDID,
NL80211_ATTR_IE_RIC,

+ NL80211_ATTR_MIN_DISCOVERABLE_INTERVAL,
+ NL80211_ATTR_MAX_DISCOVERABLE_INTERVAL,
+
/* add attributes here, update the policy in nl80211.c */

__NL80211_ATTR_AFTER_LAST,
@@ -3538,6 +3548,8 @@ enum nl80211_ap_sme_features {
* Peering Management entity which may be implemented by registering for
* beacons or NL80211_CMD_NEW_PEER_CANDIDATE events. The mesh beacon is
* still generated by the driver.
+ * @NL80211_FEATURE_P2P_PROBE_RESP_OFFLOAD: When in P2P find phase,
+ * the device responds to probe-requests in hardware.
*/
enum nl80211_feature_flags {
NL80211_FEATURE_SK_TX_STATUS = 1 << 0,
@@ -3557,6 +3569,7 @@ enum nl80211_feature_flags {
NL80211_FEATURE_ADVERTISE_CHAN_LIMITS = 1 << 14,
NL80211_FEATURE_FULL_AP_CLIENT_STATE = 1 << 15,
NL80211_FEATURE_USERSPACE_MPM = 1 << 16,
+ NL80211_FEATURE_P2P_PROBE_RESP_OFFLOAD = 1 << 17,
};

/**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index f924d45..374a667 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -378,6 +378,8 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
[NL80211_ATTR_MDID] = { .type = NLA_U16 },
[NL80211_ATTR_IE_RIC] = { .type = NLA_BINARY,
.len = IEEE80211_MAX_DATA_LEN },
+ [NL80211_ATTR_MIN_DISCOVERABLE_INTERVAL] = { .type = NLA_U32 },
+ [NL80211_ATTR_MAX_DISCOVERABLE_INTERVAL] = { .type = NLA_U32 },
};

/* policy for the key attributes */
@@ -1417,6 +1419,8 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
}
CMD(start_p2p_device, START_P2P_DEVICE);
CMD(set_mcast_rate, SET_MCAST_RATE);
+ CMD(start_p2p_find, START_P2P_FIND);
+ CMD(stop_p2p_find, STOP_P2P_FIND);

#ifdef CONFIG_NL80211_TESTMODE
CMD(testmode_cmd, TESTMODE);
@@ -8196,6 +8200,113 @@ static int nl80211_update_ft_ies(struct sk_buff *skb, struct genl_info *info)
return rdev_update_ft_ies(rdev, dev, &ft_params);
}

+static int nl80211_start_p2p_find(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct wireless_dev *wdev = info->user_ptr[1];
+ struct wiphy *wiphy = &rdev->wiphy;
+ struct cfg80211_p2p_find_params params = {};
+ struct nlattr *attr_freq = info->attrs[NL80211_ATTR_SCAN_FREQUENCIES];
+ struct nlattr *attr;
+ int err, tmp, n_channels, i = 0;
+ struct ieee80211_channel **channels = NULL;
+
+ switch (wdev->iftype) {
+ case NL80211_IFTYPE_P2P_DEVICE:
+ case NL80211_IFTYPE_STATION:
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE]))
+ return -EINVAL;
+
+ if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE_PROBE_RESP]))
+ return -EINVAL;
+
+ if (!rdev->ops->start_p2p_find || !rdev->ops->stop_p2p_find)
+ return -EOPNOTSUPP;
+
+ if (rdev->scan_req)
+ return -EBUSY;
+
+ if (attr_freq) {
+ n_channels = validate_scan_freqs(attr_freq);
+ if (!n_channels)
+ return -EINVAL;
+
+ channels = kzalloc(n_channels * sizeof(*channels), GFP_KERNEL);
+ if (!channels)
+ return -ENOMEM;
+
+ /* user specified, bail out if channel not found */
+ nla_for_each_nested(attr, attr_freq, tmp) {
+ struct ieee80211_channel *chan;
+
+ chan = ieee80211_get_channel(wiphy, nla_get_u32(attr));
+
+ if (!chan)
+ return -EINVAL;
+
+ /* ignore disabled channels */
+ if (chan->flags & IEEE80211_CHAN_DISABLED)
+ continue;
+
+ params.channels[i] = chan;
+ i++;
+ }
+ if (!i) {
+ err = -EINVAL;
+ goto out_free;
+ }
+
+ params.n_channels = i;
+ params.channels = channels;
+ }
+
+
+ attr = info->attrs[NL80211_ATTR_IE];
+ if (attr) {
+ params.probe_ie_len = nla_len(attr);
+ params.probe_ie = nla_data(attr);
+ }
+
+ attr = info->attrs[NL80211_ATTR_IE_PROBE_RESP];
+ if (attr) {
+ params.probe_resp_ie_len = nla_len(attr);
+ params.probe_resp_ie = nla_data(attr);
+ }
+
+ attr = info->attrs[NL80211_ATTR_MIN_DISCOVERABLE_INTERVAL];
+ if (attr)
+ params.min_discoverable_interval = nla_get_u32(attr);
+
+ attr = info->attrs[NL80211_ATTR_MAX_DISCOVERABLE_INTERVAL];
+ if (attr)
+ params.max_discoverable_interval = nla_get_u32(attr);
+
+ err = rdev_start_p2p_find(rdev, wdev, &params);
+
+out_free:
+ kfree(channels);
+
+ return err;
+}
+
+static int nl80211_stop_p2p_find(struct sk_buff *skb, struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct wireless_dev *wdev = info->user_ptr[1];
+
+ if (!rdev->ops->start_p2p_find || !rdev->ops->stop_p2p_find)
+ return -EOPNOTSUPP;
+
+ rdev_stop_p2p_find(rdev, wdev);
+
+ return 0;
+}
+
#define NL80211_FLAG_NEED_WIPHY 0x01
#define NL80211_FLAG_NEED_NETDEV 0x02
#define NL80211_FLAG_NEED_RTNL 0x04
@@ -8873,6 +8984,22 @@ static struct genl_ops nl80211_ops[] = {
NL80211_FLAG_NEED_RTNL,
},
{
+ .cmd = NL80211_CMD_START_P2P_FIND,
+ .doit = nl80211_start_p2p_find,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
+ .cmd = NL80211_CMD_STOP_P2P_FIND,
+ .doit = nl80211_stop_p2p_find,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
+ {
.cmd = NL80211_CMD_GET_PROTOCOL_FEATURES,
.doit = nl80211_get_protocol_features,
.policy = nl80211_policy,
@@ -10547,6 +10674,46 @@ void cfg80211_tdls_oper_request(struct net_device *dev, const u8 *peer,
}
EXPORT_SYMBOL(cfg80211_tdls_oper_request);

+static
+void cfg80211_p2p_find_notify(struct wireless_dev *wdev, int cmd, gfp_t gfp)
+{
+ struct wiphy *wiphy = wdev->wiphy;
+ struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
+ struct sk_buff *msg;
+ void *hdr;
+
+ trace_cfg80211_p2p_find_notify(wdev, cmd);
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return;
+
+ hdr = nl80211hdr_put(msg, 0, 0, 0, cmd);
+ if (!hdr) {
+ nlmsg_free(msg);
+ return;
+ }
+
+ if (genlmsg_end(msg, hdr) < 0) {
+ nlmsg_free(msg);
+ return;
+ }
+
+ genlmsg_multicast_netns(wiphy_net(&rdev->wiphy), msg, 0,
+ nl80211_scan_mcgrp.id, GFP_KERNEL);
+}
+
+void cfg80211_p2p_find_notify_start(struct wireless_dev *wdev, gfp_t gfp)
+{
+ cfg80211_p2p_find_notify(wdev, NL80211_CMD_START_P2P_FIND, gfp);
+}
+EXPORT_SYMBOL(cfg80211_p2p_find_notify_start);
+
+void cfg80211_p2p_find_notify_end(struct wireless_dev *wdev, gfp_t gfp)
+{
+ cfg80211_p2p_find_notify(wdev, NL80211_CMD_STOP_P2P_FIND, gfp);
+}
+EXPORT_SYMBOL(cfg80211_p2p_find_notify_end);
+
static int nl80211_netlink_notify(struct notifier_block * nb,
unsigned long state,
void *_notify)
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index d77e1c1..5e43d50 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -901,4 +901,23 @@ static inline int rdev_update_ft_ies(struct cfg80211_registered_device *rdev,
return ret;
}

+static inline int rdev_start_p2p_find(struct cfg80211_registered_device *rdev,
+ struct wireless_dev *wdev,
+ struct cfg80211_p2p_find_params *params)
+{
+ int ret;
+ trace_rdev_start_p2p_find(&rdev->wiphy, wdev, params);
+ ret = rdev->ops->start_p2p_find(&rdev->wiphy, wdev, params);
+ trace_rdev_return_int(&rdev->wiphy, ret);
+ return ret;
+}
+
+static inline void rdev_stop_p2p_find(struct cfg80211_registered_device *rdev,
+ struct wireless_dev *wdev)
+{
+ trace_rdev_stop_p2p_find(&rdev->wiphy, wdev);
+ rdev->ops->stop_p2p_find(&rdev->wiphy, wdev);
+ trace_rdev_return_void(&rdev->wiphy);
+}
+
#endif /* __CFG80211_RDEV_OPS */
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 23e063b..d2faecc 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -1805,6 +1805,26 @@ TRACE_EVENT(rdev_update_ft_ies,
WIPHY_PR_ARG, NETDEV_PR_ARG, __entry->md)
);

+TRACE_EVENT(rdev_start_p2p_find,
+ TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev,
+ struct cfg80211_p2p_find_params *params),
+ TP_ARGS(wiphy, wdev, params),
+ TP_STRUCT__entry(
+ WIPHY_ENTRY
+ WDEV_ENTRY
+ ),
+ TP_fast_assign(
+ WIPHY_ASSIGN;
+ WDEV_ASSIGN;
+ ),
+ TP_printk(WIPHY_PR_FMT ", " WDEV_PR_FMT, WIPHY_PR_ARG, WDEV_PR_ARG)
+);
+
+DEFINE_EVENT(wiphy_wdev_evt, rdev_stop_p2p_find,
+ TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev),
+ TP_ARGS(wiphy, wdev)
+);
+
/*************************************************************
* cfg80211 exported functions traces *
*************************************************************/
@@ -2459,6 +2479,20 @@ TRACE_EVENT(cfg80211_ft_event,
WIPHY_PR_ARG, NETDEV_PR_ARG, MAC_PR_ARG(target_ap))
);

+TRACE_EVENT(cfg80211_p2p_find_notify,
+ TP_PROTO(struct wireless_dev *wdev, int cmd),
+ TP_ARGS(wdev, cmd),
+ TP_STRUCT__entry(
+ WDEV_ENTRY
+ __field(int, cmd)
+ ),
+ TP_fast_assign(
+ WDEV_ASSIGN;
+ __entry->cmd = cmd;
+ ),
+ TP_printk(WDEV_PR_FMT ", cmd: %d", WDEV_PR_ARG, __entry->cmd)
+);
+
#endif /* !__RDEV_OPS_TRACE || TRACE_HEADER_MULTI_READ */

#undef TRACE_INCLUDE_PATH
--
1.7.10.4





2013-03-19 20:27:57

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC] P2P find offload

On Tue, 2013-03-19 at 19:35 +0200, Vladimir Kondratiev wrote:
> > I was under the impression that wpa_s allowed "progressive" (i.e.
> one
> > non-social channel per iteration) even in find? I could be wrong
> though,
> > maybe only in search.

> wpa_s do it in that "legacy" scan before find phase

No: p2p_find type=progressive

If this will no longer be supported, I guess you can say so. I also
don't really see how the API could support it, but it is a potential
issue.


> > Ok, but can you also document what the driver should add? Supported
> > rates, HT info, ... presumably?
>
> It is same case as for 'scan' or 'start_ap' - driver expect to deal
> with
> IE's same way. There we don't say anything, why do it here?

Maybe we're better now? ;-)

I guess I'm not too concerned about this.

> Anything more I expected to do with patch? Or, is it on its way to
> your git?

Send as [PATCH] for sure, but we should think about the progressive
thing.

johannes


2013-03-18 20:31:41

by Johannes Berg

[permalink] [raw]
Subject: Re: [RFC] P2P find offload

On Sun, 2013-03-17 at 10:56 +0200, Vladimir Kondratiev wrote:

> > Why not just indicate the command presence rather than a feature flag?
>
> I also thought this way, it was my impression you was asking for feature flag
> earlier:
> --cut--
> > You're also entirely missing feature advertising, so userspace can only
> > guess whether it's supported or not ...
> --cut--

Ah, well, I was asking that you advertise the feature, not necessarily
through a feature flag :-)

We advertise lots of features by having the CMD() thing in nl80211.c,
though I think we may have to change that now due to the the wiphy size
limit mess.

If mac80211 drivers support this we may need a wiphy flag that
influences the command presence though.

> For "progressive" P2P scan - do you mean normal scan or find phase? For normal
> scan, nothing changes and one can use any scan optimisations. For find phase -
> spec don't let you do so, it pretty much mandates what and when to do.

I was under the impression that wpa_s allowed "progressive" (i.e. one
non-social channel per iteration) even in find? I could be wrong though,
maybe only in search.

> > Is there really much value in indications, rather than having it start
> > right away? Similarly, in particular if sending probe responses is
> > offloaded, is there really any reason for userspace to care? OTOH, we
> > also send an indication when a scan starts, so maybe it's useful that
> > way. Just seems that the code requesting this would assume that if it
> > didn't get an error, it will be started, and not really care about when
> > it really started operating.
>
> There are 2 points:
> 1) timing. For hardware, it may take time to really start find phase.
> For card I am working on now, 60G one, it tooks some 150ms.
> I suppose, upper layers may be confused with this delay.
> 2) hardware may decide to end find phase on its own, in this case 'end'
> notification is a must; it seems more consistent to provide both 'start'
> and 'end'.

Ok, fair enough. I guess we need to mandate that (unless the command
returned an error) userspace will at least get the "end" notification,
even if it never starts, so that it can safely wait for the end and
maybe ignore the start if it wants to. That does seem to be the case
already anyway.

> OK, will do it. Split this to 2 functions,
> cfg80211_p2p_find_notify_start() and cfg80211_p2p_find_notify_end(),
> yes?
>
> Open question: can supplicant be interested in more details, like when
> 'search' started, and when 'listen' started for particular channel?

I doubt it.

> > > /**

> > Also, this is probably *additional* IEs, i.e. supported rates etc. are
> > expected to be created by the driver?
>
> In params for start_ap, it is called 'extra' IE's;
> I will call it the same here for consistency.

Ok, but can you also document what the driver should add? Supported
rates, HT info, ... presumably?

> > > + genlmsg_multicast_netns(wiphy_net(&rdev->wiphy), msg, 0,
> > > + nl80211_scan_mcgrp.id, GFP_KERNEL);
> >
> > indentation
> I double checked, identation seems OK. Checkpatch also fine.

Probably got messed up in email then.

> For trace.h, whole file uses one-tab identation after TRACE_EVENT
> or DEFINE_EVENT; yes it is not what checkpatch likes.
> But, should I follow the file style or properly indent my additions
> in non-consistent with the rest of file style?
> I left it with the same identation as the rest of file.

Please leave it consistent.

> +/**
> + * cfg80211_p2p_find_notify_end - report p2p find state ended
> + * @wdev: the wireless device reporting the event
> + * @gfp: allocation flags
> + *
> + * p2p find state may be ended either unsolicited or in response to
> + * @ops->p2p_stop_find

I think that @ might confuse the parser? I guess we'll find out when we
run it :-)

johannes