That was surprisingly difficult to read :-) Acked-by: Jarno Rajahalme <[email protected]>
> On Jul 29, 2015, at 11:42 PM, Ben Pfaff <[email protected]> wrote: > > It seems to me that tnl_xlate_init() has two almost-separate tasks. First, > it marks most of the 'wc' bits for tunnels. Second, it checks and updates > ECN bits. This commit breaks tnl_xlate_init() into two separate functions, > one for each of those tasks. > > Signed-off-by: Ben Pfaff <[email protected]> > --- > ofproto/ofproto-dpif-xlate.c | 3 +- > ofproto/tunnel.c | 85 ++++++++++++++++++++------------------------ > ofproto/tunnel.h | 3 +- > 3 files changed, 42 insertions(+), 49 deletions(-) > > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index e69605e..8acb908 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -4824,9 +4824,10 @@ xlate_actions(struct xlate_in *xin, struct xlate_out > *xout) > if (xbridge->netflow) { > netflow_mask_wc(flow, ctx.wc); > } > + tnl_wc_init(flow, xin->wc); > } > > - tnl_may_send = tnl_xlate_init(flow, xin->wc); > + tnl_may_send = tnl_process_ecn(flow); > > /* The in_port of the original packet before recirculation. */ > in_port = get_ofp_port(xbridge, flow->in_port.ofp_port); > diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c > index 372b385..a7df772 100644 > --- a/ofproto/tunnel.c > +++ b/ofproto/tunnel.c > @@ -332,67 +332,58 @@ out: > return ofport; > } > > -static bool > -tnl_ecn_ok(struct flow *flow, struct flow_wildcards *wc) > -{ > - if (is_ip_any(flow)) { > - if ((flow->tunnel.ip_tos & IP_ECN_MASK) == IP_ECN_CE) { > - if (wc) { > - wc->masks.nw_tos |= IP_ECN_MASK; > - } > - if ((flow->nw_tos & IP_ECN_MASK) == IP_ECN_NOT_ECT) { > - VLOG_WARN_RL(&rl, "dropping tunnel packet marked ECN CE" > - " but is not ECN capable"); > - return false; > - } else { > - /* Set the ECN CE value in the tunneled packet. */ > - flow->nw_tos |= IP_ECN_CE; > - } > - } > - } > - > - return true; > -} > - > /* Should be called at the beginning of action translation to initialize > * wildcards and perform any actions based on receiving on tunnel port. > * > * Returns false if the packet must be dropped. */ > bool > -tnl_xlate_init(struct flow *flow, struct flow_wildcards *wc) > +tnl_process_ecn(struct flow *flow) > { > - /* tnl_port_should_receive() examines the 'tunnel.ip_dst' field to > - * determine the presence of the tunnel metadata. However, since > tunnels' > - * datapath port numbers are different from the non-tunnel ports, and we > - * always unwildcard the 'in_port', we do not need to unwildcard > - * the 'tunnel.ip_dst' for non-tunneled packets. */ > - if (tnl_port_should_receive(flow)) { > - if (wc) { > - wc->masks.tunnel.tun_id = OVS_BE64_MAX; > - wc->masks.tunnel.ip_src = OVS_BE32_MAX; > - wc->masks.tunnel.ip_dst = OVS_BE32_MAX; > - wc->masks.tunnel.flags = (FLOW_TNL_F_DONT_FRAGMENT | > - FLOW_TNL_F_CSUM | > - FLOW_TNL_F_KEY); > - wc->masks.tunnel.ip_tos = UINT8_MAX; > - wc->masks.tunnel.ip_ttl = UINT8_MAX; > - /* The tp_src and tp_dst members in flow_tnl are set to be always > - * wildcarded, not to unwildcard them here. */ > - wc->masks.tunnel.tp_src = 0; > - wc->masks.tunnel.tp_dst = 0; > - > - memset(&wc->masks.pkt_mark, 0xff, sizeof wc->masks.pkt_mark); > - } > - if (!tnl_ecn_ok(flow, wc)) { > + if (!tnl_port_should_receive(flow)) { > + return true; > + } > + > + if (is_ip_any(flow) && (flow->tunnel.ip_tos & IP_ECN_MASK) == IP_ECN_CE) > { > + if ((flow->nw_tos & IP_ECN_MASK) == IP_ECN_NOT_ECT) { > + VLOG_WARN_RL(&rl, "dropping tunnel packet marked ECN CE" > + " but is not ECN capable"); > return false; > } > > - flow->pkt_mark &= ~IPSEC_MARK; > + /* Set the ECN CE value in the tunneled packet. */ > + flow->nw_tos |= IP_ECN_CE; > } > > + flow->pkt_mark &= ~IPSEC_MARK; > return true; > } > > +void > +tnl_wc_init(struct flow *flow, struct flow_wildcards *wc) > +{ > + if (tnl_port_should_receive(flow)) { > + wc->masks.tunnel.tun_id = OVS_BE64_MAX; > + wc->masks.tunnel.ip_src = OVS_BE32_MAX; > + wc->masks.tunnel.ip_dst = OVS_BE32_MAX; > + wc->masks.tunnel.flags = (FLOW_TNL_F_DONT_FRAGMENT | > + FLOW_TNL_F_CSUM | > + FLOW_TNL_F_KEY); > + wc->masks.tunnel.ip_tos = UINT8_MAX; > + wc->masks.tunnel.ip_ttl = UINT8_MAX; > + /* The tp_src and tp_dst members in flow_tnl are set to be always > + * wildcarded, not to unwildcard them here. */ > + wc->masks.tunnel.tp_src = 0; > + wc->masks.tunnel.tp_dst = 0; > + > + memset(&wc->masks.pkt_mark, 0xff, sizeof wc->masks.pkt_mark); > + > + if (is_ip_any(flow) > + && (flow->tunnel.ip_tos & IP_ECN_MASK) == IP_ECN_CE) { > + wc->masks.nw_tos |= IP_ECN_MASK; > + } > + } > +} > + > /* Given that 'flow' should be output to the ofport corresponding to > * 'tnl_port', updates 'flow''s tunnel headers and returns the actual datapath > * port that the output should happen on. May return ODPP_NONE if the output > diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h > index 8f9ddab..4a28df9 100644 > --- a/ofproto/tunnel.h > +++ b/ofproto/tunnel.h > @@ -38,7 +38,8 @@ int tnl_port_add(const struct ofport_dpif *, const struct > netdev *, > void tnl_port_del(const struct ofport_dpif *); > > const struct ofport_dpif *tnl_port_receive(const struct flow *); > -bool tnl_xlate_init(struct flow *, struct flow_wildcards *); > +void tnl_wc_init(struct flow *, struct flow_wildcards *); > +bool tnl_process_ecn(struct flow *); > odp_port_t tnl_port_send(const struct ofport_dpif *, struct flow *, > struct flow_wildcards *wc); > > -- > 2.1.3 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
