Add NL80211_EXT_FEATURE_VHT_MESH flag to allow drivers
to indicate support for VHT in Mesh mode.
This flag will be used by wireless drivers to indicate
the capability of VHT Mesh support and wpa_supplicant to inspect
wireless drivers' capability of VHT Mesh.
In addition to this patch, separate patches to ath10k driver and
wpa_supplicant are also sent out.
Signed-off-by: Peter Oh <[email protected]>
---
v2:
- updated commit message for details
---
include/uapi/linux/nl80211.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 1f0b4cf..4a16ed5 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4389,12 +4389,14 @@ enum nl80211_feature_flags {
/**
* enum nl80211_ext_feature_index - bit index of extended features.
* @NL80211_EXT_FEATURE_VHT_IBSS: This driver supports IBSS with VHT datarates.
+ * @NL80211_EXT_FEATURE_VHT_MESH: This driver supports Mesh with VHT datarates.
*
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
*/
enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_VHT_IBSS,
+ NL80211_EXT_FEATURE_VHT_MESH,
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
--
1.9.1
On Thu, 2015-11-12 at 13:33 -0800, Peter Oh wrote:
>
> At this moment Mesh Point is not able to be brought up by userspace
> such
> as wap_supplicant at VHT data rates because there is no
> events/NL80211
> defined that userspace can check out if drivers can support Mesh
> Point
> (802.11s) at VHT data rates or not, hence only HT data rates can be
> used
> for
> Mesh Point by userspace now.
> This patch is introducing NL80211 event to cover the issue.
No... this patch doesn't introduce an nl80211 event. It introduces a
feature flag, but I'm arguing it's useless.
Look - what's the difference that userspace would do based on this
flag? Clearly, setting the flag in the driver has no effect on kernel
code whatsoever, so your "is not able to be brought up" makes no sense.
Perhaps it'll become clearer once you post the wpa_supplicant code, but
you should still coherently explain in the commit message why this is
necessary. So far, I don't see it, since no kernel code ever depends on
this flag.
johannes
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, but that's not what
your ath10k change shows since it also does nothing at all.
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.
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.
In any case, the arguments for this patch haven't convinced me. I'm not
going to apply this without much better ones.
johannes
On 11/12/2015 01:40 PM, Johannes Berg wrote:
> On Thu, 2015-11-12 at 13:33 -0800, Peter Oh wrote:
>> At this moment Mesh Point is not able to be brought up by userspace
>> such
>> as wap_supplicant at VHT data rates because there is no
>> events/NL80211
>> defined that userspace can check out if drivers can support Mesh
>> Point
>> (802.11s) at VHT data rates or not, hence only HT data rates can be
>> used
>> for
>> Mesh Point by userspace now.
>> This patch is introducing NL80211 event to cover the issue.
> No... this patch doesn't introduce an nl80211 event. It introduces a
> feature flag, but I'm arguing it's useless.
>
> Look - what's the difference that userspace would do based on this
> flag? Clearly, setting the flag in the driver has no effect on kernel
> code whatsoever, so your "is not able to be brought up" makes no sense.
Yes directly but no indirectly.
Setting the flag in the driver will have effect to the driver eventually
by userspace since the flag will be sent up to userspace and userspace
will select data rates according to the flag.
setting NL80211_VHT_MESH flag in driver -> wpa_supplicant requests
driver capabilities via NL80211 -> driver sends up its capabilities
including VHT_MESH -> wpa_supplicant parses and check if VHT_MESH is set
-> wpa_supplicant configures the driver with VHT data rates if the flag
set or configure HT data rates.
>
> Perhaps it'll become clearer once you post the wpa_supplicant code,
This is ath10k driver diff I sent out for upstream review.
@@ -7266,6 +7266,7 @@ int ath10k_mac_register(struct ath10k *ar)
}
wiphy_ext_feature_set(ar->hw->wiphy, NL80211_EXT_FEATURE_VHT_IBSS);
+ wiphy_ext_feature_set(ar->hw->wiphy, NL80211_EXT_FEATURE_VHT_MESH);
and a part of wpa_supplicant changes.
@@ -359,6 +359,9 @@ static void wiphy_info_ext_feature_flags(struct
wiphy_info_d
if (ext_feature_isset(nla_data(tb), nla_len(tb),
NL80211_EXT_FEATURE_VHT_IBSS))
capa->flags |= WPA_DRIVER_FLAGS_VHT_IBSS;
+ if (ext_feature_isset(nla_data(tb), nla_len(tb),
+ NL80211_EXT_FEATURE_VHT_MESH))
+ capa->flags |= WPA_DRIVER_FLAGS_VHT_MESH;
@@ -1885,12 +1885,13 @@ void ibss_mesh_setup_freq(struct wpa_supplicant
*wpa_s,
- /* Not sure if mesh is ready for VHT */
+ /* For IBSS/Mesh check VHT flag */
+ if (ssid->mode == WPAS_MODE_MESH &&
+ !(wpa_s->drv_flags & WPA_DRIVER_FLAGS_VHT_MESH))
return;
> but
> you should still coherently explain in the commit message why this is
> necessary.
The both of wpa_supplicant and ath10k patches have explanation in their
commit message already.
> So far, I don't see it, since no kernel code ever depends on
> this flag.
A ath10k driver patch in upstream review is using the flag.
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.
>
> johannes
>
> _______________________________________________
> ath10k mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/ath10k
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.
> johannes
Thanks,
Peter
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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/ath10k
Thanks,
Peter
On Fri, 2015-11-13 at 15:50 -0800, Peter Oh wrote:
>
> 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.
> >
And I'm arguing that you don't need such a communication method since,
unlike IBSS, mesh doesn't restrict the channel width usable further
than the HT/VHT capabilities.
It might, on the other hand, very well be that mesh stuff is missing
some VHT compatibility today - that would be an argument for a more
comprehensive patch that then includes the feature flag to say "we've
fixed the VHT issues in mesh mode". That's quite obviously not what
you're doing though.
johannes
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.
johannes
On 11/12/2015 12:03 PM, Johannes Berg wrote:
> On Thu, 2015-11-12 at 09:59 -0800, Peter Oh wrote:
>> Add NL80211_EXT_FEATURE_VHT_MESH flag to allow drivers
>> to indicate support for VHT in Mesh mode.
>> This flag will be used by wireless drivers to indicate
>> the capability of VHT Mesh support and wpa_supplicant to inspect
>> wireless drivers' capability of VHT Mesh.
>> In addition to this patch, separate patches to ath10k driver and
>> wpa_supplicant are also sent out.
>>
> Sorry, still doesn't make sense. What does userspace even need to know
> this *for*?
At this moment Mesh Point is not able to be brought up by userspace such
as wap_supplicant at VHT data rates because there is no events/NL80211
defined that userspace can check out if drivers can support Mesh Point
(802.11s) at VHT data rates or not, hence only HT data rates can be used
for
Mesh Point by userspace now.
This patch is introducing NL80211 event to cover the issue.
> johannes
>
> _______________________________________________
> ath10k mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/ath10k
Thanks,
Peter
On Thu, 2015-11-12 at 09:59 -0800, Peter Oh wrote:
> Add NL80211_EXT_FEATURE_VHT_MESH flag to allow drivers
> to indicate support for VHT in Mesh mode.
> This flag will be used by wireless drivers to indicate
> the capability of VHT Mesh support and wpa_supplicant to inspect
> wireless drivers' capability of VHT Mesh.
> In addition to this patch, separate patches to ath10k driver and
> wpa_supplicant are also sent out.
>
Sorry, still doesn't make sense. What does userspace even need to know
this *for*?
johannes