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 <b...@nicira.com> --- 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 dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev