[PATCH V6 net-next 0/2] rtnetlink: Updates to rtnetlink_event()

[PATCH V6 net-next 0/2] rtnetlink: Updates to rtnetlink_event()

[Vladislav Yasevich]

First is the patch to add IFLA_EVENT attribute to the netlink message.  It
supports only currently white-listed events.
Like before, this is just an attribute that gets added to the rtnetlink
message only when the messaged was generated as a result of a netdev event.
In my case, this is necessary since I want to trap NETDEV_NOTIFY_PEERS
event (also possibly NETDEV_RESEND_IGMP event) and perform certain actions
in user space.  This is not possible since the messages generated as
a result of netdev events do not usually contain any changed data.  They
are just notifications.  This patch exposes this notification type to
userspace.

Second, I remove duplicate messages that a result of a change to bonding
options.  If netlink is used to configure bonding options, 2 messages
are generated, one as a result NETDEV_CHANGEINFODATA event triggered by
bonding code and one a result of device state changes triggered by
netdev_state_change (called from do_setlink).


V6: Updated names and refactored to make it less tied to netdev events.
    (From David Ahern)
V5: Rebased.  Added iproute2 patch to the series.
V4:
  * Removed the patch the removed NETDEV_CHANGENAME from event whitelist.
    It doesn't trigger duplicate messages since name changes can only be
    done while device is down and netdev_state_change() doesn't report
    changes while device is down.
  * Added a patch to clean-up duplicate messages on bonding option changes.

V3: Rebased.  Cleaned-up duplicate event.

V2: Added missed events (from David Ahern)


Vladislav Yasevich (2):
  rtnl: Add support for netdev event to link messages
  bonding: Prevent duplicate userspace notification

 drivers/net/bonding/bond_main.c    |  3 +-
 drivers/net/bonding/bond_options.c | 27 ++++++++++++++--
 include/linux/rtnetlink.h          |  3 +-
 include/net/bond_options.h         |  2 ++
 include/uapi/linux/if_link.h       | 11 +++++++
 net/core/dev.c                     |  2 +-
 net/core/rtnetlink.c               | 65 ++++++++++++++++++++++++++++++++------
 7 files changed, 99 insertions(+), 14 deletions(-)

-- 
2.7.4

[PATCH V6 net-next 1/2] rtnl: Add support for netdev event to link messages

[Vladislav Yasevich]

When netdev events happen, a rtnetlink_event() handler will send
messages for every event in it's white list.  These messages contain
current information about a particular device, but they do not include
the iformation about which event just happened.  So, it is impossible
to tell what just happend for these events.

This patch adds a new extension to RTM_NEWLINK message called IFLA_EVENT
that would have an encoding of event that triggered this
message.  This would allow the the message consumer to easily determine
if it needs to perform certain actions.

Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 include/linux/rtnetlink.h    |  3 +-
 include/uapi/linux/if_link.h | 11 ++++++++
 net/core/dev.c               |  2 +-
 net/core/rtnetlink.c         | 65 ++++++++++++++++++++++++++++++++++++++------
 4 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 57e5484..dea59c8 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -18,7 +18,8 @@ extern int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst,
 
 void rtmsg_ifinfo(int type, struct net_device *dev, unsigned change, gfp_t flags);
 struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
-				       unsigned change, gfp_t flags);
+				       unsigned change, u32 event,
+				       gfp_t flags);
 void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev,
 		       gfp_t flags);
 
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 15ac203..8ed679f 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -157,6 +157,7 @@ enum {
 	IFLA_GSO_MAX_SIZE,
 	IFLA_PAD,
 	IFLA_XDP,
+	IFLA_EVENT,
 	__IFLA_MAX
 };
 
