Skip to contentSkip to conversation

bonding: add option lp_interval for loading module

PATCH Superseded Ding Tianhong 4 participants 6 messages Dec 20, 2013 – Dec 21, 2013

Subsystems (3)
Networking Network drivers Bonding driver

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Ding Tianhong

author
#0 lore raw

PATCH 1/3 bonding: add option lp_interval for loading module

The bond driver could set the lp_interval when loading module.

Signed-off-by Ding Tianhong <[email protected]>
---
 drivers/net/bonding/bond_main.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)
drivers/net/bonding/bond_main.c +13 -2
index c0456cc..368f7e4 100644
@@ -113,6 +113,7 @@ static int all_slaves_active;
 static struct bond_params bonding_defaults;
 static int resend_igmp = BOND_DEFAULT_RESEND_IGMP;
 static int packets_per_slave = 1;
+static int lp_interval = BOND_ALB_DEFAULT_LP_INTERVAL;
 
 module_param(max_bonds, int, 0);
 MODULE_PARM_DESC(max_bonds, "Max number of bonded devices");

@@ -189,7 +190,11 @@ module_param(packets_per_slave, int, 0);
 MODULE_PARM_DESC(packets_per_slave, "Packets to send per slave in balance-rr "
 				    "mode; 0 for a random slave, 1 packet per "
 				    "slave (default), >1 packets per slave.");
-
+module_param(lp_interval, uint, 0);
+MODULE_PARM_DESC(lp_interval, "The number of seconds between instances where "
+			      "the bonding driver sends learning packets to "
+			      "each slaves peer switch. The default is 1.");
+	
 /*----------------------------- Global variables ----------------------------*/
 
 #ifdef CONFIG_NET_POLL_CONTROLLER

@@ -4271,6 +4276,12 @@ static int bond_check_params(struct bond_params *params)
 		fail_over_mac_value = BOND_FOM_NONE;
 	}
 
+	if (lp_interval == 0) {
+		pr_warning("Warning: ip_interval must be between 1 and %d, so it was reset to %d\n",
+			INT_MAX, BOND_ALB_DEFAULT_LP_INTERVAL);
+		lp_interval = BOND_ALB_DEFAULT_LP_INTERVAL;
+	}
+
 	/* fill params struct with the proper values */
 	params->mode = bond_mode;
 	params->xmit_policy = xmit_hashtype;

@@ -4290,7 +4301,7 @@ static int bond_check_params(struct bond_params *params)
 	params->all_slaves_active = all_slaves_active;
 	params->resend_igmp = resend_igmp;
 	params->min_links = min_links;
-	params->lp_interval = BOND_ALB_DEFAULT_LP_INTERVAL;
+	params->lp_interval = lp_interval;
 	if (packets_per_slave > 1)
 		params->packets_per_slave = reciprocal_value(packets_per_slave);
 	else
Signature · 1 line
1.8.0
#1 lore raw

Hello.

Quoted 11 lines · Ding Tianhong

The bond driver could set the lp_interval when loading module.

Signed-off-by Ding Tianhong <[email protected]>
---
  drivers/net/bonding/bond_main.c | 15 +++++++++++++--
  1 file changed, 13 insertions(+), 2 deletions(-)
drivers/net/bonding/bond_main.c +0 -0
index c0456cc..368f7e4 100644

[...]

drivers/net/bonding/bond_main.c static int bond_check_params(struct bond_params *params) · line 4281

Quoted 7 lines
drivers/net/bonding/bond_main.c +3 -0
@@ -4271,6 +4276,12 @@ static int bond_check_params(struct bond_params *params)
  		fail_over_mac_value = BOND_FOM_NONE;
  	}
 
+	if (lp_interval == 0) {
+		pr_warning("Warning: ip_interval must be between 1 and %d, so it was reset to %d\n",
+			INT_MAX, BOND_ALB_DEFAULT_LP_INTERVAL);

The continuation line should be aligned under ", according to the
networking coding style.

WBR, Sergei

author
#2 lore raw
Quoted 20 lines · Sergei Shtylyov

Hello.

Quoted 11 lines · Ding Tianhong

The bond driver could set the lp_interval when loading module.

Signed-off-by Ding Tianhong <[email protected]>
---
  drivers/net/bonding/bond_main.c | 15 +++++++++++++--
  1 file changed, 13 insertions(+), 2 deletions(-)
drivers/net/bonding/bond_main.c +0 -0
index c0456cc..368f7e4 100644

[...]

Quoted 4 lines
drivers/net/bonding/bond_main.c +0 -0
@@ -4271,6 +4276,12 @@ static int bond_check_params(struct bond_params *params)
          fail_over_mac_value = BOND_FOM_NONE;
      }
