Looks like Thunderbird messed up my patch when sending it.
Sorry for that - although I hope it shouldn't be a problem for a high-level review.

Best,
Valentine

On 14.10.2016 16:35, Valentine Sinitsyn wrote:
Hi all,

This is a quick attempt to implement sample action at logical port
level.The goal is to export IPFIX flows for logical ports, yet it is
easy to extend this approach to logical switches as well.

Nothing is done to provision OVS instances with required
Flow_Sample_Collector_Set and IPFIX entries at this point.

Does the approach I take looks sensible? If so, I can add tests and
re-send this patch for in-depth review.

Many thanks.

Signed-off-by: Valentine Sinitsyn <valentine.sinit...@gmail.com>

---
 include/ovn/actions.h     |  12 +++++-
 ovn/lib/actions.c         | 102
++++++++++++++++++++++++++++++++++++++++++++++
 ovn/northd/ovn-northd.c   |  77 ++++++++++++++++++++++++++++++++--
 ovn/ovn-nb.ovsschema      |  32 +++++++++++++--
 ovn/ovn-nb.xml            |  36 ++++++++++++++++
 ovn/utilities/ovn-nbctl.c |   5 +++
 ovn/utilities/ovn-trace.c |   4 ++
 7 files changed, 260 insertions(+), 8 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 2905937..7dc4bb9 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -66,7 +66,8 @@ struct simap;
     OVNACT(GET_ND,        ovnact_get_mac_bind)      \
     OVNACT(PUT_ND,        ovnact_put_mac_bind)      \
     OVNACT(PUT_DHCPV4_OPTS, ovnact_put_dhcp_opts)   \
-    OVNACT(PUT_DHCPV6_OPTS, ovnact_put_dhcp_opts)
+    OVNACT(PUT_DHCPV6_OPTS, ovnact_put_dhcp_opts)   \
+    OVNACT(SAMPLE,      ovnact_sample)
  /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */
 enum OVS_PACKED_ENUM ovnact_type {
@@ -219,6 +220,15 @@ struct ovnact_put_dhcp_opts {
     size_t n_options;
 };
 +/* OVNACT_SAMPLE */
+struct ovnact_sample {
+    struct ovnact ovnact;
+    uint32_t collector_set_id;
+    uint32_t probability;
+    uint32_t obs_domain_id;
+    uint32_t obs_point_id;
+};
+
 /* Internal use by the helpers below. */
 void ovnact_init(struct ovnact *, enum ovnact_type, size_t len);
 void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t len);
diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c
index 59131dd..e1debb3 100644
--- a/ovn/lib/actions.c
+++ b/ovn/lib/actions.c
@@ -1614,6 +1614,106 @@ free_PUT_DHCPV6_OPTS(struct ovnact_put_dhcp_opts
*pdo)
 {
     free_put_dhcp_opts(pdo);
 }
+
+static bool
+parse_sample_arg_get_int_helper(struct lexer *lexer, uint32_t *out)
+{
+    if (!lexer_force_match(lexer, LEX_T_EQUALS)) {
+    return false;
+    }
+    if (lexer->token.type == LEX_T_INTEGER) {
+    *out = ntohl(lexer->token.value.be32_int);
+    } else {
+    lexer_syntax_error(lexer, "expecting integer");
+    return false;
+    }
+    lexer_get(lexer);
+    return true;
+}
+
+static void
+parse_sample_args(struct action_context *ctx,
+                  struct ovnact_sample *cc)
+{
+    bool ok;
+
+    if (lexer_match_id(ctx->lexer, "collector_set_id")) {
+    ok = parse_sample_arg_get_int_helper(ctx->lexer,
+                                         &cc->collector_set_id);
+    if (!ok)
+        return;
+    } else if (lexer_match_id(ctx->lexer, "probability")) {
+    ok = parse_sample_arg_get_int_helper(ctx->lexer,
+                                         &cc->probability);
+    if (!ok)
+        return;
+    } else if (lexer_match_id(ctx->lexer, "obs_domain_id")) {
+    ok = parse_sample_arg_get_int_helper(ctx->lexer,
+                                         &cc->obs_domain_id);
+    if (!ok)
+        return;
+    } else if (lexer_match_id(ctx->lexer, "obs_point_id")) {
+    ok = parse_sample_arg_get_int_helper(ctx->lexer,
+                                         &cc->obs_point_id);
+    if (!ok)
+        return;
+    } else {
+        lexer_syntax_error(ctx->lexer, NULL);
+    }
+}
+
+static void
+parse_SAMPLE(struct action_context *ctx)
+{
+    struct ovnact_sample *sample = ovnact_put_SAMPLE(ctx->ovnacts);
+    if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
+        while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) {
+            parse_sample_args(ctx, sample);
+            if (ctx->lexer->error) {
+                return;
+            }
+            lexer_match(ctx->lexer, LEX_T_COMMA);
+        }
+    }
+
+    if (!sample->probability)
+    lexer_error(ctx->lexer, "sample probability can't be zero");
+}
+
+static void
+encode_SAMPLE(const struct ovnact_sample *sample,
+              const struct ovnact_encode_params *ep OVS_UNUSED,
+              struct ofpbuf *ofpacts)
+{
+    struct ofpact_sample *action = ofpact_put_SAMPLE(ofpacts);
+    action->collector_set_id = sample->collector_set_id;
+    /* sampling is 16 bit wide in the northbound database */
+    action->probability = (uint16_t)sample->probability;
+    action->obs_domain_id = sample->obs_domain_id;
+    action->obs_point_id = sample->obs_point_id;
+    action->sampling_port = OFPP_NONE;
+}
+
+static void
+format_SAMPLE(const struct ovnact_sample *sample, struct ds *s)
+{
+    ds_put_format(s,
+              "sample(collector_set_id=%" PRIu32 \
+          ", probability=%" PRIu32 \
+          ", obs_domain_id=%" PRIu32 \
+          ", obs_point_id=%" PRIu32 ")",
+          sample->collector_set_id,
+          sample->probability,
+          sample->obs_domain_id,
+          sample->obs_point_id);
+}
+
+static void
+free_SAMPLE(struct ovnact_sample *sample OVS_UNUSED)
+{
+    /* Nothing to do here */
+}
+
 
 /* Parses an assignment or exchange or put_dhcp_opts action. */
 static void
