Re: [ovs-dev] [PATCH v6 2/5] ovs-router: Cleanup parser for ovs/route/add command.

2023-03-03 Thread Eelco Chaudron



On 2 Mar 2023, at 7:34, Nobuhiro MIKI wrote:

> This patch cleans up the parser to accept pkt_mark and gw in any order.
>
> pkt_mark and gw are normally expected to be specified exactly once.
> However, as with other tools, if specified multiple times, the last
> specification is used. Also, pkt_mark and gw have separate prefix
> strings so they can be parsed in any order.
>
> Acked-by: Eelco Chaudron 
> Reviewed-by: Simon Horman 
> Signed-off-by: Nobuhiro MIKI 

Changes look good.

Acked-by: Eelco Chaudron 

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


[ovs-dev] [PATCH v6 2/5] ovs-router: Cleanup parser for ovs/route/add command.

2023-03-01 Thread Nobuhiro MIKI
This patch cleans up the parser to accept pkt_mark and gw in any order.

pkt_mark and gw are normally expected to be specified exactly once.
However, as with other tools, if specified multiple times, the last
specification is used. Also, pkt_mark and gw have separate prefix
strings so they can be parsed in any order.

Acked-by: Eelco Chaudron 
Reviewed-by: Simon Horman 
Signed-off-by: Nobuhiro MIKI 
---
 lib/ovs-router.c| 53 +
 tests/ovs-router.at | 27 +++
 2 files changed, 52 insertions(+), 28 deletions(-)

diff --git a/lib/ovs-router.c b/lib/ovs-router.c
index 5d0fbd503e9e..b5ac1edb6c65 100644
--- a/lib/ovs-router.c
+++ b/lib/ovs-router.c
@@ -345,41 +345,46 @@ ovs_router_add(struct unixctl_conn *conn, int argc,
 struct in6_addr ip6;
 uint32_t mark = 0;
 unsigned int plen;
+ovs_be32 gw = 0;
+bool is_ipv6;
 ovs_be32 ip;
 int err;
+int i;
 
 if (scan_ipv4_route(argv[1], , )) {
-ovs_be32 gw = 0;
-
-if (argc > 3) {
-if (!ovs_scan(argv[3], "pkt_mark=%"SCNi32, ) &&
-!ip_parse(argv[3], )) {
-unixctl_command_reply_error(conn, "Invalid pkt_mark or 
gateway");
-return;
-}
-}
 in6_addr_set_mapped_ipv4(, ip);
-if (gw) {
-in6_addr_set_mapped_ipv4(, gw);
-}
 plen += 96;
+is_ipv6 = false;
 } else if (scan_ipv6_route(argv[1], , )) {
-if (argc > 3) {
-if (!ovs_scan(argv[3], "pkt_mark=%"SCNi32, ) &&
-!ipv6_parse(argv[3], )) {
-unixctl_command_reply_error(conn, "Invalid pkt_mark or IPv6 
gateway");
-return;
-}
-}
+is_ipv6 = true;
 } else {
-unixctl_command_reply_error(conn, "Invalid parameters");
+unixctl_command_reply_error(conn,
+"Invalid 'ip_addr/prefix_len' parameter");
 return;
 }
-if (argc > 4) {
-if (!ovs_scan(argv[4], "pkt_mark=%"SCNi32, )) {
-unixctl_command_reply_error(conn, "Invalid pkt_mark");
-return;
+
+/* Parse optional parameters. */
+for (i = 3; i < argc; i++) {
+if (ovs_scan(argv[i], "pkt_mark=%"SCNi32, )) {
+continue;
 }
+
+if (is_ipv6) {
+if (ipv6_parse(argv[i], )) {
+continue;
+}
+} else {
+if (ip_parse(argv[i], )) {
+continue;
+}
+}
+
+unixctl_command_reply_error(conn, "Invalid pkt_mark or IP gateway");
+return;
+}
+
+if (gw) {
+in6_addr_set_mapped_ipv4(, gw);
 }
 
 err = ovs_router_insert__(mark, plen + 32, false, , plen, argv[2], 
);
diff --git a/tests/ovs-router.at b/tests/ovs-router.at
index 6dacc2954bc6..a36990f1ea1d 100644
--- a/tests/ovs-router.at
+++ b/tests/ovs-router.at
@@ -1,14 +1,33 @@
 AT_BANNER([ovs-router])
 
-AT_SETUP([appctl - route/add with gateway])
+AT_SETUP([appctl - route/add with gateway and pkt_mark])
 AT_KEYWORDS([ovs_router])
-OVS_VSWITCHD_START([add-port br0 p2 -- set Interface p2 type=gre \
-options:local_ip=2.2.2.2 options:remote_ip=1.1.1.1 \
--- add-port br0 p1  -- set interface p1 type=dummy])
+OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy])
 AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 2.2.2.2/24], [0], [OK
 ])
+AT_CHECK([ovs-appctl ovs/route/add 2.2.2.3/32 br0 pkt_mark=1], [0], [OK
+])
 AT_CHECK([ovs-appctl ovs/route/add 1.1.1.0/24 br0 2.2.2.10], [0], [OK
 ])
+AT_CHECK([ovs-appctl ovs/route/add 1.1.2.0/24 br0 2.2.2.10 pkt_mark=2], [0], 
[OK
+])
+AT_CHECK([ovs-appctl ovs/route/add 1.1.3.0/24 br0 pkt_mark=3], [2], [], [dnl
+Error while inserting route.
+ovs-appctl: ovs-vswitchd: server returned an error
+])
+AT_CHECK([ovs-appctl ovs/route/add 1.1.foo.bar/24 br0 2.2.2.10], [2], [], [dnl
+Invalid 'ip_addr/prefix_len' parameter
+ovs-appctl: ovs-vswitchd: server returned an error
+])
+AT_CHECK([ovs-appctl ovs/route/add 2.2.2.4/24 br0 pkt_mark=baz], [2], [], [dnl
+Invalid pkt_mark or IP gateway
+ovs-appctl: ovs-vswitchd: server returned an error
+])
+AT_CHECK([ovs-appctl ovs/route/show | grep User | sort], [0], [dnl
+User: 1.1.1.0/24 dev br0 GW 2.2.2.10 SRC 2.2.2.2
+User: 1.1.2.0/24 MARK 2 dev br0 GW 2.2.2.10 SRC 2.2.2.2
+User: 2.2.2.3/32 MARK 1 dev br0 SRC 2.2.2.2
+])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
-- 
2.31.1

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