From: Vlad Yasevich <vyasevic@redhat.com>
To: Jiri Pirko <jiri@resnulli.us>, David Ahern <dsa@cumulusnetworks.com>
Cc: Vladislav Yasevich <vyasevich@gmail.com>,
netdev@vger.kernel.org, roopa <roopa@cumulusnetworks.com>
Subject: Re: [PATCH net-next V3 2/2] rtnl: Add support for netdev event attribute to link messages
Date: Mon, 1 May 2017 09:35:21 -0400 [thread overview]
Message-ID: <00ddfbb9-9cf7-026d-562b-1b7a4a7129d3@redhat.com> (raw)
In-Reply-To: <20170428163804.GH1886@nanopsycho.orion>
On 04/28/2017 12:38 PM, Jiri Pirko wrote:
> Thu, Apr 27, 2017 at 09:59:38PM CEST, dsa@cumulusnetworks.com wrote:
>> On 4/27/17 1:43 PM, Vlad Yasevich wrote:
>>>> For example, NETDEV_CHANGEINFODATA is only for bonds though nothing
>>>> about the name suggests it is a bonding notification. This one was added
>>>> specifically to notify userspace (d4261e5650004), yet seems to happen
>>>> only during a changelink and that already generates a RTM_NEWLINK
>>>> message via do_setlink. Since the rtnetlink_event message does not
>>>> contain anything "NETDEV_CHANGEINFODATA" related what purpose does it
>>>> really serve besides duplicating netlink messages to userspace.
>>>>
>>>
>>> I am not sure about this one, but if you have an app trying to monitor
>>> for this event, it can't really since there is no info in the netlink message.
>>
>> I cc'ed Jiri on this thread hoping he would explain the intent.
>>
>> I propose it gets removed.
>
> Hmm, I don't really recall. But looking at it now, I agree it is
> redundant.
>
So, it looks like the notifier might be there to account for the ioctl/sysfs
interfaces.
Additionally, the message is not generated from do_setlink() if the devices is
down, so notifier accounts for that as well.
I guess, basic question is whether data carried in NETDEV_CHANGEINFODATA is useful
to user space? If yes (I can possibly see some managements apps that might be interested
in it), then we shouldn't just remove it. Possible solutions to eliminate duplicates
would be to move the notifier call into non-rtnl code paths...
Also, may be netdev_state_change() should call rtmsg_ifinfo() unconditionally?
-vlad
next prev parent reply other threads:[~2017-05-01 13:35 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-21 17:31 [PATCH v3 net-next 0/2] rtnetlink: Updates to rtnetlink_event() Vladislav Yasevich
2017-04-21 17:31 ` [PATCH net-next 1/2] rtnetlink: Disable notification for NETDEV_NAMECHANGE event Vladislav Yasevich
2017-04-21 18:08 ` David Ahern
2017-04-27 19:26 ` Vlad Yasevich
2017-04-21 17:31 ` [PATCH net-next V3 2/2] rtnl: Add support for netdev event attribute to link messages Vladislav Yasevich
2017-04-24 1:07 ` David Ahern
2017-04-24 15:14 ` Roopa Prabhu
2017-04-27 19:51 ` Vlad Yasevich
2017-04-28 0:11 ` Roopa Prabhu
2017-04-27 19:43 ` Vlad Yasevich
2017-04-27 19:59 ` David Ahern
2017-04-28 16:38 ` Jiri Pirko
2017-05-01 13:35 ` Vlad Yasevich [this message]
2017-05-02 21:57 ` David Ahern
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=00ddfbb9-9cf7-026d-562b-1b7a4a7129d3@redhat.com \
--to=vyasevic@redhat.com \
--cc=dsa@cumulusnetworks.com \
--cc=jiri@resnulli.us \
--cc=netdev@vger.kernel.org \
--cc=roopa@cumulusnetworks.com \
--cc=vyasevich@gmail.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;
as well as URLs for NNTP newsgroup(s).