On 7/23/25 3:12 PM, Dima Chumak via dev wrote:
> Introduce an optional parameter to `ovs-appctl ovs/route/show` for
> printing non-default routing tables:
> 
>   ovs-appctl ovs/route/show [table=ID|all]
> 
> Default usage is unchanged:
> 
>   ovs-appctl ovs/route/show
>   Route Table:
>   Cached: ::1/128 dev lo SRC ::1
>   Cached: 127.0.0.0/8 dev lo SRC 127.0.0.1 local
>   Cached: 10.7.7.0/24 dev br-phy1 SRC 10.7.7.17
>   Cached: 0.0.0.0/0 dev eth1 GW 10.0.0.1 SRC 10.0.0.2
> 
> New usage with a specific table displays only the routes from that
> table:
> 
>   ovs-appctl ovs/route/show table=10
>   Route Table #10:
>   Cached: 10.7.7.0/24 dev br-phy0 SRC 10.7.7.7
> 
> Special table 'all' displays all of the routes, the ones which are
> coming from a non-default table have additional field 'table' displayed:
> 
>   ovs-appctl ovs/route/show table=all
>   Cached: 10.7.7.0/24 dev br-phy1 SRC 10.7.7.17 table 20
>   Cached: 10.7.7.0/24 dev br-phy0 SRC 10.7.7.7 table 10
>   Cached: ::1/128 dev lo SRC ::1
>   Cached: 127.0.0.0/8 dev lo SRC 127.0.0.1 local
>   Cached: 10.7.7.0/24 dev br-phy1 SRC 10.7.7.17
>   Cached: 0.0.0.0/0 dev eth1 GW 10.0.0.1 SRC 10.0.0.2
> 
> Signed-off-by: Dima Chumak <dchu...@nvidia.com>
> ---
>  Documentation/howto/userspace-tunneling.rst |   2 +-
>  NEWS                                        |   2 +
>  lib/ovs-router.c                            | 180 ++++++++++++++------
>  ofproto/ofproto-tnl-unixctl.man             |   9 +-
>  tests/ovs-router.at                         |   4 +
>  5 files changed, 142 insertions(+), 55 deletions(-)
> 
> diff --git a/Documentation/howto/userspace-tunneling.rst 
> b/Documentation/howto/userspace-tunneling.rst
> index 31d82fd5e57a..1dd34cd2f5e4 100644
> --- a/Documentation/howto/userspace-tunneling.rst
> +++ b/Documentation/howto/userspace-tunneling.rst
> @@ -205,7 +205,7 @@ To add route::
>  
>  To see all routes configured::
>  
> -    $ ovs-appctl ovs/route/show
> +    $ ovs-appctl ovs/route/show [table=ID|all]
>  
>  To delete route::
>  
> diff --git a/NEWS b/NEWS
> index b5d565ecc73f..97554be8e6b4 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -24,6 +24,8 @@ v3.6.0 - xx xxx xxxx
>         core file size.
>     - ovs-appctl:
>       * Added JSON output support to the 'ovs/route/show' command.
> +     * 'ovs/route/show': added new option, table=[ID|all], to list routes 
> from
> +       a specific OVS table or all routes from all tables.

We branched for 3.6 on July 16th.  These NEWS entries should go into
the Post-v3.6.0 section.

