Re: What is the purpose of dev->gflags?

[Vladimir Oltean <olteanv@gmail.com>]

On Mon, Apr 11, 2022 at 05:26:05PM +0200, Nicolas Dichtel wrote:
> Le 08/04/2022 à 21:17, Vladimir Oltean a écrit :
> > On Fri, Apr 08, 2022 at 11:50:54AM -0700, Jakub Kicinski wrote:
> >> On Fri, 8 Apr 2022 21:30:45 +0300 Vladimir Oltean wrote:
> >>> Hello,
> >>>
> >>> I am trying to understand why dev->gflags, which holds a mask of
> >>> IFF_PROMISC | IFF_ALLMULTI, exists independently of dev->flags.
> >>>
> >>> I do see that __dev_change_flags() (called from the ioctl/rtnetlink/sysfs
> >>> code paths) updates the IFF_PROMISC and IFF_ALLMULTI bits of
> >>> dev->gflags, while the direct calls to dev_set_promiscuity()/
> >>> dev_set_allmulti() don't.
> >>>
> >>> So at first I'd be tempted to say: IFF_PROMISC | IFF_ALLMULTI are
> >>> exposed to user space when set in dev->gflags, hidden otherwise.
> >>> This would be consistent with the implementation of dev_get_flags().
> >>>
> >>> [ side note: why is that even desirable? why does it matter who made an
> >>>   interface promiscuous as long as it's promiscuous? ]
> I think this was historical, I had the same questions a long time ago.
> 
> >>
> >> Isn't that just a mechanism to make sure user space gets one "refcount"
> >> on PROMISC and ALLMULTI, while in-kernel calls are tracked individually
> >> in dev->promiscuity? User space can request promisc while say bridge
> >> already put ifc into promisc mode, in that case we want promisc to stay
> >> up even if ifc is unbridged. But setting promisc from user space
> >> multiple times has no effect, since clear with remove it. Does that
> >> help? 
> > 
> > Yes, that helps to explain one side of it, thanks. But I guess I'm still
> > confused as to why should a promiscuity setting incremented by the
> > bridge be invisible to callers of dev_get_flags (SIOCGIFFLAGS,
> > ifinfomsg::ifi_flags [ *not* IFLA_PROMISCUITY ]).
> If I remember well, the goal was to advertise these flags to userspace only when
> they were set by a userspace app and not by a kernel module (bridge, bonding, etc).
> To avoid changing that behavior, IFLA_PROMISCUITY was introduced, thus userspace
> may know if promiscuity is enabled by dumping the interface. Notifications were
> fixed later, but maybe some are still missing.

Thanks.
Would you agree that the __dev_set_allmulti() -> __dev_notify_flags()
call path is dead code? If it is, is there any problem it should be
addressing which it isn't, or can we just delete it?