Re: [PATCH 3/6] nl80211: Add CMD_CONTROL_PORT_FRAME API

[Denis Kenzior <denkenz@gmail.com>]

Hi Johannes,

>> + * @NL80211_CMD_CONTROL_PORT_FRAME: Control Port (e.g. PAE) frame TX request
>> + *	and RX notification.  This command is used both as a request to transmit
>> + *	a control port frame and as a notification that a control port frame
>> + *	has been received. %NL80211_ATTR_FRAME is used to specify the
>> + *	frame contents.  The frame is the raw EAPoL data, without ethernet or
>> + *	802.11 headers.
> 
> Never mind, so it's without Ethernet header. Is that really desirable
> though? I mean, it could be that the Ethernet address even matters (not
> sure) and it'd probably be easier to handle in (existing) userspace
> where Ethernet frames are expected now?

I also include the from address inside the NL80211 message as ATTR_MAC. 
The protocol as well.  I wrote the docs first and never updated the 
little details afterwards.  Will fix.

> 
>> + nla_put_failure:
>> +	genlmsg_cancel(msg, hdr);
> 
> nit: there's no point in cancelling if you free it (immediately).

Just following some existing code, but will fix.

> 
>> +bool cfg80211_rx_control_port(struct net_device *dev,
>> +			      const u8 *buf, size_t len,
>> +			      const u8 *addr, u16 proto, bool unencrypted)
>> +{
>> +	bool ret;
>> +
>> +	trace_cfg80211_rx_control_port(dev, buf, len, addr, proto, unencrypted);
>> +	ret = __nl80211_rx_control_port(dev, buf, len, addr, proto,
>> +					unencrypted, GFP_ATOMIC);
>> +	trace_cfg80211_return_bool(ret);
> 
> this seems wrong - you return -ERROR from __nl80211_rx_control_port()
> so you need either to pass that on as an integer to the caller, or put
> an == 0 here or something?
> 
> "Return: %true if the frame was passed to userspace"
> 

Yep, will fix.

Regards,
-Denis