[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220408183045.wpyx7tqcgcimfudu@skbuf>
Date: Fri, 8 Apr 2022 21:30:45 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Nicolas Dichtel <nicolas.dichtel@...nd.com>
Subject: What is the purpose of dev->gflags?
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? ]
But in the process of digging deeper I stumbled upon Nicolas' commit
991fb3f74c14 ("dev: always advertise rx_flags changes via netlink")
which I am still struggling to understand.
There, a call to __dev_notify_flags(gchanges=IFF_PROMISC) was added to
__dev_set_promiscuity(), called with "notify=true" from dev_set_promiscuity().
In my understanding, "gchanges" means "changes to gflags", i.e. to what
user space should know about. But as discussed above, direct calls to
dev_set_promiscuity() don't update dev->gflags, yet user space is
notified via rtmsg_ifinfo() of the promiscuity change.
Another oddity with Nicolas' commit: the other added call to
__dev_notify_flags(), this time from __dev_set_allmulti().
The logic is:
static int __dev_set_allmulti(struct net_device *dev, int inc, bool notify)
{
unsigned int old_flags = dev->flags, old_gflags = dev->gflags;
dev->flags |= IFF_ALLMULTI;
(bla bla, stuff that doesn't modify dev->gflags)
if (dev->flags ^ old_flags) {
(bla bla, more stuff that doesn't modify dev->gflags)
if (notify)
__dev_notify_flags(dev, old_flags,
dev->gflags ^ old_gflags);
~~~~~~~~~~~~~~~~~~~~~~~~
oops, dev->gflags was never
modified, so this call to
__dev_notify_flags() is
effectively dead code, since
user space is not notified,
and a NETDEV_CHANGE netdev
notifier isn't emitted
either, since IFF_ALLMULTI is
excluded from that
}
return 0;
}
Can someone please clarify what is at least the intention? As can be
seen I'm highly confused.
Thanks.
Powered by blists - more mailing lists