On 4 Jul 2024, at 9:52, 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 form the parent "sample" is made available to the nested
form -> from
> "psample" by the kernel.
Two small comments below, the rest looks good.
> Signed-off-by: Adrian Moreno <amore...@redhat.com>
> ---
> include/linux/openvswitch.h | 28 +++++++++++
> lib/dpif-netdev.c | 1 +
> lib/dpif.c | 1 +
> lib/odp-execute.c | 25 +++++++++-
> lib/odp-util.c | 93 ++++++++++++++++++++++++++++++++++++
> 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, 176 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
> index d9fb991ef..0023b65fb 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: */
None of the other definitions have this private marking, so I see no need to
start adding it here.
> + __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..71728badc 100644
> --- a/lib/dpif.c
> +++ b/lib/dpif.c
> @@ -1192,6 +1192,7 @@ 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_RECIRC: {
> struct dpif_execute execute;
> struct ofpbuf execute_actions;
> 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) {
Still not hitting this code with a test case. Is this supposed to be the
psample - local case?
> + 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..be9c8b449 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,30 @@ 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_ATTR_FOR_EACH (a, left,
> + nl_attr_get(attr), nl_attr_get_size(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 +1334,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 +2386,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);
Alignment
> + return n;
> +}
> +
> static int
> parse_action_list(struct parse_odp_context *context, const char *s,
> struct ofpbuf *actions)
> @@ -2719,6 +2791,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 +7904,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
>
> --
> 2.45.2
<
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
