[PATCH net-next 0/7] bonding: clean up and standarize logging printks

[PATCH net-next 0/7] bonding: clean up and standarize logging printks

[Jarod Wilson]

This set improves a few somewhat terse bonding debug messages, fixes some
errors in others, and then standarizes the majority of them, using new
slave_* printk macros that wrap around netdev_* to ensure both master
and slave information is provided consistently, where relevant. This set
proves very useful in debugging issues on hosts with multiple bonds.

I've run an array of LNST tests over this set, creating and destroying
quite a few different bonds of the course of testing, fixed the little
gotchas here and there, and everything looks stable and reasonable to me,
but I can't guarantee I've tested every possible message and scenario to
catch every possible "slave could be NULL" case.

Jarod Wilson (7):
  bonding: improve event debug usability
  bonding: fix error messages in bond_do_fail_over_mac
  bonding: add slave_foo printk macros
  bonding/main: convert to using slave printk macros
  bonding/802.3ad: convert to using slave printk macros
  bonding/alb: convert to using slave printk macros
  bonding/options: convert to using slave printk macros

 drivers/net/bonding/bond_3ad.c     | 222 +++++++++++----------
 drivers/net/bonding/bond_alb.c     |  30 +--
 drivers/net/bonding/bond_main.c    | 309 +++++++++++++----------------
 drivers/net/bonding/bond_options.c |  30 ++-
 include/net/bonding.h              |   9 +
 5 files changed, 293 insertions(+), 307 deletions(-)

Suggested-by: Joe Perches <joe@perches.com>
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>

-- 
2.20.1

[PATCH net-next 1/7] bonding: improve event debug usability

[Jarod Wilson]

Seeing bonding debug log data along the lines of "event: 5" is a bit spartan,
and often requires a lookup table if you don't remember what every event is.
Make use of netdev_cmd_to_name for an improved debugging experience, so for
the prior example, you'll see: "bond_netdev_event received NETDEV_REGISTER"
instead (both are prefixed with the device for which the event pertains).

CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/net/bonding/bond_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 407f4095a37a..32b6c1f9b82e 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3212,7 +3212,8 @@ static int bond_netdev_event(struct notifier_block *this,
 {
 	struct net_device *event_dev = netdev_notifier_info_to_dev(ptr);
 
-	netdev_dbg(event_dev, "event: %lx\n", event);
+	netdev_dbg(event_dev, "%s received %s\n",
+		   __func__, netdev_cmd_to_name(event));
 
 	if (!(event_dev->priv_flags & IFF_BONDING))
 		return NOTIFY_DONE;
-- 
2.20.1

[PATCH net-next 2/7] bonding: fix error messages in bond_do_fail_over_mac

[Jarod Wilson]

Passing the bond name again to debug output when referencing slave is wrong.
We're trying to set the bond's MAC to that of the new_active slave, so adjust
the error message slightly and pass in the slave's name, not the bond's.
Then we're trying to set the MAC on the old active slave, but putting the
new active slave's name in the output. While we're at it, clarify the
error messages so you know which one actually triggered.

CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/net/bonding/bond_main.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 32b6c1f9b82e..5823070f07a6 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -661,8 +661,8 @@ static void bond_do_fail_over_mac(struct bonding *bond,
 		if (new_active) {
 			rv = bond_set_dev_addr(bond->dev, new_active->dev);
 			if (rv)
-				netdev_err(bond->dev, "Error %d setting MAC of slave %s\n",
-					   -rv, bond->dev->name);
+				netdev_err(bond->dev, "Error %d setting bond MAC from slave %s\n",
+					   -rv, new_active->dev->name);
 		}
 		break;
 	case BOND_FOM_FOLLOW:
@@ -692,7 +692,7 @@ static void bond_do_fail_over_mac(struct bonding *bond,
 		rv = dev_set_mac_address(new_active->dev,
 					 (struct sockaddr *)&ss, NULL);
 		if (rv) {
-			netdev_err(bond->dev, "Error %d setting MAC of slave %s\n",
+			netdev_err(bond->dev, "Error %d setting MAC of new active slave %s\n",
 				   -rv, new_active->dev->name);
 			goto out;
 		}
@@ -707,8 +707,8 @@ static void bond_do_fail_over_mac(struct bonding *bond,
 		rv = dev_set_mac_address(old_active->dev,
 					 (struct sockaddr *)&ss, NULL);
 		if (rv)
-			netdev_err(bond->dev, "Error %d setting MAC of slave %s\n",
-				   -rv, new_active->dev->name);
+			netdev_err(bond->dev, "Error %d setting MAC of old active slave %s\n",
+				   -rv, old_active->dev->name);
 out:
 		break;
 	default:
-- 
2.20.1

[PATCH net-next 3/7] bonding: add slave_foo printk macros

[Jarod Wilson]

Where possible, we generally want both the bond master and the relevant slave
information in message output. Standardize the format using new slave_*
printk macros.

Suggested-by: Joe Perches <joe@perches.com>
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 include/net/bonding.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/net/bonding.h b/include/net/bonding.h
index b46d68acf701..676e7fae05a3 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -38,6 +38,15 @@
 #define __long_aligned __attribute__((aligned((sizeof(long)))))
 #endif
 
+#define slave_info(bond_dev, slave_dev, fmt, ...) \
+	netdev_info(bond_dev, "(slave %s): " fmt, (slave_dev)->name, ##__VA_ARGS__)
+#define slave_warn(bond_dev, slave_dev, fmt, ...) \
+	netdev_warn(bond_dev, "(slave %s): " fmt, (slave_dev)->name, ##__VA_ARGS__)
+#define slave_dbg(bond_dev, slave_dev, fmt, ...) \
+	netdev_dbg(bond_dev, "(slave %s): " fmt, (slave_dev)->name, ##__VA_ARGS__)
+#define slave_err(bond_dev, slave_dev, fmt, ...) \
+	netdev_err(bond_dev, "(slave %s): " fmt, (slave_dev)->name, ##__VA_ARGS__)
+
 #define BOND_MODE(bond) ((bond)->params.mode)
 
 /* slave list primitives */
-- 
2.20.1

[PATCH net-next 4/7] bonding/main: convert to using slave printk macros

[Jarod Wilson]

All of these printk instances benefit from having both master and slave
device information included, so convert to using a standardized macro
format and remove redundant information.

Suggested-by: Joe Perches <joe@perches.com>
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/net/bonding/bond_main.c | 306 +++++++++++++++-----------------
 1 file changed, 139 insertions(+), 167 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5823070f07a6..cad371ae18bc 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -613,8 +613,8 @@ static int bond_set_dev_addr(struct net_device *bond_dev,
 {
 	int err;
 
-	netdev_dbg(bond_dev, "bond_dev=%p slave_dev=%p slave_dev->name=%s slave_dev->addr_len=%d\n",
-		   bond_dev, slave_dev, slave_dev->name, slave_dev->addr_len);
+	slave_dbg(bond_dev, slave_dev, "bond_dev=%p slave_dev=%p slave_dev->addr_len=%d\n",
+		  bond_dev, slave_dev, slave_dev->addr_len);
 	err = dev_pre_changeaddr_notify(bond_dev, slave_dev->dev_addr, NULL);
 	if (err)
 		return err;
@@ -661,8 +661,8 @@ static void bond_do_fail_over_mac(struct bonding *bond,
 		if (new_active) {
 			rv = bond_set_dev_addr(bond->dev, new_active->dev);
 			if (rv)
-				netdev_err(bond->dev, "Error %d setting bond MAC from slave %s\n",
-					   -rv, new_active->dev->name);
+				slave_err(bond->dev, new_active->dev, "Error %d setting bond MAC from slave\n",
+					  -rv);
 		}
 		break;
 	case BOND_FOM_FOLLOW:
@@ -692,8 +692,8 @@ static void bond_do_fail_over_mac(struct bonding *bond,
 		rv = dev_set_mac_address(new_active->dev,
 					 (struct sockaddr *)&ss, NULL);
 		if (rv) {
-			netdev_err(bond->dev, "Error %d setting MAC of new active slave %s\n",
-				   -rv, new_active->dev->name);
+			slave_err(bond->dev, new_active->dev, "Error %d setting MAC of new active slave\n",
+				  -rv);
 			goto out;
 		}
 
@@ -707,8 +707,8 @@ static void bond_do_fail_over_mac(struct bonding *bond,
 		rv = dev_set_mac_address(old_active->dev,
 					 (struct sockaddr *)&ss, NULL);
 		if (rv)
-			netdev_err(bond->dev, "Error %d setting MAC of old active slave %s\n",
-				   -rv, old_active->dev->name);
+			slave_err(bond->dev, old_active->dev, "Error %d setting MAC of old active slave\n",
+				  -rv);
 out:
 		break;
 	default:
@@ -834,9 +834,8 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
 
 		if (new_active->link == BOND_LINK_BACK) {
 			if (bond_uses_primary(bond)) {
-				netdev_info(bond->dev, "making interface %s the new active one %d ms earlier\n",
-					    new_active->dev->name,
-					    (bond->params.updelay - new_active->delay) * bond->params.miimon);
+				slave_info(bond->dev, new_active->dev, "making interface the new active one %d ms earlier\n",
+					   (bond->params.updelay - new_active->delay) * bond->params.miimon);
 			}
 
 			new_active->delay = 0;
@@ -850,8 +849,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
 				bond_alb_handle_link_change(bond, new_active, BOND_LINK_UP);
 		} else {
 			if (bond_uses_primary(bond)) {
-				netdev_info(bond->dev, "making interface %s the new active one\n",
-					    new_active->dev->name);
+				slave_info(bond->dev, new_active->dev, "making interface the new active one\n");
 			}
 		}
 	}
@@ -939,7 +937,7 @@ void bond_select_active_slave(struct bonding *bond)
 			return;
 
 		if (netif_carrier_ok(bond->dev))
-			netdev_info(bond->dev, "first active interface up!\n");
+			slave_info(bond->dev, best_slave->dev, "active interface up!\n");
 		else
 			netdev_info(bond->dev, "now running without any active interface!\n");
 	}
@@ -1369,15 +1367,14 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	if (!bond->params.use_carrier &&
 	    slave_dev->ethtool_ops->get_link == NULL &&
 	    slave_ops->ndo_do_ioctl == NULL) {
-		netdev_warn(bond_dev, "no link monitoring support for %s\n",
-			    slave_dev->name);
+		slave_warn(bond_dev, slave_dev, "no link monitoring support\n");
 	}
 
 	/* already in-use? */
 	if (netdev_is_rx_handler_busy(slave_dev)) {
 		NL_SET_ERR_MSG(extack, "Device is in use and cannot be enslaved");
-		netdev_err(bond_dev,
-			   "Error: Device is in use and cannot be enslaved\n");
+		slave_err(bond_dev, slave_dev,
+			  "Error: Device is in use and cannot be enslaved\n");
 		return -EBUSY;
 	}
 
@@ -1390,21 +1387,16 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	/* vlan challenged mutual exclusion */
 	/* no need to lock since we're protected by rtnl_lock */
 	if (slave_dev->features & NETIF_F_VLAN_CHALLENGED) {
-		netdev_dbg(bond_dev, "%s is NETIF_F_VLAN_CHALLENGED\n",
-			   slave_dev->name);
+		slave_dbg(bond_dev, slave_dev, "is NETIF_F_VLAN_CHALLENGED\n");
 		if (vlan_uses_dev(bond_dev)) {
 			NL_SET_ERR_MSG(extack, "Can not enslave VLAN challenged device to VLAN enabled bond");
-			netdev_err(bond_dev, "Error: cannot enslave VLAN challenged slave %s on VLAN enabled bond %s\n",
-				   slave_dev->name, bond_dev->name);
+			slave_err(bond_dev, slave_dev, "Error: cannot enslave VLAN challenged slave on VLAN enabled bond\n");
 			return -EPERM;
 		} else {
-			netdev_warn(bond_dev, "enslaved VLAN challenged slave %s. Adding VLANs will be blocked as long as %s is part of bond %s\n",
-				    slave_dev->name, slave_dev->name,
-				    bond_dev->name);
+			slave_warn(bond_dev, slave_dev, "enslaved VLAN challenged slave. Adding VLANs will be blocked as long as it is part of bond.\n");
 		}
 	} else {
-		netdev_dbg(bond_dev, "%s is !NETIF_F_VLAN_CHALLENGED\n",
-			   slave_dev->name);
+		slave_dbg(bond_dev, slave_dev, "is !NETIF_F_VLAN_CHALLENGED\n");
 	}
 
 	/* Old ifenslave binaries are no longer supported.  These can
@@ -1414,8 +1406,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	 */
 	if (slave_dev->flags & IFF_UP) {
 		NL_SET_ERR_MSG(extack, "Device can not be enslaved while up");
-		netdev_err(bond_dev, "%s is up - this may be due to an out of date ifenslave\n",
-			   slave_dev->name);
+		slave_err(bond_dev, slave_dev, "slave is up - this may be due to an out of date ifenslave\n");
 		return -EPERM;
 	}
 
@@ -1428,14 +1419,14 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	 */
 	if (!bond_has_slaves(bond)) {
 		if (bond_dev->type != slave_dev->type) {
-			netdev_dbg(bond_dev, "change device type from %d to %d\n",
-				   bond_dev->type, slave_dev->type);
+			slave_dbg(bond_dev, slave_dev, "change device type from %d to %d\n",
+				  bond_dev->type, slave_dev->type);
 
 			res = call_netdevice_notifiers(NETDEV_PRE_TYPE_CHANGE,
 						       bond_dev);
 			res = notifier_to_errno(res);
 			if (res) {
-				netdev_err(bond_dev, "refused to change device type\n");
+				slave_err(bond_dev, slave_dev, "refused to change device type\n");
 				return -EBUSY;
 			}
 
@@ -1455,31 +1446,31 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 		}
 	} else if (bond_dev->type != slave_dev->type) {
 		NL_SET_ERR_MSG(extack, "Device type is different from other slaves");
-		netdev_err(bond_dev, "%s ether type (%d) is different from other slaves (%d), can not enslave it\n",
-			   slave_dev->name, slave_dev->type, bond_dev->type);
+		slave_err(bond_dev, slave_dev, "ether type (%d) is different from other slaves (%d), can not enslave it\n",
+			  slave_dev->type, bond_dev->type);
 		return -EINVAL;
 	}
 
 	if (slave_dev->type == ARPHRD_INFINIBAND &&
 	    BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP) {
 		NL_SET_ERR_MSG(extack, "Only active-backup mode is supported for infiniband slaves");
-		netdev_warn(bond_dev, "Type (%d) supports only active-backup mode\n",
-			    slave_dev->type);
+		slave_warn(bond_dev, slave_dev, "Type (%d) supports only active-backup mode\n",
+			   slave_dev->type);
 		res = -EOPNOTSUPP;
 		goto err_undo_flags;
 	}
 
 	if (!slave_ops->ndo_set_mac_address ||
 	    slave_dev->type == ARPHRD_INFINIBAND) {
-		netdev_warn(bond_dev, "The slave device specified does not support setting the MAC address\n");
+		slave_warn(bond_dev, slave_dev, "The slave device specified does not support setting the MAC address\n");
 		if (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP &&
 		    bond->params.fail_over_mac != BOND_FOM_ACTIVE) {
 			if (!bond_has_slaves(bond)) {
 				bond->params.fail_over_mac = BOND_FOM_ACTIVE;
-				netdev_warn(bond_dev, "Setting fail_over_mac to active for active-backup mode\n");
+				slave_warn(bond_dev, slave_dev, "Setting fail_over_mac to active for active-backup mode\n");
 			} else {
 				NL_SET_ERR_MSG(extack, "Slave device does not support setting the MAC address, but fail_over_mac is not set to active");
-				netdev_err(bond_dev, "The slave device specified does not support setting the MAC address, but fail_over_mac is not set to active\n");
+				slave_err(bond_dev, slave_dev, "The slave device specified does not support setting the MAC address, but fail_over_mac is not set to active\n");
 				res = -EOPNOTSUPP;
 				goto err_undo_flags;
 			}
@@ -1515,7 +1506,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	new_slave->original_mtu = slave_dev->mtu;
 	res = dev_set_mtu(slave_dev, bond->dev->mtu);
 	if (res) {
-		netdev_dbg(bond_dev, "Error %d calling dev_set_mtu\n", res);
+		slave_err(bond_dev, slave_dev, "Error %d calling dev_set_mtu\n", res);
 		goto err_free;
 	}
 
@@ -1536,7 +1527,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 		res = dev_set_mac_address(slave_dev, (struct sockaddr *)&ss,
 					  extack);
 		if (res) {
-			netdev_dbg(bond_dev, "Error %d calling set_mac_address\n", res);
+			slave_err(bond_dev, slave_dev, "Error %d calling set_mac_address\n", res);
 			goto err_restore_mtu;
 		}
 	}
@@ -1547,7 +1538,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	/* open the slave since the application closed it */
 	res = dev_open(slave_dev, extack);
 	if (res) {
-		netdev_dbg(bond_dev, "Opening slave %s failed\n", slave_dev->name);
+		slave_err(bond_dev, slave_dev, "Opening slave failed\n");
 		goto err_restore_mac;
 	}
 
@@ -1566,8 +1557,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 
 	res = vlan_vids_add_by_dev(slave_dev, bond_dev);
 	if (res) {
-		netdev_err(bond_dev, "Couldn't add bond vlan ids to %s\n",
-			   slave_dev->name);
+		slave_err(bond_dev, slave_dev, "Couldn't add bond vlan ids\n");
 		goto err_close;
 	}
 
@@ -1597,12 +1587,10 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 			 * supported); thus, we don't need to change
 			 * the messages for netif_carrier.
 			 */
-			netdev_warn(bond_dev, "MII and ETHTOOL support not available for interface %s, and arp_interval/arp_ip_target module parameters not specified, thus bonding will not detect link failures! see bonding.txt for details\n",
-				    slave_dev->name);
+			slave_warn(bond_dev, slave_dev, "MII and ETHTOOL support not available for slave, and arp_interval/arp_ip_target module parameters not specified, thus bonding will not detect link failures! see bonding.txt for details\n");
 		} else if (link_reporting == -1) {
 			/* unable get link status using mii/ethtool */
-			netdev_warn(bond_dev, "can't get link status from interface %s; the network driver associated with this interface does not support MII or ETHTOOL link status reporting, thus miimon has no effect on this interface\n",
-				    slave_dev->name);
+			slave_warn(bond_dev, slave_dev, "can't get link status from slave; the network driver associated with this interface does not support MII or ETHTOOL link status reporting, thus miimon has no effect on this interface\n");
 		}
 	}
 
@@ -1636,9 +1624,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 
 	if (new_slave->link != BOND_LINK_DOWN)
 		new_slave->last_link_up = jiffies;
-	netdev_dbg(bond_dev, "Initial state of slave_dev is BOND_LINK_%s\n",
-		   new_slave->link == BOND_LINK_DOWN ? "DOWN" :
-		   (new_slave->link == BOND_LINK_UP ? "UP" : "BACK"));
+	slave_dbg(bond_dev, slave_dev, "Initial state of slave is BOND_LINK_%s\n",
+		  new_slave->link == BOND_LINK_DOWN ? "DOWN" :
+		  (new_slave->link == BOND_LINK_UP ? "UP" : "BACK"));
 
 	if (bond_uses_primary(bond) && bond->params.primary[0]) {
 		/* if there is a primary slave, remember it */
@@ -1679,7 +1667,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 		bond_set_slave_inactive_flags(new_slave, BOND_SLAVE_NOTIFY_NOW);
 		break;
 	default:
-		netdev_dbg(bond_dev, "This slave is always active in trunk mode\n");
+		slave_dbg(bond_dev, slave_dev, "This slave is always active in trunk mode\n");
 
 		/* always active in trunk mode */
 		bond_set_active_slave(new_slave);
@@ -1698,7 +1686,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	if (bond->dev->npinfo) {
 		if (slave_enable_netpoll(new_slave)) {
-			netdev_info(bond_dev, "master_dev is using netpoll, but new slave device does not support netpoll\n");
+			slave_info(bond_dev, slave_dev, "master_dev is using netpoll, but new slave device does not support netpoll\n");
 			res = -EBUSY;
 			goto err_detach;
 		}
@@ -1711,19 +1699,19 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 	res = netdev_rx_handler_register(slave_dev, bond_handle_frame,
 					 new_slave);
 	if (res) {
-		netdev_dbg(bond_dev, "Error %d calling netdev_rx_handler_register\n", res);
+		slave_dbg(bond_dev, slave_dev, "Error %d calling netdev_rx_handler_register\n", res);
 		goto err_detach;
 	}
 
 	res = bond_master_upper_dev_link(bond, new_slave, extack);
 	if (res) {
-		netdev_dbg(bond_dev, "Error %d calling bond_master_upper_dev_link\n", res);
+		slave_dbg(bond_dev, slave_dev, "Error %d calling bond_master_upper_dev_link\n", res);
 		goto err_unregister;
 	}
 
 	res = bond_sysfs_slave_add(new_slave);
 	if (res) {
-		netdev_dbg(bond_dev, "Error %d calling bond_sysfs_slave_add\n", res);
+		slave_dbg(bond_dev, slave_dev, "Error %d calling bond_sysfs_slave_add\n", res);
 		goto err_upper_unlink;
 	}
 
@@ -1777,10 +1765,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
 		bond_update_slave_arr(bond, NULL);
 
 
-	netdev_info(bond_dev, "Enslaving %s as %s interface with %s link\n",
-		    slave_dev->name,
-		    bond_is_active_slave(new_slave) ? "an active" : "a backup",
-		    new_slave->link != BOND_LINK_DOWN ? "an up" : "a down");
+	slave_info(bond_dev, slave_dev, "Enslaving as %s interface with %s link\n",
+		   bond_is_active_slave(new_slave) ? "an active" : "a backup",
+		   new_slave->link != BOND_LINK_DOWN ? "an up" : "a down");
 
 	/* enslave is successful */
 	bond_queue_slave_event(new_slave);
@@ -1875,8 +1862,7 @@ static int __bond_release_one(struct net_device *bond_dev,
 	/* slave is not a slave or master is not master of this slave */
 	if (!(slave_dev->flags & IFF_SLAVE) ||
 	    !netdev_has_upper_dev(slave_dev, bond_dev)) {
-		netdev_dbg(bond_dev, "cannot release %s\n",
-			   slave_dev->name);
+		slave_dbg(bond_dev, slave_dev, "cannot release slave\n");
 		return -EINVAL;
 	}
 
@@ -1885,8 +1871,7 @@ static int __bond_release_one(struct net_device *bond_dev,
 	slave = bond_get_slave_by_dev(bond, slave_dev);
 	if (!slave) {
 		/* not a slave of this bond */
-		netdev_info(bond_dev, "%s not enslaved\n",
-			    slave_dev->name);
+		slave_info(bond_dev, slave_dev, "interface not enslaved\n");
 		unblock_netpoll_tx();
 		return -EINVAL;
 	}
@@ -1910,9 +1895,8 @@ static int __bond_release_one(struct net_device *bond_dev,
 	if (bond_mode_can_use_xmit_hash(bond))
 		bond_update_slave_arr(bond, slave);
 
-	netdev_info(bond_dev, "Releasing %s interface %s\n",
-		    bond_is_active_slave(slave) ? "active" : "backup",
-		    slave_dev->name);
+	slave_info(bond_dev, slave_dev, "Releasing %s interface\n",
+		    bond_is_active_slave(slave) ? "active" : "backup");
 
 	oldcurrent = rcu_access_pointer(bond->curr_active_slave);
 
@@ -1922,9 +1906,8 @@ static int __bond_release_one(struct net_device *bond_dev,
 		     BOND_MODE(bond) != BOND_MODE_ACTIVEBACKUP)) {
 		if (ether_addr_equal_64bits(bond_dev->dev_addr, slave->perm_hwaddr) &&
 		    bond_has_slaves(bond))
-			netdev_warn(bond_dev, "the permanent HWaddr of %s - %pM - is still in use by %s - set the HWaddr of %s to a different address to avoid conflicts\n",
-				    slave_dev->name, slave->perm_hwaddr,
-				    bond_dev->name, slave_dev->name);
+			slave_warn(bond_dev, slave_dev, "the permanent HWaddr of slave - %pM - is still in use by bond - set the HWaddr of slave to a different address to avoid conflicts\n",
+				   slave->perm_hwaddr);
 	}
 
 	if (rtnl_dereference(bond->primary_slave) == slave)
@@ -1972,8 +1955,7 @@ static int __bond_release_one(struct net_device *bond_dev,
 	bond_compute_features(bond);
 	if (!(bond_dev->features & NETIF_F_VLAN_CHALLENGED) &&
 	    (old_features & NETIF_F_VLAN_CHALLENGED))
-		netdev_info(bond_dev, "last VLAN challenged slave %s left bond %s - VLAN blocking is removed\n",
-			    slave_dev->name, bond_dev->name);
+		slave_info(bond_dev, slave_dev, "last VLAN challenged slave left bond - VLAN blocking is removed\n");
 
 	vlan_vids_del_by_dev(slave_dev, bond_dev);
 
@@ -2033,8 +2015,8 @@ int bond_release(struct net_device *bond_dev, struct net_device *slave_dev)
 /* First release a slave and then destroy the bond if no more slaves are left.
  * Must be under rtnl_lock when this function is called.
  */
-static int  bond_release_and_destroy(struct net_device *bond_dev,
-				     struct net_device *slave_dev)
+static int bond_release_and_destroy(struct net_device *bond_dev,
+				    struct net_device *slave_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 	int ret;
@@ -2042,8 +2024,7 @@ static int  bond_release_and_destroy(struct net_device *bond_dev,
 	ret = __bond_release_one(bond_dev, slave_dev, false, true);
 	if (ret == 0 && !bond_has_slaves(bond)) {
 		bond_dev->priv_flags |= IFF_DISABLE_NETPOLL;
-		netdev_info(bond_dev, "Destroying bond %s\n",
-			    bond_dev->name);
+		netdev_info(bond_dev, "Destroying bond\n");
 		bond_remove_proc_entry(bond);
 		unregister_netdevice(bond_dev);
 	}
@@ -2101,13 +2082,12 @@ static int bond_miimon_inspect(struct bonding *bond)
 			commit++;
 			slave->delay = bond->params.downdelay;
 			if (slave->delay) {
-				netdev_info(bond->dev, "link status down for %sinterface %s, disabling it in %d ms\n",
-					    (BOND_MODE(bond) ==
-					     BOND_MODE_ACTIVEBACKUP) ?
-					     (bond_is_active_slave(slave) ?
-					      "active " : "backup ") : "",
-					    slave->dev->name,
-					    bond->params.downdelay * bond->params.miimon);
+				slave_info(bond->dev, slave->dev, "link status down for %sinterface, disabling it in %d ms\n",
+					   (BOND_MODE(bond) ==
+					    BOND_MODE_ACTIVEBACKUP) ?
+					    (bond_is_active_slave(slave) ?
+					     "active " : "backup ") : "",
+					   bond->params.downdelay * bond->params.miimon);
 			}
 			/*FALLTHRU*/
 		case BOND_LINK_FAIL:
@@ -2115,10 +2095,9 @@ static int bond_miimon_inspect(struct bonding *bond)
 				/* recovered before downdelay expired */
 				bond_propose_link_state(slave, BOND_LINK_UP);
 				slave->last_link_up = jiffies;
-				netdev_info(bond->dev, "link status up again after %d ms for interface %s\n",
-					    (bond->params.downdelay - slave->delay) *
-					    bond->params.miimon,
-					    slave->dev->name);
+				slave_info(bond->dev, slave->dev, "link status up again after %d ms\n",
+					   (bond->params.downdelay - slave->delay) *
+					   bond->params.miimon);
 				commit++;
 				continue;
 			}
@@ -2141,20 +2120,18 @@ static int bond_miimon_inspect(struct bonding *bond)
 			slave->delay = bond->params.updelay;
 
 			if (slave->delay) {
-				netdev_info(bond->dev, "link status up for interface %s, enabling it in %d ms\n",
-					    slave->dev->name,
-					    ignore_updelay ? 0 :
-					    bond->params.updelay *
-					    bond->params.miimon);
+				slave_info(bond->dev, slave->dev, "link status up, enabling it in %d ms\n",
+					   ignore_updelay ? 0 :
+					   bond->params.updelay *
+					   bond->params.miimon);
 			}
 			/*FALLTHRU*/
 		case BOND_LINK_BACK:
 			if (!link_state) {
 				bond_propose_link_state(slave, BOND_LINK_DOWN);
-				netdev_info(bond->dev, "link status down again after %d ms for interface %s\n",
-					    (bond->params.updelay - slave->delay) *
-					    bond->params.miimon,
-					    slave->dev->name);
+				slave_info(bond->dev, slave->dev, "link status down again after %d ms\n",
+					   (bond->params.updelay - slave->delay) *
+					   bond->params.miimon);
 				commit++;
 				continue;
 			}
@@ -2210,9 +2187,8 @@ static void bond_miimon_commit(struct bonding *bond)
 			    bond_needs_speed_duplex(bond)) {
 				slave->link = BOND_LINK_DOWN;
 				if (net_ratelimit())
-					netdev_warn(bond->dev,
-						    "failed to get link speed/duplex for %s\n",
-						    slave->dev->name);
+					slave_warn(bond->dev, slave->dev,
+						   "failed to get link speed/duplex\n");
 				continue;
 			}
 			bond_set_slave_link_state(slave, BOND_LINK_UP,
@@ -2231,10 +2207,9 @@ static void bond_miimon_commit(struct bonding *bond)
 				bond_set_backup_slave(slave);
 			}
 
-			netdev_info(bond->dev, "link status definitely up for interface %s, %u Mbps %s duplex\n",
-				    slave->dev->name,
-				    slave->speed == SPEED_UNKNOWN ? 0 : slave->speed,
-				    slave->duplex ? "full" : "half");
+			slave_info(bond->dev, slave->dev, "link status definitely up, %u Mbps %s duplex\n",
+				   slave->speed == SPEED_UNKNOWN ? 0 : slave->speed,
+				   slave->duplex ? "full" : "half");
 
 			bond_miimon_link_change(bond, slave, BOND_LINK_UP);
 
@@ -2255,8 +2230,7 @@ static void bond_miimon_commit(struct bonding *bond)
 				bond_set_slave_inactive_flags(slave,
 							      BOND_SLAVE_NOTIFY_NOW);
 
-			netdev_info(bond->dev, "link status definitely down for interface %s, disabling it\n",
-				    slave->dev->name);
+			slave_info(bond->dev, slave->dev, "link status definitely down, disabling slave\n");
 
 			bond_miimon_link_change(bond, slave, BOND_LINK_DOWN);
 
@@ -2266,8 +2240,8 @@ static void bond_miimon_commit(struct bonding *bond)
 			continue;
 
 		default:
-			netdev_err(bond->dev, "invalid new link %d on slave %s\n",
-				   slave->new_link, slave->dev->name);
+			slave_err(bond->dev, slave->dev, "invalid new link %d on slave\n",
+				  slave->new_link);
 			slave->new_link = BOND_LINK_NOCHANGE;
 
 			continue;
@@ -2364,15 +2338,16 @@ static bool bond_has_this_ip(struct bonding *bond, __be32 ip)
  * switches in VLAN mode (especially if ports are configured as
  * "native" to a VLAN) might not pass non-tagged frames.
  */
-static void bond_arp_send(struct net_device *slave_dev, int arp_op,
-			  __be32 dest_ip, __be32 src_ip,
-			  struct bond_vlan_tag *tags)
+static void bond_arp_send(struct slave *slave, int arp_op, __be32 dest_ip,
+			  __be32 src_ip, struct bond_vlan_tag *tags)
 {
 	struct sk_buff *skb;
 	struct bond_vlan_tag *outer_tag = tags;
+	struct net_device *slave_dev = slave->dev;
+	struct net_device *bond_dev = slave->bond->dev;
 
-	netdev_dbg(slave_dev, "arp %d on slave %s: dst %pI4 src %pI4\n",
-		   arp_op, slave_dev->name, &dest_ip, &src_ip);
+	slave_dbg(bond_dev, slave_dev, "arp %d on slave: dst %pI4 src %pI4\n",
+		  arp_op, &dest_ip, &src_ip);
 
 	skb = arp_create(arp_op, ETH_P_ARP, dest_ip, slave_dev, src_ip,
 			 NULL, slave_dev->dev_addr, NULL);
@@ -2394,8 +2369,8 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op,
 			continue;
 		}
 
-		netdev_dbg(slave_dev, "inner tag: proto %X vid %X\n",
-			   ntohs(outer_tag->vlan_proto), tags->vlan_id);
+		slave_dbg(bond_dev, slave_dev, "inner tag: proto %X vid %X\n",
+			  ntohs(outer_tag->vlan_proto), tags->vlan_id);
 		skb = vlan_insert_tag_set_proto(skb, tags->vlan_proto,
 						tags->vlan_id);
 		if (!skb) {
@@ -2407,8 +2382,8 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op,
 	}
 	/* Set the outer tag */
 	if (outer_tag->vlan_id) {
-		netdev_dbg(slave_dev, "outer tag: proto %X vid %X\n",
-			   ntohs(outer_tag->vlan_proto), outer_tag->vlan_id);
+		slave_dbg(bond_dev, slave_dev, "outer tag: proto %X vid %X\n",
+			  ntohs(outer_tag->vlan_proto), outer_tag->vlan_id);
 		__vlan_hwaccel_put_tag(skb, outer_tag->vlan_proto,
 				       outer_tag->vlan_id);
 	}
@@ -2465,7 +2440,8 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 	int i;
 
 	for (i = 0; i < BOND_MAX_ARP_TARGETS && targets[i]; i++) {
-		netdev_dbg(bond->dev, "basa: target %pI4\n", &targets[i]);
+		slave_dbg(bond->dev, slave->dev, "%s: target %pI4\n",
+			  __func__, &targets[i]);
 		tags = NULL;
 
 		/* Find out through which dev should the packet go */
@@ -2479,7 +2455,7 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 				net_warn_ratelimited("%s: no route to arp_ip_target %pI4 and arp_validate is set\n",
 						     bond->dev->name,
 						     &targets[i]);
-			bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
+			bond_arp_send(slave, ARPOP_REQUEST, targets[i],
 				      0, tags);
 			continue;
 		}
@@ -2496,7 +2472,7 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 			goto found;
 
 		/* Not our device - skip */
-		netdev_dbg(bond->dev, "no path to arp_ip_target %pI4 via rt.dev %s\n",
+		slave_dbg(bond->dev, slave->dev, "no path to arp_ip_target %pI4 via rt.dev %s\n",
 			   &targets[i], rt->dst.dev ? rt->dst.dev->name : "NULL");
 
 		ip_rt_put(rt);
@@ -2505,8 +2481,7 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 found:
 		addr = bond_confirm_addr(rt->dst.dev, targets[i], 0);
 		ip_rt_put(rt);
-		bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
-			      addr, tags);
+		bond_arp_send(slave, ARPOP_REQUEST, targets[i], addr, tags);
 		kfree(tags);
 	}
 }
@@ -2516,15 +2491,15 @@ static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32
 	int i;
 
 	if (!sip || !bond_has_this_ip(bond, tip)) {
-		netdev_dbg(bond->dev, "bva: sip %pI4 tip %pI4 not found\n",
-			   &sip, &tip);
+		slave_dbg(bond->dev, slave->dev, "%s: sip %pI4 tip %pI4 not found\n",
+			   __func__, &sip, &tip);
 		return;
 	}
 
 	i = bond_get_targets_ip(bond->params.arp_targets, sip);
 	if (i == -1) {
-		netdev_dbg(bond->dev, "bva: sip %pI4 not found in targets\n",
-			   &sip);
+		slave_dbg(bond->dev, slave->dev, "%s: sip %pI4 not found in targets\n",
+			   __func__, &sip);
 		return;
 	}
 	slave->last_rx = jiffies;
@@ -2552,8 +2527,8 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
 
 	alen = arp_hdr_len(bond->dev);
 
-	netdev_dbg(bond->dev, "bond_arp_rcv: skb->dev %s\n",
-		   skb->dev->name);
+	slave_dbg(bond->dev, slave->dev, "%s: skb->dev %s\n",
+		   __func__, skb->dev->name);
 
 	if (alen > skb_headlen(skb)) {
 		arp = kmalloc(alen, GFP_ATOMIC);
@@ -2577,10 +2552,10 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
 	arp_ptr += 4 + bond->dev->addr_len;
 	memcpy(&tip, arp_ptr, 4);
 
-	netdev_dbg(bond->dev, "bond_arp_rcv: %s/%d av %d sv %d sip %pI4 tip %pI4\n",
-		   slave->dev->name, bond_slave_state(slave),
-		     bond->params.arp_validate, slave_do_arp_validate(bond, slave),
-		     &sip, &tip);
+	slave_dbg(bond->dev, slave->dev, "%s: %s/%d av %d sv %d sip %pI4 tip %pI4\n",
+		  __func__, slave->dev->name, bond_slave_state(slave),
+		  bond->params.arp_validate, slave_do_arp_validate(bond, slave),
+		  &sip, &tip);
 
 	curr_active_slave = rcu_dereference(bond->curr_active_slave);
 	curr_arp_slave = rcu_dereference(bond->current_arp_slave);
@@ -2683,12 +2658,10 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
 				 * is closed.
 				 */
 				if (!oldcurrent) {
-					netdev_info(bond->dev, "link status definitely up for interface %s\n",
-						    slave->dev->name);
+					slave_info(bond->dev, slave->dev, "link status definitely up\n");
 					do_failover = 1;
 				} else {
-					netdev_info(bond->dev, "interface %s is now up\n",
-						    slave->dev->name);
+					slave_info(bond->dev, slave->dev, "interface is now up\n");
 				}
 			}
 		} else {
@@ -2707,8 +2680,7 @@ static void bond_loadbalance_arp_mon(struct bonding *bond)
 				if (slave->link_failure_count < UINT_MAX)
 					slave->link_failure_count++;
 
-				netdev_info(bond->dev, "interface %s is now down\n",
-					    slave->dev->name);
+				slave_info(bond->dev, slave->dev, "interface is now down\n");
 
 				if (slave == oldcurrent)
 					do_failover = 1;
@@ -2858,8 +2830,7 @@ static void bond_ab_arp_commit(struct bonding *bond)
 					RCU_INIT_POINTER(bond->current_arp_slave, NULL);
 				}
 
-				netdev_info(bond->dev, "link status definitely up for interface %s\n",
-					    slave->dev->name);
+				slave_info(bond->dev, slave->dev, "link status definitely up\n");
 
 				if (!rtnl_dereference(bond->curr_active_slave) ||
 				    slave == rtnl_dereference(bond->primary_slave))
@@ -2878,8 +2849,7 @@ static void bond_ab_arp_commit(struct bonding *bond)
 			bond_set_slave_inactive_flags(slave,
 						      BOND_SLAVE_NOTIFY_NOW);
 
-			netdev_info(bond->dev, "link status definitely down for interface %s, disabling it\n",
-				    slave->dev->name);
+			slave_info(bond->dev, slave->dev, "link status definitely down, disabling slave\n");
 
 			if (slave == rtnl_dereference(bond->curr_active_slave)) {
 				RCU_INIT_POINTER(bond->current_arp_slave, NULL);
@@ -2889,8 +2859,8 @@ static void bond_ab_arp_commit(struct bonding *bond)
 			continue;
 
 		default:
-			netdev_err(bond->dev, "impossible: new_link %d on slave %s\n",
-				   slave->new_link, slave->dev->name);
+			slave_err(bond->dev, slave->dev, "impossible: new_link %d on slave\n",
+				  slave->new_link);
 			continue;
 		}
 
@@ -2961,8 +2931,7 @@ static bool bond_ab_arp_probe(struct bonding *bond)
 			bond_set_slave_inactive_flags(slave,
 						      BOND_SLAVE_NOTIFY_LATER);
 
-			netdev_info(bond->dev, "backup interface %s is now down\n",
-				    slave->dev->name);
+			slave_info(bond->dev, slave->dev, "backup interface is now down\n");
 		}
 		if (slave == curr_arp_slave)
 			found = true;
@@ -3074,6 +3043,8 @@ static int bond_master_netdev_event(unsigned long event,
 {
 	struct bonding *event_bond = netdev_priv(bond_dev);
 
+	netdev_dbg(bond_dev, "%s called\n", __func__);
+
 	switch (event) {
 	case NETDEV_CHANGENAME:
 		return bond_event_changename(event_bond);
@@ -3105,12 +3076,17 @@ static int bond_slave_netdev_event(unsigned long event,
 	 * before netdev_rx_handler_register is called in which case
 	 * slave will be NULL
 	 */
-	if (!slave)
+	if (!slave) {
+		netdev_dbg(slave_dev, "%s called on NULL slave\n", __func__);
 		return NOTIFY_DONE;
+	}
+
 	bond_dev = slave->bond->dev;
 	bond = slave->bond;
 	primary = rtnl_dereference(bond->primary_slave);
 
+	slave_dbg(bond_dev, slave_dev, "%s called\n", __func__);
+
 	switch (event) {
 	case NETDEV_UNREGISTER:
 		if (bond_dev->type != ARPHRD_ETHER)
@@ -3221,16 +3197,13 @@ static int bond_netdev_event(struct notifier_block *this,
 	if (event_dev->flags & IFF_MASTER) {
 		int ret;
 
-		netdev_dbg(event_dev, "IFF_MASTER\n");
 		ret = bond_master_netdev_event(event, event_dev);
 		if (ret != NOTIFY_DONE)
 			return ret;
 	}
 
-	if (event_dev->flags & IFF_SLAVE) {
-		netdev_dbg(event_dev, "IFF_SLAVE\n");
+	if (event_dev->flags & IFF_SLAVE)
 		return bond_slave_netdev_event(event, event_dev);
-	}
 
 	return NOTIFY_DONE;
 }
@@ -3547,12 +3520,11 @@ static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd
 
 	slave_dev = __dev_get_by_name(net, ifr->ifr_slave);
 
-	netdev_dbg(bond_dev, "slave_dev=%p:\n", slave_dev);
+	slave_dbg(bond_dev, slave_dev, "slave_dev=%p:\n", slave_dev);
 
 	if (!slave_dev)
 		return -ENODEV;
 
-	netdev_dbg(bond_dev, "slave_dev->name=%s:\n", slave_dev->name);
 	switch (cmd) {
 	case BOND_ENSLAVE_OLD:
 	case SIOCBONDENSLAVE:
@@ -3677,7 +3649,7 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu)
 	netdev_dbg(bond_dev, "bond=%p, new_mtu=%d\n", bond, new_mtu);
 
 	bond_for_each_slave(bond, slave, iter) {
-		netdev_dbg(bond_dev, "s %p c_m %p\n",
+		slave_dbg(bond_dev, slave->dev, "s %p c_m %p\n",
 			   slave, slave->dev->netdev_ops->ndo_change_mtu);
 
 		res = dev_set_mtu(slave->dev, new_mtu);
@@ -3691,8 +3663,8 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu)
 			 * means changing their mtu from timer context, which
 			 * is probably not a good idea.
 			 */
-			netdev_dbg(bond_dev, "err %d %s\n", res,
-				   slave->dev->name);
+			slave_dbg(bond_dev, slave->dev, "err %d setting mtu to %d\n",
+				  res, new_mtu);
 			goto unwind;
 		}
 	}
@@ -3710,10 +3682,9 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu)
 			break;
 
 		tmp_res = dev_set_mtu(rollback_slave->dev, bond_dev->mtu);
-		if (tmp_res) {
-			netdev_dbg(bond_dev, "unwind err %d dev %s\n",
-				   tmp_res, rollback_slave->dev->name);
-		}
+		if (tmp_res)
+			slave_dbg(bond_dev, rollback_slave->dev, "unwind err %d\n",
+				  tmp_res);
 	}
 
 	return res;
@@ -3737,7 +3708,7 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
 		return bond_alb_set_mac_address(bond_dev, addr);
 
 
-	netdev_dbg(bond_dev, "bond=%p\n", bond);
+	netdev_dbg(bond_dev, "%s: bond=%p\n", __func__, bond);
 
 	/* If fail_over_mac is enabled, do nothing and return success.
 	 * Returning an error causes ifenslave to fail.
@@ -3750,7 +3721,8 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
 		return -EADDRNOTAVAIL;
 
 	bond_for_each_slave(bond, slave, iter) {
-		netdev_dbg(bond_dev, "slave %p %s\n", slave, slave->dev->name);
+		slave_dbg(bond_dev, slave->dev, "%s: slave=%p\n",
+			  __func__, slave);
 		res = dev_set_mac_address(slave->dev, addr, NULL);
 		if (res) {
 			/* TODO: consider downing the slave
@@ -3759,7 +3731,8 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
 			 * breakage anyway until ARP finish
 			 * updating, so...
 			 */
-			netdev_dbg(bond_dev, "err %d %s\n", res, slave->dev->name);
+			slave_dbg(bond_dev, slave->dev, "%s: err %d\n",
+				  __func__, res);
 			goto unwind;
 		}
 	}
@@ -3782,8 +3755,8 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
 		tmp_res = dev_set_mac_address(rollback_slave->dev,
 					      (struct sockaddr *)&tmp_ss, NULL);
 		if (tmp_res) {
-			netdev_dbg(bond_dev, "unwind err %d dev %s\n",
-				   tmp_res, rollback_slave->dev->name);
+			slave_dbg(bond_dev, rollback_slave->dev, "%s: unwind err %d\n",
+				   __func__, tmp_res);
 		}
 	}
 
@@ -4004,9 +3977,8 @@ int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
 		if (skipslave == slave)
 			continue;
 
-		netdev_dbg(bond->dev,
-			   "Adding slave dev %s to tx hash array[%d]\n",
-			   slave->dev->name, new_arr->count);
+		slave_dbg(bond->dev, slave->dev, "Adding slave to tx hash array[%d]\n",
+			  new_arr->count);
 
 		new_arr->arr[new_arr->count++] = slave;
 	}
-- 
2.20.1

[PATCH net-next 5/7] bonding/802.3ad: convert to using slave printk macros

[Jarod Wilson]

All of these printk instances benefit from having both master and slave
device information included, so convert to using a standardized macro
format and remove redundant information.

Suggested-by: Joe Perches <joe@perches.com>
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/net/bonding/bond_3ad.c | 222 +++++++++++++++++----------------
 1 file changed, 116 insertions(+), 106 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index dfd6f315d2cc..e3b25f310936 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -325,17 +325,17 @@ static u16 __get_link_speed(struct port *port)
 		default:
 			/* unknown speed value from ethtool. shouldn't happen */
 			if (slave->speed != SPEED_UNKNOWN)
-				pr_warn_once("%s: unknown ethtool speed (%d) for port %d (set it to 0)\n",
+				pr_warn_once("%s: (slave %s): unknown ethtool speed (%d) for port %d (set it to 0)\n",
 					     slave->bond->dev->name,
-					     slave->speed,
+					     slave->dev->name, slave->speed,
 					     port->actor_port_number);
 			speed = 0;
 			break;
 		}
 	}
 
-	netdev_dbg(slave->bond->dev, "Port %d Received link speed %d update from adapter\n",
-		   port->actor_port_number, speed);
+	slave_dbg(slave->bond->dev, slave->dev, "Port %d Received link speed %d update from adapter\n",
+		  port->actor_port_number, speed);
 	return speed;
 }
 
@@ -359,14 +359,14 @@ static u8 __get_duplex(struct port *port)
 		switch (slave->duplex) {
 		case DUPLEX_FULL:
 			retval = 0x1;
-			netdev_dbg(slave->bond->dev, "Port %d Received status full duplex update from adapter\n",
-				   port->actor_port_number);
+			slave_dbg(slave->bond->dev, slave->dev, "Port %d Received status full duplex update from adapter\n",
+				  port->actor_port_number);
 			break;
 		case DUPLEX_HALF:
 		default:
 			retval = 0x0;
-			netdev_dbg(slave->bond->dev, "Port %d Received status NOT full duplex update from adapter\n",
-				   port->actor_port_number);
+			slave_dbg(slave->bond->dev, slave->dev, "Port %d Received status NOT full duplex update from adapter\n",
+				  port->actor_port_number);
 			break;
 		}
 	}
@@ -500,10 +500,12 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 		if ((port->sm_vars & AD_PORT_MATCHED) &&
 		    (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION)) {
 			partner->port_state |= AD_STATE_SYNCHRONIZATION;
-			pr_debug("%s partner sync=1\n", port->slave->dev->name);
+			slave_dbg(port->slave->bond->dev, port->slave->dev,
+				  "partner sync=1\n");
 		} else {
 			partner->port_state &= ~AD_STATE_SYNCHRONIZATION;
-			pr_debug("%s partner sync=0\n", port->slave->dev->name);
+			slave_dbg(port->slave->bond->dev, port->slave->dev,
+				  "partner sync=0\n");
 		}
 	}
 }
@@ -789,8 +791,9 @@ static inline void __update_lacpdu_from_port(struct port *port)
 	lacpdu->actor_port_priority = htons(port->actor_port_priority);
 	lacpdu->actor_port = htons(port->actor_port_number);
 	lacpdu->actor_state = port->actor_oper_port_state;
-	pr_debug("update lacpdu: %s, actor port state %x\n",
-		 port->slave->dev->name, port->actor_oper_port_state);
+	slave_dbg(port->slave->bond->dev, port->slave->dev,
+		  "update lacpdu: actor port state %x\n",
+		  port->actor_oper_port_state);
 
 	/* lacpdu->reserved_3_1              initialized
 	 * lacpdu->tlv_type_partner_info     initialized
@@ -1022,11 +1025,11 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
 
 	/* check if the state machine was changed */
 	if (port->sm_mux_state != last_state) {
-		pr_debug("Mux Machine: Port=%d (%s), Last State=%d, Curr State=%d\n",
-			 port->actor_port_number,
-			 port->slave->dev->name,
-			 last_state,
-			 port->sm_mux_state);
+		slave_dbg(port->slave->bond->dev, port->slave->dev,
+			  "Mux Machine: Port=%d, Last State=%d, Curr State=%d\n",
+			  port->actor_port_number,
+			  last_state,
+			  port->sm_mux_state);
 		switch (port->sm_mux_state) {
 		case AD_MUX_DETACHED:
 			port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION;
@@ -1140,11 +1143,11 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 
 	/* check if the State machine was changed or new lacpdu arrived */
 	if ((port->sm_rx_state != last_state) || (lacpdu)) {
-		pr_debug("Rx Machine: Port=%d (%s), Last State=%d, Curr State=%d\n",
-			 port->actor_port_number,
-			 port->slave->dev->name,
-			 last_state,
-			 port->sm_rx_state);
+		slave_dbg(port->slave->bond->dev, port->slave->dev,
+			  "Rx Machine: Port=%d, Last State=%d, Curr State=%d\n",
+			  port->actor_port_number,
+			  last_state,
+			  port->sm_rx_state);
 		switch (port->sm_rx_state) {
 		case AD_RX_INITIALIZE:
 			if (!(port->actor_oper_port_key & AD_DUPLEX_KEY_MASKS))
@@ -1192,9 +1195,8 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			/* detect loopback situation */
 			if (MAC_ADDRESS_EQUAL(&(lacpdu->actor_system),
 					      &(port->actor_system))) {
-				netdev_err(port->slave->bond->dev, "An illegal loopback occurred on adapter (%s)\n"
-				       "Check the configuration to verify that all adapters are connected to 802.3ad compliant switch ports\n",
-				       port->slave->dev->name);
+				slave_err(port->slave->bond->dev, port->slave->dev, "An illegal loopback occurred on slave\n"
+					  "Check the configuration to verify that all adapters are connected to 802.3ad compliant switch ports\n");
 				return;
 			}
 			__update_selected(lacpdu, port);
@@ -1263,8 +1265,10 @@ static void ad_tx_machine(struct port *port)
 			__update_lacpdu_from_port(port);
 
 			if (ad_lacpdu_send(port) >= 0) {
-				pr_debug("Sent LACPDU on port %d\n",
-					 port->actor_port_number);
+				slave_dbg(port->slave->bond->dev,
+					  port->slave->dev,
+					  "Sent LACPDU on port %d\n",
+					  port->actor_port_number);
 
 				/* mark ntt as false, so it will not be sent
 				 * again until demanded
@@ -1343,9 +1347,10 @@ static void ad_periodic_machine(struct port *port)
 
 	/* check if the state machine was changed */
 	if (port->sm_periodic_state != last_state) {
-		pr_debug("Periodic Machine: Port=%d, Last State=%d, Curr State=%d\n",
-			 port->actor_port_number, last_state,
-			 port->sm_periodic_state);
+		slave_dbg(port->slave->bond->dev, port->slave->dev,
+			  "Periodic Machine: Port=%d, Last State=%d, Curr State=%d\n",
+			  port->actor_port_number, last_state,
+			  port->sm_periodic_state);
 		switch (port->sm_periodic_state) {
 		case AD_NO_PERIODIC:
 			port->sm_periodic_timer_counter = 0;
@@ -1421,9 +1426,9 @@ static void ad_port_selection_logic(struct port *port, bool *update_slave_arr)
 				port->next_port_in_aggregator = NULL;
 				port->actor_port_aggregator_identifier = 0;
 
-				netdev_dbg(bond->dev, "Port %d left LAG %d\n",
-					   port->actor_port_number,
-					   temp_aggregator->aggregator_identifier);
+				slave_dbg(bond->dev, port->slave->dev, "Port %d left LAG %d\n",
+					  port->actor_port_number,
+					  temp_aggregator->aggregator_identifier);
 				/* if the aggregator is empty, clear its
 				 * parameters, and set it ready to be attached
 				 */
@@ -1436,10 +1441,10 @@ static void ad_port_selection_logic(struct port *port, bool *update_slave_arr)
 			/* meaning: the port was related to an aggregator
 			 * but was not on the aggregator port list
 			 */
-			net_warn_ratelimited("%s: Warning: Port %d (on %s) was related to aggregator %d but was not on its port list\n",
+			net_warn_ratelimited("%s: (slave %s): Warning: Port %d was related to aggregator %d but was not on its port list\n",
 					     port->slave->bond->dev->name,
-					     port->actor_port_number,
 					     port->slave->dev->name,
+					     port->actor_port_number,
 					     port->aggregator->aggregator_identifier);
 		}
 	}
@@ -1470,9 +1475,9 @@ static void ad_port_selection_logic(struct port *port, bool *update_slave_arr)
 			port->next_port_in_aggregator = aggregator->lag_ports;
 			port->aggregator->num_of_ports++;
 			aggregator->lag_ports = port;
-			netdev_dbg(bond->dev, "Port %d joined LAG %d(existing LAG)\n",
-				   port->actor_port_number,
-				   port->aggregator->aggregator_identifier);
+			slave_dbg(bond->dev, slave->dev, "Port %d joined LAG %d (existing LAG)\n",
+				  port->actor_port_number,
+				  port->aggregator->aggregator_identifier);
 
 			/* mark this port as selected */
 			port->sm_vars |= AD_PORT_SELECTED;
@@ -1517,12 +1522,13 @@ static void ad_port_selection_logic(struct port *port, bool *update_slave_arr)
 			/* mark this port as selected */
 			port->sm_vars |= AD_PORT_SELECTED;
 
-			netdev_dbg(bond->dev, "Port %d joined LAG %d(new LAG)\n",
-				   port->actor_port_number,
-				   port->aggregator->aggregator_identifier);
+			slave_dbg(bond->dev, port->slave->dev, "Port %d joined LAG %d (new LAG)\n",
+				  port->actor_port_number,
+				  port->aggregator->aggregator_identifier);
 		} else {
-			netdev_err(bond->dev, "Port %d (on %s) did not find a suitable aggregator\n",
-			       port->actor_port_number, port->slave->dev->name);
+			slave_err(bond->dev, port->slave->dev,
+				  "Port %d did not find a suitable aggregator\n",
+				  port->actor_port_number);
 		}
 	}
 	/* if all aggregator's ports are READY_N == TRUE, set ready=TRUE
@@ -1601,8 +1607,9 @@ static struct aggregator *ad_agg_selection_test(struct aggregator *best,
 		break;
 
 	default:
-		net_warn_ratelimited("%s: Impossible agg select mode %d\n",
+		net_warn_ratelimited("%s: (slave %s): Impossible agg select mode %d\n",
 				     curr->slave->bond->dev->name,
+				     curr->slave->dev->name,
 				     __get_agg_selection_mode(curr->lag_ports));
 		break;
 	}
@@ -1703,36 +1710,37 @@ static void ad_agg_selection_logic(struct aggregator *agg,
 
 	/* if there is new best aggregator, activate it */
 	if (best) {
-		netdev_dbg(bond->dev, "best Agg=%d; P=%d; a k=%d; p k=%d; Ind=%d; Act=%d\n",
+		netdev_dbg(bond->dev, "(slave %s): best Agg=%d; P=%d; a k=%d; p k=%d; Ind=%d; Act=%d\n",
+			   best->slave ? best->slave->dev->name : "NULL",
 			   best->aggregator_identifier, best->num_of_ports,
 			   best->actor_oper_aggregator_key,
 			   best->partner_oper_aggregator_key,
 			   best->is_individual, best->is_active);
-		netdev_dbg(bond->dev, "best ports %p slave %p %s\n",
-			   best->lag_ports, best->slave,
-			   best->slave ? best->slave->dev->name : "NULL");
+		netdev_dbg(bond->dev, "(slave %s): best ports %p slave %p\n",
+			   best->slave ? best->slave->dev->name : "NULL",
+			   best->lag_ports, best->slave);
 
 		bond_for_each_slave_rcu(bond, slave, iter) {
 			agg = &(SLAVE_AD_INFO(slave)->aggregator);
 
-			netdev_dbg(bond->dev, "Agg=%d; P=%d; a k=%d; p k=%d; Ind=%d; Act=%d\n",
-				   agg->aggregator_identifier, agg->num_of_ports,
-				   agg->actor_oper_aggregator_key,
-				   agg->partner_oper_aggregator_key,
-				   agg->is_individual, agg->is_active);
+			slave_dbg(bond->dev, slave->dev, "Agg=%d; P=%d; a k=%d; p k=%d; Ind=%d; Act=%d\n",
+				  agg->aggregator_identifier, agg->num_of_ports,
+				  agg->actor_oper_aggregator_key,
+				  agg->partner_oper_aggregator_key,
+				  agg->is_individual, agg->is_active);
 		}
 
-		/* check if any partner replys */
-		if (best->is_individual) {
+		/* check if any partner replies */
+		if (best->is_individual)
 			net_warn_ratelimited("%s: Warning: No 802.3ad response from the link partner for any adapters in the bond\n",
-					     best->slave ?
-					     best->slave->bond->dev->name : "NULL");
-		}
+					     bond->dev->name);
 
 		best->is_active = 1;
-		netdev_dbg(bond->dev, "LAG %d chosen as the active LAG\n",
+		netdev_dbg(bond->dev, "(slave %s): LAG %d chosen as the active LAG\n",
+			   best->slave ? best->slave->dev->name : "NULL",
 			   best->aggregator_identifier);
-		netdev_dbg(bond->dev, "Agg=%d; P=%d; a k=%d; p k=%d; Ind=%d; Act=%d\n",
+		netdev_dbg(bond->dev, "(slave %s): Agg=%d; P=%d; a k=%d; p k=%d; Ind=%d; Act=%d\n",
+			   best->slave ? best->slave->dev->name : "NULL",
 			   best->aggregator_identifier, best->num_of_ports,
 			   best->actor_oper_aggregator_key,
 			   best->partner_oper_aggregator_key,
@@ -1788,7 +1796,9 @@ static void ad_clear_agg(struct aggregator *aggregator)
 		aggregator->lag_ports = NULL;
 		aggregator->is_active = 0;
 		aggregator->num_of_ports = 0;
-		pr_debug("LAG %d was cleared\n",
+		pr_debug("%s: LAG %d was cleared\n",
+			 aggregator->slave ?
+			 aggregator->slave->dev->name : "NULL",
 			 aggregator->aggregator_identifier);
 	}
 }
@@ -1885,9 +1895,10 @@ static void ad_enable_collecting_distributing(struct port *port,
 					      bool *update_slave_arr)
 {
 	if (port->aggregator->is_active) {
-		pr_debug("Enabling port %d(LAG %d)\n",
-			 port->actor_port_number,
-			 port->aggregator->aggregator_identifier);
+		slave_dbg(port->slave->bond->dev, port->slave->dev,
+			  "Enabling port %d (LAG %d)\n",
+			  port->actor_port_number,
+			  port->aggregator->aggregator_identifier);
 		__enable_port(port);
 		/* Slave array needs update */
 		*update_slave_arr = true;
@@ -1905,9 +1916,10 @@ static void ad_disable_collecting_distributing(struct port *port,
 	if (port->aggregator &&
 	    !MAC_ADDRESS_EQUAL(&(port->aggregator->partner_system),
 			       &(null_mac_addr))) {
-		pr_debug("Disabling port %d(LAG %d)\n",
-			 port->actor_port_number,
-			 port->aggregator->aggregator_identifier);
+		slave_dbg(port->slave->bond->dev, port->slave->dev,
+			  "Disabling port %d (LAG %d)\n",
+			  port->actor_port_number,
+			  port->aggregator->aggregator_identifier);
 		__disable_port(port);
 		/* Slave array needs an update */
 		*update_slave_arr = true;
@@ -1920,7 +1932,7 @@ static void ad_disable_collecting_distributing(struct port *port,
  * @port: the port we're looking at
  */
 static void ad_marker_info_received(struct bond_marker *marker_info,
-	struct port *port)
+				    struct port *port)
 {
 	struct bond_marker marker;
 
@@ -1933,10 +1945,10 @@ static void ad_marker_info_received(struct bond_marker *marker_info,
 	marker.tlv_type = AD_MARKER_RESPONSE_SUBTYPE;
 
 	/* send the marker response */
-	if (ad_marker_send(port, &marker) >= 0) {
-		pr_debug("Sent Marker Response on port %d\n",
-			 port->actor_port_number);
-	}
+	if (ad_marker_send(port, &marker) >= 0)
+		slave_dbg(port->slave->bond->dev, port->slave->dev,
+			  "Sent Marker Response on port %d\n",
+			  port->actor_port_number);
 }
 
 /**
@@ -2085,13 +2097,12 @@ void bond_3ad_unbind_slave(struct slave *slave)
 
 	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
-		netdev_warn(bond->dev, "Trying to unbind an uninitialized port on %s\n",
-			    slave->dev->name);
+		slave_warn(bond->dev, slave->dev, "Trying to unbind an uninitialized port\n");
 		goto out;
 	}
 
-	netdev_dbg(bond->dev, "Unbinding Link Aggregation Group %d\n",
-		   aggregator->aggregator_identifier);
+	slave_dbg(bond->dev, slave->dev, "Unbinding Link Aggregation Group %d\n",
+		  aggregator->aggregator_identifier);
 
 	/* Tell the partner that this port is not suitable for aggregation */
 	port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION;
@@ -2129,13 +2140,13 @@ void bond_3ad_unbind_slave(struct slave *slave)
 			 * new aggregator
 			 */
 			if ((new_aggregator) && ((!new_aggregator->lag_ports) || ((new_aggregator->lag_ports == port) && !new_aggregator->lag_ports->next_port_in_aggregator))) {
-				netdev_dbg(bond->dev, "Some port(s) related to LAG %d - replacing with LAG %d\n",
-					   aggregator->aggregator_identifier,
-					   new_aggregator->aggregator_identifier);
+				slave_dbg(bond->dev, slave->dev, "Some port(s) related to LAG %d - replacing with LAG %d\n",
+					  aggregator->aggregator_identifier,
+					  new_aggregator->aggregator_identifier);
 
 				if ((new_aggregator->lag_ports == port) &&
 				    new_aggregator->is_active) {
-					netdev_info(bond->dev, "Removing an active aggregator\n");
+					slave_info(bond->dev, slave->dev, "Removing an active aggregator\n");
 					select_new_active_agg = 1;
 				}
 
@@ -2166,7 +2177,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 					ad_agg_selection_logic(__get_first_agg(port),
 							       &dummy_slave_update);
 			} else {
-				netdev_warn(bond->dev, "unbinding aggregator, and could not find a new aggregator for its ports\n");
+				slave_warn(bond->dev, slave->dev, "unbinding aggregator, and could not find a new aggregator for its ports\n");
 			}
 		} else {
 			/* in case that the only port related to this
@@ -2175,7 +2186,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 			select_new_active_agg = aggregator->is_active;
 			ad_clear_agg(aggregator);
 			if (select_new_active_agg) {
-				netdev_info(bond->dev, "Removing an active aggregator\n");
+				slave_info(bond->dev, slave->dev, "Removing an active aggregator\n");
 				/* select new active aggregator */
 				temp_aggregator = __get_first_agg(port);
 				if (temp_aggregator)
@@ -2185,7 +2196,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 		}
 	}
 
-	netdev_dbg(bond->dev, "Unbinding port %d\n", port->actor_port_number);
+	slave_dbg(bond->dev, slave->dev, "Unbinding port %d\n", port->actor_port_number);
 
 	/* find the aggregator that this port is connected to */
 	bond_for_each_slave(bond, slave_iter, iter) {
@@ -2208,7 +2219,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 					select_new_active_agg = temp_aggregator->is_active;
 					ad_clear_agg(temp_aggregator);
 					if (select_new_active_agg) {
-						netdev_info(bond->dev, "Removing an active aggregator\n");
+						slave_info(bond->dev, slave->dev, "Removing an active aggregator\n");
 						/* select new active aggregator */
 						ad_agg_selection_logic(__get_first_agg(port),
 							               &dummy_slave_update);
@@ -2379,9 +2390,9 @@ static int bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave)
 	switch (lacpdu->subtype) {
 	case AD_TYPE_LACPDU:
 		ret = RX_HANDLER_CONSUMED;
-		netdev_dbg(slave->bond->dev,
-			   "Received LACPDU on port %d slave %s\n",
-			   port->actor_port_number, slave->dev->name);
+		slave_dbg(slave->bond->dev, slave->dev,
+			  "Received LACPDU on port %d\n",
+			  port->actor_port_number);
 		/* Protect against concurrent state machines */
 		spin_lock(&slave->bond->mode_lock);
 		ad_rx_machine(lacpdu, port);
@@ -2395,18 +2406,18 @@ static int bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave)
 		marker = (struct bond_marker *)lacpdu;
 		switch (marker->tlv_type) {
 		case AD_MARKER_INFORMATION_SUBTYPE:
-			netdev_dbg(slave->bond->dev, "Received Marker Information on port %d\n",
-				   port->actor_port_number);
+			slave_dbg(slave->bond->dev, slave->dev, "Received Marker Information on port %d\n",
+				  port->actor_port_number);
 			ad_marker_info_received(marker, port);
 			break;
 		case AD_MARKER_RESPONSE_SUBTYPE:
-			netdev_dbg(slave->bond->dev, "Received Marker Response on port %d\n",
-				   port->actor_port_number);
+			slave_dbg(slave->bond->dev, slave->dev, "Received Marker Response on port %d\n",
+				  port->actor_port_number);
 			ad_marker_response_received(marker, port);
 			break;
 		default:
-			netdev_dbg(slave->bond->dev, "Received an unknown Marker subtype on slot %d\n",
-				   port->actor_port_number);
+			slave_dbg(slave->bond->dev, slave->dev, "Received an unknown Marker subtype on port %d\n",
+				  port->actor_port_number);
 			stat = &SLAVE_AD_INFO(slave)->stats.marker_unknown_rx;
 			atomic64_inc(stat);
 			stat = &BOND_AD_INFO(bond).stats.marker_unknown_rx;
@@ -2456,9 +2467,10 @@ static void ad_update_actor_keys(struct port *port, bool reset)
 
 		if (!reset) {
 			if (!speed) {
-				netdev_err(port->slave->dev,
-					   "speed changed to 0 for port %s",
-					   port->slave->dev->name);
+				slave_err(port->slave->bond->dev,
+					  port->slave->dev,
+					  "speed changed to 0 on port %d\n",
+					  port->actor_port_number);
 			} else if (duplex && ospeed != speed) {
 				/* Speed change restarts LACP state-machine */
 				port->sm_vars |= AD_PORT_BEGIN;
@@ -2483,17 +2495,16 @@ void bond_3ad_adapter_speed_duplex_changed(struct slave *slave)
 
 	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
-		netdev_warn(slave->bond->dev,
-			    "speed/duplex changed for uninitialized port %s\n",
-			    slave->dev->name);
+		slave_warn(slave->bond->dev, slave->dev,
+			   "speed/duplex changed for uninitialized port\n");
 		return;
 	}
 
 	spin_lock_bh(&slave->bond->mode_lock);
 	ad_update_actor_keys(port, false);
 	spin_unlock_bh(&slave->bond->mode_lock);
-	netdev_dbg(slave->bond->dev, "Port %d slave %s changed speed/duplex\n",
-		   port->actor_port_number, slave->dev->name);
+	slave_dbg(slave->bond->dev, slave->dev, "Port %d changed speed/duplex\n",
+		  port->actor_port_number);
 }
 
 /**
@@ -2513,8 +2524,7 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 
 	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
-		netdev_warn(slave->bond->dev, "link status changed for uninitialized port on %s\n",
-			    slave->dev->name);
+		slave_warn(slave->bond->dev, slave->dev, "link status changed for uninitialized port\n");
 		return;
 	}
 
@@ -2539,9 +2549,9 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 
 	spin_unlock_bh(&slave->bond->mode_lock);
 
-	netdev_dbg(slave->bond->dev, "Port %d changed link status to %s\n",
-		   port->actor_port_number,
-		   link == BOND_LINK_UP ? "UP" : "DOWN");
+	slave_dbg(slave->bond->dev, slave->dev, "Port %d changed link status to %s\n",
+		  port->actor_port_number,
+		  link == BOND_LINK_UP ? "UP" : "DOWN");
 
 	/* RTNL is held and mode_lock is released so it's safe
 	 * to update slave_array here.
-- 
2.20.1

[PATCH net-next 6/7] bonding/alb: convert to using slave printk macros

[Jarod Wilson]

All of these printk instances benefit from having both master and slave
device information included, so convert to using a standardized macro
format and remove redundant information.

Suggested-by: Joe Perches <joe@perches.com>
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/net/bonding/bond_alb.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 790e41c6fdd0..8c79bad2a9a5 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -300,7 +300,7 @@ static int rlb_arp_recv(const struct sk_buff *skb, struct bonding *bond,
 	if (arp->op_code == htons(ARPOP_REPLY)) {
 		/* update rx hash table for this ARP */
 		rlb_update_entry_from_arp(bond, arp);
-		netdev_dbg(bond->dev, "Server received an ARP Reply from client\n");
+		slave_dbg(bond->dev, slave->dev, "Server received an ARP Reply from client\n");
 	}
 out:
 	return RX_HANDLER_ANOTHER;
@@ -442,8 +442,9 @@ static void rlb_update_client(struct rlb_client_info *client_info)
 				 client_info->slave->dev->dev_addr,
 				 client_info->mac_dst);
 		if (!skb) {
-			netdev_err(client_info->slave->bond->dev,
-				   "failed to create an ARP packet\n");
+			slave_err(client_info->slave->bond->dev,
+				  client_info->slave->dev,
+				  "failed to create an ARP packet\n");
 			continue;
 		}
 
@@ -667,14 +668,15 @@ static struct slave *rlb_arp_xmit(struct sk_buff *skb, struct bonding *bond)
 		if (tx_slave)
 			bond_hw_addr_copy(arp->mac_src, tx_slave->dev->dev_addr,
 					  tx_slave->dev->addr_len);
-		netdev_dbg(bond->dev, "Server sent ARP Reply packet\n");
+		netdev_dbg(bond->dev, "(slave %s): Server sent ARP Reply packet\n",
+			   tx_slave ? tx_slave->dev->name : "NULL");
 	} else if (arp->op_code == htons(ARPOP_REQUEST)) {
 		/* Create an entry in the rx_hashtbl for this client as a
 		 * place holder.
 		 * When the arp reply is received the entry will be updated
 		 * with the correct unicast address of the client.
 		 */
-		rlb_choose_channel(skb, bond);
+		tx_slave = rlb_choose_channel(skb, bond);
 
 		/* The ARP reply packets must be delayed so that
 		 * they can cancel out the influence of the ARP request.
@@ -687,7 +689,8 @@ static struct slave *rlb_arp_xmit(struct sk_buff *skb, struct bonding *bond)
 		 * updated with their assigned mac.
 		 */
 		rlb_req_update_subnet_clients(bond, arp->ip_src);
-		netdev_dbg(bond->dev, "Server sent ARP Request packet\n");
+		netdev_dbg(bond->dev, "(slave %s): Server sent ARP Request packet\n",
+			   tx_slave ? tx_slave->dev->name : "NULL");
 	}
 
 	return tx_slave;
@@ -923,9 +926,8 @@ static void alb_send_lp_vid(struct slave *slave, u8 mac_addr[],
 	skb->priority = TC_PRIO_CONTROL;
 	skb->dev = slave->dev;
 
-	netdev_dbg(slave->bond->dev,
-		   "Send learning packet: dev %s mac %pM vlan %d\n",
-		   slave->dev->name, mac_addr, vid);
+	slave_dbg(slave->bond->dev, slave->dev,
+		  "Send learning packet: mac %pM vlan %d\n", mac_addr, vid);
 
 	if (vid)
 		__vlan_hwaccel_put_tag(skb, vlan_proto, vid);
@@ -1016,8 +1018,7 @@ static int alb_set_slave_mac_addr(struct slave *slave, u8 addr[],
 	memcpy(ss.__data, addr, len);
 	ss.ss_family = dev->type;
 	if (dev_set_mac_address(dev, (struct sockaddr *)&ss, NULL)) {
-		netdev_err(slave->bond->dev, "dev_set_mac_address of dev %s failed! ALB mode requires that the base driver support setting the hw address also when the network device's interface is open\n",
-			   dev->name);
+		slave_err(slave->bond->dev, dev, "dev_set_mac_address on slave failed! ALB mode requires that the base driver support setting the hw address also when the network device's interface is open\n");
 		return -EOPNOTSUPP;
 	}
 	return 0;
@@ -1192,12 +1193,11 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav
 		alb_set_slave_mac_addr(slave, free_mac_slave->perm_hwaddr,
 				       free_mac_slave->dev->addr_len);
 
-		netdev_warn(bond->dev, "the hw address of slave %s is in use by the bond; giving it the hw address of %s\n",
-			    slave->dev->name, free_mac_slave->dev->name);
+		slave_warn(bond->dev, slave->dev, "the slave hw address is in use by the bond; giving it the hw address of %s\n",
+			   free_mac_slave->dev->name);
 
 	} else if (has_bond_addr) {
-		netdev_err(bond->dev, "the hw address of slave %s is in use by the bond; couldn't find a slave with a free hw address to give it (this should not have happened)\n",
-			   slave->dev->name);
+		slave_err(bond->dev, slave->dev, "the slave hw address is in use by the bond; couldn't find a slave with a free hw address to give it (this should not have happened)\n");
 		return -EFAULT;
 	}
 
-- 
2.20.1

[PATCH net-next 7/7] bonding/options: convert to using slave printk macros

[Jarod Wilson]

All of these printk instances benefit from having both master and slave
device information included, so convert to using a standardized macro
format and remove redundant information.

Suggested-by: Joe Perches <joe@perches.com>
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/net/bonding/bond_options.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index b996967af8d9..c9eaf5b59654 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -787,14 +787,12 @@ static int bond_option_active_slave_set(struct bonding *bond,
 
 	if (slave_dev) {
 		if (!netif_is_bond_slave(slave_dev)) {
-			netdev_err(bond->dev, "Device %s is not bonding slave\n",
-				   slave_dev->name);
+			slave_err(bond->dev, slave_dev, "Device is not bonding slave\n");
 			return -EINVAL;
 		}
 
 		if (bond->dev != netdev_master_upper_dev_get(slave_dev)) {
-			netdev_err(bond->dev, "Device %s is not our slave\n",
-				   slave_dev->name);
+			slave_err(bond->dev, slave_dev, "Device is not our slave\n");
 			return -EINVAL;
 		}
 	}
@@ -813,18 +811,15 @@ static int bond_option_active_slave_set(struct bonding *bond,
 
 		if (new_active == old_active) {
 			/* do nothing */
-			netdev_dbg(bond->dev, "%s is already the current active slave\n",
-				   new_active->dev->name);
+			slave_dbg(bond->dev, new_active->dev, "is already the current active slave\n");
 		} else {
 			if (old_active && (new_active->link == BOND_LINK_UP) &&
 			    bond_slave_is_up(new_active)) {
-				netdev_dbg(bond->dev, "Setting %s as active slave\n",
-					   new_active->dev->name);
+				slave_dbg(bond->dev, new_active->dev, "Setting as active slave\n");
 				bond_change_active_slave(bond, new_active);
 			} else {
-				netdev_err(bond->dev, "Could not set %s as active slave; either %s is down or the link is down\n",
-					   new_active->dev->name,
-					   new_active->dev->name);
+				slave_err(bond->dev, new_active->dev, "Could not set as active slave; either %s is down or the link is down\n",
+					  new_active->dev->name);
 				ret = -EINVAL;
 			}
 		}
@@ -1136,8 +1131,7 @@ static int bond_option_primary_set(struct bonding *bond,
 
 	bond_for_each_slave(bond, slave, iter) {
 		if (strncmp(slave->dev->name, primary, IFNAMSIZ) == 0) {
-			netdev_dbg(bond->dev, "Setting %s as primary slave\n",
-				   slave->dev->name);
+			slave_dbg(bond->dev, slave->dev, "Setting as primary slave\n");
 			rcu_assign_pointer(bond->primary_slave, slave);
 			strcpy(bond->params.primary, slave->dev->name);
 			bond->force_primary = true;
@@ -1154,8 +1148,8 @@ static int bond_option_primary_set(struct bonding *bond,
 	strncpy(bond->params.primary, primary, IFNAMSIZ);
 	bond->params.primary[IFNAMSIZ - 1] = 0;
 
-	netdev_dbg(bond->dev, "Recording %s as primary, but it has not been enslaved to %s yet\n",
-		   primary, bond->dev->name);
+	netdev_dbg(bond->dev, "Recording %s as primary, but it has not been enslaved yet\n",
+		   primary);
 
 out:
 	unblock_netpoll_tx();
@@ -1382,12 +1376,12 @@ static int bond_option_slaves_set(struct bonding *bond,
 
 	switch (command[0]) {
 	case '+':
-		netdev_dbg(bond->dev, "Adding slave %s\n", dev->name);
+		slave_dbg(bond->dev, dev, "Enslaving interface\n");
 		ret = bond_enslave(bond->dev, dev, NULL);
 		break;
 
 	case '-':
-		netdev_dbg(bond->dev, "Removing slave %s\n", dev->name);
+		slave_dbg(bond->dev, dev, "Releasing interface\n");
 		ret = bond_release(bond->dev, dev);
 		break;
 
@@ -1451,7 +1445,7 @@ static int bond_option_ad_actor_system_set(struct bonding *bond,
 	return 0;
 
 err:
-	netdev_err(bond->dev, "Invalid MAC address.\n");
+	netdev_err(bond->dev, "Invalid ad_actor_system MAC address.\n");
 	return -EINVAL;
 }
 
-- 
2.20.1

[PATCH net] bonding: make debugging output more succinct

[Jarod Wilson]

Seeing bonding debug log data along the lines of "event: 5" is a bit spartan,
and often requires a lookup table if you don't remember what every event is.
Make use of netdev_cmd_to_name for an improved debugging experience, so for
the prior example, you'll see: "bond_netdev_event received NETDEV_REGISTER"
instead (both are prefixed with the device for which the event pertains).

There are also quite a few places that the netdev_dbg output could stand to
mention exactly which slave the message pertains to (gets messy if you have
multiple slaves all spewing at once to know which one they pertain to).

CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: "David S. Miller" <davem@davemloft.net>
CC: netdev@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 drivers/net/bonding/bond_main.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 407f4095a37a..4acc352b316b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3212,7 +3220,8 @@ static int bond_netdev_event(struct notifier_block *this,
 {
 	struct net_device *event_dev = netdev_notifier_info_to_dev(ptr);
 
-	netdev_dbg(event_dev, "event: %lx\n", event);
+	netdev_dbg(event_dev, "%s received %s\n",
+		   __func__, netdev_cmd_to_name(event));
 
 	if (!(event_dev->priv_flags & IFF_BONDING))
 		return NOTIFY_DONE;
-- 
2.20.1

Re: [PATCH net] bonding: make debugging output more succinct

[Jarod Wilson]

On 6/7/19 10:59 AM, Jarod Wilson wrote:
> Seeing bonding debug log data along the lines of "event: 5" is a bit spartan,
> and often requires a lookup table if you don't remember what every event is.
> Make use of netdev_cmd_to_name for an improved debugging experience, so for
> the prior example, you'll see: "bond_netdev_event received NETDEV_REGISTER"
> instead (both are prefixed with the device for which the event pertains).
> 
> There are also quite a few places that the netdev_dbg output could stand to
> mention exactly which slave the message pertains to (gets messy if you have
> multiple slaves all spewing at once to know which one they pertain to).

Argh. Please drop this one, detritus in my git tree when I hit git 
send-email caused this earlier iteration of patch 1 of the set this is 
threaded with to go out.

-- 
Jarod Wilson
jarod@redhat.com

Re: [PATCH net] bonding: make debugging output more succinct

[David Miller]

From: Jarod Wilson <jarod@redhat.com>
Date: Fri, 7 Jun 2019 11:02:34 -0400

> On 6/7/19 10:59 AM, Jarod Wilson wrote:
>> Seeing bonding debug log data along the lines of "event: 5" is a bit
>> spartan,
>> and often requires a lookup table if you don't remember what every
>> event is.
>> Make use of netdev_cmd_to_name for an improved debugging experience,
>> so for
>> the prior example, you'll see: "bond_netdev_event received
>> NETDEV_REGISTER"
>> instead (both are prefixed with the device for which the event
>> pertains).
>> There are also quite a few places that the netdev_dbg output could
>> stand to
>> mention exactly which slave the message pertains to (gets messy if you
>> have
>> multiple slaves all spewing at once to know which one they pertain
>> to).
> 
> Argh. Please drop this one, detritus in my git tree when I hit git
> send-email caused this earlier iteration of patch 1 of the set this is
> threaded with to go out.

Ok.

Re: [PATCH net-next 0/7] bonding: clean up and standarize logging printks

[David Miller]

From: Jarod Wilson <jarod@redhat.com>
Date: Fri,  7 Jun 2019 10:59:25 -0400

> This set improves a few somewhat terse bonding debug messages, fixes some
> errors in others, and then standarizes the majority of them, using new
> slave_* printk macros that wrap around netdev_* to ensure both master
> and slave information is provided consistently, where relevant. This set
> proves very useful in debugging issues on hosts with multiple bonds.
> 
> I've run an array of LNST tests over this set, creating and destroying
> quite a few different bonds of the course of testing, fixed the little
> gotchas here and there, and everything looks stable and reasonable to me,
> but I can't guarantee I've tested every possible message and scenario to
> catch every possible "slave could be NULL" case.

Series applied, thanks.