ofp-actions: Add "ingress" and "egress" options to "sample" action.

Before Open vSwitch 2.5.90, IPFIX reports from Open vSwitch didn't include
whether the packet was ingressing or egressing the switch.  Starting in
OVS 2.5.90, this information was available but only accurate if the action
included a port number that indicated a tunnel.  Conflating these two does
not always make sense (not every packet involves a tunnel!), so this patch
makes it possible for the sample action to simply say whether it's for
ingress or egress.

This is difficult to test, since the "tests" directory of OVS does not have
a proper IPFIX listener.  This passes those tests, plus a couple that just
verify that the actions are properly parsed and formatted.  Benli did test
it end-to-end in a VMware use case.

Requested-by: Benli Ye <[email protected]>
Tested-by: Benli Ye <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
Acked-by: Simon Horman <[email protected]>

NEWS

@@ -20,6 +20,7 @@ Post-v2.6.0
        control over the OpenFlow select groups selection method.  See
        "selection_method" and related options in ovs-ofctl(8) for
        details.
+     * The "sample" action now supports "ingress" and "egress" options.
    - ovs-ofctl:
      * 'bundle' command now supports packet-out messages.
      * New syntax for 'ovs-ofctl packet-out' command, which uses the

include/openvswitch/ofp-actions.h

@@ -841,16 +841,31 @@ struct ofpact_note {
     uint8_t data[];
 };
 
+/* Direction of sampled packets. */
+enum nx_action_sample_direction {
+    /* OVS will attempt to infer the sample's direction based on whether
+     * 'sampling_port' is the packet's output port.  This is generally
+     * effective except when sampling happens as part of an output to a patch
+     * port, which doesn't involve a datapath output action. */
+    NX_ACTION_SAMPLE_DEFAULT,
+
+    /* Explicit direction.  This is useful for sampling packets coming in from
+     * or going out of a patch port, where the direction cannot be inferred. */
+    NX_ACTION_SAMPLE_INGRESS,
+    NX_ACTION_SAMPLE_EGRESS
+};
+
 /* OFPACT_SAMPLE.
  *
- * Used for NXAST_SAMPLE and NXAST_SAMPLE2. */
+ * Used for NXAST_SAMPLE, NXAST_SAMPLE2, and NXAST_SAMPLE3. */
 struct ofpact_sample {
     struct ofpact ofpact;
     uint16_t probability;  /* Always positive. */
     uint32_t collector_set_id;
     uint32_t obs_domain_id;
     uint32_t obs_point_id;
     ofp_port_t sampling_port;
+    enum nx_action_sample_direction direction;
 };
 
 /* OFPACT_DEC_TTL.

lib/odp-util.c

@@ -313,12 +313,18 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr)
                               ",collector_set_id=%"PRIu32
                               ",obs_domain_id=%"PRIu32
                               ",obs_point_id=%"PRIu32
-                              ",output_port=%"PRIu32")",
+                              ",output_port=%"PRIu32,
                               cookie.flow_sample.probability,
                               cookie.flow_sample.collector_set_id,
                               cookie.flow_sample.obs_domain_id,
                               cookie.flow_sample.obs_point_id,
                               cookie.flow_sample.output_odp_port);
+                if (cookie.flow_sample.direction == NX_ACTION_SAMPLE_INGRESS) {
+                    ds_put_cstr(ds, ",ingress");
+                } else if (cookie.flow_sample.direction == NX_ACTION_SAMPLE_EGRESS) {
+                    ds_put_cstr(ds, ",egress");
+                }
+                ds_put_char(ds, ')');
             } else if (userdata_len >= sizeof cookie.ipfix
                        && cookie.type == USER_ACTION_COOKIE_IPFIX) {
                 ds_put_format(ds, ",ipfix(output_port=%"PRIu32")",
@@ -963,7 +969,7 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
                             "collector_set_id=%"SCNi32","
                             "obs_domain_id=%"SCNi32","
                             "obs_point_id=%"SCNi32","
-                            "output_port=%"SCNi32")%n",
+                            "output_port=%"SCNi32"%n",
                             &probability, &collector_set_id,
                             &obs_domain_id, &obs_point_id,
                             &output, &n1)) {
@@ -977,6 +983,21 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
             cookie.flow_sample.output_odp_port = u32_to_odp(output);
             user_data = &cookie;
             user_data_size = sizeof cookie.flow_sample;
+
+            if (ovs_scan(&s[n], ",ingress%n", &n1)) {
+                cookie.flow_sample.direction = NX_ACTION_SAMPLE_INGRESS;
+                n += n1;
+            } else if (ovs_scan(&s[n], ",egress%n", &n1)) {
+                cookie.flow_sample.direction = NX_ACTION_SAMPLE_EGRESS;
+                n += n1;
+            } else {
+                cookie.flow_sample.direction = NX_ACTION_SAMPLE_DEFAULT;
+            }
+            if (s[n] != ')') {
+                res = -EINVAL;
+                goto out;
+            }
+            n++;
         } else if (ovs_scan(&s[n], ",ipfix(output_port=%"SCNi32")%n",
                             &output, &n1) ) {
             n += n1;

lib/odp-util.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@
 #include "flow.h"
 #include "hash.h"
 #include "openvswitch/hmap.h"
+#include "openvswitch/ofp-actions.h"
 #include "odp-netlink.h"
 #include "openflow/openflow.h"
 #include "util.h"
@@ -287,14 +288,15 @@ union user_action_cookie {
         uint32_t obs_domain_id; /* Observation Domain ID. */
         uint32_t obs_point_id;  /* Observation Point ID. */
         odp_port_t output_odp_port; /* The output odp port. */
