Message ID | 20170613134246.6407-1-michael.j.dilmore@gmail.com |
---|---|
State | Rejected, archived |
Delegated to: | David Miller |
Headers | show |
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.
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
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 --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 =
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(+)