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.galaktio...@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() _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
