netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/8] rtnetlink: Cleanup user notifications for netdev events
@ 2017-04-07 21:25 David Ahern
  2017-04-07 21:25 ` [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events David Ahern
                   ` (7 more replies)
  0 siblings, 8 replies; 20+ messages in thread
From: David Ahern @ 2017-04-07 21:25 UTC (permalink / raw)
  To: netdev; +Cc: vyasevic, davem, David Ahern

Vlad's recent patch to add the event type to rtnetlink notifications
points out a number of redundant or unnecessary notifications sent to
userspace for events that are essentially internal to the kernel. Trim
the list before the new IFLA attributes become part of the UAPI.

David Ahern (8):
  rtnetlink: Do not generate notifications for MTU events
  rtnetlink: Do not generate notification for UDP_TUNNEL_PUSH_INFO
  rtnetlink: Do not generate notifications for CHANGEADDR event
  rtnetlink: Do not generate notifications for POST_TYPE_CHANGE event
  rtnetlink: Remove NETDEV_CHANGEINFODATA netdev event
  rtnetlink: Do not generate notifications for PRECHANGEUPPER event
  rtnetlink: Do not generate notifications for NOTIFY_PEERS event
  rtnetlink: Do not generate notifications for CHANGELOWERSTATE event

 drivers/net/bonding/bond_options.c |  2 --
 include/linux/netdevice.h          |  1 -
 include/uapi/linux/if_link.h       |  9 ---------
 net/core/rtnetlink.c               | 36 ------------------------------------
 4 files changed, 48 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events
  2017-04-07 21:25 [PATCH net-next 0/8] rtnetlink: Cleanup user notifications for netdev events David Ahern
