On February 13, 2019 8:17:16 AM PST, Ivan Khoronzhuk <ivan.khoronzhuk@xxxxxxxxxx> wrote: >On Tue, Jan 22, 2019 at 03:12:41PM +0200, Ivan Khoronzhuk wrote: >>On Mon, Jan 21, 2019 at 03:37:41PM -0800, Florian Fainelli wrote: >>>On 12/4/18 3:42 PM, Ivan Khoronzhuk wrote: >>>>On Tue, Dec 04, 2018 at 11:49:27AM -0800, Florian Fainelli wrote: >> >>[...] >> >>> >>>Ivan, based on the recent submission I copied you on [1], it sounds >like >>>we want to move ahead with your proposal to extend netdev_hw_addr >with a >>>vid member. >>> >>>On second thought, your approach is good and if we enclose the vid >>>member within an #if IS_ENABLED(CONFIG_VLAN)8021Q) we should be good >for >>>most foreseeable use cases, if not, we can always introduce a >variable >>>size/defined context in the future. >>> >>>Can you resubmit this patch series as non-RFC in the next few days so >I >>>can also repost mine [1] and take advantage of these changes for >>>multicast over VLAN when VLAN filtering is globally enabled on the >device. >>> >>>[1]: https://www.spinics.net/lists/netdev/msg544722.html >>> >>>Thanks! >> >>Yes, sure. I can start to do that in several days. >>Just a little busy right now. >> >>Just before doing this, maybe some comments could be added as it has >more >>attention now. Meanwhile I can send alternative variant but based on >>real dev splitting addresses between vlans. In this approach it leaves >address >>space w/o vid extension but requires more changes to vlan core. >Drawback here >>that to change one address alg traverses all related vlan addresses, >it can be >>cpu/time wasteful, if it's done regularly, but saves memory.... >> >>Basically it's implemented locally in cpsw and requires more changes >to move >>it as some vlan core auxiliary functions to be reused. But it can work >only >>with vlans directly on top of real dev, which is fixable. >> >>Core function here: >>__hw_addr_ref_sync_dev >>it is called only for address the link of which was >increased/decreased, thus >>update made only on one address, comparing it for every vlan dev. >> >>It was added with this patch: >>[1] net: core: dev_addr_lists: add auxiliary func to handle reference >>address update e7946760de5852f32 >> >>And used by this patch: >>[2] net: ethernet: ti: cpsw: fix vlan mcast 15180eca569bfe1d4d >> >>So, idea is to move [2] to be vlan core auxiliary function to be >reused >>by NIC drivers. >> >>But potentially it can bring a little more changes I assume: >> >>1) add priv_flag |= IFF_IV_FLT (independent vlan filtering). It allows >to reuse >>this flag for farther changes, probably for per vlan allmulti or so. >> >>2) real dev has to have complete list for vlans, not only their vids, >but also >>all vlandevs in device chain above it. So changes in add_vid can be >required. >>Vlan core can assign vlan dev pointer to real device only after it's >completely >>initialized. And for propagation reasons it requires every device in >>infrastructure to be aware. That seems doable, but depends not only on >me. >> >>3) Move code from [2] to be auxiliary vlan core API for setting mc and >uc. >>From this patch only one function is cpsw specific: cpsw_set_mc(). The >rest can >>be applicable on every NIC supporting IFF_IV_FLT. >> >>4) Move code from link below to do the same but for uc addresses: >>https://git.linaro.org/people/ivan.khoronzhuk/tsn_kernel.git/commit/?h=ucast_vlan_fix&id=ebc88a7d8758759322d9ff88f25f8bac51ce7219 >>here only one func cpsw specific: cpsw_set_uc() >>the rest can be generic. >> >>As third alternative, we can think about how to reduce memory for >addresses by >>reusing them or else, but this is as continuation of addr+vid >approach, and API >>probably would be the same. >> >>Then all this can be compared for proper decision. > > >Hi Florian, > >After several more investigations and tries probably better left this >idea as is. Thank you for keeping the thread alive, does that mean you are going to resubmit this patch series as-is (rebased) or are you saying that you are abandoning the idea and leaving the situation the way it is in cpsw? > >Here actually several explanations for this: >1) If even assume that we can get access to vlan devices in the above >ndev >tree (we can) that doesn't guarantee that receive vlan filters are set >replicating this structure. For example bond device can have one active >slave >but both of them in the tree having vid set, in this case addresses are >syched only with active slave, no filters should be applied to not >active slave. >this can be achieved only each address has vid context. > >2) According to 1) rx filters device structure can be created while >mc_sync() >in each rx_mode(), and then used as orthogonal info. I've tried and it >looks >not cool and consumes anyway memory and even if it's less it's still >not very >scalable. (+ no normal signal "in complex structure case" when address >should >be undated to avoid redundant cpu cycles). Not sure it can have >practical >results and be universal enouph. > >3) Assuming that every device in the tree (bond, team or else) is legal >to >modify its own address space, the real end device cannot be sure the >vlan device >address spaces reflects vid addresses that device tree want's from him. >According to this each address in address space must hold its own >context at >every device and this context is comparable with address size. > >>-- Regards, >>Ivan Khoronzhuk -- Florian
