On Tue, Feb 9, 2021 at 12:51 PM Vladimir Oltean <olte...@gmail.com> wrote: > > On Tue, Feb 09, 2021 at 12:37:38PM -0600, George McCollister wrote: > > On Tue, Feb 9, 2021 at 11:20 AM Vladimir Oltean <olte...@gmail.com> wrote: > > > > > > On Mon, Feb 08, 2021 at 11:21:26AM -0600, George McCollister wrote: > > > > > If you return zero, the software fallback is never going to kick in. > > > > > > > > For join and leave? How is this not a problem for the bridge and lag > > > > functions? They work the same way don't they? I figured it would be > > > > safe to follow what they were doing. > > > > > > I didn't say that the bridge and LAG offloading logic does the right > > > thing, but it is on its way there... > > > > > > Those "XXX not offloaded" messages were tested with cases where the > > > .port_lag_join callback _is_ present, but fails (due to things like > > > incompatible xmit hashing policy). They were not tested with the case > > > where the driver does not implement .port_lag_join at all. > > > > > > Doesn't mean you shouldn't do the right thing. I'll send some patches > > > soon, hopefully, fixing that for LAG and the bridge, you can concentrate > > > on HSR. For the non-offload scenario where the port is basically > > > standalone, we also need to disable the other bridge functions such as > > > address learning, otherwise it won't work properly, and that's where > > > I've been focusing my attention lately. You can't offload the bridge in > > > software, or a LAG, if you have address learning enabled. For HSR it's > > > even more interesting, you need to have address learning disabled even > > > when you offload the DANH/DANP. > > > > Do I just return -EOPNOTSUPP instead of 0 in dsa_switch_hsr_join and > > dsa_switch_hsr_leave? > > Yes, return -EOPNOTSUPP if the callbacks are not implemented please.
Will do. > > > I'm not sure exactly what you're saying needs to be done wrt to > > address learning with HSR. The switch does address learning > > internally. Are you saying the DSA address learning needs to be > > disabled? > > I'm saying that when you're doing any sort of redundancy protocol, the > switch will get confused by address learning if it performs it at > physical port level, because it will see the same source MAC address > coming in from 2 (or more) different physical ports. And when it sees a > packet coming in through a port that it had already learned it should be > the destination for the MAC address because an earlier packet came > having that MAC address as source, it will think it should do > hairpinning which it's configured not to => it'll drop the packet. > > Now, your switch might have some sort of concept of address learning at > logical port level, where the "logical port" would roughly correspond to > the hsr0 upper (I haven't opened the XRS700x manual to know if it does > this, sorry). Basically if you support RedBox I expect that the switch > is able to learn at the level of "this MAC address came from the HSR > ring, aka from one or both of the individual ring ports". But for > configuring that in Linux, you'd need something like: Yup, the switch has provisions to deal with this. > > ip link set hsr0 master br0 > ip link set hsr0 type bridge_slave learning on > > and then catch from DSA the switchdev notification emitted for hsr0, and > use that to configure learning on the logical port of your switch > corresponding to hsr0, instead of learning on the physical ports that > offload it. > > There are similar issues related to address learning for everything > except a bridge port, basically. > > > If that's something I need for this patch some tips on what > > to do would be appreciated because I'm a bit lost. > > I didn't say you need to change something related to learning for this > series, because you can't anyway - DSA doesn't give you the knobs to > configure address learning yet. The series where I try to add those are > here: > https://patchwork.kernel.org/project/netdevbpf/cover/20210209151936.97382-1-olte...@gmail.com/ > > The take-away is that with those changes, a DSA driver should start its > ports in standalone mode with learning disabled and flooding of all > kinds enabled, and then treat the .port_bridge_flags callback which > should do the right thing and enable/disable address learning only when > necessary. > > All I said is that address learning remaining enabled has been an issue > that prevented the non-offload scenarios from really working, but you > shouldn't be too hung up on that, and still return -EOPNOTSUPP, thereby > allowing the software fallback to kick in, even if it doesn't work. Thanks for clearing that up and explaining how this will work in the future.
