On Feb 14, 2013, at 12:04 PM, Ben Pfaff <b...@nicira.com> wrote: > On Wed, Feb 13, 2013 at 03:31:43PM -0800, Justin Pettit wrote: >> When a packet arrives on an IP tunnel, store the TOS value for later >> use. This value will be used in a couple of future commits. >> >> Signed-off-by: Justin Pettit <jpet...@nicira.com> > > ... > >> @@ -4922,6 +4928,7 @@ flow_push_stats(struct rule_dpif *rule, >> ofproto_rule_update_used(&rule->up, stats->used); >> >> initial_vals.vlan_tci = flow->vlan_tci; >> + initial_vals.tunnel_ip_tos = 0; >> action_xlate_ctx_init(&ctx, ofproto, flow, &initial_vals, rule, >> 0, NULL); >> ctx.resubmit_stats = stats; > > I think that ideally we should obtain the initial_vals from a subfacet > here. Otherwise I think we could end up doing a translation here that > is different from the initial one. For example, for the initial > translation we might drop the packet due to the ECN bits, and we don't > want to do something different on facet_push_stats(). > > Same comment in facet_learn() except that practically speaking I don't > think it can make a difference there. > > I'm not sure that initial_vals could be different on different > subfacets. Maybe we should move it to the facet.
As I discussed off-line, there may have also been an issue the TCI, too. I re-architected things a bit in my new revision so that both should be handled. >> @@ -6612,6 +6621,7 @@ xlate_actions(struct action_xlate_ctx *ctx, >> uint32_t local_odp_port; >> >> initial_vals.vlan_tci = ctx->base_flow.vlan_tci; >> + initial_vals.tunnel_ip_tos = 0; > > I think that the initializer here should come from > ctx->base_flow.tunnel.ip_tos since that's what we initialized from > initial_vals->tunnel_ip_tos back in action_xlate_ctx_init(). Good point. I'll send out my revised version shortly. --Justin _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
