| Message ID | 20231019112243.2421-3-viacheslav.galaktionov@arknetworks.am |
|---|---|
| State | Superseded |
| Delegated to: | aaron conole |
| Headers | show |
| Series | [ovs-dev,v3,1/3] lib/conntrack: Only use given packet in protocol detection. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
Viacheslav Galaktionov via dev <ovs-dev@openvswitch.org> writes: > All existing test iterations assume that the FTP server is running on a > standard port, which may not always be the case. These tests helped find > problems in conntrack alg processing with non-standard ports. > > Perform the necessary adjustments to ensure the test suite can start the > L7 server on a user-provided port. > > Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am> > --- Hi Viacheslav, Thanks for the work! I did some basic testing with this - I actually really appreciate the work for this. I had a private branch adding non-standard port support but wasn't able to finish it. I think it needs a NEWS entry - this is a user visible change related to how flows with handlers are processed. We also should update the documentation in the faq/releases.rst to indicate that userspace now will carry-over handler information. I haven't reviewed the code closely here, so I expect I will have comments once I look closer. -Aaron > tests/system-common-macros.at | 15 +++-- > tests/system-traffic.at | 106 ++++++++++++++++++++++++++++++++++ > tests/test-l7.py | 4 ++ > 3 files changed, 120 insertions(+), 5 deletions(-) > > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > index 0077a8609..24f7ad98b 100644 > --- a/tests/system-common-macros.at > +++ b/tests/system-common-macros.at > @@ -276,18 +276,23 @@ m4_define([NETNS_DAEMONIZE], > m4_define([OVS_CHECK_FIREWALL], > [AT_SKIP_IF([systemctl status firewalld 2>&1 | grep running > /dev/null])]) > > -# OVS_START_L7([namespace], [protocol]) > +# OVS_START_L7([namespace], [protocol], [port]) > # > -# Start a server serving 'protocol' within 'namespace'. The server will exit > -# when the test finishes. > +# Start a server serving 'protocol' on port 'port' within 'namespace'. > +# If 'port' is not specified, the standard one for 'protocol' will be used. > +# The server will exit when the test finishes. > # > m4_define([OVS_START_L7], > [PIDFILE=$(mktemp $2XXX.pid) > - NETNS_DAEMONIZE([$1], [[$PYTHON3 $srcdir/test-l7.py $2]], [$PIDFILE]) > + NETNS_DAEMONIZE([$1], [[$PYTHON3 $srcdir/test-l7.py $2 $3]], [$PIDFILE]) > > dnl netstat doesn't print http over IPv6 as "http6"; drop the number. > PROTO=$(echo $2 | sed -e 's/\([[a-zA-Z]]*\).*/\1/') > - OVS_WAIT_UNTIL([NS_EXEC([$1], [netstat -l | grep $PROTO])]) > + if test -z "$3"; then > + OVS_WAIT_UNTIL([NS_EXEC([$1], [netstat -l | grep $PROTO])]) > + else > + OVS_WAIT_UNTIL([NS_EXEC([$1], [netstat -ln | grep :$3])]) > + fi > ] > ) > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index 418cd32fe..002f7392a 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -5335,6 +5335,112 @@ tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src= > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > +AT_SETUP([conntrack - FTP non-standard port]) > +AT_SKIP_IF([test $HAVE_FTP = no]) > +CHECK_CONNTRACK() > +CHECK_CONNTRACK_ALG() > +OVS_TRAFFIC_VSWITCHD_START() > + > +ADD_NAMESPACES(at_ns0, at_ns1) > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") > + > +dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from ns1->ns0. > +AT_DATA([flows1.txt], [dnl > +table=0,priority=1,action=drop > +table=0,priority=10,arp,action=normal > +table=0,priority=10,icmp,action=normal > +table=0,priority=100,in_port=1,tcp,action=ct(alg=ftp,commit),2 > +table=0,priority=100,in_port=2,tcp,action=ct(table=1) > +table=1,in_port=2,tcp,ct_state=+trk+est,action=1 > +table=1,in_port=2,tcp,ct_state=+trk+rel,action=1 > +]) > + > +dnl Similar policy but without allowing all traffic from ns0->ns1. > +AT_DATA([flows2.txt], [dnl > +table=0,priority=1,action=drop > +table=0,priority=10,arp,action=normal > +table=0,priority=10,icmp,action=normal > + > +dnl Allow outgoing TCP connections, and treat them as FTP > +table=0,priority=100,in_port=1,tcp,action=ct(table=1) > +table=1,in_port=1,tcp,ct_state=+trk+new,action=ct(commit,alg=ftp),2 > +table=1,in_port=1,tcp,ct_state=+trk+est,action=2 > + > +dnl Allow incoming FTP data connections and responses to existing connections > +table=0,priority=100,in_port=2,tcp,action=ct(table=1) > +table=1,in_port=2,tcp,ct_state=+trk+new+rel,action=ct(commit),1 > +table=1,in_port=2,tcp,ct_state=+trk+est,action=1 > +table=1,in_port=2,tcp,ct_state=+trk-new+rel,action=1 > +]) > + > +dnl flows3 is same as flows1, except no ALG is specified. > +AT_DATA([flows3.txt], [dnl > +table=0,priority=1,action=drop > +table=0,priority=10,arp,action=normal > +table=0,priority=10,icmp,action=normal > +table=0,priority=100,in_port=1,tcp,action=ct(commit),2 > +table=0,priority=100,in_port=2,tcp,action=ct(table=1) > +table=1,in_port=2,tcp,ct_state=+trk+est,action=1 > +table=1,in_port=2,tcp,ct_state=+trk+rel,action=1 > +]) > + > +AT_CHECK([ovs-ofctl --bundle replace-flows br0 flows1.txt]) > + > +OVS_START_L7([at_ns0], [ftp], [11111]) > +OVS_START_L7([at_ns1], [ftp], [11111]) > + > +dnl FTP requests from p1->p0 should fail due to network failure. > +dnl Try 3 times, in 1 second intervals. > +NS_CHECK_EXEC([at_ns1], [wget ftp://10.1.1.1:11111 --no-passive-ftp -t 3 -T 1 -v -o wget1.log], [4]) > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.1)], [0], [dnl > +]) > + > +dnl FTP requests from p0->p1 should work fine. > +NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2:11111 --no-passive-ftp -t 3 -T 1 --retry-connrefused -v -o wget0.log]) > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl > +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>),helper=ftp > +]) > + > +dnl Try the second set of flows. > +AT_CHECK([ovs-ofctl --bundle replace-flows br0 flows2.txt]) > +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) > + > +dnl FTP requests from p1->p0 should fail due to network failure. > +dnl Try 3 times, in 1 second intervals. > +NS_CHECK_EXEC([at_ns1], [wget ftp://10.1.1.1:11111 --no-passive-ftp -t 3 -T 1 -v -o wget1.log], [4]) > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.1)], [0], [dnl > +]) > + > +dnl Active FTP requests from p0->p1 should work fine. > +NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2:11111 --no-passive-ftp -t 3 -T 1 --retry-connrefused -v -o wget0-1.log]) > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl > +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>),helper=ftp > +tcp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>) > +]) > + > +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) > + > +dnl Passive FTP requests from p0->p1 should work fine. > +NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2:11111 -t 3 -T 1 --retry-connrefused -v -o wget0-2.log]) > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl > +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>),helper=ftp > +]) > + > +dnl Try the third set of flows, without alg specifier. > +AT_CHECK([ovs-ofctl --bundle replace-flows br0 flows3.txt]) > +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) > + > +dnl FTP control requests from p0->p1 should work fine, but helper will not be assigned. > +NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2:11111 --no-passive-ftp -t 3 -T 1 --retry-connrefused -v -o wget0-3.log], [4]) > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl > +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>) > +]) > + > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP > + > AT_SETUP([conntrack - FTP with expectation dump]) > AT_SKIP_IF([test $HAVE_FTP = no]) > CHECK_CONNTRACK() > diff --git a/tests/test-l7.py b/tests/test-l7.py > index 32a77392c..97cd4f29a 100755 > --- a/tests/test-l7.py > +++ b/tests/test-l7.py > @@ -86,6 +86,8 @@ def main(): > description='Run basic application servers.') > parser.add_argument('proto', default='http', nargs='?', > help='protocol to serve (%s)' % protocols) > + parser.add_argument('port', default=0, nargs='?', > + help='server port number') > args = parser.parse_args() > > if args.proto not in protocols: > @@ -95,6 +97,8 @@ def main(): > constructor = SERVERS[args.proto][0] > handler = SERVERS[args.proto][1] > port = SERVERS[args.proto][2] > + if args.port != 0: > + port = args.port > srv = constructor(('', port), handler) > srv.serve_forever()
diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 0077a8609..24f7ad98b 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -276,18 +276,23 @@ m4_define([NETNS_DAEMONIZE], m4_define([OVS_CHECK_FIREWALL], [AT_SKIP_IF([systemctl status firewalld 2>&1 | grep running > /dev/null])]) -# OVS_START_L7([namespace], [protocol]) +# OVS_START_L7([namespace], [protocol], [port]) # -# Start a server serving 'protocol' within 'namespace'. The server will exit -# when the test finishes. +# Start a server serving 'protocol' on port 'port' within 'namespace'. +# If 'port' is not specified, the standard one for 'protocol' will be used. +# The server will exit when the test finishes. # m4_define([OVS_START_L7], [PIDFILE=$(mktemp $2XXX.pid) - NETNS_DAEMONIZE([$1], [[$PYTHON3 $srcdir/test-l7.py $2]], [$PIDFILE]) + NETNS_DAEMONIZE([$1], [[$PYTHON3 $srcdir/test-l7.py $2 $3]], [$PIDFILE]) dnl netstat doesn't print http over IPv6 as "http6"; drop the number. PROTO=$(echo $2 | sed -e 's/\([[a-zA-Z]]*\).*/\1/') - OVS_WAIT_UNTIL([NS_EXEC([$1], [netstat -l | grep $PROTO])]) + if test -z "$3"; then + OVS_WAIT_UNTIL([NS_EXEC([$1], [netstat -l | grep $PROTO])]) + else + OVS_WAIT_UNTIL([NS_EXEC([$1], [netstat -ln | grep :$3])]) + fi ] ) diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 418cd32fe..002f7392a 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -5335,6 +5335,112 @@ tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src= OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([conntrack - FTP non-standard port]) +AT_SKIP_IF([test $HAVE_FTP = no]) +CHECK_CONNTRACK() +CHECK_CONNTRACK_ALG() +OVS_TRAFFIC_VSWITCHD_START() + +ADD_NAMESPACES(at_ns0, at_ns1) + +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") + +dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from ns1->ns0. +AT_DATA([flows1.txt], [dnl +table=0,priority=1,action=drop +table=0,priority=10,arp,action=normal +table=0,priority=10,icmp,action=normal +table=0,priority=100,in_port=1,tcp,action=ct(alg=ftp,commit),2 +table=0,priority=100,in_port=2,tcp,action=ct(table=1) +table=1,in_port=2,tcp,ct_state=+trk+est,action=1 +table=1,in_port=2,tcp,ct_state=+trk+rel,action=1 +]) + +dnl Similar policy but without allowing all traffic from ns0->ns1. +AT_DATA([flows2.txt], [dnl +table=0,priority=1,action=drop +table=0,priority=10,arp,action=normal +table=0,priority=10,icmp,action=normal + +dnl Allow outgoing TCP connections, and treat them as FTP +table=0,priority=100,in_port=1,tcp,action=ct(table=1) +table=1,in_port=1,tcp,ct_state=+trk+new,action=ct(commit,alg=ftp),2 +table=1,in_port=1,tcp,ct_state=+trk+est,action=2 + +dnl Allow incoming FTP data connections and responses to existing connections +table=0,priority=100,in_port=2,tcp,action=ct(table=1) +table=1,in_port=2,tcp,ct_state=+trk+new+rel,action=ct(commit),1 +table=1,in_port=2,tcp,ct_state=+trk+est,action=1 +table=1,in_port=2,tcp,ct_state=+trk-new+rel,action=1 +]) + +dnl flows3 is same as flows1, except no ALG is specified. +AT_DATA([flows3.txt], [dnl +table=0,priority=1,action=drop +table=0,priority=10,arp,action=normal +table=0,priority=10,icmp,action=normal +table=0,priority=100,in_port=1,tcp,action=ct(commit),2 +table=0,priority=100,in_port=2,tcp,action=ct(table=1) +table=1,in_port=2,tcp,ct_state=+trk+est,action=1 +table=1,in_port=2,tcp,ct_state=+trk+rel,action=1 +]) + +AT_CHECK([ovs-ofctl --bundle replace-flows br0 flows1.txt]) + +OVS_START_L7([at_ns0], [ftp], [11111]) +OVS_START_L7([at_ns1], [ftp], [11111]) + +dnl FTP requests from p1->p0 should fail due to network failure. +dnl Try 3 times, in 1 second intervals. +NS_CHECK_EXEC([at_ns1], [wget ftp://10.1.1.1:11111 --no-passive-ftp -t 3 -T 1 -v -o wget1.log], [4]) +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.1)], [0], [dnl +]) + +dnl FTP requests from p0->p1 should work fine. +NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2:11111 --no-passive-ftp -t 3 -T 1 --retry-connrefused -v -o wget0.log]) +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>),helper=ftp +]) + +dnl Try the second set of flows. +AT_CHECK([ovs-ofctl --bundle replace-flows br0 flows2.txt]) +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) + +dnl FTP requests from p1->p0 should fail due to network failure. +dnl Try 3 times, in 1 second intervals. +NS_CHECK_EXEC([at_ns1], [wget ftp://10.1.1.1:11111 --no-passive-ftp -t 3 -T 1 -v -o wget1.log], [4]) +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.1)], [0], [dnl +]) + +dnl Active FTP requests from p0->p1 should work fine. +NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2:11111 --no-passive-ftp -t 3 -T 1 --retry-connrefused -v -o wget0-1.log]) +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>),helper=ftp +tcp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>) +]) + +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) + +dnl Passive FTP requests from p0->p1 should work fine. +NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2:11111 -t 3 -T 1 --retry-connrefused -v -o wget0-2.log]) +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>),helper=ftp +]) + +dnl Try the third set of flows, without alg specifier. +AT_CHECK([ovs-ofctl --bundle replace-flows br0 flows3.txt]) +AT_CHECK([ovs-appctl dpctl/flush-conntrack]) + +dnl FTP control requests from p0->p1 should work fine, but helper will not be assigned. +NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2:11111 --no-passive-ftp -t 3 -T 1 --retry-connrefused -v -o wget0-3.log], [4]) +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>) +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([conntrack - FTP with expectation dump]) AT_SKIP_IF([test $HAVE_FTP = no]) CHECK_CONNTRACK() diff --git a/tests/test-l7.py b/tests/test-l7.py index 32a77392c..97cd4f29a 100755 --- a/tests/test-l7.py +++ b/tests/test-l7.py @@ -86,6 +86,8 @@ def main(): description='Run basic application servers.') parser.add_argument('proto', default='http', nargs='?', help='protocol to serve (%s)' % protocols) + parser.add_argument('port', default=0, nargs='?', + help='server port number') args = parser.parse_args() if args.proto not in protocols: @@ -95,6 +97,8 @@ def main(): constructor = SERVERS[args.proto][0] handler = SERVERS[args.proto][1] port = SERVERS[args.proto][2] + if args.port != 0: + port = args.port srv = constructor(('', port), handler) srv.serve_forever()
All existing test iterations assume that the FTP server is running on a standard port, which may not always be the case. These tests helped find problems in conntrack alg processing with non-standard ports. Perform the necessary adjustments to ensure the test suite can start the L7 server on a user-provided port. Signed-off-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am> --- tests/system-common-macros.at | 15 +++-- tests/system-traffic.at | 106 ++++++++++++++++++++++++++++++++++ tests/test-l7.py | 4 ++ 3 files changed, 120 insertions(+), 5 deletions(-)