summaryrefslogtreecommitdiff
path: root/drivers/net/bonding/bond_3ad.c
AgeCommit message (Collapse)Author
2020-05-10bonding: prevent out of bound accessesEric Dumazet
commit f87fda00b6ed232a817c655b8d179b48bde8fdbe upstream. ether_addr_equal_64bits() requires some care about its arguments, namely that 8 bytes might be read, even if last 2 byte values are not used. KASan detected a violation with null_mac_addr and lacpdu_mcast_addr in bond_3ad.c Same problem with mac_bcast[] and mac_v6_allmcast[] in bond_alb.c : Although the 8-byte alignment was there, KASan would detect out of bound accesses. Fixes: 815117adaf5b ("bonding: use ether_addr_equal_unaligned for bond addr compare") Fixes: bb54e58929f3 ("bonding: Verify RX LACPDU has proper dest mac-addr") Fixes: 885a136c52a8 ("bonding: use compare_ether_addr_64bits() in ALB") Signed-off-by: Eric Dumazet <edumazet@google.com> Reported-by: Dmitry Vyukov <dvyukov@google.com> Acked-by: Dmitry Vyukov <dvyukov@google.com> Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Acked-by: Ding Tianhong <dingtianhong@huawei.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-12-21bonding: fix 802.3ad state sent to partner when unbinding slaveToni Peltonen
[ Upstream commit 3b5b3a3331d141e8f2a7aaae3a94dfa1e61ecbe4 ] Previously when unbinding a slave the 802.3ad implementation only told partner that the port is not suitable for aggregation by setting the port aggregation state from aggregatable to individual. This is not enough. If the physical layer still stays up and we only unbinded this port from the bond there is nothing in the aggregation status alone to prevent the partner from sending traffic towards us. To ensure that the partner doesn't consider this port at all anymore we should also disable collecting and distributing to signal that this actor is going away. Also clear AD_STATE_SYNCHRONIZATION to ensure partner exits collecting + distributing state. I have tested this behaviour againts Arista EOS switches with mlx5 cards (physical link stays up even when interface is down) and simulated the same situation virtually Linux <-> Linux with two network namespaces running two veth device pairs. In both cases setting aggregation to individual doesn't alone prevent traffic from being to sent towards this port given that the link stays up in partners end. Partner still keeps it's end in collecting + distributing state and continues until timeout is reached. In most cases this means we are losing the traffic partner sends towards our port while we wait for timeout. This is most visible with slow periodic time (LACP rate slow). Other open source implementations like Open VSwitch and libreswitch, and vendor implementations like Arista EOS, seem to disable collecting + distributing to when doing similar port disabling/detaching/removing change. With this patch kernel implementation would behave the same way and ensure partner doesn't consider our actor viable anymore. Signed-off-by: Toni Peltonen <peltzi@peltzi.fi> Signed-off-by: Jay Vosburgh <jay.vosburgh@canonical.com> Acked-by: Jonathan Toppins <jtoppins@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
2015-11-02bonding: simplify / unify event handling code for 3ad mode.Mahesh Bandewar
Old logic of updating state-machine is not required since ad_update_actor_keys() does it implicitly. The only loss is the notification differentiation between speed vs. duplex change. Now only one unified notification is printed. Signed-off-by: Mahesh Bandewar <maheshb@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-11-02bonding: unify all places where actor-oper key needs to be updated.Mahesh Bandewar
actor_admin, and actor_oper key is changed at multiple locations in the code. This patch brings all those updates into one location in an attempt to avoid possible inconsistent updates causing LACP state machine to go in weird state. The unified place is ad_update_actor_key() with simple state-machine logic - (a) If port is "duplex" then only it can participate in LACP (b) Speed change reinitializes the LACP state-machine. Signed-off-by: Mahesh Bandewar <maheshb@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-11-02bonding: Simplify __get_duplex function.Mahesh Bandewar
Eliminate 'else' clause by simply initializing variable Signed-off-by: Mahesh Bandewar <maheshb@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-07-20bonding: trivial: remove unused variablesNikolay Aleksandrov
Get rid of these: drivers/net/bonding//bond_main.c: In function ‘bond_update_slave_arr’: drivers/net/bonding//bond_main.c:3754:6: warning: variable ‘slaves_in_agg’ set but not used [-Wunused-but-set-variable] int slaves_in_agg; ^ CC [M] drivers/net/bonding//bond_3ad.o drivers/net/bonding//bond_3ad.c: In function ‘ad_marker_response_received’: drivers/net/bonding//bond_3ad.c:1870:61: warning: parameter ‘marker’ set but not used [-Wunused-but-set-parameter] static void ad_marker_response_received(struct bond_marker *marker, ^ drivers/net/bonding//bond_3ad.c:1871:19: warning: parameter ‘port’ set but not used [-Wunused-but-set-parameter] struct port *port) ^ Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-05-11bonding: Implement user key part of port_key in an AD system.Mahesh Bandewar
The port key has three components - user-key, speed-part, and duplex-part. The LSBit is for the duplex-part, next 5 bits are for the speed while the remaining 10 bits are the user defined key bits. Get these 10 bits from the user-space (through the SysFs interface) and use it to form the admin port-key. Allowed range for the user-key is 0 - 1023 (10 bits). If it is not provided then use zero for the user-key-bits (default). It can set using following example code - # modprobe bonding mode=4 # usr_port_key=$(( RANDOM & 0x3FF )) # echo $usr_port_key > /sys/class/net/bond0/bonding/ad_user_port_key # echo +eth1 > /sys/class/net/bond0/bonding/slaves ... # ip link set bond0 up Signed-off-by: Mahesh Bandewar <maheshb@google.com> Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com> [jt: * fixed up style issues reported by checkpatch * fixed up context from change in ad_actor_sys_prio patch] Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-05-11bonding: Allow userspace to set actors' macaddr in an AD-system.Mahesh Bandewar
In an AD system, the communication between actor and partner is the business between these two entities. In the current setup anyone on the same L2 can "guess" the LACPDU contents and then possibly send the spoofed LACPDUs and trick the partner causing connectivity issues for the AD system. This patch allows to use a random mac-address obscuring it's identity making it harder for someone in the L2 is do the same thing. This patch allows user-space to choose the mac-address for the AD-system. This mac-address can not be NULL or a Multicast. If the mac-address is set from user-space; kernel will honor it and will not overwrite it. In the absence (value from user space); the logic will default to using the masters' mac as the mac-address for the AD-system. It can be set using example code below - # modprobe bonding mode=4 # sys_mac_addr=$(printf '%02x:%02x:%02x:%02x:%02x:%02x' \ $(( (RANDOM & 0xFE) | 0x02 )) \ $(( RANDOM & 0xFF )) \ $(( RANDOM & 0xFF )) \ $(( RANDOM & 0xFF )) \ $(( RANDOM & 0xFF )) \ $(( RANDOM & 0xFF ))) # echo $sys_mac_addr > /sys/class/net/bond0/bonding/ad_actor_system # echo +eth1 > /sys/class/net/bond0/bonding/slaves ... # ip link set bond0 up Signed-off-by: Mahesh Bandewar <maheshb@google.com> Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com> [jt: fixed up style issues reported by checkpatch] Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-05-11bonding: Allow userspace to set actors' system_priority in AD systemMahesh Bandewar
This patch allows user to randomize the system-priority in an ad-system. The allowed range is 1 - 0xFFFF while default value is 0xFFFF. If user does not specify this value, the system defaults to 0xFFFF, which is what it was before this patch. Following example code could set the value - # modprobe bonding mode=4 # sys_prio=$(( 1 + RANDOM + RANDOM )) # echo $sys_prio > /sys/class/net/bond0/bonding/ad_actor_sys_prio # echo +eth1 > /sys/class/net/bond0/bonding/slaves ... # ip link set bond0 up Signed-off-by: Mahesh Bandewar <maheshb@google.com> Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com> [jt: * fixed up style issues reported by checkpatch * changed how the default value is set in bond_check_params(), this makes the default consistent between what gets set for a new bond and what the default is claimed to be in the bonding options.] Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-04-08bonding: Remove unnecessary initializationMahesh Bandewar
bond_3ad_bind_slave() calls ad_initialize_port() and then immediately assigns correct values making some of that initialization unnecessary. Signed-off-by: Mahesh Bandewar <maheshb@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-04-08bonding: Code re-factoring for admin, oper-key operationsMahesh Bandewar
This patch breaks the rich assignments into it's own statements and removes some duplicate code where admin-key, & oper-key are updated. Signed-off-by: Mahesh Bandewar <maheshb@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-03-31bonding: Fix another case of LACPDU not sent on slaveMahesh Bandewar
When mii-mon discovers that the link is up, it will call bond_3ad_handle_link_change() but we forget to add the LACP_ENABLED flag when we discover the speed and duplex for the slave link are normal. Change-Id: Ie8b268ecfeea0f99bf9fdcd72706c0653f9d9e49 Signed-off-by: Mahesh Bandewar <maheshb@google.com> Signed-off-by: Andy Gospodarek <gospo@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-03-31bonding: deprecate BOND_MONITOR_CHURNED in favor of existing definitionsMahesh Bandewar
AD_PORT_ACTOR_CHURN and AD_PORT_PARTNER_CHURN are already present and essentially BOND_MONITOR_CHURNED is a combination of these two definitions. Signed-off-by: Mahesh Bandewar <maheshb@google.com> Signed-off-by: Andy Gospodarek <gospo@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-03-31bonding: Remove hardcoded initializationMahesh Bandewar
Signed-off-by: Mahesh Bandewar <maheshb@google.com> Signed-off-by: Andy Gospodarek <gospo@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-02-24bonding: Implement port churn-machine (AD standard 43.4.17).Mahesh Bandewar
The Churn Detection machines detect the situation where a port is operable, but the Actor and Partner have not attached the link to an Aggregator and brought the link into operation within a bound time period. Under normal operation of the LACP, agreement between Actor and Partner should be reached very rapidly. Continued failure to reach agreement can be symptomatic of device failure. Actor-churn-detection state-machine Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com> =================================== BEGIN=True + PortEnable=False | v +------------------------+ ActorPort.Sync=True +------------------+ | ACTOR_CHURN_MONITOR | ---------------------> | NO_ACTOR_CHURN | |========================| |==================| | ActorChurn=False | ActorPort.Sync=False | ActorChurn=False | | ActorChurn.Timer=Start | <--------------------- | | +------------------------+ +------------------+ | ^ | | ActorChurn.Timer=Expired | | ActorPort.Sync=True | | | +-----------------+ | | | ACTOR_CHURN | | | |=================| | +--------------> | ActorChurn=True | ------------+ | | +-----------------+ Similar for the Partner-churn-detection. Signed-off-by: Mahesh Bandewar <maheshb@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-02-24bonding: Verify RX LACPDU has proper dest mac-addrMahesh Bandewar
The 802.1AX standard states: "The DA in LACPDUs is the Slow_Protocols_Multicast address." This patch enforces that and drops LACPDUs with destination MAC addresses other than Slow_Protocols_Multicast address Signed-off-by: Mahesh Bandewar <maheshb@google.com> Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-01-27bonding: fix LACP PDU not sent on slave port sometimesSatish Ashok
When a slave is added to a bond and it is not in full duplex mode, AD_PORT_LACP_ENABLED flag is cleared, due to this LACP PDU is not sent on slave. When the duplex is changed to full, the flag needs to be set to send LACP PDU. Cc: Andy Gospodarek <gospo@cumulusnetworks.com> Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com> Signed-off-by: Satish Ashok <sashok@cumulusnetworks.com> Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com> Signed-off-by: Jay Vosburgh <jay.vosburgh@canonical.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2015-01-27bonding: fix incorrect lacp mux state when agg not activeWilson Kok
This patch attempts to fix the following problems when an actor or partner's aggregator is not active: 1. a slave's lacp port state is marked as AD_STATE_SYNCHRONIZATION even if it is attached to an inactive aggregator. LACP advertises this state to the partner, making the partner think he can move into COLLECTING_DISTRIBUTING state even though this link will not pass traffic on the local side 2. a slave goes into COLLECTING_DISTRIBUTING state without checking if the aggregator is actually active 3. when in COLLECTING_DISTRIBUTING state, the partner parameters may change, e.g. the partner_oper_port_state.SYNCHRONIZATION. The local mux machine is not reacting to the change and continue to keep the slave and bond up 4. When bond slave leaves an inactive aggregator and joins an active aggregator, the actor oper port state need to update to SYNC state. v2: * fix style issues in bond_3ad.c Cc: Andy Gospodarek <gospo@cumulusnetworks.com> Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com> Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com> Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-11-19bonding: Introduce 4 AD link speed to fix agg_bandwidthJianhua Xie
This patch adds [2.5|20|40|56] Gbps enum definition, and fixes aggregated bandwidth calculation based on above slave links. CC: Jay Vosburgh <j.vosburgh@gmail.com> CC: Veaceslav Falico <vfalico@gmail.com> CC: Andy Gospodarek <andy@greyhouse.net> CC: David S. Miller <davem@davemloft.net> Signed-off-by: Jianhua Xie <jianhua.xie@freescale.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-11-19bonding: change AD_LINK_SPEED_BITMASK to enum to suport more speedJianhua Xie
Port Key was determined as 16 bits according to the link speed, duplex and user key (which is yet not supported). In the old speed field, 5 bits are for speed [1|10|100|1000|10000]Mbps as below: -------------------------------------------------------------- Port key :| User key | Speed | Duplex| -------------------------------------------------------------- 16 6 1 0 This patch keeps the old layout, but changes AD_LINK_SPEED_BITMASK from bit type to an enum type. In this way, the speed field can expand speed type from 5 to 32. CC: Jay Vosburgh <j.vosburgh@gmail.com> CC: Veaceslav Falico <vfalico@gmail.com> CC: Andy Gospodarek <andy@greyhouse.net> CC: David S. Miller <davem@davemloft.net> Signed-off-by: Jianhua Xie <jianhua.xie@freescale.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-11-10net: Move bonding headers under include/netDavid S. Miller
This ways drivers like cxgb4 don't need to do ugly relative includes. Reported-by: Joe Perches <joe@perches.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-10-06bonding: Simplify the xmit function for modes that use xmit_hashMahesh Bandewar
Earlier change to use usable slave array for TLB mode had an additional performance advantage. So extending the same logic to all other modes that use xmit-hash for slave selection (viz 802.3AD, and XOR modes). Also consolidating this with the earlier TLB change. The main idea is to build the usable slaves array in the control path and use that array for slave selection during xmit operation. Measured performance in a setup with a bond of 4x1G NICs with 200 instances of netperf for the modes involved (3ad, xor, tlb) cmd: netperf -t TCP_RR -H <TargetHost> -l 60 -s 5 Mode TPS-Before TPS-After 802.3ad : 468,694 493,101 TLB (lb=0): 392,583 392,965 XOR : 475,696 484,517 Signed-off-by: Mahesh Bandewar <maheshb@google.com> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-09-15bonding: trivial: style and comment fixesNikolay Aleksandrov
First adjust a couple of locking comments that were left inaccurate, then adjust comments to use the netdev styling and remove extra new lines where necessary and add a couple of new lines between declarations and code. These are all trivial styling changes, no functional change. Also removed a couple of outdated or obvious comments. This patch is by no means a complete fix of all netdev style violations but it gets the bonding closer. Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-09-13bonding: 3ad: convert to bond->mode_lockNikolay Aleksandrov
Now that we have bond->mode_lock, we can remove the state_machine_lock and use it in its place. There're no fast paths requiring the per-port spinlocks so it should be okay to consolidate them into mode_lock. Also move it inside the unbinding function as we don't want to expose mode_lock outside of the specific modes. Suggested-by: Jay Vosburgh <jay.vosburgh@canonical.com> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-09-13bonding: convert curr_slave_lock to a spinlock and rename itNikolay Aleksandrov
curr_slave_lock is now a misleading name, a much better name is mode_lock as it'll be used for each mode's purposes and it's no longer necessary to use a rwlock, a simple spinlock is enough. Suggested-by: Jay Vosburgh <jay.vosburgh@canonical.com> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-09-13bonding: 3ad: clean up curr_slave_lock usageNikolay Aleksandrov
Remove the read_lock in bond_3ad_lacpdu_recv() since when the slave is being released its rx_handler is removed before 3ad unbind, so even if packets arrive, they won't see the slave in an inconsistent state. Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-09-09bonding: 3ad: use curr_slave_lock instead of bond->lockNikolay Aleksandrov
In 3ad mode the only syncing needed by bond->lock is for the wq and the recv handler, so change them to use curr_slave_lock. There're no locking dependencies here as 3ad doesn't use curr_slave_lock at all. Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-07-15bonding: remove pr_fmt from bond_3ad.cVeaceslav Falico
To maintain the same message structure as netdev_* functions print. CC: Jay Vosburgh <j.vosburgh@gmail.com> CC: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Veaceslav Falico <vfalico@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-07-15bonding: convert bond_3ad.c to use netdev_printk instead of pr_Veaceslav Falico
Several functions left out cause we might not have at that time a valid bond/slave/port. Also, converted severa pr_ratelimited into net_ratelimited. CC: Jay Vosburgh <j.vosburgh@gmail.com> CC: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Veaceslav Falico <vfalico@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-05-16bonding: replace SLAVE_IS_OK() with bond_slave_can_tx()Veaceslav Falico
They're verifying the same thing (except of IFF_UP, which is implied for netif_running(), which is also a prerequisite). CC: Jay Vosburgh <j.vosburgh@gmail.com> CC: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Veaceslav Falico <vfalico@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-05-16bonding: convert IS_UP(slave->dev) to inline functionVeaceslav Falico
Also, remove the IFF_UP verification cause we can't be netif_running() with being also IFF_UP. CC: Jay Vosburgh <j.vosburgh@gmail.com> CC: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Veaceslav Falico <vfalico@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-05-14bonding: alloc the structure ad_info dynamically in per slavedingtianhong
The struct ad_slave_info is very huge, and only be used for 802.3ad mode, so alloc the structure dynamically could save 356 Bits for every slave in non 802.3ad mode. Cc: Jay Vosburgh <j.vosburgh@gmail.com> Cc: Veaceslav Falico <vfalico@gmail.com> Cc: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> Acked-by: Veaceslav Falico <vfalico@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-04-24bonding: Changed hashing function to just provide hashMahesh Bandewar
Modified the hash function to return just hash separating from the modulo operation that can be performed by the caller. This is to make way for the tlb mode to use the same hashing policies that are used in the 802.3ad and Xor mode. Change-Id: I276609e87e0ca213c4d1b17b79c5e0b0f3d0dd6f Signed-off-by: Mahesh Bandewar <maheshb@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-03-18bonding: ratelimit pr_warn()s in 802.3ad modeVeaceslav Falico
Only ratelimit the ones that might spam, omiting the ones from enslave/deslave. CC: Jay Vosburgh <fubar@us.ibm.com> CC: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Veaceslav Falico <vfalico@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-03-12bonding: Call dev_kfree_skby_any instead of kfree_skb.Eric W. Biederman
Replace kfree_skb with dev_kfree_skb_any in functions that can be called in hard irq and other contexts. Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-03-05Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/netDavid S. Miller
Conflicts: drivers/net/wireless/ath/ath9k/recv.c drivers/net/wireless/mwifiex/pcie.c net/ipv6/sit.c The SIT driver conflict consists of a bug fix being done by hand in 'net' (missing u64_stats_init()) whilst in 'net-next' a helper was created (netdev_alloc_pcpu_stats()) which takes care of this. The two wireless conflicts were overlapping changes. Signed-off-by: David S. Miller <davem@davemloft.net>
2014-02-26bonding: Fix RTNL: assertion failed at net/core/rtnetlink.c for ab arp monitordingtianhong
Veaceslav has reported and fix this problem by commit f2ebd477f141bc0 (bonding: restructure locking of bond_ab_arp_probe()). According Jay's opinion, the current solution is not very well, because the notification is to indicate that the interface has actually changed state in a meaningful way, but these calls in the ab ARP monitor are internal settings of the flags to allow the ARP monitor to search for a slave to become active when there are no active slaves. The flag setting to active or backup is to permit the ARP monitor's response logic to do the right thing when deciding if the test slave (current_arp_slave) is up or not. So the best way to fix the problem is that we should not send a notification when the slave is in testing state, and check the state at the end of the monitor, if the slave's state recover, avoid to send pointless notification twice. And RTNL is really a big lock, hold it regardless the slave's state changed or not when the current_active_slave is null will loss performance (every 100ms), so we should hold it only when the slave's state changed and need to notify. I revert the old commit and add new modifications. Cc: Jay Vosburgh <fubar@us.ibm.com> Cc: Veaceslav Falico <vfalico@redhat.com> Cc: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-02-26bonding: Fix RTNL: assertion failed at net/core/rtnetlink.c for 802.3ad modedingtianhong
The problem was introduced by the commit 1d3ee88ae0d (bonding: add netlink attributes to slave link dev). The bond_set_active_slave() and bond_set_backup_slave() will use rtmsg_ifinfo to send slave's states, so these two functions should be called in RTNL. In 802.3ad mode, acquiring RTNL for the __enable_port and __disable_port cases is difficult, as those calls generally already hold the state machine lock, and cannot unconditionally call rtnl_lock because either they already hold RTNL (for calls via bond_3ad_unbind_slave) or due to the potential for deadlock with bond_3ad_adapter_speed_changed, bond_3ad_adapter_duplex_changed, bond_3ad_link_change, or bond_3ad_update_lacp_rate. All four of those are called with RTNL held, and acquire the state machine lock second. The calling contexts for __enable_port and __disable_port already hold the state machine lock, and may or may not need RTNL. According to the Jay's opinion, I don't think it is a problem that the slave don't send notify message synchronously when the status changed, normally the state machine is running every 100 ms, send the notify message at the end of the state machine if the slave's state changed should be better. I fix the problem through these steps: 1). add a new function bond_set_slave_state() which could change the slave's state and call rtmsg_ifinfo() according to the input parameters called notify. 2). Add a new slave parameter which called should_notify, if the slave's state changed and don't notify yet, the parameter will be set to 1, and then if the slave's state changed again, the param will be set to 0, it indicate that the slave's state has been restored, no need to notify any one. 3). the __enable_port and __disable_port should not call rtmsg_ifinfo in the state machine lock, any change in the state of slave could set a flag in the slave, it will indicated that an rtmsg_ifinfo should be called at the end of the state machine. Cc: Jay Vosburgh <fubar@us.ibm.com> Cc: Veaceslav Falico <vfalico@redhat.com> Cc: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-02-19bonding: Invert testJoe Perches
Make the error case return early. Make the normal return at the bottom of the function. Reduces indent for readability. Signed-off-by: Joe Perches <joe@perches.com> Reviewed-by: Ding Tianhong <dingtianhong@huawei.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-02-19bonding: More use of ether_addr_copyJoe Perches
It's smaller and faster for some architectures. Signed-off-by: Joe Perches <joe@perches.com> Reviewed-by: Ding Tianhong <dingtianhong@huawei.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-02-19Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/netDavid S. Miller
Conflicts: drivers/net/bonding/bond_3ad.h drivers/net/bonding/bond_main.c Two minor conflicts in bonding, both of which were overlapping changes. Signed-off-by: David S. Miller <davem@davemloft.net>
2014-02-17bonding: 802.3ad: make aggregator_identifier bond-privateJiri Bohac
aggregator_identifier is used to assign unique aggregator identifiers to aggregators of a bond during device enslaving. aggregator_identifier is currently a global variable that is zeroed in bond_3ad_initialize(). This sequence will lead to duplicate aggregator identifiers for eth1 and eth3: create bond0 change bond0 mode to 802.3ad enslave eth0 to bond0 //eth0 gets agg id 1 enslave eth1 to bond0 //eth1 gets agg id 2 create bond1 change bond1 mode to 802.3ad enslave eth2 to bond1 //aggregator_identifier is reset to 0 //eth2 gets agg id 1 enslave eth3 to bond0 //eth3 gets agg id 2 Fix this by making aggregator_identifier private to the bond. Signed-off-by: Jiri Bohac <jbohac@suse.cz> Acked-by: Veaceslav Falico <vfalico@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-02-17bonding: Convert memcpy(foo, bar, ETH_ALEN) to ether_addr_copy(foo, bar)Joe Perches
ether_addr_copy is smaller and faster for some architectures. This relies on a stack frame being at least __aligned(2) for one use of an Ethernet address on the stack. Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-02-17bonding: Neaten pr_<level>Joe Perches
Add missing terminating newlines. Convert uses of pr_info to pr_cont in bond_check_params. Standardize upper/lower case styles. Typo fixes, remove unnecessary parentheses and periods. Alignment neatening. Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-01-13bonding: fix __get_active_agg() RCU logicVeaceslav Falico
Currently, the implementation is meaningless - once again, we take the slave structure and use it after we've exited RCU critical section. Fix this by removing the rcu_read_lock() from __get_active_agg(), and ensuring that all its callers are holding RCU. Fixes: be79bd048 ("bonding: add RCU for bond_3ad_state_machine_handler()") CC: dingtianhong@huawei.com CC: Jay Vosburgh <fubar@us.ibm.com> CC: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Veaceslav Falico <vfalico@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-01-13bonding: fix __get_first_agg RCU usageVeaceslav Falico
Currently, the RCU read lock usage is just wrong - it gets the slave struct under RCU and continues to use it when RCU lock is released. However, it's still safe to do this cause we didn't need the rcu_read_lock() initially - all of the __get_first_agg() callers are either holding RCU read lock or the RTNL lock, so that we can't sync while in it. Fixes: be79bd048 ("bonding: add RCU for bond_3ad_state_machine_handler()") CC: dingtianhong@huawei.com CC: Jay Vosburgh <fubar@us.ibm.com> CC: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Veaceslav Falico <vfalico@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-01-13bonding: fix bond_3ad_set_carrier() RCU usageVeaceslav Falico
Currently, its usage is just plainly wrong. It first gets a slave under RCU, and, after releasing the RCU lock, continues to use it - whilst it can be freed. Fix this by ensuring that bond_3ad_set_carrier() holds RCU till it uses its slave (or its agg). Fixes: be79bd048ab ("bonding: add RCU for bond_3ad_state_machine_handler()") CC: dingtianhong@huawei.com CC: Jay Vosburgh <fubar@us.ibm.com> CC: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Veaceslav Falico <vfalico@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-01-12bonding: remove dead code from 3adVeaceslav Falico
That code has been around for ages without being used. CC: Jay Vosburgh <fubar@us.ibm.com> CC: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Veaceslav Falico <vfalico@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-01-12bonding: convert 3ad to use pr_warn instead of pr_warningVeaceslav Falico
CC: Jay Vosburgh <fubar@us.ibm.com> CC: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Veaceslav Falico <vfalico@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2014-01-12bonding: clean up style for bond_3ad.cVeaceslav Falico
It's a huge mess currently, that is really hard to read. This cleanup doesn't touch the logic at all, it only breaks easy-to-fix long lines and updates comment styles. CC: Jay Vosburgh <fubar@us.ibm.com> CC: Andy Gospodarek <andy@greyhouse.net> Signed-off-by: Veaceslav Falico <vfalico@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net>