On 21 Apr 2023, at 12:00, Ales Musil wrote:
> Fix the outdated parts of SCTP test and > allow it to be run on CI, in order to do that > we just need to load sctp kernel module. > > Reported-at: https://bugzilla.redhat.com/2183516 > Signed-off-by: Ales Musil <[email protected]> > --- > v2: Rebase on top of current main. > Address comment from Eelco about not removing the sctp > module if it was already loaded before run. Thanks for changing! Two nits below, but it looks good to me. Acked-by: Eelco Chaudron <[email protected]> > --- > tests/system-common-macros.at | 13 ++++++++++++ > tests/system-ovn-kmod.at | 31 ++++++++++++++++------------- > tests/test-l7.py | 37 +++++++++++++---------------------- > 3 files changed, 44 insertions(+), 37 deletions(-) > > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > index c6ba7fb4f..e6f204cc1 100644 > --- a/tests/system-common-macros.at > +++ b/tests/system-common-macros.at > @@ -330,6 +330,19 @@ m4_define([OVS_CHECK_CT_CLEAR], > m4_define([OVS_CHECK_CT_ZERO_SNAT], > [AT_SKIP_IF([! grep -q "Datapath supports ct_zero_snat" > ovs-vswitchd.log])]) > > +# LOAD_MODULE([name]) > +# > +# Tries to load specified kernel module and removes it after > +# if it wasn't loaded before this call. > +# > +m4_define([LOAD_MODULE], > + [if ! lsmod | grep -q $1; then > + on_exit 'modprobe -q -r $1' > + fi > + AT_CHECK([modprobe $1]) Maybe the modprobe needs to be inside the if statement? I guess it does not matter for the end result, so I’m fine either way. > + ] > +) > + > # OVN_TEST_IPV6_PREFIX_DELEGATION() > m4_define([OVN_TEST_IPV6_PREFIX_DELEGATION], > [ > diff --git a/tests/system-ovn-kmod.at b/tests/system-ovn-kmod.at > index 981cc598c..29e0824d7 100644 > --- a/tests/system-ovn-kmod.at > +++ b/tests/system-ovn-kmod.at > @@ -9,6 +9,9 @@ AT_SKIP_IF([test $HAVE_SCTP = no]) > AT_SKIP_IF([test $HAVE_NC = no]) > AT_KEYWORDS([ovnlb sctp]) > > +# Make sure the SCTP kernel module is loaded Not sure how strict you guys are with comments ending with a dot. Maybe the committer can add this. > +LOAD_MODULE([sctp]) > + > CHECK_CONNTRACK() > CHECK_CONNTRACK_NAT() > ovn_start > @@ -127,8 +130,8 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | > FORMAT_CT(30.0.0.1) | > sed -e 's/zone=[[0-9]]*/zone=<cleared>/' | > sed -e 's/vtag_orig=[[0-9]]*/vtag_orig=<cleared>/' | > sed -e 's/vtag_reply=[[0-9]]*/vtag_reply=<cleared>/' | uniq], [0], [dnl > -sctp,orig=(src=172.16.1.2,dst=30.0.0.1,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>,vtag_orig=<cleared>,vtag_reply=<cleared>) > -sctp,orig=(src=172.16.1.2,dst=30.0.0.1,sport=<cleared>,dport=<cleared>),reply=(src=192.168.2.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>,vtag_orig=<cleared>,vtag_reply=<cleared>) > +sctp,orig=(src=172.16.1.2,dst=30.0.0.1,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>,vtag_orig=<cleared>,vtag_reply=<cleared>) > +sctp,orig=(src=172.16.1.2,dst=30.0.0.1,sport=<cleared>,dport=<cleared>),reply=(src=192.168.2.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>,vtag_orig=<cleared>,vtag_reply=<cleared>) > ]) > > dnl Test load-balancing that includes L4 ports in NAT. > @@ -142,20 +145,19 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | > FORMAT_CT(30.0.0.2) | > sed -e 's/zone=[[0-9]]*/zone=<cleared>/' | > sed -e 's/vtag_orig=[[0-9]]*/vtag_orig=<cleared>/' | > sed -e 's/vtag_reply=[[0-9]]*/vtag_reply=<cleared>/' | uniq], [0], [dnl > -sctp,orig=(src=172.16.1.2,dst=30.0.0.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>,vtag_orig=<cleared>,vtag_reply=<cleared>) > -sctp,orig=(src=172.16.1.2,dst=30.0.0.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.2.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>,vtag_orig=<cleared>,vtag_reply=<cleared>) > +sctp,orig=(src=172.16.1.2,dst=30.0.0.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>,vtag_orig=<cleared>,vtag_reply=<cleared>) > +sctp,orig=(src=172.16.1.2,dst=30.0.0.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.2.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>,vtag_orig=<cleared>,vtag_reply=<cleared>) > ]) > > check_est_flows () { > - n=$(ovs-ofctl dump-flows br-int table=14 | grep \ > -"priority=120,ct_state=+est+trk,sctp,metadata=0x2,nw_dst=30.0.0.2,tp_dst=8000" > \ > -| grep nat | sed -n 's/.*n_packets=\([[0-9]]\{1,\}\).*/\1/p') > + n=$(ovs-ofctl dump-flows br-int table=15 | grep "+est" \ > + | grep "ct_mark=$1" | sed -n > 's/.*n_packets=\([[0-9]]\{1,\}\).*/\1/p') > > echo "n_packets=$n" > - test "$n" != 0 > + test -n "$n" && test "$n" != "0" > } > > -OVS_WAIT_UNTIL([check_est_flows], [check established flows]) > +OVS_WAIT_UNTIL([check_est_flows 0x2], [check established flows]) > > > ovn-nbctl set logical_router R2 options:lb_force_snat_ip="20.0.0.2" > @@ -167,13 +169,14 @@ ovn-nbctl destroy load_balancer $uuid > uuid=`ovn-nbctl create load_balancer protocol=sctp > vips:30.0.0.1="192.168.1.2,192.168.2.2"` > ovn-nbctl set logical_router R2 load_balancer=$uuid > > +check ovs-appctl dpctl/flush-conntrack > + > # Config OVN load-balancer with another VIP (this time with ports). > ovn-nbctl set load_balancer $uuid > vips:'"30.0.0.2:8000"'='"192.168.1.2:12345,192.168.2.2:12345"' > > ovn-nbctl list load_balancer > ovn-sbctl dump-flows R2 > -OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow13 dump-flows br-int table=41 | \ > -grep 'nat(src=20.0.0.2)']) > +OVS_WAIT_UNTIL([ovs-ofctl -O OpenFlow13 dump-flows br-int table=43 | grep > 'nat(src=20.0.0.2)']) > > dnl Test load-balancing that includes L4 ports in NAT. > for i in `seq 1 20`; do > @@ -186,8 +189,8 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | > FORMAT_CT(30.0.0.2) | > sed -e 's/zone=[[0-9]]*/zone=<cleared>/' | > sed -e 's/vtag_orig=[[0-9]]*/vtag_orig=<cleared>/' | > sed -e 's/vtag_reply=[[0-9]]*/vtag_reply=<cleared>/' | uniq], [0], [dnl > -sctp,orig=(src=172.16.1.2,dst=30.0.0.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>,vtag_orig=<cleared>,vtag_reply=<cleared>) > -sctp,orig=(src=172.16.1.2,dst=30.0.0.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.2.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>,vtag_orig=<cleared>,vtag_reply=<cleared>) > +sctp,orig=(src=172.16.1.2,dst=30.0.0.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.1.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=10,protoinfo=(state=<cleared>,vtag_orig=<cleared>,vtag_reply=<cleared>) > +sctp,orig=(src=172.16.1.2,dst=30.0.0.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.2.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,mark=10,protoinfo=(state=<cleared>,vtag_orig=<cleared>,vtag_reply=<cleared>) > ]) > > AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(20.0.0.2) | > @@ -198,7 +201,7 @@ > sctp,orig=(src=172.16.1.2,dst=192.168.1.2,sport=<cleared>,dport=<cleared>),reply > > sctp,orig=(src=172.16.1.2,dst=192.168.2.2,sport=<cleared>,dport=<cleared>),reply=(src=192.168.2.2,dst=20.0.0.2,sport=<cleared>,dport=<cleared>),zone=<cleared>,protoinfo=(state=<cleared>,vtag_orig=<cleared>,vtag_reply=<cleared>) > ]) > > -OVS_WAIT_UNTIL([check_est_flows], [check established flows]) > +OVS_WAIT_UNTIL([check_est_flows 0xa], [check established flows]) > > OVS_APP_EXIT_AND_WAIT([ovn-controller]) > > diff --git a/tests/test-l7.py b/tests/test-l7.py > index 701c63f0d..6e592f0dc 100755 > --- a/tests/test-l7.py > +++ b/tests/test-l7.py > @@ -74,29 +74,20 @@ def get_tftpd(): > > > def get_sctp(): > - try: > - import socket > - import sctp > - except ImportError: > - print("Failed to import for SCTP") > - server = None > - pass > - else: > - class OVSSCTPServer(object): > - def __init__(self, listen, handler=None): > - self.sock = sctp.sctpsocket_tcp(socket.AF_INET) > - self.sock.bind(listen) > - self.sock.listen() > - > - def serve_forever(self): > - while True: > - client, _ = self.sock.accept() > - client.send(b"SCRAM\r\n") > - client.close() > - > - server = [OVSSCTPServer, None, 12345] > - > - return server > + class OVSSCTPServer(object): > + def __init__(self, listen, handler=None): > + self.sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM, > + socket.IPPROTO_SCTP) > + self.sock.bind(listen) > + self.sock.listen() > + > + def serve_forever(self): > + while True: > + client, _ = self.sock.accept() > + client.sendall(b"SCRAM\r\n") > + client.close() > + > + return [OVSSCTPServer, None, 12345] > > > def main(): > -- > 2.40.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
