[ovs-dev] [PATCH] ofp-msgs: Added NXT_REQUESTFORWARD for OF1.0-1.3

Ben Pfaff blp at ovn.org
Thu Oct 25 16:54:11 UTC 2018


On Tue, Oct 23, 2018 at 06:58:57PM -0700, Zak Whittington wrote:
> Backported OFPT14_REQUESTFORWARD to OF1.0-1.3 as a Nicira
> extension.
> 
> VMware-BZ: 2136594
> Signed-off-by: Zak Whittington <zwhitt.vmware at gmail.com>

Thanks for the revision.  It looks good!  I have only a few comments.

First, the new test checks for OF1.3, but I'm pretty sure that the
currently intended use case is OF1.0, so I'd prefer to see one more
test, for OF1.0.

I'm appending a few style fixes as an incremental patch for you to fold
in, with the following reasoning:

There is a misspelling of Nicira as "Nicera" in one place.

coding-style.rst says:

    Break long lines before the ternary operators ? and :, rather than after
    them, e.g.

    ::

        return (out_port != VIGP_CONTROL_PATH
                ? alpheus_output_port(dp, skb, out_port)
                : alpheus_output_control(dp, skb, fwd_save_skb(skb),
                                         VIGR_ACTION));

also:

    Enclose single statements in braces:

    ::

        if (a > b) {
            return a;
        } else {
            return b;
        }

diff --git a/include/openvswitch/ofp-monitor.h b/include/openvswitch/ofp-monitor.h
index 7015200803a3..5951260d20ff 100644
--- a/include/openvswitch/ofp-monitor.h
+++ b/include/openvswitch/ofp-monitor.h
@@ -116,7 +116,7 @@ struct ofpbuf *ofputil_encode_flow_monitor_cancel(uint32_t id);
 
 struct ofputil_requestforward {
     ovs_be32 xid;
-    /* Also used for OF 1.0-1.3 when using Nicera Extension: */
+    /* Also used for OF 1.0-1.3 when using Nicira Extension: */
     enum ofp14_requestforward_reason reason;
     union {
         /* reason == OFPRFR_METER_MOD. */
diff --git a/lib/ofp-monitor.c b/lib/ofp-monitor.c
index 4a9a6503d4ad..2661a5a5849d 100644
--- a/lib/ofp-monitor.c
+++ b/lib/ofp-monitor.c
@@ -812,9 +812,9 @@ ofputil_encode_requestforward(const struct ofputil_requestforward *rf,
     inner_oh->xid = rf->xid;
     inner_oh->length = htons(inner->size);
 
-    struct ofpbuf *outer = ofpraw_alloc_xid((ofp_version < OFP14_VERSION) ?
-                                            OFPRAW_NXT_REQUESTFORWARD :
-                                            OFPRAW_OFPT14_REQUESTFORWARD,
+    struct ofpbuf *outer = ofpraw_alloc_xid(ofp_version < OFP14_VERSION
+                                            ? OFPRAW_NXT_REQUESTFORWARD
+                                            : OFPRAW_OFPT14_REQUESTFORWARD,
                                             ofp_version, htonl(0),
                                             inner->size);
     ofpbuf_put(outer, inner->data, inner->size);
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index a9b52f986057..9612ec19a2a9 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1693,8 +1693,9 @@ connmgr_send_requestforward(struct connmgr *mgr, const struct ofconn *source,
     LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
         /* METER_MOD only supported in OF13 and up. */
         if (rf->reason == OFPRFR_METER_MOD &&
-            rconn_get_version(ofconn->rconn) < OFP13_VERSION)
+            rconn_get_version(ofconn->rconn) < OFP13_VERSION) {
             continue;
+        }
 
         if (ofconn_receives_async_msg(ofconn, OAM_REQUESTFORWARD, rf->reason)
             && ofconn != source) {


More information about the dev mailing list