Re: [ovs-dev] [PATCH v5 13/16] system-tests: Run conntrack tests with userspace.

2016-07-28 Thread Daniele Di Proietto 





On 27/07/2016 14:18, "Ben Pfaff"  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
>>  wrote:
>> > On 27/07/2016 13:12, "Joe Stringer"  wrote:
>> >
>> >>On 26 July 2016 at 17:58, Daniele Di Proietto  
>> >>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 
>> >>> Acked-by: Joe Stringer 
>> >>> Acked-by: Flavio Leitner 
>> >>> ---
>> >>
>> >>
>> >>
>> >>> @@ -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 
>> >>> 1"])
>> >>>
>> >>>  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=/g'], [0], [dnl
>> >>> + n_packets=1, n_bytes=, priority=100,udp,in_port=1 
>> >>> actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2
>> >>> + n_packets=1, n_bytes=, 
>> >>> priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 
>> >>> actions=output:1
>> >>> + n_packets=1, n_bytes=, 
>> >>> priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0)
>> >>> + n_packets=2, n_bytes=, 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
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v5 13/16] system-tests: Run conntrack tests with userspace.

2016-07-27 Thread Ben Pfaff 
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
>  wrote:
> > On 27/07/2016 13:12, "Joe Stringer"  wrote:
> >
> >>On 26 July 2016 at 17:58, Daniele Di Proietto  
> >>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 
> >>> Acked-by: Joe Stringer 
> >>> Acked-by: Flavio Leitner 
> >>> ---
> >>
> >>
> >>
> >>> @@ -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 
> >>> 1"])
> >>>
> >>>  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=/g'], [0], [dnl
> >>> + n_packets=1, n_bytes=, priority=100,udp,in_port=1 
> >>> actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2
> >>> + n_packets=1, n_bytes=, 
> >>> priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1
> >>> + n_packets=1, n_bytes=, 
> >>> priority=100,ct_state=-trk,icmp,in_port=2 actions=ct(table=0)
> >>> + n_packets=2, n_bytes=, 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.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v5 13/16] system-tests: Run conntrack tests with userspace.

2016-07-27 Thread Jesse Gross 
On Wed, Jul 27, 2016 at 1:40 PM, Daniele Di Proietto
 wrote:
> On 27/07/2016 13:12, "Joe Stringer"  wrote:
>
>>On 26 July 2016 at 17:58, Daniele Di Proietto  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 
>>> Acked-by: Joe Stringer 
>>> Acked-by: Flavio Leitner 
>>> ---
>>
>>
>>
>>> @@ -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 
>>> 1"])
>>>
>>>  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=/g'], [0], [dnl
>>> + n_packets=1, n_bytes=, priority=100,udp,in_port=1 
>>> actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2
>>> + n_packets=1, n_bytes=, 
>>> priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1
>>> + n_packets=1, n_bytes=, priority=100,ct_state=-trk,icmp,in_port=2 
>>> actions=ct(table=0)
>>> + n_packets=2, n_bytes=, 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.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v5 13/16] system-tests: Run conntrack tests with userspace.

2016-07-27 Thread Daniele Di Proietto 





On 27/07/2016 13:12, "Joe Stringer"  wrote:

>On 26 July 2016 at 17:58, Daniele Di Proietto  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 
>> Acked-by: Joe Stringer 
>> Acked-by: Flavio Leitner 
>> ---
>
>
>
>> @@ -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 
>> 1"])
>>
>>  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=/g'], [0], [dnl
>> + n_packets=1, n_bytes=, priority=100,udp,in_port=1 
>> actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2
>> + n_packets=1, n_bytes=, 
>> priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1
>> + n_packets=1, n_bytes=, priority=100,ct_state=-trk,icmp,in_port=2 
>> actions=ct(table=0)
>> + n_packets=2, n_bytes=, 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);
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH v5 13/16] system-tests: Run conntrack tests with userspace.

2016-07-27 Thread Joe Stringer 
On 26 July 2016 at 17:58, Daniele Di Proietto  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 
> Acked-by: Joe Stringer 
> Acked-by: Flavio Leitner 
> ---



> @@ -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 
> 1"])
>
>  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=/g'], [0], [dnl
> + n_packets=1, n_bytes=, priority=100,udp,in_port=1 
> actions=ct(commit,exec(load:0x1->NXM_NX_CT_MARK[[]])),output:2
> + n_packets=1, n_bytes=, 
> priority=100,ct_state=+rel+trk,ct_mark=0x1,icmp,in_port=2 actions=output:1
> + n_packets=1, n_bytes=, priority=100,ct_state=-trk,icmp,in_port=2 
> actions=ct(table=0)
> + n_packets=2, n_bytes=, 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?
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH v5 13/16] system-tests: Run conntrack tests with userspace.

2016-07-26 Thread Daniele Di Proietto 
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 
Acked-by: Joe Stringer 
Acked-by: Flavio Leitner 
---
 tests/system-kmod-macros.at  | 28 +
 tests/system-ovn.at  | 10 +---
 tests/system-traffic.at  | 54 +---
 tests/system-userspace-macros.at | 45 ++---
 4 files changed, 116 insertions(+), 21 deletions(-)

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=/'], [0], [dnl
-icmp,orig=(src=172.16.1.2,dst=30.0.0.2,id=),reply=(src=192.168.1.2,dst=172.16.1.2,id=),zone=
+icmp,orig=(src=172.16.1.2,dst=30.0.0.2,id=,type=8,code=0),reply=(src=192.168.1.2,dst=172.16.1.2,id=,type=0,code=0),zone=
 ])
 
 # 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=/'], [0], [dnl
-icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=),reply=(src=172.16.1.2,dst=30.0.0.1,id=),zone=
+icmp,orig=(src=192.168.2.2,dst=172.16.1.2,id=,type=8,code=0),reply=(src=172.16.1.2,dst=30.0.0.1,id=,type=0,code=0),zone=
 ])
 
 # 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=/'], [0], [dnl
-icmp,orig=(src=192.168.2.2,dst=30.0.0.2,id=),reply=(src=192.168.1.2,dst=192.168.2.2,id=),zone=
+icmp,orig=(src=192.168.2.2,dst=30.0.0.2,id=,type=8,code=0),reply=(src=192.168.1.2,dst=192.168.2.2,id=,type=0,code=0),zone=
 ])
 
 # 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=/'], [0], [dnl
-icmp,orig=(src=192.168.2.2,dst=192.168.1.2,id=),reply=(src=192.168.1.2,dst=30.0.0.1,id=),zone=
+icmp,orig=(src=192.168.2.2,dst=192.168.1.2,id=,type=8,code=0),reply=(src=192.168.1.2,dst=30.0.0.1,id=,type=0,code=0),zone=
 ])
 
 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