Signed-off-by: Ben Pfaff <[email protected]>
---
ofproto/ofproto-dpif.c | 58 ++++++++++++++-----------------------------
ofproto/ofproto-provider.h | 14 ++++-------
ofproto/ofproto.c | 16 +++++++++---
3 files changed, 36 insertions(+), 52 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index f1d2ae0..44cbd17 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4592,13 +4592,6 @@ rule_construct(struct rule *rule_)
struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
struct rule_dpif *victim;
uint8_t table_id;
- enum ofperr error;
-
- error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
- &rule->up.cr.flow, ofproto->up.max_ports);
- if (error) {
- return error;
- }
rule->packet_count = 0;
rule->byte_count = 0;
@@ -4701,15 +4694,6 @@ static void
rule_modify_actions(struct rule *rule_)
{
struct rule_dpif *rule = rule_dpif_cast(rule_);
- struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
- enum ofperr error;
-
- error = ofpacts_check(rule->up.ofpacts, rule->up.ofpacts_len,
- &rule->up.cr.flow, ofproto->up.max_ports);
- if (error) {
- ofoperation_complete(rule->up.pending, error);
- return;
- }
complete_operation(rule);
}
@@ -6359,36 +6343,32 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf
*packet,
const struct ofpact *ofpacts, size_t ofpacts_len)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
- enum ofperr error;
+ struct odputil_keybuf keybuf;
+ struct dpif_flow_stats stats;
- error = ofpacts_check(ofpacts, ofpacts_len, flow, ofproto->up.max_ports);
- if (!error) {
- struct odputil_keybuf keybuf;
- struct dpif_flow_stats stats;
+ struct ofpbuf key;
- struct ofpbuf key;
+ struct action_xlate_ctx ctx;
+ uint64_t odp_actions_stub[1024 / 8];
+ struct ofpbuf odp_actions;
- struct action_xlate_ctx ctx;
- uint64_t odp_actions_stub[1024 / 8];
- struct ofpbuf odp_actions;
+ ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
+ odp_flow_key_from_flow(&key, flow);
- ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
- odp_flow_key_from_flow(&key, flow);
+ dpif_flow_stats_extract(flow, packet, &stats);
- dpif_flow_stats_extract(flow, packet, &stats);
+ action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci, NULL,
+ packet_get_tcp_flags(packet, flow), packet);
+ ctx.resubmit_stats = &stats;
- action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci, NULL,
- packet_get_tcp_flags(packet, flow), packet);
- ctx.resubmit_stats = &stats;
+ ofpbuf_use_stub(&odp_actions,
+ odp_actions_stub, sizeof odp_actions_stub);
+ xlate_actions(&ctx, ofpacts, ofpacts_len, &odp_actions);
+ dpif_execute(ofproto->dpif, key.data, key.size,
+ odp_actions.data, odp_actions.size, packet);
+ ofpbuf_uninit(&odp_actions);
- ofpbuf_use_stub(&odp_actions,
- odp_actions_stub, sizeof odp_actions_stub);
- xlate_actions(&ctx, ofpacts, ofpacts_len, &odp_actions);
- dpif_execute(ofproto->dpif, key.data, key.size,
- odp_actions.data, odp_actions.size, packet);
- ofpbuf_uninit(&odp_actions);
- }
- return error;
+ return 0;
}
/* NetFlow. */
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 2f62473..80cd55f 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -803,11 +803,7 @@ struct ofproto_class {
* registers, then it is an error if 'rule->cr' does not wildcard all
* registers.
*
- * - Validate that 'rule->ofpacts' is a sequence of well-formed actions
- * that the datapath can correctly implement. If your ofproto
- * implementation only implements a subset of the actions that Open
- * vSwitch understands, then you should implement your own action
- * validation.
+ * - Validate that the datapath can correctly implement 'rule->ofpacts'.
*
* - If the rule is valid, update the datapath flow table, adding the new
* rule or replacing the existing one.
@@ -889,8 +885,8 @@ struct ofproto_class {
*
* ->rule_modify_actions() should set the following in motion:
*
- * - Validate that the actions now in 'rule' are well-formed OpenFlow
- * actions that the datapath can correctly implement.
+ * - Validate that the datapath can correctly implement the actions now
+ * in 'rule'.
*
* - Update the datapath flow table with the new actions.
*
@@ -943,8 +939,8 @@ struct ofproto_class {
* The caller retains ownership of 'packet' and of 'ofpacts', so
* ->packet_out() should not modify or free them.
*
- * This function must validate that it can implement 'ofpacts'. If not,
- * then it should return an OpenFlow error code.
+ * This function must validate that it can correctly implement 'ofpacts'.
+ * If not, then it should return an OpenFlow error code.
*
* 'flow' reflects the flow information for 'packet'. All of the
* information in 'flow' is extracted from 'packet', except for
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index cbcf0d2..17131ca 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2138,10 +2138,13 @@ handle_packet_out(struct ofconn *ofconn, const struct
ofp_packet_out *opo)
ofpbuf_use_const(payload, po.packet, po.packet_len);
}
- /* Send out packet. */
+ /* Verify actions against packet, then send packet if successful. */
flow_extract(payload, 0, 0, po.in_port, &flow);
- error = p->ofproto_class->packet_out(p, payload, &flow,
- po.ofpacts, po.ofpacts_len);
+ error = ofpacts_check(po.ofpacts, po.ofpacts_len, &flow, p->max_ports);
+ if (!error) {
+ error = p->ofproto_class->packet_out(p, payload, &flow,
+ po.ofpacts, po.ofpacts_len);
+ }
ofpbuf_delete(payload);
exit_free_ofpacts:
@@ -3237,7 +3240,12 @@ handle_flow_mod(struct ofconn *ofconn, const struct
ofp_header *oh)
* dropped from OpenFlow in the near future. There is no good error
* code, so just state that the flow table is full. */
error = OFPERR_OFPFMFC_ALL_TABLES_FULL;
- } else {
+ }
+ if (!error) {
+ error = ofpacts_check(fm.ofpacts, fm.ofpacts_len,
+ &fm.cr.flow, ofproto->max_ports);
+ }
+ if (!error) {
error = handle_flow_mod__(ofconn_get_ofproto(ofconn), ofconn, &fm, oh);
}
if (error) {
--
1.7.2.5
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
