[ovs-dev] [PATCH 2/2 v2] ofproto: This patch adds support for egress pipeline processing

niti1489 at gmail.com niti1489 at gmail.com
Wed Jan 27 11:45:06 UTC 2016


From: Niti Rohilla <niti.rohilla at tcs.com>

Following have been implemented:
- When a packet is sent to output port, it will start egress
  processing in the context of the output port.
- Added check to forbid adding output or group action in the
  egress action-set to prevent changing output port.
- Added check to forbid adding output or group action via apply-
  action instruction in the egress tables as egress mirroring is
  currently not supported.
- Added check to forbid the ingress flow tables to direct the
  packets to egress flow tables via GOTO_TABLE instruction.

Signed-off-by: Niti Rohilla <niti.rohilla at tcs.com>
---
Difference between v1->v2

- Added check to forbid adding group and output action via apply-action
  instruction in egress tables as egress mirroring is currently not
  supported.
- Output port is added to the actset_output when a packet starts egress
  processing.
- Rebased with latest master

 lib/ofp-actions.c            |  45 +++++++++++++++++
 lib/ofp-util.h               |   1 +
 ofproto/ofproto-dpif-xlate.c | 114 ++++++++++++++++++++++++++++++-------------
 ofproto/ofproto-dpif.c       |   5 ++
 ofproto/ofproto-dpif.h       |   3 ++
 ofproto/ofproto-provider.h   |   1 +
 ofproto/ofproto.c            |   4 ++
 tests/ofproto-dpif.at        |  19 ++++++++
 utilities/ovs-ofctl.c        | 103 +++++++++++++++++++++++++++++++++++---
 9 files changed, 252 insertions(+), 43 deletions(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 651c91e..bba0769 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -6401,6 +6401,16 @@ ofpact_check__(enum ofputil_protocol *usable_protocols, struct ofpact *a,
 
     switch (a->type) {
     case OFPACT_OUTPUT:
+       /* This check forbid adding group action via apply-action instruction
+        * in the egress tables because currently egress mirroring is not
+        * supported. */
+        if (*usable_protocols & OFPUTIL_P_OF15_UP) {
+            if (ofputil_egress_table_id(0) &&
+                table_id >= ofputil_egress_table_id(0)) {
+                VLOG_WARN_RL(&rl, "disallowed action in action set");
+                return OFPERR_OFPBAC_BAD_ARGUMENT;
+            }
+        }
         return ofpact_check_output_port(ofpact_get_OUTPUT(a)->port,
                                         max_ports);
 
@@ -6622,6 +6632,21 @@ ofpact_check__(enum ofputil_protocol *usable_protocols, struct ofpact *a,
          * consistency of an action set. */
         struct ofpact_nest *on = ofpact_get_WRITE_ACTIONS(a);
         enum ofputil_protocol p = *usable_protocols;
+        struct ofpact *a;
+        /* This check forbid adding output or group action in the egress
+         * action-set to prevent changing output port, if egress table is
+         * supported and configured. */
+        if (*usable_protocols & OFPUTIL_P_OF15_UP) {
+            OFPACT_FOR_EACH (a, on->actions, ofpact_nest_get_action_len(on)) {
+                if (ofputil_egress_table_id(0) &&
+                    table_id >= ofputil_egress_table_id(0)) {
+                    if (a->type == OFPACT_OUTPUT || a->type == OFPACT_GROUP) {
+                        VLOG_WARN_RL(&rl, "disallowed action in action set");
+                        return OFPERR_OFPBAC_BAD_ARGUMENT;
+                    }
+                }
+            }
+        }
         return ofpacts_check(on->actions, ofpact_nest_get_action_len(on),
                              flow, max_ports, table_id, n_tables, &p);
     }
@@ -6643,10 +6668,30 @@ ofpact_check__(enum ofputil_protocol *usable_protocols, struct ofpact *a,
             || (n_tables != 255 && goto_table >= n_tables)) {
             return OFPERR_OFPBIC_BAD_TABLE_ID;
         }
+        /* This check forbids the ingress flow tables to direct the packets to
+         * egress flow tables via GOTO_TABLE instruction. */
+        if (*usable_protocols & OFPUTIL_P_OF15_UP) {
+            if (ofputil_egress_table_id(0) &&
+                table_id < ofputil_egress_table_id(0)) {
+                if (goto_table >= ofputil_egress_table_id(0)) {
+                    return OFPERR_OFPBIC_BAD_TABLE_ID;
+                }
+            }
+        }
         return 0;
     }
 
     case OFPACT_GROUP:
+        /* This check forbid adding group action via apply-action instruction
+         * in the egress tables because currently egress mirroring is not
+         * supported. */
+        if (*usable_protocols & OFPUTIL_P_OF15_UP) {
+            if (ofputil_egress_table_id(0) &&
+                table_id >= ofputil_egress_table_id(0)) {
+                VLOG_WARN_RL(&rl, "disallowed action in action set");
+                return OFPERR_OFPBAC_BAD_ARGUMENT;
+            }
+        }
         return 0;
 
     case OFPACT_UNROLL_XLATE:
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 6e5e117..8ec061f 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -332,6 +332,7 @@ enum ofperr ofputil_decode_flow_mod(struct ofputil_flow_mod *,
                                     struct ofpbuf *ofpacts,
                                     ofp_port_t max_port,
                                     uint8_t max_table);
+
 struct ofpbuf *ofputil_encode_flow_mod(const struct ofputil_flow_mod *,
                                        enum ofputil_protocol);
 
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 4b25bf4..748d5ec 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -319,6 +319,8 @@ struct xlate_ctx {
     struct ofpbuf action_set;   /* Action set. */
 
     enum xlate_error error;     /* Translation failed. */
+    bool egress_processing;     /* Flag to check whether to start egress
+                                 * processing in  this context or not. */
 };
 
 const char *xlate_strerror(enum xlate_error error)
@@ -2905,9 +2907,10 @@ clear_conntrack(struct flow *flow)
 }
 
 static void
-compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
-                        const struct xlate_bond_recirc *xr, bool check_stp)
+compose_output__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
+                 const struct xlate_bond_recirc *xr)
 {
+
     const struct xport *xport = get_ofp_port(ctx->xbridge, ofp_port);
     struct flow_wildcards *wc = ctx->wc;
     struct flow *flow = &ctx->xin->flow;
@@ -2924,38 +2927,6 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
     BUILD_ASSERT_DECL(FLOW_WC_SEQ == 35);
     memset(&flow_tnl, 0, sizeof flow_tnl);
 
-    if (!xport) {
-        xlate_report(ctx, "Nonexistent output port");
-        return;
-    } else if (xport->config & OFPUTIL_PC_NO_FWD) {
-        xlate_report(ctx, "OFPPC_NO_FWD set, skipping output");
-        return;
-    } else if (check_stp) {
-        if (is_stp(&ctx->base_flow)) {
-            if (!xport_stp_should_forward_bpdu(xport) &&
-                !xport_rstp_should_manage_bpdu(xport)) {
-                if (ctx->xbridge->stp != NULL) {
-                    xlate_report(ctx, "STP not in listening state, "
-                            "skipping bpdu output");
-                } else if (ctx->xbridge->rstp != NULL) {
-                    xlate_report(ctx, "RSTP not managing BPDU in this state, "
-                            "skipping bpdu output");
-                }
-                return;
-            }
-        } else if (!xport_stp_forward_state(xport) ||
-                   !xport_rstp_forward_state(xport)) {
-            if (ctx->xbridge->stp != NULL) {
-                xlate_report(ctx, "STP not in forwarding state, "
-                        "skipping output");
-            } else if (ctx->xbridge->rstp != NULL) {
-                xlate_report(ctx, "RSTP not in forwarding state, "
-                        "skipping output");
-            }
-            return;
-        }
-    }
-
     if (xport->peer) {
         const struct xport *peer = xport->peer;
         struct flow old_flow = ctx->xin->flow;
@@ -3193,6 +3164,79 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
 }
 
 static void
+compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
+                        const struct xlate_bond_recirc *xr, bool check_stp)
+{
+    const struct xport *xport = get_ofp_port(ctx->xbridge, ofp_port);
+    struct flow *flow = &ctx->xin->flow;
+
+    if (!xport) {
+        xlate_report(ctx, "Nonexistent output port");
+        return;
+    } else if (xport->config & OFPUTIL_PC_NO_FWD) {
+        xlate_report(ctx, "OFPPC_NO_FWD set, skipping output");
+        return;
+    } else if (check_stp) {
+        if (is_stp(&ctx->base_flow)) {
+            if (!xport_stp_should_forward_bpdu(xport) &&
+                !xport_rstp_should_manage_bpdu(xport)) {
+                if (ctx->xbridge->stp != NULL) {
+                    xlate_report(ctx, "STP not in listening state, "
+                            "skipping bpdu output");
+                } else if (ctx->xbridge->rstp != NULL) {
+                    xlate_report(ctx, "RSTP not managing BPDU in this state, "
+                            "skipping bpdu output");
+                }
+                return;
+            }
+        } else if (!xport_stp_forward_state(xport) ||
+                   !xport_rstp_forward_state(xport)) {
+            if (ctx->xbridge->stp != NULL) {
+                xlate_report(ctx, "STP not in forwarding state, "
+                        "skipping output");
+            } else if (ctx->xbridge->rstp != NULL) {
+                xlate_report(ctx, "RSTP not in forwarding state, "
+                        "skipping output");
+            }
+            return;
+        }
+    }
+
+    if (ofproto_first_egress_table_id(ctx->xbridge->ofproto) != 0) {
+        if (ctx->egress_processing == false) {
+            struct ofpact_output *output;
+
+            ctx->egress_processing = true;
+            output = ofpact_put_OUTPUT(&ctx->action_set);
+            output->port = ofp_port;
+            ctx->xin->flow.actset_output = ofp_port;
+
+            xlate_table_action(ctx, flow->in_port.ofp_port,
+                               ofproto_first_egress_table_id(
+                               ctx->xbridge->ofproto), true, true);
+            if (ctx->action_set.size) {
+                /* Translate action set only if not dropping the packet and
+                 * not recirculating. */
+                if (!exit_recirculates(ctx)) {
+                    xlate_action_set(ctx);
+                    ctx->egress_processing = false;
+                }
+            }
+            /* Check if need to recirculate. */
+            if (exit_recirculates(ctx)) {
+                compose_recirculate_action(ctx);
+            }
+            compose_output__(ctx, ofp_port, xr);
+        }
+    }
+    else {
+         compose_output__(ctx, ofp_port, xr);
+    }
+
+}
+
+
+static void
 compose_output_action(struct xlate_ctx *ctx, ofp_port_t ofp_port,
                       const struct xlate_bond_recirc *xr)
 {
@@ -4764,7 +4808,6 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
 
         case OFPACT_GOTO_TABLE: {
             struct ofpact_goto_table *ogt = ofpact_get_GOTO_TABLE(a);
-
             /* Allow ctx->table_id == TBL_INTERNAL, which will be greater
              * than ogt->table_id. This is to allow goto_table actions that
              * triggered recirculation: ctx->table_id will be TBL_INTERNAL
@@ -5118,6 +5161,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
 
         .action_set_has_group = false,
         .action_set = OFPBUF_STUB_INITIALIZER(action_set_stub),
+        .egress_processing = false,
     };
 
     /* 'base_flow' reflects the packet as it came in, but we need it to reflect
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 89e06aa..6f1b8dc 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5724,6 +5724,11 @@ ofproto_dpif_get_uuid(const struct ofproto_dpif *ofproto)
     return &ofproto->uuid;
 }
 
+uint8_t ofproto_first_egress_table_id(struct ofproto_dpif *ofproto)
+{
+    return ofproto->up.first_egress_table_id;
+}
+
 const struct ofproto_class ofproto_dpif_class = {
     init,
     enumerate_types,
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 0064178..15a64ef 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -180,6 +180,9 @@ int ofproto_dpif_delete_internal_flow(struct ofproto_dpif *, struct match *,
                                       int priority);
 
 const struct uuid *ofproto_dpif_get_uuid(const struct ofproto_dpif *);
+
+uint8_t ofproto_first_egress_table_id(struct ofproto_dpif *);
+
 
 /* struct rule_dpif has struct rule as it's first member. */
 #define RULE_CAST(RULE) ((struct rule *)RULE)
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 3a84c5d..3112662 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1804,6 +1804,7 @@ void ofproto_flush_flows(struct ofproto *);
 enum ofperr ofproto_check_ofpacts(struct ofproto *,
                                   const struct ofpact ofpacts[],
                                   size_t ofpacts_len);
+
 
 static inline const struct rule_actions *
 rule_get_actions(const struct rule *rule)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index fdcbb80..f86982d 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -5308,10 +5308,12 @@ handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
     }
 
     ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
+    ovs_mutex_lock(&ofproto_mutex);
     error = ofputil_decode_flow_mod(&ofm.fm, oh, ofconn_get_protocol(ofconn),
                                     &ofpacts,
                                     u16_to_ofp(ofproto->max_ports),
                                     ofproto->n_tables);
+    ovs_mutex_unlock(&ofproto_mutex);
     if (!error) {
         error = ofproto_check_ofpacts(ofproto, ofm.fm.ofpacts,
                                       ofm.fm.ofpacts_len);
@@ -7102,11 +7104,13 @@ handle_bundle_add(struct ofconn *ofconn, const struct ofp_header *oh)
         uint64_t ofpacts_stub[1024 / 8];
 
         ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub);
+        ovs_mutex_lock(&ofproto_mutex);
         error = ofputil_decode_flow_mod(&bmsg->ofm.fm, badd.msg,
                                         ofconn_get_protocol(ofconn),
                                         &ofpacts,
                                         u16_to_ofp(ofproto->max_ports),
                                         ofproto->n_tables);
+        ovs_mutex_unlock(&ofproto_mutex);
         /* Move actions to heap. */
         bmsg->ofm.fm.ofpacts = ofpbuf_steal_data(&ofpacts);
 
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index bfb1b56..d188d7c 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -7172,3 +7172,22 @@ dpif_netdev|DBG|flow_add: recirc_id=0,in_port=1,vlan_tci=0xf063/0x1000,dl_type=0
 ])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([ofproto-dpif - egress table])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [2])
