Return-path: Received: from xc.sipsolutions.net ([83.246.72.84]:48400 "EHLO sipsolutions.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751338AbZCUIEC (ORCPT ); Sat, 21 Mar 2009 04:04:02 -0400 Subject: Re: [PATCH 1/4] nl80211: Remove NL80211_CMD_SET_MGMT_EXTRA_IE From: Johannes Berg To: Jouni Malinen Cc: "John W. Linville" , linux-wireless@vger.kernel.org In-Reply-To: <20090320192348.433430144@atheros.com> References: <20090320192115.448175935@atheros.com> <20090320192348.433430144@atheros.com> Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-kVRV9zVVHSnCdGGxxNUP" Date: Sat, 21 Mar 2009 09:03:52 +0100 Message-Id: <1237622632.5100.164.camel@johannes.local> (sfid-20090321_090409_000569_65041519) Mime-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org List-ID: --=-kVRV9zVVHSnCdGGxxNUP Content-Type: text/plain Content-Transfer-Encoding: quoted-printable On Fri, 2009-03-20 at 21:21 +0200, Jouni Malinen wrote: > plain text document attachment (mac80211-remove-mlme-extra-ie.patch) > The functionality that NL80211_CMD_SET_MGMT_EXTRA_IE provided can now > be achieved with cleaner design by adding IE(s) into > NL80211_CMD_TRIGGER_SCAN, NL80211_CMD_AUTHENTICATE, > NL80211_CMD_ASSOCIATE, NL80211_CMD_DEAUTHENTICATE, and > NL80211_CMD_DISASSOCIATE. >=20 > Since this is a very recently added command and there are no known (or > known planned) applications using NL80211_CMD_SET_MGMT_EXTRA_IE and > taken into account how much extra complexity it adds to the IE > processing we have now (and need to add in the future to fix IE order > in couple of frames), it looks like the best option is to just remove > the implementation of this command for now. The enum values themselves > are left to avoid changing the nl80211 command or attribute numbers. >=20 > Signed-off-by: Jouni Malinen Acked-by: Johannes Berg > --- > include/linux/nl80211.h | 8 +++- > include/net/cfg80211.h | 26 ------------- > net/mac80211/cfg.c | 86 --------------------------------------= ------- > net/mac80211/ieee80211_i.h | 15 ------- > net/mac80211/iface.c | 7 --- > net/mac80211/mlme.c | 36 +----------------- > net/mac80211/util.c | 29 ++------------- > net/wireless/nl80211.c | 47 ------------------------ > 8 files changed, 14 insertions(+), 240 deletions(-) >=20 > --- uml.orig/include/linux/nl80211.h 2009-03-20 17:24:12.000000000 +0200 > +++ uml/include/linux/nl80211.h 2009-03-20 18:03:53.000000000 +0200 > @@ -142,6 +142,12 @@ > * %NL80211_ATTR_IE. If the command succeeds, the requested data will be > * added to all specified management frames generated by > * kernel/firmware/driver. > + * Note: This command has been removed and it is only reserved at this > + * point to avoid re-using existing command number. The functionality th= is > + * command was planned for has been provided with cleaner design with th= e > + * option to specify additional IEs in NL80211_CMD_TRIGGER_SCAN, > + * NL80211_CMD_AUTHENTICATE, NL80211_CMD_ASSOCIATE, > + * NL80211_CMD_DEAUTHENTICATE, and NL80211_CMD_DISASSOCIATE. > * > * @NL80211_CMD_GET_SCAN: get scan results > * @NL80211_CMD_TRIGGER_SCAN: trigger a new scan with the given paramete= rs > @@ -238,7 +244,7 @@ enum nl80211_commands { > NL80211_CMD_GET_MESH_PARAMS, > NL80211_CMD_SET_MESH_PARAMS, > =20 > - NL80211_CMD_SET_MGMT_EXTRA_IE, > + NL80211_CMD_SET_MGMT_EXTRA_IE /* reserved; not used */, > =20 > NL80211_CMD_GET_REG, > =20 > --- uml.orig/include/net/cfg80211.h 2009-03-20 17:21:50.000000000 +0200 > +++ uml/include/net/cfg80211.h 2009-03-20 18:03:53.000000000 +0200 > @@ -471,26 +471,6 @@ struct ieee80211_txq_params { > u8 aifs; > }; > =20 > -/** > - * struct mgmt_extra_ie_params - Extra management frame IE parameters > - * > - * Used to add extra IE(s) into management frames. If the driver cannot = add the > - * requested data into all management frames of the specified subtype th= at are > - * generated in kernel or firmware/hardware, it must reject the configur= ation > - * call. The IE data buffer is added to the end of the specified managem= ent > - * frame body after all other IEs. This addition is not applied to frame= s that > - * are injected through a monitor interface. > - * > - * @subtype: Management frame subtype > - * @ies: IE data buffer or %NULL to remove previous data > - * @ies_len: Length of @ies in octets > - */ > -struct mgmt_extra_ie_params { > - u8 subtype; > - u8 *ies; > - int ies_len; > -}; > - > /* from net/wireless.h */ > struct wiphy; > =20 > @@ -743,8 +723,6 @@ struct cfg80211_disassoc_request { > * > * @set_channel: Set channel > * > - * @set_mgmt_extra_ie: Set extra IE data for management frames > - * > * @scan: Request to do a scan. If returning zero, the scan request is g= iven > * the driver, and will be valid until passed to cfg80211_scan_done(). > * For scan results, call cfg80211_inform_bss(); you can call this outsi= de > @@ -828,10 +806,6 @@ struct cfg80211_ops { > struct ieee80211_channel *chan, > enum nl80211_channel_type channel_type); > =20 > - int (*set_mgmt_extra_ie)(struct wiphy *wiphy, > - struct net_device *dev, > - struct mgmt_extra_ie_params *params); > - > int (*scan)(struct wiphy *wiphy, struct net_device *dev, > struct cfg80211_scan_request *request); > =20 > --- uml.orig/net/mac80211/cfg.c 2009-03-20 17:21:50.000000000 +0200 > +++ uml/net/mac80211/cfg.c 2009-03-20 18:03:53.000000000 +0200 > @@ -1181,91 +1181,6 @@ static int ieee80211_set_channel(struct=20 > return ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL); > } > =20 > -static int set_mgmt_extra_ie_sta(struct ieee80211_sub_if_data *sdata, > - u8 subtype, u8 *ies, size_t ies_len) > -{ > - struct ieee80211_local *local =3D sdata->local; > - struct ieee80211_if_managed *ifmgd =3D &sdata->u.mgd; > - > - switch (subtype) { > - case IEEE80211_STYPE_PROBE_REQ >> 4: > - if (local->ops->hw_scan) > - break; > - kfree(ifmgd->ie_probereq); > - ifmgd->ie_probereq =3D ies; > - ifmgd->ie_probereq_len =3D ies_len; > - return 0; > - case IEEE80211_STYPE_PROBE_RESP >> 4: > - kfree(ifmgd->ie_proberesp); > - ifmgd->ie_proberesp =3D ies; > - ifmgd->ie_proberesp_len =3D ies_len; > - return 0; > - case IEEE80211_STYPE_AUTH >> 4: > - kfree(ifmgd->ie_auth); > - ifmgd->ie_auth =3D ies; > - ifmgd->ie_auth_len =3D ies_len; > - return 0; > - case IEEE80211_STYPE_ASSOC_REQ >> 4: > - kfree(ifmgd->ie_assocreq); > - ifmgd->ie_assocreq =3D ies; > - ifmgd->ie_assocreq_len =3D ies_len; > - return 0; > - case IEEE80211_STYPE_REASSOC_REQ >> 4: > - kfree(ifmgd->ie_reassocreq); > - ifmgd->ie_reassocreq =3D ies; > - ifmgd->ie_reassocreq_len =3D ies_len; > - return 0; > - case IEEE80211_STYPE_DEAUTH >> 4: > - kfree(ifmgd->ie_deauth); > - ifmgd->ie_deauth =3D ies; > - ifmgd->ie_deauth_len =3D ies_len; > - return 0; > - case IEEE80211_STYPE_DISASSOC >> 4: > - kfree(ifmgd->ie_disassoc); > - ifmgd->ie_disassoc =3D ies; > - ifmgd->ie_disassoc_len =3D ies_len; > - return 0; > - } > - > - return -EOPNOTSUPP; > -} > - > -static int ieee80211_set_mgmt_extra_ie(struct wiphy *wiphy, > - struct net_device *dev, > - struct mgmt_extra_ie_params *params) > -{ > - struct ieee80211_sub_if_data *sdata; > - u8 *ies; > - size_t ies_len; > - int ret =3D -EOPNOTSUPP; > - > - if (params->ies) { > - ies =3D kmemdup(params->ies, params->ies_len, GFP_KERNEL); > - if (ies =3D=3D NULL) > - return -ENOMEM; > - ies_len =3D params->ies_len; > - } else { > - ies =3D NULL; > - ies_len =3D 0; > - } > - > - sdata =3D IEEE80211_DEV_TO_SUB_IF(dev); > - > - switch (sdata->vif.type) { > - case NL80211_IFTYPE_STATION: > - ret =3D set_mgmt_extra_ie_sta(sdata, params->subtype, > - ies, ies_len); > - break; > - default: > - ret =3D -EOPNOTSUPP; > - break; > - } > - > - if (ret) > - kfree(ies); > - return ret; > -} > - > #ifdef CONFIG_PM > static int ieee80211_suspend(struct wiphy *wiphy) > { > @@ -1465,7 +1380,6 @@ struct cfg80211_ops mac80211_config_ops=20 > .change_bss =3D ieee80211_change_bss, > .set_txq_params =3D ieee80211_set_txq_params, > .set_channel =3D ieee80211_set_channel, > - .set_mgmt_extra_ie =3D ieee80211_set_mgmt_extra_ie, > .suspend =3D ieee80211_suspend, > .resume =3D ieee80211_resume, > .scan =3D ieee80211_scan, > --- uml.orig/net/mac80211/ieee80211_i.h 2009-03-20 17:24:12.000000000 +02= 00 > +++ uml/net/mac80211/ieee80211_i.h 2009-03-20 18:03:53.000000000 +0200 > @@ -323,21 +323,6 @@ struct ieee80211_if_managed { > int wmm_last_param_set; > =20 > /* Extra IE data for management frames */ > - u8 *ie_probereq; > - size_t ie_probereq_len; > - u8 *ie_proberesp; > - size_t ie_proberesp_len; > - u8 *ie_auth; > - size_t ie_auth_len; > - u8 *ie_assocreq; > - size_t ie_assocreq_len; > - u8 *ie_reassocreq; > - size_t ie_reassocreq_len; > - u8 *ie_deauth; > - size_t ie_deauth_len; > - u8 *ie_disassoc; > - size_t ie_disassoc_len; > - > u8 *sme_auth_ie; > size_t sme_auth_ie_len; > }; > --- uml.orig/net/mac80211/iface.c 2009-03-20 17:24:12.000000000 +0200 > +++ uml/net/mac80211/iface.c 2009-03-20 18:03:53.000000000 +0200 > @@ -653,13 +653,6 @@ static void ieee80211_teardown_sdata(str > kfree(sdata->u.mgd.extra_ie); > kfree(sdata->u.mgd.assocreq_ies); > kfree(sdata->u.mgd.assocresp_ies); > - kfree(sdata->u.mgd.ie_probereq); > - kfree(sdata->u.mgd.ie_proberesp); > - kfree(sdata->u.mgd.ie_auth); > - kfree(sdata->u.mgd.ie_assocreq); > - kfree(sdata->u.mgd.ie_reassocreq); > - kfree(sdata->u.mgd.ie_deauth); > - kfree(sdata->u.mgd.ie_disassoc); > kfree(sdata->u.mgd.sme_auth_ie); > break; > case NL80211_IFTYPE_WDS: > --- uml.orig/net/mac80211/mlme.c 2009-03-20 17:24:12.000000000 +0200 > +++ uml/net/mac80211/mlme.c 2009-03-20 18:03:53.000000000 +0200 > @@ -82,38 +82,23 @@ static int ieee80211_compatible_rates(st > =20 > /* frame sending functions */ > =20 > -static void add_extra_ies(struct sk_buff *skb, u8 *ies, size_t ies_len) > -{ > - if (ies) > - memcpy(skb_put(skb, ies_len), ies, ies_len); > -} > - > static void ieee80211_send_assoc(struct ieee80211_sub_if_data *sdata) > { > struct ieee80211_if_managed *ifmgd =3D &sdata->u.mgd; > struct ieee80211_local *local =3D sdata->local; > struct sk_buff *skb; > struct ieee80211_mgmt *mgmt; > - u8 *pos, *ies, *ht_ie, *e_ies; > + u8 *pos, *ies, *ht_ie; > int i, len, count, rates_len, supp_rates_len; > u16 capab; > struct ieee80211_bss *bss; > int wmm =3D 0; > struct ieee80211_supported_band *sband; > u32 rates =3D 0; > - size_t e_ies_len; > - > - if (ifmgd->flags & IEEE80211_STA_PREV_BSSID_SET) { > - e_ies =3D sdata->u.mgd.ie_reassocreq; > - e_ies_len =3D sdata->u.mgd.ie_reassocreq_len; > - } else { > - e_ies =3D sdata->u.mgd.ie_assocreq; > - e_ies_len =3D sdata->u.mgd.ie_assocreq_len; > - } > =20 > skb =3D dev_alloc_skb(local->hw.extra_tx_headroom + > sizeof(*mgmt) + 200 + ifmgd->extra_ie_len + > - ifmgd->ssid_len + e_ies_len); > + ifmgd->ssid_len); > if (!skb) { > printk(KERN_DEBUG "%s: failed to allocate buffer for assoc " > "frame\n", sdata->dev->name); > @@ -304,8 +289,6 @@ static void ieee80211_send_assoc(struct=20 > memcpy(pos, &sband->ht_cap.mcs, sizeof(sband->ht_cap.mcs)); > } > =20 > - add_extra_ies(skb, e_ies, e_ies_len); > - > kfree(ifmgd->assocreq_ies); > ifmgd->assocreq_ies_len =3D (skb->data + skb->len) - ies; > ifmgd->assocreq_ies =3D kmalloc(ifmgd->assocreq_ies_len, GFP_KERNEL); > @@ -323,19 +306,8 @@ static void ieee80211_send_deauth_disass > struct ieee80211_if_managed *ifmgd =3D &sdata->u.mgd; > struct sk_buff *skb; > struct ieee80211_mgmt *mgmt; > - u8 *ies; > - size_t ies_len; > =20 > - if (stype =3D=3D IEEE80211_STYPE_DEAUTH) { > - ies =3D sdata->u.mgd.ie_deauth; > - ies_len =3D sdata->u.mgd.ie_deauth_len; > - } else { > - ies =3D sdata->u.mgd.ie_disassoc; > - ies_len =3D sdata->u.mgd.ie_disassoc_len; > - } > - > - skb =3D dev_alloc_skb(local->hw.extra_tx_headroom + sizeof(*mgmt) + > - ies_len); > + skb =3D dev_alloc_skb(local->hw.extra_tx_headroom + sizeof(*mgmt)); > if (!skb) { > printk(KERN_DEBUG "%s: failed to allocate buffer for " > "deauth/disassoc frame\n", sdata->dev->name); > @@ -353,8 +325,6 @@ static void ieee80211_send_deauth_disass > /* u.deauth.reason_code =3D=3D u.disassoc.reason_code */ > mgmt->u.deauth.reason_code =3D cpu_to_le16(reason); > =20 > - add_extra_ies(skb, ies, ies_len); > - > ieee80211_tx_skb(sdata, skb, ifmgd->flags & IEEE80211_STA_MFP_ENABLED); > } > =20 > --- uml.orig/net/mac80211/util.c 2009-03-20 17:21:50.000000000 +0200 > +++ uml/net/mac80211/util.c 2009-03-20 18:03:53.000000000 +0200 > @@ -846,16 +846,9 @@ void ieee80211_send_auth(struct ieee8021 > struct ieee80211_local *local =3D sdata->local; > struct sk_buff *skb; > struct ieee80211_mgmt *mgmt; > - const u8 *ie_auth =3D NULL; > - int ie_auth_len =3D 0; > - > - if (sdata->vif.type =3D=3D NL80211_IFTYPE_STATION) { > - ie_auth_len =3D sdata->u.mgd.ie_auth_len; > - ie_auth =3D sdata->u.mgd.ie_auth; > - } > =20 > skb =3D dev_alloc_skb(local->hw.extra_tx_headroom + > - sizeof(*mgmt) + 6 + extra_len + ie_auth_len); > + sizeof(*mgmt) + 6 + extra_len); > if (!skb) { > printk(KERN_DEBUG "%s: failed to allocate buffer for auth " > "frame\n", sdata->dev->name); > @@ -877,8 +870,6 @@ void ieee80211_send_auth(struct ieee8021 > mgmt->u.auth.status_code =3D cpu_to_le16(0); > if (extra) > memcpy(skb_put(skb, extra_len), extra, extra_len); > - if (ie_auth) > - memcpy(skb_put(skb, ie_auth_len), ie_auth, ie_auth_len); > =20 > ieee80211_tx_skb(sdata, skb, encrypt); > } > @@ -891,20 +882,11 @@ void ieee80211_send_probe_req(struct iee > struct ieee80211_supported_band *sband; > struct sk_buff *skb; > struct ieee80211_mgmt *mgmt; > - u8 *pos, *supp_rates, *esupp_rates =3D NULL, *extra_preq_ie =3D NULL; > - int i, extra_preq_ie_len =3D 0; > - > - switch (sdata->vif.type) { > - case NL80211_IFTYPE_STATION: > - extra_preq_ie_len =3D sdata->u.mgd.ie_probereq_len; > - extra_preq_ie =3D sdata->u.mgd.ie_probereq; > - break; > - default: > - break; > - } > + u8 *pos, *supp_rates, *esupp_rates =3D NULL; > + int i; > =20 > skb =3D dev_alloc_skb(local->hw.extra_tx_headroom + sizeof(*mgmt) + 200= + > - ie_len + extra_preq_ie_len); > + ie_len); > if (!skb) { > printk(KERN_DEBUG "%s: failed to allocate buffer for probe " > "request\n", sdata->dev->name); > @@ -953,9 +935,6 @@ void ieee80211_send_probe_req(struct iee > =20 > if (ie) > memcpy(skb_put(skb, ie_len), ie, ie_len); > - if (extra_preq_ie) > - memcpy(skb_put(skb, extra_preq_ie_len), extra_preq_ie, > - extra_preq_ie_len); > =20 > ieee80211_tx_skb(sdata, skb, 0); > } > --- uml.orig/net/wireless/nl80211.c 2009-03-20 18:03:40.000000000 +0200 > +++ uml/net/wireless/nl80211.c 2009-03-20 18:03:53.000000000 +0200 > @@ -269,7 +269,6 @@ static int nl80211_send_wiphy(struct sk_ > CMD(add_mpath, NEW_MPATH); > CMD(set_mesh_params, SET_MESH_PARAMS); > CMD(change_bss, SET_BSS); > - CMD(set_mgmt_extra_ie, SET_MGMT_EXTRA_IE); > CMD(auth, AUTHENTICATE); > CMD(assoc, ASSOCIATE); > CMD(deauth, DEAUTHENTICATE); > @@ -2355,46 +2354,6 @@ static int nl80211_set_reg(struct sk_buf > return -EINVAL; > } > =20 > -static int nl80211_set_mgmt_extra_ie(struct sk_buff *skb, > - struct genl_info *info) > -{ > - struct cfg80211_registered_device *drv; > - int err; > - struct net_device *dev; > - struct mgmt_extra_ie_params params; > - > - memset(¶ms, 0, sizeof(params)); > - > - if (!info->attrs[NL80211_ATTR_MGMT_SUBTYPE]) > - return -EINVAL; > - params.subtype =3D nla_get_u8(info->attrs[NL80211_ATTR_MGMT_SUBTYPE]); > - if (params.subtype > 15) > - return -EINVAL; /* FC Subtype field is 4 bits (0..15) */ > - > - if (info->attrs[NL80211_ATTR_IE]) { > - params.ies =3D nla_data(info->attrs[NL80211_ATTR_IE]); > - params.ies_len =3D nla_len(info->attrs[NL80211_ATTR_IE]); > - } > - > - rtnl_lock(); > - > - err =3D get_drv_dev_by_info_ifindex(info->attrs, &drv, &dev); > - if (err) > - goto out_rtnl; > - > - if (drv->ops->set_mgmt_extra_ie) > - err =3D drv->ops->set_mgmt_extra_ie(&drv->wiphy, dev, ¶ms); > - else > - err =3D -EOPNOTSUPP; > - > - cfg80211_put_dev(drv); > - dev_put(dev); > - out_rtnl: > - rtnl_unlock(); > - > - return err; > -} > - > static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *i= nfo) > { > struct cfg80211_registered_device *drv; > @@ -3044,12 +3003,6 @@ static struct genl_ops nl80211_ops[] =3D { > .flags =3D GENL_ADMIN_PERM, > }, > { > - .cmd =3D NL80211_CMD_SET_MGMT_EXTRA_IE, > - .doit =3D nl80211_set_mgmt_extra_ie, > - .policy =3D nl80211_policy, > - .flags =3D GENL_ADMIN_PERM, > - }, > - { > .cmd =3D NL80211_CMD_TRIGGER_SCAN, > .doit =3D nl80211_trigger_scan, > .policy =3D nl80211_policy, >=20 > --=20 >=20 --=-kVRV9zVVHSnCdGGxxNUP Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Comment: Johannes Berg (powerbook) iQIcBAABAgAGBQJJxJ9lAAoJEKVg1VMiehFYpdUP/3vTZBUj8yfWPM51rAbxTeQm Lohb8RCecvpt5XR/R2zE6UNKwW9tuFok6IQ4t7A/+ISSJPyoEhSqAr0sBhWy/fZv lPIgOBo22/ggEPShj+jRoxsqDo8ZaQb/aDwWFH6yUis2y7miTug7pFYpC/P2m4ef wgLLxe6DJJGy3vrj51+2kK1O29wjab9TxsUNhRu5+1Ba8fDEmi5JjSS8G6h9gROV 75Ju4U8rs/SHhlFvq5/XI6guIXRrr07iJqXDGtmd/L4GrPTG3SNNL4iCyqK2SjTz blAW591gd9pbLD6TXWx7P1durFe72iQYwdcpIJz08ST8zC3XLgzWAWzFE0Qo+MSa W/cfetnKR3AbSNMLv2PFSmWpyKmCW1vF6Ohz7GbFPCsV6OtX1LUwXqpWTX9M1MiV kXiv8soMNFGOEQUtgjRqXJz2AG/FbCTr00bUSx391tyCH7r2PEkL3Ap99KNa1QbH 3TSG1gdMWdojwrx5tOyCc0VMPWtMEigWuCeKv/GRwlSMQKSXfPmpbmTvE4vMHUTZ VOG8WnFn87ShwRnwv6g2kNnwjPbFzDU7zSK+WIwbPP/PFj/r8Dt3CJz6FaV8yklJ WrJ3mjXSg7McS2rfsL8VRh4V+H799vID/j7ZZhr51KuScfmLjMhdVOaXhrx7M2/P njZXT/3JcAxkXSsZPQmm =0LZ+ -----END PGP SIGNATURE----- --=-kVRV9zVVHSnCdGGxxNUP--