Thanks, it's really more realistic.

On 2016/7/28 22:53, Guru Shetty wrote:


On 28 July 2016 at 07:48, Guru Shetty <g...@ovn.org <mailto:g...@ovn.org>> wrote:



    On 28 July 2016 at 06:55, Dong Jun <do...@dtdream.com
    <mailto:do...@dtdream.com>> wrote:

        Yes, this test case fail currently and success with the
        modification.
        If there is another same test case, ignore this patch is OK.


    Thank you for the test. I applied the linked patch by Chandra as
    it was already reviewed and applied your unit test and also added
    you in AUTHORS. I did look at your second patch of the series.
    Please read CodingStyle.md for any future contributions.


I forgot to mention that I did add the following incremental to your test to make the test case more realistic. (Also, we still have a bug wherein we allow the router to respond to ICMP messages for a SNAT IP. This can be racy.)


@@ -243,19 +243,19 @@ ovn-nbctl lsp-add alice alice1 \

 # Add a SNAT rule
 ovn-nbctl -- --id=@nat create nat type="snat" logical_ip=192.168.1.2 \
-    external_ip=20.0.0.2 -- add logical_router R2 nat @nat
+    external_ip=172.16.1.1 -- add logical_router R2 nat @nat

 # South-North SNAT: 'foo1' pings 'alice1'. But 'alice1' receives traffic
-# from 20.0.0.2
+# from 172.16.1.1
NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 172.16.1.2 | FORMAT_PING], \
 [0], [dnl
 3 packets transmitted, 3 received, 0% packet loss, time 0ms
 ])

 # We verify that SNAT indeed happened via 'dump-conntrack' command.
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(20.0.0.2) | \
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.1) | \
 sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