@@ -911,4 +912,14 @@ enum {
 
 #define IFLA_XDP_MAX (__IFLA_XDP_MAX - 1)
 
+enum {
+	IFLA_EVENT_NONE,
+	IFLA_EVENT_REBOOT,		/* internal reset / reboot */
+	IFLA_EVENT_FEATURES,		/* change in offload features */
+	IFLA_EVENT_BONDING_FAILOVER,	/* change in active slave */
+	IFLA_EVENT_NOTIFY_PEERS,	/* re-sent grat. arp/ndisc */
+	IFLA_EVENT_IGMP_RESEND,		/* re-sent IGMP JOIN */
+	IFLA_EVENT_BONDING_OPTIONS,	/* change in bonding options */
+};
+
 #endif /* _UAPI_LINUX_IF_LINK_H */
diff --git a/net/core/dev.c b/net/core/dev.c
index 3d98fbf..06e0a74 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7084,7 +7084,7 @@ static void rollback_registered_many(struct list_head *head)
 
 		if (!dev->rtnl_link_ops ||
 		    dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
-			skb = rtmsg_ifinfo_build_skb(RTM_DELLINK, dev, ~0U,
+			skb = rtmsg_ifinfo_build_skb(RTM_DELLINK, dev, ~0U, 0,
 						     GFP_KERNEL);
 
 		/*
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index dab2834..07218eb 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -941,6 +941,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
 	       + nla_total_size(MAX_PHYS_ITEM_ID_LEN) /* IFLA_PHYS_SWITCH_ID */
 	       + nla_total_size(IFNAMSIZ) /* IFLA_PHYS_PORT_NAME */
 	       + rtnl_xdp_size() /* IFLA_XDP */
+	       + nla_total_size(4)  /* IFLA_EVENT */
 	       + nla_total_size(1); /* IFLA_PROTO_DOWN */
 
 }
@@ -1282,9 +1283,40 @@ static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
 	return err;
 }
 
+static u32 rtnl_get_event(unsigned long event)
+{
+	u32 rtnl_event_type = IFLA_EVENT_NONE;
+
+	switch (event) {
+	case NETDEV_REBOOT:
+		rtnl_event_type = IFLA_EVENT_REBOOT;
+		break;
+	case NETDEV_FEAT_CHANGE:
+		rtnl_event_type = IFLA_EVENT_FEATURES;
+		break;
+	case NETDEV_BONDING_FAILOVER:
+		rtnl_event_type = IFLA_EVENT_BONDING_FAILOVER;
+		break;
+	case NETDEV_NOTIFY_PEERS:
+		rtnl_event_type = IFLA_EVENT_NOTIFY_PEERS;
+		break;
+	case NETDEV_RESEND_IGMP:
+		rtnl_event_type = IFLA_EVENT_IGMP_RESEND;
+		break;
+	case NETDEV_CHANGEINFODATA:
+		rtnl_event_type = IFLA_EVENT_BONDING_OPTIONS;
+		break;
+	default:
+		break;
+	}
+
+	return rtnl_event_type;
+}
+
 static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			    int type, u32 pid, u32 seq, u32 change,
-			    unsigned int flags, u32 ext_filter_mask)
+			    unsigned int flags, u32 ext_filter_mask,
+			    u32 event)
 {
 	struct ifinfomsg *ifm;
 	struct nlmsghdr *nlh;
@@ -1333,6 +1365,11 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	    nla_put_u8(skb, IFLA_PROTO_DOWN, dev->proto_down))
 		goto nla_put_failure;
 
+	if (event != IFLA_EVENT_NONE) {
+		if (nla_put_u32(skb, IFLA_EVENT, event))
+			goto nla_put_failure;
+	}
+
 	if (rtnl_fill_link_ifmap(skb, dev))
 		goto nla_put_failure;
 
@@ -1467,6 +1504,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_LINK_NETNSID]	= { .type = NLA_S32 },
 	[IFLA_PROTO_DOWN]	= { .type = NLA_U8 },
 	[IFLA_XDP]		= { .type = NLA_NESTED },
+	[IFLA_EVENT]		= { .type = NLA_U32 },
 };
 
 static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
@@ -1626,7 +1664,7 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb)
 					       NETLINK_CB(cb->skb).portid,
 					       cb->nlh->nlmsg_seq, 0,
 					       flags,
