[ovs-dev,repost] ofproto: Add support to watch controller port liveness in fast-failover group
diff mbox series

Message ID 1580725966-9177-1-git-send-email-vishal.deep.ajmera@ericsson.com
State New
Headers show
Series
  • [ovs-dev,repost] ofproto: Add support to watch controller port liveness in fast-failover group
Related show

Commit Message

Archana Holla via dev Feb. 3, 2020, 10:32 a.m. UTC
Currently fast-failover group does not support checking liveness of controller
port (OFPP_CONTROLLER). However this feature can be useful for selecting
alternate pipeline when controller connection itself is down for e.g.
by using local DHCP server to reply for any DHCP request originating from VMs.

This patch adds the support for watching controller port liveness in fast-
failover group. Controller port is considered live when atleast one
of-connection is alive.

Example usage:

ovs-ofctl add-group br-int 'group_id=1234,type=ff,
          bucket=watch_port:CONTROLLER,actions:<A>,
          bucket=watch_port:1,actions:<B>

Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>
---
 NEWS                         |  1 +
 lib/ofp-group.c              |  3 ++-
 ofproto/ofproto-dpif-xlate.c |  5 ++++-
 ofproto/ofproto-dpif.c       | 10 ++++++++++
 ofproto/ofproto-dpif.h       |  3 +++
 ofproto/ofproto.c            |  3 ++-
 6 files changed, 22 insertions(+), 3 deletions(-)

Comments

Archana Holla via dev Feb. 12, 2020, 8:41 a.m. UTC | #1
> 
> Currently fast-failover group does not support checking liveness of
controller
> port (OFPP_CONTROLLER). However this feature can be useful for selecting
> alternate pipeline when controller connection itself is down for e.g.
> by using local DHCP server to reply for any DHCP request originating from
> VMs.
> 
> This patch adds the support for watching controller port liveness in fast-
> failover group. Controller port is considered live when atleast one
> of-connection is alive.
> 
> Example usage:
> 
> ovs-ofctl add-group br-int 'group_id=1234,type=ff,
>           bucket=watch_port:CONTROLLER,actions:<A>,
>           bucket=watch_port:1,actions:<B>
> 

Hi Ben, any comments on this patch?

Warm Regards,
Vishal Ajmera
Archana Holla via dev March 2, 2020, 3:27 a.m. UTC | #2
> > Currently fast-failover group does not support checking liveness of
> controller
> > port (OFPP_CONTROLLER). However this feature can be useful for selecting
> > alternate pipeline when controller connection itself is down for e.g.
> > by using local DHCP server to reply for any DHCP request originating
from
> > VMs.
> >
> > This patch adds the support for watching controller port liveness in
fast-
> > failover group. Controller port is considered live when atleast one
> > of-connection is alive.
> >
> > Example usage:
> >
> > ovs-ofctl add-group br-int 'group_id=1234,type=ff,
> >           bucket=watch_port:CONTROLLER,actions:<A>,
> >           bucket=watch_port:1,actions:<B>
> >
> 
> Hi Ben, any comments on this patch?
> 

Hi Ben,

Can you help reviewing this patch and merge if it looks ok? We (from
Ericsson) are currently working on an SDN feature requiring this support.

Warm Regards,
Vishal Ajmera
Ben Pfaff March 6, 2020, 9:36 p.m. UTC | #3
On Mon, Feb 03, 2020 at 11:32:46AM +0100, Vishal Deep Ajmera via dev wrote:
> Currently fast-failover group does not support checking liveness of controller
> port (OFPP_CONTROLLER). However this feature can be useful for selecting
> alternate pipeline when controller connection itself is down for e.g.
> by using local DHCP server to reply for any DHCP request originating from VMs.
> 
> This patch adds the support for watching controller port liveness in fast-
> failover group. Controller port is considered live when atleast one
> of-connection is alive.
> 
> Example usage:
> 
> ovs-ofctl add-group br-int 'group_id=1234,type=ff,
>           bucket=watch_port:CONTROLLER,actions:<A>,
>           bucket=watch_port:1,actions:<B>
> 
> Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com>

I applied this to master.  Thanks!