+AT_CHECK([ovs-ofctl -O Openflow15 set-first-egress-table br0 200])
+AT_DATA([flows.txt], [dnl
+table=0 in_port=2,ip actions=output(1),write_actions(set_field:10.1.1.11->ip_src,output(1))
+table=0, in_port=1,actions=mod_dl_dst:00:00:00:00:08:aa,output:2
+table=200, in_port=2,actions=mod_dl_src:00:e0:4c:36:09:55
+table=200, in_port=1,actions=mod_dl_dst:00:00:00:00:08:ad
+])
+AT_CHECK([ovs-ofctl -O OpenFlow15 add-flows br0 flows.txt])
+AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=2,dl_src=00:00:00:00:08:ad,dl_dst=00:e0:4c:36:09:5e,dl_type=0x0800,nw_src=10.1.1.20,nw_dst=10.1.1.30,nw_proto=1,nw_tos=0,nw_ttl=128,icmp_type=8,icmp_code=0'], [0], [stdout])
+AT_CHECK([tail -2 stdout], [0],
+  [Megaflow: recirc_id=0,ip,in_port=2,dl_src=00:00:00:00:08:ad,nw_src=10.1.1.20,nw_frag=no
+Datapath actions: set(eth(src=00:e0:4c:36:09:55)),1,set(ipv4(src=10.1.1.11)),1
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 106fdde..4e8f296 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1299,6 +1299,61 @@ ofctl_queue_get_config(struct ovs_cmdl_context *ctx)
     vconn_close(vconn);
 }
 
+static void
+fetch_table_features(struct vconn *vconn)
+{
+    struct ofpbuf *request;
+    ovs_be32 send_xid;
+    bool done = false;
+    bool found = false;
+    struct ofputil_table_features *tf = NULL;
+    struct ofputil_table_features tf_reply;
+
+    request = ofputil_encode_table_features_request(tf, vconn_get_version(vconn));
+
+    send_xid = ((struct ofp_header *) request->data)->xid;
+    send_openflow_buffer(vconn, request);
+    while (!done) {
+        ovs_be32 recv_xid;
+        struct ofpbuf *reply;
+
+        run(vconn_recv_block(vconn, &reply), "OpenFlow packet receive failed");
+        recv_xid = ((struct ofp_header *) reply->data)->xid;
+        if (send_xid == recv_xid) {
+            struct ofp_header *oh = reply->data;
+            enum ofptype type;
+            struct ofpbuf b;
+            uint16_t flags;
+
+            ofpbuf_use_const(&b, oh, ntohs(oh->length));
+            if (ofptype_pull(&type, &b)
+                || type != OFPTYPE_TABLE_FEATURES_STATS_REPLY) {
+                ovs_fatal(0, "received bad reply: %s",
+                          ofp_to_string(reply->data, reply->size,
+                                        verbosity + 1));
+            }
+            flags = ofpmp_flags(oh);
+            done = !(flags & OFPSF_REPLY_MORE);
+            if (found) {
+                /* We've already found the table desc consisting of current
+                 * table configuration, but we need to drain the queue of
+                 * any other replies for this request. */
+                continue;
+            }
+            while (! ofputil_decode_table_features(&b, &tf_reply, true)) {
+                if ((tf_reply.features & OFPTFF_FIRST_EGRESS) != 0) {
+                    found = true;
+                    ofputil_egress_table_id(tf_reply.table_id);
+                    break;
+                }
+            }
+        } else {
+            VLOG_DBG("received reply with xid %08"PRIx32" "
+                     "!= expected %08"PRIx32, recv_xid, send_xid);
+        }
+        ofpbuf_delete(reply);
+    }
+}
 static enum ofputil_protocol
 open_vconn_for_flow_mod(const char *remote, struct vconn **vconnp,
                         enum ofputil_protocol usable_protocols)
@@ -1395,6 +1450,7 @@ ofctl_flow_mod_file(int argc OVS_UNUSED, char *argv[], int command)
     struct ofputil_flow_mod *fms = NULL;
     size_t n_fms = 0;
     char *error;
+    uint32_t allowed_versions = get_allowed_ofp_versions();
 
     if (command == OFPFC_ADD) {
         /* Allow the file to specify a mix of commands.  If none specified at
@@ -1402,10 +1458,25 @@ ofctl_flow_mod_file(int argc OVS_UNUSED, char *argv[], int command)
          * this is backwards compatible. */
         command = -2;
     }
-    error = parse_ofp_flow_mod_file(argv[2], command, &fms, &n_fms,
-                                    &usable_protocols);
-    if (error) {
-        ovs_fatal(0, "%s", error);
+
+    if (allowed_versions & (1u << OFP15_VERSION)) {
+
+        struct vconn *vconn;
+        open_vconn_for_flow_mod(argv[1], &vconn, OFPUTIL_P_OF15_UP);
+
+        fetch_table_features(vconn);
+        error = parse_ofp_flow_mod_file(argv[2], command, &fms, &n_fms,
+                                        &usable_protocols);
+        if (error) {
+            ovs_fatal(0, "%s", error);
+        }
+        vconn_close(vconn);
+    } else {
+        error = parse_ofp_flow_mod_file(argv[2], command, &fms, &n_fms,
+                                        &usable_protocols);
+        if (error) {
+            ovs_fatal(0, "%s", error);
+        }
     }
     ofctl_flow_mod__(argv[1], fms, n_fms, usable_protocols);
     free(fms);
@@ -1421,10 +1492,26 @@ ofctl_flow_mod(int argc, char *argv[], uint16_t command)
         char *error;
         enum ofputil_protocol usable_protocols;
 
-        error = parse_ofp_flow_mod_str(&fm, argc > 2 ? argv[2] : "", command,
-                                       &usable_protocols);
-        if (error) {
-            ovs_fatal(0, "%s", error);
+        uint32_t allowed_versions = get_allowed_ofp_versions();
+
+        if (allowed_versions & (1u << OFP15_VERSION)) {
+
+            struct vconn *vconn;
+            open_vconn_for_flow_mod(argv[1], &vconn, OFPUTIL_P_OF15_UP);
+
+            fetch_table_features(vconn);
+            error = parse_ofp_flow_mod_str(&fm, argc > 2 ? argv[2] : "", command,
+                                           &usable_protocols);
+            if (error) {
+                ovs_fatal(0, "%s", error);
+            }
+            vconn_close(vconn);
+        } else {
+            error = parse_ofp_flow_mod_str(&fm, argc > 2 ? argv[2] : "", command,
+                                           &usable_protocols);
+            if (error) {
+                ovs_fatal(0, "%s", error);
+            }
         }
         ofctl_flow_mod__(argv[1], &fm, 1, usable_protocols);
     }
-- 
2.5.0




More information about the dev mailing list