[ovs-dev] [PATCH v2 09/13] ovn-trace: Add some basic tracing for ct_snat and ct_dnat actions.

Ben Pfaff blp at ovn.org
Thu May 4 17:34:40 UTC 2017


On Wed, May 03, 2017 at 06:43:32PM -0700, Mickey Spiegel wrote:
> On Wed, May 3, 2017 at 8:45 AM, Ben Pfaff <blp at ovn.org> wrote:
> 
> > Without this support, ovn-trace is not very useful with OpenStack, which
> > uses connection tracking extensively.
> >
> > Signed-off-by: Ben Pfaff <blp at ovn.org>
> >
> 
> Acked-by: Mickey Spiegel <mickeys.dev at gmail.com>
> 
> A couple of minor comments below.
> 
> ---
> >  ovn/utilities/ovn-trace.8.xml | 50 ++++++++++++++++++++++++++++++
> > +++++++++++
> >  ovn/utilities/ovn-trace.c     | 52 ++++++++++++++++++++++++++++++
> > ++++++++++---
> >  2 files changed, 99 insertions(+), 3 deletions(-)
> >
> > diff --git a/ovn/utilities/ovn-trace.8.xml b/ovn/utilities/ovn-trace.8.xml
> > index 8bb329bfbd71..b2d46ac3d50b 100644
> > --- a/ovn/utilities/ovn-trace.8.xml
> > +++ b/ovn/utilities/ovn-trace.8.xml
> > @@ -166,6 +166,56 @@
> >      output;
> >    </pre>
> >
> > +  <h2>Stateful Actions</h2>
> >
> 
> The flow is a little funny. At the beginning of the output section:
> 
>     <code>ovn-trace</code> supports the three different forms of output,
> each
>     described in a separate section below.
> 
> Now there is another h2 section in between the three sections describing
> the different forms of output.
> 
> Perhaps just use <h3>?

Oops, thanks for pointing out the documentation issues.  I changed it to
<h1> and moved it after the output sections.

> +
> > +  <p>
> > +    Some OVN logical actions use or update state that is not available in
> > the
> > +    southbound database.  <code>ovn-trace</code> handles these actions as
> > +    described below:
> > +  </p>
> > +
> > +  <dl>
> > +    <dt>ct_next</dt>
> > +    <dd>
> > +      By default <code>ovn-trace</code> treats flows as ``tracked'' and
> > +      ``established.''  The <code>--ct</code> option overrides this
> > behavior;
> > +      refer to its description for more information.
> > +    </dd>
> > +
> > +    <dt>ct_commit</dt>
> > +    <dd>
> > +      This action is treated as a no-op.
> > +    </dd>
> > +
> > +    <dt>ct_dnat</dt>
> > +    <dt>ct_snat</dt>
> > +    <dd>
> > +      <p>
> > +        When one of these action is used without arguments, to ``un-NAT''
> > a
> >
> 
> s/action/actions

Thanks.

