Re: [PATCH] ath10k: add dynamic vlan support

[Manikanta Pubbisetty <mpubbise@codeaurora.org>]

On 9/3/2018 4:09 PM, Johannes Berg wrote:

> Hi,
>
> Sorry ... this got delayed again.
>
>> I had introduced a change db3bdcb9c3ff (" mac80211: allow AP_VLAN
>> operation on crypto controlled devices ") for supporting VLAN
>> functionality on ath10k devices; this commit has broken 4 addr support
>> on ath10k devices as I was advertising the AP/VLAN support
>> conditionally. Since 4 addr operation is tied to AP/VLAN support, with
>> this change, only the chips which support VLAN functionality can support
>> 4 addr operation but other ath10k chips don't.
> Right.
>
>>   From what I can understand from our previous discussions, we had to
>> separate the 4addr support from the AP/VLAN iftype but doing so could
>> lead to backward compatibility issues. I have the code which separates
>> the 4addr functionality from AP/VLAN but the bigger problem is the
>> backward compatibility.
> Ok.
>
>> I am hoping that now I have set the context correctly :)
> Thanks!
>
>>> I think we have to keep the 4-addr handling in AP_VLAN iftype either
>>> way, to not break existing hostapd. We could introduce a separate
>>> AP_VLAN_NO_4ADDR and then require updating hostapd to get non-4addr
>>> VLAN, but that also seems awkward.
>>>
>>> Since hostapd doesn't currently check anything...
>>>
>>> Ok, no, I'm confused now. If we just clear WIPHY_FLAG_4ADDR_AP, don't
>>> we
>>> get what we want? 4-addr AP_VLAN interfaces would no longer be
>>> permitted
>>> to be created?
>> As I explained above, the agenda is to provide the driver (in this case,
>> ath10k) a better control for advertising the device capabilities; only
>> few ath10k chips can support VLAN functionality but all of them can
>> support 4 addr operation.
> So the problematic part isn't actually the *4-addr* (fake) VLAN, the
> problematic part is the actual AP_VLAN - I suppose because that uses a
> separate GTK.
>
>
> So I guess the only, mostly backward compatible way to really separate
> the two would be to
>
> 1) move WIPHY_FLAG_4ADDR_{AP,STATION} to be nl80211 ext feature flags,
>     I guess the STATION always should've been since there's nothing that
>     indicates support for it today in the API
>
> along with one of:
>
> 2a) Add a new AP_VLAN ext feature flag that indicates the AP_VLAN is not
>      only supported for 4-addr
>
> 2b) Allow creating an AP_VLAN interface in 4-addr mode in
>      nl80211_new_interface() even when AP_VLAN is not in the supported
>      interface combinations, if (and only if) WIPHY_FLAG_4ADDR_AP is set
>      (or rather the new extended feature flag after doing 1). Update the
>      language in the documentation somewhere indicating this.
>
>
> Hostapd clearly deals with both 2a and 2b since it never bothers to
> check the combinations. As a result, I prefer 2b rather than 2a since it
> doesn't add any weird combinations to the API that would be impossible.

Sure Johannes!!

Thanks,
Manikanta

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