On Mar 5, 2013, at 9:51 AM, Ben Pfaff <b...@nicira.com> wrote:
> "git am" says:
>
> Applying: ofproto-dpif: Make initial packet value handling generic.
> /home/blp/nicira/ovs/.git/rebase-apply/patch:140: trailing whitespace.
> action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
> warning: 1 line adds whitespace errors.
Fixed.
> The "*" in '*initial_vals->vlan_tci' is wrong I think:
D'oh.
> The use of 'one_subfacet' here worries me. 'one_subfacet' can
> theoretically not be in use at any given time.
I wonder if we should update the comment for 'one_subfacet':
/* Storage for a single subfacet, to reduce malloc() time and space
* overhead. (A facet always has at least one subfacet and in the common
* case has exactly one subfacet.) */
struct subfacet one_subfacet;
My reading of that lead me to believe that it would always have a value. I'm
happy to supply a patch, if you agree.
> It would be better to
> find a subfacet via the facet's list of subfacets:
>> + action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
>> + &facet->one_subfacet.initial_vals, rule, 0, NULL);
>> ctx.resubmit_stats = stats;
>> xlate_actions_for_side_effects(&ctx, rule->up.ofpacts,
>> rule->up.ofpacts_len);
Fixed.
I've attached an incremental at the end of this message.
--Justin
-=-=-=-=-=-=-=-=-
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index a0f3b13..fa8a4b2 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3664,7 +3664,7 @@ drop_key_clear(struct dpif_backer *backer)
* flow->vlan_tci correctly for the VLAN of the VLAN splinter port, and pushes
* a VLAN header onto 'packet' (if it is nonnull).
*
- * Optionally, if nonnull, sets '*initial_vals->vlan_tci' to the VLAN TCI
+ * Optionally, if nonnull, sets 'initial_vals->vlan_tci' to the VLAN TCI
* with which the packet was really received, that is, the actual VLAN
* TCI extracted by odp_flow_key_to_flow(). (This differs from the
* value returned in flow->vlan_tci only for packets received on VLAN
@@ -4427,6 +4427,8 @@ static void
facet_learn(struct facet *facet)
{
struct ofproto_dpif *ofproto = ofproto_dpif_cast(facet->rule->up.ofproto);
+ struct subfacet *subfacet= CONTAINER_OF(list_front(&facet->subfacets),
+ struct subfacet, list_node);
struct action_xlate_ctx ctx;
if (!facet->has_learn
@@ -4436,8 +4438,8 @@ facet_learn(struct facet *facet)
return;
}
- action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
- &facet->one_subfacet.initial_vals,
+ action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
+ &subfacet->initial_vals,
facet->rule, facet->tcp_flags, NULL);
ctx.may_learn = true;
xlate_actions_for_side_effects(&ctx, facet->rule->up.ofpacts,
@@ -4944,12 +4946,14 @@ flow_push_stats(struct facet *facet, const struct dpif_f
{
struct rule_dpif *rule = facet->rule;
struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
+ struct subfacet *subfacet = CONTAINER_OF(list_front(&facet->subfacets),
+ struct subfacet, list_node);
struct action_xlate_ctx ctx;
ofproto_rule_update_used(&rule->up, stats->used);
action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
- &facet->one_subfacet.initial_vals, rule, 0, NULL);
+ &subfacet->initial_vals, rule, 0, NULL);
ctx.resubmit_stats = stats;
xlate_actions_for_side_effects(&ctx, rule->up.ofpacts,
rule->up.ofpacts_len);
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