-icmp,orig=(src=192.168.1.2,dst=172.16.1.2,id=<cleared>),reply=(src=172.16.1.2,dst=20.0.0.2,id=<cleared>),zon
+icmp,orig=(src=192.168.1.2,dst=172.16.1.2,id=<cleared>),reply=(src=172.16.1.2,dst=172.16.1.1,id=<cleared>),z
 ])


        On 2016/7/28 21:31, Guru Shetty wrote:



            On 27 July 2016 at 23:30, Dongjun <do...@dtdream.com
            <mailto:do...@dtdream.com> <mailto:do...@dtdream.com
            <mailto:do...@dtdream.com>>> wrote:

                Signed-off-by: Dongjun <do...@dtdream.com
            <mailto:do...@dtdream.com> <mailto:do...@dtdream.com
            <mailto:do...@dtdream.com>>>


            Usually, we have a test with the corresponding code
            change. Can you have a look at
            https://www.mail-archive.com/dev@openvswitch.org/msg66099.html
            and see whether you are happy with the code change?

                ---
                 tests/system-ovn.at <http://system-ovn.at>
            <http://system-ovn.at> | 106
            +++++++++++++++++++++++++++++++++++++++++++++++++++-
                 1 file changed, 105 insertions(+), 1 deletion(-)
                 mode change 100644 => 100755 tests/system-ovn.at
            <http://system-ovn.at>
                <http://system-ovn.at>

                diff --git a/tests/system-ovn.at
            <http://system-ovn.at> <http://system-ovn.at>
                b/tests/system-ovn.at <http://system-ovn.at>
            <http://system-ovn.at>
                old mode 100644
                new mode 100755
                index 13f380f..cb50fd4
                --- a/tests/system-ovn.at <http://system-ovn.at>
            <http://system-ovn.at>
                +++ b/tests/system-ovn.at <http://system-ovn.at>
            <http://system-ovn.at>

                @@ -1,4 +1,4 @@
                -AT_SETUP([ovn -- 2 LRs connected via LS, gateway
            router, NAT])
                +AT_SETUP([ovn -- 2 LRs connected via LS, gateway
            router, SNAT and
                DNAT])
                 AT_KEYWORDS([ovnnat])

                 CHECK_CONNTRACK()
                @@ -168,6 +168,110 @@ as
                 OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port
            patch-.*/d"])
                 AT_CLEANUP

                +AT_SETUP([ovn -- 2 LRs connected via LS, gateway
            router, easy SNAT])
                +AT_KEYWORDS([ovnnat])
                +
                +CHECK_CONNTRACK()
                +ovn_start
                +OVS_TRAFFIC_VSWITCHD_START()
                +ADD_BR([br-int])
                +
                +# Set external-ids in br-int needed for ovn-controller
                +ovs-vsctl \
                +        -- set Open_vSwitch .
            external-ids:system-id=hv1 \
                +        -- set Open_vSwitch .
            external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
                +        -- set Open_vSwitch .
            external-ids:ovn-encap-type=geneve \
                +        -- set Open_vSwitch .
            external-ids:ovn-encap-ip=169.0.0.1 \
                +        -- set bridge br-int fail-mode=secure
                other-config:disable-in-band=true
                +
                +# Start ovn-controller
                +start_daemon ovn-controller
                +
                +# Logical network:
                +# Two LRs - R1 and R2 that are connected to each
            other via LS "join"
                +# in 20.0.0.0/24 <http://20.0.0.0/24>
            <http://20.0.0.0/24> network. R1 has switchess
                foo (192.168.1.0/24 <http://192.168.1.0/24>
            <http://192.168.1.0/24>) connected to it.
                +# R2 has alice (172.16.1.0/24 <http://172.16.1.0/24>
            <http://172.16.1.0/24>) connectedto it.
                +# R2 is a gateway router on which we add NAT rules.
                +#
                +#    foo -- R1 -- join - R2 -- alice
                +
                +ovn-nbctl lr-add R1
                +ovn-nbctl lr-add R2 -- set Logical_Router R2
            options:chassis=hv1
                +
                +ovn-nbctl ls-add foo
                +ovn-nbctl ls-add alice
                +ovn-nbctl ls-add join
                +
                +ovn-nbctl lrp-add R1 foo 00:00:01:01:02:03
            192.168.1.1/24 <http://192.168.1.1/24>
                <http://192.168.1.1/24>
                +ovn-nbctl lrp-add R2 alice 00:00:02:01:02:03
            172.16.1.1/24 <http://172.16.1.1/24>
                <http://172.16.1.1/24>
                +ovn-nbctl lrp-add R1 R1_join 00:00:04:01:02:03
            20.0.0.1/24 <http://20.0.0.1/24>
                <http://20.0.0.1/24>
                +ovn-nbctl lrp-add R2 R2_join 00:00:04:01:02:04
            20.0.0.2/24 <http://20.0.0.2/24>
                <http://20.0.0.2/24>
                +
                +# Connect foo to R1
                +ovn-nbctl lsp-add foo rp-foo -- set
            Logical_Switch_Port rp-foo \
                +    type=router options:router-port=foo
                addresses=\"00:00:01:01:02:03\"
                +
                +# Connect alice to R2
                +ovn-nbctl lsp-add alice rp-alice -- set
            Logical_Switch_Port
                rp-alice \
                +    type=router options:router-port=alice
                addresses=\"00:00:02:01:02:03\"
                +
                +# Connect R1 to join
                +ovn-nbctl lsp-add join r1-join -- set
            Logical_Switch_Port r1-join \
                +    type=router options:router-port=R1_join
                addresses='"00:00:04:01:02:03"'
                +
                +# Connect R2 to join
                +ovn-nbctl lsp-add join r2-join -- set
            Logical_Switch_Port r2-join \
                +    type=router options:router-port=R2_join
                addresses='"00:00:04:01:02:04"'
                +
                +# Static routes.
                +ovn-nbctl lr-route-add R1 172.16.1.0/24
            <http://172.16.1.0/24> <http://172.16.1.0/24>
                20.0.0.2
                +ovn-nbctl lr-route-add R2 192.168.0.0/16
            <http://192.168.0.0/16> <http://192.168.0.0/16>
                20.0.0.1
                +
                +# Logical port 'foo1' in switch 'foo'.
                +ADD_NAMESPACES(foo1)
                +ADD_VETH(foo1, foo1, br-int, "192.168.1.2/24
            <http://192.168.1.2/24>
                <http://192.168.1.2/24>", "f0:00:00:01:02:03", \
                +         "192.168.1.1")
                +ovn-nbctl lsp-add foo foo1 \
                +-- lsp-set-addresses foo1 "f0:00:00:01:02:03 192.168.1.2"
                +
                +# Logical port 'alice1' in switch 'alice'.
                +ADD_NAMESPACES(alice1)
                +ADD_VETH(alice1, alice1, br-int, "172.16.1.2/24
            <http://172.16.1.2/24>
                <http://172.16.1.2/24>", "f0:00:00:01:02:04", \

                +         "172.16.1.1")
                +ovn-nbctl lsp-add alice alice1 \
                +-- lsp-set-addresses alice1 "f0:00:00:01:02:04
            172.16.1.2"
                +
                +# Add a SNAT rule
                +ovn-nbctl -- --id=@nat create nat type="snat"
                logical_ip=192.168.1.2 \
                +    external_ip=20.0.0.2 -- add logical_router R2 nat
            @nat
                +
                +# South-North SNAT: 'foo1' pings 'alice1'. But
            'alice1' receives
                traffic
                +# from 20.0.0.2
                +NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2
            172.16.1.2 |
                FORMAT_PING], \
                +[0], [dnl
                +3 packets transmitted, 3 received, 0% packet loss,
            time 0ms
                +])
                +
                +# We verify that SNAT indeed happened via
            'dump-conntrack' command.
                +AT_CHECK([ovs-appctl dpctl/dump-conntrack |
            FORMAT_CT(20.0.0.2) | \
                +sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl
            
+icmp,orig=(src=192.168.1.2,dst=172.16.1.2,id=<cleared>),reply=(src=172.16.1.2,dst=20.0.0.2,id=<cleared>),zone=<cleared>
                +])
                +
            +OVS_APP_EXIT_AND_WAIT([ovn-controller])
                +
                +as ovn-sb
            +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
                +
                +as ovn-nb
            +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
                +
                +as northd
                +OVS_APP_EXIT_AND_WAIT([ovn-northd])
                +
                +as
                +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port
            patch-.*/d"])
                +AT_CLEANUP

                 AT_SETUP([ovn -- load-balancing])
                 AT_KEYWORDS([ovnlb])
                --
                1.8.3.1

            _______________________________________________
                dev mailing list
            dev@openvswitch.org <mailto:dev@openvswitch.org>
            <mailto:dev@openvswitch.org <mailto:dev@openvswitch.org>>
            http://openvswitch.org/mailman/listinfo/dev



        _______________________________________________
        dev mailing list
        dev@openvswitch.org <mailto:dev@openvswitch.org>
        http://openvswitch.org/mailman/listinfo/dev




_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to