bfd: Detect Multiplier configuration

Mult value (bfd.DetectMult in RFC5880) is hard-coded and equal to 3 in
current openvswitch. As a consequence remote and local mult is the same.

In this commit the mult (Detect Multiplier/bfd.DetectMult/Detect Mult)
can be set on each interface setting the mult=<value> in bfd Column
in Interface table of ovsdb database.
Example:
ovs-vsctl set Interface p1 bfd:mult=4
sets mult=4 on p1 interface

The modification based on RFC5880 June 2010.
The relevant paragraphs are:
4.1. Generic BFD Control Packet Format
6.8.4. Calculating the Detection Time
6.8.7. Transmitting BFD Control Packets
6.8.12. Detect Multiplier Change

The mult value is set to default 3 if it is not set in ovsdb. This
provides backward compatibility to previous openvswitch behaviour.
The RFC5880 says in 6.8.1 that DetectMult shall be a non-zero integer.
In RFC5880 4.1. "Detect Mult" has 8 bit length and is declared
as a 8 bit unsigned integer in bfd.c.
Consequently mult value shall be greater than 0 and less then 256.
In case of incorrect mult value is given in ovsdb the default value (3)
will be set and a message is logged into ovs-vswitchd.log on that.
Local or remote mult value change is also logged into ovs-vswitchd.log.

Since remote and local mult is not the same calculation of detect time
has been changed. Due to RFC5880 6.8.4 Detection Time is calculated using
mult value of the remote system.
Detection time is recalculated due to remote mult change.

The BFD packet transmission jitter is different in case of mult=1
due to RFC5880 6.8.7. The maximum interval of the transmitted bfd packet
is 90% of the transmission interval.

The value of remote mult is printed in the last line of the output of
ovs-appctl bfd/show command with label: Remote Detect Mult.

There is a feature in openvswitch connected with forwarding_if_rx that
is not the part of RFC5880. This feature also uses mult value but it is
not specified if local or remote since it was the
same in original code. The relevant description in code:
   /* 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. */

Due to lack of specification local mult value is used for calculation of
forwarding_if_rx_detect_time. This detect time is recalculated at mult
change if forwarding_if_rx is true and bfd is in UP state.

A new unit test has been added: "bfd - Edit the Detect Mult values"
The following cases are tested:
- Without setting mult the mult will be the default value (3).
- The setting of the lowest (1) and highest (255) valid mult value
  and the detection of remote mult value.
- The setting of out of range mult value (0, 256) in ovsdb results
  sets default value in ovs-vswitchd
- Clearing non default mult value from ovsdb results sets default
  value in ovs-vswitchd.

Signed-off-by: Gábor Szűcs <gabor.sz.cs@ericsson.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>

1 files changed, 27 insertions, 3 deletions

diff --git a/lib/bfd.c b/lib/bfd.c
index 383be20ef..4174e8b8e 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -146,6 +146,7 @@ BUILD_ASSERT_DECL(BFD_PACKET_LEN == sizeof(struct msg));
 #define VERS_SHIFT 5
 #define STATE_MASK 0xC0
 #define FLAGS_MASK 0x3f
+#define DEFAULT_MULT 3
 
 struct bfd {
     struct hmap_node node;        /* In 'all_bfds'. */
@@ -155,6 +156,7 @@ struct bfd {
 
     bool cpath_down;              /* Concatenated Path Down. */
     uint8_t mult;                 /* bfd.DetectMult. */
+    uint8_t rmt_mult;             /* Remote bfd.DetectMult. */
 
     struct netdev *netdev;
     uint64_t rx_packets;          /* Packets received by 'netdev'. */
@@ -370,7 +372,8 @@ bfd_configure(struct bfd *bfd, const char *name, const struct smap *cfg,
 
         bfd->diag = DIAG_NONE;
         bfd->min_tx = 1000;
-        bfd->mult = 3;
+        bfd->rmt_mult = 0;
+        bfd->mult = DEFAULT_MULT;
         ovs_refcount_init(&bfd->ref_cnt);
         bfd->netdev = netdev_ref(netdev);
         bfd->rx_packets = bfd_rx_packets(bfd);
@@ -389,6 +392,13 @@ bfd_configure(struct bfd *bfd, const char *name, const struct smap *cfg,
         bfd_status_changed(bfd);
     }
 
+    int old_mult = bfd->mult;
+    int new_mult = smap_get_int(cfg, "mult", DEFAULT_MULT);
+    if (new_mult < 1 || new_mult > 255) {
+        new_mult = DEFAULT_MULT;
+    }
+    bfd->mult = new_mult;
+
     bfd->oam = smap_get_bool(cfg, "oam", false);
 
     atomic_store_relaxed(&bfd->check_tnl_key,
@@ -458,6 +468,9 @@ bfd_configure(struct bfd *bfd, const char *name, const struct smap *cfg,
         } else {
             bfd->forwarding_if_rx_detect_time = 0;
         }
+    } else if (bfd->state == STATE_UP && bfd->forwarding_if_rx
+               && old_mult != new_mult) {
+        bfd_forwarding_if_rx_update(bfd);
     }
 
     if (need_poll) {
@@ -805,6 +818,12 @@ bfd_process_packet(struct bfd *bfd, const struct flow *flow,
         bfd->flags |= FLAG_FINAL;
     }
 
+    if (bfd->rmt_mult != msg->mult) {
+        VLOG_INFO("Interface %s remote mult value %d changed to %d",
+                   bfd->name, bfd->rmt_mult, msg->mult);
+        bfd->rmt_mult = msg->mult;
+    }
+
     rmt_min_rx = MAX(ntohl(msg->min_rx) / 1000, 1);
     if (bfd->rmt_min_rx != rmt_min_rx) {
         bfd->rmt_min_rx = rmt_min_rx;
@@ -815,7 +834,7 @@ bfd_process_packet(struct bfd *bfd, const struct flow *flow,
     }
 
     bfd->rmt_min_tx = MAX(ntohl(msg->min_tx) / 1000, 1);
-    bfd->detect_time = bfd_rx_interval(bfd) * bfd->mult + time_msec();
+    bfd->detect_time = bfd_rx_interval(bfd) * bfd->rmt_mult + time_msec();
 
     if (bfd->state == STATE_ADMIN_DOWN) {
         VLOG_DBG_RL(&rl, "Administratively down, dropping control message.");
@@ -976,7 +995,11 @@ static void
 bfd_set_next_tx(struct bfd *bfd) OVS_REQUIRES(mutex)
 {
     long long int interval = bfd_tx_interval(bfd);
-    interval -= interval * random_range(26) / 100;
+    if (bfd->mult == 1) {
+        interval -= interval * (10 + random_range(16)) / 100;
+    } else {
+        interval -= interval * random_range(26) / 100;
+    }
     bfd->next_tx = bfd->last_tx + interval;
 }
 
@@ -1268,6 +1291,7 @@ bfd_put_details(struct ds *ds, const struct bfd *bfd) OVS_REQUIRES(mutex)
                   bfd->rmt_min_tx);
     ds_put_format(ds, "\tRemote Minimum RX Interval: %lldms\n",
                   bfd->rmt_min_rx);
+    ds_put_format(ds, "\tRemote Detect Multiplier: %d\n", bfd->rmt_mult);
 }
 
 static void