bonding: void calling rtmsg_ifinfo for netlink notifications

PATCH In review Xin Long 2 participants 6 messages Oct 23, 2017

Subsystems (3)

Networking Network drivers Bonding driver

lore

Share & tools

lore

Xin Long

author

2017-10-23 10:45 UTC #0 lore raw

v2 cover bonding: void calling rtmsg_ifinfo for netlink notifications

It's better to send notifications to userspace by the events in rtnetlink_event, instead of calling rtmsg_ifinfo directly.

This patcheset is to remove rtmsg_ifinfo called in bonding, the notifications can be handled by NETDEV_CHANGEUPPER and NETDEV_CHANGELOWERSTATE events in rtnetlink_event.

It could also fix some redundant notifications from bonding.

v1->v2:
- post to net-next.git instead of net.git, for it's more like an improvement for bonding

Xin Long (3):
bonding: remove rtmsg_ifinfo called in bond_master_upper_dev_link
rtnetlink: bring NETDEV_CHANGELOWERSTATE event process back to
rtnetlink_event
bonding: remove rtmsg_ifinfo called after bond_lower_state_changed

 drivers/net/bonding/bond_main.c | 11 +++--------
 include/net/bonding.h           |  4 ----
 net/core/rtnetlink.c            |  2 +-
 3 files changed, 4 insertions(+), 13 deletions(-)

Signature · 1 line

2.1.0

Xin Long

author

2017-10-23 10:45 UTC #1 lore raw

PATCH v2 1/3 bonding: remove rtmsg_ifinfo called in bond_master_upper_dev_link

Since commit 42e52bf9e3ae ("net: add netnotifier event for upper device change"), netdev_master_upper_dev_link has generated NETDEV_CHANGEUPPER event which would send a notification to userspace in rtnetlink_event.

There's no need to call rtmsg_ifinfo to send the notification any more. So this patch is to remove it from bond_master_upper_dev_link as well as bond_upper_dev_unlink to avoid the redundant notifications.

Signed-off-by Xin Long <[email protected]>

---
 drivers/net/bonding/bond_main.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

drivers/net/bonding/bond_main.c +3 -8

index 172eeeb..18b58e1 100644
@@ -1221,22 +1221,17 @@ static int bond_master_upper_dev_link(struct bonding *bond, struct slave *slave,
 				      struct netlink_ext_ack *extack)
 {
 	struct netdev_lag_upper_info lag_upper_info;
-	int err;
 
 	lag_upper_info.tx_type = bond_lag_tx_type(bond);
-	err = netdev_master_upper_dev_link(slave->dev, bond->dev, slave,
-					   &lag_upper_info, extack);
-	if (err)
-		return err;
-	rtmsg_ifinfo(RTM_NEWLINK, slave->dev, IFF_SLAVE, GFP_KERNEL);
-	return 0;
+
+	return netdev_master_upper_dev_link(slave->dev, bond->dev, slave,
+					    &lag_upper_info, extack);
 }
 
 static void bond_upper_dev_unlink(struct bonding *bond, struct slave *slave)
 {
 	netdev_upper_dev_unlink(slave->dev, bond->dev);
 	slave->dev->flags &= ~IFF_SLAVE;
-	rtmsg_ifinfo(RTM_NEWLINK, slave->dev, IFF_SLAVE, GFP_KERNEL);
 }
 
 static struct slave *bond_alloc_slave(struct bonding *bond)

Signature · 1 line

2.1.0

Xin Long

author

2017-10-23 10:45 UTC #2 lore raw

PATCH v2 2/3 rtnetlink: bring NETDEV_CHANGELOWERSTATE event process back to rtnetlink_event

This patch is to bring NETDEV_CHANGELOWERSTATE event process back to rtnetlink_event so that bonding could use it instead of calling rtmsg_ifinfo to send a notification to userspace after netdev lower state is changed in the later patch.

Signed-off-by Xin Long <[email protected]>

---
 net/core/rtnetlink.c | 1 +
 1 file changed, 1 insertion(+)

net/core/rtnetlink.c +1 -0

index df8dba9..854a848 100644
@@ -4385,6 +4385,7 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
 	case NETDEV_CHANGEUPPER:
 	case NETDEV_RESEND_IGMP:
 	case NETDEV_CHANGEINFODATA:
+	case NETDEV_CHANGELOWERSTATE:
 	case NETDEV_CHANGE_TX_QUEUE_LEN:
 		rtmsg_ifinfo_event(RTM_NEWLINK, dev, 0, rtnl_get_event(event),
 				   GFP_KERNEL, NULL);

Signature · 1 line

