The openvswitch selftests currently contain a few cases for managing the
datapath, which includes creating datapath instances, adding interfaces,
and doing some basic feature / upcall tests. This is useful to validate
the control path.
Add the ability to program some of the more common flows with actions. This
can be improved overtime to include regression testing, etc.
Changes from original:
1. Fix issue when parsing ipv6 in the NAT action
2. Fix issue calculating length during ctact parsing
3. Fix error message when invalid bridge is passed
4. Fold in Adrian's patch to support key masks
Aaron Conole (4):
selftests: openvswitch: add an initial flow programming case
selftests: openvswitch: add a test for ipv4 forwarding
selftests: openvswitch: add basic ct test case parsing
selftests: openvswitch: add ct-nat test case with ipv4
Adrian Moreno (1):
selftests: openvswitch: support key masks
.../selftests/net/openvswitch/openvswitch.sh | 223 +++++++
.../selftests/net/openvswitch/ovs-dpctl.py | 601 +++++++++++++++++-
2 files changed, 800 insertions(+), 24 deletions(-)
--
2.40.1
Building on the previous work, add a very simplistic NAT case
using ipv4. This just tests dnat transformation
Signed-off-by: Aaron Conole <[email protected]>
---
.../selftests/net/openvswitch/openvswitch.sh | 64 ++++++++++++++++
.../selftests/net/openvswitch/ovs-dpctl.py | 75 +++++++++++++++++++
2 files changed, 139 insertions(+)
diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index 40a66c72af0f..dced4f612a78 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -14,6 +14,7 @@ tests="
arp_ping eth-arp: Basic arp ping between two NS
ct_connect_v4 ip4-ct-xon: Basic ipv4 tcp connection using ct
connect_v4 ip4-xon: Basic ipv4 ping between two NS
+ nat_connect_v4 ip4-nat-xon: Basic ipv4 tcp connection via NAT
netlink_checks ovsnl: validate netlink attrs and settings
upcall_interfaces ovs: test the upcall interfaces"
@@ -300,6 +301,69 @@ test_connect_v4 () {
return 0
}
+# nat_connect_v4 test
+# - client has 1500 byte MTU
+# - server has 1500 byte MTU
+# - use ICMP to ping in each direction
+# - only allow CT state stuff to pass through new in c -> s
+test_nat_connect_v4 () {
+ which nc >/dev/null 2>/dev/null || return $ksft_skip
+
+ sbx_add "test_nat_connect_v4" || return $?
+
+ ovs_add_dp "test_nat_connect_v4" nat4 || return 1
+ info "create namespaces"
+ for ns in client server; do
+ ovs_add_netns_and_veths "test_nat_connect_v4" "nat4" "$ns" \
+ "${ns:0:1}0" "${ns:0:1}1" || return 1
+ done
+
+ ip netns exec client ip addr add 172.31.110.10/24 dev c1
+ ip netns exec client ip link set c1 up
+ ip netns exec server ip addr add 172.31.110.20/24 dev s1
+ ip netns exec server ip link set s1 up
+
+ ip netns exec client ip route add default via 172.31.110.20
+
+ ovs_add_flow "test_nat_connect_v4" nat4 \
+ 'in_port(1),eth(),eth_type(0x0806),arp()' '2' || return 1
+ ovs_add_flow "test_nat_connect_v4" nat4 \
+ 'in_port(2),eth(),eth_type(0x0806),arp()' '1' || return 1
+ ovs_add_flow "test_nat_connect_v4" nat4 \
+ "ct_state(-trk),in_port(1),eth(),eth_type(0x0800),ipv4(dst=192.168.0.20)" \
+ "ct(commit,nat(dst=172.31.110.20)),recirc(0x1)"
+ ovs_add_flow "test_nat_connect_v4" nat4 \
+ "ct_state(-trk),in_port(2),eth(),eth_type(0x0800),ipv4()" \
+ "ct(commit,nat),recirc(0x2)"
+
+ ovs_add_flow "test_nat_connect_v4" nat4 \
+ "recirc_id(0x1),ct_state(+trk-inv),in_port(1),eth(),eth_type(0x0800),ipv4()" "2"
+ ovs_add_flow "test_nat_connect_v4" nat4 \
+ "recirc_id(0x2),ct_state(+trk-inv),in_port(2),eth(),eth_type(0x0800),ipv4()" "1"
+
+ # do a ping
+ ovs_sbx "test_nat_connect_v4" ip netns exec client ping 192.168.0.20 -c 3 || return 1
+
+ # create an echo server in 'server'
+ echo "server" | \
+ ovs_netns_spawn_daemon "test_nat_connect_v4" "server" \
+ nc -lvnp 4443
+ ovs_sbx "test_nat_connect_v4" ip netns exec client nc -i 1 -zv 192.168.0.20 4443 || return 1
+
+ # Now test in the other direction (should fail)
+ echo "client" | \
+ ovs_netns_spawn_daemon "test_nat_connect_v4" "client" \
+ nc -lvnp 4443
+ ovs_sbx "test_nat_connect_v4" ip netns exec client nc -i 1 -zv 172.31.110.10 4443
+ if [ $? == 0 ]; then
+ info "connect to client was successful"
+ return 1
+ fi
+
+ info "done..."
+ return 0
+}
+
# netlink_validation
# - Create a dp
# - check no warning with "old version" simulation
diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
index 6e258ab9e635..258c9ef263d9 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -530,6 +530,81 @@ class ovsactions(nla):
else:
ctact["attrs"].append([scan[1], None])
actstr = actstr[strspn(actstr, ", ") :]
+ # it seems strange to put this here, but nat() is a complex
+ # sub-action and this lets it sit anywhere in the ct() action
+ if actstr.startswith("nat"):
+ actstr = actstr[3:]
+ natact = ovsactions.ctact.natattr()
+
+ if actstr.startswith("("):
+ t = None
+ actstr = actstr[1:]
+ if actstr.startswith("src"):
+ t = "OVS_NAT_ATTR_SRC"
+ actstr = actstr[3:]
+ elif actstr.startswith("dst"):
+ t = "OVS_NAT_ATTR_DST"
+ actstr = actstr[3:]
+
+ actstr, ip_block_min = parse_extract_field(
+ actstr, "=", "([0-9a-fA-F:\.\[]+)", str, False
+ )
+ actstr, ip_block_max = parse_extract_field(
+ actstr, "-", "([0-9a-fA-F:\.\[]+)", str, False
+ )
+
+ # [XXXX:YYY::Z]:123
+ # following RFC 3986
+ # More complete parsing, ala RFC5952 isn't
+ # supported.
+ if actstr.startswith("]"):
+ actstr = actstr[1:]
+ if ip_block_min is not None and \
+ ip_block_min.startswith("["):
+ ip_block_min = ip_block_min[1:]
+ if ip_block_max is not None and \
+ ip_block_max.startswith("["):
+ ip_block_max = ip_block_max[1:]
+
+ actstr, proto_min = parse_extract_field(
+ actstr, ":", "(\d+)", int, False
+ )
+ actstr, proto_max = parse_extract_field(
+ actstr, "-", "(\d+)", int, False
+ )
+
+ if t is not None:
+ natact["attrs"].append([t, None])
+
+ if ip_block_min is not None:
+ natact["attrs"].append(
+ ["OVS_NAT_ATTR_IP_MIN", ip_block_min]
+ )
+ if ip_block_max is not None:
+ natact["attrs"].append(
+ ["OVS_NAT_ATTR_IP_MAX", ip_block_max]
+ )
+ if proto_min is not None:
+ natact["attrs"].append(
+ ["OVS_NAT_ATTR_PROTO_MIN", proto_min]
+ )
+ if proto_max is not None:
+ natact["attrs"].append(
+ ["OVS_NAT_ATTR_PROTO_MAX", proto_max]
+ )
+
+ for natscan in (
+ ("persist", "OVS_NAT_ATTR_PERSISTENT"),
+ ("hash", "OVS_NAT_ATTR_PROTO_HASH"),
+ ("random", "OVS_NAT_ATTR_PROTO_RANDOM"),
+ ):
+ if actstr.startswith(natscan[0]):
+ actstr = actstr[len(natscan[0]) :]
+ natact["attrs"].append([natscan[1], None])
+ actstr = actstr[strspn(actstr, ", ") :]
+
+ ctact["attrs"].append(["OVS_CT_ATTR_NAT", natact])
+ actstr = actstr[strspn(actstr, ",) ") :]
self["attrs"].append(["OVS_ACTION_ATTR_CT", ctact])
parsed = True
--
2.40.1
Forwarding via ct() action is an important use case for openvswitch, but
generally would require using a full ovs-vswitchd to get working. Add a
ct action parser for basic ct test case.
Signed-off-by: Aaron Conole <[email protected]>
---
NOTE: 3 lines flag the line-length checkpatch warning, but there didnt
seem to be a really good way of breaking the lines smaller.
.../selftests/net/openvswitch/openvswitch.sh | 68 +++++++++++++++++++
.../selftests/net/openvswitch/ovs-dpctl.py | 39 +++++++++++
2 files changed, 107 insertions(+)
diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index 5d60a9466dab..40a66c72af0f 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -12,6 +12,7 @@ TRACING=0
tests="
arp_ping eth-arp: Basic arp ping between two NS
+ ct_connect_v4 ip4-ct-xon: Basic ipv4 tcp connection using ct
connect_v4 ip4-xon: Basic ipv4 ping between two NS
netlink_checks ovsnl: validate netlink attrs and settings
upcall_interfaces ovs: test the upcall interfaces"
@@ -193,6 +194,73 @@ test_arp_ping () {
return 0
}
+# ct_connect_v4 test
+# - client has 1500 byte MTU
+# - server has 1500 byte MTU
+# - use ICMP to ping in each direction
+# - only allow CT state stuff to pass through new in c -> s
+test_ct_connect_v4 () {
+
+ which nc >/dev/null 2>/dev/null || return $ksft_skip
+
+ sbx_add "test_ct_connect_v4" || return $?
+
+ ovs_add_dp "test_ct_connect_v4" ct4 || return 1
+ info "create namespaces"
+ for ns in client server; do
+ ovs_add_netns_and_veths "test_ct_connect_v4" "ct4" "$ns" \
+ "${ns:0:1}0" "${ns:0:1}1" || return 1
+ done
+
+ ip netns exec client ip addr add 172.31.110.10/24 dev c1
+ ip netns exec client ip link set c1 up
+ ip netns exec server ip addr add 172.31.110.20/24 dev s1
+ ip netns exec server ip link set s1 up
+
+ # Add forwarding for ARP and ip packets - completely wildcarded
+ ovs_add_flow "test_ct_connect_v4" ct4 \
+ 'in_port(1),eth(),eth_type(0x0806),arp()' '2' || return 1
+ ovs_add_flow "test_ct_connect_v4" ct4 \
+ 'in_port(2),eth(),eth_type(0x0806),arp()' '1' || return 1
+ ovs_add_flow "test_ct_connect_v4" ct4 \
+ 'ct_state(-trk),eth(),eth_type(0x0800),ipv4()' \
+ 'ct(commit),recirc(0x1)' || return 1
+ ovs_add_flow "test_ct_connect_v4" ct4 \
+ 'recirc_id(0x1),ct_state(+trk+new),in_port(1),eth(),eth_type(0x0800),ipv4(src=172.31.110.10)' \
+ '2' || return 1
+ ovs_add_flow "test_ct_connect_v4" ct4 \
+ 'recirc_id(0x1),ct_state(+trk+est),in_port(1),eth(),eth_type(0x0800),ipv4(src=172.31.110.10)' \
+ '2' || return 1
+ ovs_add_flow "test_ct_connect_v4" ct4 \
+ 'recirc_id(0x1),ct_state(+trk+est),in_port(2),eth(),eth_type(0x0800),ipv4(dst=172.31.110.10)' \
+ '1' || return 1
+ ovs_add_flow "test_ct_connect_v4" ct4 \
+ 'recirc_id(0x1),ct_state(+trk+inv),eth(),eth_type(0x0800),ipv4()' 'drop' || \
+ return 1
+
+ # do a ping
+ ovs_sbx "test_ct_connect_v4" ip netns exec client ping 172.31.110.20 -c 3 || return 1
+
+ # create an echo server in 'server'
+ echo "server" | \
+ ovs_netns_spawn_daemon "test_ct_connect_v4" "server" \
+ nc -lvnp 4443
+ ovs_sbx "test_ct_connect_v4" ip netns exec client nc -i 1 -zv 172.31.110.20 4443 || return 1
+
+ # Now test in the other direction (should fail)
+ echo "client" | \
+ ovs_netns_spawn_daemon "test_ct_connect_v4" "client" \
+ nc -lvnp 4443
+ ovs_sbx "test_ct_connect_v4" ip netns exec client nc -i 1 -zv 172.31.110.10 4443
+ if [ $? == 0 ]; then
+ info "ct connect to client was successful"
+ return 1
+ fi
+
+ info "done..."
+ return 0
+}
+
# connect_v4 test
# - client has 1500 byte MTU
# - server has 1500 byte MTU
diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
index 2b869e89c51d..6e258ab9e635 100644
--- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
+++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
@@ -62,6 +62,15 @@ def macstr(mac):
return outstr
+def strcspn(str1, str2):
+ tot = 0
+ for char in str1:
+ if str2.find(char) != -1:
+ return tot
+ tot += 1
+ return tot
+
+
def strspn(str1, str2):
tot = 0
for char in str1:
@@ -496,6 +505,36 @@ class ovsactions(nla):
actstr = actstr[strspn(actstr, ", ") :]
parsed = True
+ if parse_starts_block(actstr, "ct(", False):
+ actstr = actstr[len("ct(") :]
+ ctact = ovsactions.ctact()
+
+ for scan in (
+ ("commit", "OVS_CT_ATTR_COMMIT", None),
+ ("force_commit", "OVS_CT_ATTR_FORCE_COMMIT", None),
+ ("zone", "OVS_CT_ATTR_ZONE", int),
+ ("mark", "OVS_CT_ATTR_MARK", int),
+ ("helper", "OVS_CT_ATTR_HELPER", lambda x, y: str(x)),
+ ("timeout", "OVS_CT_ATTR_TIMEOUT", lambda x, y: str(x)),
+ ):
+ if actstr.startswith(scan[0]):
+ actstr = actstr[len(scan[0]) :]
+ if scan[2] is not None:
+ if actstr[0] != "=":
+ raise ValueError("Invalid ct attr")
+ actstr = actstr[1:]
+ pos = strcspn(actstr, ",)")
+ datum = scan[2](actstr[:pos], 0)
+ ctact["attrs"].append([scan[1], datum])
+ actstr = actstr[pos:]
+ else:
+ ctact["attrs"].append([scan[1], None])
+ actstr = actstr[strspn(actstr, ", ") :]
+
+ self["attrs"].append(["OVS_ACTION_ATTR_CT", ctact])
+ parsed = True
+
+ actstr = actstr[strspn(actstr, "), ") :]
if not parsed:
raise ValueError("Action str: '%s' not supported" % actstr)
--
2.40.1
On 7/28/23 13:59, Aaron Conole wrote:
> Building on the previous work, add a very simplistic NAT case
> using ipv4. This just tests dnat transformation
>
> Signed-off-by: Aaron Conole <[email protected]>
> ---
> .../selftests/net/openvswitch/openvswitch.sh | 64 ++++++++++++++++
> .../selftests/net/openvswitch/ovs-dpctl.py | 75 +++++++++++++++++++
> 2 files changed, 139 insertions(+)
>
> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> index 40a66c72af0f..dced4f612a78 100755
> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> @@ -14,6 +14,7 @@ tests="
> arp_ping eth-arp: Basic arp ping between two NS
> ct_connect_v4 ip4-ct-xon: Basic ipv4 tcp connection using ct
> connect_v4 ip4-xon: Basic ipv4 ping between two NS
> + nat_connect_v4 ip4-nat-xon: Basic ipv4 tcp connection via NAT
> netlink_checks ovsnl: validate netlink attrs and settings
> upcall_interfaces ovs: test the upcall interfaces"
>
> @@ -300,6 +301,69 @@ test_connect_v4 () {
> return 0
> }
>
> +# nat_connect_v4 test
> +# - client has 1500 byte MTU
> +# - server has 1500 byte MTU
> +# - use ICMP to ping in each direction
> +# - only allow CT state stuff to pass through new in c -> s
> +test_nat_connect_v4 () {
> + which nc >/dev/null 2>/dev/null || return $ksft_skip
> +
> + sbx_add "test_nat_connect_v4" || return $?
> +
> + ovs_add_dp "test_nat_connect_v4" nat4 || return 1
> + info "create namespaces"
> + for ns in client server; do
> + ovs_add_netns_and_veths "test_nat_connect_v4" "nat4" "$ns" \
> + "${ns:0:1}0" "${ns:0:1}1" || return 1
> + done
> +
> + ip netns exec client ip addr add 172.31.110.10/24 dev c1
> + ip netns exec client ip link set c1 up
> + ip netns exec server ip addr add 172.31.110.20/24 dev s1
> + ip netns exec server ip link set s1 up
> +
> + ip netns exec client ip route add default via 172.31.110.20
> +
> + ovs_add_flow "test_nat_connect_v4" nat4 \
> + 'in_port(1),eth(),eth_type(0x0806),arp()' '2' || return 1
> + ovs_add_flow "test_nat_connect_v4" nat4 \
> + 'in_port(2),eth(),eth_type(0x0806),arp()' '1' || return 1
> + ovs_add_flow "test_nat_connect_v4" nat4 \
> + "ct_state(-trk),in_port(1),eth(),eth_type(0x0800),ipv4(dst=192.168.0.20)" \
> + "ct(commit,nat(dst=172.31.110.20)),recirc(0x1)"
> + ovs_add_flow "test_nat_connect_v4" nat4 \
> + "ct_state(-trk),in_port(2),eth(),eth_type(0x0800),ipv4()" \
> + "ct(commit,nat),recirc(0x2)"
> +
> + ovs_add_flow "test_nat_connect_v4" nat4 \
> + "recirc_id(0x1),ct_state(+trk-inv),in_port(1),eth(),eth_type(0x0800),ipv4()" "2"
> + ovs_add_flow "test_nat_connect_v4" nat4 \
> + "recirc_id(0x2),ct_state(+trk-inv),in_port(2),eth(),eth_type(0x0800),ipv4()" "1"
> +
> + # do a ping
> + ovs_sbx "test_nat_connect_v4" ip netns exec client ping 192.168.0.20 -c 3 || return 1
> +
> + # create an echo server in 'server'
> + echo "server" | \
> + ovs_netns_spawn_daemon "test_nat_connect_v4" "server" \
> + nc -lvnp 4443
> + ovs_sbx "test_nat_connect_v4" ip netns exec client nc -i 1 -zv 192.168.0.20 4443 || return 1
> +
> + # Now test in the other direction (should fail)
> + echo "client" | \
> + ovs_netns_spawn_daemon "test_nat_connect_v4" "client" \
> + nc -lvnp 4443
> + ovs_sbx "test_nat_connect_v4" ip netns exec client nc -i 1 -zv 172.31.110.10 4443
> + if [ $? == 0 ]; then
> + info "connect to client was successful"
> + return 1
> + fi
> +
> + info "done..."
> + return 0
> +}
> +
> # netlink_validation
> # - Create a dp
> # - check no warning with "old version" simulation
> diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> index 6e258ab9e635..258c9ef263d9 100644
> --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> @@ -530,6 +530,81 @@ class ovsactions(nla):
> else:
> ctact["attrs"].append([scan[1], None])
> actstr = actstr[strspn(actstr, ", ") :]
> + # it seems strange to put this here, but nat() is a complex
> + # sub-action and this lets it sit anywhere in the ct() action
> + if actstr.startswith("nat"):
> + actstr = actstr[3:]
> + natact = ovsactions.ctact.natattr()
> +
> + if actstr.startswith("("):
> + t = None
> + actstr = actstr[1:]
> + if actstr.startswith("src"):
> + t = "OVS_NAT_ATTR_SRC"
> + actstr = actstr[3:]
> + elif actstr.startswith("dst"):
> + t = "OVS_NAT_ATTR_DST"
> + actstr = actstr[3:]
> +
> + actstr, ip_block_min = parse_extract_field(
> + actstr, "=", "([0-9a-fA-F:\.\[]+)", str, False
> + )
> + actstr, ip_block_max = parse_extract_field(
> + actstr, "-", "([0-9a-fA-F:\.\[]+)", str, False
> + )
> +
> + # [XXXX:YYY::Z]:123
> + # following RFC 3986
> + # More complete parsing, ala RFC5952 isn't
> + # supported.
> + if actstr.startswith("]"):
> + actstr = actstr[1:]
> + if ip_block_min is not None and \
> + ip_block_min.startswith("["):
> + ip_block_min = ip_block_min[1:]
> + if ip_block_max is not None and \
> + ip_block_max.startswith("["):
> + ip_block_max = ip_block_max[1:]
> +
> + actstr, proto_min = parse_extract_field(
> + actstr, ":", "(\d+)", int, False
> + )
> + actstr, proto_max = parse_extract_field(
> + actstr, "-", "(\d+)", int, False
> + )
I'm still struggling to make this part work:
On the one hand, ipv6 seems not fully supported by ovs-dpctl.py. If I try adding
an ipv6 flow I end up needing to add a function such as as the following and use
it to parse "ipv6()" field:
def convert_ipv6(data):
ip, _, mask = data.partition('/')
max_ip = pow(2, 128) - 1
if not ip:
ip = mask = 0
elif not mask:
mask = max
elif mask.isdigit():
mask = (max_ip << (128 - int(mask))) & max_ip
return ipaddress.IPv6Address(ip).packed, ipaddress.IPv6Address(mask).packed
OTOH, trying to support ipv6 makes ct ip/port range parsing more complex, for
instance, this action: "ct(nat(src=10.0.0.240-10.0.0.254:32768-65535))"
fails, because it's parsed as:
ip_block_min = 10.0.0.240
ip_block_max = 10.0.0.254:32768
proto_min = None
proto_max = 65535
I would say we could drop ipv6 support for nat() action, making it simpler to
parse or first detect if we're parsing ipv4 or ipv6 and use appropriate regexp
on each case. E.g:
https://github.com/openvswitch/ovs/blob/d460c473ebf9e9ab16da44cbfbb13a4911352195/python/ovs/flow/decoders.py#L430-L486
Another approach would be to stop trying to be human friendly and use an easier
to parse syntax, something closer to key-value, e.g:
"ct(ip_block_min=10.0.0.240, ip_block_max=10.0.0.254, proto_min=32768,
proto_max=65535)". It'd be more verbose and not compatible with ovs tooling but
this is a testing tool afterall. WDYT?
> +
> + if t is not None:
> + natact["attrs"].append([t, None])
> +
> + if ip_block_min is not None:
> + natact["attrs"].append(
> + ["OVS_NAT_ATTR_IP_MIN", ip_block_min]
> + )
> + if ip_block_max is not None:
> + natact["attrs"].append(
> + ["OVS_NAT_ATTR_IP_MAX", ip_block_max]
> + )
> + if proto_min is not None:
> + natact["attrs"].append(
> + ["OVS_NAT_ATTR_PROTO_MIN", proto_min]
> + )
> + if proto_max is not None:
> + natact["attrs"].append(
> + ["OVS_NAT_ATTR_PROTO_MAX", proto_max]
> + )
> +
> + for natscan in (
> + ("persist", "OVS_NAT_ATTR_PERSISTENT"),
odp-util.c defines this flag as "persistent", not sure if you intend to keep it
compatible at all.
> + ("hash", "OVS_NAT_ATTR_PROTO_HASH"),
> + ("random", "OVS_NAT_ATTR_PROTO_RANDOM"),
> + ):
> + if actstr.startswith(natscan[0]):
> + actstr = actstr[len(natscan[0]) :]
> + natact["attrs"].append([natscan[1], None])
> + actstr = actstr[strspn(actstr, ", ") :]
> +
> + ctact["attrs"].append(["OVS_CT_ATTR_NAT", natact])
> + actstr = actstr[strspn(actstr, ",) ") :]
>
> self["attrs"].append(["OVS_ACTION_ATTR_CT", ctact])
> parsed = True
--
Adrián Moreno
On 7/28/23 13:59, Aaron Conole wrote:
> Forwarding via ct() action is an important use case for openvswitch, but
> generally would require using a full ovs-vswitchd to get working. Add a
> ct action parser for basic ct test case.
>
> Signed-off-by: Aaron Conole <[email protected]>
Reviewed-by: Adrian Moreno <[email protected]>
> ---
> NOTE: 3 lines flag the line-length checkpatch warning, but there didnt
> seem to be a really good way of breaking the lines smaller.
>
> .../selftests/net/openvswitch/openvswitch.sh | 68 +++++++++++++++++++
> .../selftests/net/openvswitch/ovs-dpctl.py | 39 +++++++++++
> 2 files changed, 107 insertions(+)
>
> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> index 5d60a9466dab..40a66c72af0f 100755
> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
> @@ -12,6 +12,7 @@ TRACING=0
>
> tests="
> arp_ping eth-arp: Basic arp ping between two NS
> + ct_connect_v4 ip4-ct-xon: Basic ipv4 tcp connection using ct
> connect_v4 ip4-xon: Basic ipv4 ping between two NS
> netlink_checks ovsnl: validate netlink attrs and settings
> upcall_interfaces ovs: test the upcall interfaces"
> @@ -193,6 +194,73 @@ test_arp_ping () {
> return 0
> }
>
> +# ct_connect_v4 test
> +# - client has 1500 byte MTU
> +# - server has 1500 byte MTU
> +# - use ICMP to ping in each direction
> +# - only allow CT state stuff to pass through new in c -> s
> +test_ct_connect_v4 () {
> +
> + which nc >/dev/null 2>/dev/null || return $ksft_skip
> +
> + sbx_add "test_ct_connect_v4" || return $?
> +
> + ovs_add_dp "test_ct_connect_v4" ct4 || return 1
> + info "create namespaces"
> + for ns in client server; do
> + ovs_add_netns_and_veths "test_ct_connect_v4" "ct4" "$ns" \
> + "${ns:0:1}0" "${ns:0:1}1" || return 1
> + done
> +
> + ip netns exec client ip addr add 172.31.110.10/24 dev c1
> + ip netns exec client ip link set c1 up
> + ip netns exec server ip addr add 172.31.110.20/24 dev s1
> + ip netns exec server ip link set s1 up
> +
> + # Add forwarding for ARP and ip packets - completely wildcarded
> + ovs_add_flow "test_ct_connect_v4" ct4 \
> + 'in_port(1),eth(),eth_type(0x0806),arp()' '2' || return 1
> + ovs_add_flow "test_ct_connect_v4" ct4 \
> + 'in_port(2),eth(),eth_type(0x0806),arp()' '1' || return 1
> + ovs_add_flow "test_ct_connect_v4" ct4 \
> + 'ct_state(-trk),eth(),eth_type(0x0800),ipv4()' \
> + 'ct(commit),recirc(0x1)' || return 1
> + ovs_add_flow "test_ct_connect_v4" ct4 \
> + 'recirc_id(0x1),ct_state(+trk+new),in_port(1),eth(),eth_type(0x0800),ipv4(src=172.31.110.10)' \
> + '2' || return 1
> + ovs_add_flow "test_ct_connect_v4" ct4 \
> + 'recirc_id(0x1),ct_state(+trk+est),in_port(1),eth(),eth_type(0x0800),ipv4(src=172.31.110.10)' \
> + '2' || return 1
> + ovs_add_flow "test_ct_connect_v4" ct4 \
> + 'recirc_id(0x1),ct_state(+trk+est),in_port(2),eth(),eth_type(0x0800),ipv4(dst=172.31.110.10)' \
> + '1' || return 1
> + ovs_add_flow "test_ct_connect_v4" ct4 \
> + 'recirc_id(0x1),ct_state(+trk+inv),eth(),eth_type(0x0800),ipv4()' 'drop' || \
> + return 1
> +
> + # do a ping
> + ovs_sbx "test_ct_connect_v4" ip netns exec client ping 172.31.110.20 -c 3 || return 1
> +
> + # create an echo server in 'server'
> + echo "server" | \
> + ovs_netns_spawn_daemon "test_ct_connect_v4" "server" \
> + nc -lvnp 4443
> + ovs_sbx "test_ct_connect_v4" ip netns exec client nc -i 1 -zv 172.31.110.20 4443 || return 1
> +
> + # Now test in the other direction (should fail)
> + echo "client" | \
> + ovs_netns_spawn_daemon "test_ct_connect_v4" "client" \
> + nc -lvnp 4443
> + ovs_sbx "test_ct_connect_v4" ip netns exec client nc -i 1 -zv 172.31.110.10 4443
> + if [ $? == 0 ]; then
> + info "ct connect to client was successful"
> + return 1
> + fi
> +
> + info "done..."
> + return 0
> +}
> +
> # connect_v4 test
> # - client has 1500 byte MTU
> # - server has 1500 byte MTU
> diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> index 2b869e89c51d..6e258ab9e635 100644
> --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
> @@ -62,6 +62,15 @@ def macstr(mac):
> return outstr
>
>
> +def strcspn(str1, str2):
> + tot = 0
> + for char in str1:
> + if str2.find(char) != -1:
> + return tot
> + tot += 1
> + return tot
> +
> +
> def strspn(str1, str2):
> tot = 0
> for char in str1:
> @@ -496,6 +505,36 @@ class ovsactions(nla):
> actstr = actstr[strspn(actstr, ", ") :]
> parsed = True
>
> + if parse_starts_block(actstr, "ct(", False):
> + actstr = actstr[len("ct(") :]
> + ctact = ovsactions.ctact()
> +
> + for scan in (
> + ("commit", "OVS_CT_ATTR_COMMIT", None),
> + ("force_commit", "OVS_CT_ATTR_FORCE_COMMIT", None),
> + ("zone", "OVS_CT_ATTR_ZONE", int),
> + ("mark", "OVS_CT_ATTR_MARK", int),
> + ("helper", "OVS_CT_ATTR_HELPER", lambda x, y: str(x)),
> + ("timeout", "OVS_CT_ATTR_TIMEOUT", lambda x, y: str(x)),
> + ):
> + if actstr.startswith(scan[0]):
> + actstr = actstr[len(scan[0]) :]
> + if scan[2] is not None:
> + if actstr[0] != "=":
> + raise ValueError("Invalid ct attr")
> + actstr = actstr[1:]
> + pos = strcspn(actstr, ",)")
> + datum = scan[2](actstr[:pos], 0)
> + ctact["attrs"].append([scan[1], datum])
> + actstr = actstr[pos:]
> + else:
> + ctact["attrs"].append([scan[1], None])
> + actstr = actstr[strspn(actstr, ", ") :]
> +
> + self["attrs"].append(["OVS_ACTION_ATTR_CT", ctact])
> + parsed = True
> +
> + actstr = actstr[strspn(actstr, "), ") :]
> if not parsed:
> raise ValueError("Action str: '%s' not supported" % actstr)
>
--
Adrián Moreno
Adrian Moreno <[email protected]> writes:
> On 7/28/23 13:59, Aaron Conole wrote:
>> Building on the previous work, add a very simplistic NAT case
>> using ipv4. This just tests dnat transformation
>> Signed-off-by: Aaron Conole <[email protected]>
>> ---
>> .../selftests/net/openvswitch/openvswitch.sh | 64 ++++++++++++++++
>> .../selftests/net/openvswitch/ovs-dpctl.py | 75 +++++++++++++++++++
>> 2 files changed, 139 insertions(+)
>> diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh
>> b/tools/testing/selftests/net/openvswitch/openvswitch.sh
>> index 40a66c72af0f..dced4f612a78 100755
>> --- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
>> +++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
>> @@ -14,6 +14,7 @@ tests="
>> arp_ping eth-arp: Basic arp ping between two NS
>> ct_connect_v4 ip4-ct-xon: Basic ipv4 tcp connection using ct
>> connect_v4 ip4-xon: Basic ipv4 ping between two NS
>> + nat_connect_v4 ip4-nat-xon: Basic ipv4 tcp connection via NAT
>> netlink_checks ovsnl: validate netlink attrs and settings
>> upcall_interfaces ovs: test the upcall interfaces"
>> @@ -300,6 +301,69 @@ test_connect_v4 () {
>> return 0
>> }
>> +# nat_connect_v4 test
>> +# - client has 1500 byte MTU
>> +# - server has 1500 byte MTU
>> +# - use ICMP to ping in each direction
>> +# - only allow CT state stuff to pass through new in c -> s
>> +test_nat_connect_v4 () {
>> + which nc >/dev/null 2>/dev/null || return $ksft_skip
>> +
>> + sbx_add "test_nat_connect_v4" || return $?
>> +
>> + ovs_add_dp "test_nat_connect_v4" nat4 || return 1
>> + info "create namespaces"
>> + for ns in client server; do
>> + ovs_add_netns_and_veths "test_nat_connect_v4" "nat4" "$ns" \
>> + "${ns:0:1}0" "${ns:0:1}1" || return 1
>> + done
>> +
>> + ip netns exec client ip addr add 172.31.110.10/24 dev c1
>> + ip netns exec client ip link set c1 up
>> + ip netns exec server ip addr add 172.31.110.20/24 dev s1
>> + ip netns exec server ip link set s1 up
>> +
>> + ip netns exec client ip route add default via 172.31.110.20
>> +
>> + ovs_add_flow "test_nat_connect_v4" nat4 \
>> + 'in_port(1),eth(),eth_type(0x0806),arp()' '2' || return 1
>> + ovs_add_flow "test_nat_connect_v4" nat4 \
>> + 'in_port(2),eth(),eth_type(0x0806),arp()' '1' || return 1
>> + ovs_add_flow "test_nat_connect_v4" nat4 \
>> + "ct_state(-trk),in_port(1),eth(),eth_type(0x0800),ipv4(dst=192.168.0.20)" \
>> + "ct(commit,nat(dst=172.31.110.20)),recirc(0x1)"
>> + ovs_add_flow "test_nat_connect_v4" nat4 \
>> + "ct_state(-trk),in_port(2),eth(),eth_type(0x0800),ipv4()" \
>> + "ct(commit,nat),recirc(0x2)"
>> +
>> + ovs_add_flow "test_nat_connect_v4" nat4 \
>> + "recirc_id(0x1),ct_state(+trk-inv),in_port(1),eth(),eth_type(0x0800),ipv4()" "2"
>> + ovs_add_flow "test_nat_connect_v4" nat4 \
>> + "recirc_id(0x2),ct_state(+trk-inv),in_port(2),eth(),eth_type(0x0800),ipv4()" "1"
>> +
>> + # do a ping
>> + ovs_sbx "test_nat_connect_v4" ip netns exec client ping 192.168.0.20 -c 3 || return 1
>> +
>> + # create an echo server in 'server'
>> + echo "server" | \
>> + ovs_netns_spawn_daemon "test_nat_connect_v4" "server" \
>> + nc -lvnp 4443
>> + ovs_sbx "test_nat_connect_v4" ip netns exec client nc -i 1 -zv 192.168.0.20 4443 || return 1
>> +
>> + # Now test in the other direction (should fail)
>> + echo "client" | \
>> + ovs_netns_spawn_daemon "test_nat_connect_v4" "client" \
>> + nc -lvnp 4443
>> + ovs_sbx "test_nat_connect_v4" ip netns exec client nc -i 1 -zv 172.31.110.10 4443
>> + if [ $? == 0 ]; then
>> + info "connect to client was successful"
>> + return 1
>> + fi
>> +
>> + info "done..."
>> + return 0
>> +}
>> +
>> # netlink_validation
>> # - Create a dp
>> # - check no warning with "old version" simulation
>> diff --git a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
>> index 6e258ab9e635..258c9ef263d9 100644
>> --- a/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
>> +++ b/tools/testing/selftests/net/openvswitch/ovs-dpctl.py
>> @@ -530,6 +530,81 @@ class ovsactions(nla):
>> else:
>> ctact["attrs"].append([scan[1], None])
>> actstr = actstr[strspn(actstr, ", ") :]
>> + # it seems strange to put this here, but nat() is a complex
>> + # sub-action and this lets it sit anywhere in the ct() action
>> + if actstr.startswith("nat"):
>> + actstr = actstr[3:]
>> + natact = ovsactions.ctact.natattr()
>> +
>> + if actstr.startswith("("):
>> + t = None
>> + actstr = actstr[1:]
>> + if actstr.startswith("src"):
>> + t = "OVS_NAT_ATTR_SRC"
>> + actstr = actstr[3:]
>> + elif actstr.startswith("dst"):
>> + t = "OVS_NAT_ATTR_DST"
>> + actstr = actstr[3:]
>> +
>> + actstr, ip_block_min = parse_extract_field(
>> + actstr, "=", "([0-9a-fA-F:\.\[]+)", str, False
>> + )
>> + actstr, ip_block_max = parse_extract_field(
>> + actstr, "-", "([0-9a-fA-F:\.\[]+)", str, False
>> + )
>> +
>> + # [XXXX:YYY::Z]:123
>> + # following RFC 3986
>> + # More complete parsing, ala RFC5952 isn't
>> + # supported.
>> + if actstr.startswith("]"):
>> + actstr = actstr[1:]
>> + if ip_block_min is not None and \
>> + ip_block_min.startswith("["):
>> + ip_block_min = ip_block_min[1:]
>> + if ip_block_max is not None and \
>> + ip_block_max.startswith("["):
>> + ip_block_max = ip_block_max[1:]
>> +
>> + actstr, proto_min = parse_extract_field(
>> + actstr, ":", "(\d+)", int, False
>> + )
>> + actstr, proto_max = parse_extract_field(
>> + actstr, "-", "(\d+)", int, False
>> + )
>
> I'm still struggling to make this part work:
> On the one hand, ipv6 seems not fully supported by ovs-dpctl.py. If I
> try adding an ipv6 flow I end up needing to add a function such as as
> the following and use it to parse "ipv6()" field:
Let's just drop the ipv6 stuff from the NAT action support, and we can
add it later when I address the flow key side of it.
> def convert_ipv6(data):
> ip, _, mask = data.partition('/')
> max_ip = pow(2, 128) - 1
>
> if not ip:
> ip = mask = 0
> elif not mask:
> mask = max
> elif mask.isdigit():
> mask = (max_ip << (128 - int(mask))) & max_ip
>
> return ipaddress.IPv6Address(ip).packed, ipaddress.IPv6Address(mask).packed
>
> OTOH, trying to support ipv6 makes ct ip/port range parsing more
> complex, for instance, this action:
> "ct(nat(src=10.0.0.240-10.0.0.254:32768-65535))"
>
> fails, because it's parsed as:
> ip_block_min = 10.0.0.240
> ip_block_max = 10.0.0.254:32768
> proto_min = None
> proto_max = 65535
>
> I would say we could drop ipv6 support for nat() action, making it
> simpler to parse or first detect if we're parsing ipv4 or ipv6 and use
> appropriate regexp on each case. E.g:
> https://github.com/openvswitch/ovs/blob/d460c473ebf9e9ab16da44cbfbb13a4911352195/python/ovs/flow/decoders.py#L430-L486
ACK
> Another approach would be to stop trying to be human friendly and use
> an easier to parse syntax, something closer to key-value, e.g:
> "ct(ip_block_min=10.0.0.240, ip_block_max=10.0.0.254, proto_min=32768,
> proto_max=65535)". It'd be more verbose and not compatible with ovs
> tooling but this is a testing tool afterall. WDYT?
I do like that we can use the flow output from the running ovs-vswitchd
to generate test cases. I'd like, as much as possible, to try and keep
it compatible.
>> +
>> + if t is not None:
>> + natact["attrs"].append([t, None])
>> +
>> + if ip_block_min is not None:
>> + natact["attrs"].append(
>> + ["OVS_NAT_ATTR_IP_MIN", ip_block_min]
>> + )
>> + if ip_block_max is not None:
>> + natact["attrs"].append(
>> + ["OVS_NAT_ATTR_IP_MAX", ip_block_max]
>> + )
>> + if proto_min is not None:
>> + natact["attrs"].append(
>> + ["OVS_NAT_ATTR_PROTO_MIN", proto_min]
>> + )
>> + if proto_max is not None:
>> + natact["attrs"].append(
>> + ["OVS_NAT_ATTR_PROTO_MAX", proto_max]
>> + )
>> +
>> + for natscan in (
>> + ("persist", "OVS_NAT_ATTR_PERSISTENT"),
>
> odp-util.c defines this flag as "persistent", not sure if you intend
> to keep it compatible at all.
I will adjust when I resubmit.
>> + ("hash", "OVS_NAT_ATTR_PROTO_HASH"),
>> + ("random", "OVS_NAT_ATTR_PROTO_RANDOM"),
>> + ):
>> + if actstr.startswith(natscan[0]):
>> + actstr = actstr[len(natscan[0]) :]
>> + natact["attrs"].append([natscan[1], None])
>> + actstr = actstr[strspn(actstr, ", ") :]
>> +
>> + ctact["attrs"].append(["OVS_CT_ATTR_NAT", natact])
>> + actstr = actstr[strspn(actstr, ",) ") :]
>> self["attrs"].append(["OVS_ACTION_ATTR_CT",
>> ctact])
>> parsed = True