@ 2017-04-07 21:25 ` David Ahern
  2017-04-08 18:06   ` Roopa Prabhu
  2017-04-09  1:36   ` Vlad Yasevich
  2017-04-07 21:25 ` [PATCH net-next 2/8] rtnetlink: Do not generate notification for UDP_TUNNEL_PUSH_INFO David Ahern
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 20+ messages in thread
From: David Ahern @ 2017-04-07 21:25 UTC (permalink / raw)
  To: netdev; +Cc: vyasevic, davem, David Ahern

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.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 include/uapi/linux/if_link.h | 2 --
 net/core/rtnetlink.c         | 8 --------
 2 files changed, 10 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 97f6d302f627..e8b7e9342cc0 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -903,7 +903,6 @@ enum {
 enum {
 	IFLA_EVENT_UNSPEC,
 	IFLA_EVENT_REBOOT,
-	IFLA_EVENT_CHANGE_MTU,
 	IFLA_EVENT_CHANGE_ADDR,
 	IFLA_EVENT_CHANGE_NAME,
 	IFLA_EVENT_FEAT_CHANGE,
@@ -912,7 +911,6 @@ enum {
 	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,
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index b2bd4c9ee860..72d365ae14b3 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1285,9 +1285,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
 	case NETDEV_REBOOT:
 		rtnl_event = IFLA_EVENT_REBOOT;
 		break;
-	case NETDEV_CHANGEMTU:
-		rtnl_event = IFLA_EVENT_CHANGE_MTU;
-		break;
 	case NETDEV_CHANGEADDR:
 		rtnl_event = IFLA_EVENT_CHANGE_ADDR;
 		break;
@@ -1312,9 +1309,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
 	case NETDEV_RESEND_IGMP:
 		rtnl_event = IFLA_EVENT_RESEND_IGMP;
 		break;
-	case NETDEV_PRECHANGEMTU:
-		rtnl_event = IFLA_EVENT_PRE_CHANGE_MTU;
-		break;
 	case NETDEV_CHANGEINFODATA:
 		rtnl_event = IFLA_EVENT_CHANGE_INFO_DATA;
 		break;
@@ -4191,7 +4185,6 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
 
 	switch (event) {
 	case NETDEV_REBOOT:
-	case NETDEV_CHANGEMTU:
 	case NETDEV_CHANGEADDR:
 	case NETDEV_CHANGENAME:
 	case NETDEV_FEAT_CHANGE:
@@ -4200,7 +4193,6 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
 	case NETDEV_NOTIFY_PEERS:
 	case NETDEV_CHANGEUPPER:
 	case NETDEV_RESEND_IGMP:
-	case NETDEV_PRECHANGEMTU:
 	case NETDEV_CHANGEINFODATA:
 	case NETDEV_PRECHANGEUPPER:
 	case NETDEV_CHANGELOWERSTATE:
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 2/8] rtnetlink: Do not generate notification for UDP_TUNNEL_PUSH_INFO
  2017-04-07 21:25 [PATCH net-next 0/8] rtnetlink: Cleanup user notifications for netdev events David Ahern
  2017-04-07 21:25 ` [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events David Ahern
@ 2017-04-07 21:25 ` David Ahern
  2017-04-07 21:25 ` [PATCH net-next 3/8] rtnetlink: Do not generate notifications for CHANGEADDR event David Ahern
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: David Ahern @ 2017-04-07 21:25 UTC (permalink / raw)
  To: netdev; +Cc: vyasevic, davem, David Ahern

NETDEV_UDP_TUNNEL_PUSH_INFO is an internal notifier; nothing userspace
can do so don't generate a netlink notification.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 include/uapi/linux/if_link.h | 1 -
 net/core/rtnetlink.c         | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index e8b7e9342cc0..064218a840b7 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -914,7 +914,6 @@ enum {
 	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,
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 72d365ae14b3..1a46be074fd6 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1318,9 +1318,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
 	case NETDEV_CHANGELOWERSTATE:
 		rtnl_event = IFLA_EVENT_CHANGE_LOWER_STATE;
 		break;
-	case NETDEV_UDP_TUNNEL_PUSH_INFO:
-		rtnl_event = IFLA_EVENT_UDP_TUNNEL_PUSH_INFO;
-		break;
 	case NETDEV_CHANGE_TX_QUEUE_LEN:
 		rtnl_event = IFLA_EVENT_CHANGE_TX_QUEUE_LEN;
 		break;
@@ -4196,7 +4193,6 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
 	case NETDEV_CHANGEINFODATA:
 	case NETDEV_PRECHANGEUPPER:
 	case NETDEV_CHANGELOWERSTATE:
-	case NETDEV_UDP_TUNNEL_PUSH_INFO:
 	case NETDEV_CHANGE_TX_QUEUE_LEN:
 		rtmsg_ifinfo_event(RTM_NEWLINK, dev, 0, event, GFP_KERNEL);
 		break;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 3/8] rtnetlink: Do not generate notifications for CHANGEADDR event
  2017-04-07 21:25 [PATCH net-next 0/8] rtnetlink: Cleanup user notifications for netdev events David Ahern
  2017-04-07 21:25 ` [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events David Ahern
  2017-04-07 21:25 ` [PATCH net-next 2/8] rtnetlink: Do not generate notification for UDP_TUNNEL_PUSH_INFO David Ahern
@ 2017-04-07 21:25 ` David Ahern
  2017-04-07 21:25 ` [PATCH net-next 4/8] rtnetlink: Do not generate notifications for POST_TYPE_CHANGE event David Ahern
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: David Ahern @ 2017-04-07 21:25 UTC (permalink / raw)
  To: netdev; +Cc: vyasevic, davem, David Ahern

Changing hardware address generates redundant messages:

[LINK]11: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default event CHANGE_ADDR
    link/ether 02:02:02:02:02:02 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 02:02:02:02:02:02 brd ff:ff:ff:ff:ff:ff

Do not send a notification for the CHANGEADDR notifier.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 include/uapi/linux/if_link.h | 1 -
 net/core/rtnetlink.c         | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 064218a840b7..df5ade1bc684 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -903,7 +903,6 @@ enum {
 enum {
 	IFLA_EVENT_UNSPEC,
 	IFLA_EVENT_REBOOT,
-	IFLA_EVENT_CHANGE_ADDR,
 	IFLA_EVENT_CHANGE_NAME,
 	IFLA_EVENT_FEAT_CHANGE,
 	IFLA_EVENT_BONDING_FAILOVER,
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 1a46be074fd6..1503138ebfe1 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1285,9 +1285,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
 	case NETDEV_REBOOT:
 		rtnl_event = IFLA_EVENT_REBOOT;
 		break;
-	case NETDEV_CHANGEADDR:
-		rtnl_event = IFLA_EVENT_CHANGE_ADDR;
-		break;
 	case NETDEV_CHANGENAME:
 		rtnl_event = IFLA_EVENT_CHANGE_NAME;
 		break;
@@ -4182,7 +4179,6 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
 
 	switch (event) {
 	case NETDEV_REBOOT:
-	case NETDEV_CHANGEADDR:
 	case NETDEV_CHANGENAME:
 	case NETDEV_FEAT_CHANGE:
 	case NETDEV_BONDING_FAILOVER:
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 4/8] rtnetlink: Do not generate notifications for POST_TYPE_CHANGE event
  2017-04-07 21:25 [PATCH net-next 0/8] rtnetlink: Cleanup user notifications for netdev events David Ahern
                   ` (2 preceding siblings ...)
  2017-04-07 21:25 ` [PATCH net-next 3/8] rtnetlink: Do not generate notifications for CHANGEADDR event David Ahern
@ 2017-04-07 21:25 ` David Ahern
  2017-04-07 21:25 ` [PATCH net-next 5/8] rtnetlink: Remove NETDEV_CHANGEINFODATA netdev event David Ahern
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: David Ahern @ 2017-04-07 21:25 UTC (permalink / raw)
  To: netdev; +Cc: vyasevic, davem, David Ahern

Changing the master device for a link generates many messages; the one
generated for POST_TYPE_CHANGE is redundant:

[LINK]11: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master br1 state UNKNOWN group default event POST_TYPE_CHANGE
    link/ether 02:02:02:02:02:03 brd ff:ff:ff:ff:ff:ff
[LINK]11: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue master br1 state UNKNOWN group default
    link/ether 02:02:02:02:02:03 brd ff:ff:ff:ff:ff:ff

Remove POST_TYPE_CHANGE from the list of notifiers that generate
notifications.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 include/uapi/linux/if_link.h | 1 -
 net/core/rtnetlink.c         | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index df5ade1bc684..80f3574a4630 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -906,7 +906,6 @@ enum {
 	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,
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 1503138ebfe1..739b06ac3e7f 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1294,9 +1294,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
 	case NETDEV_BONDING_FAILOVER:
 		rtnl_event = IFLA_EVENT_BONDING_FAILOVER;
 		break;
-	case NETDEV_POST_TYPE_CHANGE:
-		rtnl_event = IFLA_EVENT_POST_TYPE_CHANGE;
-		break;
 	case NETDEV_NOTIFY_PEERS:
 		rtnl_event = IFLA_EVENT_NOTIFY_PEERS;
 		break;
@@ -4182,7 +4179,6 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
 	case NETDEV_CHANGENAME:
 	case NETDEV_FEAT_CHANGE:
 	case NETDEV_BONDING_FAILOVER:
-	case NETDEV_POST_TYPE_CHANGE:
 	case NETDEV_NOTIFY_PEERS:
 	case NETDEV_CHANGEUPPER:
 	case NETDEV_RESEND_IGMP:
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 5/8] rtnetlink: Remove NETDEV_CHANGEINFODATA netdev event
  2017-04-07 21:25 [PATCH net-next 0/8] rtnetlink: Cleanup user notifications for netdev events David Ahern
                   ` (3 preceding siblings ...)
  2017-04-07 21:25 ` [PATCH net-next 4/8] rtnetlink: Do not generate notifications for POST_TYPE_CHANGE event David Ahern
@ 2017-04-07 21:25 ` David Ahern
  2017-04-07 21:25 ` [PATCH net-next 6/8] rtnetlink: Do not generate notifications for PRECHANGEUPPER event David Ahern
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 20+ messages in thread
From: David Ahern @ 2017-04-07 21:25 UTC (permalink / raw)
  To: netdev; +Cc: vyasevic, davem, David Ahern

Nobody cares about the event, so don't send it.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 drivers/net/bonding/bond_options.c | 2 --
 include/linux/netdevice.h          | 1 -
 include/uapi/linux/if_link.h       | 1 -
 net/core/rtnetlink.c               | 4 ----
 4 files changed, 8 deletions(-)

diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 1bcbb8913e17..533518a64496 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -673,8 +673,6 @@ 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)
-		call_netdevice_notifiers(NETDEV_CHANGEINFODATA, bond->dev);
 
 	return ret;
 }
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cc07c3be2705..bd0d26fbec2b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2279,7 +2279,6 @@ struct netdev_lag_lower_state_info {
 #define NETDEV_CHANGEUPPER	0x0015
 #define NETDEV_RESEND_IGMP	0x0016
 #define NETDEV_PRECHANGEMTU	0x0017 /* notify before mtu change happened */
-#define NETDEV_CHANGEINFODATA	0x0018
 #define NETDEV_BONDING_INFO	0x0019
 #define NETDEV_PRECHANGEUPPER	0x001A
 #define NETDEV_CHANGELOWERSTATE	0x001B
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 80f3574a4630..a500a12d0131 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -909,7 +909,6 @@ enum {
 	IFLA_EVENT_NOTIFY_PEERS,
 	IFLA_EVENT_CHANGE_UPPER,
 	IFLA_EVENT_RESEND_IGMP,
-	IFLA_EVENT_CHANGE_INFO_DATA,
 	IFLA_EVENT_PRE_CHANGE_UPPER,
 	IFLA_EVENT_CHANGE_LOWER_STATE,
 	IFLA_EVENT_CHANGE_TX_QUEUE_LEN,
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 739b06ac3e7f..702723c5db58 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1303,9 +1303,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
 	case NETDEV_RESEND_IGMP:
 		rtnl_event = IFLA_EVENT_RESEND_IGMP;
 		break;
-	case NETDEV_CHANGEINFODATA:
-		rtnl_event = IFLA_EVENT_CHANGE_INFO_DATA;
-		break;
 	case NETDEV_PRECHANGEUPPER:
 		rtnl_event = IFLA_EVENT_PRE_CHANGE_UPPER;
 		break;
@@ -4182,7 +4179,6 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
 	case NETDEV_NOTIFY_PEERS:
 	case NETDEV_CHANGEUPPER:
 	case NETDEV_RESEND_IGMP:
-	case NETDEV_CHANGEINFODATA:
 	case NETDEV_PRECHANGEUPPER:
 	case NETDEV_CHANGELOWERSTATE:
 	case NETDEV_CHANGE_TX_QUEUE_LEN:
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 6/8] rtnetlink: Do not generate notifications for PRECHANGEUPPER event
  2017-04-07 21:25 [PATCH net-next 0/8] rtnetlink: Cleanup user notifications for netdev events David Ahern
                   ` (4 preceding siblings ...)
  2017-04-07 21:25 ` [PATCH net-next 5/8] rtnetlink: Remove NETDEV_CHANGEINFODATA netdev event David Ahern
@ 2017-04-07 21:25 ` David Ahern
  2017-04-07 21:25 ` [PATCH net-next 7/8] rtnetlink: Do not generate notifications for NOTIFY_PEERS event David Ahern
  2017-04-07 21:25 ` [PATCH net-next 8/8] rtnetlink: Do not generate notifications for CHANGELOWERSTATE event David Ahern
  7 siblings, 0 replies; 20+ messages in thread
From: David Ahern @ 2017-04-07 21:25 UTC (permalink / raw)
  To: netdev; +Cc: vyasevic, davem, David Ahern

PRECHANGEUPPER is an internal event; do not generate userspace
notifications.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 include/uapi/linux/if_link.h | 1 -
 net/core/rtnetlink.c         | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index a500a12d0131..4fa3bf3eb21d 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -909,7 +909,6 @@ enum {
 	IFLA_EVENT_NOTIFY_PEERS,
 	IFLA_EVENT_CHANGE_UPPER,
 	IFLA_EVENT_RESEND_IGMP,
-	IFLA_EVENT_PRE_CHANGE_UPPER,
 	IFLA_EVENT_CHANGE_LOWER_STATE,
 	IFLA_EVENT_CHANGE_TX_QUEUE_LEN,
 };
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 702723c5db58..e2b0ec5174e7 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1303,9 +1303,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
 	case NETDEV_RESEND_IGMP:
 		rtnl_event = IFLA_EVENT_RESEND_IGMP;
 		break;
-	case NETDEV_PRECHANGEUPPER:
-		rtnl_event = IFLA_EVENT_PRE_CHANGE_UPPER;
-		break;
 	case NETDEV_CHANGELOWERSTATE:
 		rtnl_event = IFLA_EVENT_CHANGE_LOWER_STATE;
 		break;
@@ -4179,7 +4176,6 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
 	case NETDEV_NOTIFY_PEERS:
 	case NETDEV_CHANGEUPPER:
 	case NETDEV_RESEND_IGMP:
-	case NETDEV_PRECHANGEUPPER:
 	case NETDEV_CHANGELOWERSTATE:
 	case NETDEV_CHANGE_TX_QUEUE_LEN:
 		rtmsg_ifinfo_event(RTM_NEWLINK, dev, 0, event, GFP_KERNEL);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 7/8] rtnetlink: Do not generate notifications for NOTIFY_PEERS event
  2017-04-07 21:25 [PATCH net-next 0/8] rtnetlink: Cleanup user notifications for netdev events David Ahern
                   ` (5 preceding siblings ...)
  2017-04-07 21:25 ` [PATCH net-next 6/8] rtnetlink: Do not generate notifications for PRECHANGEUPPER event David Ahern
@ 2017-04-07 21:25 ` David Ahern
  2017-04-09  1:37   ` Vlad Yasevich
  2017-04-07 21:25 ` [PATCH net-next 8/8] rtnetlink: Do not generate notifications for CHANGELOWERSTATE event David Ahern
  7 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2017-04-07 21:25 UTC (permalink / raw)
  To: netdev; +Cc: vyasevic, davem, David Ahern

NOTIFY_PEERS is an internal event; do not generate userspace
notifications.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 include/uapi/linux/if_link.h | 1 -
 net/core/rtnetlink.c         | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 4fa3bf3eb21d..8f23e9dde667 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -906,7 +906,6 @@ enum {
 	IFLA_EVENT_CHANGE_NAME,
 	IFLA_EVENT_FEAT_CHANGE,
 	IFLA_EVENT_BONDING_FAILOVER,
-	IFLA_EVENT_NOTIFY_PEERS,
 	IFLA_EVENT_CHANGE_UPPER,
 	IFLA_EVENT_RESEND_IGMP,
 	IFLA_EVENT_CHANGE_LOWER_STATE,
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index e2b0ec5174e7..d2587aa339c4 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1294,9 +1294,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
 	case NETDEV_BONDING_FAILOVER:
 		rtnl_event = IFLA_EVENT_BONDING_FAILOVER;
 		break;
-	case NETDEV_NOTIFY_PEERS:
-		rtnl_event = IFLA_EVENT_NOTIFY_PEERS;
-		break;
 	case NETDEV_CHANGEUPPER:
 		rtnl_event = IFLA_EVENT_CHANGE_UPPER;
 		break;
@@ -4173,7 +4170,6 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
 	case NETDEV_CHANGENAME:
 	case NETDEV_FEAT_CHANGE:
 	case NETDEV_BONDING_FAILOVER:
-	case NETDEV_NOTIFY_PEERS:
 	case NETDEV_CHANGEUPPER:
 	case NETDEV_RESEND_IGMP:
 	case NETDEV_CHANGELOWERSTATE:
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH net-next 8/8] rtnetlink: Do not generate notifications for CHANGELOWERSTATE event
  2017-04-07 21:25 [PATCH net-next 0/8] rtnetlink: Cleanup user notifications for netdev events David Ahern
                   ` (6 preceding siblings ...)
  2017-04-07 21:25 ` [PATCH net-next 7/8] rtnetlink: Do not generate notifications for NOTIFY_PEERS event David Ahern
@ 2017-04-07 21:25 ` David Ahern
  7 siblings, 0 replies; 20+ messages in thread
From: David Ahern @ 2017-04-07 21:25 UTC (permalink / raw)
  To: netdev; +Cc: vyasevic, davem, David Ahern

CHANGELOWERSTATE is an internal event; do not generate userspace
notifications.

Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
 include/uapi/linux/if_link.h | 1 -
 net/core/rtnetlink.c         | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 8f23e9dde667..37e19966fbf3 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -908,7 +908,6 @@ enum {
 	IFLA_EVENT_BONDING_FAILOVER,
 	IFLA_EVENT_CHANGE_UPPER,
 	IFLA_EVENT_RESEND_IGMP,
-	IFLA_EVENT_CHANGE_LOWER_STATE,
 	IFLA_EVENT_CHANGE_TX_QUEUE_LEN,
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d2587aa339c4..21c03861aaf3 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1300,9 +1300,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
 	case NETDEV_RESEND_IGMP:
 		rtnl_event = IFLA_EVENT_RESEND_IGMP;
 		break;
-	case NETDEV_CHANGELOWERSTATE:
-		rtnl_event = IFLA_EVENT_CHANGE_LOWER_STATE;
-		break;
 	case NETDEV_CHANGE_TX_QUEUE_LEN:
 		rtnl_event = IFLA_EVENT_CHANGE_TX_QUEUE_LEN;
 		break;
@@ -4172,7 +4169,6 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
 	case NETDEV_BONDING_FAILOVER:
 	case NETDEV_CHANGEUPPER:
 	case NETDEV_RESEND_IGMP:
-	case NETDEV_CHANGELOWERSTATE:
 	case NETDEV_CHANGE_TX_QUEUE_LEN:
 		rtmsg_ifinfo_event(RTM_NEWLINK, dev, 0, event, GFP_KERNEL);
 		break;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events
  2017-04-07 21:25 ` [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events David Ahern
@ 2017-04-08 18:06   ` Roopa Prabhu
  2017-04-08 18:13     ` David Ahern
  2017-04-09  1:36   ` Vlad Yasevich
  1 sibling, 1 reply; 20+ messages in thread
From: Roopa Prabhu @ 2017-04-08 18:06 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, vyasevic, davem

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).

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events
  2017-04-08 18:06   ` Roopa Prabhu
@ 2017-04-08 18:13     ` David Ahern
  2017-04-08 18:18       ` Roopa Prabhu
  0 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2017-04-08 18:13 UTC (permalink / raw)
  To: Roopa Prabhu; +Cc: netdev, vyasevic, davem

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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events
  2017-04-08 18:13     ` David Ahern
@ 2017-04-08 18:18       ` Roopa Prabhu
  2017-04-10 15:39         ` Vlad Yasevich
  0 siblings, 1 reply; 20+ messages in thread
From: Roopa Prabhu @ 2017-04-08 18:18 UTC (permalink / raw)
  To: David Ahern; +Cc: netdev, vyasevic, davem

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events
  2017-04-07 21:25 ` [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events David Ahern
  2017-04-08 18:06   ` Roopa Prabhu
@ 2017-04-09  1:36   ` Vlad Yasevich
  2017-04-09  2:58     ` David Ahern
  1 sibling, 1 reply; 20+ messages in thread
From: Vlad Yasevich @ 2017-04-09  1:36 UTC (permalink / raw)
  To: David Ahern, netdev; +Cc: davem

On 04/07/2017 05: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.

Actually, I have plans for the CHANGE_MTU event.  The last message is not an event. If you
remove the event, it is much harder to track mtu changes.

-vlad
> 
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> ---
>  include/uapi/linux/if_link.h | 2 --
>  net/core/rtnetlink.c         | 8 -------
>  2 files changed, 10 deletions(-)
> 

> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 97f6d302f627..e8b7e9342cc0 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -903,7 +903,6 @@ enum {
>  enum {
>  	IFLA_EVENT_UNSPEC,
>  	IFLA_EVENT_REBOOT,
> -	IFLA_EVENT_CHANGE_MTU,
>  	IFLA_EVENT_CHANGE_ADDR,
>  	IFLA_EVENT_CHANGE_NAME,
>  	IFLA_EVENT_FEAT_CHANGE,
> @@ -912,7 +911,6 @@ enum {
>  	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,
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index b2bd4c9ee860..72d365ae14b3 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1285,9 +1285,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
>  	case NETDEV_REBOOT:
>  		rtnl_event = IFLA_EVENT_REBOOT;
>  		break;
> -	case NETDEV_CHANGEMTU:
> -		rtnl_event = IFLA_EVENT_CHANGE_MTU;
> -		break;
>  	case NETDEV_CHANGEADDR:
>  		rtnl_event = IFLA_EVENT_CHANGE_ADDR;
>  		break;
> @@ -1312,9 +1309,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
>  	case NETDEV_RESEND_IGMP:
>  		rtnl_event = IFLA_EVENT_RESEND_IGMP;
>  		break;
> -	case NETDEV_PRECHANGEMTU:
> -		rtnl_event = IFLA_EVENT_PRE_CHANGE_MTU;
> -		break;
>  	case NETDEV_CHANGEINFODATA:
>  		rtnl_event = IFLA_EVENT_CHANGE_INFO_DATA;
>  		break;
> @@ -4191,7 +4185,6 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
>  
>  	switch (event) {
>  	case NETDEV_REBOOT:
> -	case NETDEV_CHANGEMTU:
>  	case NETDEV_CHANGEADDR:
>  	case NETDEV_CHANGENAME:
>  	case NETDEV_FEAT_CHANGE:
> @@ -4200,7 +4193,6 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
>  	case NETDEV_NOTIFY_PEERS:
>  	case NETDEV_CHANGEUPPER:
>  	case NETDEV_RESEND_IGMP:
> -	case NETDEV_PRECHANGEMTU:
>  	case NETDEV_CHANGEINFODATA:
>  	case NETDEV_PRECHANGEUPPER:
>  	case NETDEV_CHANGELOWERSTATE:
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 7/8] rtnetlink: Do not generate notifications for NOTIFY_PEERS event
  2017-04-07 21:25 ` [PATCH net-next 7/8] rtnetlink: Do not generate notifications for NOTIFY_PEERS event David Ahern
@ 2017-04-09  1:37   ` Vlad Yasevich
  0 siblings, 0 replies; 20+ messages in thread
From: Vlad Yasevich @ 2017-04-09  1:37 UTC (permalink / raw)
  To: David Ahern, netdev; +Cc: davem

On 04/07/2017 05:25 PM, David Ahern wrote:
> NOTIFY_PEERS is an internal event; do not generate userspace
> notifications.

We actually need this event to support macvtap over bonding
as well as to resolve some issues with VMs using a bonded uplink
on the host.

-vlad

> 
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> ---
>  include/uapi/linux/if_link.h | 1 -
>  net/core/rtnetlink.c         | 4 ----
>  2 files changed, 5 deletions(-)
> 
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 4fa3bf3eb21d..8f23e9dde667 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -906,7 +906,6 @@ enum {
>  	IFLA_EVENT_CHANGE_NAME,
>  	IFLA_EVENT_FEAT_CHANGE,
>  	IFLA_EVENT_BONDING_FAILOVER,
> -	IFLA_EVENT_NOTIFY_PEERS,
>  	IFLA_EVENT_CHANGE_UPPER,
>  	IFLA_EVENT_RESEND_IGMP,
>  	IFLA_EVENT_CHANGE_LOWER_STATE,
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index e2b0ec5174e7..d2587aa339c4 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1294,9 +1294,6 @@ static int rtnl_fill_link_event(struct sk_buff *skb, unsigned long event)
>  	case NETDEV_BONDING_FAILOVER:
>  		rtnl_event = IFLA_EVENT_BONDING_FAILOVER;
>  		break;
> -	case NETDEV_NOTIFY_PEERS:
> -		rtnl_event = IFLA_EVENT_NOTIFY_PEERS;
> -		break;
>  	case NETDEV_CHANGEUPPER:
>  		rtnl_event = IFLA_EVENT_CHANGE_UPPER;
>  		break;
> @@ -4173,7 +4170,6 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
>  	case NETDEV_CHANGENAME:
>  	case NETDEV_FEAT_CHANGE:
>  	case NETDEV_BONDING_FAILOVER:
> -	case NETDEV_NOTIFY_PEERS:
>  	case NETDEV_CHANGEUPPER:
>  	case NETDEV_RESEND_IGMP:
>  	case NETDEV_CHANGELOWERSTATE:
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events
  2017-04-09  1:36   ` Vlad Yasevich
@ 2017-04-09  2:58     ` David Ahern
  0 siblings, 0 replies; 20+ messages in thread
From: David Ahern @ 2017-04-09  2:58 UTC (permalink / raw)
  To: vyasevic, netdev; +Cc: davem

On 4/8/17 9:36 PM, Vlad Yasevich wrote:
> On 04/07/2017 05: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.
> Actually, I have plans for the CHANGE_MTU event.  The last message is not an event. If you
> remove the event, it is much harder to track mtu changes.

it is a set of redundant messages. Roopa suggested using a bitmask for
the events changed by do_setlink which gets added to the last message
seen above. In doing so you only generate 1 message. The above causes
libnl (for example):

- message 1: delete dummy1 from its cache and re-create it
- message 2: delete dummy1 from its cache and re-create it
- message 3: delete dummy1 from its cache and re-create it

All you need is a single message at the end with an attribute that says
"MTU changed"

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events
  2017-04-08 18:18       ` Roopa Prabhu
@ 2017-04-10 15:39         ` Vlad Yasevich
  2017-04-10 15:49           ` David Ahern
  0 siblings, 1 reply; 20+ messages in thread
From: Vlad Yasevich @ 2017-04-10 15:39 UTC (permalink / raw)
  To: Roopa Prabhu, David Ahern; +Cc: netdev, davem

On 04/08/2017 02:18 PM, Roopa Prabhu wrote:
> 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
> 

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...)

-vlad

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events
  2017-04-10 15:39         ` Vlad Yasevich
@ 2017-04-10 15:49           ` David Ahern
  2017-04-15  1:51             ` Vlad Yasevich
  0 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2017-04-10 15:49 UTC (permalink / raw)
  To: vyasevic, Roopa Prabhu; +Cc: netdev, davem

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.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events
  2017-04-10 15:49           ` David Ahern
@ 2017-04-15  1:51             ` Vlad Yasevich
  2017-04-15  4:26               ` David Ahern
  0 siblings, 1 reply; 20+ messages in thread
From: Vlad Yasevich @ 2017-04-15  1:51 UTC (permalink / raw)
  To: David Ahern, Roopa Prabhu; +Cc: netdev, davem

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events
  2017-04-15  1:51             ` Vlad Yasevich
@ 2017-04-15  4:26               ` David Ahern
  2017-04-18  7:52                 ` Nicolas Dichtel
  0 siblings, 1 reply; 20+ messages in thread
From: David Ahern @ 2017-04-15  4:26 UTC (permalink / raw)
  To: vyasevic, Roopa Prabhu; +Cc: netdev, davem, Nicolas Dichtel

+Nicolas

On 4/14/17 7:51 PM, Vlad Yasevich wrote:
> 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
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events
  2017-04-15  4:26               ` David Ahern
@ 2017-04-18  7:52                 ` Nicolas Dichtel
  0 siblings, 0 replies; 20+ messages in thread
From: Nicolas Dichtel @ 2017-04-18  7:52 UTC (permalink / raw)
  To: David Ahern, vyasevic, Roopa Prabhu; +Cc: netdev, davem

Le 15/04/2017 à 06:26, David Ahern a écrit :
> +Nicolas
> 
> On 4/14/17 7:51 PM, Vlad Yasevich wrote:
>> 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.
If I remember well, the goal was to trigger a netlink event as soon as
'something' changed. It was not the case before that patch, some updates (I
don't remember which one) were not advertised via netlink.

>>
>> 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?
The rtnl msg is mandatory.


Regards,
Nicolas

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2017-04-18  7:52 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-07 21:25 [PATCH net-next 0/8] rtnetlink: Cleanup user notifications for netdev events David Ahern
2017-04-07 21:25 ` [PATCH net-next 1/8] rtnetlink: Do not generate notifications for MTU events David Ahern
2017-04-08 18:06   ` Roopa Prabhu
2017-04-08 18:13     ` David Ahern
2017-04-08 18:18       ` Roopa Prabhu
2017-04-10 15:39         ` Vlad Yasevich
2017-04-10 15:49           ` David Ahern
2017-04-15  1:51             ` Vlad Yasevich
2017-04-15  4:26               ` David Ahern
2017-04-18  7:52                 ` Nicolas Dichtel
2017-04-09  1:36   ` Vlad Yasevich
2017-04-09  2:58     ` David Ahern
2017-04-07 21:25 ` [PATCH net-next 2/8] rtnetlink: Do not generate notification for UDP_TUNNEL_PUSH_INFO David Ahern
2017-04-07 21:25 ` [PATCH net-next 3/8] rtnetlink: Do not generate notifications for CHANGEADDR event David Ahern
2017-04-07 21:25 ` [PATCH net-next 4/8] rtnetlink: Do not generate notifications for POST_TYPE_CHANGE event David Ahern
2017-04-07 21:25 ` [PATCH net-next 5/8] rtnetlink: Remove NETDEV_CHANGEINFODATA netdev event David Ahern
2017-04-07 21:25 ` [PATCH net-next 6/8] rtnetlink: Do not generate notifications for PRECHANGEUPPER event David Ahern
2017-04-07 21:25 ` [PATCH net-next 7/8] rtnetlink: Do not generate notifications for NOTIFY_PEERS event David Ahern
2017-04-09  1:37   ` Vlad Yasevich
2017-04-07 21:25 ` [PATCH net-next 8/8] rtnetlink: Do not generate notifications for CHANGELOWERSTATE event David Ahern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).