Looks good to me, this is very clear!
On Mon, Sep 23, 2013 at 10:49 AM, Ben Pfaff <b...@nicira.com> wrote:
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> ofproto/ofproto-dpif.c | 108
> ++++++++++++++++++------------------------------
> 1 file changed, 41 insertions(+), 67 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 93db491..1c82318 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3956,22 +3956,46 @@ facet_free(struct facet *facet)
>
> /* Executes, within 'ofproto', the 'n_actions' actions in 'actions' on
> * 'packet', which arrived on 'in_port'. */
> -static bool
> -execute_odp_actions(struct ofproto_dpif *ofproto, const struct flow *flow,
> - const struct nlattr *odp_actions, size_t actions_len,
> - struct ofpbuf *packet)
> +static int
> +execute_actions(struct ofproto *ofproto_, const struct flow *flow,
> + struct rule_dpif *rule,
> + const struct ofpact *ofpacts, size_t ofpacts_len,
> + struct ofpbuf *packet)
> {
> + struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> struct odputil_keybuf keybuf;
> + struct dpif_flow_stats stats;
> + struct xlate_out xout;
> + struct xlate_in xin;
> + ofp_port_t in_port;
> struct ofpbuf key;
> int error;
>
> + ovs_assert((rule != NULL) != (ofpacts != NULL));
> +
> + dpif_flow_stats_extract(flow, packet, time_msec(), &stats);
> + if (rule) {
> + rule_dpif_credit_stats(rule, &stats);
> + }
> +
> + xlate_in_init(&xin, ofproto, flow, rule, stats.tcp_flags, packet);
> + xin.ofpacts = ofpacts;
> + xin.ofpacts_len = ofpacts_len;
> + xin.resubmit_stats = &stats;
> + xlate_actions(&xin, &xout);
> +
> ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
> - odp_flow_key_from_flow(&key, flow,
> - ofp_port_to_odp_port(ofproto,
> flow->in_port.ofp_port));
> + in_port = flow->in_port.ofp_port;
> + if (in_port == OFPP_NONE) {
> + in_port = OFPP_LOCAL;
> + }
> + odp_flow_key_from_flow(&key, flow, ofp_port_to_odp_port(ofproto,
> in_port));
>
> error = dpif_execute(ofproto->backer->dpif, key.data, key.size,
> - odp_actions, actions_len, packet);
> - return !error;
> + xout.odp_actions.data, xout.odp_actions.size,
> packet);
> + xlate_out_uninit(&xout);
> +
> + return error;
> }
>
> /* Remove 'facet' from its ofproto and free up the associated memory:
> @@ -4899,22 +4923,7 @@ static void
> rule_dpif_execute(struct rule_dpif *rule, const struct flow *flow,
> struct ofpbuf *packet)
> {
> - struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
> - struct dpif_flow_stats stats;
> - struct xlate_out xout;
> - struct xlate_in xin;
> -
> - dpif_flow_stats_extract(flow, packet, time_msec(), &stats);
> - rule_dpif_credit_stats(rule, &stats);
> -
> - xlate_in_init(&xin, ofproto, flow, rule, stats.tcp_flags, packet);
> - xin.resubmit_stats = &stats;
> - xlate_actions(&xin, &xout);
> -
> - execute_odp_actions(ofproto, flow, xout.odp_actions.data,
> - xout.odp_actions.size, packet);
> -
> - xlate_out_uninit(&xout);
> + execute_actions(rule->up.ofproto, flow, rule, NULL, 0, packet);
> }
>
> static enum ofperr
> @@ -4949,46 +4958,27 @@ static int
> send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
> {
> struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofport->up.ofproto);
> - uint64_t odp_actions_stub[1024 / 8];
> - struct ofpbuf odp_actions;
> - struct dpif_flow_stats stats;
> struct ofpact_output output;
> - struct xlate_out xout;
> - struct xlate_in xin;
> - struct flow flow;
> union flow_in_port in_port_;
> + struct flow flow;
> int error;
>
> - ofpbuf_use_stub(&odp_actions, odp_actions_stub, sizeof
> odp_actions_stub);
> -
> - /* Use OFPP_NONE as the in_port to avoid special packet processing. */
> - in_port_.ofp_port = OFPP_NONE;
> - flow_extract(packet, 0, 0, NULL, &in_port_, &flow);
> - dpif_flow_stats_extract(&flow, packet, time_msec(), &stats);
> -
> ofpact_init(&output.ofpact, OFPACT_OUTPUT, sizeof output);
> output.port = ofport->up.ofp_port;
> output.max_len = 0;
>
> - xlate_in_init(&xin, ofproto, &flow, NULL, 0, packet);
> - xin.ofpacts_len = sizeof output;
> - xin.ofpacts = &output.ofpact;
> - xin.resubmit_stats = &stats;
> - xlate_actions(&xin, &xout);
> -
> - /* The kernel, however, doesn't know about OFPP_NONE. Use a real
> port. */
> - flow.in_port.ofp_port = OFPP_LOCAL;
> - error = execute_odp_actions(ofproto, &flow,
> - xout.odp_actions.data,
> xout.odp_actions.size,
> - packet);
> - xlate_out_uninit(&xout);
> + in_port_.ofp_port = OFPP_NONE;
> + flow_extract(packet, 0, 0, NULL, &in_port_, &flow);
>
> + error = execute_actions(&ofproto->up, &flow, NULL,
> + &output.ofpact, sizeof output, packet);
> if (error) {
> VLOG_WARN_RL(&rl, "%s: failed to send packet on port %s (%s)",
> ofproto->up.name,
> netdev_get_name(ofport->up.netdev),
> ovs_strerror(error));
> }
>
> +
> ofproto->stats.tx_packets++;
> ofproto->stats.tx_bytes += packet->size;
> return error;
> @@ -5048,27 +5038,11 @@ set_frag_handling(struct ofproto *ofproto_,
> }
>
> static enum ofperr
> -packet_out(struct ofproto *ofproto_, struct ofpbuf *packet,
> +packet_out(struct ofproto *ofproto, struct ofpbuf *packet,
> const struct flow *flow,
> const struct ofpact *ofpacts, size_t ofpacts_len)
> {
> - struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
> - struct dpif_flow_stats stats;
> - struct xlate_out xout;
> - struct xlate_in xin;
> -
> - dpif_flow_stats_extract(flow, packet, time_msec(), &stats);
> -
> - xlate_in_init(&xin, ofproto, flow, NULL, stats.tcp_flags, packet);
> - xin.resubmit_stats = &stats;
> - xin.ofpacts_len = ofpacts_len;
> - xin.ofpacts = ofpacts;
> -
> - xlate_actions(&xin, &xout);
> - execute_odp_actions(ofproto, flow,
> - xout.odp_actions.data, xout.odp_actions.size,
> packet);
> - xlate_out_uninit(&xout);
> -
> + execute_actions(ofproto, flow, NULL, ofpacts, ofpacts_len, packet);
> return 0;
> }
>
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