@@ -1685,6 +1785,8 @@ parse_action(struct action_context *ctx)
         parse_get_mac_bind(ctx, 128, ovnact_put_GET_ND(ctx->ovnacts));
     } else if (lexer_match_id(ctx->lexer, "put_nd")) {
         parse_put_mac_bind(ctx, 128, ovnact_put_PUT_ND(ctx->ovnacts));
+    } else if (lexer_match_id(ctx->lexer, "sample")) {
+    parse_SAMPLE(ctx);
     } else {
         lexer_syntax_error(ctx->lexer, "expecting action");
     }
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index eeeb41d..fd3616e 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -110,6 +110,7 @@ enum ovn_stage {
     PIPELINE_STAGE(SWITCH, IN,  DHCP_OPTIONS,   10,
"ls_in_dhcp_options") \
     PIPELINE_STAGE(SWITCH, IN,  DHCP_RESPONSE,  11,
"ls_in_dhcp_response") \
     PIPELINE_STAGE(SWITCH, IN,  L2_LKUP,        12, "ls_in_l2_lkup")    \
+    PIPELINE_STAGE(SWITCH, IN,  SAMPLE,         13, "ls_in_sample")      \
                                                                       \
     /* Logical switch egress stages. */                               \
     PIPELINE_STAGE(SWITCH, OUT, PRE_LB,       0, "ls_out_pre_lb")     \
@@ -120,6 +121,7 @@ enum ovn_stage {
     PIPELINE_STAGE(SWITCH, OUT, STATEFUL,     5, "ls_out_stateful")    \
     PIPELINE_STAGE(SWITCH, OUT, PORT_SEC_IP,  6, "ls_out_port_sec_ip")
   \
     PIPELINE_STAGE(SWITCH, OUT, PORT_SEC_L2,  7, "ls_out_port_sec_l2")
   \
+    PIPELINE_STAGE(SWITCH, OUT, SAMPLE,       8, "ls_out_sample")      \
                                                                       \
     /* Logical router ingress stages. */                              \
     PIPELINE_STAGE(ROUTER, IN,  ADMISSION,   0, "lr_in_admission")    \
@@ -2591,6 +2593,24 @@ build_stateful(struct ovn_datapath *od, struct
hmap *lflows)
 }
  static void
+format_sample(struct ds *actions, const struct nbrec_ipfix_options
*ipfix_opts)
+{
+    ds_put_format(actions,
+              "sample(collector_set_id=%" PRIu64 ", probability=%" PRIu64,
+          ipfix_opts->collector_set_id, ipfix_opts->sampling);
+
+    if (ipfix_opts->obs_domain_id)
+    ds_put_format(actions,
+              ", obs_domain_id=%" PRIu64, *ipfix_opts->obs_domain_id);
+
+    if (ipfix_opts->obs_point_id)
+    ds_put_format(actions,
+              ", obs_point_id=%" PRIu64, *ipfix_opts->obs_point_id);
+
+    ds_put_cstr(actions, "); ");
+}
+
+static void
 build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
                     struct hmap *lflows, struct hmap *mcgroups)
 {
@@ -2920,11 +2940,12 @@ build_lswitch_flows(struct hmap *datapaths,
struct hmap *ports,
                               ETH_ADDR_ARGS(mac));
                  ds_clear(&actions);
-                ds_put_format(&actions, "outport = %s; output;",
op->json_key);
+                ds_put_format(&actions, "outport = %s; next;",
op->json_key);
                 ovn_lflow_add(lflows, op->od, S_SWITCH_IN_L2_LKUP, 50,
                               ds_cstr(&match), ds_cstr(&actions));
             } else if (!strcmp(op->nbsp->addresses[i], "unknown")) {
                 if (lsp_is_enabled(op->nbsp)) {
+            /* TODO: Consider sampling here as well */
                     ovn_multicast_add(mcgroups, &mc_unknown, op);
                     op->od->has_unknown = true;
                 }
@@ -2939,7 +2960,7 @@ build_lswitch_flows(struct hmap *datapaths, struct
hmap *ports,
                               ETH_ADDR_ARGS(mac));
                  ds_clear(&actions);
-                ds_put_format(&actions, "outport = %s; output;",
op->json_key);
+                ds_put_format(&actions, "outport = %s; next;",
op->json_key);
                 ovn_lflow_add(lflows, op->od, S_SWITCH_IN_L2_LKUP, 50,
                               ds_cstr(&match), ds_cstr(&actions));
             } else {
@@ -2960,8 +2981,32 @@ build_lswitch_flows(struct hmap *datapaths,
struct hmap *ports,
          if (od->has_unknown) {
             ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 0, "1",
-                          "outport = \""MC_UNKNOWN"\"; output;");
+                          "outport = \""MC_UNKNOWN"\"; next;");
+        }
+    }
+
+    /* Ingress table 13: Sampling (priority 50) */
+    HMAP_FOR_EACH (op, key_node, ports) {
+        if (!op->nbsp || !op->nbsp->ipfix_options) {
+            continue;
+        }
+
+    ds_clear(&match);
+    ds_put_format(&match, "inport == %s", op->json_key);
+
+    ds_clear(&actions);
+    format_sample(&actions, op->nbsp->ipfix_options);
+    ds_put_format(&actions, "output;");
+    ovn_lflow_add(lflows, op->od, S_SWITCH_IN_SAMPLE, 50,
+              ds_cstr(&match), ds_cstr(&actions));
+    }
+
+    /* Ingress table 13: Sampling (priority 0) - default output action */
+    HMAP_FOR_EACH (od, key_node, datapaths) {
+        if (!od->nbs) {
+            continue;
         }
+    ovn_lflow_add(lflows, od, S_SWITCH_IN_SAMPLE, 0, "1", "output;");
     }
      /* Egress tables 6: Egress port security - IP (priority 0)
@@ -2997,7 +3042,7 @@ build_lswitch_flows(struct hmap *datapaths, struct
hmap *ports,
             build_port_security_l2("eth.dst", op->ps_addrs,
op->n_ps_addrs,
                                    &match);
             ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_PORT_SEC_L2, 50,
-                          ds_cstr(&match), "output;");
+                          ds_cstr(&match), "next;");
         } else {
             ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_PORT_SEC_L2, 150,
                           ds_cstr(&match), "drop;");
@@ -3008,6 +3053,30 @@ build_lswitch_flows(struct hmap *datapaths,
struct hmap *ports,
         }
     }
 +    /* Egress table 8: Sampling (priority 50) */
