On 03/30/2017 09:39 AM, Vlad Yasevich wrote: > On 03/29/2017 03:11 PM, David Ahern wrote: >> On 3/29/17 11:05 AM, Vlad Yasevich wrote: >>> On 03/29/2017 12:37 PM, Roopa Prabhu wrote: >>>> On 3/29/17, 5:23 AM, Vlad Yasevich wrote: >>>>> [ resending to list. hit the wrong reply button last time ] >>>>> >>>>> On 03/27/2017 06:58 PM, David Miller wrote: >>>>>> From: Vladislav Yasevich <vyasev...@gmail.com> >>>>>> Date: Sat, 25 Mar 2017 21:59:47 -0400 >>>>>> >>>>>>> RTNL currently generates notifications on some netdev notifier events. >>>>>>> However, user space has no idea what changed. All it sees is the >>>>>>> data and has to infer what has changed. For some events that is not >>>>>>> possible. >>>>>>> >>>>>>> This patch adds a new field to RTM_NEWLINK message called IFLA_EVENT >>>>>>> that would have an encoding of the which event triggered this >>>>>>> notification. Currectly, only 2 events (NETDEV_NOTIFY_PEERS and >>>>>>> NETDEV_MTUCHANGED) are supported. These events could be interesting >>>>>>> in the virt space to trigger additional configuration commands to VMs. >>>>>>> Other events of interest may be added later. >>>>>>> >>>>>>> Signed-off-by: Vladislav Yasevich <vyase...@redhat.com> >>>>>> At what point do we start providing the metadata for the changed >>>>>> values as well? You'd probably need to provide both the old and >>>>>> new values to cover all cases. >>>>> I don't think if that would be possible because of when events are >>>>> triggered. >>>>> We send these notifications after all the changes have already been made, >>>>> so >>>>> it might be tough to carry old data. >>>>> >>>>> Looking at just the two events I am supporting in this patch, we could >>>>> actually >>>>> supply the old mtu data through a NETDEV_PRECHANGEMTU event, if it is >>>>> necessary. >>>> >>>> But, NETDEV_PRECHANGEMTU will be a unnecessary notification to userspace >>>> without >>>> changes. There are already enough notifications generated for links (I >>>> know you are not >>>> suggesting adding it here) >>> >>> Actually, this one already triggers a link notification to userspace. It >>> just has >>> no event data in it to tell you that. :) >> >> Is it intentional or unintentional? perhaps rtnetlink_event should be a >> whitelist -- events that userspace should be notified about. Seems like >> NETDEV_ events have been added without rtnetlink_event getting updated. > > I think a 'whitelist' was attempted, but as you mentioned, it hasn't been > updated... > I'll defer the definitive answer to someone else. It seems Patrick added a > comment > in commit a2835763 to update the white list and it's been a few times. >
This is actually an interesting point. Looking at some commits that have added events to black list in rtnetlink-event, it might have been much easier to debug those issues if we had the 'event' encoding in the netlink message. I think it might be worthwhile to add all allowed event types to this new encoding so we can userspace can see just what's its getting. -vlad >> For example, does userspace care about NETDEV_UDP_TUNNEL_PUSH_INFO or >> NETDEV_CHANGE_TX_QUEUE_LEN? >> > > Probably not the first, but possibly the second. If txquelen is changed on a > device, > some apps might want to know about it. >
