From: Vlad Yasevich <vyasevic@redhat.com>
To: nicolas.dichtel@6wind.com, David Ahern <dsahern@gmail.com>,
Xin Long <lucien.xin@gmail.com>,
network dev <netdev@vger.kernel.org>
Cc: davem@davemloft.net, hannes@stressinduktion.org
Subject: Re: [PATCH net 5/6] rtnetlink: check DO_SETLINK_NOTIFY correctly in do_setlink
Date: Thu, 26 Oct 2017 08:52:48 +0200 [thread overview]
Message-ID: <0e37130c-bbf3-c75e-5cee-c3a08206873f@redhat.com> (raw)
In-Reply-To: <a783ef9d-e66c-b906-fcfd-4da5d560af9d@6wind.com>
On 10/16/2017 02:20 PM, Nicolas Dichtel wrote:
> Le 16/10/2017 à 03:17, David Ahern a écrit :
>> [ cc'ed Nicolas ]
>>
>> On 10/15/17 4:13 AM, Xin Long wrote:
>>> The check 'status & DO_SETLINK_NOTIFY' in do_setlink doesn't really
>>> work after status & DO_SETLINK_MODIFIED, as:
>>>
>>> DO_SETLINK_MODIFIED 0x1
>>> DO_SETLINK_NOTIFY 0x3
>>>
>>> Considering that notifications are suppposed to be sent only when
>>> status have the flag DO_SETLINK_NOTIFY, the right check would be:
>>>
>>> (status & DO_SETLINK_NOTIFY) == DO_SETLINK_NOTIFY
>>>
>>> This would avoid lots of duplicated notifications when setting some
>>> properties of a link.
>>>
>>> Fixes: ba9989069f4e ("rtnl/do_setlink(): notify when a netdev is modified")
>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> Good catch, thank you.
>
> Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>
So I found this the first timer around when looking at this code, but was told that
notification are expected anytime we modified any setting thus the code was simply
checking for MODIFIED bit. Has that thinking changed?
-vlad
next prev parent reply other threads:[~2017-10-26 6:52 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-15 10:13 [PATCH net 0/6] rtnetlink: a bunch of fixes for userspace notifications in changing dev properties Xin Long
2017-10-15 10:13 ` [PATCH net 1/6] rtnetlink: bring NETDEV_CHANGEMTU event process back in rtnetlink_event Xin Long
2017-10-15 10:13 ` [PATCH net 2/6] rtnetlink: bring NETDEV_CHANGE_TX_QUEUE_LEN " Xin Long
2017-10-15 10:13 ` [PATCH net 3/6] rtnetlink: bring NETDEV_POST_TYPE_CHANGE " Xin Long
2017-10-15 10:13 ` [PATCH net 4/6] rtnetlink: bring NETDEV_CHANGEUPPER " Xin Long
2017-10-15 10:13 ` [PATCH net 5/6] rtnetlink: check DO_SETLINK_NOTIFY correctly in do_setlink Xin Long
2017-10-15 10:13 ` [PATCH net 6/6] rtnetlink: do not set notification for tx_queue_len " Xin Long
2017-10-16 1:17 ` David Ahern
2017-10-16 1:17 ` [PATCH net 5/6] rtnetlink: check DO_SETLINK_NOTIFY correctly " David Ahern
2017-10-16 12:20 ` Nicolas Dichtel
2017-10-26 6:52 ` Vlad Yasevich [this message]
2017-10-26 7:41 ` Nicolas Dichtel
2017-10-26 8:21 ` [PATCH net] Revert "rtnetlink: check DO_SETLINK_NOTIFY correctly in do_setlink" Nicolas Dichtel
2017-10-26 9:10 ` Xin Long
2017-10-26 10:18 ` Nicolas Dichtel
2017-10-26 15:22 ` David Ahern
2017-10-27 13:27 ` David Miller
2017-10-26 14:59 ` [PATCH net 5/6] rtnetlink: check DO_SETLINK_NOTIFY correctly in do_setlink David Ahern
2017-10-16 1:16 ` [PATCH net 4/6] rtnetlink: bring NETDEV_CHANGEUPPER event process back in rtnetlink_event David Ahern
2017-10-16 1:16 ` [PATCH net 3/6] rtnetlink: bring NETDEV_POST_TYPE_CHANGE " David Ahern
2017-10-16 1:16 ` [PATCH net 2/6] rtnetlink: bring NETDEV_CHANGE_TX_QUEUE_LEN " David Ahern
2017-10-16 19:49 ` [PATCH net 0/6] rtnetlink: a bunch of fixes for userspace notifications in changing dev properties David Miller
2017-10-24 20:44 ` Stefano Brivio
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=0e37130c-bbf3-c75e-5cee-c3a08206873f@redhat.com \
--to=vyasevic@redhat.com \
--cc=davem@davemloft.net \
--cc=dsahern@gmail.com \
--cc=hannes@stressinduktion.org \
--cc=lucien.xin@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=nicolas.dichtel@6wind.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).