[ovs-dev] [PATCH 13/26] tunnel: Break tnl_xlate_init() into two separate functions.

Jarno Rajahalme jrajahalme at nicira.com
Fri Jul 31 21:15:07 UTC 2015


That was surprisingly difficult to read :-)

Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>

> On Jul 29, 2015, at 11:42 PM, Ben Pfaff <blp at nicira.com> 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 <blp at 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list