>     - ovs-vsctl:
>       * Now exits with error code 160 (ERROR_BAD_ARGUMENTS) on Windows and
>         65 (EX_DATAERR) on other platforms if it fails while waiting for
> diff --git a/lib/ovs-router.c b/lib/ovs-router.c
> index 2addc7b43d07..eaf688467389 100644
> --- a/lib/ovs-router.c
> +++ b/lib/ovs-router.c
> @@ -642,22 +642,15 @@ ovs_router_del(struct unixctl_conn *conn, int argc 
> OVS_UNUSED,
>  }
>  
>  static void
> -ovs_router_show_json(struct json **routes)
> +ovs_router_show_json(struct json **json_entries, const struct classifier 
> *cls,
> +                     uint32_t table)
>  {
> -    int n_rules = classifier_count(&cls_main);
> -    struct json **json_entries = NULL;
> +    int n_rules = classifier_count(cls);
> +    struct ds ds = DS_EMPTY_INITIALIZER;
>      struct ovs_router_entry *rt;
> -    struct ds ds;
>      int i = 0;
>  
> -    if (!n_rules) {
> -        goto out;
> -    }
> -
> -    json_entries = xmalloc(n_rules * sizeof *json_entries);
> -    ds_init(&ds);
> -
> -    CLS_FOR_EACH (rt, cr, &cls_main) {
> +    CLS_FOR_EACH (rt, cr, cls) {
>          bool user = rt->priority != rt->plen && !rt->local;
>          uint8_t plen = rt->plen;
>          struct json *json, *nh;
> @@ -673,6 +666,7 @@ ovs_router_show_json(struct json **routes)
>              plen -= 96;
>          }
>  
> +        json_object_put(json, "table", json_integer_create(table));
>          json_object_put(json, "user", json_boolean_create(user));
>          json_object_put(json, "local", json_boolean_create(rt->local));
>          json_object_put(json, "priority", json_integer_create(rt->priority));
> @@ -702,48 +696,53 @@ ovs_router_show_json(struct json **routes)
>      }
>  
>      ds_destroy(&ds);
> -
> -out:
> -    *routes = json_array_create(json_entries, i);
>  }
>  
>  static void
> -ovs_router_show_text(struct ds *ds)
> +ovs_router_show_text(struct ds *ds, const struct classifier *cls,
> +                     uint32_t table, bool show_header)
>  {
> -    struct classifier *std_cls[] = { &cls_local, &cls_main, &cls_default };
>      struct ovs_router_entry *rt;
>  
> -    ds_put_format(ds, "Route Table:\n");
> -    for (int i = 0; i < ARRAY_SIZE(std_cls); i++) {
> -        CLS_FOR_EACH (rt, cr, std_cls[i]) {
> -            uint8_t plen;
> -            if (rt->priority == rt->plen || rt->local) {
> -                ds_put_format(ds, "Cached: ");
> -            } else {
> -                ds_put_format(ds, "User: ");
> -            }
> -            ipv6_format_mapped(&rt->nw_addr, ds);
> -            plen = rt->plen;
> -            if (IN6_IS_ADDR_V4MAPPED(&rt->nw_addr)) {
> -                plen -= 96;
> -            }
> -            ds_put_format(ds, "/%"PRIu8, plen);
> -            if (rt->mark) {
> -                ds_put_format(ds, " MARK %"PRIu32, rt->mark);
> -            }
> +    if (show_header) {
> +        if (route_table_is_standard_id(table)) {
> +            ds_put_format(ds, "Route Table:\n");
> +        } else {
> +            ds_put_format(ds, "Route Table #%u:\n", table);

PRIu32

> +        }
> +    }
>  
> -            ds_put_format(ds, " dev %s", rt->output_netdev);
> -            if (ipv6_addr_is_set(&rt->gw)) {
> -                ds_put_format(ds, " GW ");
> -                ipv6_format_mapped(&rt->gw, ds);
> -            }
> -            ds_put_format(ds, " SRC ");
> -            ipv6_format_mapped(&rt->src_addr, ds);
> -            if (rt->local) {
> -                ds_put_format(ds, " local");
> -            }
> -            ds_put_format(ds, "\n");
> +    CLS_FOR_EACH (rt, cr, cls) {
> +        uint8_t plen;
> +        if (rt->priority == rt->plen || rt->local) {
> +            ds_put_format(ds, "Cached: ");
> +        } else {
> +            ds_put_format(ds, "User: ");
> +        }
> +        ipv6_format_mapped(&rt->nw_addr, ds);
> +        plen = rt->plen;
> +        if (IN6_IS_ADDR_V4MAPPED(&rt->nw_addr)) {
> +            plen -= 96;
> +        }
> +        ds_put_format(ds, "/%"PRIu8, plen);
> +        if (rt->mark) {
> +            ds_put_format(ds, " MARK %"PRIu32, rt->mark);
> +        }
> +
> +        ds_put_format(ds, " dev %s", rt->output_netdev);
> +        if (ipv6_addr_is_set(&rt->gw)) {
> +            ds_put_format(ds, " GW ");
> +            ipv6_format_mapped(&rt->gw, ds);
> +        }
> +        ds_put_format(ds, " SRC ");
> +        ipv6_format_mapped(&rt->src_addr, ds);
> +        if (rt->local) {
> +            ds_put_format(ds, " local");
>          }
> +        if (!route_table_is_standard_id(table) && !show_header) {
> +            ds_put_format(ds, " table %u", table);

PRIu32

> +        }
> +        ds_put_format(ds, "\n");
>      }
>  }
>  
> @@ -751,15 +750,94 @@ static void
>  ovs_router_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
>                 const char *argv[] OVS_UNUSED, void *aux OVS_UNUSED)
>  {
> +    struct ds ds = DS_EMPTY_INITIALIZER;
> +    struct classifier *cls = &cls_main;
> +    uint32_t table = CLS_MAIN;
> +
> +    if (argc > 1) {
> +        if (!strcmp(argv[1], "table=all")) {
> +            table = CLS_ALL;
> +        } else if (!ovs_scan(argv[1], "table=%"SCNi32, &table)) {

SCNu32 ?

> +            unixctl_command_reply_error(conn, "Invalid table format");
> +            return;
> +        }
> +    }
> +
> +    if (table != CLS_ALL) {
> +        cls = cls_find(table);
> +        if (!cls) {
> +            ds_put_format(&ds, "Invalid param, table '%s' not found", 
> argv[1]);

Please, don't shorten the words in user-visible text, but also
the "Invalid param" part seems unnecessary.

> +            unixctl_command_reply_error(conn, ds_cstr_ro(&ds));
> +            ds_destroy(&ds);
> +            return;
> +        }
> +    }
> +
>      if (unixctl_command_get_output_format(conn) == UNIXCTL_OUTPUT_FMT_JSON) {
> -        struct json *routes;
> +        struct json *routes, **json_entries = NULL;
> +        struct json **cls_entries;
> +        size_t num_routes = 0;
> +
> +        if (table == CLS_ALL) {
> +            struct clsmap_node *node;
> +
> +            ovs_mutex_lock(&mutex);
> +
> +            HMAP_FOR_EACH (node, hash_node, &clsmap) {
> +                num_routes += node->cls.n_rules;
> +            }
>  
> -        ovs_router_show_json(&routes);
> +            num_routes += cls_main.n_rules;
> +            json_entries = xzalloc(num_routes * sizeof *json_entries);
> +            cls_entries = json_entries;
> +
> +            HMAP_FOR_EACH (node, hash_node, &clsmap) {
> +                ovs_router_show_json(cls_entries, &node->cls, node->table);

We have so many allocation inside, so it doesn't really matter if we save a
couple reallocs by counting the exact number of elements beforehand.  We should
just create an empty json array and pass it into ovs_router_show_json(), where
we can use json_array_add().  It will be much easier to understand what's going
on here without manual allocations and the pointer arithmetic.

> +                cls_entries += node->cls.n_rules;
> +            }
> +            ovs_router_show_json(cls_entries, &cls_main, CLS_MAIN);
> +
> +            ovs_mutex_unlock(&mutex);
> +        } else if (route_table_is_standard_id(table)) {
> +            num_routes += cls_local.n_rules + cls_main.n_rules +
> +                          cls_default.n_rules;
> +            json_entries = xzalloc(num_routes * sizeof *json_entries);
> +            cls_entries = json_entries;
> +
> +            ovs_router_show_json(cls_entries, &cls_local, CLS_LOCAL);
> +            cls_entries += cls_local.n_rules;
> +            ovs_router_show_json(cls_entries, &cls_main, CLS_MAIN);
> +            cls_entries += cls_main.n_rules;
> +            ovs_router_show_json(cls_entries, &cls_default, CLS_DEFAULT);
> +        } else {
> +            if (cls->n_rules) {
> +                num_routes = cls->n_rules;
> +                json_entries = xmalloc(num_routes * sizeof *json_entries);
> +                ovs_router_show_json(json_entries, cls, table);
> +            }
> +        }
> +
> +        routes = json_array_create(json_entries, num_routes);
>          unixctl_command_reply_json(conn, routes);
>      } else {
> -        struct ds ds = DS_EMPTY_INITIALIZER;
> +        if (table == CLS_ALL) {
> +            struct clsmap_node *node;
>  
> -        ovs_router_show_text(&ds);
> +            ovs_router_show_text(&ds, &cls_local, CLS_LOCAL, false);
> +            ovs_mutex_lock(&mutex);
> +            HMAP_FOR_EACH (node, hash_node, &clsmap) {
> +                ovs_router_show_text(&ds, &node->cls, node->table, false);
> +            }
> +            ovs_mutex_unlock(&mutex);
> +            ovs_router_show_text(&ds, &cls_main, CLS_MAIN, false);
> +            ovs_router_show_text(&ds, &cls_default, CLS_DEFAULT, false);
> +        } else if (route_table_is_standard_id(table)) {
> +            ovs_router_show_text(&ds, &cls_local, CLS_LOCAL, true);
> +            ovs_router_show_text(&ds, &cls_main, CLS_MAIN, false);
> +            ovs_router_show_text(&ds, &cls_default, CLS_DEFAULT, false);
> +        } else {
> +            ovs_router_show_text(&ds, cls, table, true);
> +        }
>          unixctl_command_reply(conn, ds_cstr(&ds));
>          ds_destroy(&ds);
>      }
> @@ -935,7 +1013,7 @@ ovs_router_init(void)
>                                   "ip/plen dev [gw] "
>                                   "[pkt_mark=mark] [src=src_ip]",
>                                   2, 5, ovs_router_add, NULL);
> -        unixctl_command_register("ovs/route/show", "", 0, 0,
> +        unixctl_command_register("ovs/route/show", "[table=ID|all]", 0, 1,
>                                   ovs_router_show, NULL);
>          unixctl_command_register("ovs/route/del", "ip/plen "
>                                   "[pkt_mark=mark]", 1, 2, ovs_router_del,
> diff --git a/ofproto/ofproto-tnl-unixctl.man b/ofproto/ofproto-tnl-unixctl.man
> index a801cfdccc5c..b9e4d99b2bf8 100644
> --- a/ofproto/ofproto-tnl-unixctl.man
> +++ b/ofproto/ofproto-tnl-unixctl.man
> @@ -7,9 +7,12 @@ Adds \fIip\fR/\fIplen\fR route to vswitchd routing table. 
> \fIoutput_bridge\fR
>  needs to be OVS bridge name.  This command is useful if OVS cached
>  routes does not look right.
>  .
> -.IP "\fBovs/route/show\fR"
> -Print all routes in OVS routing table, This includes routes cached
> -from system routing table and user configured routes.
> +.IP "\fBovs/route/show [table=\fIID|all\fB]\fR"

The '|' should not be italic.

For some reson all the commands in this file do not close the
original bold after the command name ends while they should...

> +Print routes in OVS routing table. This includes routes cached
> +from system routing table and user configured routes. By default, the 
> contents
> +of the main routing table is displayed, unless requested otherwise with

s/main table/all the default tables (local, main, default)/

> +\fItable\fR parameter. In this case the contents of a specific table ID or of
> +all routing tables is printed.
>  .
>  .IP "\fBovs/route/del ip/plen [pkt_mark=mark]\fR"
>  Delete ip/plen route from OVS routing table.
> diff --git a/tests/ovs-router.at b/tests/ovs-router.at
> index 641b780a582a..dac096cc1aec 100644
> --- a/tests/ovs-router.at
> +++ b/tests/ovs-router.at
> @@ -39,6 +39,7 @@ AT_CHECK([ovs-appctl --format=json --pretty 
> ovs/route/show], [0], [dnl
>      "prefix": 24,
>      "prefsrc": "2.2.2.2",
>      "priority": 184,
> +    "table": 254,
>      "user": false},
>    {
>      "dst": "1.1.1.0",
> @@ -50,6 +51,7 @@ AT_CHECK([ovs-appctl --format=json --pretty 
> ovs/route/show], [0], [dnl
>      "prefix": 24,
>      "prefsrc": "2.2.2.2",
>      "priority": 152,
> +    "table": 254,
>      "user": true},
>    {
>      "dst": "1.1.2.0",
> @@ -62,6 +64,7 @@ AT_CHECK([ovs-appctl --format=json --pretty 
> ovs/route/show], [0], [dnl
>      "prefix": 24,
>      "prefsrc": "2.2.2.2",
>      "priority": 152,
> +    "table": 254,
>      "user": true},
>    {
>      "dst": "2.2.2.3",
> @@ -73,6 +76,7 @@ AT_CHECK([ovs-appctl --format=json --pretty 
> ovs/route/show], [0], [dnl
>      "prefix": 32,
>      "prefsrc": "2.2.2.2",
>      "priority": 160,
> +    "table": 254,
>      "user": true}]]
>  ])
>  OVS_VSWITCHD_STOP

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to