+        enum nx_action_sample_direction direction;
     } flow_sample;
 
     struct {
         uint16_t   type;            /* USER_ACTION_COOKIE_IPFIX. */
         odp_port_t output_odp_port; /* The output odp port. */
     } ipfix;
 };
-BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 20);
+BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 24);
 
 size_t odp_put_userspace_action(uint32_t pid,
                                 const void *userdata, size_t userdata_size,

lib/ofp-actions.c

@@ -291,6 +291,8 @@ enum ofp_raw_action_type {
     NXAST_RAW_SAMPLE,
     /* NX1.0+(38): struct nx_action_sample2. */
     NXAST_RAW_SAMPLE2,
+    /* NX1.0+(41): struct nx_action_sample2. */
+    NXAST_RAW_SAMPLE3,
 
     /* NX1.0+(34): struct nx_action_conjunction. */
     NXAST_RAW_CONJUNCTION,
@@ -4824,10 +4826,13 @@ struct nx_action_sample {
 };
 OFP_ASSERT(sizeof(struct nx_action_sample) == 24);
 
-/* Action structure for NXAST_SAMPLE2.
+/* Action structure for NXAST_SAMPLE2 and NXAST_SAMPLE3.
  *
- * This replacement for NXAST_SAMPLE makes it support exporting
- * egress tunnel information. */
+ * NXAST_SAMPLE2 was added in Open vSwitch 2.5.90.  Compared to NXAST_SAMPLE,
+ * it adds support for exporting egress tunnel information.
+ *
+ * NXAST_SAMPLE3 was added in Open vSwitch 2.6.90.  Compared to NXAST_SAMPLE2,
+ * it adds support for the 'direction' field. */
 struct nx_action_sample2 {
     ovs_be16 type;                  /* OFPAT_VENDOR. */
     ovs_be16 len;                   /* Length is 32. */
@@ -4838,7 +4843,8 @@ struct nx_action_sample2 {
     ovs_be32 obs_domain_id;         /* ID of sampling observation domain. */
     ovs_be32 obs_point_id;          /* ID of sampling observation point. */
     ovs_be16 sampling_port;         /* Sampling port. */
-    uint8_t  pad[6];                /* Pad to a multiple of 8 bytes */
+    uint8_t  direction;             /* NXAST_SAMPLE3 only. */
+    uint8_t  zeros[5];              /* Pad to a multiple of 8 bytes */
  };
  OFP_ASSERT(sizeof(struct nx_action_sample2) == 32);
 
@@ -4855,8 +4861,8 @@ decode_NXAST_RAW_SAMPLE(const struct nx_action_sample *nas,
     sample->collector_set_id = ntohl(nas->collector_set_id);
     sample->obs_domain_id = ntohl(nas->obs_domain_id);
     sample->obs_point_id = ntohl(nas->obs_point_id);
-    /* Default value for sampling port is OFPP_NONE */
     sample->sampling_port = OFPP_NONE;
+    sample->direction = NX_ACTION_SAMPLE_DEFAULT;
 
     if (sample->probability == 0) {
         return OFPERR_OFPBAC_BAD_ARGUMENT;
@@ -4866,19 +4872,18 @@ decode_NXAST_RAW_SAMPLE(const struct nx_action_sample *nas,
 }
 
 static enum ofperr
-decode_NXAST_RAW_SAMPLE2(const struct nx_action_sample2 *nas,
-                         enum ofp_version ofp_version OVS_UNUSED,
-                         struct ofpbuf *out)
+decode_SAMPLE2(const struct nx_action_sample2 *nas,
+               enum ofp_raw_action_type raw,
+               enum nx_action_sample_direction direction,
+               struct ofpact_sample *sample)
 {
-    struct ofpact_sample *sample;
-
-    sample = ofpact_put_SAMPLE(out);
-    sample->ofpact.raw = NXAST_RAW_SAMPLE2;
+    sample->ofpact.raw = raw;
     sample->probability = ntohs(nas->probability);
     sample->collector_set_id = ntohl(nas->collector_set_id);
     sample->obs_domain_id = ntohl(nas->obs_domain_id);
     sample->obs_point_id = ntohl(nas->obs_point_id);
     sample->sampling_port = u16_to_ofp(ntohs(nas->sampling_port));
+    sample->direction = direction;
 
     if (sample->probability == 0) {
         return OFPERR_OFPBAC_BAD_ARGUMENT;
@@ -4887,18 +4892,55 @@ decode_NXAST_RAW_SAMPLE2(const struct nx_action_sample2 *nas,
     return 0;
 }
 
+static enum ofperr
+decode_NXAST_RAW_SAMPLE2(const struct nx_action_sample2 *nas,
+                         enum ofp_version ofp_version OVS_UNUSED,
+                         struct ofpbuf *out)
+{
+    return decode_SAMPLE2(nas, NXAST_RAW_SAMPLE2, NX_ACTION_SAMPLE_DEFAULT,
+                          ofpact_put_SAMPLE(out));
+}
+
+static enum ofperr
+decode_NXAST_RAW_SAMPLE3(const struct nx_action_sample2 *nas,
+                         enum ofp_version ofp_version OVS_UNUSED,
+                         struct ofpbuf *out)
+{
+    struct ofpact_sample *sample = ofpact_put_SAMPLE(out);
+    if (!is_all_zeros(nas->zeros, sizeof nas->zeros)) {
+        return OFPERR_NXBRC_MUST_BE_ZERO;
+    }
+    if (nas->direction != NX_ACTION_SAMPLE_DEFAULT &&
+        nas->direction != NX_ACTION_SAMPLE_INGRESS &&
+        nas->direction != NX_ACTION_SAMPLE_EGRESS) {
+        VLOG_WARN_RL(&rl, "invalid sample direction %"PRIu8, nas->direction);
+        return OFPERR_OFPBAC_BAD_ARGUMENT;
+    }
+    return decode_SAMPLE2(nas, NXAST_RAW_SAMPLE3, nas->direction, sample);
+}
+
+static void
+encode_SAMPLE2(const struct ofpact_sample *sample,
+               struct nx_action_sample2 *nas)
+{
+    nas->probability = htons(sample->probability);
+    nas->collector_set_id = htonl(sample->collector_set_id);
+    nas->obs_domain_id = htonl(sample->obs_domain_id);
+    nas->obs_point_id = htonl(sample->obs_point_id);
+    nas->sampling_port = htons(ofp_to_u16(sample->sampling_port));
+    nas->direction = sample->direction;
+}
+
 static void
 encode_SAMPLE(const struct ofpact_sample *sample,
               enum ofp_version ofp_version OVS_UNUSED, struct ofpbuf *out)
 {
-    if (sample->ofpact.raw == NXAST_RAW_SAMPLE2
-        || sample->sampling_port != OFPP_NONE) {
-        struct nx_action_sample2 *nas = put_NXAST_SAMPLE2(out);
-        nas->probability = htons(sample->probability);
-        nas->collector_set_id = htonl(sample->collector_set_id);
-        nas->obs_domain_id = htonl(sample->obs_domain_id);
-        nas->obs_point_id = htonl(sample->obs_point_id);
-        nas->sampling_port = htons(ofp_to_u16(sample->sampling_port));
+    if (sample->ofpact.raw == NXAST_RAW_SAMPLE3
+        || sample->direction != NX_ACTION_SAMPLE_DEFAULT) {
+        encode_SAMPLE2(sample, put_NXAST_SAMPLE3(out));
+    } else if (sample->ofpact.raw == NXAST_RAW_SAMPLE2
+               || sample->sampling_port != OFPP_NONE) {
+        encode_SAMPLE2(sample, put_NXAST_SAMPLE2(out));
     } else {
         struct nx_action_sample *nas = put_NXAST_SAMPLE(out);
         nas->probability = htons(sample->probability);
@@ -4919,6 +4961,7 @@ parse_SAMPLE(char *arg, struct ofpbuf *ofpacts,
 {
     struct ofpact_sample *os = ofpact_put_SAMPLE(ofpacts);
     os->sampling_port = OFPP_NONE;
+    os->direction = NX_ACTION_SAMPLE_DEFAULT;
 
     char *key, *value;
     while (ofputil_parse_key_value(&arg, &key, &value)) {
@@ -4939,6 +4982,10 @@ parse_SAMPLE(char *arg, struct ofpbuf *ofpacts,
             if (!ofputil_port_from_string(value, &os->sampling_port)) {
                 error = xasprintf("%s: unknown port", value);
             }
+        } else if (!strcmp(key, "ingress")) {
+            os->direction = NX_ACTION_SAMPLE_INGRESS;
+        } else if (!strcmp(key, "egress")) {
+            os->direction = NX_ACTION_SAMPLE_EGRESS;
         } else {
             error = xasprintf("invalid key \"%s\" in \"sample\" argument",
                               key);
@@ -4970,6 +5017,11 @@ format_SAMPLE(const struct ofpact_sample *a, struct ds *s)
         ds_put_format(s, ",%ssampling_port=%s%"PRIu16,
                       colors.param, colors.end, a->sampling_port);
     }
+    if (a->direction == NX_ACTION_SAMPLE_INGRESS) {
+        ds_put_format(s, ",%singress%s", colors.param, colors.end);
+    } else if (a->direction == NX_ACTION_SAMPLE_EGRESS) {
+        ds_put_format(s, ",%segress%s", colors.param, colors.end);
+    }
     ds_put_format(s, "%s)%s", colors.paren, colors.end);
 }
 

ofproto/ofproto-dpif-ipfix.c

@@ -1562,6 +1562,7 @@ ipfix_cache_entry_init(struct ipfix_flow_cache_entry *entry,
                        const struct dp_packet *packet, const struct flow *flow,
                        uint64_t packet_delta_count, uint32_t obs_domain_id,
                        uint32_t obs_point_id, odp_port_t output_odp_port,
+                       enum nx_action_sample_direction direction,
                        const struct dpif_ipfix_port *tunnel_port,
                        const struct flow_tnl *tunnel_key)
 {
@@ -1648,7 +1649,9 @@ ipfix_cache_entry_init(struct ipfix_flow_cache_entry *entry,
         data_common = dp_packet_put_zeros(&msg, sizeof *data_common);
         data_common->observation_point_id = htonl(obs_point_id);
         data_common->flow_direction =
-            (output_odp_port == ODPP_NONE) ? INGRESS_FLOW : EGRESS_FLOW;
+            (direction == NX_ACTION_SAMPLE_INGRESS ? INGRESS_FLOW
+             : direction == NX_ACTION_SAMPLE_EGRESS ? EGRESS_FLOW
+             : output_odp_port == ODPP_NONE ? INGRESS_FLOW : EGRESS_FLOW);
         data_common->source_mac_address = flow->dl_src;
         data_common->destination_mac_address = flow->dl_dst;
         data_common->ethernet_type = flow->dl_type;
@@ -1884,6 +1887,7 @@ dpif_ipfix_sample(struct dpif_ipfix_exporter *exporter,
                   const struct dp_packet *packet, const struct flow *flow,
                   uint64_t packet_delta_count, uint32_t obs_domain_id,
                   uint32_t obs_point_id, odp_port_t output_odp_port,
+                  enum nx_action_sample_direction direction,
                   const struct dpif_ipfix_port *tunnel_port,
                   const struct flow_tnl *tunnel_key)
 {
@@ -1895,8 +1899,8 @@ dpif_ipfix_sample(struct dpif_ipfix_exporter *exporter,
     sampled_packet_type = ipfix_cache_entry_init(entry, packet,
                                                  flow, packet_delta_count,
                                                  obs_domain_id, obs_point_id,
-                                                 output_odp_port, tunnel_port,
-                                                 tunnel_key);
+                                                 output_odp_port, direction,
+                                                 tunnel_port, tunnel_key);
     ipfix_cache_update(exporter, entry, sampled_packet_type);
 }
 
@@ -1959,7 +1963,8 @@ dpif_ipfix_bridge_sample(struct dpif_ipfix *di, const struct dp_packet *packet,
                       packet_delta_count,
                       di->bridge_exporter.options->obs_domain_id,
                       di->bridge_exporter.options->obs_point_id,
-                      output_odp_port, tunnel_port, tunnel_key);
+                      output_odp_port, NX_ACTION_SAMPLE_DEFAULT,
+                      tunnel_port, tunnel_key);
     ovs_mutex_unlock(&mutex);
 }
 
@@ -2002,7 +2007,8 @@ dpif_ipfix_flow_sample(struct dpif_ipfix *di, const struct dp_packet *packet,
                           packet_delta_count,
                           cookie->flow_sample.obs_domain_id,
                           cookie->flow_sample.obs_point_id,
-                          output_odp_port, tunnel_port, tunnel_key);
+                          output_odp_port, cookie->flow_sample.direction,
+                          tunnel_port, tunnel_key);
     }
     ovs_mutex_unlock(&mutex);
 }

ofproto/ofproto-dpif-xlate.c

@@ -4282,6 +4282,7 @@ xlate_sample_action(struct xlate_ctx *ctx,
             .obs_domain_id = os->obs_domain_id,
             .obs_point_id = os->obs_point_id,
             .output_odp_port = output_odp_port,
+            .direction = os->direction,
         }
     };
     compose_sample_action(ctx, probability, &cookie, sizeof cookie.flow_sample,

tests/odp.at

@@ -256,7 +256,9 @@ userspace(pid=9765,slow_path(cfm),tunnel_out_port=10)
 userspace(pid=1234567,userdata(0102030405060708090a0b0c0d0e0f),actions)
 userspace(pid=1234567,userdata(0102030405060708090a0b0c0d0e0f),tunnel_out_port=10)
 userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,obs_domain_id=2345,obs_point_id=3456,output_port=10))
+userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,obs_domain_id=2345,obs_point_id=3456,output_port=10,ingress))
 userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,obs_domain_id=2345,obs_point_id=3456,output_port=10),tunnel_out_port=10)
+userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,obs_domain_id=2345,obs_point_id=3456,output_port=10,egress),tunnel_out_port=10)
 userspace(pid=6633,ipfix(output_port=10))
 userspace(pid=6633,ipfix(output_port=10),tunnel_out_port=10)
 set(in_port(2))

tests/ofp-actions.at

@@ -130,6 +130,9 @@ ffff 0018 00002320 001d 3039 00005BA0 00008707 0000B26E
 # actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
 ffff 0020 00002320 0026 3039 00005BA0 00008707 0000B26E DDD50000 00000000
 
+# actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789,egress)
+ffff 0020 00002320 0029 3039 00005BA0 00008707 0000B26E DDD50200 00000000
+
 # bad OpenFlow10 actions: OFPBAC_BAD_LEN
 & ofp_actions|WARN|OpenFlow action OFPAT_OUTPUT length 240 exceeds action buffer length 8
 & ofp_actions|WARN|bad action at offset 0 (OFPBAC_BAD_LEN):

tests/ovs-ofctl.at

@@ -189,7 +189,9 @@ sctp actions=drop
 sctp actions=drop
 in_port=0 actions=resubmit:0
 actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
+actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,ingress)
 actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
+actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789,egress)
 ip,actions=ct(nat)
 ip,actions=ct(commit,nat(dst))
 ip,actions=ct(commit,nat(src))
@@ -220,7 +222,9 @@ OFPT_FLOW_MOD: ADD sctp actions=drop
 OFPT_FLOW_MOD: ADD sctp actions=drop
 OFPT_FLOW_MOD: ADD in_port=0 actions=resubmit:0
 OFPT_FLOW_MOD: ADD actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678)
+OFPT_FLOW_MOD: ADD actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,ingress)
 OFPT_FLOW_MOD: ADD actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789)
+OFPT_FLOW_MOD: ADD actions=sample(probability=12345,collector_set_id=23456,obs_domain_id=34567,obs_point_id=45678,sampling_port=56789,egress)
 OFPT_FLOW_MOD: ADD ip actions=ct(nat)
 OFPT_FLOW_MOD: ADD ip actions=ct(commit,nat(dst))
 OFPT_FLOW_MOD: ADD ip actions=ct(commit,nat(src))