diff mbox series

[ovs-dev,v2,02/13] odp-util: Add support OVS_ACTION_ATTR_PSAMPLE.

Message ID 20240711233238.1038670-3-amorenoz@redhat.com
State Superseded
Headers show
Series Introduce local sampling with NXAST_SAMPLE action. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Adrián Moreno July 11, 2024, 11:32 p.m. UTC
Add support for parsing and formatting the new action.

Also, flag OVS_ACTION_ATTR_SAMPLE as requiring datapath assistance if it
contains a nested OVS_ACTION_ATTR_PSAMPLE. The reason is that the
sampling rate from the parent "sample" is made available to the nested
"psample" by the kernel.

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 include/linux/openvswitch.h  | 28 +++++++++++
 lib/dpif-netdev.c            |  1 +
 lib/dpif.c                   |  3 +-
 lib/odp-execute.c            | 25 +++++++++-
 lib/odp-util.c               | 91 ++++++++++++++++++++++++++++++++++++
 lib/odp-util.h               |  3 ++
 ofproto/ofproto-dpif-ipfix.c |  1 +
 ofproto/ofproto-dpif-sflow.c |  1 +
 python/ovs/flow/odp.py       |  8 ++++
 tests/odp.at                 | 16 +++++++
 10 files changed, 175 insertions(+), 2 deletions(-)

Comments

Eelco Chaudron July 12, 2024, 2:29 p.m. UTC | #1
On 12 Jul 2024, at 1:32, Adrian Moreno wrote:

> Add support for parsing and formatting the new action.
>
> Also, flag OVS_ACTION_ATTR_SAMPLE as requiring datapath assistance if it
> contains a nested OVS_ACTION_ATTR_PSAMPLE. The reason is that the
> sampling rate from the parent "sample" is made available to the nested
> "psample" by the kernel.
>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>

Change look good to me, however one small nit below, guess this happened when moving the comment back in.

Assume this will be fixed in next revision.

Acked-by: Eelco Chaudron <echaudro@redhat.com>

> ---
>  include/linux/openvswitch.h  | 28 +++++++++++
>  lib/dpif-netdev.c            |  1 +
>  lib/dpif.c                   |  3 +-
>  lib/odp-execute.c            | 25 +++++++++-
>  lib/odp-util.c               | 91 ++++++++++++++++++++++++++++++++++++
>  lib/odp-util.h               |  3 ++
>  ofproto/ofproto-dpif-ipfix.c |  1 +
>  ofproto/ofproto-dpif-sflow.c |  1 +
>  python/ovs/flow/odp.py       |  8 ++++
>  tests/odp.at                 | 16 +++++++
>  10 files changed, 175 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
> index d9fb991ef..ac4f67c6a 100644
> --- a/include/linux/openvswitch.h
> +++ b/include/linux/openvswitch.h
> @@ -992,6 +992,31 @@ struct check_pkt_len_arg {
>  };
>  #endif
>
> +#define OVS_PSAMPLE_COOKIE_MAX_SIZE 16
> +/**
> + * enum ovs_pample_attr - Attributes for %OVS_ACTION_ATTR_PSAMPLE
> + * action.
> + *
> + * @OVS_PSAMPLE_ATTR_GROUP: 32-bit number to identify the source of the
> + * sample.
> + * @OVS_PSAMPLE_ATTR_COOKIE: An optional variable-length binary cookie that
> + * contains user-defined metadata. The maximum length is
> + * OVS_PSAMPLE_COOKIE_MAX_SIZE bytes.
> + *
> + * Sends the packet to the psample multicast group with the specified group and
> + * cookie. It is possible to combine this action with the
> + * %OVS_ACTION_ATTR_TRUNC action to limit the size of the sample.
> + */
> +enum ovs_psample_attr {
> +        OVS_PSAMPLE_ATTR_GROUP = 1,    /* u32 number. */
> +        OVS_PSAMPLE_ATTR_COOKIE,       /* Optional, user specified cookie. */
> +
> +	/* private: */

nit: The kernel file has private aligned differently.

	OVS_PSAMPLE_ATTR_GROUP = 1,	/* u32 number. */
	OVS_PSAMPLE_ATTR_COOKIE,	/* Optional, user specified cookie. */

	/* private: */
	__OVS_PSAMPLE_ATTR_MAX
};

