On 03/04/2019 21:17, Nikolay Aleksandrov wrote: > On 03/04/2019 20:53, Nikolay Aleksandrov wrote: >> On 03/04/2019 20:43, Mike Manning wrote: >>> On 02/04/2019 20:22, Nikolay Aleksandrov wrote: >>>> On 02/04/2019 18:35, Mike Manning wrote: >>>>> In the case of vlan filtering on bridges, the bridge may also have the >>>>> corresponding vlan devices as upper devices. A vlan bridge binding mode >>>>> is added to allow the link state of the vlan device to track only the >>>>> state of the subset of bridge ports that are also members of the vlan, >>>>> rather than that of all bridge ports. This mode is set with a vlan flag >>>>> rather than a bridge sysfs so that the 8021q module is aware that it >>>>> should not set the link state for the vlan device. >>>>> >>>>> If bridge vlan is configured, the bridge device event handling results >>>>> in the link state for an upper device being set, if it is a vlan device >>>>> with the vlan bridge binding mode enabled. This also sets a >>>>> vlan_bridge_binding flag so that subsequent UP/DOWN/CHANGE events for >>>>> the ports in that bridge result in a link state update of the vlan >>>>> device if required. >>>>> >>>>> The link state of the vlan device is up if there is at least one bridge >>>>> port that is a vlan member that is admin & oper up, otherwise its oper >>>>> state is IF_OPER_LOWERLAYERDOWN. >>>>> >>>>> Signed-off-by: Mike Manning <mmann...@vyatta.att-mail.com> >>>>> --- >>>>> net/bridge/br.c | 23 ++++++-- >>>>> net/bridge/br_private.h | 17 ++++++ >>>>> net/bridge/br_vlan.c | 143 >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> 3 files changed, 179 insertions(+), 4 deletions(-) >>>>> >>>> Hi, >>>> Please CC bridge maintainers when sending bridge patches. >>> Thank you very much for the review, I will CC you and Roopa when I have >>> the v1 series ready. >>>> One question/thought - can't we add a ports_up counter in the vlan's master >>>> struct and keep how many ports are up for that vlan ? >>> >>> This would have been my preferred choice, but for this one would need to >>> know the old link state for a port so as to determine if/what link state >>> transition has occurred for a NETDEV_CHANGE notification. This is if >>> only a single counter is kept for the vlan for all ports (also it might >>> be difficult to recover from an error in the counter). I could see it >>> working if one kept track of the operational state for each port in the >>> vlan in a data structure specific to this purpose i.e. that is more >>> efficient than the existing walk. However, speed in processing these >>> state changes is not that important, also the link state is quickly >>> determined when it might matter more, i.e. on link up of a port. >>> >> >> Indeed, the NETDEV_CHANGE is harder, but we can keep the last known carrier >> state >> in the per-port structure and make a decision based on that and the new >> state. >> That wouldn't require any additional structures. Speed is important to us >> when >> we deploy the bridge at scale, we have tests with thousands of vlans and >> devices >> where this walk would become expensive on link flaps. >> > > In fact we already have a similar tracking field used for the port state, > maybe > it can be used as an indicator. That state needs to be taken into account > anyway > or the carrier state would be wrong. >
Nevermind the last sentence, spoke too quickly. An additional structure may be needed after all, this will need some investigating. >>>> The important part would be to keep it correct, i.e. vlan_add/del should >>>> inc/dec >>>> as well as port up/down. Then we can directly update its carrier on port >>>> event >>>> without doing a possible O(n^2) walk, we just need to walk over the port >>>> vlans >>>> and adjust counters which is always O(n) based on num of that port's vlans. >>>> >>>> Some more comments below. >>> I will make all the other changes you have requested. >>> >> >> Thanks! >> >
