Skip to contentSkip to conversation

fix up RTM_SETLINK handling

PATCH In review Roland Dreier 2 participants 8 messages Jan 15, 2003 – Jan 16, 2003

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Roland Dreier

author
#0 lore raw

PATCH 1/1 fix up RTM_SETLINK handling

This patch fixes up some oversights I noticed with my code for handling RTM_SETLINK messages and brings do_setlink() more in line with SIOCSIFHWADDR handling in net/core/dev.c. In particular, the patch adds some checks that were missing, uses the set_mac_address method rather than memcpy() directly into dev->dev_addr, and calls the netdev notifier chain.

To call the netdev notifier chain I had to make netdev_chain not be static. I added the declaration to <linux/netdevice.h> but I am open to other ways to give rtnetlink.c access to netdev_chain.

Best,
Roland

 include/linux/netdevice.h |    1 +
 net/core/dev.c            |    2 +-
 net/core/rtnetlink.c      |   18 ++++++++++++++++--
 3 files changed, 18 insertions(+), 3 deletions(-)

===== include/linux/netdevice.h 1.27 vs edited =====

include/linux/netdevice.h +1 -0
@@ -462,6 +462,7 @@
 extern struct net_device		loopback_dev;		/* The loopback */
 extern struct net_device		*dev_base;		/* All devices */
 extern rwlock_t				dev_base_lock;		/* Device list lock */
+extern struct notifier_block		*netdev_chain;
 
 extern int			netdev_boot_setup_add(char *name, struct ifmap *map);
 extern int 			netdev_boot_setup_check(struct net_device *dev);

===== net/core/dev.c 1.52 vs edited =====

net/core/dev.c +1 -1
@@ -188,7 +188,7 @@
  *	Our notifier list
  */
 
-static struct notifier_block *netdev_chain;
+struct notifier_block *netdev_chain;
 
 /*
  *	Device drivers call our routines to queue packets here. We empty the

===== net/core/rtnetlink.c 1.6 vs edited =====

net/core/rtnetlink.c +16 -2
@@ -234,10 +234,20 @@
 	err = -EINVAL;
 
 	if (ida[IFLA_ADDRESS - 1]) {
+		if (!dev->set_mac_address) {
+			err = -EOPNOTSUPP;
+			goto out;
+		}
+		if (!netif_device_present(dev)) {
+			err = -ENODEV;
+			goto out;
+		}
 		if (ida[IFLA_ADDRESS - 1]->rta_len != RTA_LENGTH(dev->addr_len))
 			goto out;
-		memcpy(dev->dev_addr, RTA_DATA(ida[IFLA_ADDRESS - 1]),
-		       dev->addr_len);
+
+		err = dev->set_mac_address(dev, RTA_DATA(ida[IFLA_ADDRESS - 1]));
+		if (err)
+			goto out;
 	}
 
 	if (ida[IFLA_BROADCAST - 1]) {

@@ -250,6 +260,10 @@
 	err = 0;
 
 out:
+	if (!err)
+		notifier_call_chain(&netdev_chain,
+				    NETDEV_CHANGEADDR, dev);
+
 	dev_put(dev);
 	return err;
 }
#1 lore raw

From: Roland Dreier <[email protected]>
Date: 14 Jan 2003 23:01:00 -0800

To call the netdev notifier chain I had to make netdev_chain not be static. I added the declaration to <linux/netdevice.h> but I am open to other ways to give rtnetlink.c access to netdev_chain.

Ummm, what is the problem with using register_netdevice_notifier()? It is precisely there so that netdev_chain need not be exported.

author
#2 lore raw

Roland> To call the netdev notifier chain I had to make Roland> netdev_chain not be static. I added the declaration to Roland> <linux/netdevice.h> but I am open to other ways to give Roland> rtnetlink.c access to netdev_chain.

David> Ummm, what is the problem with using
David> register_netdevice_notifier()? It is precisely there so David> that netdev_chain need not be exported.

do_setlink() needs to do the opposite of registering a notifier -- it's changing an interface's address, so it needs to call the netdev_chain with NETDEV_CHANGEADDR (just like the handler for SIOCSIFHWADDR does).

If you prefer, I could add a call_netdevice_notifiers() to dev.c and leave netdev_chain static. Just let me know and I'll make a new patch.

Thanks,
Roland

#3 lore raw

From: Roland Dreier <[email protected]>
Date: 15 Jan 2003 16:22:22 -0800

If you prefer, I could add a call_netdevice_notifiers() to dev.c and leave netdev_chain static. Just let me know and I'll make a new patch.

Yes, that is the preferred way to do this.

author
#4 lore raw

OK, here's a new version of my RTM_SETLINK fixups. I added a function call_netdevice_notifiers() instead of making netdev_chain not static.

Thanks,
Roland

 include/linux/netdevice.h |    1 +
 net/core/dev.c            |   12 ++++++++++++
 net/core/rtnetlink.c      |   17 +++++++++++++++--
 3 files changed, 28 insertions(+), 2 deletions(-)

===== include/linux/netdevice.h 1.27 vs edited =====

include/linux/netdevice.h +1 -0
@@ -480,6 +480,7 @@
 extern int		unregister_netdevice(struct net_device *dev);
 extern int 		register_netdevice_notifier(struct notifier_block *nb);
 extern int		unregister_netdevice_notifier(struct notifier_block *nb);
+extern int		call_netdevice_notifiers(unsigned long val, void *v);
 extern int		dev_new_index(void);
 extern struct net_device	*dev_get_by_index(int ifindex);
 extern struct net_device	*__dev_get_by_index(int ifindex);

===== net/core/dev.c 1.52 vs edited =====

net/core/dev.c +12 -0
@@ -877,6 +877,18 @@
 	return notifier_chain_unregister(&netdev_chain, nb);
 }
 
+/**
+ *	call_netdevice_notifiers - call all network notifier blocks
+ *
+ *	Call all network notifier blocks.  Parameters and return value
+ *	are as for notifier_call_chain().
+ */
+
+int call_netdevice_notifiers(unsigned long val, void *v)
+{
+	return notifier_call_chain(&netdev_chain, val, v);
+}
+
 /*
  *	Support routine. Sends outgoing frames to any network
  *	taps currently in use.

===== net/core/rtnetlink.c 1.6 vs edited =====

net/core/rtnetlink.c +15 -2
@@ -234,10 +234,20 @@
 	err = -EINVAL;
 
 	if (ida[IFLA_ADDRESS - 1]) {
+		if (!dev->set_mac_address) {
+			err = -EOPNOTSUPP;
+			goto out;
+		}
+		if (!netif_device_present(dev)) {
+			err = -ENODEV;
+			goto out;
+		}
 		if (ida[IFLA_ADDRESS - 1]->rta_len != RTA_LENGTH(dev->addr_len))
 			goto out;
-		memcpy(dev->dev_addr, RTA_DATA(ida[IFLA_ADDRESS - 1]),
-		       dev->addr_len);
+
+		err = dev->set_mac_address(dev, RTA_DATA(ida[IFLA_ADDRESS - 1]));
+		if (err)
+			goto out;
 	}
 
 	if (ida[IFLA_BROADCAST - 1]) {

@@ -250,6 +260,9 @@
 	err = 0;
 
 out:
+	if (!err)
+		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
+
 	dev_put(dev);
 	return err;
 }
#5 lore raw

From: Roland Dreier <[email protected]>
Date: 15 Jan 2003 17:24:39 -0800

I added a function call_netdevice_notifiers() instead of making netdev_chain not static.

Missing module symbol export.

author
#6 lore raw

Added EXPORT_SYMBOL for call_netdevice_notifiers().

Thanks,
Roland

 include/linux/netdevice.h |    1 +
 net/core/dev.c            |   12 ++++++++++++
 net/core/rtnetlink.c      |   17 +++++++++++++++--
 net/netsyms.c             |    1 +
 4 files changed, 29 insertions(+), 2 deletions(-)

===== include/linux/netdevice.h 1.27 vs edited =====

include/linux/netdevice.h +1 -0
@@ -480,6 +480,7 @@
 extern int		unregister_netdevice(struct net_device *dev);
 extern int 		register_netdevice_notifier(struct notifier_block *nb);
 extern int		unregister_netdevice_notifier(struct notifier_block *nb);
+extern int		call_netdevice_notifiers(unsigned long val, void *v);
 extern int		dev_new_index(void);
 extern struct net_device	*dev_get_by_index(int ifindex);
 extern struct net_device	*__dev_get_by_index(int ifindex);

===== net/netsyms.c 1.40 vs edited =====

net/netsyms.c +1 -0
@@ -500,6 +500,7 @@
 /* Device callback registration */
 EXPORT_SYMBOL(register_netdevice_notifier);
 EXPORT_SYMBOL(unregister_netdevice_notifier);
