From: Ben Pfaff Date: Fri, 20 Jul 2012 01:25:02 +0000 (-0700) Subject: ofproto: Move ofpacts_check() calls from ofproto-dpif to ofproto. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=548de4ddb3efed9b7e5e543fec636ae5f077eda7;p=openvswitch ofproto: Move ofpacts_check() calls from ofproto-dpif to ofproto. Signed-off-by: Ben Pfaff --- diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index beb12f0a..3aa29a83 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4638,13 +4638,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; @@ -4747,15 +4740,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); } @@ -6443,36 +6427,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, time_msec(), &stats); - dpif_flow_stats_extract(flow, packet, time_msec(), &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 72efb141..c319d8fa 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 ae24fd68..d773c91c 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -2141,10 +2141,13 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh) 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: @@ -3257,7 +3260,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) {