Re: [PATCH V6] cfg80211: introduce critical protocol indication from user-space

["Arend van Spriel" <arend@broadcom.com>]

On 04/16/2013 04:12 PM, Johannes Berg wrote:
>
>
>> I reworked the patch based on your comment in this V6. It
>> applies to the master branch on top of commit:
>>
>> 5253ffb mac80211: always pick a basic rate to tx RTS/CTS for pre-HT rates
>
> Yep, applies, thanks. I think you missed something, I can fix it but
> wanted to ask:
>
>
>> + * @crit_proto_started: true if crit_proto_start() has been done.
>>    */
>>   struct wireless_dev {
>
>> +	bool crit_proto_started;
>
> This is no longer needed, right?
>

oops. That should go indeed.

>> +	NL80211_CMD_CRIT_PROTOCOL_START,
>> +	NL80211_CMD_CRIT_PROTOCOL_STOP,
>> +	NL80211_CMD_CRIT_PROTOCOL_STOPPED_EVENT,
>
> Why use a separate command ID? Usually we use the same _STOP for the
> event as well, I think? Except maybe scan which you can't stop? Not
> sure ... Anyway I don't mind, just wondering if there was a special
> reason to do this.
>

This is my first nl80211 event :-) I looked at the ft_event thingy. I am 
fine using the _STOP instead.

>> + nla_put_failure:
>> +	if (hdr)
>> +		genlmsg_cancel(msg, hdr);
>
> There's not really a reason to cancel, but we still do most of the time.
> I guess we can keep it, but it doesn't matter :)

If it not really needed it may call for separate patch removing all 
occurrences?

>> --- a/net/wireless/rdev-ops.h
>> +++ b/net/wireless/rdev-ops.h
>> @@ -875,7 +875,7 @@ static inline void rdev_stop_p2p_device(struct cfg80211_registered_device *rdev,
>>   	trace_rdev_stop_p2p_device(&rdev->wiphy, wdev);
>>   	rdev->ops->stop_p2p_device(&rdev->wiphy, wdev);
>>   	trace_rdev_return_void(&rdev->wiphy);
>> -}					
>> +}
>
> Heh, thanks.

Have to thank my editor, I guess. Trailing whitespace?

Gr. AvS