All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] cfg80211: Allow NL80211_ATTR_IFINDEX to be added to vendor events
@ 2015-02-20 14:35 Jouni Malinen
  2015-02-20 14:35 ` [PATCH 2/2] mac80211_hwsim: Add ifindex to the example vendor event Jouni Malinen
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jouni Malinen @ 2015-02-20 14:35 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Ahmad Kholaif, Jouni Malinen

From: Ahmad Kholaif <akholaif@qca.qualcomm.com>

This adds a new alternative static inline wrapper to
cfg80211_vendor_event_alloc() with an additional argument
struct wireless_dev *wdev.

__cfg80211_alloc_event_skb() is modified to take in *wdev argument
(either NULL which would be the existing cfg80211_vendor_event_alloc()
case or not-NULL which would the new wrapper case; if wdev != NULL, the
NL80211_ATTR_IFINDEX is added to the vendor event.

These changes make it easier for drivers to add ifindex indication in
vendor events cleanly.

Signed-off-by: Ahmad Kholaif <akholaif@qca.qualcomm.com>
Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 include/net/cfg80211.h | 39 +++++++++++++++++++++++++++++++++++----
 net/wireless/nl80211.c | 15 +++++++++++----
 2 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 64e09e1..55d1648 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4263,7 +4263,8 @@ struct sk_buff *__cfg80211_alloc_event_skb(struct wiphy *wiphy,
 					   enum nl80211_commands cmd,
 					   enum nl80211_attrs attr,
 					   int vendor_event_idx,
-					   int approxlen, gfp_t gfp);
+					   int approxlen, gfp_t gfp,
+					   struct wireless_dev *wdev);
 
 void __cfg80211_send_event_skb(struct sk_buff *skb, gfp_t gfp);
 
@@ -4333,7 +4334,36 @@ cfg80211_vendor_event_alloc(struct wiphy *wiphy, int approxlen,
 {
 	return __cfg80211_alloc_event_skb(wiphy, NL80211_CMD_VENDOR,
 					  NL80211_ATTR_VENDOR_DATA,
-					  event_idx, approxlen, gfp);
+					  event_idx, approxlen, gfp, NULL);
+}
+
+/**
+ * cfg80211_vendor_event_alloc_ext - allocate vendor-specific event skb
+ * @wiphy: the wiphy
+ * @event_idx: index of the vendor event in the wiphy's vendor_events
+ * @approxlen: an upper bound of the length of the data that will
+ *	be put into the skb
+ * @gfp: allocation flags
+ * @wdev: the wireless device
+ *
+ * This function allocates and pre-fills an skb for an event on the
+ * vendor-specific multicast group. This is otherwise identical to
+ * cfg80211_vendor_event_alloc(), but ifindex of the specified wireless device
+ * is added to the event message before the vendor data attribute.
+ *
+ * When done filling the skb, call cfg80211_vendor_event() with the
+ * skb to send the event.
+ *
+ * Return: An allocated and pre-filled skb. %NULL if any errors happen.
+ */
+static inline struct sk_buff *
+cfg80211_vendor_event_alloc_ext(struct wiphy *wiphy, int approxlen,
+				int event_idx, gfp_t gfp,
+				struct wireless_dev *wdev)
+{
+	return __cfg80211_alloc_event_skb(wiphy, NL80211_CMD_VENDOR,
+					  NL80211_ATTR_VENDOR_DATA,
+					  event_idx, approxlen, gfp, wdev);
 }
 
 /**
@@ -4342,7 +4372,8 @@ cfg80211_vendor_event_alloc(struct wiphy *wiphy, int approxlen,
  * @gfp: allocation flags
  *
  * This function sends the given @skb, which must have been allocated
- * by cfg80211_vendor_event_alloc(), as an event. It always consumes it.
+ * by cfg80211_vendor_event_alloc() or cfg80211_vendor_event_alloc_ext(), as an
+ * event. It always consumes it.
  */
 static inline void cfg80211_vendor_event(struct sk_buff *skb, gfp_t gfp)
 {
@@ -4434,7 +4465,7 @@ cfg80211_testmode_alloc_event_skb(struct wiphy *wiphy, int approxlen, gfp_t gfp)
 {
 	return __cfg80211_alloc_event_skb(wiphy, NL80211_CMD_TESTMODE,
 					  NL80211_ATTR_TESTDATA, -1,
-					  approxlen, gfp);
+					  approxlen, gfp, NULL);
 }
 
 /**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 454d7a0..ed2b98e 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7377,7 +7377,7 @@ __cfg80211_alloc_vendor_skb(struct cfg80211_registered_device *rdev,
 			    enum nl80211_commands cmd,
 			    enum nl80211_attrs attr,
 			    const struct nl80211_vendor_cmd_info *info,
-			    gfp_t gfp)
+			    gfp_t gfp, struct wireless_dev *wdev)
 {
 	struct sk_buff *skb;
 	void *hdr;
@@ -7405,6 +7405,12 @@ __cfg80211_alloc_vendor_skb(struct cfg80211_registered_device *rdev,
 			goto nla_put_failure;
 	}
 
+	if (wdev && wdev->netdev) {
+		if (nla_put_u32(skb, NL80211_ATTR_IFINDEX,
+				wdev->netdev->ifindex))
+			goto nla_put_failure;
+	}
+
 	data = nla_nest_start(skb, attr);
 
 	((void **)skb->cb)[0] = rdev;
@@ -7422,7 +7428,8 @@ struct sk_buff *__cfg80211_alloc_event_skb(struct wiphy *wiphy,
 					   enum nl80211_commands cmd,
 					   enum nl80211_attrs attr,
 					   int vendor_event_idx,
-					   int approxlen, gfp_t gfp)
+					   int approxlen, gfp_t gfp,
+					   struct wireless_dev *wdev)
 {
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
 	const struct nl80211_vendor_cmd_info *info;
@@ -7445,7 +7452,7 @@ struct sk_buff *__cfg80211_alloc_event_skb(struct wiphy *wiphy,
 	}
 
 	return __cfg80211_alloc_vendor_skb(rdev, approxlen, 0, 0,
-					   cmd, attr, info, gfp);
+					   cmd, attr, info, gfp, wdev);
 }
 EXPORT_SYMBOL(__cfg80211_alloc_event_skb);
 
@@ -9893,7 +9900,7 @@ struct sk_buff *__cfg80211_alloc_reply_skb(struct wiphy *wiphy,
 	return __cfg80211_alloc_vendor_skb(rdev, approxlen,
 					   rdev->cur_cmd_info->snd_portid,
 					   rdev->cur_cmd_info->snd_seq,
-					   cmd, attr, NULL, GFP_KERNEL);
+					   cmd, attr, NULL, GFP_KERNEL, NULL);
 }
 EXPORT_SYMBOL(__cfg80211_alloc_reply_skb);
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] mac80211_hwsim: Add ifindex to the example vendor event
  2015-02-20 14:35 [PATCH 1/2] cfg80211: Allow NL80211_ATTR_IFINDEX to be added to vendor events Jouni Malinen
@ 2015-02-20 14:35 ` Jouni Malinen
  2015-02-20 19:28 ` [PATCH 1/2] cfg80211: Allow NL80211_ATTR_IFINDEX to be added to vendor events Arend van Spriel
  2015-02-23 15:37 ` Johannes Berg
  2 siblings, 0 replies; 6+ messages in thread
