Subject: Re: [RFC] nl80211: New command for adding extra IE(s) into
	management frames
From: Johannes Berg <johannes@sipsolutions.net>
To: Jouni Malinen <j@w1.fi>
Cc: linux-wireless@vger.kernel.org
In-Reply-To: <20090112122605.GB9153@jm.kir.nu>
References: <20090112122605.GB9153@jm.kir.nu>
Content-Type: text/plain
Date: Mon, 12 Jan 2009 14:08:44 +0100
Message-Id: <1231765724.2998.10.camel@home.berg> (sfid-20090112_140915_535207_2B9DA592)
Mime-Version: 1.0
Sender: linux-wireless-owner@vger.kernel.org

On Mon, 2009-01-12 at 14:26 +0200, Jouni Malinen wrote:
> A new nl80211 command, NL80211_CMD_SET_MGMT_EXTRA_IE, can be used to
> add arbitrary IE data into the end of management frames in client
> modes. The interface allows extra IEs to be configured for each
> management frame subtype, but only some of them (ProbeReq, ProbeResp,
> Auth, (Re)AssocReq, Deauth, Disassoc) are currently used.
> 
> This makes it easier to implement IEEE 802.11 extensions like WPS and
> FT that add IE(s) into some management frames. In addition, this can
> be useful for testing and experimentation purposes.

Looks sane to me. Would it make sense to provide support for getting
this information as well?

> +	if (!drv->ops->set_mgmt_extra_ie) {
> +		err = -EOPNOTSUPP;
> +		goto out;
> +	}

You could check for that earlier and avoid the goto, I think.

> +/**
> + * struct mgmt_extra_ie_params - Extra management frame IE parameters
> + *
> + * Used to add extra IE(s) into management frames in client modes.

That could be a little more specific, I think. You seem to implement it
only for STA/IBSS, but it could make sense for mesh as well, for
instance to add things into the beacon there? Same with IBSS I guess?

I think we should provide, to userspace, a guarantee such as:

  If this operation completes successfully, then any frame generated by
  the kernel or hardware with that subtype will include the information.

This obviously excludes injected frames and that could be stated
explicitly. Also means that the "client modes" above should be removed
and instead the handler has to just fail in other modes and for
unsupported frame types.

Therefore, beacons should include it in ibss/mesh, and mac80211's cfg
handler should reject attempts to configure anything it doesn't support.
Since then you need to do things based on the frame type, I think I'd
rather open code the subtypes array into ie_presp, ie_preq, etc.

johannes