All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Oh <poh@codeaurora.org>
To: Johannes Berg <johannes@sipsolutions.net>,
	Peter Oh <poh@qca.qualcomm.com>,
	ath10k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH v2] cfg80211: add VHT support for Mesh
Date: Fri, 13 Nov 2015 15:50:37 -0800	[thread overview]
Message-ID: <5646774D.2000801@codeaurora.org> (raw)
In-Reply-To: <1447400275.3271.2.camel@sipsolutions.net>


On 11/12/2015 11:37 PM, Johannes Berg wrote:
> On Thu, 2015-11-12 at 15:05 -0800, Peter Oh wrote:
>> On 11/12/2015 02:32 PM, Johannes Berg wrote:
>>> On Thu, 2015-11-12 at 14:28 -0800, Peter Oh wrote:
>>>>    
>>>> Exactly the same communication mechanism and purpose are used
>>>> with
>>>> NL80211_EXT_FEATURE_VHT_IBSS which is already a part of NL80211
>>>> feature
>>>> flag.
>>>> The new feature flag, NL80211_EXT_FEATURE_VHT_MESH, follows the
>>>> same
>>>> purpose and usage.
>>> No, it doesn't. Check how the _IBSS one is used in the code to
>>> actually
>>> *do* something.
>> that's right. so take a look reset of explanation for this patch.
>>
> Still not making sense.
>
> I *suspect* that you think that the existing code is broken, and can't
> use VHT mesh and requires driver changes for it,
I'm saying, cannot use VHT Mesh by wpa_supplicant because a lack of 
communication method between driver and wpa_supplicant, hence extending 
NL80211 feature flag.
>   but that's not what
> your ath10k change shows since it also does nothing at all.
ath10k is advertising its VHT Mesh capability using NL80211 feature flag 
in the patch which wpa_supplicant can listen and catch up.
>
> Right now, I see no reason whatsoever to apply either one of those two
> patches. There are no functional changes, so wpa_supplicant could
> enable VHT mesh by checking VHT capabilities or so instead of a special
> feature flag.
You're asking wpa_supplicant enhancement which is not fair to engineers 
that did not involve the design implementation.
Also if you think Mesh should check VHT capabilities instead of a 
special feature flag, you had to ask the same thing to _VHT_IBSS.
_VHT_IBSS feature flag is also used to advertise driver's capability of 
VHT for IBSS instead of checking driver's information elements.
if IBSS had read VHT capability from driver's IE, _VHT_IBSS flag used at 
nl80211_join_ibss also shouldn't have existed.
>
> I also suspect that perhaps mesh *should* be checking like IBSS does,
> although I also would actually *prefer* that we can assume VHT mesh
> works if the driver advertises VHT support and mesh support separately,
> i.e. a new feature flag really isn't necessary.
Both IBSS and Mesh may support VHT without any feature flags. However 
this patch's approach is come from _VHT_IBSS feature flag which you 
already approved and so exists on upstream.
If you're asking not to use _VHT_MESH feature flag and look for another 
way, your request affects to wpa_supplicant design of ibss and mesh, 
since mesh and ibss frequency info are handled at the same function and 
the function is designed to use _VHT_IBSS feature flag. If Mesh cannot 
use the _VHT_MESH feature flag, the design of function cannot keep the 
consistency of capability check.

If you're saying _VHT_MESH feature flag is different from _VHT_IBSS 
because of what it is doing at nl80211_join_ibss function, I will add 
another patch to nl80211_join_mesh function to make _VHT_MESH feature 
flag the same as _VHT_IBSS. Will you be convinced then?

>
> In any case, the arguments for this patch haven't convinced me. I'm not
> going to apply this without much better ones.
>
> johannes
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k
Thanks,
Peter

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

WARNING: multiple messages have this Message-ID (diff)
From: Peter Oh <poh@codeaurora.org>
To: Johannes Berg <johannes@sipsolutions.net>,
	Peter Oh <poh@qca.qualcomm.com>,
	ath10k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH v2] cfg80211: add VHT support for Mesh
Date: Fri, 13 Nov 2015 15:50:37 -0800	[thread overview]
Message-ID: <5646774D.2000801@codeaurora.org> (raw)
In-Reply-To: <1447400275.3271.2.camel@sipsolutions.net>