-					       ext_filter_mask);
+					       ext_filter_mask, 0);
 
 			if (err < 0) {
 				if (likely(skb->len))
@@ -2736,7 +2774,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		return -ENOBUFS;
 
 	err = rtnl_fill_ifinfo(nskb, dev, RTM_NEWLINK, NETLINK_CB(skb).portid,
-			       nlh->nlmsg_seq, 0, 0, ext_filter_mask);
+			       nlh->nlmsg_seq, 0, 0, ext_filter_mask, 0);
 	if (err < 0) {
 		/* -EMSGSIZE implies BUG in if_nlmsg_size */
 		WARN_ON(err == -EMSGSIZE);
@@ -2808,7 +2846,8 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
 }
 
 struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
-				       unsigned int change, gfp_t flags)
+				       unsigned int change,
+				       u32 event, gfp_t flags)
 {
 	struct net *net = dev_net(dev);
 	struct sk_buff *skb;
@@ -2819,7 +2858,7 @@ struct sk_buff *rtmsg_ifinfo_build_skb(int type, struct net_device *dev,
 	if (skb == NULL)
 		goto errout;
 
-	err = rtnl_fill_ifinfo(skb, dev, type, 0, 0, change, 0, 0);
+	err = rtnl_fill_ifinfo(skb, dev, type, 0, 0, change, 0, 0, event);
 	if (err < 0) {
 		/* -EMSGSIZE implies BUG in if_nlmsg_size() */
 		WARN_ON(err == -EMSGSIZE);
@@ -2840,18 +2879,25 @@ void rtmsg_ifinfo_send(struct sk_buff *skb, struct net_device *dev, gfp_t flags)
 	rtnl_notify(skb, net, 0, RTNLGRP_LINK, NULL, flags);
 }
 
-void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
-		  gfp_t flags)
+static void rtmsg_ifinfo_event(int type, struct net_device *dev,
+			       unsigned int change, u32 event,
+			       gfp_t flags)
 {
 	struct sk_buff *skb;
 
 	if (dev->reg_state != NETREG_REGISTERED)
 		return;
 
-	skb = rtmsg_ifinfo_build_skb(type, dev, change, flags);
+	skb = rtmsg_ifinfo_build_skb(type, dev, change, event, flags);
 	if (skb)
 		rtmsg_ifinfo_send(skb, dev, flags);
 }
+
+void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
+		  gfp_t flags)
+{
+	rtmsg_ifinfo_event(type, dev, change, IFLA_EVENT_NONE, flags);
+}
 EXPORT_SYMBOL(rtmsg_ifinfo);
 
 static int nlmsg_populate_fdb_fill(struct sk_buff *skb,
@@ -4165,7 +4211,8 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
 	case NETDEV_NOTIFY_PEERS:
 	case NETDEV_RESEND_IGMP:
 	case NETDEV_CHANGEINFODATA:
-		rtmsg_ifinfo(RTM_NEWLINK, dev, 0, GFP_KERNEL);
+		rtmsg_ifinfo_event(RTM_NEWLINK, dev, 0, rtnl_get_event(event),
+				   GFP_KERNEL);
 		break;
 	default:
 		break;
-- 
2.7.4

[PATCH V6 net-next 2/2] bonding: Prevent duplicate userspace notification

[Vladislav Yasevich]

Whenever a user changes bonding options, a NETDEV_CHANGEINFODATA
notificatin is generated which results in a rtnelink message to
be sent.  While runnig 'ip monitor', we can actually see 2 messages,
one a result of the event, and the other a result of state change
that is generated bo netdev_state_change().  However, this is not
always the case. If bonding changes were done via sysfs or ifenslave
(old ioctl interface), then only 1 message is seen.

This patch removes duplicate messages in the case of using netlink
to configure bonding.  It introduceds a separte function that
triggers a netdev event and uses that function in the syfs and ioctl
cases.

This was discovered while auditing all the different envents and
continues the effort of cleaning up duplicated netlink messages.

CC: David Ahern <dsa@cumulusnetworks.com>
CC: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 drivers/net/bonding/bond_main.c    |  3 ++-
 drivers/net/bonding/bond_options.c | 27 +++++++++++++++++++++++++--
 include/net/bond_options.h         |  2 ++
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7331331..d7aa137 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3481,7 +3481,8 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
 	case BOND_CHANGE_ACTIVE_OLD:
 	case SIOCBONDCHANGEACTIVE:
 		bond_opt_initstr(&newval, slave_dev->name);
-		res = __bond_opt_set(bond, BOND_OPT_ACTIVE_SLAVE, &newval);
+		res = __bond_opt_set_notify(bond, BOND_OPT_ACTIVE_SLAVE,
+					    &newval);
 		break;
 	default:
 		res = -EOPNOTSUPP;
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 1bcbb89..8ca6833 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -673,7 +673,30 @@ int __bond_opt_set(struct bonding *bond,
 out:
 	if (ret)
 		bond_opt_error_interpret(bond, opt, ret, val);
-	else if (bond->dev->reg_state == NETREG_REGISTERED)
+
+	return ret;
+}
+/**
+ * __bond_opt_set_notify - set a bonding option
+ * @bond: target bond device
+ * @option: option to set
+ * @val: value to set it to
+ *
+ * This function is used to change the bond's option value and trigger
+ * a notification to user sapce. It can be used for both enabling/changing
+ * an option and for disabling it. RTNL lock must be obtained before calling
+ * this function.
+ */
+int __bond_opt_set_notify(struct bonding *bond,
+			  unsigned int option, struct bond_opt_value *val)
+{
+	int ret = -ENOENT;
+
+	ASSERT_RTNL();
+
+	ret = __bond_opt_set(bond, option, val);
+
+	if (!ret && (bond->dev->reg_state == NETREG_REGISTERED))
 		call_netdevice_notifiers(NETDEV_CHANGEINFODATA, bond->dev);
 
 	return ret;
@@ -696,7 +719,7 @@ int bond_opt_tryset_rtnl(struct bonding *bond, unsigned int option, char *buf)
 	if (!rtnl_trylock())
 		return restart_syscall();
 	bond_opt_initstr(&optval, buf);
-	ret = __bond_opt_set(bond, option, &optval);
+	ret = __bond_opt_set_notify(bond, option, &optval);
 	rtnl_unlock();
 
 	return ret;
diff --git a/include/net/bond_options.h b/include/net/bond_options.h
index 1797235..d79d28f 100644
--- a/include/net/bond_options.h
+++ b/include/net/bond_options.h
@@ -104,6 +104,8 @@ struct bond_option {
 
 int __bond_opt_set(struct bonding *bond, unsigned int option,
 		   struct bond_opt_value *val);
+int __bond_opt_set_notify(struct bonding *bond, unsigned int option,
+			  struct bond_opt_value *val);
 int bond_opt_tryset_rtnl(struct bonding *bond, unsigned int option, char *buf);
 
 const struct bond_opt_value *bond_opt_parse(const struct bond_option *opt,
-- 
2.7.4

[PATCH V6 net-next 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 | 11 +++++++++++
 ip/ipaddress.c          | 21 +++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index 5a3a048..c0a6769 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
 };
 
@@ -909,4 +910,14 @@ enum {
 
 #define IFLA_XDP_MAX (__IFLA_XDP_MAX - 1)
 
+enum {
+	IFLA_EVENT_NONE,
+	IFLA_EVENT_REBOOT,		/* internal reset / reboot */
+	IFLA_EVENT_FEATURES,		/* change in offload features */
+	IFLA_EVENT_BONDING_FAILOVER,	/* hange in active slave */
+	IFLA_EVENT_NOTIFY_PEERS,	/* re-sent grat. arp/ndisc */
+	IFLA_EVENT_IGMP_RESEND,		/* re-sent IGMP JOIN */
+	IFLA_EVENT_BONDING_OPTIONS,	/* change in bonding options */
+};
+
 #endif /* _LINUX_IF_LINK_H */
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index b8d9c7d..c6e7413 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -753,6 +753,24 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
 	return 0;
 }
 
+static const char *netdev_events[] = {"NONE",
+				      "REBOOT",
+				      "FEATURE CHANGE",
+				      "BONDING FAILOVER",
+				      "NOTIFY PEERS",
+				      "RESEND IGMP",
+				      "BONDING OPTION"};
+
+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 +876,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 V6 net-next 1/2] rtnl: Add support for netdev event to link messages

[David Ahern]

On 5/27/17 8:14 AM, Vladislav Yasevich wrote:
> When netdev events happen, a rtnetlink_event() handler will send
> messages for every event in it's white list.  These messages contain
> current information about a particular device, but they do not include
> the iformation about which event just happened.  So, it is impossible
> to tell what just happend for these events.
> 
> This patch adds a new extension to RTM_NEWLINK message called IFLA_EVENT
> that would have an encoding of event that triggered this
> message.  This would allow the the message consumer to easily determine
> if it needs to perform certain actions.
> 
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
>  include/linux/rtnetlink.h    |  3 +-
>  include/uapi/linux/if_link.h | 11 ++++++++
>  net/core/dev.c               |  2 +-
>  net/core/rtnetlink.c         | 65 ++++++++++++++++++++++++++++++++++++++------
>  4 files changed, 70 insertions(+), 11 deletions(-)

Acked-by: David Ahern <dsahern@gmail.com>

Re: [PATCH V6 net-next 2/2] bonding: Prevent duplicate userspace notification

[David Ahern]

On 5/27/17 8:14 AM, Vladislav Yasevich wrote:
> Whenever a user changes bonding options, a NETDEV_CHANGEINFODATA
> notificatin is generated which results in a rtnelink message to
> be sent.  While runnig 'ip monitor', we can actually see 2 messages,
> one a result of the event, and the other a result of state change
> that is generated bo netdev_state_change().  However, this is not
> always the case. If bonding changes were done via sysfs or ifenslave
> (old ioctl interface), then only 1 message is seen.
> 
> This patch removes duplicate messages in the case of using netlink
> to configure bonding.  It introduceds a separte function that
> triggers a netdev event and uses that function in the syfs and ioctl
> cases.
> 
> This was discovered while auditing all the different envents and
> continues the effort of cleaning up duplicated netlink messages.
> 
> CC: David Ahern <dsa@cumulusnetworks.com>
> CC: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
>  drivers/net/bonding/bond_main.c    |  3 ++-
>  drivers/net/bonding/bond_options.c | 27 +++++++++++++++++++++++++--
>  include/net/bond_options.h         |  2 ++
>  3 files changed, 29 insertions(+), 3 deletions(-)
> 

Acked-by: David Ahern <dsahern@gmail.com>

Re: [PATCH V6 net-next 0/2] rtnetlink: Updates to rtnetlink_event()

[David Miller]

From: Vladislav Yasevich <vyasevich@gmail.com>
Date: Sat, 27 May 2017 10:14:33 -0400

> First is the patch to add IFLA_EVENT attribute to the netlink message.  It
> supports only currently white-listed events.
> Like before, this is just an attribute that gets added to the rtnetlink
> message only when the messaged was generated as a result of a netdev event.
> In my case, this is necessary since I want to trap NETDEV_NOTIFY_PEERS
> event (also possibly NETDEV_RESEND_IGMP event) and perform certain actions
> in user space.  This is not possible since the messages generated as
> a result of netdev events do not usually contain any changed data.  They
> are just notifications.  This patch exposes this notification type to
> userspace.
> 
> Second, I remove duplicate messages that a result of a change to bonding
> options.  If netlink is used to configure bonding options, 2 messages
> are generated, one as a result NETDEV_CHANGEINFODATA event triggered by
> bonding code and one a result of device state changes triggered by
> netdev_state_change (called from do_setlink).
 ...

Series applied, thanks Vlad.

Re: [PATCH V6 net-next iproute] ip: Add support for netdev events to monitor

[David Ahern]

On 5/27/17 8:14 AM, Vladislav Yasevich wrote:
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index b8d9c7d..c6e7413 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -753,6 +753,24 @@ int print_linkinfo_brief(const struct sockaddr_nl *who,
>  	return 0;
>  }
>  
> +static const char *netdev_events[] = {"NONE",

I would prefer ifla_events rather than netdev just to reinforce the
separation that an IFLA_EVENT does not necessarily correspond to a
NETDEV event.

Re: [PATCH V6 net-next iproute] ip: Add support for netdev events to monitor

[Stephen Hemminger]

On Sat, 27 May 2017 10:14:36 -0400
Vladislav Yasevich <vyasevich@gmail.com> wrote:

>  
> +static const char *netdev_events[] = {"NONE",
> +				      "REBOOT",
> +				      "FEATURE CHANGE",
> +				      "BONDING FAILOVER",
> +				      "NOTIFY PEERS",
> +				      "RESEND IGMP",
> +				      "BONDING OPTION"};

Overall this looks fine, I will pickup the if_link.h from net-next.

One stylistic change.

Please add simple line break, and initialize by value:

static const char *netdev_events[] = {
	[IFLA_EVENT_NONE]	= "NONE",
...

Do you want some prefix or bounding around the event output?
Also a little concerned that the output format change may break some program
could the new output be at the end of the line?

Re: [PATCH V6 net-next iproute] ip: Add support for netdev events to monitor

[Vlad Yasevich]

On 05/30/2017 01:12 PM, Stephen Hemminger wrote:
> On Sat, 27 May 2017 10:14:36 -0400
> Vladislav Yasevich <vyasevich@gmail.com> wrote:
> 
>>  
>> +static const char *netdev_events[] = {"NONE",
>> +				      "REBOOT",
>> +				      "FEATURE CHANGE",
>> +				      "BONDING FAILOVER",
>> +				      "NOTIFY PEERS",
>> +				      "RESEND IGMP",
>> +				      "BONDING OPTION"};
> 
> Overall this looks fine, I will pickup the if_link.h from net-next.
> 
> One stylistic change.
> 
> Please add simple line break, and initialize by value:
> 
> static const char *netdev_events[] = {
> 	[IFLA_EVENT_NONE]	= "NONE",
> ...
> 
> Do you want some prefix or bounding around the event output?

Don't really care about output from my side.  If you think some prefix
would be good, I can surely add it.

> Also a little concerned that the output format change may break some program
> could the new output be at the end of the line?
> 

I can try moving it to the end.

Thanks
-vlad

Re: [PATCH V6 net-next iproute] ip: Add support for netdev events to monitor

[Vlad Yasevich]

On 05/30/2017 02:26 PM, Vlad Yasevich wrote:
> On 05/30/2017 01:12 PM, Stephen Hemminger wrote:
>> On Sat, 27 May 2017 10:14:36 -0400
>> Vladislav Yasevich <vyasevich@gmail.com> wrote:
>>
>>>  
>>> +static const char *netdev_events[] = {"NONE",
>>> +				      "REBOOT",
>>> +				      "FEATURE CHANGE",
>>> +				      "BONDING FAILOVER",
>>> +				      "NOTIFY PEERS",
>>> +				      "RESEND IGMP",
>>> +				      "BONDING OPTION"};
>>
>> Overall this looks fine, I will pickup the if_link.h from net-next.
>>
>> One stylistic change.
>>
>> Please add simple line break, and initialize by value:
>>
>> static const char *netdev_events[] = {
>> 	[IFLA_EVENT_NONE]	= "NONE",
>> ...
>>
>> Do you want some prefix or bounding around the event output?
> 
> Don't really care about output from my side.  If you think some prefix
> would be good, I can surely add it.
> 
>> Also a little concerned that the output format change may break some program
>> could the new output be at the end of the line?
>>
> 
> I can try moving it to the end.
>

Hi Stephen

So, I looked again at this patch and the output change I am proposing would look
something like this 'ip monitor':

1: lo: <LOOPBACK,UP,LOWER_UP> mtu 6500 qdisc noqueue state UNKNOWN group default event
FEATURE CHANGE
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00


It's already at the end of the like ant has an 'event' prefix similar to all the other
entries on the top line.  Does that look OK?  I don't think that would break anything.

Thanks
-vlad