[PATCH] Add printk for bonding module packets_per_slave parameter

[PATCH] Add printk for bonding module packets_per_slave parameter

[Michael Dilmore]

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(+)

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 =
-- 
2.11.0

Re: [PATCH] Add printk for bonding module packets_per_slave parameter

[David Miller]

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.

Re: [PATCH] Add printk for bonding module packets_per_slave parameter

[Joe Perches]

On Tue, 2017-06-13 at 11:34 -0400, David Miller wrote:
> 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.

I agree.  Noisy logging output is not good.

Perhaps a general conversion of the dozens
of existing netdev_info uses in this file to
netdev_dbg and adding this at netdev_dbg is
appropriate.

Something like:
---
 drivers/net/bonding/bond_options.c | 119 +++++++++++++++++++------------------
 1 file changed, 60 insertions(+), 59 deletions(-)

diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 8ca683396fcc..9dec49b1b8ae 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -750,8 +750,8 @@ static int bond_option_mode_set(struct bonding *bond,
 		bond->params.arp_interval = 0;
 		/* set miimon to default value */
 		bond->params.miimon = BOND_DEFAULT_MIIMON;
-		netdev_info(bond->dev, "Setting MII monitoring interval to %d\n",
-			    bond->params.miimon);
+		netdev_dbg(bond->dev, "Setting MII monitoring interval to %d\n",
+			   bond->params.miimon);
 	}
 
 	/* don't cache arp_validate between modes */
