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>

4 files changed, 100 insertions, 11 deletions

diff --git a/NEWS b/NEWS
index 000573367..82004c845 100644
--- a/NEWS
+++ b/NEWS
@@ -57,6 +57,7 @@ Post-v2.7.0
      * New vxlan tunnel extension "gpe" to support VXLAN-GPE tunnels.
      * Transparently pop and push Ethernet headers at transmit/reception
        of packets to/from L3 tunnels.
+   - The BFD detection multiplier is now user-configurable.
 
 v2.7.0 - 21 Feb 2017
 ---------------------
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
diff --git a/tests/bfd.at b/tests/bfd.at
index 9cc22454e..5ba99a4e1 100644
--- a/tests/bfd.at
+++ b/tests/bfd.at
@@ -1,10 +1,9 @@
 AT_BANNER([bfd])
 
 m4_define([BFD_CHECK], [
-AT_CHECK([ovs-appctl bfd/show $1 | sed -e '/Time:/d' | sed -e '/Discriminator/d' | sed -e '/Interval:/d'],[0],
+AT_CHECK([ovs-appctl bfd/show $1 | sed -e '/Time:/d' | sed -e '/Discriminator/d' | sed -e '/Interval:/d'| sed -e '/Multiplier/d'],[0],
 [dnl
 	Forwarding: $2
-	Detect Multiplier: 3
 	Concatenated Path Down: $3
 
 	Local Flags: $4
@@ -42,6 +41,14 @@ $3
 ])
 ])
 
+m4_define([BFD_CHECK_MULT], [
+AT_CHECK([ovs-appctl bfd/show $1 | sed -n '/Detect Multiplier/p'],[0],
+[dnl
+	Detect Multiplier: $2
+	Remote Detect Multiplier: $3
+])
+])
+
 AT_SETUP([bfd - basic config on different bridges])
 #Create 2 bridges connected by patch ports and enable BFD
 OVS_VSWITCHD_START(
@@ -1043,3 +1050,53 @@ check_liveness 3 LIVE
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([bfd - Edit the Detect Mult values])
+#Create 2 bridges connected by patch ports and enable BFD
+OVS_VSWITCHD_START()
+ovs-appctl time/stop
+AT_CHECK([ ovs-vsctl -- add-br br1 -- \
+           set bridge br1 datapath-type=dummy ])
+AT_CHECK([ ovs-vsctl -- add-port br1 p1 -- set Interface p1 type=patch\
+           options:peer=p0 ])
+AT_CHECK([ ovs-vsctl -- add-port br0 p0 -- set Interface p0 type=patch\
+           options:peer=p1 ])
+AT_CHECK([ ovs-vsctl -- set interface p0 bfd:enable=true ])
+AT_CHECK([ ovs-vsctl -- set interface p1 bfd:enable=true ])
+ovs-appctl time/warp 3100 100
+#Verify that BFD has been enabled on both interfaces.
+BFD_CHECK([p1], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic])
+BFD_CHECK([p0], [true], [false], [none], [up], [No Diagnostic], [none], [up], [No Diagnostic])
+#Verify that default mult values are 3.
+BFD_CHECK_MULT([p0], [3], [3])
+BFD_CHECK_MULT([p1], [3], [3])
+#Set the mult values to valid range border mult(p0)=1 mult(p1)=255.
+AT_CHECK([ovs-vsctl set interface p0 bfd:mult=1])
+AT_CHECK([ovs-vsctl set interface p1 bfd:mult=255])
+ovs-appctl time/warp 3100 100
+BFD_CHECK_MULT([p0], [1], [255])
+BFD_CHECK_MULT([p1], [255], [1])
+
+#Set the mult values out valid range border mult(p0)=0 mult(p1)=256.
+AT_CHECK([ovs-vsctl set interface p0 bfd:mult=0])
+AT_CHECK([ovs-vsctl set interface p1 bfd:mult=256])
+ovs-appctl time/warp 3100 100
+BFD_CHECK_MULT([p0], [3], [3])
+BFD_CHECK_MULT([p1], [3], [3])
+
+#Set valid non default mult values mult(p0)=8 mult(p1)=125.
+AT_CHECK([ovs-vsctl set interface p0 bfd:mult=8])
+AT_CHECK([ovs-vsctl set interface p1 bfd:mult=125])
+ovs-appctl time/warp 3100 100
+BFD_CHECK_MULT([p0], [8], [125])
+BFD_CHECK_MULT([p1], [125], [8])
+
+#Clear mult values. Detect mult values shall be default 3 again.
+AT_CHECK([ovs-vsctl remove interface p0 bfd mult])
+AT_CHECK([ovs-vsctl remove interface p1 bfd mult])
+ovs-appctl time/warp 3100 100
+BFD_CHECK_MULT([p0], [3], [3])
+BFD_CHECK_MULT([p1], [3], [3])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index eba66a797..892f83923 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -2838,12 +2838,12 @@
         BFD operates by regularly transmitting BFD control messages at a rate
         negotiated independently in each direction.  Each endpoint specifies
         the rate at which it expects to receive control messages, and the rate
-        at which it is willing to transmit them.  Open vSwitch uses a detection
-        multiplier of three, meaning that an endpoint signals a connectivity
-        fault if three consecutive BFD control messages fail to arrive.  In the
-        case of a unidirectional connectivity issue, the system not receiving
-        BFD control messages signals the problem to its peer in the messages it
-        transmits.
+        at which it is willing to transmit them.  By default, Open vSwitch uses
+        a detection multiplier of three, meaning that an endpoint signals a
+        connectivity fault if three consecutive BFD control messages fail to
+        arrive.  In the case of a unidirectional connectivity issue, the system
+        not receiving BFD control messages signals the problem to its peer in
+        the messages it transmits.
       </p>
 
       <p>
@@ -2954,6 +2954,13 @@
           this to true, BFD packets will be marked as OAM if encapsulated in
           one of these tunnels.
         </column>
+
+        <column name="bfd" key="mult"
+                type='{"type": "integer", "minInteger": 1, "maxInteger": 255}'>
+          The BFD detection multiplier, which defaults to 3.  An endpoint
+          signals a connectivity fault if the given number of consecutive BFD
+          control messages fail to arrive.
+        </column>
       </group>
 
       <group title="BFD Status">