+EXPORT_SYMBOL(call_netdevice_notifiers);
 
 /* support for loadable net drivers */
 #ifdef CONFIG_NET

===== net/core/dev.c 1.52 vs edited =====

net/core/dev.c +12 -0
@@ -877,6 +877,18 @@
 	return notifier_chain_unregister(&netdev_chain, nb);
 }
 
+/**
+ *	call_netdevice_notifiers - call all network notifier blocks
+ *
+ *	Call all network notifier blocks.  Parameters and return value
+ *	are as for notifier_call_chain().
+ */
+
+int call_netdevice_notifiers(unsigned long val, void *v)
+{
+	return notifier_call_chain(&netdev_chain, val, v);
+}
+
 /*
  *	Support routine. Sends outgoing frames to any network
  *	taps currently in use.

===== net/core/rtnetlink.c 1.6 vs edited =====

net/core/rtnetlink.c +15 -2
@@ -234,10 +234,20 @@
 	err = -EINVAL;
 
 	if (ida[IFLA_ADDRESS - 1]) {
+		if (!dev->set_mac_address) {
+			err = -EOPNOTSUPP;
+			goto out;
+		}
+		if (!netif_device_present(dev)) {
+			err = -ENODEV;
+			goto out;
+		}
 		if (ida[IFLA_ADDRESS - 1]->rta_len != RTA_LENGTH(dev->addr_len))
 			goto out;
-		memcpy(dev->dev_addr, RTA_DATA(ida[IFLA_ADDRESS - 1]),
-		       dev->addr_len);
+
+		err = dev->set_mac_address(dev, RTA_DATA(ida[IFLA_ADDRESS - 1]));
+		if (err)
+			goto out;
 	}
 
 	if (ida[IFLA_BROADCAST - 1]) {

@@ -250,6 +260,9 @@
 	err = 0;
 
 out:
+	if (!err)
+		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
+
 	dev_put(dev);
 	return err;
 }
#7 lore raw

From: Roland Dreier <[email protected]>
Date: 15 Jan 2003 17:39:40 -0800

Added EXPORT_SYMBOL for call_netdevice_notifiers().

Patch applied, thanks a lot for fixing this patch up right for me.