[ovs-dev] [PATCH 2/3] ofproto/trace: Add support for tracing conntrack recirculation

Darrell Ball dball at vmware.com
Wed Jun 21 18:58:55 UTC 2017



On 6/21/17, 11:06 AM, "ovs-dev-bounces at openvswitch.org on behalf of Yi-Hung Wei" <ovs-dev-bounces at openvswitch.org on behalf of yihung.wei at gmail.com> wrote:

    Previously, a user need to run ofproto/trace multiple times to derive the
    final datapath actions if a flow hit conntrack actions that involves
    recirculation. To improve the usability of ofproto/trace, in this patch,
    we keep track of the conntrack actions, and automatically run the
    recirculation process so that a user only need to execute the ofproto/trace
    command once. Currently, this patch sets the default ct_state as
    trk and est in the automatic recirculation process.

Why is ‘est’ part of the default ?

 A following patch
    will provide an option to customize ct_state.
    
    Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
    ---
     ofproto/ofproto-dpif-trace.c | 88 +++++++++++++++++++++++++++++++++++++-------
     ofproto/ofproto-dpif-trace.h | 22 +++++++++++
     ofproto/ofproto-dpif-xlate.c | 24 ++++++++++--
     ofproto/ofproto-dpif-xlate.h |  4 ++
     tests/ofproto-dpif.at        | 35 ++++++++++++++++++
     5 files changed, 156 insertions(+), 17 deletions(-)
    
    diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
    index c7f0e48caa16..d8cdd92493cf 100644
    --- a/ofproto/ofproto-dpif-trace.c
    +++ b/ofproto/ofproto-dpif-trace.c
    @@ -86,6 +86,39 @@ oftrace_node_destroy(struct oftrace_node *node)
         }
     }
     
    +void
    +oftrace_add_recirc_node(struct ovs_list *recirc_queue,
    +                        enum oftrace_recirc_type type, struct flow *flow,
    +                        uint32_t recirc_id)
    +{
    +    struct oftrace_recirc_node *node = xmalloc(sizeof *node);
    +    ovs_list_push_back(recirc_queue, &node->node);
    +
    +    node->type = type;
    +    node->recirc_id = recirc_id;
    +    node->flow = xmalloc(sizeof *flow);
    +    memcpy(node->flow, flow, sizeof *flow);
    +    node->flow->recirc_id = recirc_id;
    +
    +    if (type != OFT_INIT) {
    +        struct recirc_id_node *rid_node = CONST_CAST(
    +            struct recirc_id_node *, recirc_id_node_find(recirc_id));
    +        if (rid_node) {
    +            ovs_refcount_try_ref_rcu(&rid_node->refcount);
    +        }
    +    }
    +}
    +
    +static void
    +oftrace_recirc_node_destroy(struct oftrace_recirc_node *node)
    +{
    +    if (node->type != OFT_INIT) {
    +        recirc_free_id(node->recirc_id);
    +    }
    +    free(node->flow);
    +    free(node);
    +}
    +
     static void
     oftrace_node_print_details(struct ds *output,
                                const struct ovs_list *nodes, int level)
    @@ -419,20 +452,11 @@ exit:
         ofpbuf_uninit(&ofpacts);
     }
     
    -/* Implements a "trace" through 'ofproto''s flow table, appending a textual
    - * description of the results to 'output'.
    - *
    - * The trace follows a packet with the specified 'flow' through the flow
    - * table.  'packet' may be nonnull to trace an actual packet, with consequent
    - * side effects (if it is nonnull then its flow must be 'flow').
    - *
    - * If 'ofpacts' is nonnull then its 'ofpacts_len' bytes specify the actions to
    - * trace, otherwise the actions are determined by a flow table lookup. */
     static void
    -ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
    -              const struct dp_packet *packet,
    -              const struct ofpact ofpacts[], size_t ofpacts_len,
    -              struct ds *output)
    +ofproto_trace__(struct ofproto_dpif *ofproto, struct flow *flow,
    +                const struct dp_packet *packet, struct ovs_list *recirc_queue,
    +                const struct ofpact ofpacts[], size_t ofpacts_len,
    +                struct ds *output)
     {
         struct ofpbuf odp_actions;
         ofpbuf_init(&odp_actions, 0);
    @@ -447,6 +471,7 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
         xin.ofpacts = ofpacts;
         xin.ofpacts_len = ofpacts_len;
         xin.trace = &trace;
    +    xin.recirc_queue = recirc_queue;
     
         /* Copy initial flow out of xin.flow.  It differs from '*flow' because
          * xlate_in_init() initializes actset_output to OFPP_UNSET. */
    @@ -503,6 +528,43 @@ ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
         oftrace_node_list_destroy(&trace);
     }
     
    +/* Implements a "trace" through 'ofproto''s flow table, appending a textual
    + * description of the results to 'output'.
    + *
    + * The trace follows a packet with the specified 'flow' through the flow
    + * table.  'packet' may be nonnull to trace an actual packet, with consequent
    + * side effects (if it is nonnull then its flow must be 'flow').
    + *
    + * If 'ofpacts' is nonnull then its 'ofpacts_len' bytes specify the actions to
    + * trace, otherwise the actions are determined by a flow table lookup. */
    +static void
    +ofproto_trace(struct ofproto_dpif *ofproto, struct flow *flow,
    +              const struct dp_packet *packet,
    +              const struct ofpact ofpacts[], size_t ofpacts_len,
    +              struct ds *output)
    +{
    +    struct ovs_list recirc_queue = OVS_LIST_INITIALIZER(&recirc_queue);
    +    oftrace_add_recirc_node(&recirc_queue, OFT_INIT, flow, flow->recirc_id);
    +
    +    while (!ovs_list_is_empty(&recirc_queue)) {
    +        struct ovs_list *node = ovs_list_pop_front(&recirc_queue);
    +        struct oftrace_recirc_node *recirc_node;
    +
    +        ASSIGN_CONTAINER(recirc_node, node, node);
    +
    +        if (recirc_node->type == OFT_CONNTRACK) {
    +            recirc_node->flow->ct_state = CS_ESTABLISHED | CS_TRACKED;
    +            ds_put_cstr(output, "\n\nResume conntrack prcessing with "
    +                                "default ct_state=trk|est.\n");
    +            ds_put_char_multiple(output, '-', 79);
    +            ds_put_char(output, '\n');
    +        }
    +        ofproto_trace__(ofproto, recirc_node->flow, packet, &recirc_queue,
    +                        ofpacts, ofpacts_len, output);
    +        oftrace_recirc_node_destroy(recirc_node);
    +    }
    +}
    +
     void
     ofproto_dpif_trace_init(void)
     {
    diff --git a/ofproto/ofproto-dpif-trace.h b/ofproto/ofproto-dpif-trace.h
    index e3d3b393cec8..f7299fd42848 100644
    --- a/ofproto/ofproto-dpif-trace.h
    +++ b/ofproto/ofproto-dpif-trace.h
    @@ -45,6 +45,16 @@ enum oftrace_node_type {
         OFT_ERROR,                  /* An erroneous situation, worth logging. */
     };
     
    +/* Type of a node within a recirculation queue. */
    +enum oftrace_recirc_type {
    +    /* The initial flow to be traced. */
    +    OFT_INIT,
    +    /* A flow recirculates because of the following actions. */
    +    OFT_CONNTRACK,
    +    OFT_MPLS,
    +    OFT_BOND,
    +};
    +
     /* A node within a trace. */
     struct oftrace_node {
         struct ovs_list node;       /* In parent. */
    @@ -54,9 +64,21 @@ struct oftrace_node {
         char *text;
     };
     
    +/* A node within a recirculation queue. */
    +struct oftrace_recirc_node {
    +    struct ovs_list node;       /* In recirc_queue. */
    +
    +    enum oftrace_recirc_type type;
    +    uint32_t recirc_id;
    +    struct flow *flow;
    +};
    +
     void ofproto_dpif_trace_init(void);
     
     struct oftrace_node *oftrace_report(struct ovs_list *, enum oftrace_node_type,
                                         const char *text);
    +void oftrace_add_recirc_node(struct ovs_list *recirc_queue,
    +                             enum oftrace_recirc_type, struct flow *,
    +                             uint32_t recirc_id);
     
     #endif /* ofproto-dpif-trace.h */
    diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
    index 48c4bad4ac0b..dbe10e4a1b37 100644
    --- a/ofproto/ofproto-dpif-xlate.c
    +++ b/ofproto/ofproto-dpif-xlate.c
    @@ -4356,9 +4356,14 @@ emit_continuation(struct xlate_ctx *ctx, const struct frozen_state *state)
         }
     }
     
    -static void
    +/* Create a forzen state, and allocate a unique recirc id for the given state.
    + * Returns a non-zero value if recirc id is allocated succesfully. Returns 0
    + * otherwise.
    + **/
    +static uint32_t
     finish_freezing__(struct xlate_ctx *ctx, uint8_t table)
     {
    +    uint32_t id = 0;
         ovs_assert(ctx->freezing);
     
         struct frozen_state state = {
    @@ -4385,11 +4390,11 @@ finish_freezing__(struct xlate_ctx *ctx, uint8_t table)
              * recirculation context, will be returned if possible.
              * The life-cycle of this recirc id is managed by associating it
              * with the udpif key ('ukey') created for each new datapath flow. */
    -        uint32_t id = recirc_alloc_id_ctx(&state);
    +        id = recirc_alloc_id_ctx(&state);
             if (!id) {
                 xlate_report_error(ctx, "Failed to allocate recirculation id");
                 ctx->error = XLATE_NO_RECIRCULATION_CONTEXT;
    -            return;
    +            return 0;
             }
             recirc_refs_add(&ctx->xout->recircs, id);
     
    @@ -4408,6 +4413,7 @@ finish_freezing__(struct xlate_ctx *ctx, uint8_t table)
     
         /* Undo changes done by freezing. */
         ctx_cancel_freeze(ctx);
    +    return id;
     }
     
     /* Called only when we're freezing. */
    @@ -4425,8 +4431,17 @@ finish_freezing(struct xlate_ctx *ctx)
     static void
     compose_recirculate_and_fork(struct xlate_ctx *ctx, uint8_t table)
     {
    +    uint32_t recirc_id;
         ctx->freezing = true;
    -    finish_freezing__(ctx, table);
    +    recirc_id = finish_freezing__(ctx, table);
    +
    +    if (OVS_UNLIKELY(ctx->xin->trace) && recirc_id) {
    +        xlate_report(ctx, OFT_DETAIL, "A clone of the packet is forked to "
    +                     "recirculation. The forked pipeline will be resumed at "
    +                     "table %u.", table);
    +       oftrace_add_recirc_node(ctx->xin->recirc_queue, OFT_CONNTRACK,
    +                               &ctx->xin->flow, recirc_id);
    +    }

Was there a reason this is not done in finish_freezing__() itself ?

     }
     
     static void
    @@ -5970,6 +5985,7 @@ xlate_in_init(struct xlate_in *xin, struct ofproto_dpif *ofproto,
         xin->wc = wc;
         xin->odp_actions = odp_actions;
         xin->in_packet_out = false;
    +    xin->recirc_queue = NULL;
     
         /* Do recirc lookup. */
         xin->frozen_state = NULL;
    diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
    index 68e114afb9ae..a86d5c6c9862 100644
    --- a/ofproto/ofproto-dpif-xlate.h
    +++ b/ofproto/ofproto-dpif-xlate.h
    @@ -158,6 +158,10 @@ struct xlate_in {
     
         /* If true, the packet to be translated is from a packet_out msg. */
         bool in_packet_out;
    +
    +    /* ofproto/trace maintains this queue to trace flows that requires
    +     * recirculation. */
    +    struct ovs_list *recirc_queue;
     };
     
     void xlate_ofproto_set(struct ofproto_dpif *, const char *name, struct dpif *,
    diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
    index e2228661fd09..c51c689b9f46 100644
    --- a/tests/ofproto-dpif.at
    +++ b/tests/ofproto-dpif.at
    @@ -9710,6 +9710,41 @@ udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:0a,dl_dst=50:54:00:00:00:09,nw_src=10.
     OVS_VSWITCHD_STOP
     AT_CLEANUP
     
    +AT_SETUP([ofproto-dpif - conntrack - ofproto/trace])
    +OVS_VSWITCHD_START
    +
    +add_of_ports br0 1 2
    +
    +AT_DATA([flows.txt], [dnl
    +dnl Table 0
    +dnl
    +table=0,priority=100,arp,action=normal
    +table=0,priority=10,udp,action=ct(table=1,zone=0)
    +table=0,priority=1,action=drop
    +dnl
    +dnl Table 1
    +dnl
    +table=1,priority=10,in_port=1,ct_state=+trk+new,udp,action=ct(commit,zone=0),2
    +table=1,priority=10,in_port=1,ct_state=+trk+est,udp,action=2
    +table=1,priority=10,in_port=2,ct_state=+trk+est,udp,action=1
    +table=1,priority=1,action=drop
    +])
    +
    +AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
    +
    +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=2,udp'], [0], [stdout])
    +AT_CHECK([tail -1 stdout], [0],
    +  [Datapath actions: 1
    +])
    +
    +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,udp'], [0], [stdout])
    +AT_CHECK([tail -1 stdout], [0],
    +  [Datapath actions: 2
    +])
    +
    +OVS_VSWITCHD_STOP
    +AT_CLEANUP
    +
     AT_SETUP([ofproto - set mtu])
     OVS_VSWITCHD_START
     
    -- 
    2.7.4
    
    _______________________________________________
    dev mailing list
    dev at openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=QOZanmWX0YHWiBy9aCXt-fJrmiZ8cArSz7hCi1ycHJ0&s=rrSI6U4YywzDuey9wOT0zollHWtE25fMnp3wfjL_GCE&e= 
    



More information about the dev mailing list