> +        __OVS_PSAMPLE_ATTR_MAX
> +};
> +
> +#define OVS_PSAMPLE_ATTR_MAX (__OVS_PSAMPLE_ATTR_MAX - 1)
> +
>  /**
>   * enum ovs_action_attr - Action types.
>   *

<SNIP>
diff mbox series

Patch

diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
index d9fb991ef..ac4f67c6a 100644
--- a/include/linux/openvswitch.h
+++ b/include/linux/openvswitch.h
@@ -992,6 +992,31 @@  struct check_pkt_len_arg {
 };
 #endif
 
+#define OVS_PSAMPLE_COOKIE_MAX_SIZE 16
+/**
+ * enum ovs_pample_attr - Attributes for %OVS_ACTION_ATTR_PSAMPLE
+ * action.
+ *
+ * @OVS_PSAMPLE_ATTR_GROUP: 32-bit number to identify the source of the
+ * sample.
+ * @OVS_PSAMPLE_ATTR_COOKIE: An optional variable-length binary cookie that
+ * contains user-defined metadata. The maximum length is
+ * OVS_PSAMPLE_COOKIE_MAX_SIZE bytes.
+ *
+ * Sends the packet to the psample multicast group with the specified group and
+ * cookie. It is possible to combine this action with the
+ * %OVS_ACTION_ATTR_TRUNC action to limit the size of the sample.
+ */
+enum ovs_psample_attr {
+        OVS_PSAMPLE_ATTR_GROUP = 1,    /* u32 number. */
+        OVS_PSAMPLE_ATTR_COOKIE,       /* Optional, user specified cookie. */
+
+	/* private: */
+        __OVS_PSAMPLE_ATTR_MAX
+};
+
+#define OVS_PSAMPLE_ATTR_MAX (__OVS_PSAMPLE_ATTR_MAX - 1)
+
 /**
  * enum ovs_action_attr - Action types.
  *
@@ -1056,6 +1081,8 @@  struct check_pkt_len_arg {
  * of l3 tunnel flag in the tun_flags field of OVS_ACTION_ATTR_ADD_MPLS
  * argument.
  * @OVS_ACTION_ATTR_DROP: Explicit drop action.
+ * @OVS_ACTION_ATTR_PSAMPLE: Send a sample of the packet to external observers
+ * via psample.
  */
 
 enum ovs_action_attr {
@@ -1087,6 +1114,7 @@  enum ovs_action_attr {
 	OVS_ACTION_ATTR_ADD_MPLS,     /* struct ovs_action_add_mpls. */
 	OVS_ACTION_ATTR_DEC_TTL,      /* Nested OVS_DEC_TTL_ATTR_*. */
 	OVS_ACTION_ATTR_DROP,         /* u32 xlate_error. */
+	OVS_ACTION_ATTR_PSAMPLE,      /* Nested OVS_PSAMPLE_ATTR_*. */
 
 #ifndef __KERNEL__
 	OVS_ACTION_ATTR_TUNNEL_PUSH,   /* struct ovs_action_push_tnl*/
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index c7f9e1490..f0594e5f5 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -9519,6 +9519,7 @@  dp_execute_cb(void *aux_, struct dp_packet_batch *packets_,
     case OVS_ACTION_ATTR_DROP:
     case OVS_ACTION_ATTR_ADD_MPLS:
     case OVS_ACTION_ATTR_DEC_TTL:
+    case OVS_ACTION_ATTR_PSAMPLE:
     case __OVS_ACTION_ATTR_MAX:
         OVS_NOT_REACHED();
     }
diff --git a/lib/dpif.c b/lib/dpif.c
index 23eb18495..94db4630e 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1192,6 +1192,8 @@  dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
     case OVS_ACTION_ATTR_TUNNEL_PUSH:
     case OVS_ACTION_ATTR_TUNNEL_POP:
     case OVS_ACTION_ATTR_USERSPACE:
+    case OVS_ACTION_ATTR_PSAMPLE:
+    case OVS_ACTION_ATTR_SAMPLE:
     case OVS_ACTION_ATTR_RECIRC: {
         struct dpif_execute execute;
         struct ofpbuf execute_actions;
@@ -1278,7 +1280,6 @@  dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
     case OVS_ACTION_ATTR_POP_MPLS:
     case OVS_ACTION_ATTR_SET:
     case OVS_ACTION_ATTR_SET_MASKED:
-    case OVS_ACTION_ATTR_SAMPLE:
     case OVS_ACTION_ATTR_TRUNC:
     case OVS_ACTION_ATTR_PUSH_ETH:
     case OVS_ACTION_ATTR_POP_ETH:
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 081e4d432..15577d539 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -818,13 +818,13 @@  requires_datapath_assistance(const struct nlattr *a)
     case OVS_ACTION_ATTR_RECIRC:
     case OVS_ACTION_ATTR_CT:
     case OVS_ACTION_ATTR_METER:
+    case OVS_ACTION_ATTR_PSAMPLE:
         return true;
 
     case OVS_ACTION_ATTR_SET:
     case OVS_ACTION_ATTR_SET_MASKED:
     case OVS_ACTION_ATTR_PUSH_VLAN:
     case OVS_ACTION_ATTR_POP_VLAN:
-    case OVS_ACTION_ATTR_SAMPLE:
     case OVS_ACTION_ATTR_HASH:
     case OVS_ACTION_ATTR_PUSH_MPLS:
     case OVS_ACTION_ATTR_POP_MPLS:
@@ -841,6 +841,28 @@  requires_datapath_assistance(const struct nlattr *a)
     case OVS_ACTION_ATTR_DROP:
         return false;
 
+    case OVS_ACTION_ATTR_SAMPLE: {
+        /* Nested "psample" actions rely on the datapath executing the
+         * parent "sample", storing the probability and making it available
+         * when the nested "psample" is run. */
+        const struct nlattr *attr;
+        unsigned int left;
+
+        NL_NESTED_FOR_EACH (attr, left, a) {
+            if (nl_attr_type(attr) == OVS_SAMPLE_ATTR_ACTIONS) {
+                const struct nlattr *act;
+                unsigned int act_left;
+
+                NL_NESTED_FOR_EACH (act, act_left, attr) {
+                    if (nl_attr_type(act) == OVS_ACTION_ATTR_PSAMPLE) {
+                        return true;
+                    }
+                }
+            }
+        }
+        return false;
+    }
+
     case OVS_ACTION_ATTR_UNSPEC:
     case __OVS_ACTION_ATTR_MAX:
         OVS_NOT_REACHED();
@@ -1229,6 +1251,7 @@  odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
         case OVS_ACTION_ATTR_CT:
         case OVS_ACTION_ATTR_UNSPEC:
         case OVS_ACTION_ATTR_DEC_TTL:
+        case OVS_ACTION_ATTR_PSAMPLE:
         case __OVS_ACTION_ATTR_MAX:
         /* The following actions are handled by the scalar implementation. */
         case OVS_ACTION_ATTR_POP_VLAN:
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 724e6f2bc..d3245223d 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -145,6 +145,7 @@  odp_action_len(uint16_t type)
     case OVS_ACTION_ATTR_ADD_MPLS: return sizeof(struct ovs_action_add_mpls);
     case OVS_ACTION_ATTR_DEC_TTL: return ATTR_LEN_VARIABLE;
     case OVS_ACTION_ATTR_DROP: return sizeof(uint32_t);
+    case OVS_ACTION_ATTR_PSAMPLE: return ATTR_LEN_VARIABLE;
 
     case OVS_ACTION_ATTR_UNSPEC:
     case __OVS_ACTION_ATTR_MAX:
@@ -1150,6 +1151,28 @@  format_dec_ttl_action(struct ds *ds, const struct nlattr *attr,
     ds_put_format(ds, "))");
 }
 
+static void
+format_odp_psample_action(struct ds *ds, const struct nlattr *attr)
+{
+    const struct nlattr *a;
+    unsigned int left;
+
+    ds_put_cstr(ds, "psample(");
+    NL_NESTED_FOR_EACH (a, left, attr) {
+        switch (a->nla_type) {
+        case OVS_PSAMPLE_ATTR_GROUP:
+            ds_put_format(ds, "group=%"PRIu32",", nl_attr_get_u32(a));
+            break;
+        case OVS_PSAMPLE_ATTR_COOKIE:
+            ds_put_cstr(ds, "cookie=");
+            ds_put_hex(ds, nl_attr_get(a), nl_attr_get_size(a));
+            break;
+        }
+    }
+    ds_chomp(ds, ',');
+    ds_put_char(ds, ')');
+}
+
 static void
 format_odp_action(struct ds *ds, const struct nlattr *a,
                   const struct hmap *portno_names)
@@ -1309,6 +1332,9 @@  format_odp_action(struct ds *ds, const struct nlattr *a,
     case OVS_ACTION_ATTR_DROP:
         ds_put_cstr(ds, "drop");
         break;
+    case OVS_ACTION_ATTR_PSAMPLE:
+        format_odp_psample_action(ds, a);
+        break;
     case OVS_ACTION_ATTR_UNSPEC:
     case __OVS_ACTION_ATTR_MAX:
     default:
@@ -2358,6 +2384,50 @@  out:
     return ret;
 }
 
+static int
+parse_odp_psample_action(const char *s, struct ofpbuf *actions)
+{
+    char buf[2 * OVS_PSAMPLE_COOKIE_MAX_SIZE + 1];
+    uint8_t cookie[OVS_PSAMPLE_COOKIE_MAX_SIZE];
+    bool has_group = false;
+    size_t cookie_len = 0;
+    uint32_t group;
+    int n = 0;
+
+    if (!ovs_scan_len(s, &n, "psample(")) {
+        return -EINVAL;
+    }
+
+    while (s[n] != ')') {
+        n += strspn(s + n, delimiters);
+
+        if (!has_group && ovs_scan_len(s, &n, "group=%"SCNi32, &group)) {
+            has_group = true;
+            continue;
+        }
+
+        if (!cookie_len &&
+            ovs_scan_len(s, &n, "cookie=0x%32[0-9a-fA-F]", buf) && n > 7) {
+            struct ofpbuf b;
+
+            ofpbuf_use_stub(&b, cookie, OVS_PSAMPLE_COOKIE_MAX_SIZE);
+            ofpbuf_put_hex(&b, buf, &cookie_len);
+            ofpbuf_uninit(&b);
+            continue;
+        }
+        return -EINVAL;
+    }
+    n++;
+
+    if (!has_group) {
+        return -EINVAL;
+    }
+
+    odp_put_psample_action(actions, group, cookie_len ? cookie : NULL,
+                           cookie_len);
+    return n;
+}
+
 static int
 parse_action_list(struct parse_odp_context *context, const char *s,
                   struct ofpbuf *actions)
@@ -2719,6 +2789,10 @@  parse_odp_action__(struct parse_odp_context *context, const char *s,
         }
     }
 
+    if (!strncmp(s, "psample(", 8)) {
+        return parse_odp_psample_action(s, actions);
+    }
+
     {
         struct ovs_action_push_tnl data;
         int n;
@@ -7828,6 +7902,23 @@  odp_put_tnl_push_action(struct ofpbuf *odp_actions,
     nl_msg_put_unspec(odp_actions, OVS_ACTION_ATTR_TUNNEL_PUSH, data, size);
 }
 
+void
+odp_put_psample_action(struct ofpbuf *odp_actions, uint32_t group_id,
+                       uint8_t *cookie, size_t cookie_len)
+{
+    size_t offset = nl_msg_start_nested_with_flag(odp_actions,
+                                                  OVS_ACTION_ATTR_PSAMPLE);
+
+    nl_msg_put_u32(odp_actions, OVS_PSAMPLE_ATTR_GROUP, group_id);
+    if (cookie && cookie_len) {
+        ovs_assert(cookie_len <= OVS_PSAMPLE_COOKIE_MAX_SIZE);
+        nl_msg_put_unspec(odp_actions, OVS_PSAMPLE_ATTR_COOKIE, cookie,
+                          cookie_len);
+    }
+
+    nl_msg_end_nested(odp_actions, offset);
+}
+
 
 /* The commit_odp_actions() function and its helpers. */
 
diff --git a/lib/odp-util.h b/lib/odp-util.h
index 8c7baa680..e454dbfcd 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -376,6 +376,9 @@  void odp_put_pop_eth_action(struct ofpbuf *odp_actions);
 void odp_put_push_eth_action(struct ofpbuf *odp_actions,
                              const struct eth_addr *eth_src,
                              const struct eth_addr *eth_dst);
+void odp_put_psample_action(struct ofpbuf *odp_actions,
+                            uint32_t group_id, uint8_t *cookie,
+                            size_t cookie_len);
 
 static inline void odp_decode_gbp_raw(uint32_t gbp_raw,
                                       ovs_be16 *id,
diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index cd65dae7e..15b656233 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -3136,6 +3136,7 @@  dpif_ipfix_read_actions(const struct flow *flow,
         case OVS_ACTION_ATTR_DROP:
         case OVS_ACTION_ATTR_ADD_MPLS:
         case OVS_ACTION_ATTR_DEC_TTL:
+        case OVS_ACTION_ATTR_PSAMPLE:
         case __OVS_ACTION_ATTR_MAX:
         default:
             break;
diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
index 80405b68a..fb12cf419 100644
--- a/ofproto/ofproto-dpif-sflow.c
+++ b/ofproto/ofproto-dpif-sflow.c
@@ -1237,6 +1237,7 @@  dpif_sflow_read_actions(const struct flow *flow,
         case OVS_ACTION_ATTR_DROP:
         case OVS_ACTION_ATTR_ADD_MPLS:
         case OVS_ACTION_ATTR_DEC_TTL:
+        case OVS_ACTION_ATTR_PSAMPLE:
         case __OVS_ACTION_ATTR_MAX:
         default:
             break;
diff --git a/python/ovs/flow/odp.py b/python/ovs/flow/odp.py
index a8f8c067a..572dbebe9 100644
--- a/python/ovs/flow/odp.py
+++ b/python/ovs/flow/odp.py
@@ -343,6 +343,14 @@  class ODPFlow(Flow):
                     }
                 )
             ),
+            "psample": nested_kv_decoder(
+                KVDecoders(
+                    {
+                        "group": decode_int,
+                        "cookie": decode_default,
+                    }
+                )
+            )
         }
 
         _decoders["sample"] = nested_kv_decoder(
diff --git a/tests/odp.at b/tests/odp.at
index ba20604e4..402b2386d 100644
--- a/tests/odp.at
+++ b/tests/odp.at
@@ -393,6 +393,10 @@  check_pkt_len(size=200,gt(ct(nat)),le(drop))
 check_pkt_len(size=200,gt(set(eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15))),le(set(eth(src=00:01:02:03:04:06,dst=10:11:12:13:14:16))))
 lb_output(1)
 add_mpls(label=200,tc=7,ttl=64,bos=1,eth_type=0x8847)
+psample(group=12,cookie=0xf1020304050607080910111213141516)
+psample(group=12)
+sample(sample=50.0%,actions(psample(group=12,cookie=0xf1020304)))
+sample(sample=50.0%,actions(userspace(pid=42,userdata(0102030400000000)),psample(group=12)))
 ])
 AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0],
   [`cat actions.txt`
@@ -406,11 +410,23 @@  AT_DATA([actions.txt], [dnl
 encap_nsh@:{@
 tnl_push(tnl_port(6),header(size=94,type=112,eth(dst=f8:bc:12:44:34:b6,src=f8:bc:12:46:58:e0,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::92,label=0,proto=43,tclass=0x0,hlimit=64),srv6(segments_left=2,segs(2001:cafe::90,2001:cafe::91))),out_port(1))
 tnl_push(tnl_port(6),header(size=126,type=112,eth(dst=f8:bc:12:44:34:b6,src=f8:bc:12:46:58:e0,dl_type=0x86dd),ipv6(src=2001:cafe::88,dst=2001:cafe::92,label=0,proto=43,tclass=0x0,hlimit=64),srv6(segments_left=2,segs(2001:cafe::90,2001:cafe::91,2001:cafe::92,2001:cafe::93))),out_port(1))
+psample(group_id=12,cookie=0x0102030405060708090a0b0c0d0e0f0f0f)
+psample(cookie=0x010203)
+psample(group=12,cookie=0x010203,group=12)
+psample(group=abc)
+psample(group=12,cookie=wrong)
+psample()
 ])
 AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0], [dnl
 odp_actions_from_string: error
 odp_actions_from_string: error
 odp_actions_from_string: error
+odp_actions_from_string: error
+odp_actions_from_string: error
+odp_actions_from_string: error
+odp_actions_from_string: error
+odp_actions_from_string: error
+odp_actions_from_string: error
 ])
 AT_CLEANUP