This patch series fixes the test breakage encountered with the tunnel
scalability series. Thanks!
On 25 November 2013 14:26, Alex Wang <al...@nicira.com> wrote:
> Commit (9cdc68a1 bfd: Add forwarding flag to struct bfd.) allows
> several functions to call bfd_forwarding__() and update the
> forwarding flag. When forwarding_if_rx feature is enabled, this
> introduces a race condition among threads calling these functions
> and trying to update the flag. And this may cause forwarding flag
> flapping.
>
> This commit fixes the above issue by limiting that only bfd_run()
> and bfd_process_packet() can update the forwarding flag. And when
> forwarding_if_rx is enabled, only bfd_run() can update the forwarding
> flag. Other functions can only read the value of the forwarding flag.
>
> Reported-by: Joe Stringer <joestrin...@nicira.com>
> Signed-off-by: Alex Wang <al...@nicira.com>
> ---
> lib/bfd.c | 34 +++++++++++++++++++++++-----------
> 1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/lib/bfd.c b/lib/bfd.c
> index 4582f2e..2b2cad8 100644
> --- a/lib/bfd.c
> +++ b/lib/bfd.c
> @@ -215,7 +215,7 @@ static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
> static struct hmap all_bfds__ = HMAP_INITIALIZER(&all_bfds__);
> static struct hmap *const all_bfds OVS_GUARDED_BY(mutex) = &all_bfds__;
>
> -static bool bfd_forwarding__(struct bfd *) OVS_REQUIRES(mutex);
> +static bool bfd_update_forwarding(struct bfd *) OVS_REQUIRES(mutex);
> static bool bfd_in_poll(const struct bfd *) OVS_REQUIRES(mutex);
> static void bfd_poll(struct bfd *bfd) OVS_REQUIRES(mutex);
> static const char *bfd_diag_str(enum diag) OVS_REQUIRES(mutex);
> @@ -246,15 +246,16 @@ static void log_msg(enum vlog_level, const struct msg
> *, const char *message,
>
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(20, 20);
>
> -/* Returns true if the interface on which 'bfd' is running may be used to
> - * forward traffic according to the BFD session state. */
> +/* Returns the forwarding flag value. The flag is true if the interface on
> + * which 'bfd' is running may be used to forward traffic according to the BFD
> + * session state. */
> bool
> bfd_forwarding(struct bfd *bfd) OVS_EXCLUDED(mutex)
> {
> bool ret;
>
> ovs_mutex_lock(&mutex);
> - ret = bfd_forwarding__(bfd);
> + ret = bfd->forwarding;
> ovs_mutex_unlock(&mutex);
> return ret;
> }
> @@ -507,6 +508,10 @@ bfd_run(struct bfd *bfd) OVS_EXCLUDED(mutex)
> || bfd->in_decay != old_in_decay) {
> bfd_poll(bfd);
> }
> +
> + /* Updates the forwarding flag. */
> + bfd_update_forwarding(bfd);
> +
> ovs_mutex_unlock(&mutex);
> }
>
> @@ -786,6 +791,11 @@ bfd_process_packet(struct bfd *bfd, const struct flow
> *flow,
> }
> /* XXX: RFC 5880 Section 6.8.6 Demand mode related calculations here. */
>
> + /* Updates the forwarding flag value if not in demand mode. */
> + if (!bfd->forwarding_if_rx) {
> + bfd_update_forwarding(bfd);
> + }
> +
> out:
> ovs_mutex_unlock(&mutex);
> }
> @@ -812,9 +822,15 @@ 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.
> + *
> + * When forwarding_if_rx is disabled, this function can be called
> + * by bfd_process_packet() and bfd_run(). When forwarding_if_rx
> + * is enabled, this function can only be called by bfd_run().
> + * This is to prevent the forwarding flag flapping due to race
> + * between calling threads. */
> static bool
> -bfd_forwarding__(struct bfd *bfd) OVS_REQUIRES(mutex)
> +bfd_update_forwarding(struct bfd *bfd) OVS_REQUIRES(mutex)
> {
> long long int time;
> bool forwarding_old = bfd->forwarding;
> @@ -1033,9 +1049,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
> @@ -1100,13 +1113,12 @@ bfd_check_rx(struct bfd *bfd) OVS_REQUIRES(mutex)
> }
> }
>
> -/* Updates the forwarding_if_rx_detect_time and the forwarding flag. */
> +/* Updates the forwarding_if_rx_detect_time. */
> 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
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
