From: Alex Wang Date: Wed, 16 Apr 2014 23:00:40 +0000 (-0700) Subject: bfd: Require bfd control packet received in forwarding_if_rx mode. X-Git-Tag: sliver-openvswitch-2.2.90-1~3^2~23 X-Git-Url: http://git.onelab.eu/?p=sliver-openvswitch.git;a=commitdiff_plain;h=34c88624ad02129a1b477717fe5d3928530dccbe bfd: Require bfd control packet received in forwarding_if_rx mode. This commit adds a requirement that bfd session must receive at least one bfd control packet every 100 * bfd->cfg_min_rx amount of time in forwarding_if_rx mode. Otherwise, even if the data packets are received on the monitored interface, the bfd->forwarding is still false. Since the datapath flow is not purged when the userspace Open Vswitch crashes, data packet can still be forwarded through the tunnel and fool the remote BFD session in forwarding_if_rx mode. Thus, this commit can prevent the remote BFD session from falsely declaring tunnel liveness in this situation. Signed-off-by: Alex Wang Acked-by: Ethan Jackson --- diff --git a/lib/bfd.c b/lib/bfd.c index e3e3ae5f1..2d53bd27d 100644 --- a/lib/bfd.c +++ b/lib/bfd.c @@ -203,6 +203,12 @@ struct bfd { bool forwarding_if_rx; long long int forwarding_if_rx_detect_time; + /* When 'bfd->forwarding_if_rx' is set, at least one bfd control packet + * is required to be received every 100 * bfd->cfg_min_rx. If bfd + * control packet is not received within this interval, even if data + * packets are received, the bfd->forwarding will still be false. */ + long long int demand_rx_bfd_time; + /* BFD decay related variables. */ bool in_decay; /* True when bfd is in decay. */ int decay_min_rx; /* min_rx is set to decay_min_rx when */ @@ -841,6 +847,10 @@ bfd_process_packet(struct bfd *bfd, const struct flow *flow, } /* XXX: RFC 5880 Section 6.8.6 Demand mode related calculations here. */ + if (bfd->forwarding_if_rx) { + bfd->demand_rx_bfd_time = time_msec() + 100 * bfd->cfg_min_rx; + } + out: bfd_forwarding__(bfd); ovs_mutex_unlock(&mutex); @@ -876,19 +886,22 @@ bfd_set_netdev(struct bfd *bfd, const struct netdev *netdev) static bool bfd_forwarding__(struct bfd *bfd) OVS_REQUIRES(mutex) { - long long int time; + long long int now = time_msec(); + bool forwarding_if_rx; bool last_forwarding = bfd->last_forwarding; if (bfd->forwarding_override != -1) { return bfd->forwarding_override == 1; } - time = bfd->forwarding_if_rx_detect_time; - bfd->last_forwarding = (bfd->state == STATE_UP - || (bfd->forwarding_if_rx && time > time_msec())) - && bfd->rmt_diag != DIAG_PATH_DOWN - && bfd->rmt_diag != DIAG_CPATH_DOWN - && bfd->rmt_diag != DIAG_RCPATH_DOWN; + forwarding_if_rx = bfd->forwarding_if_rx + && bfd->forwarding_if_rx_detect_time > now + && bfd->demand_rx_bfd_time > now; + + bfd->last_forwarding = (bfd->state == STATE_UP || forwarding_if_rx) + && bfd->rmt_diag != DIAG_PATH_DOWN + && bfd->rmt_diag != DIAG_CPATH_DOWN + && bfd->rmt_diag != DIAG_RCPATH_DOWN; if (bfd->last_forwarding != last_forwarding) { bfd->flap_count++; bfd_status_changed(bfd); diff --git a/tests/bfd.at b/tests/bfd.at index 3723d604e..f6f55928f 100644 --- a/tests/bfd.at +++ b/tests/bfd.at @@ -401,10 +401,9 @@ AT_CLEANUP # forwarding_if_rx Test1 # Test1 tests the case when bfd is only enabled on one end of the link. -# Under this situation, the bfd state should be DOWN and the forwarding -# flag should be FALSE by default. However, if forwarding_if_rx is -# enabled, as long as there is packet received, the bfd forwarding flag -# should be TRUE. +# Under this situation, the forwarding flag should always be false, even +# though there is data packet received, since there is no bfd control +# packet received during the demand_rx_bfd interval. AT_SETUP([bfd - bfd forwarding_if_rx - bfd on one side]) OVS_VSWITCHD_START([add-br br1 -- set bridge br1 datapath-type=dummy -- \ add-port br1 p1 -- set Interface p1 type=patch \ @@ -438,15 +437,8 @@ do AT_CHECK([ovs-ofctl packet-out br1 3 2 "90e2ba01475000101856b2e80806000108000604000100101856b2e80202020300000000000002020202"], [0], [stdout], []) done -# the forwarding flag should be true, since there is data received. -BFD_CHECK([p0], [true], [false], [none], [down], [No Diagnostic], [none], [down], [No Diagnostic]) - -# reset bfd forwarding_if_rx. -AT_CHECK([ovs-vsctl set Interface p0 bfd:forwarding_if_rx=false], [0]) -# forwarding flag should turn to false since the STATE is DOWN. +# the forwarding flag should be false, due to the demand_rx_bfd. BFD_CHECK([p0], [false], [false], [none], [down], [No Diagnostic], [none], [down], [No Diagnostic]) -BFD_CHECK_TX([p0], [1000ms], [1000ms], [0ms]) -BFD_CHECK_RX([p0], [500ms], [500ms], [1ms]) AT_CHECK([ovs-vsctl del-br br1], [0], [ignore]) AT_CLEANUP @@ -605,6 +597,74 @@ BFD_CHECK_RX([p0], [1000ms], [1000ms], [300ms]) AT_CHECK([ovs-vsctl del-br br1], [0], [ignore]) AT_CLEANUP +# forwarding_if_rx Test4 +# Test4 is for testing the demand_rx_bfd feature. +# bfd is enabled on both ends of the link. +AT_SETUP([bfd - bfd forwarding_if_rx - demand_rx_bfd]) +OVS_VSWITCHD_START([add-br br1 -- set bridge br1 datapath-type=dummy -- \ + add-port br1 p1 -- set Interface p1 type=patch \ + options:peer=p0 ofport_request=2 -- \ + add-port br0 p0 -- set Interface p0 type=patch \ + options:peer=p1 ofport_request=1 -- \ + set Interface p0 bfd:enable=true bfd:min_tx=300 bfd:min_rx=300 bfd:forwarding_if_rx=true -- \ + set Interface p1 bfd:enable=true bfd:min_tx=500 bfd:min_rx=500]) + +ovs-appctl time/stop +# advance the clock, to stablize the states. +for i in `seq 0 19`; do ovs-appctl time/warp 500; done +BFD_CHECK([p0], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic]) +BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic]) +BFD_CHECK_TX([p0], [500ms], [300ms], [500ms]) + +# disable the bfd on p1. +AT_CHECK([ovs-vsctl set Interface p1 bfd:enable=false], [0]) + +# advance clock by 4000ms, while receiving packets. +# the STATE should go DOWN, due to Control Detection Time Expired. +# but forwarding flag should be still true. +for i in `seq 0 7` +do + ovs-appctl time/warp 500 + AT_CHECK([ovs-ofctl packet-out br1 3 2 "90e2ba01475000101856b2e80806000108000604000100101856b2e80202020300000000000002020202"], + [0], [stdout], []) +done +BFD_CHECK([p0], [true], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic]) + +# advance clock long enough to trigger the demand_bfd_rx interval +# (100 * bfd->cfm_min_rx), forwarding flag should go down since there +# is no bfd control packet received during the demand_rx_bfd. +for i in `seq 0 120` +do + ovs-appctl time/warp 300 + AT_CHECK([ovs-ofctl packet-out br1 3 2 "90e2ba01475000101856b2e80806000108000604000100101856b2e80202020300000000000002020202"], + [0], [stdout], []) +done +BFD_CHECK([p0], [false], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic]) + +# now enable the bfd on p1 again. +AT_CHECK([ovs-vsctl set Interface p1 bfd:enable=true], [0]) +# advance clock by 5000ms. and p1 and p0 should be all up. +for i in `seq 0 9`; do ovs-appctl time/warp 500; done +BFD_CHECK([p0], [true], [false], [none], [up], [Control Detection Time Expired], [none], [up], [No Diagnostic]) +BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [none], [up], [Control Detection Time Expired]) +BFD_CHECK_TX([p0], [500ms], [300ms], [500ms]) + +# disable the bfd on p1 again. +AT_CHECK([ovs-vsctl set Interface p1 bfd:enable=false], [0]) +# advance clock long enough to trigger the demand_rx_bfd, +# forwarding flag should go down since there is no bfd control packet +# received during the demand_rx_bfd. +for i in `seq 0 120` +do + ovs-appctl time/warp 300 + AT_CHECK([ovs-ofctl packet-out br1 3 2 "90e2ba01475000101856b2e80806000108000604000100101856b2e80202020300000000000002020202"], + [0], [stdout], []) +done +BFD_CHECK([p0], [false], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic]) + +AT_CHECK([ovs-vsctl del-br br1], [0], [ignore]) +AT_CLEANUP + # test bfd:flap_count. # This test contains three part: # part 1. tests the flap_count on normal bfd monitored link. @@ -683,6 +743,7 @@ BFD_VSCTL_LIST_IFACE([p1], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["1"] # Part-3 now turn on forwarding_if_rx. AT_CHECK([ovs-vsctl set Interface p0 bfd:forwarding_if_rx=true], [0]) +for i in `seq 0 10`; do ovs-appctl time/warp 100; done # disable the bfd on p1. AT_CHECK([ovs-vsctl set Interface p1 bfd:enable=false], [0]) @@ -699,9 +760,9 @@ BFD_CHECK([p0], [true], [false], [none], [down], [Control Detection Time Expired # flap_count should remain unchanged. BFD_VSCTL_LIST_IFACE([p0], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["5"]) -# stop the traffic for 4000ms, the forwarding flag of p0 should turn false. +# stop the traffic for more than 100 * bfd->cfm_min_rx ms, the forwarding flag of p0 should turn false. # and there should be the increment of flap_count. -for i in `seq 0 7`; do ovs-appctl time/warp 500; done +for i in `seq 0 120`; do ovs-appctl time/warp 100; done BFD_CHECK([p0], [false], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic]) BFD_VSCTL_LIST_IFACE([p0], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["6"]) @@ -712,21 +773,15 @@ do AT_CHECK([ovs-ofctl packet-out br1 3 2 "90e2ba01475000101856b2e80806000108000604000100101856b2e80202020300000000000002020202"], [0], [stdout], []) done -BFD_CHECK([p0], [true], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic]) -# flap_count should be incremented again. -BFD_VSCTL_LIST_IFACE([p0], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["7"]) - -# stop the traffic for 4000ms, the forwarding flag of p0 should turn false. -# and there should be the increment of flap_count. -for i in `seq 0 7`; do ovs-appctl time/warp 500; done +# forwarding should be false, since there is still no bfd control packet received. BFD_CHECK([p0], [false], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic]) -BFD_VSCTL_LIST_IFACE([p0], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["8"]) +BFD_VSCTL_LIST_IFACE([p0], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["6"]) # turn on the bfd on p1. AT_CHECK([ovs-vsctl set interface p1 bfd:enable=true]) for i in `seq 0 49`; do ovs-appctl time/warp 100; done # even though there is no data traffic, since p1 bfd is on again, should increment the flap_count. -BFD_VSCTL_LIST_IFACE([p0], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["9"]) +BFD_VSCTL_LIST_IFACE([p0], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["7"]) BFD_VSCTL_LIST_IFACE([p1], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["1"]) OVS_VSWITCHD_STOP diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index aa9168d17..7f2fd587d 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -1968,10 +1968,13 @@ - True to consider the interface capable of packet I/O as long as it - continues to receive any packets (not just BFD packets). This - prevents link congestion that causes consecutive BFD control packets - to be lost from marking the interface down. + When true, traffic received on the + is used to indicate the capability of packet + I/O. BFD control packets are still transmitted and received. At + least one BFD control packet must be received every 100 * amount of time. Otherwise, even if + traffic are received, the + will be false.