diff mbox

Add printk for bonding module packets_per_slave parameter

Message ID 20170613134246.6407-1-michael.j.dilmore@gmail.com
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Michael Dilmore June 13, 2017, 1:42 p.m. UTC
The packets per slave parameter used by round robin mode does not have a printk debug
message in its set function in bond_options.c. Adding such a function would aid debugging
of round-robin mode and allow the user to more easily verify that the parameter has been
set correctly. I should add that I'm motivated by my own experience here - it's not
obvious from output of tools such as wireshark and ifstat that the parameter is working
correctly, and with the differences in bonding configuration across different distributions,
it would have been comforting to see this output.

Signed-off-by: Michael Dilmore <michael.j.dilmore@gmail.com>

cc: Veaceslav Falico <vfalico@gmail.com>,Andy Gospodarek <andy@greyhouse.net>,netdev@vger.kernel.org,linux-kernel@vger.kernel.org
---
 drivers/net/bonding/bond_options.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

David Miller June 13, 2017, 3:34 p.m. UTC | #1
From: Michael Dilmore <michael.j.dilmore@gmail.com>
Date: Tue, 13 Jun 2017 14:42:46 +0100

> The packets per slave parameter used by round robin mode does not have a printk debug
> message in its set function in bond_options.c. Adding such a function would aid debugging
> of round-robin mode and allow the user to more easily verify that the parameter has been
> set correctly. I should add that I'm motivated by my own experience here - it's not
> obvious from output of tools such as wireshark and ifstat that the parameter is working
> correctly, and with the differences in bonding configuration across different distributions,
> it would have been comforting to see this output.
> 
> Signed-off-by: Michael Dilmore <michael.j.dilmore@gmail.com>
> 
> cc: Veaceslav Falico <vfalico@gmail.com>,Andy Gospodarek <andy@greyhouse.net>,netdev@vger.kernel.org,linux-kernel@vger.kernel.org

You can verify things by simplying reading the value back.

If every parameter emitted a kernel log message, it would be
unreadable.

I'm not applying this, sorry.
kernel test robot June 13, 2017, 9:18 p.m. UTC | #2
Hi Michael,

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.12-rc5 next-20170613]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Michael-Dilmore/Add-printk-for-bonding-module-packets_per_slave-parameter/20170614-045412
config: i386-randconfig-x014-06122022 (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=i386 

All warnings (new ones prefixed by >>):

   drivers/net//bonding/bond_options.c: In function 'bond_option_pps_set':
>> drivers/net//bonding/bond_options.c:1259:56: warning: format '%d' expects argument of type 'int', but argument 3 has type 'u64 {aka const long long unsigned int}' [-Wformat=]
     netdev_info(bond->dev, "Setting packets per slave to %d\n",
                                                           ^

vim +1259 drivers/net//bonding/bond_options.c

  1243		bond_set_carrier(bond);
  1244	
  1245		return 0;
  1246	}
  1247	
  1248	static int bond_option_lp_interval_set(struct bonding *bond,
  1249					       const struct bond_opt_value *newval)
  1250	{
  1251		bond->params.lp_interval = newval->value;
  1252	
  1253		return 0;
  1254	}
  1255	
  1256	static int bond_option_pps_set(struct bonding *bond,
  1257				       const struct bond_opt_value *newval)
  1258	{
> 1259		netdev_info(bond->dev, "Setting packets per slave to %d\n",
  1260			    newval->value);
  1261		bond->params.packets_per_slave = newval->value;
  1262		if (newval->value > 0) {
  1263			bond->params.reciprocal_packets_per_slave =
  1264				reciprocal_value(newval->value);
  1265		} else {
  1266			/* reciprocal_packets_per_slave is unused if
  1267			 * packets_per_slave is 0 or 1, just initialize it

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot June 13, 2017, 11:12 p.m. UTC | #3
Hi Michael,

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.12-rc5 next-20170613]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Michael-Dilmore/Add-printk-for-bonding-module-packets_per_slave-parameter/20170614-045412
config: tile-tilegx_defconfig (attached as .config)
compiler: tilegx-linux-gcc (GCC) 4.6.2
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=tile 

All warnings (new ones prefixed by >>):

   drivers/net/bonding/bond_options.c: In function 'bond_option_pps_set':
>> drivers/net/bonding/bond_options.c:1260:7: warning: format '%d' expects argument of type 'int', but argument 3 has type 'u64' [-Wformat]

vim +1260 drivers/net/bonding/bond_options.c

  1244	
  1245		return 0;
  1246	}
  1247	
  1248	static int bond_option_lp_interval_set(struct bonding *bond,
  1249					       const struct bond_opt_value *newval)
  1250	{
  1251		bond->params.lp_interval = newval->value;
  1252	
  1253		return 0;
  1254	}
  1255	
  1256	static int bond_option_pps_set(struct bonding *bond,
  1257				       const struct bond_opt_value *newval)
  1258	{
  1259		netdev_info(bond->dev, "Setting packets per slave to %d\n",
> 1260			    newval->value);
  1261		bond->params.packets_per_slave = newval->value;
  1262		if (newval->value > 0) {
  1263			bond->params.reciprocal_packets_per_slave =
  1264				reciprocal_value(newval->value);
  1265		} else {
  1266			/* reciprocal_packets_per_slave is unused if
  1267			 * packets_per_slave is 0 or 1, just initialize it
  1268			 */

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

Patch

diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 1bcbb8913e17..4da0de1bab0a 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -1233,6 +1233,8 @@  static int bond_option_lp_interval_set(struct bonding *bond,
 static int bond_option_pps_set(struct bonding *bond,
 			       const struct bond_opt_value *newval)
 {
+	netdev_info(bond->dev, "Setting packets per slave to %d\n",
+		    newval->value);
 	bond->params.packets_per_slave = newval->value;
 	if (newval->value > 0) {
 		bond->params.reciprocal_packets_per_slave =