+    HMAP_FOR_EACH (op, key_node, ports) {
+        if (!op->nbsp || !op->nbsp->ipfix_options) {
+            continue;
+        }
+
+    ds_clear(&match);
+    ds_put_format(&match, "outport == %s", op->json_key);
+
+    ds_clear(&actions);
+    format_sample(&actions, op->nbsp->ipfix_options);
+    ds_put_format(&actions, "output;");
+    ovn_lflow_add(lflows, op->od, S_SWITCH_OUT_SAMPLE, 50,
+              ds_cstr(&match), ds_cstr(&actions));
+    }
+
+    /* Egress table 8: Sampling (priority 0) - default output action */
+    HMAP_FOR_EACH (od, key_node, datapaths) {
+        if (!od->nbs) {
+            continue;
+        }
+    ovn_lflow_add(lflows, od, S_SWITCH_OUT_SAMPLE, 0, "1", "output;");
+    }
+
     ds_destroy(&match);
     ds_destroy(&actions);
 }
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index b7e70aa..fa95e89 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -1,7 +1,7 @@
 {
     "name": "OVN_Northbound",
-    "version": "5.3.3",
-    "cksum": "2442952958 9945",
+    "version": "5.3.4",
+    "cksum": "859559804 10811",
     "tables": {
         "NB_Global": {
             "columns": {
@@ -79,6 +79,11 @@
                                             "refType": "weak"},
                                  "min": 0,
                                  "max": 1}},
+                "ipfix_options": {"type": {"key": {"type": "uuid",
+                                           "refTable": "IPFIX_Options",
+                                           "refType": "weak"},
+                                 "min": 0,
+                                 "max": 1}},
                 "external_ids": {
                     "type": {"key": "string", "value": "string",
                              "min": 0, "max": "unlimited"}}},