Quoted 9 lines

drivers/net/bonding/bond_main.c static int bond_check_params(struct bond_params *params) · line 4281

Quoted 3 lines
drivers/net/bonding/bond_main.c +3 -0
+    if (lp_interval == 0) {
+        pr_warning("Warning: ip_interval must be between 1 and %d, so it was reset to %d\n",
+            INT_MAX, BOND_ALB_DEFAULT_LP_INTERVAL);

The continuation line should be aligned under ", according to the networking coding style.

WBR, Sergei

Ok, I will fix it, thanks.

Regards
Ding

Quoted 1 line
#3 lore raw
In reply to Ding Tianhong

drivers/net/bonding/bond_main.c static int bond_check_params(struct bond_params *params) · line 4284

Quoted 10 lines · Ding Tianhong
drivers/net/bonding/bond_main.c +6 -0
@@ -4271,6 +4276,12 @@ static int bond_check_params(struct bond_params *params)
 		fail_over_mac_value = BOND_FOM_NONE;
 	}
 
+	if (lp_interval == 0) {
+		pr_warning("Warning: ip_interval must be between 1 and %d, so it was reset to %d\n",
+			INT_MAX, BOND_ALB_DEFAULT_LP_INTERVAL);
+		lp_interval = BOND_ALB_DEFAULT_LP_INTERVAL;
+	}
+

What do you think about using the new bond_option_xxx_set() functions for module parameters also? The above code, for example, is duplicated in bond_option_lp_interval_set(). This way, we have a unified solution, where option bounds checking, compatibility, warnings, etc happened in one place for all three modes of input: sysfs, netlink, and module params.

-scott

#4 lore raw
In reply to Ding Tianhong

drivers/net/bonding/bond_main.c static int bond_check_params(struct bond_params *params) · line 4284

Quoted 10 lines · Ding Tianhong
drivers/net/bonding/bond_main.c +6 -0
@@ -4271,6 +4276,12 @@ static int bond_check_params(struct bond_params *params)
 		fail_over_mac_value = BOND_FOM_NONE;
 	}
 
+	if (lp_interval == 0) {
+		pr_warning("Warning: ip_interval must be between 1 and %d, so it was reset to %d\n",
+			INT_MAX, BOND_ALB_DEFAULT_LP_INTERVAL);
+		lp_interval = BOND_ALB_DEFAULT_LP_INTERVAL;
+	}
+

[sorry, replied with wrong email]

What do you think about using the new bond_option_xxx_set() functions for module parameters also? The above code, for example, is duplicated in bond_option_lp_interval_set(). This way, we have a unified solution, where option bounds checking, compatibility, warnings, etc happened in one place for all three modes of input: sysfs, netlink, and module params.

-scott

author
#5 lore raw
Quoted 16 lines · Scott Feldman

drivers/net/bonding/bond_main.c static int bond_check_params(struct bond_params *params) · line 4284

Quoted 10 lines · Ding Tianhong
drivers/net/bonding/bond_main.c +6 -0
@@ -4271,6 +4276,12 @@ static int bond_check_params(struct bond_params *params)
 		fail_over_mac_value = BOND_FOM_NONE;
 	}
 
+	if (lp_interval == 0) {
+		pr_warning("Warning: ip_interval must be between 1 and %d, so it was reset to %d\n",
+			INT_MAX, BOND_ALB_DEFAULT_LP_INTERVAL);
+		lp_interval = BOND_ALB_DEFAULT_LP_INTERVAL;
+	}
+

[sorry, replied with wrong email]

Quoted 4 lines

What do you think about using the new bond_option_xxx_set() functions for module parameters also? The above code, for example, is duplicated in bond_option_lp_interval_set(). This way, we have a unified solution, where option bounds checking, compatibility, warnings, etc happened in one place for all three modes of input: sysfs, netlink, and module params.

-scott

Yes, I agree with you, and I will wait for you to finish the bond_option_xxx_set() and then make the bond_check_params() use them, I think the code will be more simplify. Thanks

Regards
ding

Quoted 6 lines

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html