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

From: Jarod Wilson
Date: Fri May 24 2019 - 12:13:48 EST


On 5/24/19 11:19 AM, Joe Perches wrote:
On Fri, 2019-05-24 at 09:56 -0400, 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).
[]
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
[]
@@ -1515,7 +1515,8 @@ 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);
+ netdev_dbg(bond_dev, "Error %d calling dev_set_mtu for slave %s\n",
+ res, slave_dev->name);

Perhaps better to add and use helper mechanisms like:

#define slave_dbg(bond_dev, slave_dev, fmt, ...) \
netdev_dbg(bond_dev, "(slave %s) " fmt, (slave_dev)->name, ##__VA_ARGS__)

So this might be
slave_dbg(bond_dev, slave_dev, "Error %d calling dev_set_mtu\n",
res);
etc...

So there would be a unified style to grep in the logs.

I do kind of like that idea. Might also need slave_info and friends as well if you really want to get consistent, and eliminate bond_dev as an arg to it, since you can figure that out from slave_dev. I'd be game to take that little project on. Might be worth peeling out the netdev_cmd_to_name() bit from this for consideration right now, since it's not quite part of that same conversion.

--
Jarod Wilson
jarod@xxxxxxxxxx