[PATCH v3 iproute] ip: Add support for netdev events to monitor

[PATCH v3 iproute] ip: Add support for netdev events to monitor

[Vladislav Yasevich]

Add IFLA_EVENT handling so that event types can be viewed with
'monitor' command.  This gives a little more information for why
a given message was receivied.

Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 include/linux/if_link.h | 21 +++++++++++++++++++++
 ip/ipaddress.c          | 31 +++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index b0bdbd6..b6d211a 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -157,6 +157,7 @@ enum {
 	IFLA_GSO_MAX_SIZE,
 	IFLA_PAD,
 	IFLA_XDP,
+	IFLA_EVENT,
 	__IFLA_MAX
 };
 
@@ -890,4 +891,24 @@ enum {
 
 #define IFLA_XDP_MAX (__IFLA_XDP_MAX - 1)
 
+enum {
+	IFLA_EVENT_UNSPEC,
+	IFLA_EVENT_REBOOT,
+	IFLA_EVENT_CHANGE_MTU,
+	IFLA_EVENT_CHANGE_ADDR,
+	IFLA_EVENT_CHANGE_NAME,
+	IFLA_EVENT_FEAT_CHANGE,
+	IFLA_EVENT_BONDING_FAILOVER,
+	IFLA_EVENT_POST_TYPE_CHANGE,
+	IFLA_EVENT_NOTIFY_PEERS,
+	IFLA_EVENT_CHANGE_UPPER,
+	IFLA_EVENT_RESEND_IGMP,
+	IFLA_EVENT_PRE_CHANGE_MTU,
+	IFLA_EVENT_CHANGE_INFO_DATA,
+	IFLA_EVENT_PRE_CHANGE_UPPER,
+	IFLA_EVENT_CHANGE_LOWER_STATE,
+	IFLA_EVENT_UDP_TUNNEL_PUSH_INFO,
+	IFLA_EVENT_CHANGE_TX_QUEUE_LEN,
+};
+
 #endif /* _LINUX_IF_LINK_H */
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index b8d9c7d..ffcfa5e 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -753,6 +753,34 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
 	return 0;
 }
 
+static const char *netdev_events[] = {"UNKNOWN",
+				      "REBOOT",
+				      "CHANGE_MTU",
+				      "CHANGE_ADDR",
+				      "CHANGE_NAME",
+				      "FEATURE_CHANGE",
+				      "BONDING_FAILOVER",
+				      "POST_TYPE_CHANGE",
+				      "NOTIFY_PEERS",
+				      "CHANGE_UPPER",
+				      "RESEND_IGMP",
+				      "PRE_CHANGE_MTU",
+				      "CHANGE_INFO_DATA",
+				      "PRE_CHANGE_UPPER",
+				      "CHANGE_LOWER_STATE",
+				      "UDP_TUNNEL_PUSH_INFO",
+				      "CHANGE_TXQUEUE_LEN"};
+
+static void print_dev_event(FILE *f, __u32 event)
+{
+	if (event >= ARRAY_SIZE(netdev_events))
+		fprintf(f, "event %d ", event);
+	else {
+		if (event)
+			fprintf(f, "event %s ", netdev_events[event]);
+	}
+}
+
 int print_linkinfo(const struct sockaddr_nl *who,
 		   struct nlmsghdr *n, void *arg)
 {
@@ -858,6 +886,9 @@ int print_linkinfo(const struct sockaddr_nl *who,
 	if (filter.showqueue)
 		print_queuelen(fp, tb);
 
+	if (tb[IFLA_EVENT])
+		print_dev_event(fp, rta_getattr_u32(tb[IFLA_EVENT]));
+
 	if (!filter.family || filter.family == AF_PACKET || show_details) {
 		SPRINT_BUF(b1);
 		fprintf(fp, "%s", _SL_);
-- 
2.7.4

Re: [PATCH v3 iproute] ip: Add support for netdev events to monitor

[David Ahern]

On 4/4/17 9:25 AM, Vladislav Yasevich wrote:
> Add IFLA_EVENT handling so that event types can be viewed with
> 'monitor' command.  This gives a little more information for why
> a given message was receivied.
> 
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
>  include/linux/if_link.h | 21 +++++++++++++++++++++
>  ip/ipaddress.c          | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+)

per comments on the email thread about reducing notifications, the
kernel patch for this should be reverted (and hence this iproute2 patch
is not needed) in favor of using a bitmask. Right now there are too many
redundant notifications to userspace.

Vlad: can you look at a bitmask version of IFLA_EVENT that shows what
changed in the newlink message from do_setlink()?

Re: [PATCH v3 iproute] ip: Add support for netdev events to monitor

[David Miller]

From: David Ahern <dsa@cumulusnetworks.com>
Date: Sat, 8 Apr 2017 18:24:06 -0400

> per comments on the email thread about reducing notifications, the
> kernel patch for this should be reverted (and hence this iproute2 patch
> is not needed) in favor of using a bitmask. Right now there are too many
> redundant notifications to userspace.

I must have missed something in all the discussion, which patch needs
to be reverted from my tree exactly?

Re: [PATCH v3 iproute] ip: Add support for netdev events to monitor

[David Ahern]

On 4/8/17 10:33 PM, David Miller wrote:
> From: David Ahern <dsa@cumulusnetworks.com>
> Date: Sat, 8 Apr 2017 18:24:06 -0400
> 
>> per comments on the email thread about reducing notifications, the
>> kernel patch for this should be reverted (and hence this iproute2 patch
>> is not needed) in favor of using a bitmask. Right now there are too many
>> redundant notifications to userspace.
> 
> I must have missed something in all the discussion, which patch needs
> to be reverted from my tree exactly?
> 

Here's the thread:
https://www.spinics.net/lists/netdev/msg429154.html

The comment is that def12888c161 is adding a uapi that leads to way too
many notifications (e.g., on a setlink).

It would be more efficient (read less notifications) to have do_setlink
emit a single message with the IFLA_EVENT (or something else
appropriately named) that indicates what attributes changed. Right now,
a change MTU leads to 3 notifications causing unnecessary churn in
userspace to track what the state of the link is.

Re: [PATCH v3 iproute] ip: Add support for netdev events to monitor

[David Miller]

From: David Ahern <dsa@cumulusnetworks.com>
Date: Sat, 8 Apr 2017 22:54:07 -0400

> On 4/8/17 10:33 PM, David Miller wrote:
>> From: David Ahern <dsa@cumulusnetworks.com>
>> Date: Sat, 8 Apr 2017 18:24:06 -0400
>> 
>>> per comments on the email thread about reducing notifications, the
>>> kernel patch for this should be reverted (and hence this iproute2 patch
>>> is not needed) in favor of using a bitmask. Right now there are too many
>>> redundant notifications to userspace.
>> 
>> I must have missed something in all the discussion, which patch needs
>> to be reverted from my tree exactly?
>> 
> 
> Here's the thread:
> https://www.spinics.net/lists/netdev/msg429154.html
> 
> The comment is that def12888c161 is adding a uapi that leads to way too
> many notifications (e.g., on a setlink).
> 
> It would be more efficient (read less notifications) to have do_setlink
> emit a single message with the IFLA_EVENT (or something else
> appropriately named) that indicates what attributes changed. Right now,
> a change MTU leads to 3 notifications causing unnecessary churn in
> userspace to track what the state of the link is.

Ok, I'll queue up the revert.  I guess this means your rtnetlink
patch set is going to need changes or a respin, therefore.