On 11/12/2015 11:37 PM, Johannes Berg wrote:
> On Thu, 2015-11-12 at 15:05 -0800, Peter Oh wrote:
>> On 11/12/2015 02:32 PM, Johannes Berg wrote:
>>> On Thu, 2015-11-12 at 14:28 -0800, Peter Oh wrote:
>>>>    
>>>> Exactly the same communication mechanism and purpose are used
>>>> with
>>>> NL80211_EXT_FEATURE_VHT_IBSS which is already a part of NL80211
>>>> feature
>>>> flag.
>>>> The new feature flag, NL80211_EXT_FEATURE_VHT_MESH, follows the
>>>> same
>>>> purpose and usage.
>>> No, it doesn't. Check how the _IBSS one is used in the code to
>>> actually
>>> *do* something.
>> that's right. so take a look reset of explanation for this patch.
>>
> Still not making sense.
>
> I *suspect* that you think that the existing code is broken, and can't
> use VHT mesh and requires driver changes for it,
I'm saying, cannot use VHT Mesh by wpa_supplicant because a lack of 
communication method between driver and wpa_supplicant, hence extending 
NL80211 feature flag.
>   but that's not what
> your ath10k change shows since it also does nothing at all.
ath10k is advertising its VHT Mesh capability using NL80211 feature flag 
in the patch which wpa_supplicant can listen and catch up.
>
> Right now, I see no reason whatsoever to apply either one of those two
> patches. There are no functional changes, so wpa_supplicant could
> enable VHT mesh by checking VHT capabilities or so instead of a special
> feature flag.
You're asking wpa_supplicant enhancement which is not fair to engineers 
that did not involve the design implementation.
Also if you think Mesh should check VHT capabilities instead of a 
special feature flag, you had to ask the same thing to _VHT_IBSS.
_VHT_IBSS feature flag is also used to advertise driver's capability of 
VHT for IBSS instead of checking driver's information elements.
if IBSS had read VHT capability from driver's IE, _VHT_IBSS flag used at 
nl80211_join_ibss also shouldn't have existed.
>
> I also suspect that perhaps mesh *should* be checking like IBSS does,
> although I also would actually *prefer* that we can assume VHT mesh
> works if the driver advertises VHT support and mesh support separately,
> i.e. a new feature flag really isn't necessary.
Both IBSS and Mesh may support VHT without any feature flags. However 
this patch's approach is come from _VHT_IBSS feature flag which you 
already approved and so exists on upstream.
If you're asking not to use _VHT_MESH feature flag and look for another 
way, your request affects to wpa_supplicant design of ibss and mesh, 
since mesh and ibss frequency info are handled at the same function and 
the function is designed to use _VHT_IBSS feature flag. If Mesh cannot 
use the _VHT_MESH feature flag, the design of function cannot keep the 
consistency of capability check.

If you're saying _VHT_MESH feature flag is different from _VHT_IBSS 
because of what it is doing at nl80211_join_ibss function, I will add 
another patch to nl80211_join_mesh function to make _VHT_MESH feature 
flag the same as _VHT_IBSS. Will you be convinced then?

>
> In any case, the arguments for this patch haven't convinced me. I'm not
> going to apply this without much better ones.
>
> johannes
>
> _______________________________________________
> ath10k mailing list
> ath10k@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/ath10k
Thanks,
Peter

  reply	other threads:[~2015-11-13 23:51 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-12 17:59 [PATCH v2] cfg80211: add VHT support for Mesh Peter Oh
2015-11-12 17:59 ` Peter Oh
2015-11-12 20:03 ` Johannes Berg
2015-11-12 20:03   ` Johannes Berg
2015-11-12 21:33   ` Peter Oh
2015-11-12 21:33     ` Peter Oh
2015-11-12 21:40     ` Johannes Berg
2015-11-12 21:40       ` Johannes Berg
2015-11-12 22:28       ` Peter Oh
2015-11-12 22:28         ` Peter Oh
2015-11-12 22:32         ` Johannes Berg
2015-11-12 22:32           ` Johannes Berg
2015-11-12 23:05           ` Peter Oh
2015-11-12 23:05             ` Peter Oh
2015-11-13  7:37             ` Johannes Berg
2015-11-13  7:37               ` Johannes Berg
2015-11-13 23:50               ` Peter Oh [this message]
2015-11-13 23:50                 ` Peter Oh
2015-11-15 14:38                 ` Johannes Berg
2015-11-15 14:38                   ` 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=5646774D.2000801@codeaurora.org \
    --to=poh@codeaurora.org \
    --cc=ath10k@lists.infradead.org \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=poh@qca.qualcomm.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.