On Wed, Jun 26, 2024 at 02:14:27PM GMT, Eelco Chaudron wrote:
> On 5 Jun 2024, at 22:23, Adrian Moreno wrote:
>
> > This simple program reads from psample and prints the packets to stdout.
>
> Some comments below.
>
> //Eelco
>
> > Signed-off-by: Adrian Moreno <[email protected]>
> > ---
> > include/linux/automake.mk | 1 +
> > include/linux/psample.h | 68 +++++++++
> > tests/automake.mk | 3 +-
> > tests/test-psample.c | 282 ++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 353 insertions(+), 1 deletion(-)
> > create mode 100644 include/linux/psample.h
> > create mode 100644 tests/test-psample.c
> >
> > diff --git a/include/linux/automake.mk b/include/linux/automake.mk
> > index cdae5eedc..ac306b53c 100644
> > --- a/include/linux/automake.mk
> > +++ b/include/linux/automake.mk
> > @@ -3,6 +3,7 @@ noinst_HEADERS += \
> > include/linux/netfilter/nf_conntrack_sctp.h \
> > include/linux/openvswitch.h \
> > include/linux/pkt_cls.h \
> > + include/linux/psample.h \
> > include/linux/gen_stats.h \
> > include/linux/tc_act/tc_mpls.h \
> > include/linux/tc_act/tc_pedit.h \
> > diff --git a/include/linux/psample.h b/include/linux/psample.h
> > new file mode 100644
> > index 000000000..d5761b730
> > --- /dev/null
> > +++ b/include/linux/psample.h
> > @@ -0,0 +1,68 @@
> > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > +#ifndef __LINUX_PSAMPLE_H
> > +#define __LINUX_PSAMPLE_H
> > +
> > +enum {
> > + PSAMPLE_ATTR_IIFINDEX,
> > + PSAMPLE_ATTR_OIFINDEX,
> > + PSAMPLE_ATTR_ORIGSIZE,
> > + PSAMPLE_ATTR_SAMPLE_GROUP,
> > + PSAMPLE_ATTR_GROUP_SEQ,
> > + PSAMPLE_ATTR_SAMPLE_RATE,
> > + PSAMPLE_ATTR_DATA,
> > + PSAMPLE_ATTR_GROUP_REFCOUNT,
> > + PSAMPLE_ATTR_TUNNEL,
> > +
> > + PSAMPLE_ATTR_PAD,
> > + PSAMPLE_ATTR_OUT_TC, /* u16 */
> > + PSAMPLE_ATTR_OUT_TC_OCC, /* u64, bytes */
> > + PSAMPLE_ATTR_LATENCY, /* u64, nanoseconds */
> > + PSAMPLE_ATTR_TIMESTAMP, /* u64, nanoseconds */
> > + PSAMPLE_ATTR_PROTO, /* u16 */
> > + PSAMPLE_ATTR_USER_COOKIE, /* binary, user provided data */
> > + PSAMPLE_ATTR_SAMPLE_PROBABILITY,/* no argument, interpret rate in
> > + * PSAMPLE_ATTR_SAMPLE_RATE as a
> > + * probability scaled 0 - U32_MAX.
> > + */
> > +
> > + __PSAMPLE_ATTR_MAX
> > +};
> > +
> > +enum psample_command {
> > + PSAMPLE_CMD_SAMPLE,
> > + PSAMPLE_CMD_GET_GROUP,
> > + PSAMPLE_CMD_NEW_GROUP,
> > + PSAMPLE_CMD_DEL_GROUP,
> > + PSAMPLE_CMD_SAMPLE_FILTER_SET,
> > +};
> > +
> > +enum psample_tunnel_key_attr {
> > + PSAMPLE_TUNNEL_KEY_ATTR_ID, /* be64 Tunnel ID */
> > + PSAMPLE_TUNNEL_KEY_ATTR_IPV4_SRC, /* be32 src IP address. */
> > + PSAMPLE_TUNNEL_KEY_ATTR_IPV4_DST, /* be32 dst IP address. */
> > + PSAMPLE_TUNNEL_KEY_ATTR_TOS, /* u8 Tunnel IP ToS. */
> > + PSAMPLE_TUNNEL_KEY_ATTR_TTL, /* u8 Tunnel IP TTL. */
> > + PSAMPLE_TUNNEL_KEY_ATTR_DONT_FRAGMENT, /* No argument, set DF. */
> > + PSAMPLE_TUNNEL_KEY_ATTR_CSUM, /* No argument. CSUM
> > packet. */
> > + PSAMPLE_TUNNEL_KEY_ATTR_OAM, /* No argument. OAM frame.
> > */
> > + PSAMPLE_TUNNEL_KEY_ATTR_GENEVE_OPTS, /* Array of Geneve options.
> > */
> > + PSAMPLE_TUNNEL_KEY_ATTR_TP_SRC, /* be16 src Transport Port.
> > */
> > + PSAMPLE_TUNNEL_KEY_ATTR_TP_DST, /* be16 dst Transport Port.
> > */
> > + PSAMPLE_TUNNEL_KEY_ATTR_VXLAN_OPTS, /* Nested VXLAN opts* */
> > + PSAMPLE_TUNNEL_KEY_ATTR_IPV6_SRC, /* struct in6_addr src IPv6
> > address. */
> > + PSAMPLE_TUNNEL_KEY_ATTR_IPV6_DST, /* struct in6_addr dst IPv6
> > address. */
> > + PSAMPLE_TUNNEL_KEY_ATTR_PAD,
> > + PSAMPLE_TUNNEL_KEY_ATTR_ERSPAN_OPTS, /* struct erspan_metadata */
> > + PSAMPLE_TUNNEL_KEY_ATTR_IPV4_INFO_BRIDGE, /* No argument.
> > IPV4_INFO_BRIDGE mode.*/
> > + __PSAMPLE_TUNNEL_KEY_ATTR_MAX
> > +};
> > +
> > +/* Can be overridden at runtime by module option */
> > +#define PSAMPLE_ATTR_MAX (__PSAMPLE_ATTR_MAX - 1)
> > +
> > +#define PSAMPLE_NL_MCGRP_CONFIG_NAME "config"
> > +#define PSAMPLE_NL_MCGRP_SAMPLE_NAME "packets"
> > +#define PSAMPLE_GENL_NAME "psample"
> > +#define PSAMPLE_GENL_VERSION 1
> > +
> > +#endif
> > diff --git a/tests/automake.mk b/tests/automake.mk
> > index 04f48f2d8..edfc2cb33 100644
> > --- a/tests/automake.mk
> > +++ b/tests/automake.mk
> > @@ -499,7 +499,8 @@ endif
> > if LINUX
> > tests_ovstest_SOURCES += \
> > tests/test-netlink-conntrack.c \
> > - tests/test-netlink-policy.c
> > + tests/test-netlink-policy.c \
> > + tests/test-psample.c
> > endif
> >
> > tests_ovstest_LDADD = lib/libopenvswitch.la
> > diff --git a/tests/test-psample.c b/tests/test-psample.c
> > new file mode 100644
> > index 000000000..f04d903b1
> > --- /dev/null
> > +++ b/tests/test-psample.c
> > @@ -0,0 +1,282 @@
> > +/*
> > + * Copyright (c) 2024 Red Hat, Inc.
> > + *
> > + * Licensed under the Apache License, Version 2.0 (the "License");
> > + * you may not use this file except in compliance with the License.
> > + * You may obtain a copy of the License at:
> > + *
> > + * http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing, software
> > + * distributed under the License is distributed on an "AS IS" BASIS,
> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > + * See the License for the specific language governing permissions and
> > + * limitations under the License.
> > + */
> > +
> > +#include <config.h>
> > +#undef NDEBUG
> > +#include <errno.h>
> > +#include <getopt.h>
> > +#include <stdbool.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +
> > +#include <linux/psample.h>
> > +
> > +#include "command-line.h"
> > +#include "dp-packet.h"
> > +#include "util.h"
> > +#include "netlink.h"
> > +#include "netlink-socket.h"
> > +#include "openvswitch/ofp-actions.h"
> > +#include "openvswitch/ofp-print.h"
> > +#include "openvswitch/types.h"
> > +#include "openvswitch/uuid.h"
> > +#include "openvswitch/vlog.h"
> > +#include "ovstest.h"
> > +
> > +VLOG_DEFINE_THIS_MODULE(test_psample);
> > +
> > +static uint32_t group_id = 0;
> > +static bool has_filter;
> > +static int psample_family = 0;
>
> Reverse Christmas tree order.
>
Ack.
> > +static void usage(void)
> > +{
> > + printf("%s: psample collector test utility\n"
> > + "usage: %s [OPTIONS] [GROUP]\n"
> > + "where GROUP is the psample group_id to listen on. "
> > + "If none is provided all events are printed.\n",
> > + program_name, program_name);
>
> Misaligned
>
Ack.
> > + vlog_usage();
> > + printf("\nOther Options:\n"
> > + " -h, --help display this help message\n");
> > +}
> > +
> > +static void parse_options(int argc, char *argv[])
> > +{
> > + enum {
> > + VLOG_OPTION_ENUMS
> > + };
> > + static const struct option long_options[] = {
> > + {"group", required_argument, NULL, 'g'},
> > + {"help", no_argument, NULL, 'h'},
> > + VLOG_LONG_OPTIONS,
> > + {NULL, 0, NULL, 0},
> > + };
> > + char *tmp_short_options, *short_options;
> > + int ret = EXIT_SUCCESS;
> > + bool do_exit = false;
> > +
> > + tmp_short_options =
> > ovs_cmdl_long_options_to_short_options(long_options);
> > + short_options = xasprintf("+%s", tmp_short_options);
> > +
> > + while (!do_exit) {
> > + int option;
> > +
> > + option = getopt_long(argc, argv, short_options, long_options,
> > NULL);
> > + if (option == -1) {
> > + break;
> > + }
> > +
> > + switch (option) {
> > +
> > + VLOG_OPTION_HANDLERS
> > +
> > + case 'h':
> > + usage();
> > + do_exit = true;
> > + ret = EXIT_SUCCESS;
> > + break;
> > +
> > + case '?':
> > + do_exit = true;
> > + ret = EXIT_FAILURE;
> > + break;
> > +
> > + default:
> > + OVS_NOT_REACHED();
> > + }
> > + }
> > +
> > + free(tmp_short_options);
> > + free(short_options);
> > + if (do_exit) {
> > + exit(ret);
> > + }
> > +}
> > +
> > +static int connect_psample_socket(struct nl_sock **sock)
> > +{
> > + unsigned int psample_packet_mcgroup;
> > + int error;
> > +
> > + error = nl_lookup_genl_family(PSAMPLE_GENL_NAME , &psample_family);
> > + if (error) {
> > + VLOG_ERR("PSAMPLE_GENL_NAME not found: %s", ovs_strerror(error));
> > + return error;
> > + }
> > +
> > + error = nl_lookup_genl_mcgroup(PSAMPLE_GENL_NAME,
> > + PSAMPLE_NL_MCGRP_SAMPLE_NAME,
> > + &psample_packet_mcgroup);
> > + if (error) {
> > + VLOG_ERR("psample packet multicast group not found: %s",
> > + ovs_strerror(error));
> > + return error;
> > + }
> > +
> > + error = nl_sock_create(NETLINK_GENERIC, sock);
> > + if (error) {
> > + VLOG_ERR("cannot create netlink socket: %s ", ovs_strerror(error));
> > + return error;
> > + }
> > +
> > + nl_sock_listen_all_nsid(*sock, true);
> > +
> > + error = nl_sock_join_mcgroup(*sock, psample_packet_mcgroup);
> > + if (error) {
> > + nl_sock_destroy(*sock);
> > + *sock = NULL;
> > + VLOG_ERR("cannot join psample multicast group: %s",
> > + ovs_strerror(error));
> > + return error;
> > + }
> > + return 0;
> > +}
> > +
> > +/* Internal representation of a sample. */
> > +struct sample {
> > + struct dp_packet packet;
> > + uint32_t group_id;
> > + uint32_t obs_domain_id;
> > + uint32_t obs_point_id;
> > + bool has_cookie;
> > +};
> > +
> > +static inline void
> > +sample_clear(struct sample *sample) {
> > + sample->group_id = 0;
> > + sample->obs_domain_id = 0;
> > + sample->obs_point_id = 0;
> > + sample->has_cookie = false;
> > + dp_packet_clear(&sample->packet);
> > +}
> > +
> > +static int
> > +parse_psample(struct ofpbuf *buf, struct sample *sample) {
> > + static const struct nl_policy psample_packet_policy[] = {
> > + [PSAMPLE_ATTR_SAMPLE_GROUP] = { .type = NL_A_U32 },
> > + [PSAMPLE_ATTR_DATA] = { .type = NL_A_UNSPEC,
> > + .optional = true, },
> > + [PSAMPLE_ATTR_USER_COOKIE] = { .type = NL_A_UNSPEC,
> > + .optional = true },
> > + };
> > +
> > + struct ofpbuf b = ofpbuf_const_initializer(buf->data, buf->size);
> > + struct nlmsghdr *nlmsg = ofpbuf_try_pull(&b, sizeof *nlmsg);
> > + struct genlmsghdr *genl = ofpbuf_try_pull(&b, sizeof *genl);
> > + struct nlattr *attr;
> > + const char *cookie;
> > +
> > + struct nlattr *a[ARRAY_SIZE(psample_packet_policy)];
> > + if (!nlmsg || !genl
> > + || !nl_policy_parse(&b, 0, psample_packet_policy, a,
> > + ARRAY_SIZE(psample_packet_policy))) {
> > + return EINVAL;
> > + }
> > +
> > + attr = a[PSAMPLE_ATTR_DATA];
> > + if (attr) {
> > + dp_packet_push(&sample->packet, nl_attr_get(attr),
> > + nl_attr_get_size(attr));
> > + }
> > +
> > + sample->group_id = nl_attr_get_u32(a[PSAMPLE_ATTR_SAMPLE_GROUP]);
> > +
> > + attr = a[PSAMPLE_ATTR_USER_COOKIE];
> > + if (attr && nl_attr_get_size(attr) ==
> > + sizeof sample->obs_domain_id + sizeof sample->obs_point_id) {
> > + cookie = nl_attr_get(attr);
> > +
> > + sample->has_cookie = true;
> > + memcpy(&sample->obs_domain_id, cookie, sizeof
> > sample->obs_domain_id);
> > + memcpy(&sample->obs_point_id, cookie + sizeof
> > sample->obs_domain_id,
> > + sizeof sample->obs_point_id);
> > + }
> > + return 0;
> > +}
> > +
> > +static void run(struct nl_sock *sock)
> > +{
> > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 10);
> > + struct sample sample = {};
> > + int error;
> > +
> > + dp_packet_init(&sample.packet, 1500);
> > +
> > + for (;;) {
> > + uint64_t buf_stub[4096 / 8];
> > + struct ofpbuf buf;
> > +
> > + sample_clear(&sample);
> > +
> > + ofpbuf_use_stub(&buf, buf_stub, sizeof buf_stub);
> > + error = nl_sock_recv(sock, &buf, NULL, true);
> > +
> > + if (error == ENOBUFS) {
> > + fprintf(stderr, "[missed events]\n");
> > + continue;
> > + } else if (error == EAGAIN) {
> > + continue;
> > + } else if (error) {
> > + VLOG_ERR_RL(&rl, "error reading samples: %i", error);
> Do we want to do a 'continue' here and retry?
I initially had an ovs_fatal here, now that it's just a log, sure, let's
avoid trying to print garbage.
> > + }
> > +
> > + error = parse_psample(&buf, &sample);
> > + if (error) {
> > + VLOG_ERR_RL(&rl, "error parsing samples: %i", error);
> Do we want to do a 'continue' here and retry?
Same
> > + }
> > +
> > + if (!has_filter || sample.group_id == group_id) {
> > + fprintf(stdout, "group_id=0x%"PRIx32" ",
> > + sample.group_id);
>
> Think this can be a single line.
>
Ack.
> > + if (sample.has_cookie) {
> > + fprintf(stdout,
> > + "obs_domain=0x%"PRIx32",obs_point=0x%"PRIx32" ",
> > + sample.obs_domain_id, sample.obs_point_id);
> > + }
> > + ofp_print_dp_packet(stdout, &sample.packet);
> > + }
> > + fflush(stdout);
> > + }
> > +}
> > +
> > +static void
> > +test_psample_main(int argc, char *argv[])
> > +{
> > + struct nl_sock *sock;
> > + int error;
> > +
> > + parse_options(argc, argv);
> > +
> > + if (argc - optind > 1) {
> > + ovs_fatal(0, "at most one positional argument supported "
> > + "(use --help for help)");
> > + } else if (argc - optind == 1) {
> > + if (!ovs_scan(argv[optind], "%"SCNu32, &group_id)) {
>
> Maybe use the str_to_uint() family for better error checking? It will happily
> accept 1e for example.
I'll give it a try. Thanks.
>
> > + ovs_fatal(0, "invalid group id");
> > + }
> > + has_filter = true;
> > + }
> > +
> > + error = connect_psample_socket(&sock);
> > + if (error) {
> > + ovs_fatal(error, "failed to connect to psample socket");
> > + }
> > +
> > + run(sock);
> > +}
> > +
> > +OVSTEST_REGISTER("test-psample", test_psample_main);
> > --
> > 2.45.1
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