@@ -794,7 +794,7 @@ static int bond_option_active_slave_set(struct bonding *bond,
 	block_netpoll_tx();
 	/* check to see if we are clearing active */
 	if (!slave_dev) {
-		netdev_info(bond->dev, "Clearing current active slave\n");
+		netdev_dbg(bond->dev, "Clearing current active slave\n");
 		RCU_INIT_POINTER(bond->curr_active_slave, NULL);
 		bond_select_active_slave(bond);
 	} else {
@@ -805,13 +805,13 @@ static int bond_option_active_slave_set(struct bonding *bond,
 
 		if (new_active == old_active) {
 			/* do nothing */
-			netdev_info(bond->dev, "%s is already the current active slave\n",
-				    new_active->dev->name);
+			netdev_dbg(bond->dev, "%s is already the current active slave\n",
+				   new_active->dev->name);
 		} else {
 			if (old_active && (new_active->link == BOND_LINK_UP) &&
 			    bond_slave_is_up(new_active)) {
-				netdev_info(bond->dev, "Setting %s as active slave\n",
-					    new_active->dev->name);
+				netdev_dbg(bond->dev, "Setting %s as active slave\n",
+					   new_active->dev->name);
 				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",
@@ -833,17 +833,17 @@ static int bond_option_active_slave_set(struct bonding *bond,
 static int bond_option_miimon_set(struct bonding *bond,
 				  const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting MII monitoring interval to %llu\n",
-		    newval->value);
+	netdev_dbg(bond->dev, "Setting MII monitoring interval to %llu\n",
+		   newval->value);
 	bond->params.miimon = newval->value;
 	if (bond->params.updelay)
-		netdev_info(bond->dev, "Note: Updating updelay (to %d) since it is a multiple of the miimon value\n",
-			bond->params.updelay * bond->params.miimon);
+		netdev_dbg(bond->dev, "Note: Updating updelay (to %d) since it is a multiple of the miimon value\n",
+			   bond->params.updelay * bond->params.miimon);
 	if (bond->params.downdelay)
-		netdev_info(bond->dev, "Note: Updating downdelay (to %d) since it is a multiple of the miimon value\n",
-			    bond->params.downdelay * bond->params.miimon);
+		netdev_dbg(bond->dev, "Note: Updating downdelay (to %d) since it is a multiple of the miimon value\n",
+			   bond->params.downdelay * bond->params.miimon);
 	if (newval->value && bond->params.arp_interval) {
-		netdev_info(bond->dev, "MII monitoring cannot be used with ARP monitoring - disabling ARP monitoring...\n");
+		netdev_dbg(bond->dev, "MII monitoring cannot be used with ARP monitoring - disabling ARP monitoring...\n");
 		bond->params.arp_interval = 0;
 		if (bond->params.arp_validate)
 			bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
@@ -885,8 +885,8 @@ static int bond_option_updelay_set(struct bonding *bond,
 			    bond->params.miimon);
 	}
 	bond->params.updelay = value / bond->params.miimon;
-	netdev_info(bond->dev, "Setting up delay to %d\n",
-		    bond->params.updelay * bond->params.miimon);
+	netdev_dbg(bond->dev, "Setting up delay to %d\n",
+		   bond->params.updelay * bond->params.miimon);
 
 	return 0;
 }
@@ -907,8 +907,8 @@ static int bond_option_downdelay_set(struct bonding *bond,
 			    bond->params.miimon);
 	}
 	bond->params.downdelay = value / bond->params.miimon;
-	netdev_info(bond->dev, "Setting down delay to %d\n",
-		    bond->params.downdelay * bond->params.miimon);
+	netdev_dbg(bond->dev, "Setting down delay to %d\n",
+		   bond->params.downdelay * bond->params.miimon);
 
 	return 0;
 }
@@ -916,8 +916,7 @@ static int bond_option_downdelay_set(struct bonding *bond,
 static int bond_option_use_carrier_set(struct bonding *bond,
 				       const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting use_carrier to %llu\n",
-		    newval->value);
+	netdev_dbg(bond->dev, "Setting use_carrier to %llu\n", newval->value);
 	bond->params.use_carrier = newval->value;
 
 	return 0;
@@ -930,16 +929,16 @@ static int bond_option_use_carrier_set(struct bonding *bond,
 static int bond_option_arp_interval_set(struct bonding *bond,
 					const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting ARP monitoring interval to %llu\n",
-		    newval->value);
+	netdev_dbg(bond->dev, "Setting ARP monitoring interval to %llu\n",
+		   newval->value);
 	bond->params.arp_interval = newval->value;
 	if (newval->value) {
 		if (bond->params.miimon) {
-			netdev_info(bond->dev, "ARP monitoring cannot be used with MII monitoring. Disabling MII monitoring\n");
+			netdev_dbg(bond->dev, "ARP monitoring cannot be used with MII monitoring. Disabling MII monitoring\n");
 			bond->params.miimon = 0;
 		}
 		if (!bond->params.arp_targets[0])
-			netdev_info(bond->dev, "ARP monitoring has been set up, but no ARP targets have been specified\n");
+			netdev_dbg(bond->dev, "ARP monitoring has been set up, but no ARP targets have been specified\n");
 	}
 	if (bond->dev->flags & IFF_UP) {
 		/* If the interface is up, we may need to fire off
@@ -1000,7 +999,7 @@ static int _bond_option_arp_ip_target_add(struct bonding *bond, __be32 target)
 		return -EINVAL;
 	}
 
-	netdev_info(bond->dev, "Adding ARP target %pI4\n", &target);
+	netdev_dbg(bond->dev, "Adding ARP target %pI4\n", &target);
 
 	_bond_options_arp_ip_target_set(bond, ind, target, jiffies);
 
@@ -1036,7 +1035,7 @@ static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
 	if (ind == 0 && !targets[1] && bond->params.arp_interval)
 		netdev_warn(bond->dev, "Removing last arp target with arp_interval on\n");
 
-	netdev_info(bond->dev, "Removing ARP target %pI4\n", &target);
+	netdev_dbg(bond->dev, "Removing ARP target %pI4\n", &target);
 
 	bond_for_each_slave(bond, slave, iter) {
 		targets_rx = slave->target_last_arp_rx;
@@ -1088,8 +1087,8 @@ static int bond_option_arp_ip_targets_set(struct bonding *bond,
 static int bond_option_arp_validate_set(struct bonding *bond,
 					const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting arp_validate to %s (%llu)\n",
-		    newval->string, newval->value);
+	netdev_dbg(bond->dev, "Setting arp_validate to %s (%llu)\n",
+		   newval->string, newval->value);
 
 	if (bond->dev->flags & IFF_UP) {
 		if (!newval->value)
@@ -1105,8 +1104,8 @@ static int bond_option_arp_validate_set(struct bonding *bond,
 static int bond_option_arp_all_targets_set(struct bonding *bond,
 					   const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting arp_all_targets to %s (%llu)\n",
-		    newval->string, newval->value);
+	netdev_dbg(bond->dev, "Setting arp_all_targets to %s (%llu)\n",
+		   newval->string, newval->value);
 	bond->params.arp_all_targets = newval->value;
 
 	return 0;
@@ -1126,7 +1125,7 @@ static int bond_option_primary_set(struct bonding *bond,
 		*p = '\0';
 	/* check to see if we are clearing primary */
 	if (!strlen(primary)) {
-		netdev_info(bond->dev, "Setting primary slave to None\n");
+		netdev_dbg(bond->dev, "Setting primary slave to None\n");
 		RCU_INIT_POINTER(bond->primary_slave, NULL);
 		memset(bond->params.primary, 0, sizeof(bond->params.primary));
 		bond_select_active_slave(bond);
@@ -1135,8 +1134,8 @@ 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_info(bond->dev, "Setting %s as primary slave\n",
-				    slave->dev->name);
+			netdev_dbg(bond->dev, "Setting %s as primary slave\n",
+				   slave->dev->name);
 			rcu_assign_pointer(bond->primary_slave, slave);
 			strcpy(bond->params.primary, slave->dev->name);
 			bond_select_active_slave(bond);
@@ -1145,15 +1144,15 @@ static int bond_option_primary_set(struct bonding *bond,
 	}
 
 	if (rtnl_dereference(bond->primary_slave)) {
-		netdev_info(bond->dev, "Setting primary slave to None\n");
+		netdev_dbg(bond->dev, "Setting primary slave to None\n");
 		RCU_INIT_POINTER(bond->primary_slave, NULL);
 		bond_select_active_slave(bond);
 	}
 	strncpy(bond->params.primary, primary, IFNAMSIZ);
 	bond->params.primary[IFNAMSIZ - 1] = 0;
 
-	netdev_info(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 to %s yet\n",
+		   primary, bond->dev->name);
 
 out:
 	unblock_netpoll_tx();
@@ -1164,8 +1163,8 @@ static int bond_option_primary_set(struct bonding *bond,
 static int bond_option_primary_reselect_set(struct bonding *bond,
 					    const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting primary_reselect to %s (%llu)\n",
-		    newval->string, newval->value);
+	netdev_dbg(bond->dev, "Setting primary_reselect to %s (%llu)\n",
+		   newval->string, newval->value);
 	bond->params.primary_reselect = newval->value;
 
 	block_netpoll_tx();
@@ -1178,8 +1177,8 @@ static int bond_option_primary_reselect_set(struct bonding *bond,
 static int bond_option_fail_over_mac_set(struct bonding *bond,
 					 const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting fail_over_mac to %s (%llu)\n",
-		    newval->string, newval->value);
+	netdev_dbg(bond->dev, "Setting fail_over_mac to %s (%llu)\n",
+		   newval->string, newval->value);
 	bond->params.fail_over_mac = newval->value;
 
 	return 0;
@@ -1188,8 +1187,8 @@ static int bond_option_fail_over_mac_set(struct bonding *bond,
 static int bond_option_xmit_hash_policy_set(struct bonding *bond,
 					    const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting xmit hash policy to %s (%llu)\n",
-		    newval->string, newval->value);
+	netdev_dbg(bond->dev, "Setting xmit hash policy to %s (%llu)\n",
+		   newval->string, newval->value);
 	bond->params.xmit_policy = newval->value;
 
 	return 0;
@@ -1198,8 +1197,8 @@ static int bond_option_xmit_hash_policy_set(struct bonding *bond,
 static int bond_option_resend_igmp_set(struct bonding *bond,
 				       const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting resend_igmp to %llu\n",
-		    newval->value);
+	netdev_dbg(bond->dev, "Setting resend_igmp to %llu\n",
+		   newval->value);
 	bond->params.resend_igmp = newval->value;
 
 	return 0;
@@ -1237,8 +1236,8 @@ static int bond_option_all_slaves_active_set(struct bonding *bond,
 static int bond_option_min_links_set(struct bonding *bond,
 				     const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting min links value to %llu\n",
-		    newval->value);
+	netdev_dbg(bond->dev, "Setting min links value to %llu\n",
+		   newval->value);
 	bond->params.min_links = newval->value;
 	bond_set_carrier(bond);
 
@@ -1256,6 +1255,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_dbg(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 =
@@ -1274,8 +1275,8 @@ static int bond_option_pps_set(struct bonding *bond,
 static int bond_option_lacp_rate_set(struct bonding *bond,
 				     const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting LACP rate to %s (%llu)\n",
-		    newval->string, newval->value);
+	netdev_dbg(bond->dev, "Setting LACP rate to %s (%llu)\n",
+		   newval->string, newval->value);
 	bond->params.lacp_fast = newval->value;
 	bond_3ad_update_lacp_rate(bond);
 
@@ -1285,8 +1286,8 @@ static int bond_option_lacp_rate_set(struct bonding *bond,
 static int bond_option_ad_select_set(struct bonding *bond,
 				     const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting ad_select to %s (%llu)\n",
-		    newval->string, newval->value);
+	netdev_dbg(bond->dev, "Setting ad_select to %s (%llu)\n",
+		   newval->string, newval->value);
 	bond->params.ad_select = newval->value;
 
 	return 0;
@@ -1377,12 +1378,12 @@ static int bond_option_slaves_set(struct bonding *bond,
 
 	switch (command[0]) {
 	case '+':
-		netdev_info(bond->dev, "Adding slave %s\n", dev->name);
+		netdev_dbg(bond->dev, "Adding slave %s\n", dev->name);
 		ret = bond_enslave(bond->dev, dev);
 		break;
 
 	case '-':
-		netdev_info(bond->dev, "Removing slave %s\n", dev->name);
+		netdev_dbg(bond->dev, "Removing slave %s\n", dev->name);
 		ret = bond_release(bond->dev, dev);
 		break;
 
@@ -1402,8 +1403,8 @@ static int bond_option_slaves_set(struct bonding *bond,
 static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
 					  const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting dynamic-lb to %s (%llu)\n",
-		    newval->string, newval->value);
+	netdev_dbg(bond->dev, "Setting dynamic-lb to %s (%llu)\n",
+		   newval->string, newval->value);
 	bond->params.tlb_dynamic_lb = newval->value;
 
 	return 0;
@@ -1412,8 +1413,8 @@ static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
 static int bond_option_ad_actor_sys_prio_set(struct bonding *bond,
 					     const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting ad_actor_sys_prio to %llu\n",
-		    newval->value);
+	netdev_dbg(bond->dev, "Setting ad_actor_sys_prio to %llu\n",
+		   newval->value);
 
 	bond->params.ad_actor_sys_prio = newval->value;
 	bond_3ad_update_ad_actor_settings(bond);
@@ -1442,7 +1443,7 @@ static int bond_option_ad_actor_system_set(struct bonding *bond,
 	if (!is_valid_ether_addr(mac))
 		goto err;
 
-	netdev_info(bond->dev, "Setting ad_actor_system to %pM\n", mac);
+	netdev_dbg(bond->dev, "Setting ad_actor_system to %pM\n", mac);
 	ether_addr_copy(bond->params.ad_actor_system, mac);
 	bond_3ad_update_ad_actor_settings(bond);
 
@@ -1456,8 +1457,8 @@ static int bond_option_ad_actor_system_set(struct bonding *bond,
 static int bond_option_ad_user_port_key_set(struct bonding *bond,
 					    const struct bond_opt_value *newval)
 {
-	netdev_info(bond->dev, "Setting ad_user_port_key to %llu\n",
-		    newval->value);
+	netdev_dbg(bond->dev, "Setting ad_user_port_key to %llu\n",
+		   newval->value);
 
 	bond->params.ad_user_port_key = newval->value;
 	return 0;

Re: [PATCH] Add printk for bonding module packets_per_slave parameter

[Jonathan Toppins]

On 06/13/2017 12:21 PM, Joe Perches wrote:
> On Tue, 2017-06-13 at 11:34 -0400, David Miller wrote:
>> 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.
> 
> I agree.  Noisy logging output is not good.
> 
> Perhaps a general conversion of the dozens
> of existing netdev_info uses in this file to
> netdev_dbg and adding this at netdev_dbg is
> appropriate.

In general I agree. The few times I have debugged bonds, I always ended
up enabling debug prinks anyway. I don't see a problem moving these to
debug as well.

Adding nik whom converted a lot of this code to common paths for input.

> 
> Something like:
> ---
>  drivers/net/bonding/bond_options.c | 119 +++++++++++++++++++------------------
>  1 file changed, 60 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
> index 8ca683396fcc..9dec49b1b8ae 100644
> --- a/drivers/net/bonding/bond_options.c
> +++ b/drivers/net/bonding/bond_options.c
> @@ -750,8 +750,8 @@ static int bond_option_mode_set(struct bonding *bond,
>  		bond->params.arp_interval = 0;
>  		/* set miimon to default value */
>  		bond->params.miimon = BOND_DEFAULT_MIIMON;
> -		netdev_info(bond->dev, "Setting MII monitoring interval to %d\n",
> -			    bond->params.miimon);
> +		netdev_dbg(bond->dev, "Setting MII monitoring interval to %d\n",
> +			   bond->params.miimon);
>  	}
>  
>  	/* don't cache arp_validate between modes */
> @@ -794,7 +794,7 @@ static int bond_option_active_slave_set(struct bonding *bond,
>  	block_netpoll_tx();
>  	/* check to see if we are clearing active */
>  	if (!slave_dev) {
> -		netdev_info(bond->dev, "Clearing current active slave\n");
> +		netdev_dbg(bond->dev, "Clearing current active slave\n");
>  		RCU_INIT_POINTER(bond->curr_active_slave, NULL);
>  		bond_select_active_slave(bond);
>  	} else {
> @@ -805,13 +805,13 @@ static int bond_option_active_slave_set(struct bonding *bond,
>  
>  		if (new_active == old_active) {
>  			/* do nothing */
> -			netdev_info(bond->dev, "%s is already the current active slave\n",
> -				    new_active->dev->name);
> +			netdev_dbg(bond->dev, "%s is already the current active slave\n",
> +				   new_active->dev->name);
>  		} else {
>  			if (old_active && (new_active->link == BOND_LINK_UP) &&
>  			    bond_slave_is_up(new_active)) {
> -				netdev_info(bond->dev, "Setting %s as active slave\n",
> -					    new_active->dev->name);
> +				netdev_dbg(bond->dev, "Setting %s as active slave\n",
> +					   new_active->dev->name);
>  				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",
> @@ -833,17 +833,17 @@ static int bond_option_active_slave_set(struct bonding *bond,
>  static int bond_option_miimon_set(struct bonding *bond,
>  				  const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting MII monitoring interval to %llu\n",
> -		    newval->value);
> +	netdev_dbg(bond->dev, "Setting MII monitoring interval to %llu\n",
> +		   newval->value);
>  	bond->params.miimon = newval->value;
>  	if (bond->params.updelay)
> -		netdev_info(bond->dev, "Note: Updating updelay (to %d) since it is a multiple of the miimon value\n",
> -			bond->params.updelay * bond->params.miimon);
> +		netdev_dbg(bond->dev, "Note: Updating updelay (to %d) since it is a multiple of the miimon value\n",
> +			   bond->params.updelay * bond->params.miimon);
>  	if (bond->params.downdelay)
> -		netdev_info(bond->dev, "Note: Updating downdelay (to %d) since it is a multiple of the miimon value\n",
> -			    bond->params.downdelay * bond->params.miimon);
> +		netdev_dbg(bond->dev, "Note: Updating downdelay (to %d) since it is a multiple of the miimon value\n",
> +			   bond->params.downdelay * bond->params.miimon);
>  	if (newval->value && bond->params.arp_interval) {
> -		netdev_info(bond->dev, "MII monitoring cannot be used with ARP monitoring - disabling ARP monitoring...\n");
> +		netdev_dbg(bond->dev, "MII monitoring cannot be used with ARP monitoring - disabling ARP monitoring...\n");
>  		bond->params.arp_interval = 0;
>  		if (bond->params.arp_validate)
>  			bond->params.arp_validate = BOND_ARP_VALIDATE_NONE;
> @@ -885,8 +885,8 @@ static int bond_option_updelay_set(struct bonding *bond,
>  			    bond->params.miimon);
>  	}
>  	bond->params.updelay = value / bond->params.miimon;
> -	netdev_info(bond->dev, "Setting up delay to %d\n",
> -		    bond->params.updelay * bond->params.miimon);
> +	netdev_dbg(bond->dev, "Setting up delay to %d\n",
> +		   bond->params.updelay * bond->params.miimon);
>  
>  	return 0;
>  }
> @@ -907,8 +907,8 @@ static int bond_option_downdelay_set(struct bonding *bond,
>  			    bond->params.miimon);
>  	}
>  	bond->params.downdelay = value / bond->params.miimon;
> -	netdev_info(bond->dev, "Setting down delay to %d\n",
> -		    bond->params.downdelay * bond->params.miimon);
> +	netdev_dbg(bond->dev, "Setting down delay to %d\n",
> +		   bond->params.downdelay * bond->params.miimon);
>  
>  	return 0;
>  }
> @@ -916,8 +916,7 @@ static int bond_option_downdelay_set(struct bonding *bond,
>  static int bond_option_use_carrier_set(struct bonding *bond,
>  				       const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting use_carrier to %llu\n",
> -		    newval->value);
> +	netdev_dbg(bond->dev, "Setting use_carrier to %llu\n", newval->value);
>  	bond->params.use_carrier = newval->value;
>  
>  	return 0;
> @@ -930,16 +929,16 @@ static int bond_option_use_carrier_set(struct bonding *bond,
>  static int bond_option_arp_interval_set(struct bonding *bond,
>  					const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting ARP monitoring interval to %llu\n",
> -		    newval->value);
> +	netdev_dbg(bond->dev, "Setting ARP monitoring interval to %llu\n",
> +		   newval->value);
>  	bond->params.arp_interval = newval->value;
>  	if (newval->value) {
>  		if (bond->params.miimon) {
> -			netdev_info(bond->dev, "ARP monitoring cannot be used with MII monitoring. Disabling MII monitoring\n");
> +			netdev_dbg(bond->dev, "ARP monitoring cannot be used with MII monitoring. Disabling MII monitoring\n");
>  			bond->params.miimon = 0;
>  		}
>  		if (!bond->params.arp_targets[0])
> -			netdev_info(bond->dev, "ARP monitoring has been set up, but no ARP targets have been specified\n");
> +			netdev_dbg(bond->dev, "ARP monitoring has been set up, but no ARP targets have been specified\n");
>  	}
>  	if (bond->dev->flags & IFF_UP) {
>  		/* If the interface is up, we may need to fire off
> @@ -1000,7 +999,7 @@ static int _bond_option_arp_ip_target_add(struct bonding *bond, __be32 target)
>  		return -EINVAL;
>  	}
>  
> -	netdev_info(bond->dev, "Adding ARP target %pI4\n", &target);
> +	netdev_dbg(bond->dev, "Adding ARP target %pI4\n", &target);
>  
>  	_bond_options_arp_ip_target_set(bond, ind, target, jiffies);
>  
> @@ -1036,7 +1035,7 @@ static int bond_option_arp_ip_target_rem(struct bonding *bond, __be32 target)
>  	if (ind == 0 && !targets[1] && bond->params.arp_interval)
>  		netdev_warn(bond->dev, "Removing last arp target with arp_interval on\n");
>  
> -	netdev_info(bond->dev, "Removing ARP target %pI4\n", &target);
> +	netdev_dbg(bond->dev, "Removing ARP target %pI4\n", &target);
>  
>  	bond_for_each_slave(bond, slave, iter) {
>  		targets_rx = slave->target_last_arp_rx;
> @@ -1088,8 +1087,8 @@ static int bond_option_arp_ip_targets_set(struct bonding *bond,
>  static int bond_option_arp_validate_set(struct bonding *bond,
>  					const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting arp_validate to %s (%llu)\n",
> -		    newval->string, newval->value);
> +	netdev_dbg(bond->dev, "Setting arp_validate to %s (%llu)\n",
> +		   newval->string, newval->value);
>  
>  	if (bond->dev->flags & IFF_UP) {
>  		if (!newval->value)
> @@ -1105,8 +1104,8 @@ static int bond_option_arp_validate_set(struct bonding *bond,
>  static int bond_option_arp_all_targets_set(struct bonding *bond,
>  					   const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting arp_all_targets to %s (%llu)\n",
> -		    newval->string, newval->value);
> +	netdev_dbg(bond->dev, "Setting arp_all_targets to %s (%llu)\n",
> +		   newval->string, newval->value);
>  	bond->params.arp_all_targets = newval->value;
>  
>  	return 0;
> @@ -1126,7 +1125,7 @@ static int bond_option_primary_set(struct bonding *bond,
>  		*p = '\0';
>  	/* check to see if we are clearing primary */
>  	if (!strlen(primary)) {
> -		netdev_info(bond->dev, "Setting primary slave to None\n");
> +		netdev_dbg(bond->dev, "Setting primary slave to None\n");
>  		RCU_INIT_POINTER(bond->primary_slave, NULL);
>  		memset(bond->params.primary, 0, sizeof(bond->params.primary));
>  		bond_select_active_slave(bond);
> @@ -1135,8 +1134,8 @@ 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_info(bond->dev, "Setting %s as primary slave\n",
> -				    slave->dev->name);
> +			netdev_dbg(bond->dev, "Setting %s as primary slave\n",
> +				   slave->dev->name);
>  			rcu_assign_pointer(bond->primary_slave, slave);
>  			strcpy(bond->params.primary, slave->dev->name);
>  			bond_select_active_slave(bond);
> @@ -1145,15 +1144,15 @@ static int bond_option_primary_set(struct bonding *bond,
>  	}
>  
>  	if (rtnl_dereference(bond->primary_slave)) {
> -		netdev_info(bond->dev, "Setting primary slave to None\n");
> +		netdev_dbg(bond->dev, "Setting primary slave to None\n");
>  		RCU_INIT_POINTER(bond->primary_slave, NULL);
>  		bond_select_active_slave(bond);
>  	}
>  	strncpy(bond->params.primary, primary, IFNAMSIZ);
>  	bond->params.primary[IFNAMSIZ - 1] = 0;
>  
> -	netdev_info(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 to %s yet\n",
> +		   primary, bond->dev->name);
>  
>  out:
>  	unblock_netpoll_tx();
> @@ -1164,8 +1163,8 @@ static int bond_option_primary_set(struct bonding *bond,
>  static int bond_option_primary_reselect_set(struct bonding *bond,
>  					    const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting primary_reselect to %s (%llu)\n",
> -		    newval->string, newval->value);
> +	netdev_dbg(bond->dev, "Setting primary_reselect to %s (%llu)\n",
> +		   newval->string, newval->value);
>  	bond->params.primary_reselect = newval->value;
>  
>  	block_netpoll_tx();
> @@ -1178,8 +1177,8 @@ static int bond_option_primary_reselect_set(struct bonding *bond,
>  static int bond_option_fail_over_mac_set(struct bonding *bond,
>  					 const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting fail_over_mac to %s (%llu)\n",
> -		    newval->string, newval->value);
> +	netdev_dbg(bond->dev, "Setting fail_over_mac to %s (%llu)\n",
> +		   newval->string, newval->value);
>  	bond->params.fail_over_mac = newval->value;
>  
>  	return 0;
> @@ -1188,8 +1187,8 @@ static int bond_option_fail_over_mac_set(struct bonding *bond,
>  static int bond_option_xmit_hash_policy_set(struct bonding *bond,
>  					    const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting xmit hash policy to %s (%llu)\n",
> -		    newval->string, newval->value);
> +	netdev_dbg(bond->dev, "Setting xmit hash policy to %s (%llu)\n",
> +		   newval->string, newval->value);
>  	bond->params.xmit_policy = newval->value;
>  
>  	return 0;
> @@ -1198,8 +1197,8 @@ static int bond_option_xmit_hash_policy_set(struct bonding *bond,
>  static int bond_option_resend_igmp_set(struct bonding *bond,
>  				       const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting resend_igmp to %llu\n",
> -		    newval->value);
> +	netdev_dbg(bond->dev, "Setting resend_igmp to %llu\n",
> +		   newval->value);
>  	bond->params.resend_igmp = newval->value;
>  
>  	return 0;
> @@ -1237,8 +1236,8 @@ static int bond_option_all_slaves_active_set(struct bonding *bond,
>  static int bond_option_min_links_set(struct bonding *bond,
>  				     const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting min links value to %llu\n",
> -		    newval->value);
> +	netdev_dbg(bond->dev, "Setting min links value to %llu\n",
> +		   newval->value);
>  	bond->params.min_links = newval->value;
>  	bond_set_carrier(bond);
>  
> @@ -1256,6 +1255,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_dbg(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 =
> @@ -1274,8 +1275,8 @@ static int bond_option_pps_set(struct bonding *bond,
>  static int bond_option_lacp_rate_set(struct bonding *bond,
>  				     const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting LACP rate to %s (%llu)\n",
> -		    newval->string, newval->value);
> +	netdev_dbg(bond->dev, "Setting LACP rate to %s (%llu)\n",
> +		   newval->string, newval->value);
>  	bond->params.lacp_fast = newval->value;
>  	bond_3ad_update_lacp_rate(bond);
>  
> @@ -1285,8 +1286,8 @@ static int bond_option_lacp_rate_set(struct bonding *bond,
>  static int bond_option_ad_select_set(struct bonding *bond,
>  				     const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting ad_select to %s (%llu)\n",
> -		    newval->string, newval->value);
> +	netdev_dbg(bond->dev, "Setting ad_select to %s (%llu)\n",
> +		   newval->string, newval->value);
>  	bond->params.ad_select = newval->value;
>  
>  	return 0;
> @@ -1377,12 +1378,12 @@ static int bond_option_slaves_set(struct bonding *bond,
>  
>  	switch (command[0]) {
>  	case '+':
> -		netdev_info(bond->dev, "Adding slave %s\n", dev->name);
> +		netdev_dbg(bond->dev, "Adding slave %s\n", dev->name);
>  		ret = bond_enslave(bond->dev, dev);
>  		break;
>  
>  	case '-':
> -		netdev_info(bond->dev, "Removing slave %s\n", dev->name);
> +		netdev_dbg(bond->dev, "Removing slave %s\n", dev->name);
>  		ret = bond_release(bond->dev, dev);
>  		break;
>  
> @@ -1402,8 +1403,8 @@ static int bond_option_slaves_set(struct bonding *bond,
>  static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
>  					  const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting dynamic-lb to %s (%llu)\n",
> -		    newval->string, newval->value);
> +	netdev_dbg(bond->dev, "Setting dynamic-lb to %s (%llu)\n",
> +		   newval->string, newval->value);
>  	bond->params.tlb_dynamic_lb = newval->value;
>  
>  	return 0;
> @@ -1412,8 +1413,8 @@ static int bond_option_tlb_dynamic_lb_set(struct bonding *bond,
>  static int bond_option_ad_actor_sys_prio_set(struct bonding *bond,
>  					     const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting ad_actor_sys_prio to %llu\n",
> -		    newval->value);
> +	netdev_dbg(bond->dev, "Setting ad_actor_sys_prio to %llu\n",
> +		   newval->value);
>  
>  	bond->params.ad_actor_sys_prio = newval->value;
>  	bond_3ad_update_ad_actor_settings(bond);
> @@ -1442,7 +1443,7 @@ static int bond_option_ad_actor_system_set(struct bonding *bond,
>  	if (!is_valid_ether_addr(mac))
>  		goto err;
>  
> -	netdev_info(bond->dev, "Setting ad_actor_system to %pM\n", mac);
> +	netdev_dbg(bond->dev, "Setting ad_actor_system to %pM\n", mac);
>  	ether_addr_copy(bond->params.ad_actor_system, mac);
>  	bond_3ad_update_ad_actor_settings(bond);
>  
> @@ -1456,8 +1457,8 @@ static int bond_option_ad_actor_system_set(struct bonding *bond,
>  static int bond_option_ad_user_port_key_set(struct bonding *bond,
>  					    const struct bond_opt_value *newval)
>  {
> -	netdev_info(bond->dev, "Setting ad_user_port_key to %llu\n",
> -		    newval->value);
> +	netdev_dbg(bond->dev, "Setting ad_user_port_key to %llu\n",
> +		   newval->value);
>  
>  	bond->params.ad_user_port_key = newval->value;
>  	return 0;
> 

Re: [PATCH] Add printk for bonding module packets_per_slave parameter

[Joe Perches]

On Tue, 2017-06-13 at 12:42 -0400, Jonathan Toppins wrote:
> On 06/13/2017 12:21 PM, Joe Perches wrote:
> > On Tue, 2017-06-13 at 11:34 -0400, David Miller wrote:
> > > 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.
[]
> > > 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.
> > 
> > I agree.  Noisy logging output is not good.
> > 
> > Perhaps a general conversion of the dozens
> > of existing netdev_info uses in this file to
> > netdev_dbg and adding this at netdev_dbg is
> > appropriate.
> 
> In general I agree. The few times I have debugged bonds, I always ended
> up enabling debug prinks anyway. I don't see a problem moving these to
> debug as well.
> 
> Adding nik whom converted a lot of this code to common paths for input.

If Nikolay agrees with the conversion, it's trivial.
Please submit it.  I did it just for reference.

Stylistic nits about the existing file:

There are some inconsistencies in pr_info/pr_err uses
with invalid inputs.

It would also be nicer if the forward static declarations
were removed and the static definitions reordered.

Re: [PATCH] Add printk for bonding module packets_per_slave parameter

[Nikolay Aleksandrov]

On 13/06/17 20:00, Joe Perches wrote:
> On Tue, 2017-06-13 at 12:42 -0400, Jonathan Toppins wrote:
>> On 06/13/2017 12:21 PM, Joe Perches wrote:
>>> On Tue, 2017-06-13 at 11:34 -0400, David Miller wrote:
>>>> 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.
> []
>>>> 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.
>>>
>>> I agree.  Noisy logging output is not good.
>>>
>>> Perhaps a general conversion of the dozens
>>> of existing netdev_info uses in this file to
>>> netdev_dbg and adding this at netdev_dbg is
>>> appropriate.
>>
>> In general I agree. The few times I have debugged bonds, I always ended
>> up enabling debug prinks anyway. I don't see a problem moving these to
>> debug as well.
>>
>> Adding nik whom converted a lot of this code to common paths for input.
> 
> If Nikolay agrees with the conversion, it's trivial.
> Please submit it.  I did it just for reference.
> 
> Stylistic nits about the existing file:
> 
> There are some inconsistencies in pr_info/pr_err uses
> with invalid inputs.
> 
> It would also be nicer if the forward static declarations
> were removed and the static definitions reordered.
> 

Agreed, there are many ways to extract the values.

Re: [PATCH] Add printk for bonding module packets_per_slave parameter

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Tue, 13 Jun 2017 09:21:03 -0700

> On Tue, 2017-06-13 at 11:34 -0400, David Miller wrote:
>> 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.
> 
> I agree.  Noisy logging output is not good.
> 
> Perhaps a general conversion of the dozens
> of existing netdev_info uses in this file to
> netdev_dbg and adding this at netdev_dbg is
> appropriate.
> 
> Something like:

Agreed, please submit this formally.

Re: [PATCH] Add printk for bonding module packets_per_slave parameter

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2015 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31513 bytes --]

Re: [PATCH] Add printk for bonding module packets_per_slave parameter

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1959 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 16527 bytes --]