| Message ID | 20250723131253.3704827-7-dchumak@nvidia.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | ovs-router: Multi-table routing infrastructure. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 7/23/25 3:12 PM, Dima Chumak via dev wrote: > Introduce an optional parameter to `ovs-appctl ovs/route/show` for > adding/deleting user routes in non-default routing tables. > > A table with ID doesn't have to exist before adding a route, OVS will > create a new table if it is missing and will add a user route there. > Though, to effect route lookup such table has to be referenced by a > routing rule. > > Signed-off-by: Dima Chumak <dchumak@nvidia.com> > --- > Documentation/howto/userspace-tunneling.rst | 4 +- > NEWS | 2 + > lib/ovs-router.c | 64 +++++++++++++++++---- > ofproto/ofproto-tnl-unixctl.man | 12 ++-- > 4 files changed, 64 insertions(+), 18 deletions(-) > > diff --git a/Documentation/howto/userspace-tunneling.rst b/Documentation/howto/userspace-tunneling.rst > index e443b85e67c9..1c7e551e0726 100644 > --- a/Documentation/howto/userspace-tunneling.rst > +++ b/Documentation/howto/userspace-tunneling.rst > @@ -201,7 +201,7 @@ Tunnel routing table > > To add route:: > > - $ ovs-appctl ovs/route/add <IP address>/<prefix length> <output-bridge-name> <gw> > + $ ovs-appctl ovs/route/add <IP address>/<prefix length> <output-bridge-name> <gw> [table=ID] > > To see all routes configured:: > > @@ -213,7 +213,7 @@ To see all router rules configured:: > > To delete route:: > > - $ ovs-appctl ovs/route/del <IP address>/<prefix length> > + $ ovs-appctl ovs/route/del <IP address>/<prefix length> [table=ID] > > To look up and display the route for a destination:: > > diff --git a/NEWS b/NEWS > index ed32c543559d..c8f4592a6e2f 100644 > --- a/NEWS > +++ b/NEWS > @@ -27,6 +27,8 @@ v3.6.0 - xx xxx xxxx > * 'ovs/route/show': added new option, table=[ID|all], to list routes from > a specific OVS table or all routes from all tables. > * Added a new sub-command, ovs/router/rule/show, to list OVS router rules. > + * 'ovs/route/add' and 'ovs/route/del': added new option, table=ID, to > + add/delete a route from a specific OVS table. > - 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 6cbe08625f5f..ba4c43ab1d2e 100644 > --- a/lib/ovs-router.c > +++ b/lib/ovs-router.c > @@ -489,7 +489,7 @@ rt_entry_delete__(const struct cls_rule *cr, struct classifier *cls) > } > > static bool > -rt_entry_delete(uint32_t mark, uint8_t priority, > +rt_entry_delete(struct classifier *cls, uint32_t mark, uint8_t priority, > const struct in6_addr *ip6_dst, uint8_t plen) > { > const struct cls_rule *cr; > @@ -502,10 +502,10 @@ rt_entry_delete(uint32_t mark, uint8_t priority, > cls_rule_init(&rule, &match, priority); > > /* Find the exact rule. */ > - cr = classifier_find_rule_exactly(&cls_main, &rule, OVS_VERSION_MAX); > + cr = classifier_find_rule_exactly(cls, &rule, OVS_VERSION_MAX); > if (cr) { > ovs_mutex_lock(&mutex); > - rt_entry_delete__(cr, &cls_main); > + rt_entry_delete__(cr, cls); > ovs_mutex_unlock(&mutex); > > res = true; > @@ -544,6 +544,7 @@ ovs_router_add(struct unixctl_conn *conn, int argc, > struct in6_addr src6 = in6addr_any; > struct in6_addr gw6 = in6addr_any; > char src6_s[IPV6_SCAN_LEN + 1]; > + uint32_t table = CLS_MAIN; > struct in6_addr ip6; > uint32_t mark = 0; > unsigned int plen; > @@ -589,6 +590,22 @@ ovs_router_add(struct unixctl_conn *conn, int argc, > } > } > > + if (ovs_scan(argv[i], "table=%"SCNi32, &table)) { SCNu32 > + continue; > + } else if (ovs_scan(argv[i], "table=local")) { > + table = CLS_LOCAL; > + continue; > + } else if (ovs_scan(argv[i], "table=main")) { > + table = CLS_MAIN; > + continue; > + } else if (ovs_scan(argv[i], "table=default")) { > + table = CLS_DEFAULT; > + continue; > + } else if (ovs_scan(argv[i], "table=")) { > + unixctl_command_reply_error(conn, "Invalid table format"); > + return; > + } > + > unixctl_command_reply_error(conn, > "Invalid pkt_mark, IP gateway or src_ip"); > return; > @@ -601,7 +618,7 @@ ovs_router_add(struct unixctl_conn *conn, int argc, > in6_addr_set_mapped_ipv4(&src6, src); > } > > - err = ovs_router_insert__(CLS_MAIN, mark, plen + 32, false, &ip6, plen, > + err = ovs_router_insert__(table, mark, plen + 32, false, &ip6, plen, > argv[2], &gw6, &src6); > if (err) { > unixctl_command_reply_error(conn, "Error while inserting route."); > @@ -614,10 +631,13 @@ static void > ovs_router_del(struct unixctl_conn *conn, int argc OVS_UNUSED, > const char *argv[], void *aux OVS_UNUSED) > { > + struct classifier *cls = &cls_main; > + uint32_t table = CLS_MAIN; > struct in6_addr ip6; > uint32_t mark = 0; > unsigned int plen; > ovs_be32 ip; > + int i; > > if (scan_ipv4_route(argv[1], &ip, &plen)) { > in6_addr_set_mapped_ipv4(&ip6, ip); > @@ -626,14 +646,34 @@ ovs_router_del(struct unixctl_conn *conn, int argc OVS_UNUSED, > unixctl_command_reply_error(conn, "Invalid parameters"); > return; > } > - if (argc > 2) { > - if (!ovs_scan(argv[2], "pkt_mark=%"SCNi32, &mark)) { > - unixctl_command_reply_error(conn, "Invalid pkt_mark"); > + > + /* Parse optional parameters. */ > + for (i = 2; i < argc; i++) { > + if (ovs_scan(argv[i], "pkt_mark=%"SCNi32, &mark)) { > + continue; > + } > + > + if (ovs_scan(argv[i], "table=%"SCNi32, &table)) { SCNu32 I see the packet mark used i for some reason, might be good to fix that as well. > + cls = cls_find(table); > + if (!cls) { > + struct ds ds = DS_EMPTY_INITIALIZER; > + > + ds_put_format(&ds, "Table %s not found", argv[i]); > + unixctl_command_reply_error(conn, ds_cstr_ro(&ds)); > + ds_destroy(&ds); > + return; > + } > + continue; > + } else if (ovs_scan(argv[i], "table=")) { > + unixctl_command_reply_error(conn, "Invalid table format"); > return; > } > + > + unixctl_command_reply_error(conn, "Invalid pkt_mark or table"); > + return; > } > > - if (rt_entry_delete(mark, plen + 32, &ip6, plen)) { > + if (rt_entry_delete(cls, mark, plen + 32, &ip6, plen)) { > unixctl_command_reply(conn, "OK"); > seq_change(tnl_conf_seq); > } else { > @@ -1102,13 +1142,13 @@ ovs_router_init(void) > fatal_signal_add_hook(ovs_router_flush_handler, NULL, NULL, true); > unixctl_command_register("ovs/route/add", > "ip/plen dev [gw] " > - "[pkt_mark=mark] [src=src_ip]", > - 2, 5, ovs_router_add, NULL); > + "[pkt_mark=mark] [src=src_ip] [table=id]", > + 2, 6, ovs_router_add, NULL); > 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, > - NULL); > + "[pkt_mark=mark] [table=id]", 1, 3, > + ovs_router_del, NULL); > unixctl_command_register("ovs/route/lookup", "ip_addr " > "[pkt_mark=mark]", 1, 2, > ovs_router_lookup_cmd, NULL); > diff --git a/ofproto/ofproto-tnl-unixctl.man b/ofproto/ofproto-tnl-unixctl.man > index 2ee17ddd4742..654f405109e6 100644 > --- a/ofproto/ofproto-tnl-unixctl.man > +++ b/ofproto/ofproto-tnl-unixctl.man > @@ -2,10 +2,12 @@ > These commands query and modify OVS tunnel components. > . > .IP "\fBovs/route/add \fIip\fB/\fIplen\fB \fIoutput_bridge\fB \ > -[\fIgw\fB] [pkt_mark=\fImark\fB] [src=\fIsrc_ip\fB]\fR" > +[\fIgw\fB] [pkt_mark=\fImark\fB] [src=\fIsrc_ip\fB] [table=\fIID\fB]\fR" All other paramenters are lowercase and you also used lowercase id while registering commands. I wonder if it's better to keep it lowercase here and in all other commands as well, just for consistency. > 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. > +routes does not look right. A non-standard table ID can be specified, in which ID/id should be in italic font. > +case the route is added to that routing table. The table is created if it > +doesn't exist. > . > .IP "\fBovs/route/show [table=\fIID|all\fB]\fR" > Print routes in OVS routing table. This includes routes cached > @@ -14,8 +16,10 @@ of the main routing table is displayed, unless requested otherwise with > \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. > +.IP "\fBovs/route/del ip/plen [pkt_mark=mark] [table=\fIID\fB]\fR" > +Delete ip/plen route from OVS routing table. The standard routing table is used > +by default, or a specific custom table when ID is provided via \fItable\fR > +parameter. Same for this command as well. It seems inconsistent how it was before, so maybe a good time to fix the fonts for the ip/len and the mark as well. > . > .IP "\fBovs/route/rule/show\fR" > Print routing rules in OVS. This includes routing rules cached from the system
diff --git a/Documentation/howto/userspace-tunneling.rst b/Documentation/howto/userspace-tunneling.rst index e443b85e67c9..1c7e551e0726 100644 --- a/Documentation/howto/userspace-tunneling.rst +++ b/Documentation/howto/userspace-tunneling.rst @@ -201,7 +201,7 @@ Tunnel routing table To add route:: - $ ovs-appctl ovs/route/add <IP address>/<prefix length> <output-bridge-name> <gw> + $ ovs-appctl ovs/route/add <IP address>/<prefix length> <output-bridge-name> <gw> [table=ID] To see all routes configured:: @@ -213,7 +213,7 @@ To see all router rules configured:: To delete route:: - $ ovs-appctl ovs/route/del <IP address>/<prefix length> + $ ovs-appctl ovs/route/del <IP address>/<prefix length> [table=ID] To look up and display the route for a destination:: diff --git a/NEWS b/NEWS index ed32c543559d..c8f4592a6e2f 100644 --- a/NEWS +++ b/NEWS @@ -27,6 +27,8 @@ v3.6.0 - xx xxx xxxx * 'ovs/route/show': added new option, table=[ID|all], to list routes from a specific OVS table or all routes from all tables. * Added a new sub-command, ovs/router/rule/show, to list OVS router rules. + * 'ovs/route/add' and 'ovs/route/del': added new option, table=ID, to + add/delete a route from a specific OVS table. - 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 6cbe08625f5f..ba4c43ab1d2e 100644 --- a/lib/ovs-router.c +++ b/lib/ovs-router.c @@ -489,7 +489,7 @@ rt_entry_delete__(const struct cls_rule *cr, struct classifier *cls) } static bool -rt_entry_delete(uint32_t mark, uint8_t priority, +rt_entry_delete(struct classifier *cls, uint32_t mark, uint8_t priority, const struct in6_addr *ip6_dst, uint8_t plen) { const struct cls_rule *cr; @@ -502,10 +502,10 @@ rt_entry_delete(uint32_t mark, uint8_t priority, cls_rule_init(&rule, &match, priority); /* Find the exact rule. */ - cr = classifier_find_rule_exactly(&cls_main, &rule, OVS_VERSION_MAX); + cr = classifier_find_rule_exactly(cls, &rule, OVS_VERSION_MAX); if (cr) { ovs_mutex_lock(&mutex); - rt_entry_delete__(cr, &cls_main); + rt_entry_delete__(cr, cls); ovs_mutex_unlock(&mutex); res = true; @@ -544,6 +544,7 @@ ovs_router_add(struct unixctl_conn *conn, int argc, struct in6_addr src6 = in6addr_any; struct in6_addr gw6 = in6addr_any; char src6_s[IPV6_SCAN_LEN + 1]; + uint32_t table = CLS_MAIN; struct in6_addr ip6; uint32_t mark = 0; unsigned int plen; @@ -589,6 +590,22 @@ ovs_router_add(struct unixctl_conn *conn, int argc, } } + if (ovs_scan(argv[i], "table=%"SCNi32, &table)) { + continue; + } else if (ovs_scan(argv[i], "table=local")) { + table = CLS_LOCAL; + continue; + } else if (ovs_scan(argv[i], "table=main")) { + table = CLS_MAIN; + continue; + } else if (ovs_scan(argv[i], "table=default")) { + table = CLS_DEFAULT; + continue; + } else if (ovs_scan(argv[i], "table=")) { + unixctl_command_reply_error(conn, "Invalid table format"); + return; + } + unixctl_command_reply_error(conn, "Invalid pkt_mark, IP gateway or src_ip"); return; @@ -601,7 +618,7 @@ ovs_router_add(struct unixctl_conn *conn, int argc, in6_addr_set_mapped_ipv4(&src6, src); } - err = ovs_router_insert__(CLS_MAIN, mark, plen + 32, false, &ip6, plen, + err = ovs_router_insert__(table, mark, plen + 32, false, &ip6, plen, argv[2], &gw6, &src6); if (err) { unixctl_command_reply_error(conn, "Error while inserting route."); @@ -614,10 +631,13 @@ static void ovs_router_del(struct unixctl_conn *conn, int argc OVS_UNUSED, const char *argv[], void *aux OVS_UNUSED) { + struct classifier *cls = &cls_main; + uint32_t table = CLS_MAIN; struct in6_addr ip6; uint32_t mark = 0; unsigned int plen; ovs_be32 ip; + int i; if (scan_ipv4_route(argv[1], &ip, &plen)) { in6_addr_set_mapped_ipv4(&ip6, ip); @@ -626,14 +646,34 @@ ovs_router_del(struct unixctl_conn *conn, int argc OVS_UNUSED, unixctl_command_reply_error(conn, "Invalid parameters"); return; } - if (argc > 2) { - if (!ovs_scan(argv[2], "pkt_mark=%"SCNi32, &mark)) { - unixctl_command_reply_error(conn, "Invalid pkt_mark"); + + /* Parse optional parameters. */ + for (i = 2; i < argc; i++) { + if (ovs_scan(argv[i], "pkt_mark=%"SCNi32, &mark)) { + continue; + } + + if (ovs_scan(argv[i], "table=%"SCNi32, &table)) { + cls = cls_find(table); + if (!cls) { + struct ds ds = DS_EMPTY_INITIALIZER; + + ds_put_format(&ds, "Table %s not found", argv[i]); + unixctl_command_reply_error(conn, ds_cstr_ro(&ds)); + ds_destroy(&ds); + return; + } + continue; + } else if (ovs_scan(argv[i], "table=")) { + unixctl_command_reply_error(conn, "Invalid table format"); return; } + + unixctl_command_reply_error(conn, "Invalid pkt_mark or table"); + return; } - if (rt_entry_delete(mark, plen + 32, &ip6, plen)) { + if (rt_entry_delete(cls, mark, plen + 32, &ip6, plen)) { unixctl_command_reply(conn, "OK"); seq_change(tnl_conf_seq); } else { @@ -1102,13 +1142,13 @@ ovs_router_init(void) fatal_signal_add_hook(ovs_router_flush_handler, NULL, NULL, true); unixctl_command_register("ovs/route/add", "ip/plen dev [gw] " - "[pkt_mark=mark] [src=src_ip]", - 2, 5, ovs_router_add, NULL); + "[pkt_mark=mark] [src=src_ip] [table=id]", + 2, 6, ovs_router_add, NULL); 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, - NULL); + "[pkt_mark=mark] [table=id]", 1, 3, + ovs_router_del, NULL); unixctl_command_register("ovs/route/lookup", "ip_addr " "[pkt_mark=mark]", 1, 2, ovs_router_lookup_cmd, NULL); diff --git a/ofproto/ofproto-tnl-unixctl.man b/ofproto/ofproto-tnl-unixctl.man index 2ee17ddd4742..654f405109e6 100644 --- a/ofproto/ofproto-tnl-unixctl.man +++ b/ofproto/ofproto-tnl-unixctl.man @@ -2,10 +2,12 @@ These commands query and modify OVS tunnel components. . .IP "\fBovs/route/add \fIip\fB/\fIplen\fB \fIoutput_bridge\fB \ -[\fIgw\fB] [pkt_mark=\fImark\fB] [src=\fIsrc_ip\fB]\fR" +[\fIgw\fB] [pkt_mark=\fImark\fB] [src=\fIsrc_ip\fB] [table=\fIID\fB]\fR" 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. +routes does not look right. A non-standard table ID can be specified, in which +case the route is added to that routing table. The table is created if it +doesn't exist. . .IP "\fBovs/route/show [table=\fIID|all\fB]\fR" Print routes in OVS routing table. This includes routes cached @@ -14,8 +16,10 @@ of the main routing table is displayed, unless requested otherwise with \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. +.IP "\fBovs/route/del ip/plen [pkt_mark=mark] [table=\fIID\fB]\fR" +Delete ip/plen route from OVS routing table. The standard routing table is used +by default, or a specific custom table when ID is provided via \fItable\fR +parameter. . .IP "\fBovs/route/rule/show\fR" Print routing rules in OVS. This includes routing rules cached from the system
Introduce an optional parameter to `ovs-appctl ovs/route/show` for adding/deleting user routes in non-default routing tables. A table with ID doesn't have to exist before adding a route, OVS will create a new table if it is missing and will add a user route there. Though, to effect route lookup such table has to be referenced by a routing rule. Signed-off-by: Dima Chumak <dchumak@nvidia.com> --- Documentation/howto/userspace-tunneling.rst | 4 +- NEWS | 2 + lib/ovs-router.c | 64 +++++++++++++++++---- ofproto/ofproto-tnl-unixctl.man | 12 ++-- 4 files changed, 64 insertions(+), 18 deletions(-)