| Message ID | 1446926401-55723-23-git-send-email-joestringer@nicira.com |
|---|---|
| State | Superseded |
| Headers | show |
Small comments inline. Otherwise: Acked-by: Daniele Di Proietto <diproiettod@vmware.com> Thanks for the test On 07/11/2015 12:00, "Joe Stringer" <joestringer@nicira.com> wrote: >Add an additional test that ensures that when receiving packets from >internal ports that reside in a foreign namespace, the conntrack >information is not populated in the flow. > >Signed-off-by: Joe Stringer <joestringer@nicira.com> >--- > tests/system-common-macros.at | 12 ++++++++++++ > tests/system-traffic.at | 41 >+++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 53 insertions(+) > >diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at >index f0da5893905b..581c779e3e28 100644 >--- a/tests/system-common-macros.at >+++ b/tests/system-common-macros.at >@@ -43,6 +43,18 @@ m4_define([NS_CHECK_EXEC], > # appropriate type, and allows additional arguments to be passed. > m4_define([ADD_BR], [ovs-vsctl _ADD_BR([$1]) -- $2]) > >+# ADD_INT([port], [namespace], [ovs-br], [ip_addr]) >+# >+# Add an internal port to 'ovs-br', then shift it into 'namespace' and >+# configure it with 'ip_addr' (specified in CIDR notation). >+m4_define([ADD_INT], >+ [ AT_CHECK([ovs-vsctl add-port $3 $1 -- set int $1 type=internal]) >+ AT_CHECK([ip link set $1 netns $2]) >+ NS_CHECK_EXEC([$2], [ip addr add $4 dev $1]) >+ NS_CHECK_EXEC([$2], [ip link set dev $1 up]) >+ ] >+) >+ > # ADD_VETH([port], [namespace], [ovs-br], [ip_addr]) > # > # Add a pair of veth ports. 'port' will be added to name space >'namespace', >diff --git a/tests/system-traffic.at b/tests/system-traffic.at >index 3b47cced678f..abe00e149feb 100644 >--- a/tests/system-traffic.at >+++ b/tests/system-traffic.at >@@ -566,6 +566,47 @@ TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=<cleared> >dport=<cleared> src=10.1.1.2 > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > >+AT_SETUP([conntrack - multiple zones, internal ports]) >+CHECK_CONNTRACK() >+OVS_TRAFFIC_VSWITCHD_START( >+ [set-fail-mode br0 secure -- ]) >+ >+ADD_NAMESPACES(at_ns0, at_ns1) >+ >+ADD_INT(p0, at_ns0, br0, "10.1.1.1/24") >+ADD_INT(p1, at_ns1, br0, "10.1.1.2/24") >+ >+dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from >ns1->ns0. >+dnl >+dnl If skb->nfct is leaking from inside the namespace, this test will >fail. >+AT_DATA([flows.txt], [dnl >+priority=1,action=drop >+priority=10,arp,action=normal >+priority=10,icmp,action=normal >+priority=100,in_port=1,tcp,ct_state=-trk,action=ct(commit,zone=1),ct(comm >it,zone=2),2 I think ct(commit,zone=1) can be removed (unless I misunderstood your intentions here) >+priority=100,in_port=2,ct_state=-trk,tcp,action=ct(table=0,zone=2) >+priority=100,in_port=2,ct_state=+trk,ct_zone=2,tcp,action=1 >+]) >+ >+AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) >+ >+dnl HTTP requests from p0->p1 should work fine. >+NETNS_DAEMONIZE([at_ns1], [[$PYTHON $srcdir/test-l7.py]], [http0.pid]) >+NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 3 -T 1 --retry-connrefused -v >-o wget0.log]) >+ >+dnl (again) HTTP requests from p0->p1 should work fine. >+NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 3 -T 1 --retry-connrefused -v >-o wget0.log]) >+ >+AT_CHECK([conntrack -L 2>&1 | FORMAT_CT(10.1.1.2)], [0], [dnl >+SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=<cleared> dport=<cleared> >[[UNREPLIED]] src=10.1.1.2 dst=10.1.1.1 sport=<cleared> dport=<cleared> >mark=0 zone=1 use=1 If we remove ct(commit,zone=1), the above line should disappear >+TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=<cleared> dport=<cleared> >src=10.1.1.2 dst=10.1.1.1 sport=<cleared> dport=<cleared> [[ASSURED]] >mark=0 zone=2 use=1 >+]) >+ >+OVS_TRAFFIC_VSWITCHD_STOP(["dnl >+/ioctl(SIOCGIFINDEX) on .* device failed: No such device/d >+/removing policing failed: No such device/d"]) >+AT_CLEANUP >+ > AT_SETUP([conntrack - multiple zones, local]) > CHECK_CONNTRACK() > OVS_TRAFFIC_VSWITCHD_START( >-- >2.1.4 > >_______________________________________________ >dev mailing list >dev@openvswitch.org >https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma >n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Sm >B5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=kYMdScISW4Ygvqc_WAVYcLXDrdnPA4 >dBTjFsJU7vOTo&s=tCXXiGnmFGpEgfvDK3NN5KmcQcrluQgXgXYcJP_dR3w&e=
On 24 November 2015 at 10:49, Daniele Di Proietto <diproiettod@vmware.com> wrote: > Small comments inline. Otherwise: > > Acked-by: Daniele Di Proietto <diproiettod@vmware.com> > > Thanks for the test > > On 07/11/2015 12:00, "Joe Stringer" <joestringer@nicira.com> wrote: > >>Add an additional test that ensures that when receiving packets from >>internal ports that reside in a foreign namespace, the conntrack >>information is not populated in the flow. >> >>Signed-off-by: Joe Stringer <joestringer@nicira.com> >>--- >> tests/system-common-macros.at | 12 ++++++++++++ >> tests/system-traffic.at | 41 >>+++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 53 insertions(+) >> >>diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at >>index f0da5893905b..581c779e3e28 100644 >>--- a/tests/system-common-macros.at >>+++ b/tests/system-common-macros.at >>@@ -43,6 +43,18 @@ m4_define([NS_CHECK_EXEC], >> # appropriate type, and allows additional arguments to be passed. >> m4_define([ADD_BR], [ovs-vsctl _ADD_BR([$1]) -- $2]) >> >>+# ADD_INT([port], [namespace], [ovs-br], [ip_addr]) >>+# >>+# Add an internal port to 'ovs-br', then shift it into 'namespace' and >>+# configure it with 'ip_addr' (specified in CIDR notation). >>+m4_define([ADD_INT], >>+ [ AT_CHECK([ovs-vsctl add-port $3 $1 -- set int $1 type=internal]) >>+ AT_CHECK([ip link set $1 netns $2]) >>+ NS_CHECK_EXEC([$2], [ip addr add $4 dev $1]) >>+ NS_CHECK_EXEC([$2], [ip link set dev $1 up]) >>+ ] >>+) >>+ >> # ADD_VETH([port], [namespace], [ovs-br], [ip_addr]) >> # >> # Add a pair of veth ports. 'port' will be added to name space >>'namespace', >>diff --git a/tests/system-traffic.at b/tests/system-traffic.at >>index 3b47cced678f..abe00e149feb 100644 >>--- a/tests/system-traffic.at >>+++ b/tests/system-traffic.at >>@@ -566,6 +566,47 @@ TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=<cleared> >>dport=<cleared> src=10.1.1.2 >> OVS_TRAFFIC_VSWITCHD_STOP >> AT_CLEANUP >> >>+AT_SETUP([conntrack - multiple zones, internal ports]) >>+CHECK_CONNTRACK() >>+OVS_TRAFFIC_VSWITCHD_START( >>+ [set-fail-mode br0 secure -- ]) >>+ >>+ADD_NAMESPACES(at_ns0, at_ns1) >>+ >>+ADD_INT(p0, at_ns0, br0, "10.1.1.1/24") >>+ADD_INT(p1, at_ns1, br0, "10.1.1.2/24") >>+ >>+dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from >>ns1->ns0. >>+dnl >>+dnl If skb->nfct is leaking from inside the namespace, this test will >>fail. >>+AT_DATA([flows.txt], [dnl >>+priority=1,action=drop >>+priority=10,arp,action=normal >>+priority=10,icmp,action=normal >>+priority=100,in_port=1,tcp,ct_state=-trk,action=ct(commit,zone=1),ct(comm >>it,zone=2),2 > > I think ct(commit,zone=1) can be removed (unless I misunderstood > your intentions here) You're right, I was intending this to be more of a test of "multiple namespaces", not "multiple zones". I'll fix this up and push it soon, thanks for the review!
diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index f0da5893905b..581c779e3e28 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -43,6 +43,18 @@ m4_define([NS_CHECK_EXEC], # appropriate type, and allows additional arguments to be passed. m4_define([ADD_BR], [ovs-vsctl _ADD_BR([$1]) -- $2]) +# ADD_INT([port], [namespace], [ovs-br], [ip_addr]) +# +# Add an internal port to 'ovs-br', then shift it into 'namespace' and +# configure it with 'ip_addr' (specified in CIDR notation). +m4_define([ADD_INT], + [ AT_CHECK([ovs-vsctl add-port $3 $1 -- set int $1 type=internal]) + AT_CHECK([ip link set $1 netns $2]) + NS_CHECK_EXEC([$2], [ip addr add $4 dev $1]) + NS_CHECK_EXEC([$2], [ip link set dev $1 up]) + ] +) + # ADD_VETH([port], [namespace], [ovs-br], [ip_addr]) # # Add a pair of veth ports. 'port' will be added to name space 'namespace', diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 3b47cced678f..abe00e149feb 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -566,6 +566,47 @@ TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=<cleared> dport=<cleared> src=10.1.1.2 OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([conntrack - multiple zones, internal ports]) +CHECK_CONNTRACK() +OVS_TRAFFIC_VSWITCHD_START( + [set-fail-mode br0 secure -- ]) + +ADD_NAMESPACES(at_ns0, at_ns1) + +ADD_INT(p0, at_ns0, br0, "10.1.1.1/24") +ADD_INT(p1, at_ns1, br0, "10.1.1.2/24") + +dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from ns1->ns0. +dnl +dnl If skb->nfct is leaking from inside the namespace, this test will fail. +AT_DATA([flows.txt], [dnl +priority=1,action=drop +priority=10,arp,action=normal +priority=10,icmp,action=normal +priority=100,in_port=1,tcp,ct_state=-trk,action=ct(commit,zone=1),ct(commit,zone=2),2 +priority=100,in_port=2,ct_state=-trk,tcp,action=ct(table=0,zone=2) +priority=100,in_port=2,ct_state=+trk,ct_zone=2,tcp,action=1 +]) + +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) + +dnl HTTP requests from p0->p1 should work fine. +NETNS_DAEMONIZE([at_ns1], [[$PYTHON $srcdir/test-l7.py]], [http0.pid]) +NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 3 -T 1 --retry-connrefused -v -o wget0.log]) + +dnl (again) HTTP requests from p0->p1 should work fine. +NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 3 -T 1 --retry-connrefused -v -o wget0.log]) + +AT_CHECK([conntrack -L 2>&1 | FORMAT_CT(10.1.1.2)], [0], [dnl +SYN_SENT src=10.1.1.1 dst=10.1.1.2 sport=<cleared> dport=<cleared> [[UNREPLIED]] src=10.1.1.2 dst=10.1.1.1 sport=<cleared> dport=<cleared> mark=0 zone=1 use=1 +TIME_WAIT src=10.1.1.1 dst=10.1.1.2 sport=<cleared> dport=<cleared> src=10.1.1.2 dst=10.1.1.1 sport=<cleared> dport=<cleared> [[ASSURED]] mark=0 zone=2 use=1 +]) + +OVS_TRAFFIC_VSWITCHD_STOP(["dnl +/ioctl(SIOCGIFINDEX) on .* device failed: No such device/d +/removing policing failed: No such device/d"]) +AT_CLEANUP + AT_SETUP([conntrack - multiple zones, local]) CHECK_CONNTRACK() OVS_TRAFFIC_VSWITCHD_START(
Add an additional test that ensures that when receiving packets from internal ports that reside in a foreign namespace, the conntrack information is not populated in the flow. Signed-off-by: Joe Stringer <joestringer@nicira.com> --- tests/system-common-macros.at | 12 ++++++++++++ tests/system-traffic.at | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+)