> This is only desirable for use with VLAN splinters and should be reverted>
> when this feature is no longer needed. I'm breaking it out here only to>
> make the series easier to review.
I think we should have an XXX comment in the code to this affect.
It's going to be easy to forget if this information is only in the
commit message.
if (cookie.type == USER_ACTION_COOKIE_SFLOW) {
if (ofproto->sflow) {
-
odp_flow_key_to_flow(upcall->key, upcall->key_len, &flow);
dpif_sflow_received(ofproto->sflow, upcall->packet, &flow, &cookie);
While you're
here, could you please break up this dpif_sflow line, it's
slightly too long.
Looks good,
Ethan
On Tue, Nov 15, 2011 at 17:17, Ben Pfaff <b...@nicira.com> wrote:
> In an upcoming commit, VLAN splinters can cause the VLAN TCI of a packet
> received on an interface to differ from the logical VLAN TCI. That is,
> a packet that is received on a Linux VLAN network device has no VLAN (so
> its initial VLAN TCI is 0) but we logically treat it as if it has the VLAN
> associated with the VLAN device.
>
> This is only desirable for use with VLAN splinters and should be reverted
> when this feature is no longer needed. I'm breaking it out here only to
> make the series easier to review.
> ---
> ofproto/ofproto-dpif.c | 104
> ++++++++++++++++++++++++++++++++++--------------
> 1 files changed, 74 insertions(+), 30 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 167ec07..88bfc1e 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -225,7 +225,7 @@ struct action_xlate_ctx {
>
> static void action_xlate_ctx_init(struct action_xlate_ctx *,
> struct ofproto_dpif *, const struct flow *,
> - const struct ofpbuf *);
> + ovs_be16 initial_tci, const struct ofpbuf
> *);
> static struct ofpbuf *xlate_actions(struct action_xlate_ctx *,
> const union ofp_action *in, size_t n_in);
>
> @@ -348,12 +348,14 @@ struct subfacet {
> struct nlattr *actions; /* Datapath actions. */
>
> bool installed; /* Installed in datapath? */
> +
> + ovs_be16 initial_tci; /* Initial VLAN TCI value. */
> };
>
> static struct subfacet *subfacet_create(struct ofproto_dpif *, struct facet
> *,
> enum odp_key_fitness,
> const struct nlattr *key,
> - size_t key_len);
> + size_t key_len, ovs_be16
> initial_tci);
> static struct subfacet *subfacet_find(struct ofproto_dpif *,
> const struct nlattr *key, size_t
> key_len,
> const struct flow *);
> @@ -2086,6 +2088,7 @@ struct flow_miss {
> enum odp_key_fitness key_fitness;
> const struct nlattr *key;
> size_t key_len;
> + ovs_be16 initial_tci;
> struct list packets;
> };
>
> @@ -2175,7 +2178,8 @@ process_special(struct ofproto_dpif *ofproto, const
> struct flow *flow,
> static struct flow_miss *
> flow_miss_create(struct hmap *todo, const struct flow *flow,
> enum odp_key_fitness key_fitness,
> - const struct nlattr *key, size_t key_len)
> + const struct nlattr *key, size_t key_len,
> + ovs_be16 initial_tci)
> {
> uint32_t hash = flow_hash(flow, 0);
> struct flow_miss *miss;
> @@ -2192,6 +2196,7 @@ flow_miss_create(struct hmap *todo, const struct flow
> *flow,
> miss->key_fitness = key_fitness;
> miss->key = key;
> miss->key_len = key_len;
> + miss->initial_tci = initial_tci;
> list_init(&miss->packets);
> return miss;
> }
> @@ -2237,7 +2242,8 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct
> flow_miss *miss,
> }
>
> subfacet = subfacet_create(ofproto, facet,
> - miss->key_fitness, miss->key, miss->key_len);
> + miss->key_fitness, miss->key, miss->key_len,
> + miss->initial_tci);
>
> LIST_FOR_EACH_SAFE (packet, next_packet, list_node, &miss->packets) {
> list_remove(&packet->list_node);
> @@ -2294,6 +2300,22 @@ handle_flow_miss(struct ofproto_dpif *ofproto, struct
> flow_miss *miss,
> }
> }
>
> +static enum odp_key_fitness
> +ofproto_dpif_extract_flow_key(const struct ofproto_dpif *ofproto OVS_UNUSED,
> + const struct nlattr *key, size_t key_len,
> + struct flow *flow, ovs_be16 *initial_tci)
> +{
> + enum odp_key_fitness fitness;
> +
> + fitness = odp_flow_key_to_flow(key, key_len, flow);
> + if (fitness == ODP_FIT_ERROR) {
> + return fitness;
> + }
> + *initial_tci = flow->vlan_tci;
> +
> + return fitness;
> +}
> +
> static void
> handle_miss_upcalls(struct ofproto_dpif *ofproto, struct dpif_upcall
> *upcalls,
> size_t n_upcalls)
> @@ -2319,11 +2341,14 @@ handle_miss_upcalls(struct ofproto_dpif *ofproto,
> struct dpif_upcall *upcalls,
> for (upcall = upcalls; upcall < &upcalls[n_upcalls]; upcall++) {
> enum odp_key_fitness fitness;
> struct flow_miss *miss;
> + ovs_be16 initial_tci;
> struct flow flow;
>
> /* Obtain metadata and check userspace/kernel agreement on flow match,
> * then set 'flow''s header pointers. */
> - fitness = odp_flow_key_to_flow(upcall->key, upcall->key_len, &flow);
> + fitness = ofproto_dpif_extract_flow_key(ofproto,
> + upcall->key, upcall->key_len,
> + &flow, &initial_tci);
> if (fitness == ODP_FIT_ERROR) {
> continue;
> }
> @@ -2339,7 +2364,7 @@ handle_miss_upcalls(struct ofproto_dpif *ofproto,
> struct dpif_upcall *upcalls,
>
> /* Add other packets to a to-do list. */
> miss = flow_miss_create(&todo, &flow, fitness,
> - upcall->key, upcall->key_len);
> + upcall->key, upcall->key_len, initial_tci);
> list_push_back(&miss->packets, &upcall->packet->list_node);
> }
>
> @@ -2390,21 +2415,27 @@ static void
> handle_userspace_upcall(struct ofproto_dpif *ofproto,
> struct dpif_upcall *upcall)
> {
> - struct flow flow;
> struct user_action_cookie cookie;
> + enum odp_key_fitness fitness;
> + ovs_be16 initial_tci;
> + struct flow flow;
>
> memcpy(&cookie, &upcall->userdata, sizeof(cookie));
>
> + fitness = ofproto_dpif_extract_flow_key(ofproto, upcall->key,
> + upcall->key_len, &flow,
> + &initial_tci);
> + if (fitness == ODP_FIT_ERROR) {
> + return;
> + }
> +
> if (cookie.type == USER_ACTION_COOKIE_SFLOW) {
> if (ofproto->sflow) {
> - odp_flow_key_to_flow(upcall->key, upcall->key_len, &flow);
> dpif_sflow_received(ofproto->sflow, upcall->packet, &flow,
> &cookie);
> }
> ofpbuf_delete(upcall->packet);
> -
> } else if (cookie.type == USER_ACTION_COOKIE_CONTROLLER) {
> COVERAGE_INC(ofproto_dpif_ctlr_action);
> - odp_flow_key_to_flow(upcall->key, upcall->key_len, &flow);
> send_packet_in_action(ofproto, upcall->packet, upcall->userdata,
> &flow, false);
> } else {
> @@ -2816,7 +2847,8 @@ facet_account(struct ofproto_dpif *ofproto, struct
> facet *facet)
> if (facet->has_learn || facet->has_normal) {
> struct action_xlate_ctx ctx;
>
> - action_xlate_ctx_init(&ctx, ofproto, &facet->flow, NULL);
> + action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
> + facet->flow.vlan_tci, NULL);
> ctx.may_learn = true;
> ofpbuf_delete(xlate_actions(&ctx, facet->rule->up.actions,
> facet->rule->up.n_actions));
> @@ -3004,7 +3036,8 @@ facet_revalidate(struct ofproto_dpif *ofproto, struct
> facet *facet)
> struct ofpbuf *odp_actions;
> bool should_install;
>
> - action_xlate_ctx_init(&ctx, ofproto, &facet->flow, NULL);
> + action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
> + subfacet->initial_tci, NULL);
> odp_actions = xlate_actions(&ctx, new_rule->up.actions,
> new_rule->up.n_actions);
> actions_changed = (subfacet->actions_len != odp_actions->size
> @@ -3150,7 +3183,7 @@ flow_push_stats(const struct rule_dpif *rule,
> push.bytes = bytes;
> push.used = used;
>
> - action_xlate_ctx_init(&push.ctx, ofproto, flow, NULL);
> + action_xlate_ctx_init(&push.ctx, ofproto, flow, flow->vlan_tci, NULL);
> push.ctx.resubmit_hook = push_resubmit;
> ofpbuf_delete(xlate_actions(&push.ctx,
> rule->up.actions, rule->up.n_actions));
> @@ -3188,7 +3221,7 @@ subfacet_find__(struct ofproto_dpif *ofproto,
> static struct subfacet *
> subfacet_create(struct ofproto_dpif *ofproto, struct facet *facet,
> enum odp_key_fitness key_fitness,
> - const struct nlattr *key, size_t key_len)
> + const struct nlattr *key, size_t key_len, ovs_be16
> initial_tci)
> {
> uint32_t key_hash = odp_flow_key_hash(key, key_len);
> struct subfacet *subfacet;
> @@ -3215,6 +3248,7 @@ subfacet_create(struct ofproto_dpif *ofproto, struct
> facet *facet,
> subfacet->key_len = key_len;
> }
> subfacet->installed = false;
> + subfacet->initial_tci = initial_tci;
>
> return subfacet;
> }
> @@ -3282,7 +3316,8 @@ subfacet_make_actions(struct ofproto_dpif *p, struct
> subfacet *subfacet,
> struct ofpbuf *odp_actions;
> struct action_xlate_ctx ctx;
>
> - action_xlate_ctx_init(&ctx, p, &facet->flow, packet);
> + action_xlate_ctx_init(&ctx, p, &facet->flow, subfacet->initial_tci,
> + packet);
> odp_actions = xlate_actions(&ctx, rule->up.actions, rule->up.n_actions);
> facet->tags = ctx.tags;
> facet->may_install = ctx.may_set_up_flow;
> @@ -3559,7 +3594,7 @@ rule_execute(struct rule *rule_, const struct flow
> *flow,
> struct ofpbuf *odp_actions;
> size_t size;
>
> - action_xlate_ctx_init(&ctx, ofproto, flow, packet);
> + action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci, packet);
> odp_actions = xlate_actions(&ctx, rule->up.actions, rule->up.n_actions);
> size = packet->size;
> if (execute_odp_actions(ofproto, flow, odp_actions->data,
> @@ -4427,10 +4462,13 @@ do_xlate_actions(const union ofp_action *in, size_t
> n_in,
> static void
> action_xlate_ctx_init(struct action_xlate_ctx *ctx,
> struct ofproto_dpif *ofproto, const struct flow *flow,
> - const struct ofpbuf *packet)
> + ovs_be16 initial_tci, const struct ofpbuf *packet)
> {
> ctx->ofproto = ofproto;
> ctx->flow = *flow;
> + ctx->base_flow = ctx->flow;
> + ctx->base_flow.tun_id = 0;
> + ctx->base_flow.vlan_tci = initial_tci;
> ctx->packet = packet;
> ctx->may_learn = packet != NULL;
> ctx->resubmit_hook = NULL;
> @@ -4451,8 +4489,6 @@ xlate_actions(struct action_xlate_ctx *ctx,
> ctx->nf_output_iface = NF_OUT_DROP;
> ctx->recurse = 0;
> ctx->original_priority = ctx->flow.priority;
> - ctx->base_flow = ctx->flow;
> - ctx->base_flow.tun_id = 0;
> ctx->table_id = 0;
> ctx->exit = false;
>
> @@ -5248,7 +5284,7 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf
> *packet,
> ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
> odp_flow_key_from_flow(&key, flow);
>
> - action_xlate_ctx_init(&ctx, ofproto, flow, packet);
> + action_xlate_ctx_init(&ctx, ofproto, flow, flow->vlan_tci, packet);
> odp_actions = xlate_actions(&ctx, ofp_actions, n_ofp_actions);
> dpif_execute(ofproto->dpif, key.data, key.size,
> odp_actions->data, odp_actions->size, packet);
> @@ -5451,6 +5487,7 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, const
> char *args_,
> struct ofpbuf odp_key;
> struct ofpbuf *packet;
> struct rule_dpif *rule;
> + ovs_be16 initial_tci;
> struct ds result;
> struct flow flow;
> char *s;
> @@ -5460,6 +5497,17 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, const
> char *args_,
> ds_init(&result);
>
> dpname = strtok_r(args, " ", &save_ptr);
> + if (!dpname) {
> + unixctl_command_reply(conn, 501, "Bad command syntax");
> + goto exit;
> + }
> +
> + ofproto = ofproto_dpif_lookup(dpname);
> + if (!ofproto) {
> + unixctl_command_reply(conn, 501, "Unknown ofproto (use ofproto/list "
> + "for help)");
> + goto exit;
> + }
> arg1 = strtok_r(NULL, " ", &save_ptr);
> arg2 = strtok_r(NULL, " ", &save_ptr);
> arg3 = strtok_r(NULL, " ", &save_ptr);
> @@ -5477,8 +5525,10 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, const
> char *args_,
> }
>
> /* Convert odp_key to flow. */
> - error = odp_flow_key_to_flow(odp_key.data, odp_key.size, &flow);
> - if (error) {
> + error = ofproto_dpif_extract_flow_key(ofproto, odp_key.data,
> + odp_key.size, &flow,
> + &initial_tci);
> + if (error == ODP_FIT_ERROR) {
> unixctl_command_reply(conn, 501, "Invalid flow");
> goto exit;
> }
> @@ -5517,18 +5567,12 @@ ofproto_unixctl_trace(struct unixctl_conn *conn,
> const char *args_,
> free(s);
>
> flow_extract(packet, priority, tun_id, in_port, &flow);
> + initial_tci = flow.vlan_tci;
> } else {
> unixctl_command_reply(conn, 501, "Bad command syntax");
> goto exit;
> }
>
> - ofproto = ofproto_dpif_lookup(dpname);
> - if (!ofproto) {
> - unixctl_command_reply(conn, 501, "Unknown ofproto (use ofproto/list "
> - "for help)");
> - goto exit;
> - }
> -
> ds_put_cstr(&result, "Flow: ");
> flow_format(&result, &flow);
> ds_put_char(&result, '\n');
> @@ -5541,7 +5585,7 @@ ofproto_unixctl_trace(struct unixctl_conn *conn, const
> char *args_,
>
> trace.result = &result;
> trace.flow = flow;
> - action_xlate_ctx_init(&trace.ctx, ofproto, &flow, packet);
> + action_xlate_ctx_init(&trace.ctx, ofproto, &flow, initial_tci,
> packet);
> trace.ctx.resubmit_hook = trace_resubmit;
> odp_actions = xlate_actions(&trace.ctx,
> rule->up.actions, rule->up.n_actions);
> --
> 1.7.4.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
