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 <amore...@redhat.com>
> > ---
> >  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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to