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: <34331f55-e3ef-4cfa-f911-4201d5a2014e@redhat.com>
Date:   Fri, 14 Apr 2017 21:51:58 -0400
From:   Vlad Yasevich <vyasevic@...hat.com>
To:     David Ahern <dsa@...ulusnetworks.com>,
        Roopa Prabhu <roopa@...ulusnetworks.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for
 MTU events

On 04/10/2017 11:49 AM, David Ahern wrote:
> On 4/10/17 9:39 AM, Vlad Yasevich wrote:
>> OK, so this will work for the events that are generated as a result of device state change
>> (like mtu, address, and others).
>>
>> However, the original event data may be needed for other events that may be
>> of use to userspace like NETDEV_NOTIFY_PEERS and NETDEV_RESEND_IGMP (possibly others...)
> 
> sure. My objection is to multiple messages with identical content.
> 
> I think the rtnetlink_event message is unique for those 2 netdev events,
> so no objections if it has value.
> 

So, I've been looking at adding a bitmap and collecting all modification, however
I ran into an interesting issue in do_setlink.

Currently the notifications from do_setlink() don't appear to work as one would
expect and it's somewhat confusing upon deeper inspection.

We have 2 values DO_SETLINK_MODIFIED (1) and DO_SETLINK_NOTIFY (3).  These 2 'attempt'
to do different jobs, but really fail at it.  The function will generate notifications
regardless of which of the above values is used.

Those changes were done in commit ba9989069f4e426b1e0ed7018eacc9e1ba607095 (cc Nicolas
just in case he remembers the history)

I am not sure which changes should really trigger a call netdev_state_change(), thus this
message.  Right now, all changes done in this function trigger them.  If that's how that
should function, that I can simplify the code.  If not, then some of the changes may
require us to export the event to the user.

To use the dreaded NETDEV_CHANGEMTU event as an example, we used to generate 3 messages
(PRECHANGEMTU, CHANGEMTU, and a message from netdev_state_change).  With recent changes,
we now only generate a message from netdev_state_change.  However, mtu change is tagged
with DO_SETLINK_MODIFIED which doesn't include the notify bit.  So, should there be a
NETDEV_CHANGE event associated with this change and a rtnl message (as it is now) or not?
It's unclear.

-vlad