| Message ID | 1424442925-10778-1-git-send-email-jouni@qca.qualcomm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);