From d5e73f7be850323ae3adbbe84ed37a38b0c31476 Mon Sep 17 00:00:00 2001 From: Mahesh Bandewar Date: Wed, 8 Mar 2017 10:55:51 -0800 Subject: [PATCH 1/5] bonding: restructure arp-monitor In preparation to move the work-queue initialization to port creation from current port_open phase. Work-queue initialization does not make sense every time we do 'ifup/ifdown'. So moving to port creation phase. Arp monitoring work depends on the bonding mode and that is not tied to the port creation and can change anytime during the life after port creation. So this restructuring allows us to move the initialization at creation without losing the ability to arm the correct work for the mode user has selected. Signed-off-by: Mahesh Bandewar Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 8a4ba8b88e52..619f0c65f18a 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -2575,10 +2575,8 @@ static bool bond_time_in_interval(struct bonding *bond, unsigned long last_act, * arp is transmitted to generate traffic. see activebackup_arp_monitor for * arp monitoring in active backup mode. */ -static void bond_loadbalance_arp_mon(struct work_struct *work) +static void bond_loadbalance_arp_mon(struct bonding *bond) { - struct bonding *bond = container_of(work, struct bonding, - arp_work.work); struct slave *slave, *oldcurrent; struct list_head *iter; int do_failover = 0, slave_state_changed = 0; @@ -2916,10 +2914,8 @@ check_state: return should_notify_rtnl; } -static void bond_activebackup_arp_mon(struct work_struct *work) +static void bond_activebackup_arp_mon(struct bonding *bond) { - struct bonding *bond = container_of(work, struct bonding, - arp_work.work); bool should_notify_peers = false; bool should_notify_rtnl = false; int delta_in_ticks; @@ -2972,6 +2968,17 @@ re_arm: } } +static void bond_arp_monitor(struct work_struct *work) +{ + struct bonding *bond = container_of(work, struct bonding, + arp_work.work); + + if (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP) + bond_activebackup_arp_mon(bond); + else + bond_loadbalance_arp_mon(bond); +} + /*-------------------------- netdev event handling --------------------------*/ /* Change device name */ @@ -3228,10 +3235,7 @@ static void bond_work_init_all(struct bonding *bond) bond_resend_igmp_join_requests_delayed); INIT_DELAYED_WORK(&bond->alb_work, bond_alb_monitor); INIT_DELAYED_WORK(&bond->mii_work, bond_mii_monitor); - if (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP) - INIT_DELAYED_WORK(&bond->arp_work, bond_activebackup_arp_mon); - else - INIT_DELAYED_WORK(&bond->arp_work, bond_loadbalance_arp_mon); + INIT_DELAYED_WORK(&bond->arp_work, bond_arp_monitor); INIT_DELAYED_WORK(&bond->ad_work, bond_3ad_state_machine_handler); INIT_DELAYED_WORK(&bond->slave_arr_work, bond_slave_arr_handler); } From 4493b81bea24269df898339dee638d7c5cb2b2df Mon Sep 17 00:00:00 2001 From: Mahesh Bandewar Date: Wed, 8 Mar 2017 10:55:54 -0800 Subject: [PATCH 2/5] bonding: initialize work-queues during creation of bond Initializing work-queues every time ifup operation performed is unnecessary and can be performed only once when the port is created. Signed-off-by: Mahesh Bandewar Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 619f0c65f18a..1329110ed85f 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -3270,8 +3270,6 @@ static int bond_open(struct net_device *bond_dev) } } - bond_work_init_all(bond); - if (bond_is_lb(bond)) { /* bond_alb_initialize must be called before the timer * is started. @@ -4691,6 +4689,8 @@ int bond_create(struct net *net, const char *name) netif_carrier_off(bond_dev); + bond_work_init_all(bond); + rtnl_unlock(); if (res < 0) bond_destructor(bond_dev); From 8b426dc54cf4056984bab7dfa48c92ee79a46434 Mon Sep 17 00:00:00 2001 From: Mahesh Bandewar Date: Wed, 8 Mar 2017 10:55:56 -0800 Subject: [PATCH 3/5] bonding: remove hardcoded value Eliminate hard-coded value and use the default that is set. Signed-off-by: Mahesh Bandewar Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 1329110ed85f..0f9f5ceae80e 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -4254,6 +4254,7 @@ static int bond_check_params(struct bond_params *params) int arp_all_targets_value; u16 ad_actor_sys_prio = 0; u16 ad_user_port_key = 0; + int tlb_dynamic_lb = 0; /* Convert string parameters. */ if (mode) { @@ -4566,6 +4567,17 @@ static int bond_check_params(struct bond_params *params) } ad_user_port_key = valptr->value; + if (bond_mode == BOND_MODE_TLB) { + bond_opt_initstr(&newval, "default"); + valptr = bond_opt_parse(bond_opt_get(BOND_OPT_TLB_DYNAMIC_LB), + &newval); + if (!valptr) { + pr_err("Error: No tlb_dynamic_lb default value"); + return -EINVAL; + } + tlb_dynamic_lb = valptr->value; + } + if (lp_interval == 0) { pr_warn("Warning: ip_interval must be between 1 and %d, so it was reset to %d\n", INT_MAX, BOND_ALB_DEFAULT_LP_INTERVAL); @@ -4593,7 +4605,7 @@ static int bond_check_params(struct bond_params *params) params->min_links = min_links; params->lp_interval = lp_interval; params->packets_per_slave = packets_per_slave; - params->tlb_dynamic_lb = 1; /* Default value */ + params->tlb_dynamic_lb = tlb_dynamic_lb; params->ad_actor_sys_prio = ad_actor_sys_prio; eth_zero_addr(params->ad_actor_system); params->ad_user_port_key = ad_user_port_key; From ec891c8b8da2c3862a5f76ba72f6d140c418d812 Mon Sep 17 00:00:00 2001 From: Mahesh Bandewar Date: Wed, 8 Mar 2017 10:55:59 -0800 Subject: [PATCH 4/5] bonding: remove "port-moved" state that was never implemented LACP state-machine defines "port-moved" state when the same ActorSystemID and Port are seen in a LACPDU received on different port. The state is never set since it's not implemented. However the state-machine attempts to clear that state occasionally. LACP state machine is already complicated and since this state is not implemented, removing it's checks makes the state-machine little simpler. Signed-off-by: Mahesh Bandewar Signed-off-by: David S. Miller --- drivers/net/bonding/bond_3ad.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index edc70ffad660..431926bba9f4 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -1052,8 +1052,7 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port) port->sm_rx_state = AD_RX_INITIALIZE; port->sm_vars |= AD_PORT_CHURNED; /* check if port is not enabled */ - } else if (!(port->sm_vars & AD_PORT_BEGIN) - && !port->is_enabled && !(port->sm_vars & AD_PORT_MOVED)) + } else if (!(port->sm_vars & AD_PORT_BEGIN) && !port->is_enabled) port->sm_rx_state = AD_RX_PORT_DISABLED; /* check if new lacpdu arrived */ else if (lacpdu && ((port->sm_rx_state == AD_RX_EXPIRED) || @@ -1081,11 +1080,8 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port) /* if no lacpdu arrived and no timer is on */ switch (port->sm_rx_state) { case AD_RX_PORT_DISABLED: - if (port->sm_vars & AD_PORT_MOVED) - port->sm_rx_state = AD_RX_INITIALIZE; - else if (port->is_enabled - && (port->sm_vars - & AD_PORT_LACP_ENABLED)) + if (port->is_enabled && + (port->sm_vars & AD_PORT_LACP_ENABLED)) port->sm_rx_state = AD_RX_EXPIRED; else if (port->is_enabled && ((port->sm_vars @@ -1115,7 +1111,6 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port) port->sm_vars &= ~AD_PORT_SELECTED; __record_default(port); port->actor_oper_port_state &= ~AD_STATE_EXPIRED; - port->sm_vars &= ~AD_PORT_MOVED; port->sm_rx_state = AD_RX_PORT_DISABLED; /* Fall Through */ From dc9c4d0fe023b508f1b41dbe2a3f3133f81b4d29 Mon Sep 17 00:00:00 2001 From: Mahesh Bandewar Date: Wed, 8 Mar 2017 10:56:02 -0800 Subject: [PATCH 5/5] bonding: reduce scope of some global variables Many of the bond param variables are declared global while it's not really necessary for these variables to be global. So moving them to the location these are used. Signed-off-by: Mahesh Bandewar Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 0f9f5ceae80e..ba934020dfaa 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -201,12 +201,6 @@ atomic_t netpoll_block_tx = ATOMIC_INIT(0); unsigned int bond_net_id __read_mostly; -static __be32 arp_target[BOND_MAX_ARP_TARGETS]; -static int arp_ip_count; -static int bond_mode = BOND_MODE_ROUNDROBIN; -static int xmit_hashtype = BOND_XMIT_POLICY_LAYER2; -static int lacp_fast; - /*-------------------------- Forward declarations ---------------------------*/ static int bond_init(struct net_device *bond_dev); @@ -4254,6 +4248,11 @@ static int bond_check_params(struct bond_params *params) int arp_all_targets_value; u16 ad_actor_sys_prio = 0; u16 ad_user_port_key = 0; + __be32 arp_target[BOND_MAX_ARP_TARGETS]; + int arp_ip_count; + int bond_mode = BOND_MODE_ROUNDROBIN; + int xmit_hashtype = BOND_XMIT_POLICY_LAYER2; + int lacp_fast = 0; int tlb_dynamic_lb = 0; /* Convert string parameters. */