| Message ID | 1469581096-42007-14-git-send-email-diproiettod@vmware.com |
|---|---|
| State | Accepted |
| Headers | show |
On 26 July 2016 at 17:58, Daniele Di Proietto <diproiettod@vmware.com> wrote: > The userspace connection tracker doesn't support ALGs, frag reassembly > or NAT yet, so skip those tests. > > Also, connection tracking state input from a local port is not possible > in userspace. > > The userspace datapath pads all frames with 0, to make them at > least 64 bytes. > > Finally, the userspace datapath checks for the IPv4 header checksum, so > fix those in the hardcoded packets. > > Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> > Acked-by: Joe Stringer <joe@ovn.org> > Acked-by: Flavio Leitner <fbl@sysclose.org> > --- <snip> > @@ -1324,11 +1327,11 @@ dnl UDP packets from ns0->ns1 should solicit "destination unreachable" response. > NS_CHECK_EXEC([at_ns0], [bash -c "echo a | nc $NC_EOF_OPT -u 10.1.1.2 10000"]) > > AT_CHECK([ovs-appctl revalidator/purge], [0]) > -AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort | grep -v drop], [0], [dnl > - n_packets=1, n_bytes=44, priority=100,udp,in_port=1 actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2 > - n_packets=1, n_bytes=72, priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1 > - n_packets=1, n_bytes=72, priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0) > - n_packets=2, n_bytes=84, priority=10,arp actions=NORMAL > +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort | grep -v drop | sed -e 's/n_bytes=[[0-9]]*/n_bytes=<cleared>/g'], [0], [dnl > + n_packets=1, n_bytes=<cleared>, priority=100,udp,in_port=1 actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2 > + n_packets=1, n_bytes=<cleared>, priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1 > + n_packets=1, n_bytes=<cleared>, priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0) > + n_packets=2, n_bytes=<cleared>, priority=10,arp actions=NORMAL > NXST_FLOW reply: > ]) I think this is a completely orthogonal point, but it's still a bit surprising to me that the n_bytes would differ when receiving short packets in kernel vs. userspace datapaths. I follow that userspace pads shorter packets on receive, but shouldn't we be able to attribute these stats consistently, regardless of the datapath?
On 27/07/2016 13:12, "Joe Stringer" <joe@ovn.org> wrote: >On 26 July 2016 at 17:58, Daniele Di Proietto <diproiettod@vmware.com> wrote: >> The userspace connection tracker doesn't support ALGs, frag reassembly >> or NAT yet, so skip those tests. >> >> Also, connection tracking state input from a local port is not possible >> in userspace. >> >> The userspace datapath pads all frames with 0, to make them at >> least 64 bytes. >> >> Finally, the userspace datapath checks for the IPv4 header checksum, so >> fix those in the hardcoded packets. >> >> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> >> Acked-by: Joe Stringer <joe@ovn.org> >> Acked-by: Flavio Leitner <fbl@sysclose.org> >> --- > ><snip> > >> @@ -1324,11 +1327,11 @@ dnl UDP packets from ns0->ns1 should solicit "destination unreachable" response. >> NS_CHECK_EXEC([at_ns0], [bash -c "echo a | nc $NC_EOF_OPT -u 10.1.1.2 10000"]) >> >> AT_CHECK([ovs-appctl revalidator/purge], [0]) >> -AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort | grep -v drop], [0], [dnl >> - n_packets=1, n_bytes=44, priority=100,udp,in_port=1 actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2 >> - n_packets=1, n_bytes=72, priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1 >> - n_packets=1, n_bytes=72, priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0) >> - n_packets=2, n_bytes=84, priority=10,arp actions=NORMAL >> +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort | grep -v drop | sed -e 's/n_bytes=[[0-9]]*/n_bytes=<cleared>/g'], [0], [dnl >> + n_packets=1, n_bytes=<cleared>, priority=100,udp,in_port=1 actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2 >> + n_packets=1, n_bytes=<cleared>, priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1 >> + n_packets=1, n_bytes=<cleared>, priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0) >> + n_packets=2, n_bytes=<cleared>, priority=10,arp actions=NORMAL >> NXST_FLOW reply: >> ]) > >I think this is a completely orthogonal point, but it's still a bit >surprising to me that the n_bytes would differ when receiving short >packets in kernel vs. userspace datapaths. I follow that userspace >pads shorter packets on receive, but shouldn't we be able to attribute >these stats consistently, regardless of the datapath? That's a good point. We call dp_packet_pad() in netdev_linux_recv(). That used to be in netdev_recv() and can be traced back to the initial OVS commit. Here's a comment in netdev_recv(): COVERAGE_INC(netdev_received); buffer->size += n_bytes; /* When the kernel internally sends out an Ethernet frame on an * interface, it gives us a copy *before* padding the frame to the * minimum length. Thus, when it sends out something like an ARP * request, we see a too-short frame. So pad it out to the minimum * length. */ pad_to_minimum_length(buffer);
On Wed, Jul 27, 2016 at 1:40 PM, Daniele Di Proietto <diproiettod@vmware.com> wrote: > On 27/07/2016 13:12, "Joe Stringer" <joe@ovn.org> wrote: > >>On 26 July 2016 at 17:58, Daniele Di Proietto <diproiettod@vmware.com> wrote: >>> The userspace connection tracker doesn't support ALGs, frag reassembly >>> or NAT yet, so skip those tests. >>> >>> Also, connection tracking state input from a local port is not possible >>> in userspace. >>> >>> The userspace datapath pads all frames with 0, to make them at >>> least 64 bytes. >>> >>> Finally, the userspace datapath checks for the IPv4 header checksum, so >>> fix those in the hardcoded packets. >>> >>> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> >>> Acked-by: Joe Stringer <joe@ovn.org> >>> Acked-by: Flavio Leitner <fbl@sysclose.org> >>> --- >> >><snip> >> >>> @@ -1324,11 +1327,11 @@ dnl UDP packets from ns0->ns1 should solicit "destination unreachable" response. >>> NS_CHECK_EXEC([at_ns0], [bash -c "echo a | nc $NC_EOF_OPT -u 10.1.1.2 10000"]) >>> >>> AT_CHECK([ovs-appctl revalidator/purge], [0]) >>> -AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort | grep -v drop], [0], [dnl >>> - n_packets=1, n_bytes=44, priority=100,udp,in_port=1 actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2 >>> - n_packets=1, n_bytes=72, priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1 >>> - n_packets=1, n_bytes=72, priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0) >>> - n_packets=2, n_bytes=84, priority=10,arp actions=NORMAL >>> +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort | grep -v drop | sed -e 's/n_bytes=[[0-9]]*/n_bytes=<cleared>/g'], [0], [dnl >>> + n_packets=1, n_bytes=<cleared>, priority=100,udp,in_port=1 actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2 >>> + n_packets=1, n_bytes=<cleared>, priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1 >>> + n_packets=1, n_bytes=<cleared>, priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0) >>> + n_packets=2, n_bytes=<cleared>, priority=10,arp actions=NORMAL >>> NXST_FLOW reply: >>> ]) >> >>I think this is a completely orthogonal point, but it's still a bit >>surprising to me that the n_bytes would differ when receiving short >>packets in kernel vs. userspace datapaths. I follow that userspace >>pads shorter packets on receive, but shouldn't we be able to attribute >>these stats consistently, regardless of the datapath? > > That's a good point. > > We call dp_packet_pad() in netdev_linux_recv(). That used to be in netdev_recv() and can be traced back to the initial OVS commit. Here's a comment in netdev_recv(): > > COVERAGE_INC(netdev_received); > buffer->size += n_bytes; > > /* When the kernel internally sends out an Ethernet frame on an > * interface, it gives us a copy *before* padding the frame to the > * minimum length. Thus, when it sends out something like an ARP > * request, we see a too-short frame. So pad it out to the minimum > * length. */ > pad_to_minimum_length(buffer); I wonder if anything in OVS actually cares about this anymore? I don't know the history of that comment. I should point out that there are other sins in stats accounting (TSO in the kernel is probably the worst since it causes the flow stats to not match what is actually seen on the wire). This wouldn't be the worst inconsistency but if there is any easy fix that would be nice.
On Wed, Jul 27, 2016 at 01:51:00PM -0700, Jesse Gross wrote: > On Wed, Jul 27, 2016 at 1:40 PM, Daniele Di Proietto > <diproiettod@vmware.com> wrote: > > On 27/07/2016 13:12, "Joe Stringer" <joe@ovn.org> wrote: > > > >>On 26 July 2016 at 17:58, Daniele Di Proietto <diproiettod@vmware.com> wrote: > >>> The userspace connection tracker doesn't support ALGs, frag reassembly > >>> or NAT yet, so skip those tests. > >>> > >>> Also, connection tracking state input from a local port is not possible > >>> in userspace. > >>> > >>> The userspace datapath pads all frames with 0, to make them at > >>> least 64 bytes. > >>> > >>> Finally, the userspace datapath checks for the IPv4 header checksum, so > >>> fix those in the hardcoded packets. > >>> > >>> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> > >>> Acked-by: Joe Stringer <joe@ovn.org> > >>> Acked-by: Flavio Leitner <fbl@sysclose.org> > >>> --- > >> > >><snip> > >> > >>> @@ -1324,11 +1327,11 @@ dnl UDP packets from ns0->ns1 should solicit "destination unreachable" response. > >>> NS_CHECK_EXEC([at_ns0], [bash -c "echo a | nc $NC_EOF_OPT -u 10.1.1.2 10000"]) > >>> > >>> AT_CHECK([ovs-appctl revalidator/purge], [0]) > >>> -AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort | grep -v drop], [0], [dnl > >>> - n_packets=1, n_bytes=44, priority=100,udp,in_port=1 actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2 > >>> - n_packets=1, n_bytes=72, priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1 > >>> - n_packets=1, n_bytes=72, priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0) > >>> - n_packets=2, n_bytes=84, priority=10,arp actions=NORMAL > >>> +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort | grep -v drop | sed -e 's/n_bytes=[[0-9]]*/n_bytes=<cleared>/g'], [0], [dnl > >>> + n_packets=1, n_bytes=<cleared>, priority=100,udp,in_port=1 actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2 > >>> + n_packets=1, n_bytes=<cleared>, priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1 > >>> + n_packets=1, n_bytes=<cleared>, priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0) > >>> + n_packets=2, n_bytes=<cleared>, priority=10,arp actions=NORMAL > >>> NXST_FLOW reply: > >>> ]) > >> > >>I think this is a completely orthogonal point, but it's still a bit > >>surprising to me that the n_bytes would differ when receiving short > >>packets in kernel vs. userspace datapaths. I follow that userspace > >>pads shorter packets on receive, but shouldn't we be able to attribute > >>these stats consistently, regardless of the datapath? > > > > That's a good point. > > > > We call dp_packet_pad() in netdev_linux_recv(). That used to be in netdev_recv() and can be traced back to the initial OVS commit. Here's a comment in netdev_recv(): > > > > COVERAGE_INC(netdev_received); > > buffer->size += n_bytes; > > > > /* When the kernel internally sends out an Ethernet frame on an > > * interface, it gives us a copy *before* padding the frame to the > > * minimum length. Thus, when it sends out something like an ARP > > * request, we see a too-short frame. So pad it out to the minimum > > * length. */ > > pad_to_minimum_length(buffer); > > I wonder if anything in OVS actually cares about this anymore? I don't > know the history of that comment. I don't remember the origin anymore but it was probably my comment. It's possible that some code in OVS assumed that packets were at least 64 bytes long at some point. For example, flow_extract() might have been able to be slightly simplified on the basis that access to the IPv4 header couldn't be beyond the end of the buffer. I doubt we do that kind of optimization any longer.
On 27/07/2016 14:18, "Ben Pfaff" <blp@ovn.org> wrote: >On Wed, Jul 27, 2016 at 01:51:00PM -0700, Jesse Gross wrote: >> On Wed, Jul 27, 2016 at 1:40 PM, Daniele Di Proietto >> <diproiettod@vmware.com> wrote: >> > On 27/07/2016 13:12, "Joe Stringer" <joe@ovn.org> wrote: >> > >> >>On 26 July 2016 at 17:58, Daniele Di Proietto <diproiettod@vmware.com> wrote: >> >>> The userspace connection tracker doesn't support ALGs, frag reassembly >> >>> or NAT yet, so skip those tests. >> >>> >> >>> Also, connection tracking state input from a local port is not possible >> >>> in userspace. >> >>> >> >>> The userspace datapath pads all frames with 0, to make them at >> >>> least 64 bytes. >> >>> >> >>> Finally, the userspace datapath checks for the IPv4 header checksum, so >> >>> fix those in the hardcoded packets. >> >>> >> >>> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> >> >>> Acked-by: Joe Stringer <joe@ovn.org> >> >>> Acked-by: Flavio Leitner <fbl@sysclose.org> >> >>> --- >> >> >> >><snip> >> >> >> >>> @@ -1324,11 +1327,11 @@ dnl UDP packets from ns0->ns1 should solicit "destination unreachable" response. >> >>> NS_CHECK_EXEC([at_ns0], [bash -c "echo a | nc $NC_EOF_OPT -u 10.1.1.2 10000"]) >> >>> >> >>> AT_CHECK([ovs-appctl revalidator/purge], [0]) >> >>> -AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort | grep -v drop], [0], [dnl >> >>> - n_packets=1, n_bytes=44, priority=100,udp,in_port=1 actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2 >> >>> - n_packets=1, n_bytes=72, priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1 >> >>> - n_packets=1, n_bytes=72, priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0) >> >>> - n_packets=2, n_bytes=84, priority=10,arp actions=NORMAL >> >>> +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort | grep -v drop | sed -e 's/n_bytes=[[0-9]]*/n_bytes=<cleared>/g'], [0], [dnl >> >>> + n_packets=1, n_bytes=<cleared>, priority=100,udp,in_port=1 actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2 >> >>> + n_packets=1, n_bytes=<cleared>, priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1 >> >>> + n_packets=1, n_bytes=<cleared>, priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0) >> >>> + n_packets=2, n_bytes=<cleared>, priority=10,arp actions=NORMAL >> >>> NXST_FLOW reply: >> >>> ]) >> >> >> >>I think this is a completely orthogonal point, but it's still a bit >> >>surprising to me that the n_bytes would differ when receiving short >> >>packets in kernel vs. userspace datapaths. I follow that userspace >> >>pads shorter packets on receive, but shouldn't we be able to attribute >> >>these stats consistently, regardless of the datapath? >> > >> > That's a good point. >> > >> > We call dp_packet_pad() in netdev_linux_recv(). That used to be in netdev_recv() and can be traced back to the initial OVS commit. Here's a comment in netdev_recv(): >> > >> > COVERAGE_INC(netdev_received); >> > buffer->size += n_bytes; >> > >> > /* When the kernel internally sends out an Ethernet frame on an >> > * interface, it gives us a copy *before* padding the frame to the >> > * minimum length. Thus, when it sends out something like an ARP >> > * request, we see a too-short frame. So pad it out to the minimum >> > * length. */ >> > pad_to_minimum_length(buffer); >> >> I wonder if anything in OVS actually cares about this anymore? I don't >> know the history of that comment. > >I don't remember the origin anymore but it was probably my comment. >It's possible that some code in OVS assumed that packets were at least >64 bytes long at some point. For example, flow_extract() might have >been able to be slightly simplified on the basis that access to the IPv4 >header couldn't be beyond the end of the buffer. > >I doubt we do that kind of optimization any longer. Thanks for the clarifying. I agree, it doesn't look like it's needed anymore. I've sent a couple of patches to remove that, along with the connection tracker system tests for userspace here: http://openvswitch.org/pipermail/dev/2016-July/076659.html
diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at index 2134db7..e1b5707 100644 --- a/tests/system-kmod-macros.at +++ b/tests/system-kmod-macros.at @@ -67,3 +67,31 @@ m4_define([CHECK_CONNTRACK], on_exit 'ovstest test-netlink-conntrack flush' ] ) + +# CHECK_CONNTRACK_ALG() +# +# Perform requirements checks for running conntrack ALG tests. The kernel +# supports ALG, so no check is needed. +# +m4_define([CHECK_CONNTRACK_ALG]) + +# CHECK_CONNTRACK_FRAG() +# +# Perform requirements checks for running conntrack fragmentations tests. +# The kernel always supports fragmentation, so no check is needed. +m4_define([CHECK_CONNTRACK_FRAG]) + +# CHECK_CONNTRACK_LOCAL_STACK() +# +# Perform requirements checks for running conntrack tests with local stack. +# The kernel always supports reading the connection state of an skb coming +# from an internal port, without an explicit ct() action, so no check is +# needed. +m4_define([CHECK_CONNTRACK_LOCAL_STACK]) + +# CHECK_CONNTRACK_NAT() +# +# Perform requirements checks for running conntrack NAT tests. The kernel +# always supports NAT, so no check is needed. +# +m4_define([CHECK_CONNTRACK_NAT]) diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 13f380f..c043f74 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -2,6 +2,7 @@ AT_SETUP([ovn -- 2 LRs connected via LS, gateway router, NAT]) AT_KEYWORDS([ovnnat]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() ovn_start OVS_TRAFFIC_VSWITCHD_START() ADD_BR([br-int]) @@ -111,7 +112,7 @@ NS_CHECK_EXEC([alice1], [ping -q -c 3 -i 0.3 -w 2 30.0.0.2 | FORMAT_PING], \ # Check conntrack entries. AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.2) | \ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl -icmp,orig=(src=172.16.1.2,dst=30.0.0.2,id=<cleared>),reply=(src=192.168.1.2,dst=172.16.1.2,id=<cleared>),zone=<cleared> +icmp,orig=(src=172.16.1.2,dst=30.0.0.2,id=<cleared>,type=8,code=0),reply=(src=192.168.1.2,dst=172.16.1.2,id=<cleared>,type=0,code=0),zone=<cleared> ]) # South-North SNAT: 'bar1' pings 'alice1'. But 'alice1' receives traffic @@ -124,7 +125,7 @@ NS_CHECK_EXEC([bar1], [ping -q -c 3 -i 0.3 -w 2 172.16.1.2 | FORMAT_PING], \ # We verify that SNAT indeed happened via 'dump-conntrack' command. AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.1) | \ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl -icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=<cleared>),reply=(src=172.16.1.2,dst=30.0.0.1,id=<cleared>),zone=<cleared> +icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=<cleared>,type=8,code=0),reply=(src=172.16.1.2,dst=30.0.0.1,id=<cleared>,type=0,code=0),zone=<cleared> ]) # Add static routes to handle east-west NAT. @@ -143,14 +144,14 @@ NS_CHECK_EXEC([bar1], [ping -q -c 3 -i 0.3 -w 2 30.0.0.2 | FORMAT_PING], \ # 30.0.0.2 to R2, it hits the DNAT rule and converts 30.0.0.2 to 192.168.1.2 AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.2) | \ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl -icmp,orig=(src=192.168.2.2,dst=30.0.0.2,id=<cleared>),reply=(src=192.168.1.2,dst=192.168.2.2,id=<cleared>),zone=<cleared> +icmp,orig=(src=192.168.2.2,dst=30.0.0.2,id=<cleared>,type=8,code=0),reply=(src=192.168.1.2,dst=192.168.2.2,id=<cleared>,type=0,code=0),zone=<cleared> ]) # As we have a SNAT rule that converts 192.168.2.2 to 30.0.0.1, the source is # SNATted and 'foo1' receives it. AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(30.0.0.1) | \ sed -e 's/zone=[[0-9]]*/zone=<cleared>/'], [0], [dnl -icmp,orig=(src=192.168.2.2,dst=192.168.1.2,id=<cleared>),reply=(src=192.168.1.2,dst=30.0.0.1,id=<cleared>),zone=<cleared> +icmp,orig=(src=192.168.2.2,dst=192.168.1.2,id=<cleared>,type=8,code=0),reply=(src=192.168.1.2,dst=30.0.0.1,id=<cleared>,type=0,code=0),zone=<cleared> ]) OVS_APP_EXIT_AND_WAIT([ovn-controller]) @@ -173,6 +174,7 @@ AT_SETUP([ovn -- load-balancing]) AT_KEYWORDS([ovnlb]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() ovn_start OVS_TRAFFIC_VSWITCHD_START() ADD_BR([br-int]) diff --git a/tests/system-traffic.at b/tests/system-traffic.at index a337950..0b4b4b7 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -510,13 +510,13 @@ AT_CAPTURE_FILE([ofctl_monitor.log]) AT_CHECK([ovs-ofctl monitor br0 65534 invalid_ttl --detach --no-chdir --pidfile 2> ofctl_monitor.log]) dnl Send an unsolicited reply from port 2. This should be dropped. -AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 2 ct\(table=0\) '50540000000a50540000000908004500001c00000000001100000a0101020a0101010002000100080000']) +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 2 ct\(table=0\) '50540000000a50540000000908004500001c000000000011a4cd0a0101020a0101010002000100080000']) dnl OK, now start a new connection from port 1. -AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 1 ct\(commit\),controller '50540000000a50540000000908004500001c00000000001100000a0101010a0101020001000200080000']) +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 1 ct\(commit\),controller '50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000']) dnl Now try a reply from port 2. -AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 2 ct\(table=0\) '50540000000a50540000000908004500001c00000000001100000a0101020a0101010002000100080000']) +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 2 ct\(table=0\) '50540000000a50540000000908004500001c000000000011a4cd0a0101020a0101010002000100080000']) dnl Check this output. We only see the latter two packets, not the first. AT_CHECK([cat ofctl_monitor.log], [0], [dnl @@ -906,6 +906,7 @@ AT_CLEANUP AT_SETUP([conntrack - multiple zones, local]) CHECK_CONNTRACK() +CHECK_CONNTRACK_LOCAL_STACK() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0) @@ -953,6 +954,7 @@ AT_CLEANUP AT_SETUP([conntrack - multiple namespaces, internal ports]) CHECK_CONNTRACK() +CHECK_CONNTRACK_LOCAL_STACK() OVS_TRAFFIC_VSWITCHD_START( [set-fail-mode br0 secure -- ]) @@ -993,6 +995,7 @@ AT_CLEANUP AT_SETUP([conntrack - multi-stage pipeline, local]) CHECK_CONNTRACK() +CHECK_CONNTRACK_LOCAL_STACK() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0) @@ -1324,11 +1327,11 @@ dnl UDP packets from ns0->ns1 should solicit "destination unreachable" response. NS_CHECK_EXEC([at_ns0], [bash -c "echo a | nc $NC_EOF_OPT -u 10.1.1.2 10000"]) AT_CHECK([ovs-appctl revalidator/purge], [0]) -AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort | grep -v drop], [0], [dnl - n_packets=1, n_bytes=44, priority=100,udp,in_port=1 actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2 - n_packets=1, n_bytes=72, priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1 - n_packets=1, n_bytes=72, priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0) - n_packets=2, n_bytes=84, priority=10,arp actions=NORMAL +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort | grep -v drop | sed -e 's/n_bytes=[[0-9]]*/n_bytes=<cleared>/g'], [0], [dnl + n_packets=1, n_bytes=<cleared>, priority=100,udp,in_port=1 actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2 + n_packets=1, n_bytes=<cleared>, priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1 + n_packets=1, n_bytes=<cleared>, priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0) + n_packets=2, n_bytes=<cleared>, priority=10,arp actions=NORMAL NXST_FLOW reply: ]) @@ -1382,6 +1385,7 @@ AT_CLEANUP AT_SETUP([conntrack - FTP]) AT_SKIP_IF([test $HAVE_PYFTPDLIB = no]) CHECK_CONNTRACK() +CHECK_CONNTRACK_ALG() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -1468,6 +1472,7 @@ AT_CLEANUP AT_SETUP([conntrack - IPv6 FTP]) AT_SKIP_IF([test $HAVE_PYFTPDLIB = no]) CHECK_CONNTRACK() +CHECK_CONNTRACK_ALG() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -1524,6 +1529,7 @@ AT_CLEANUP AT_SETUP([conntrack - FTP with multiple expectations]) AT_SKIP_IF([test $HAVE_PYFTPDLIB = no]) CHECK_CONNTRACK() +CHECK_CONNTRACK_ALG() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -1588,6 +1594,7 @@ AT_CLEANUP AT_SETUP([conntrack - IPv4 fragmentation ]) CHECK_CONNTRACK() +CHECK_CONNTRACK_FRAG() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -1621,6 +1628,7 @@ AT_CLEANUP AT_SETUP([conntrack - IPv4 fragmentation expiry]) CHECK_CONNTRACK() +CHECK_CONNTRACK_FRAG() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -1651,6 +1659,7 @@ AT_CLEANUP AT_SETUP([conntrack - IPv4 fragmentation + vlan]) CHECK_CONNTRACK() +CHECK_CONNTRACK_FRAG() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -1686,6 +1695,7 @@ AT_CLEANUP AT_SETUP([conntrack - IPv6 fragmentation]) CHECK_CONNTRACK() +CHECK_CONNTRACK_FRAG() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -1725,6 +1735,7 @@ AT_CLEANUP AT_SETUP([conntrack - IPv6 fragmentation expiry]) CHECK_CONNTRACK() +CHECK_CONNTRACK_FRAG() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -1765,6 +1776,7 @@ AT_CLEANUP AT_SETUP([conntrack - IPv6 fragmentation + vlan]) CHECK_CONNTRACK() +CHECK_CONNTRACK_FRAG() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -1808,6 +1820,7 @@ AT_CLEANUP AT_SETUP([conntrack - Fragmentation over vxlan]) OVS_CHECK_VXLAN() CHECK_CONNTRACK() +CHECK_CONNTRACK_FRAG() OVS_TRAFFIC_VSWITCHD_START() ADD_BR([br-underlay]) @@ -1859,6 +1872,7 @@ AT_CLEANUP AT_SETUP([conntrack - IPv6 Fragmentation over vxlan]) OVS_CHECK_VXLAN() CHECK_CONNTRACK() +CHECK_CONNTRACK_FRAG() OVS_TRAFFIC_VSWITCHD_START() ADD_BR([br-underlay]) @@ -1942,12 +1956,12 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 1 10.1.1.2 | FORMAT_PING], [0], [dnl 1 packets transmitted, 0 received, 100% packet loss, time 0ms ]) -AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort], [0], [dnl - n_packets=1, n_bytes=98, priority=100,ip,in_port=1 actions=resubmit(,1),resubmit(,2) - n_packets=2, n_bytes=84, priority=150,arp actions=NORMAL - table=1, n_packets=1, n_bytes=98, ip actions=ct(table=3) - table=2, n_packets=1, n_bytes=98, ip actions=ct(table=3) - table=3, n_packets=2, n_bytes=196, ip actions=drop +AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort | sed -e 's/n_bytes=[[0-9]]*/n_bytes=<cleared>/g'], [0], [dnl + n_packets=1, n_bytes=<cleared>, priority=100,ip,in_port=1 actions=resubmit(,1),resubmit(,2) + n_packets=2, n_bytes=<cleared>, priority=150,arp actions=NORMAL + table=1, n_packets=1, n_bytes=<cleared>, ip actions=ct(table=3) + table=2, n_packets=1, n_bytes=<cleared>, ip actions=ct(table=3) + table=3, n_packets=2, n_bytes=<cleared>, ip actions=drop NXST_FLOW reply: ]) @@ -1957,6 +1971,7 @@ AT_CLEANUP AT_SETUP([conntrack - simple SNAT]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -2003,6 +2018,7 @@ AT_CLEANUP AT_SETUP([conntrack - SNAT with port range]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -2050,6 +2066,7 @@ AT_CLEANUP AT_SETUP([conntrack - more complex SNAT]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -2101,6 +2118,7 @@ AT_CLEANUP AT_SETUP([conntrack - simple DNAT]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -2155,6 +2173,7 @@ AT_CLEANUP AT_SETUP([conntrack - more complex DNAT]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -2215,6 +2234,7 @@ AT_CLEANUP AT_SETUP([conntrack - ICMP related with NAT]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -2276,6 +2296,7 @@ AT_CLEANUP AT_SETUP([conntrack - FTP with NAT]) AT_SKIP_IF([test $HAVE_PYFTPDLIB = no]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() @@ -2357,6 +2378,7 @@ AT_CLEANUP AT_SETUP([conntrack - FTP with NAT 2]) AT_SKIP_IF([test $HAVE_PYFTPDLIB = no]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -2424,6 +2446,7 @@ AT_CLEANUP AT_SETUP([conntrack - IPv6 HTTP with NAT]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -2467,6 +2490,7 @@ AT_CLEANUP AT_SETUP([conntrack - IPv6 FTP with NAT]) AT_SKIP_IF([test $HAVE_PYFTPDLIB = no]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns0, at_ns1) @@ -2524,6 +2548,7 @@ AT_CLEANUP AT_SETUP([conntrack - DNAT load balancing]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns1, at_ns2, at_ns3, at_ns4) @@ -2610,6 +2635,7 @@ AT_CLEANUP AT_SETUP([conntrack - DNAT load balancing with NC]) CHECK_CONNTRACK() +CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() ADD_NAMESPACES(at_ns1, at_ns2, at_ns3, at_ns4, at_ns5) diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at index c09a4aa..213425f 100644 --- a/tests/system-userspace-macros.at +++ b/tests/system-userspace-macros.at @@ -60,9 +60,48 @@ m4_define([CONFIGURE_VETH_OFFLOADS], # CHECK_CONNTRACK() # -# Perform requirements checks for running conntrack tests, and flush the -# kernel conntrack tables when the test is finished. +# Perform requirements checks for running conntrack tests. # m4_define([CHECK_CONNTRACK], - [AT_SKIP_IF(true)] + [AT_SKIP_IF([test $HAVE_PYTHON = no])] ) + +# CHECK_CONNTRACK_ALG() +# +# Perform requirements checks for running conntrack ALG tests. The userspace +# doesn't support ALGs yet, so skip the tests +# +m4_define([CHECK_CONNTRACK_ALG], +[ + AT_SKIP_IF([:]) +]) + +# CHECK_CONNTRACK_FRAG() +# +# Perform requirements checks for running conntrack fragmentations tests. +# The userspace doesn't support fragmentation yet, so skip the tests. +m4_define([CHECK_CONNTRACK_FRAG], +[ + AT_SKIP_IF([:]) +]) + +# CHECK_CONNTRACK_LOCAL_STACK() +# +# Perform requirements checks for running conntrack tests with local stack. +# While the kernel connection tracker automatically passes all the connection +# tracking state from an internal port to the OpenvSwitch kernel module, there +# is simply no way of doing that with the userspace, so skip the tests. +m4_define([CHECK_CONNTRACK_LOCAL_STACK], +[ + AT_SKIP_IF([:]) +]) + +# CHECK_CONNTRACK_NAT() +# +# Perform requirements checks for running conntrack NAT tests. The userspace +# doesn't support NATs yet, so skip the tests +# +m4_define([CHECK_CONNTRACK_NAT], +[ + AT_SKIP_IF([:]) +])