Commit b9f602533e2f5c32a09a3a75904e5373cb6e6377
Committed by
David S. Miller
1 parent
c127bdf9f6
Exists in
master
and in
39 other branches
bonding: make ab_arp select active slaves as other modes
When I was implementing primary_passive option (formely named primary_lazy) I've run into troubles with ab_arp. This is the only mode which is not using bond_select_active_slave() function to select active slave and instead it selects it itself. This seems to be not the right behaviour and it would be better to do it in bond_select_active_slave() for all cases. This patch makes this happen. Please review. Signed-off-by: Jiri Pirko <jpirko@redhat.com> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 1 changed file with 35 additions and 87 deletions Side-by-side Diff
drivers/net/bonding/bond_main.c
... | ... | @@ -1093,15 +1093,8 @@ |
1093 | 1093 | return NULL; /* still no slave, return NULL */ |
1094 | 1094 | } |
1095 | 1095 | |
1096 | - /* | |
1097 | - * first try the primary link; if arping, a link must tx/rx | |
1098 | - * traffic before it can be considered the curr_active_slave. | |
1099 | - * also, we would skip slaves between the curr_active_slave | |
1100 | - * and primary_slave that may be up and able to arp | |
1101 | - */ | |
1102 | 1096 | if ((bond->primary_slave) && |
1103 | - (!bond->params.arp_interval) && | |
1104 | - (IS_UP(bond->primary_slave->dev))) { | |
1097 | + bond->primary_slave->link == BOND_LINK_UP) { | |
1105 | 1098 | new_active = bond->primary_slave; |
1106 | 1099 | } |
1107 | 1100 | |
... | ... | @@ -1109,15 +1102,14 @@ |
1109 | 1102 | old_active = new_active; |
1110 | 1103 | |
1111 | 1104 | bond_for_each_slave_from(bond, new_active, i, old_active) { |
1112 | - if (IS_UP(new_active->dev)) { | |
1113 | - if (new_active->link == BOND_LINK_UP) { | |
1114 | - return new_active; | |
1115 | - } else if (new_active->link == BOND_LINK_BACK) { | |
1116 | - /* link up, but waiting for stabilization */ | |
1117 | - if (new_active->delay < mintime) { | |
1118 | - mintime = new_active->delay; | |
1119 | - bestslave = new_active; | |
1120 | - } | |
1105 | + if (new_active->link == BOND_LINK_UP) { | |
1106 | + return new_active; | |
1107 | + } else if (new_active->link == BOND_LINK_BACK && | |
1108 | + IS_UP(new_active->dev)) { | |
1109 | + /* link up, but waiting for stabilization */ | |
1110 | + if (new_active->delay < mintime) { | |
1111 | + mintime = new_active->delay; | |
1112 | + bestslave = new_active; | |
1121 | 1113 | } |
1122 | 1114 | } |
1123 | 1115 | } |
... | ... | @@ -2932,18 +2924,6 @@ |
2932 | 2924 | } |
2933 | 2925 | } |
2934 | 2926 | |
2935 | - read_lock(&bond->curr_slave_lock); | |
2936 | - | |
2937 | - /* | |
2938 | - * Trigger a commit if the primary option setting has changed. | |
2939 | - */ | |
2940 | - if (bond->primary_slave && | |
2941 | - (bond->primary_slave != bond->curr_active_slave) && | |
2942 | - (bond->primary_slave->link == BOND_LINK_UP)) | |
2943 | - commit++; | |
2944 | - | |
2945 | - read_unlock(&bond->curr_slave_lock); | |
2946 | - | |
2947 | 2927 | return commit; |
2948 | 2928 | } |
2949 | 2929 | |
2950 | 2930 | |
2951 | 2931 | |
2952 | 2932 | |
2953 | 2933 | |
2954 | 2934 | |
2955 | 2935 | |
2956 | 2936 | |
2957 | 2937 | |
2958 | 2938 | |
2959 | 2939 | |
2960 | 2940 | |
2961 | 2941 | |
2962 | 2942 | |
2963 | 2943 | |
2964 | 2944 | |
2965 | 2945 | |
... | ... | @@ -2964,90 +2944,58 @@ |
2964 | 2944 | continue; |
2965 | 2945 | |
2966 | 2946 | case BOND_LINK_UP: |
2967 | - write_lock_bh(&bond->curr_slave_lock); | |
2968 | - | |
2969 | - if (!bond->curr_active_slave && | |
2970 | - time_before_eq(jiffies, dev_trans_start(slave->dev) + | |
2971 | - delta_in_ticks)) { | |
2947 | + if ((!bond->curr_active_slave && | |
2948 | + time_before_eq(jiffies, | |
2949 | + dev_trans_start(slave->dev) + | |
2950 | + delta_in_ticks)) || | |
2951 | + bond->curr_active_slave != slave) { | |
2972 | 2952 | slave->link = BOND_LINK_UP; |
2973 | - bond_change_active_slave(bond, slave); | |
2974 | 2953 | bond->current_arp_slave = NULL; |
2975 | 2954 | |
2976 | 2955 | pr_info(DRV_NAME |
2977 | - ": %s: %s is up and now the " | |
2978 | - "active interface\n", | |
2979 | - bond->dev->name, slave->dev->name); | |
2956 | + ": %s: link status definitely " | |
2957 | + "up for interface %s.\n", | |
2958 | + bond->dev->name, slave->dev->name); | |
2980 | 2959 | |
2981 | - } else if (bond->curr_active_slave != slave) { | |
2982 | - /* this slave has just come up but we | |
2983 | - * already have a current slave; this can | |
2984 | - * also happen if bond_enslave adds a new | |
2985 | - * slave that is up while we are searching | |
2986 | - * for a new slave | |
2987 | - */ | |
2988 | - slave->link = BOND_LINK_UP; | |
2989 | - bond_set_slave_inactive_flags(slave); | |
2990 | - bond->current_arp_slave = NULL; | |
2960 | + if (!bond->curr_active_slave || | |
2961 | + (slave == bond->primary_slave)) | |
2962 | + goto do_failover; | |
2991 | 2963 | |
2992 | - pr_info(DRV_NAME | |
2993 | - ": %s: backup interface %s is now up\n", | |
2994 | - bond->dev->name, slave->dev->name); | |
2995 | 2964 | } |
2996 | 2965 | |
2997 | - write_unlock_bh(&bond->curr_slave_lock); | |
2966 | + continue; | |
2998 | 2967 | |
2999 | - break; | |
3000 | - | |
3001 | 2968 | case BOND_LINK_DOWN: |
3002 | 2969 | if (slave->link_failure_count < UINT_MAX) |
3003 | 2970 | slave->link_failure_count++; |
3004 | 2971 | |
3005 | 2972 | slave->link = BOND_LINK_DOWN; |
2973 | + bond_set_slave_inactive_flags(slave); | |
3006 | 2974 | |
3007 | - if (slave == bond->curr_active_slave) { | |
3008 | - pr_info(DRV_NAME | |
3009 | - ": %s: link status down for active " | |
3010 | - "interface %s, disabling it\n", | |
3011 | - bond->dev->name, slave->dev->name); | |
2975 | + pr_info(DRV_NAME | |
2976 | + ": %s: link status definitely down for " | |
2977 | + "interface %s, disabling it\n", | |
2978 | + bond->dev->name, slave->dev->name); | |
3012 | 2979 | |
3013 | - bond_set_slave_inactive_flags(slave); | |
3014 | - | |
3015 | - write_lock_bh(&bond->curr_slave_lock); | |
3016 | - | |
3017 | - bond_select_active_slave(bond); | |
3018 | - if (bond->curr_active_slave) | |
3019 | - bond->curr_active_slave->jiffies = | |
3020 | - jiffies; | |
3021 | - | |
3022 | - write_unlock_bh(&bond->curr_slave_lock); | |
3023 | - | |
2980 | + if (slave == bond->curr_active_slave) { | |
3024 | 2981 | bond->current_arp_slave = NULL; |
3025 | - | |
3026 | - } else if (slave->state == BOND_STATE_BACKUP) { | |
3027 | - pr_info(DRV_NAME | |
3028 | - ": %s: backup interface %s is now down\n", | |
3029 | - bond->dev->name, slave->dev->name); | |
3030 | - | |
3031 | - bond_set_slave_inactive_flags(slave); | |
2982 | + goto do_failover; | |
3032 | 2983 | } |
3033 | - break; | |
3034 | 2984 | |
2985 | + continue; | |
2986 | + | |
3035 | 2987 | default: |
3036 | 2988 | pr_err(DRV_NAME |
3037 | 2989 | ": %s: impossible: new_link %d on slave %s\n", |
3038 | 2990 | bond->dev->name, slave->new_link, |
3039 | 2991 | slave->dev->name); |
2992 | + continue; | |
3040 | 2993 | } |
3041 | - } | |
3042 | 2994 | |
3043 | - /* | |
3044 | - * No race with changes to primary via sysfs, as we hold rtnl. | |
3045 | - */ | |
3046 | - if (bond->primary_slave && | |
3047 | - (bond->primary_slave != bond->curr_active_slave) && | |
3048 | - (bond->primary_slave->link == BOND_LINK_UP)) { | |
2995 | +do_failover: | |
2996 | + ASSERT_RTNL(); | |
3049 | 2997 | write_lock_bh(&bond->curr_slave_lock); |
3050 | - bond_change_active_slave(bond, bond->primary_slave); | |
2998 | + bond_select_active_slave(bond); | |
3051 | 2999 | write_unlock_bh(&bond->curr_slave_lock); |
3052 | 3000 | } |
3053 | 3001 |