From: Jouni Malinen @ 2015-02-20 14:35 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linux-wireless, Jouni Malinen

This provides an example of a vendor event that is associated with a
single vif and allows user space tools to process it in the context of
that specified vif rather than full wiphy.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 drivers/net/wireless/mac80211_hwsim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index d2bba4c..580eaab 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -367,7 +367,7 @@ static int mac80211_hwsim_vendor_cmd_test(struct wiphy *wiphy,
 	 *
 	 * event_idx = 0 (index in mac80211_hwsim_vendor_commands)
 	 */
-	skb = cfg80211_vendor_event_alloc(wiphy, 100, 0, GFP_KERNEL);
+	skb = cfg80211_vendor_event_alloc_ext(wiphy, 100, 0, GFP_KERNEL, wdev);
 	if (skb) {
 		/* skb_put() or nla_put() will fill up data within
 		 * NL80211_ATTR_VENDOR_DATA.
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] cfg80211: Allow NL80211_ATTR_IFINDEX to be added to vendor events
  2015-02-20 14:35 [PATCH 1/2] cfg80211: Allow NL80211_ATTR_IFINDEX to be added to vendor events Jouni Malinen
  2015-02-20 14:35 ` [PATCH 2/2] mac80211_hwsim: Add ifindex to the example vendor event Jouni Malinen
@ 2015-02-20 19:28 ` Arend van Spriel
  2015-02-23 15:37 ` Johannes Berg
  2 siblings, 0 replies; 6+ messages in thread
From: Arend van Spriel @ 2015-02-20 19:28 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: Johannes Berg, linux-wireless, Ahmad Kholaif

On 02/20/15 15:35, Jouni Malinen wrote:
> @@ -7405,6 +7405,12 @@ __cfg80211_alloc_vendor_skb(struct cfg80211_registered_device *rdev,
>   			goto nla_put_failure;
>   	}
>
> +	if (wdev&&  wdev->netdev) {
> +		if (nla_put_u32(skb, NL80211_ATTR_IFINDEX,
> +				wdev->netdev->ifindex))
> +			goto nla_put_failure;
> +	}
> +

Could consider putting wdev id in there as well.

Regards,
Arend

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] cfg80211: Allow NL80211_ATTR_IFINDEX to be added to vendor events
  2015-02-20 14:35 [PATCH 1/2] cfg80211: Allow NL80211_ATTR_IFINDEX to be added to vendor events Jouni Malinen
  2015-02-20 14:35 ` [PATCH 2/2] mac80211_hwsim: Add ifindex to the example vendor event Jouni Malinen
  2015-02-20 19:28 ` [PATCH 1/2] cfg80211: Allow NL80211_ATTR_IFINDEX to be added to vendor events Arend van Spriel
@ 2015-02-23 15:37 ` Johannes Berg
  2015-02-23 19:51   ` Kholaif, Ahmad
  2 siblings, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2015-02-23 15:37 UTC (permalink / raw)
  To: Jouni Malinen; +Cc: linux-wireless, Ahmad Kholaif

In addition to what Arend said,


> @@ -4263,7 +4263,8 @@ struct sk_buff *__cfg80211_alloc_event_skb(struct wiphy *wiphy,
>  					   enum nl80211_commands cmd,
>  					   enum nl80211_attrs attr,
>  					   int vendor_event_idx,
> -					   int approxlen, gfp_t gfp);
> +					   int approxlen, gfp_t gfp,
> +					   struct wireless_dev *wdev);

This is really strange. IMHO the wdev should be the second argument -
certainly usually gfp is the last one so it shouldn't be after that.

> +/**
> + * cfg80211_vendor_event_alloc_ext - allocate vendor-specific event skb
> + * @wiphy: the wiphy
> + * @event_idx: index of the vendor event in the wiphy's vendor_events
> + * @approxlen: an upper bound of the length of the data that will
> + *	be put into the skb
> + * @gfp: allocation flags
> + * @wdev: the wireless device
> + *
> + * This function allocates and pre-fills an skb for an event on the
> + * vendor-specific multicast group. This is otherwise identical to
> + * cfg80211_vendor_event_alloc(), but ifindex of the specified wireless device
> + * is added to the event message before the vendor data attribute.
> + *
> + * When done filling the skb, call cfg80211_vendor_event() with the
> + * skb to send the event.
> + *
> + * Return: An allocated and pre-filled skb. %NULL if any errors happen.
> + */
> +static inline struct sk_buff *
> +cfg80211_vendor_event_alloc_ext(struct wiphy *wiphy, int approxlen,
> +				int event_idx, gfp_t gfp,
> +				struct wireless_dev *wdev)
> +{
> +	return __cfg80211_alloc_event_skb(wiphy, NL80211_CMD_VENDOR,
> +					  NL80211_ATTR_VENDOR_DATA,
> +					  event_idx, approxlen, gfp, wdev);
>  }

This doesn't seem necessary, why not just update the original function
to add and document the new optional argument?

[however, in the unlikely even that you can convince me otherwise we may
have to add this to the documentation?]

johannes


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH 1/2] cfg80211: Allow NL80211_ATTR_IFINDEX to be added to vendor events
  2015-02-23 15:37 ` Johannes Berg
@ 2015-02-23 19:51   ` Kholaif, Ahmad
  2015-02-24  8:05     ` Johannes Berg
  0 siblings, 1 reply; 6+ messages in thread
From: Kholaif, Ahmad @ 2015-02-23 19:51 UTC (permalink / raw)
  To: Johannes Berg, Malinen, Jouni
  Cc: linux-wireless, Hullur Subramanyam, Amarnath

Pj4gKy8qKg0KPj4gKyAqIGNmZzgwMjExX3ZlbmRvcl9ldmVudF9hbGxvY19leHQgLSBhbGxvY2F0
ZSB2ZW5kb3Itc3BlY2lmaWMgZXZlbnQgDQo+PiArc2tiDQo+PiArICogQHdpcGh5OiB0aGUgd2lw
aHkNCj4+ICsgKiBAZXZlbnRfaWR4OiBpbmRleCBvZiB0aGUgdmVuZG9yIGV2ZW50IGluIHRoZSB3
aXBoeSdzIHZlbmRvcl9ldmVudHMNCj4+ICsgKiBAYXBwcm94bGVuOiBhbiB1cHBlciBib3VuZCBv
ZiB0aGUgbGVuZ3RoIG9mIHRoZSBkYXRhIHRoYXQgd2lsbA0KPj4gKyAqCWJlIHB1dCBpbnRvIHRo
ZSBza2INCj4+ICsgKiBAZ2ZwOiBhbGxvY2F0aW9uIGZsYWdzDQo+PiArICogQHdkZXY6IHRoZSB3
aXJlbGVzcyBkZXZpY2UNCj4+ICsgKg0KPj4gKyAqIFRoaXMgZnVuY3Rpb24gYWxsb2NhdGVzIGFu
ZCBwcmUtZmlsbHMgYW4gc2tiIGZvciBhbiBldmVudCBvbiB0aGUNCj4+ICsgKiB2ZW5kb3Itc3Bl
Y2lmaWMgbXVsdGljYXN0IGdyb3VwLiBUaGlzIGlzIG90aGVyd2lzZSBpZGVudGljYWwgdG8NCj4+
ICsgKiBjZmc4MDIxMV92ZW5kb3JfZXZlbnRfYWxsb2MoKSwgYnV0IGlmaW5kZXggb2YgdGhlIHNw
ZWNpZmllZCANCj4+ICt3aXJlbGVzcyBkZXZpY2UNCj4+ICsgKiBpcyBhZGRlZCB0byB0aGUgZXZl
bnQgbWVzc2FnZSBiZWZvcmUgdGhlIHZlbmRvciBkYXRhIGF0dHJpYnV0ZS4NCj4+ICsgKg0KPj4g
KyAqIFdoZW4gZG9uZSBmaWxsaW5nIHRoZSBza2IsIGNhbGwgY2ZnODAyMTFfdmVuZG9yX2V2ZW50
KCkgd2l0aCB0aGUNCj4+ICsgKiBza2IgdG8gc2VuZCB0aGUgZXZlbnQuDQo+ICsgKg0KPiArICog
UmV0dXJuOiBBbiBhbGxvY2F0ZWQgYW5kIHByZS1maWxsZWQgc2tiLiAlTlVMTCBpZiBhbnkgZXJy
b3JzIGhhcHBlbi4NCj4gKyAqLw0KPiArc3RhdGljIGlubGluZSBzdHJ1Y3Qgc2tfYnVmZiAqDQo+
ICtjZmc4MDIxMV92ZW5kb3JfZXZlbnRfYWxsb2NfZXh0KHN0cnVjdCB3aXBoeSAqd2lwaHksIGlu
dCBhcHByb3hsZW4sDQo+ICsJCQkJaW50IGV2ZW50X2lkeCwgZ2ZwX3QgZ2ZwLA0KPiArCQkJCXN0
cnVjdCB3aXJlbGVzc19kZXYgKndkZXYpDQo+ICt7DQo+ICsJcmV0dXJuIF9fY2ZnODAyMTFfYWxs
b2NfZXZlbnRfc2tiKHdpcGh5LCBOTDgwMjExX0NNRF9WRU5ET1IsDQo+ICsJCQkJCSAgTkw4MDIx
MV9BVFRSX1ZFTkRPUl9EQVRBLA0KPiArCQkJCQkgIGV2ZW50X2lkeCwgYXBwcm94bGVuLCBnZnAs
IHdkZXYpOw0KPiAgfQ0KDQo+IFRoaXMgZG9lc24ndCBzZWVtIG5lY2Vzc2FyeSwgd2h5IG5vdCBq
dXN0IHVwZGF0ZSB0aGUgb3JpZ2luYWwgZnVuY3Rpb24gdG8gYWRkIGFuZCBkb2N1bWVudCB0aGUg
bmV3IG9wdGlvbmFsIGFyZ3VtZW50Pw0KDQo+W2hvd2V2ZXIsIGluIHRoZSB1bmxpa2VseSBldmVu
IHRoYXQgeW91IGNhbiBjb252aW5jZSBtZSBvdGhlcndpc2Ugd2UgbWF5IGhhdmUgdG8gYWRkIHRo
aXMgdG8gdGhlIGRvY3VtZW50YXRpb24/XQ0KDQpUaGlzIG1lYW5zIHRoYXQgdGhpcyBrZXJuZWwg
Y2hhbmdlIGNhbid0IGJlIHB1bGxlZCBpbiB3aXRob3V0IGNvcnJlc3BvbmRpbmcgZHJpdmVyIGNo
YW5nZXMgdG8gY2FsbCAgY2ZnODAyMTFfdmVuZG9yX2V2ZW50X2FsbG9jKCkgd2l0aCBhIE5VTEwg
Zm9yIHdkZXYuIFBsZWFzZSBjb25maXJtIGlmIHRoaXMgaXMgYWNjZXB0YWJsZTsgb3RoZXJ3aXNl
LCB3ZSB3b3VsZCBuZWVkIHRvIHVzZSB0aGUgbmV3IHdyYXBwZXIgZGVmaW5lZCBhYm92ZS4NCg0K
VGhhbmtzLA0KQWhtYWQgS2hvbGFpZg0K

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] cfg80211: Allow NL80211_ATTR_IFINDEX to be added to vendor events
  2015-02-23 19:51   ` Kholaif, Ahmad
@ 2015-02-24  8:05     ` Johannes Berg
  0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2015-02-24  8:05 UTC (permalink / raw)
  To: Kholaif, Ahmad
  Cc: Malinen, Jouni, linux-wireless, Hullur Subramanyam, Amarnath


> This means that this kernel change can't be pulled in without
> corresponding driver changes to call  cfg80211_vendor_event_alloc()
> with a NULL for wdev. Please confirm if this is acceptable; otherwise,
> we would need to use the new wrapper defined above.

This is fine to me, there are very few such users anyway (make sure to
update in a single patch.)

johannes


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-02-24  8:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-20 14:35 [PATCH 1/2] cfg80211: Allow NL80211_ATTR_IFINDEX to be added to vendor events Jouni Malinen
2015-02-20 14:35 ` [PATCH 2/2] mac80211_hwsim: Add ifindex to the example vendor event Jouni Malinen
2015-02-20 19:28 ` [PATCH 1/2] cfg80211: Allow NL80211_ATTR_IFINDEX to be added to vendor events Arend van Spriel
2015-02-23 15:37 ` Johannes Berg
2015-02-23 19:51   ` Kholaif, Ahmad
2015-02-24  8:05     ` Johannes Berg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.