public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Nicolas Dichtel <nicolas.dichtel@6wind.com>
Subject: What is the purpose of dev->gflags?
Date: Fri, 8 Apr 2022 21:30:45 +0300	[thread overview]
Message-ID: <20220408183045.wpyx7tqcgcimfudu@skbuf> (raw)

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.

             reply	other threads:[~2022-04-08 18:31 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-08 18:30 Vladimir Oltean [this message]
2022-04-08 18:50 ` What is the purpose of dev->gflags? Jakub Kicinski
2022-04-08 19:17   ` Vladimir Oltean
2022-04-11 15:26     ` Nicolas Dichtel
2022-04-11 15:33       ` Vladimir Oltean
2022-04-11 15:43         ` Nicolas Dichtel
2022-04-11 15:49           ` Vladimir Oltean
2022-04-11 16:10             ` Nicolas Dichtel
2022-04-11 16:20               ` Vladimir Oltean
2022-04-11 16:27                 ` Nicolas Dichtel
2022-04-11 16:50                   ` Vladimir Oltean
2022-04-12 15:57                     ` Nicolas Dichtel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220408183045.wpyx7tqcgcimfudu@skbuf \
    --to=olteanv@gmail.com \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.dichtel@6wind.com \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox