netdev - Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <58E92966.5000506@cumulusnetworks.com>
Date:   Sat, 08 Apr 2017 11:18:14 -0700
From:   Roopa Prabhu <roopa@...ulusnetworks.com>
To:     David Ahern <dsa@...ulusnetworks.com>
CC:     netdev@...r.kernel.org, vyasevic@...hat.com, davem@...emloft.net
Subject: Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications
 for MTU events

On 4/8/17, 11:13 AM, David Ahern wrote:
> On 4/8/17 2:06 PM, Roopa Prabhu wrote:
>> On 4/7/17, 2:25 PM, David Ahern wrote:
>>> Changing MTU on a link currently causes 3 messages to be sent to userspace:
>>>
>>> [LINK]11: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1490 qdisc noqueue state UNKNOWN group default event PRE_CHANGE_MTU
>>>     link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>>>
>>> [LINK]11: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default event CHANGE_MTU
>>>     link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>>>
>>> [LINK]11: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
>>>     link/ether f2:52:5c:6d:21:f3 brd ff:ff:ff:ff:ff:ff
>>>
>>> Remove the PRE_CHANGE_MTU and CHANGE_MTU messages.
>>>
>>>
>> This change is good... multiple notifications for the same event does not help in large scale links setups. However, this
>> reverts what vlad was trying to do with his patchset. Vlad's patch-set relies on the rtnl notifications generated from
>> notifiers (rtnetlink_event) to add  specific event (IFLA_EVENT) in notifications.
>>
>> The third notification in your example above is the correct one and is an aggregate notification for a set of changes, but
>> it cannot really fill in all types of events in the single IFLA_EVENT attribute as it stands today.  IFLA_EVENT should be
>> a bitmask to include all events in this case (i had indicated this in vlads first version).
>>
> Agreed. I think it would be best to revert def12888c161 before the UAPI
> goes out.
>
> The change can instead add the IFLA_EVENT as a bitmask mentioned here to
> note the changes in a setlink. On top of that, remove the notifications
> for the events I mentioned in this set to reduce the overhead on userspace.

ack