netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Erik Stromdahl <erik.stromdahl@gmail.com>
To: Jouni Malinen <j@w1.fi>
Cc: johannes@sipsolutions.net, johannes.berg@intel.com,
	kvalo@codeaurora.org, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH] mac80211_hwsim: Added vendor echo command
Date: Sat, 25 Jun 2016 18:08:01 +0200	[thread overview]
Message-ID: <576EAC61.1050900@gmail.com> (raw)
In-Reply-To: <20160624204759.GA15514@w1.fi>

The mac80211_hwsim driver is currently defining the QCA vendor ID 
0x001374 for some reason.

Also, all current vendor commands are using the "QCA_" prefix, hence the 
reason why I decided to add the new vendor command with the "QCA_" 
prefix as well.

Anyway, I understand that you don't want the hwsim driver bloated with 
definitions that does not belong there.

Is there any reason why hwsim uses the QCA vendor id?
Perhaps it would be best if the hwsim driver would define its own vendor id?

On 06/24/2016 10:47 PM, Jouni Malinen wrote:
> On Fri, Jun 24, 2016 at 10:13:54AM +0200, Erik Stromdahl wrote:
>> The purpose of the echo command is to provide a test
>> facility for user space programs.
>
>> diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
>> @@ -332,14 +332,16 @@ static const struct ieee80211_rate hwsim_rates[] = {
>>   #define QCA_NL80211_SUBCMD_TEST 1
>> +#define QCA_NL80211_SUBCMD_ECHO 2
>
> NAK. That QCA vendor specific value has not been assigned nor have I
> even seen a request to assign such a value.
>
>>   enum qca_nl80211_vendor_subcmds {
>>   	QCA_WLAN_VENDOR_ATTR_TEST = 8,
>> -	QCA_WLAN_VENDOR_ATTR_MAX = QCA_WLAN_VENDOR_ATTR_TEST
>> +	QCA_WLAN_VENDOR_ATTR_ECHO,
>
> And this vendor attribute value has already been assigned for another
> purpose.
>
> It is no acceptable to pick arbitrary values of identifiers without
> proper request to the owner of the OUI that is used to assign the
> values.
>
> As far as the QCA vendor specific identifiers are concerned, their
> assignment process is described here:
> http://w1.fi/cgit/hostap/plain/src/common/qca-vendor.h
>
> Please do not submit kernel changes that use any unassigned vendor
> identifier or even worse, a value that have already been assigned for a
> completely different purpose.
>
>

  reply	other threads:[~2016-06-25 16:08 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-24  8:13 [PATCH] mac80211_hwsim: Added vendor echo command Erik Stromdahl
     [not found] ` <1466756034-5485-1-git-send-email-erik.stromdahl-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-06-24 20:47   ` Jouni Malinen
2016-06-25 16:08     ` Erik Stromdahl [this message]
     [not found]       ` <576EAC61.1050900-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-06-25 18:32         ` Jouni Malinen
2016-06-25 19:57           ` Johannes Berg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=576EAC61.1050900@gmail.com \
    --to=erik.stromdahl@gmail.com \
    --cc=j@w1.fi \
    --cc=johannes.berg@intel.com \
    --cc=johannes@sipsolutions.net \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).