2.1.0

Xin Long

author

2017-10-23 10:45 UTC #3 lore raw

PATCH v2 3/3 bonding: remove rtmsg_ifinfo called after bond_lower_state_changed

After the patch 'rtnetlink: bring NETDEV_CHANGELOWERSTATE event process back to rtnetlink_event', bond_lower_state_changed would generate NETDEV_CHANGEUPPER event which would send a notification to userspace in rtnetlink_event.

There's no need to call rtmsg_ifinfo to send the notification any more. So this patch is to remove it from these places after bond_lower_state_changed.

Besides, after this, rtmsg_ifinfo is not needed to be exported.

Signed-off-by Xin Long <[email protected]>

---
 include/net/bonding.h | 4 ----
 net/core/rtnetlink.c  | 1 -
 2 files changed, 5 deletions(-)

include/net/bonding.h +0 -4

index 2860cc6..f801fc9 100644
@@ -330,7 +330,6 @@ static inline void bond_set_active_slave(struct slave *slave)
 		slave->backup = 0;
 		bond_queue_slave_event(slave);
 		bond_lower_state_changed(slave);
-		rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
 	}
 }

@@ -340,7 +339,6 @@ static inline void bond_set_backup_slave(struct slave *slave)
 		slave->backup = 1;
 		bond_queue_slave_event(slave);
 		bond_lower_state_changed(slave);
-		rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
 	}
 }

@@ -353,7 +351,6 @@ static inline void bond_set_slave_state(struct slave *slave,
 	slave->backup = slave_state;
 	if (notify) {
 		bond_lower_state_changed(slave);
-		rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
 		bond_queue_slave_event(slave);
 		slave->should_notify = 0;
 	} else {

@@ -385,7 +382,6 @@ static inline void bond_slave_state_notify(struct bonding *bond)
 	bond_for_each_slave(bond, tmp, iter) {
 		if (tmp->should_notify) {
 			bond_lower_state_changed(tmp);
-			rtmsg_ifinfo(RTM_NEWLINK, tmp->dev, 0, GFP_ATOMIC);
 			tmp->should_notify = 0;
 		}
 	}

net/core/rtnetlink.c +0 -1

index 854a848..de24d39 100644
@@ -2989,7 +2989,6 @@ void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
 {
 	rtmsg_ifinfo_event(type, dev, change, rtnl_get_event(0), flags, NULL);
 }
-EXPORT_SYMBOL(rtmsg_ifinfo);
 
 void rtmsg_ifinfo_newnet(int type, struct net_device *dev, unsigned int change,
 			 gfp_t flags, int *new_nsid)

Signature · 1 line

2.1.0

kbuild test robot

build bot

2017-10-23 14:45 UTC #4 lore raw

Re: [PATCHv2 net-next 3/3] bonding: remove rtmsg_ifinfo called after bond_lower_state_changed

url: https://github.com/0day-ci/linux/commits/Xin-Long/bonding-void-calling-rtmsg_ifinfo-for-netlink-notifications/20171023-203332 config: x86_64-rhel (attached as .config) compiler: gcc-6 (Debian…

Hi Xin,

[auto build test ERROR on net-next/master]

url: https://github.com/0day-ci/linux/commits/Xin-Long/bonding-void-calling-rtmsg_ifinfo-for-netlink-notifications/20171023-203332
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:

        # save the attached .config to linux build tree
        make ARCH=x86_64

All errors (new ones prefixed by >>):

>> ERROR: "rtmsg_ifinfo" [net/bridge/bridge.ko] undefined!

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Xin Long

author

2017-10-23 15:15 UTC #5 lore raw

Re: [PATCHv2 net-next 3/3] bonding: remove rtmsg_ifinfo called after bond_lower_state_changed

Quoted 4 lines · kbuild test robot

Hi Xin,
[auto build test ERROR on net-next/master]

Quoted 10 lines

url: https://github.com/0day-ci/linux/commits/Xin-Long/bonding-void-calling-rtmsg_ifinfo-for-netlink-notifications/20171023-203332
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64
All errors (new ones prefixed by >>):
Quoted 1 line
Quoted 1 line
ERROR: "rtmsg_ifinfo" [net/bridge/bridge.ko] undefined!

sorry, didn't notice bridge is still using rtmsg_ifinfo.

rtmsg_ifinfo actually is no need to be called in br_del_if(), since patch:
dc709f3 rtnetlink: bring NETDEV_CHANGEUPPER event process back in
rtnetlink_event

I will propose another patch to remove this.
for this bonding one, I will post v3 without removing rtmsg_ifinfo export first.