> 
> > +        packet, <code>ovn-trace</code> assumes that no NAT state was
> > available
> > +        and treats it as a no-op.
> > +      </p>
> > +
> > +      <p>
> > +        With an argument, <code>ovn-trace</code> sets the IP destination
> > or
> > +        source, as appropriate, to the given address. It also sets
> > +        <code>ct.dnat</code> or <code>ct.snat</code> to 1 to indicate
> > that NAT
> > +        has taken place.
> > +      </p>
> > +    </dd>
> > +
> > +    <dt>ct_lb</dt>
> > +    <dd>
> > +      Not yet implemented; currently implemented as a no-op.
> > +    </dd>
> > +
> > +    <dt>put_arp</dt>
> > +    <dt>put_nd</dt>
> > +    <dd>
> > +      This action is treated as a no-op.
> > +    </dd>
> > +  </dl>
> > +
> >    <h2>Summary Output</h2>
> >
> >    <p>
> > diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
> > index 3a0780eb931e..860fd4b26be0 100644
> > --- a/ovn/utilities/ovn-trace.c
> > +++ b/ovn/utilities/ovn-trace.c
> > @@ -346,6 +346,8 @@ struct ovntrace_datapath {
> >      size_t n_flows, allocated_flows;
> >
> >      struct hmap mac_bindings;   /* Contains "struct
> > ovntrace_mac_binding"s. */
> > +
> > +    bool has_local_l3gateway;
> >  };
> >
> >  struct ovntrace_port {
> > @@ -570,6 +572,9 @@ read_ports(void)
> >                      port->peer->peer = port;
> >                  }
> >              }
> > +        } else if (!strcmp(sbpb->type, "l3gateway")) {
> > +            /* Treat all gateways as local for our purposes. */
> > +            dp->has_local_l3gateway = true;
> >          }
> >      }
> >
> > @@ -1522,6 +1527,46 @@ execute_ct_next(const struct ovnact_ct_next
> > *ct_next,
> >  }
> >
> >  static void
> > +execute_ct_nat(const struct ovnact_ct_nat *ct_nat,
> > +               const struct ovntrace_datapath *dp, struct flow *uflow,
> > +               enum ovnact_pipeline pipeline, struct ovs_list *super)
> > +{
> > +    bool is_dst = ct_nat->ovnact.type == OVNACT_CT_DNAT;
> > +    if (!is_dst && dp->has_local_l3gateway && !ct_nat->ip) {
> > +        /* "ct_snat;" has no visible effect in a gateway router. */
> > +        return;
> > +    }
> > +    const char *direction = is_dst ? "dst" : "src";
> > +
> > +    /* Make a sub-node for attaching the next table,
> > +     * and figure out the changes if any. */
> > +    struct flow ct_flow = *uflow;
> > +    struct ds s = DS_EMPTY_INITIALIZER;
> > +    ds_put_format(&s, "ct_%cnat", direction[0]);
> > +    if (ct_nat->ip) {
> > +        ds_put_format(&s, "(ip4.%s="IP_FMT")", direction,
> > IP_ARGS(ct_nat->ip));
> > +        ovs_be32 *ip = is_dst ? &ct_flow.nw_dst : &ct_flow.nw_src;
> > +        *ip = ct_nat->ip;
> > +
> > +        uint8_t state = is_dst ? CS_DST_NAT : CS_SRC_NAT;
> > +        ct_flow.ct_state |= state;
> > +    } else {
> > +        ds_put_format(&s, " /* assuming no un-%cnat entry, so no change
> > */",
> > +                      direction[0]);
> > +    }
> > +    struct ovntrace_node *node = ovntrace_node_append(
> > +        super, OVNTRACE_NODE_TRANSFORMATION, "%s", ds_cstr(&s));
> > +    ds_destroy(&s);
> > +
> > +    /* Trace the actions in the next table. */
> > +    trace__(dp, &ct_flow, ct_nat->ltable, pipeline, &node->subs);
> > +
> > +    /* Upon return, we will trace the actions following the ct action in
> > the
> > +     * original table.  The pipeline forked, so we're using the original
> > +     * flow, not ct_flow. */
> > +}
> > +
> > +static void
> >  trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len,
> >                const struct ovntrace_datapath *dp, struct flow *uflow,
> >                uint8_t table_id, enum ovnact_pipeline pipeline,
> > @@ -1585,10 +1630,11 @@ trace_actions(const struct ovnact *ovnacts, size_t
> > ovnacts_len,
> >              break;
> >
> >          case OVNACT_CT_DNAT:
> > +            execute_ct_nat(ovnact_get_CT_DNAT(a), dp, uflow, pipeline,
> > super);
> > +            break;
> > +
> >          case OVNACT_CT_SNAT:
> > -            ovntrace_node_append(super, OVNTRACE_NODE_ERROR,
> > -                                 "*** ct_dnat and ct_snat actions "
> > -                                 "not implemented");
> > +            execute_ct_nat(ovnact_get_CT_SNAT(a), dp, uflow, pipeline,
> > super);
> >              break;
> >
> >          case OVNACT_CT_LB:
> > --
> > 2.10.2
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >


More information about the dev mailing list