On 30/04/2024 20:01, Joseph Huang wrote: > On 4/29/2024 9:21 PM, Vladimir Oltean wrote: >> On Mon, Apr 29, 2024 at 04:14:03PM -0400, Joseph Huang wrote: >>> How about the following syntax? I think it satisfies all the "not breaking >>> existing behavior" requirements (new option defaults to off, and missing >>> user space netlink attributes does not change the existing behavior): >>> >>> mcast_flood off >>> all off >>> mcast_flood off mcast_flood_rfc4541 off >>> all off >>> mcast_flood off mcast_flood_rfc4541 on >>> 224.0.0.X and ff02::1 on, the rest off >>> mcast_flood on >>> all on >>> mcast_flood on mcast_flood_rfc4541 off >>> all on (mcast_flood on overrides mcast_flood_rfc4541) >>> mcast_flood on mcast_flood_rfc4541 on >>> all on >>> mcast_flood_rfc4541 off >>> invalid (mcast_flood_rfc4541 is only valid if mcast_flood [on | off] is >>> specified first) >>> mcast_flood_rfc4541 on >>> invalid (mcast_flood_rfc4541 is only valid if mcast_flood [on | off] is >>> specified first) >> >> A bridge port defaults to having BR_MCAST_FLOOD set - see new_nbp(). >> Netlink attributes are only there to _change_ the state of properties in >> the kernel. They don't need to be specified by user space if there's >> nothing to be changed. "Only valid if another netlink attribute comes >> first" makes no sense. You can alter 2 bridge port flags as part of the >> same netlink message, or as part of different netlink messages (sent >> over sockets of other processes). >> >>> >>> Think of mcast_flood_rfc4541 like a pet door if you will. >> >> Ultimately, as far as I see it, both the OR-based and the AND-based UAPI >> addition could be made to work in a way that's perhaps similarly backwards >> compatible. It needs to be worked out with the bridge maintainers. Given >> that I'm not doing great with my spare time, I will take a back seat on >> that. > > Nik, do you have any objection to the following proposal? > > mcast_flood -> default/ off on > (existing flag) missing (specified/ (specified/ > (on) nlmsg) nlmsg) > > mcast_flood_rfc4541 > (proposed new flag) > | > v > default/ flood all no flood flood all > missing > (off) > > off flood all no flood flood all > (specified/nlmsg) > > on flood all flood 4541 flood all > (specified/nlmsg) ^^^^^^^^^^ > only behavior change > > > Basically the attributes are OR'ed together to form the final flooding decision. > > Looks good to me. Please make use of the boolopt uapi to avoid adding new nl attributes. Thanks, Nik
