From: Alex Wang Date: Tue, 10 Dec 2013 01:34:53 +0000 (-0800) Subject: bfd: Change the update of forwarding flag. X-Git-Tag: sliver-openvswitch-2.0.90-1~1^2~2 X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=a1aeea86475db086ce95679962fb6d03d0a645f3;hp=309d9dacd3c254a8d2d92d0589aeada247f54089;p=sliver-openvswitch.git bfd: Change the update of forwarding flag. Currently, we update the forwarding flag in bfd_set_state() and in bfd_forwarding_if_rx_update() if bfd_forwarding_if_rx is enabled. However, these are not the exact places where the forwarding flag needs to be updated. The exact places are in the bfd_process_packet() where bfd status are changed based on received control packet, and in the flow_push_stats() and compose_output_action__() where the rx_packet counter is updated. This commit changes the update of forwarding flag to the places mentioned above. Signed-off-by: Alex Wang Signed-off-by: Ethan Jackson Acked-by: Ethan Jackson --- diff --git a/lib/bfd.c b/lib/bfd.c index 25129c90d..1df5acd05 100644 --- a/lib/bfd.c +++ b/lib/bfd.c @@ -187,7 +187,7 @@ struct bfd { long long int next_tx; /* Next TX time. */ long long int detect_time; /* RFC 5880 6.8.4 Detection time. */ - bool forwarding; /* Interface capability of packet I/O. */ + bool last_forwarding; /* Last calculation of forwarding flag. */ int forwarding_override; /* Manual override of 'forwarding' status. */ atomic_bool check_tnl_key; /* Verify tunnel key of inbound packets? */ @@ -234,7 +234,7 @@ static void bfd_put_details(struct ds *, const struct bfd *) static uint64_t bfd_rx_packets(const struct bfd *) OVS_REQUIRES(mutex); static void bfd_try_decay(struct bfd *) OVS_REQUIRES(mutex); static void bfd_decay_update(struct bfd *) OVS_REQUIRES(mutex); -static void bfd_check_rx(struct bfd *) OVS_REQUIRES(mutex); + static void bfd_forwarding_if_rx_update(struct bfd *) OVS_REQUIRES(mutex); static void bfd_unixctl_show(struct unixctl_conn *, int argc, const char *argv[], void *aux OVS_UNUSED); @@ -259,6 +259,20 @@ bfd_forwarding(struct bfd *bfd) OVS_EXCLUDED(mutex) return ret; } +/* When forwarding_if_rx is enabled, if there are packets received, + * updates forwarding_if_rx_detect_time. */ +void +bfd_account_rx(struct bfd *bfd, const struct dpif_flow_stats *stats) +{ + if (stats->n_packets && bfd->forwarding_if_rx) { + ovs_mutex_lock(&mutex); + bfd_forwarding__(bfd); + bfd_forwarding_if_rx_update(bfd); + bfd_forwarding__(bfd); + ovs_mutex_unlock(&mutex); + } +} + /* Returns a 'smap' of key value pairs representing the status of 'bfd' * intended for the OVS database. */ void @@ -266,7 +280,9 @@ bfd_get_status(const struct bfd *bfd, struct smap *smap) OVS_EXCLUDED(mutex) { ovs_mutex_lock(&mutex); - smap_add(smap, "forwarding", bfd->forwarding ? "true" : "false"); + smap_add(smap, "forwarding", + bfd_forwarding__(CONST_CAST(struct bfd *, bfd)) + ? "true" : "false"); smap_add(smap, "state", bfd_state_str(bfd->state)); smap_add(smap, "diagnostic", bfd_diag_str(bfd->diag)); smap_add_format(smap, "flap_count", "%"PRIu64, bfd->flap_count); @@ -316,7 +332,6 @@ bfd_configure(struct bfd *bfd, const char *name, const struct smap *cfg, if (!bfd) { bfd = xzalloc(sizeof *bfd); bfd->name = xstrdup(name); - bfd->forwarding = false; bfd->forwarding_override = -1; bfd->disc = generate_discriminator(); hmap_insert(all_bfds, &bfd->node, bfd->disc); @@ -490,6 +505,7 @@ bfd_run(struct bfd *bfd) OVS_EXCLUDED(mutex) if (bfd->state > STATE_DOWN && now >= bfd->detect_time) { bfd_set_state(bfd, STATE_DOWN, DIAG_EXPIRED); + bfd_forwarding__(bfd); } /* Decay may only happen when state is STATE_UP, bfd->decay_min_rx is @@ -499,9 +515,6 @@ bfd_run(struct bfd *bfd) OVS_EXCLUDED(mutex) bfd_try_decay(bfd); } - /* Always checks the reception of any packet. */ - bfd_check_rx(bfd); - if (bfd->min_tx != bfd->cfg_min_tx || (bfd->min_rx != bfd->cfg_min_rx && bfd->min_rx != bfd->decay_min_rx) || bfd->in_decay != old_in_decay) { @@ -638,6 +651,8 @@ bfd_process_packet(struct bfd *bfd, const struct flow *flow, /* Increments the decay rx counter. */ bfd->decay_rx_ctl++; + bfd_forwarding__(bfd); + if (flow->nw_ttl != 255) { /* XXX Should drop in the kernel to prevent DOS. */ goto out; @@ -787,6 +802,7 @@ bfd_process_packet(struct bfd *bfd, const struct flow *flow, /* XXX: RFC 5880 Section 6.8.6 Demand mode related calculations here. */ out: + bfd_forwarding__(bfd); ovs_mutex_unlock(&mutex); } @@ -812,27 +828,31 @@ bfd_set_netdev(struct bfd *bfd, const struct netdev *netdev) /* Updates the forwarding flag. If override is not configured and - * the forwarding flag value changes, increments the flap count. */ + * the forwarding flag value changes, increments the flap count. + * + * Note this function may be called multiple times in a function + * (e.g. bfd_account_rx) before and after the bfd state or status + * change. This is to capture any forwarding flag flap. */ static bool bfd_forwarding__(struct bfd *bfd) OVS_REQUIRES(mutex) { long long int time; - bool forwarding_old = bfd->forwarding; + bool last_forwarding = bfd->last_forwarding; if (bfd->forwarding_override != -1) { return bfd->forwarding_override == 1; } time = bfd->forwarding_if_rx_detect_time; - bfd->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; - if (bfd->forwarding != forwarding_old) { + 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; + if (bfd->last_forwarding != last_forwarding) { bfd->flap_count++; } - return bfd->forwarding; + return bfd->last_forwarding; } /* Helpers. */ @@ -1033,9 +1053,6 @@ bfd_set_state(struct bfd *bfd, enum state state, enum diag diag) bfd_decay_update(bfd); } } - - /* Updates the forwarding flag. */ - bfd_forwarding__(bfd); } static uint64_t @@ -1081,32 +1098,11 @@ bfd_decay_update(struct bfd * bfd) OVS_REQUIRES(mutex) bfd->decay_detect_time = MAX(bfd->decay_min_rx, 2000) + time_msec(); } -/* Checks if there are packets received during the time since last call. - * If forwarding_if_rx is enabled and packets are received, updates the - * forwarding_if_rx_detect_time. */ -static void -bfd_check_rx(struct bfd *bfd) OVS_REQUIRES(mutex) -{ - uint64_t rx_packets = bfd_rx_packets(bfd); - int64_t diff; - - diff = rx_packets - bfd->rx_packets; - bfd->rx_packets = rx_packets; - if (diff < 0) { - VLOG_INFO_RL(&rl, "rx_packets count is smaller than last time."); - } - if (bfd->forwarding_if_rx && diff > 0) { - bfd_forwarding_if_rx_update(bfd); - } -} - -/* Updates the forwarding_if_rx_detect_time and the forwarding flag. */ static void bfd_forwarding_if_rx_update(struct bfd *bfd) OVS_REQUIRES(mutex) { int64_t incr = bfd_rx_interval(bfd) * bfd->mult; bfd->forwarding_if_rx_detect_time = MAX(incr, 2000) + time_msec(); - bfd_forwarding__(bfd); } static uint32_t @@ -1152,7 +1148,9 @@ bfd_find_by_name(const char *name) OVS_REQUIRES(mutex) static void bfd_put_details(struct ds *ds, const struct bfd *bfd) OVS_REQUIRES(mutex) { - ds_put_format(ds, "\tForwarding: %s\n", bfd->forwarding ? "true" : "false"); + ds_put_format(ds, "\tForwarding: %s\n", + bfd_forwarding__(CONST_CAST(struct bfd *, bfd)) + ? "true" : "false"); ds_put_format(ds, "\tDetect Multiplier: %d\n", bfd->mult); ds_put_format(ds, "\tConcatenated Path Down: %s\n", bfd->cpath_down ? "true" : "false"); diff --git a/lib/bfd.h b/lib/bfd.h index 0ad86e94e..4e7d4cb46 100644 --- a/lib/bfd.h +++ b/lib/bfd.h @@ -22,6 +22,7 @@ #include struct bfd; +struct dpif_flow_stats; struct flow; struct flow_wildcards; struct netdev; @@ -46,6 +47,7 @@ struct bfd *bfd_configure(struct bfd *, const char *name, struct bfd *bfd_ref(const struct bfd *); void bfd_unref(struct bfd *); +void bfd_account_rx(struct bfd *, const struct dpif_flow_stats *); bool bfd_forwarding(struct bfd *); void bfd_get_status(const struct bfd *, struct smap *); void bfd_set_netdev(struct bfd *, const struct netdev *); diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 24bade17b..b0f61ca12 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -1718,6 +1718,9 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port, if (ctx->xin->resubmit_stats) { netdev_vport_inc_tx(xport->netdev, ctx->xin->resubmit_stats); netdev_vport_inc_rx(peer->netdev, ctx->xin->resubmit_stats); + if (peer->bfd) { + bfd_account_rx(peer->bfd, ctx->xin->resubmit_stats); + } } return; diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index f90636b2d..5e0a7f0aa 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4273,6 +4273,9 @@ flow_push_stats(struct ofproto_dpif *ofproto, struct flow *flow, in_port = get_ofp_port(ofproto, flow->in_port.ofp_port); if (in_port && in_port->is_tunnel) { netdev_vport_inc_rx(in_port->up.netdev, stats); + if (in_port->bfd) { + bfd_account_rx(in_port->bfd, stats); + } } xlate_in_init(&xin, ofproto, flow, NULL, stats->tcp_flags, NULL); diff --git a/tests/bfd.at b/tests/bfd.at index 83b424990..ccb62b54e 100644 --- a/tests/bfd.at +++ b/tests/bfd.at @@ -716,7 +716,7 @@ BFD_VSCTL_LIST_IFACE([p1], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["1"] # turn bfd on p1 off, should increment the bfd:flap_count on p0. AT_CHECK([ovs-vsctl set interface p1 bfd:enable=false]) -for i in `seq 0 19`; do ovs-appctl time/warp 100; done +for i in `seq 0 49`; 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"], ["2"]) AT_CHECK([ovs-vsctl list interface p1 | sed -n "s/^.*flap_count=\(.*\), forwarding.*$/\1/p"]) @@ -724,7 +724,7 @@ AT_CHECK([ovs-vsctl list interface p1 | sed -n "s/^.*flap_count=\(.*\), forwardi # turn bfd on p1 on again, should increment the bfd:flap_count on p0. # p1 should still have flap_count = "1", since it is reset. AT_CHECK([ovs-vsctl set interface p1 bfd:enable=true]) -for i in `seq 0 19`; do ovs-appctl time/warp 100; done +for i in `seq 0 49`; do ovs-appctl time/warp 100; done BFD_VSCTL_LIST_IFACE([p0], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["3"]) BFD_VSCTL_LIST_IFACE([p1], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["1"]) @@ -736,7 +736,7 @@ OK # turn bfd on p1 off, should not increment the bfd:flap_count on p0, since forwarding_override is on. AT_CHECK([ovs-vsctl set interface p1 bfd:enable=false]) -for i in `seq 0 19`; do ovs-appctl time/warp 100; done +for i in `seq 0 49`; do ovs-appctl time/warp 100; done BFD_CHECK([p0], [true], [false], [none], [down], [Control Detection Time Expired], [none], [down], [No Diagnostic]) BFD_VSCTL_LIST_IFACE([p0], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["3"]) AT_CHECK([ovs-vsctl list interface p1 | sed -n "s/^.*flap_count=\(.*\), forwarding.*$/\1/p"]) @@ -744,7 +744,7 @@ AT_CHECK([ovs-vsctl list interface p1 | sed -n "s/^.*flap_count=\(.*\), forwardi # turn bfd on p1 on again, should not increment the bfd:flap_count on p0, since forwarding override is on. # p1 should still have flap_count = "1", since it is reset. AT_CHECK([ovs-vsctl set interface p1 bfd:enable=true]) -for i in `seq 0 19`; do ovs-appctl time/warp 100; done +for i in `seq 0 49`; do ovs-appctl time/warp 100; done BFD_VSCTL_LIST_IFACE([p0], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["3"]) BFD_VSCTL_LIST_IFACE([p1], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["1"]) @@ -755,9 +755,9 @@ OK # turn bfd on p1 off and on, should increment the bfd:flap_count on p0. AT_CHECK([ovs-vsctl set interface p1 bfd:enable=false]) -for i in `seq 0 19`; do ovs-appctl time/warp 100; done +for i in `seq 0 49`; do ovs-appctl time/warp 100; done AT_CHECK([ovs-vsctl set interface p1 bfd:enable=true]) -for i in `seq 0 19`; do ovs-appctl time/warp 100; done +for i in `seq 0 49`; do ovs-appctl time/warp 100; done BFD_VSCTL_LIST_IFACE([p0], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["5"]) BFD_VSCTL_LIST_IFACE([p1], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["1"]) @@ -804,7 +804,7 @@ BFD_VSCTL_LIST_IFACE([p0], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["8"] # turn on the bfd on p1. AT_CHECK([ovs-vsctl set interface p1 bfd:enable=true]) -for i in `seq 0 19`; do ovs-appctl time/warp 100; done +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([p1], ["s/^.*flap_count=\(.*\), forwarding.*$/\1/p"], ["1"])