Hangbin Liu
2021-Oct-20 01:02 UTC
[Bridge] [PATCH net] net: bridge: mcast: Do not allow users to set IGMP counter/timer to zero
On Tue, Oct 19, 2021 at 07:09:42PM +0300, Nikolay Aleksandrov wrote:> > I started this patch when I saw there is not limit for setting > > multicast_membership_interval to 0, which will cause bridge remove the > > mdb directly after adding. Do you think this is a problem. > > > > And what about others? I don't think there is a meaning to set other intervals > > to 0. > > > > The problem is not if there is meaning, we cannot start restricting option values now after > they've become uapi (and have been for a very long time) because we can break user-space even > though chances are pretty low. I don't think this patch is acceptable, I commented on the other > patch issues but they don't matter because of this.OK, I got your mean, we should not restrict the configurations based on whether there is a meaning. Thanks Hangbin
Stephen Hemminger
2021-Oct-20 15:19 UTC
[Bridge] [PATCH net] net: bridge: mcast: Do not allow users to set IGMP counter/timer to zero
On Wed, 20 Oct 2021 09:02:01 +0800 Hangbin Liu <liuhangbin at gmail.com> wrote:> On Tue, Oct 19, 2021 at 07:09:42PM +0300, Nikolay Aleksandrov wrote: > > > I started this patch when I saw there is not limit for setting > > > multicast_membership_interval to 0, which will cause bridge remove the > > > mdb directly after adding. Do you think this is a problem. > > > > > > And what about others? I don't think there is a meaning to set other intervals > > > to 0. > > > > > > > The problem is not if there is meaning, we cannot start restricting option values now after > > they've become uapi (and have been for a very long time) because we can break user-space even > > though chances are pretty low. I don't think this patch is acceptable, I commented on the other > > patch issues but they don't matter because of this. > > OK, I got your mean, we should not restrict the configurations based on whether > there is a meaning. > > Thanks > HangbinMaybe the bridge command could enforce that the value set are sane relative to the RFC? We already fixup some things in iproute2 utilities to workaround places where changing defaults in kernel would break userspace.