Skip to content

Commit

Permalink
bfd: Require bfd control packet received in forwarding_if_rx mode.
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Acked-by: Ethan Jackson <[email protected]>
  • Loading branch information
@yew011
yew011 committed Apr 30, 2014
1 parent 5767a79 commit 34c8862
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 34 deletions.
27 changes: 20 additions & 7 deletions lib/bfd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
101 changes: 78 additions & 23 deletions tests/bfd.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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])

Expand All @@ -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"])

Expand All @@ -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
Expand Down
11 changes: 7 additions & 4 deletions vswitchd/vswitch.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1968,10 +1968,13 @@
</column>

<column name="bfd" key="forwarding_if_rx" type='{"type": "boolean"}'>
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 <code>true</code>, traffic received on the
<ref table="Interface"/> 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 * <ref
column="bfd" key="min_rx"/> amount of time. Otherwise, even if
traffic are received, the <ref column="bfd" key="forwarding"/>
will be <code>false</code>.
</column>

<column name="bfd" key="cpath_down" type='{"type": "boolean"}'>
Expand Down

0 comments on commit 34c8862

Please sign in to comment.