netdev - Re: What is the correct way to install an L2 multicast route into a bridge?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20200708131000.vs4fkjorvob6zyku@skbuf>
Date:   Wed, 8 Jul 2020 16:10:00 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
Cc:     roopa@...ulusnetworks.com, netdev@...r.kernel.org
Subject: Re: What is the correct way to install an L2 multicast route into a
 bridge?

On Wed, Jul 08, 2020 at 03:55:23PM +0300, Nikolay Aleksandrov wrote:
> On 08/07/2020 14:17, Nikolay Aleksandrov wrote:
> > On 08/07/2020 14:07, Nikolay Aleksandrov wrote:
> >> On 08/07/2020 12:42, Vladimir Oltean wrote:
> >>> On Wed, Jul 08, 2020 at 12:16:27PM +0300, Nikolay Aleksandrov wrote:
> >>>> On 08/07/2020 12:04, Vladimir Oltean wrote:
> [snip]
> >>>>
> >>>
> >>> Thanks, Nikolay.
> >>> Isn't mdb_modify() already netlink-based? I think you're talking about
> >>> some changes to 'struct br_mdb_entry' which would be necessary. What
> >>> changes would be needed, do you know (both in the 'workaround' case as
> >>> well as in 'fully netlink')?
> >>>
> >>> -Vladimir
> >>>
> >>
> >> That is netlink-based, but the uAPI (used also for add/del/dump) uses a fixed-size struct
> >> which is very inconvenient and hard to extend. I plan to add MDBv2 which uses separate
> >> netlink attributes and can be easily extended as we plan to add some new features and will
> >> need that flexibility. It will use a new container attribute for the notifications as well.
> >>
> >> In the workaround case IIRC you'd have to add a new protocol type to denote the L2 routes, and
> > 
> > Actually drop the whole /workaround/ comment altogether. It can be implemented fairly straight-forward
> > even with the struct we got now. You don't need any new attributes.
> > I just had forgotten the details and spoke too quickly. :)
> > 
> >> re-work the lookup logic to include L2 in non-IP case. You'd have to edit the multicast fast-path,
> >> and everything else that assumes the frame has to be IP/IPv6. I'm sure I'm missing some details as
> >> last I did this was over an year ago where I made a quick and dirty hack that implemented it with proto = 0
> >> to denote an L2 entry just as a proof of concept.
> >> Also you would have to make sure all of that is compatible with current user-space code. For example
> >> iproute2/bridge/mdb.c considers that proto can be only IPv4 or IPv6 if it's not v4, i.e. it will
> >> print the new L2 entries as :: IPv6 entries until it's fixed.
> >>
> >> Obviously some of the items for the workaround case are valid in all cases for L2 routes (e.g. fast-path/lookup edit).
> >> But I think it's not that hard to implement without affecting the fast path much or even at all.
> >>
> >> Cheers,
> >>  Nik
> >>
> 
> I found the patch and rebased it against net-next. I want to stress that it is unfinished and
> barely tested, it was just a hack to enable L2 entries and forwarding.
> If you're interested and find it useful please feel free to take it over as
> I don't have time right now.
> 
> Thanks,
>  Nik
> 
> 

Thanks! I'll give it a try, and I'll submit it if I get it to work
properly and see no regressions with IP multicast.

-Vladimir