@@ -194,6 +199,27 @@
                 "external_ids": {
                     "type": {"key": "string", "value": "string",
                              "min": 0, "max": "unlimited"}}},
-            "isRoot": true}
+            "isRoot": true},
+    "IPFIX_Options": {
+        "columns": {
+        "collector_set_id": {
+            "type": {"key": {"type": "integer",
+            "minInteger": 0,
+            "maxInteger": 4294967295}}},
+        "sampling": {
+            "type": {"key": {"type": "integer",
+            "minInteger": 1,
+            "maxInteger": 65535}}},
+        "obs_domain_id": {
+            "type": {"key": {"type": "integer",
+            "minInteger": 0,
+            "maxInteger": 4294967295},
+            "min": 0, "max": 1}},
+        "obs_point_id": {
+            "type": {"key": {"type": "integer",
+            "minInteger": 0,
+            "maxInteger": 4294967295},
+            "min": 0, "max": 1}}},
+                "isRoot": true}
     }
 }
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index c45a444..382b0ca 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -629,6 +629,12 @@
         See <em>External IDs</em> at the beginning of this document.
       </column>
     </group>
+
+    <column name="ipfix_options">
+      This column defines the IPFIX options (see <ref
table="IPFIX_Options"/>
+      table) for this port to use. If unset, no IPFIX flows are
genreated for
+      traffic on this logical port.
+    </column>
   </table>
    <table name="Address_Set" title="Address Sets">
@@ -1356,4 +1362,34 @@
       </column>
     </group>
   </table>
+
+  <table name="IPFIX_Options" title="IPFIX Options">
+    <p>
+      This table allows to configure IPFIX flow sample at logical
switch or
+      port level. Currently, OVN doesn't create
Flow_Sample_Collector_Set or
+      IPFIX entries at OVS where physical ports actually reside. This is
+      considered to be a CMS task.
+    </p>
+
+    <column name="collector_set_id">
+      The Flow_Sample_Collector_Set entry identifier on target OVS
instance.
+      This is an integer value between 0 and 2^32-1. Also see the note
above.
+    </column>
+
+    <column name="sampling">
+      The rate at which packets should be sampled and sent to each target
+      collector. Setting it to N means that in average, one of N packets
+      will be sampled.
+    </column>
+
+    <column name="obs_domain_id">
+      The IPFIX Observation Domain ID sent in each IPFIX packet.  If not
+      specified, defaults to 0.
+    </column>
+
+    <column name="obs_point_id">
+      The IPFIX Observation Point ID sent in each IPFIX flow record. If
not
+      specified, defaults to 0.
+    </column>
+  </table>
 </database>
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 2148665..d112614 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -2163,6 +2163,11 @@ static const struct ctl_table_class tables[] = {
        NULL},
       {NULL, NULL, NULL}}},
 +    {&nbrec_table_ipfix_options,
+     {{&nbrec_table_ipfix_options, NULL,
+       NULL},
+      {NULL, NULL, NULL}}},
+
     {NULL, {{NULL, NULL, NULL}, {NULL, NULL, NULL}}}
 };
 
diff --git a/ovn/utilities/ovn-trace.c b/ovn/utilities/ovn-trace.c
index f5607df..f31301c 100644
--- a/ovn/utilities/ovn-trace.c
+++ b/ovn/utilities/ovn-trace.c
@@ -1289,6 +1289,10 @@ trace_actions(const struct ovnact *ovnacts,
size_t ovnacts_len,
         case OVNACT_PUT_DHCPV6_OPTS:
             execute_put_dhcp_opts(ovnact_get_PUT_DHCPV6_OPTS(a), uflow);
             break;
+
+    case OVNACT_SAMPLE:
+        /* Nothing to do for tracing */
+        break;
         }
      }

--
С уважением,
Синицын Валентин
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to