From: Niti Rohilla <niti.rohi...@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.rohi...@tcs.com>
---
Difference between v2->v3
- Changed ADD_OF_PORTS function and its syntax with add_of_ports
function in tests/ofproto-dpif.at.
- Rebsaed with latest master
lib/ofp-actions.c | 45 +++++++++++++++++
lib/ofp-util.h | 1 +
ofproto/ofproto-dpif-xlate.c | 113 ++++++++++++++++++++++++++++++-------------
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(+), 42 deletions(-)
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 74a2ec1..5692dba 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 a6ea067..281a4df 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)
{
@@ -5136,6 +5180,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 adfaeb6..07b4e01 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 1da43b3..4f8f61c 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1825,6 +1825,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 dc31b7c..9a4a770 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -5305,10 +5305,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);
@@ -7099,11 +7101,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 a372d36..795bdeb 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -7194,3 +7194,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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
