On 2/8/24 19:17, Ales Musil wrote:
> Add ovn-debug binary tool that can be extended with commands that
> might be useful for tests/debugging of OVN environment. Currently
> the tool supports only two commands:
>
> 1) "lflow-stage-to-ltable STAGE_NAME" that converts stage name into
> logical flow table id.
>
> 2) "lflow-stage-to-oftable STAGE_NAME" that converts stage name into
> OpenFlow table id.
>
> For now it will be used in tests to get rid remaining hardcoded table
> numbers.
>
> Signed-off-by: Ales Musil <[email protected]>
> ---
Hi Ales,
This looks OK to me too but I have a minor style comment below.
> NEWS | 5 ++
> README.rst | 1 +
> debian/ovn-common.install | 1 +
> debian/ovn-common.manpages | 1 +
> rhel/ovn-fedora.spec.in | 2 +
> utilities/.gitignore | 2 +
> utilities/automake.mk | 10 ++-
> utilities/ovn-debug.8.xml | 28 +++++++
> utilities/ovn-debug.c | 155 +++++++++++++++++++++++++++++++++++++
> 9 files changed, 204 insertions(+), 1 deletion(-)
> create mode 100644 utilities/ovn-debug.8.xml
> create mode 100644 utilities/ovn-debug.c
>
> diff --git a/NEWS b/NEWS
> index 7114b96d1..b979e54d7 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,5 +1,10 @@
> Post v24.03.0
> -------------
> + - Add ovn-debug tool containing two commands.
> + "lflow-stage-to-ltable STAGE_NAME" that converts stage name into logical
> + flow table id.
> + "lflow-stage-to-oftable STAGE_NAME" that converts stage name into
> OpenFlow
> + table id.
>
> OVN v24.03.0 - xx xxx xxxx
> --------------------------
> diff --git a/README.rst b/README.rst
> index 6fb717742..428cd8ee8 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -56,6 +56,7 @@ The main components of this distribution are:
> - ovn-sbctl, a tool for interfacing with the southbound database.
> - ovn-trace, a debugging utility that allows for tracing of packets through
> the logical network.
> +- ovn-debug, a tool to simplify debugging of OVN setup.
> - Scripts and specs for building RPMs.
>
> What other documentation is available?
> diff --git a/debian/ovn-common.install b/debian/ovn-common.install
> index 050d1c63a..fc48f07e4 100644
> --- a/debian/ovn-common.install
> +++ b/debian/ovn-common.install
> @@ -5,6 +5,7 @@ usr/bin/ovn-ic-nbctl
> usr/bin/ovn-ic-sbctl
> usr/bin/ovn-trace
> usr/bin/ovn_detrace.py
> +usr/bin/ovn-debug
> usr/share/ovn/scripts/ovn-ctl
> usr/share/ovn/scripts/ovndb-servers.ocf
> usr/share/ovn/scripts/ovn-lib
> diff --git a/debian/ovn-common.manpages b/debian/ovn-common.manpages
> index 1fa3d9cb3..e864512e3 100644
> --- a/debian/ovn-common.manpages
> +++ b/debian/ovn-common.manpages
> @@ -11,3 +11,4 @@ utilities/ovn-ic-nbctl.8
> utilities/ovn-ic-sbctl.8
> utilities/ovn-trace.8
> utilities/ovn-detrace.1
> +utilities/ovn-debug.8
> diff --git a/rhel/ovn-fedora.spec.in b/rhel/ovn-fedora.spec.in
> index 03c1f27c5..670f1ca9e 100644
> --- a/rhel/ovn-fedora.spec.in
> +++ b/rhel/ovn-fedora.spec.in
> @@ -495,6 +495,7 @@ fi
> %{_bindir}/ovn-appctl
> %{_bindir}/ovn-ic-nbctl
> %{_bindir}/ovn-ic-sbctl
> +%{_bindir}/ovn-debug
> %{_datadir}/ovn/scripts/ovn-ctl
> %{_datadir}/ovn/scripts/ovn-lib
> %{_datadir}/ovn/scripts/ovndb-servers.ocf
> @@ -515,6 +516,7 @@ fi
> %{_mandir}/man8/ovn-ic.8*
> %{_mandir}/man5/ovn-ic-nb.5*
> %{_mandir}/man5/ovn-ic-sb.5*
> +%{_mandir}/man8/ovn-debug.8*
> %{_prefix}/lib/ocf/resource.d/ovn/ovndb-servers
> %config(noreplace) %{_sysconfdir}/logrotate.d/ovn
> %{_unitdir}/[email protected]
> diff --git a/utilities/.gitignore b/utilities/.gitignore
> index da237563b..3ae97b00f 100644
> --- a/utilities/.gitignore
> +++ b/utilities/.gitignore
> @@ -13,6 +13,8 @@
> /ovn-trace.8
> /ovn_detrace.py
> /ovn-detrace.1
> +/ovn-debug
> +/ovn-debug.8
> /ovn-docker-overlay-driver
> /ovn-docker-underlay-driver
> /ovn-lib
> diff --git a/utilities/automake.mk b/utilities/automake.mk
> index c44563c26..6a2b96e66 100644
> --- a/utilities/automake.mk
> +++ b/utilities/automake.mk
> @@ -11,7 +11,8 @@ man_MANS += \
> utilities/ovn-ic-sbctl.8 \
> utilities/ovn-trace.8 \
> utilities/ovn-detrace.1 \
> - utilities/ovn-appctl.8
> + utilities/ovn-appctl.8 \
> + utilities/ovn-debug.8
>
> MAN_ROOTS += \
> utilities/ovn-detrace.1.in
> @@ -34,6 +35,7 @@ EXTRA_DIST += \
> utilities/ovn-ic-sbctl.8.xml \
> utilities/ovn-appctl.8.xml \
> utilities/ovn-trace.8.xml \
> + utilities/ovn-debug.8.xml \
> utilities/ovn_detrace.py.in \
> utilities/ovndb-servers.ocf \
> utilities/checkpatch.py \
> @@ -62,6 +64,7 @@ CLEANFILES += \
> utilities/ovn-ic-nbctl.8 \
> utilities/ovn-ic-sbctl.8 \
> utilities/ovn-trace.8 \
> + utilities/ovn-debug.8 \
> utilities/ovn-detrace.1 \
> utilities/ovn-detrace \
> utilities/ovn_detrace.py \
> @@ -119,4 +122,9 @@ UNINSTALL_LOCAL += ovn-detrace-uninstall
> ovn-detrace-uninstall:
> rm -f $(DESTDIR)$(bindir)/ovn-detrace
>
> +# ovn-debug
> +bin_PROGRAMS += utilities/ovn-debug
> +utilities_ovn_debug_SOURCES = utilities/ovn-debug.c
> +utilities_ovn_debug_LDADD = lib/libovn.la $(OVSDB_LIBDIR)/libovsdb.la
> $(OVS_LIBDIR)/libopenvswitch.la
> +
> include utilities/bugtool/automake.mk
> diff --git a/utilities/ovn-debug.8.xml b/utilities/ovn-debug.8.xml
> new file mode 100644
> index 000000000..bdd208328
> --- /dev/null
> +++ b/utilities/ovn-debug.8.xml
> @@ -0,0 +1,28 @@
> +<?xml version="1.0" encoding="utf-8"?>
> +<manpage program="ovn-debug" section="8" title="ovn-debug">
> + <h1>Name</h1>
> + <p>ovn-debug -- Open Virtual Network debug tool</p>
> +
> + <h1>Synopsis</h1>
> + <p><code>ovn-debug</code> <var>COMMAND</var> <var>[ARG...]</var></p>
> +
> + <h1>Description</h1>
> + <p>
> + <code>ovn-debug</code>, OVN debug tool, is a tool to help with
> + debugging of OVN setup.
> + </p>
> +
> + <h1>Commands</h1>
> + <dl>
> + <dt><code>lflow-stage-to-ltable <var>STAGE_NAME</var></code></dt>
> + <dd>
> + Convert the logical flow stage name e.g. <code>ls_in_lb</code>
> into
> + the logical flow table number e.g. <code>13</code>.
> + </dd>
> + <dt><code>lflow-stage-to-oftable <var>STAGE_NAME</var></code></dt>
> + <dd>
> + Convert the logical flow stage name e.g. <code>ls_in_lb</code>
> into
> + the OpenFlow table number e.g. <code>21</code>.
> + </dd>
> + </dl>
> +</manpage>
> diff --git a/utilities/ovn-debug.c b/utilities/ovn-debug.c
> new file mode 100644
> index 000000000..04b273a4f
> --- /dev/null
> +++ b/utilities/ovn-debug.c
> @@ -0,0 +1,155 @@
> +/* 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>
> +#include <getopt.h>
> +#include <stdint.h>
> +
> +#include "command-line.h"
> +#include "controller/lflow.h"
> +#include "northd/northd.h"
> +#include "ovn-util.h"
> +
> +struct ovn_lflow_stage {
> + const char *name;
> + uint8_t table_id;
> + enum ovn_pipeline pipeline;
> +};
> +
> +static const struct ovn_lflow_stage ovn_lflow_stages[] = {
> +#define PIPELINE_STAGE(DP_TYPE, PIPELINE, STAGE, TABLE, NAME) \
> + (struct ovn_lflow_stage) { \
> + .name = NAME, \
> + .table_id = TABLE, \
> + .pipeline = P_##PIPELINE, \
> + },
> + PIPELINE_STAGES
> +#undef PIPELINE_STAGE
> +};
> +
> +static const struct ovn_lflow_stage *
> +ovn_lflow_stage_find_by_name(const char *name)
> +{
> +
> + for (size_t i = 0; i < ARRAY_SIZE(ovn_lflow_stages); i++) {
> + const struct ovn_lflow_stage *stage = &ovn_lflow_stages[i];
> + if (!strcmp(stage->name, name)) {
> + return stage;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static void
> +lflow_stage_to_table(struct ovs_cmdl_context *ctx)
> +{
> + const char *name = ctx->argv[1];
> + const struct ovn_lflow_stage *stage = ovn_lflow_stage_find_by_name(name);
> +
> + if (!stage) {
> + ovs_fatal(0, "Couldn't find OVN logical flow stage with name \"%s\"",
> + name);
> + }
> +
> + uint8_t table = stage->table_id;
> +
> + if (!strcmp("lflow-stage-to-oftable", ctx->argv[0])) {
> + table += stage->pipeline == P_IN ?
> + OFTABLE_LOG_INGRESS_PIPELINE :
> + OFTABLE_LOG_EGRESS_PIPELINE;
Please follow the coding style and break long lines before the ternary
operators '?' and ':', rather than after them:
https://github.com/ovn-org/ovn/blob/main/Documentation/internals/contributing/coding-style.rst#expressions
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