Patch
diff mbox series

diff --git a/NEWS b/NEWS
index 9bbe71d..354291a 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,7 @@  Post-v2.13.0
    - OpenFlow:
      * The OpenFlow ofp_desc/serial_num may now be configured by setting the
        value of other-config:dp-sn in the Bridge table.
+     * Added support to watch CONTROLLER port status in fast failover group.
 
 
 v2.13.0 - xx xxx xxxx
diff --git a/lib/ofp-group.c b/lib/ofp-group.c
index b675e80..bf0f8af 100644
--- a/lib/ofp-group.c
+++ b/lib/ofp-group.c
@@ -660,7 +660,8 @@  parse_bucket_str(struct ofputil_bucket *bucket, char *str_,
         } else if (!strcasecmp(key, "watch_port")) {
             if (!ofputil_port_from_string(value, port_map, &bucket->watch_port)
                 || (ofp_to_u16(bucket->watch_port) >= ofp_to_u16(OFPP_MAX)
-                    && bucket->watch_port != OFPP_ANY)) {
+                    && bucket->watch_port != OFPP_ANY
+                    && bucket->watch_port != OFPP_CONTROLLER)) {
                 error = xasprintf("%s: invalid watch_port", value);
             }
         } else if (!strcasecmp(key, "watch_group")) {
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 0b45ecf..adf57a5 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1888,9 +1888,12 @@  bucket_is_alive(const struct xlate_ctx *ctx,
 
     return (!ofputil_bucket_has_liveness(bucket)
             || (bucket->watch_port != OFPP_ANY
+               && bucket->watch_port != OFPP_CONTROLLER
                && odp_port_is_alive(ctx, bucket->watch_port))
             || (bucket->watch_group != OFPG_ANY
-               && group_is_alive(ctx, bucket->watch_group, depth + 1)));
+               && group_is_alive(ctx, bucket->watch_group, depth + 1))
+            || (bucket->watch_port == OFPP_CONTROLLER
+               && ofproto_is_alive(&ctx->xbridge->ofproto->up)));
 }
 
 static void
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 0222ec8..65f214a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1798,6 +1798,7 @@  run(struct ofproto *ofproto_)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     uint64_t new_seq, new_dump_seq;
+    bool is_connected;
 
     if (mbridge_need_revalidate(ofproto->mbridge)) {
         ofproto->backer->need_revalidate = REV_RECONFIGURE;
@@ -1866,6 +1867,15 @@  run(struct ofproto *ofproto_)
         ofproto->backer->need_revalidate = REV_MCAST_SNOOPING;
     }
 
+    /* Check if controller connection is toggled. */
+    is_connected = ofproto_is_alive(&ofproto->up);
+    if (ofproto->is_controller_connected != is_connected) {
+        ofproto->is_controller_connected = is_connected;
+        /* Trigger revalidation as fast failover group monitoring
+         * controller port may need to check liveness again. */
+        ofproto->backer->need_revalidate = REV_RECONFIGURE;
+    }
+
     new_dump_seq = seq_read(udpif_dump_seq(ofproto->backer->udpif));
     if (ofproto->dump_seq != new_dump_seq) {
         struct rule *rule, *next_rule;
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index c9d5df3..aee61d6 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -342,6 +342,9 @@  struct ofproto_dpif {
     struct guarded_list ams;      /* Contains "struct ofproto_async_msgs"s. */
     struct seq *ams_seq;          /* For notifying 'ams' reception. */
     uint64_t ams_seqno;
+
+    bool is_controller_connected; /* True if any controller admitted this
+                                   * switch connection. */
 };
 
 struct ofproto_dpif *ofproto_dpif_lookup_by_name(const char *name);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index e259128..0fbd6c3 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1906,7 +1906,8 @@  ofproto_wait(struct ofproto *p)
 bool
 ofproto_is_alive(const struct ofproto *p)
 {
-    return connmgr_has_controllers(p->connmgr);
+    return (connmgr_has_controllers(p->connmgr)
+            && connmgr_is_any_controller_admitted(p->connmgr));
 }
 
 /* Adds some memory usage statistics for 'ofproto' into 'usage', for use with