| Message ID | 20210210164933.1167136-1-aconole@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] conntrack: Add a test for IPv4 UDP zero checksum | expand |
On Wed, Feb 10, 2021 at 11:49:33AM -0500, Aaron Conole wrote: > Recently, during some conntrack testing a bug was uncovered in a DPDK > PMD, which doesn't support an IPv4 packet with a zero checksum value. > In order to show that the connection tracking code in userspace > supports IPv4 UDP with a zero checksum, add a test case to enforce > this behavior. > > Reported-at: http://mails.dpdk.org/archives/dev/2021-January/198528.html > Reported-by: Paolo Valerio <pvalerio@redhat.com> > Signed-off-by: Aaron Conole <aconole@redhat.com> > --- > tests/system-traffic.at | 43 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 43 insertions(+) > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index fb5b9a36d2..4971ccc966 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -5927,6 +5927,48 @@ ovs-appctl dpif/dump-flows br0 > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > + > +AT_SETUP([conntrack - IPv4 UDP zero checksum]) > +dnl This tracks sending zero checksum packets for udp over ipv4 > +CHECK_CONNTRACK() > +OVS_TRAFFIC_VSWITCHD_START() > +OVS_CHECK_CT_CLEAR() > + > +ADD_NAMESPACES(at_ns0, at_ns1) > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "f0:00:00:01:01:01") > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "f0:00:00:01:01:02") > +dnl setup ct flows > +AT_DATA([flows.txt], [dnl > +table=0,priority=10 ip,udp,ct_state=-trk action=ct(zone=1,table=1) > +table=0,priority=0 action=drop > +table=1,priority=10 ct_state=-est+trk+new,ip,ct_zone=1,in_port=1 action=ct(commit,table=2) > +table=1,priority=10 ct_state=+est-new+trk,ct_zone=1,in_port=1 action=resubmit(,2) > +table=1,priority=0 action=drop > +table=2,priority=10 ct_state=+trk+new,in_port=1 action=2 > +table=2,priority=10 ct_state=+trk+est action=2 > +]) > + > +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) > + > +# sending udp pkt > +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f0 00 00 01 01 02 f0 00 00 01 01 01 08 00 45 00 00 28 00 01 00 00 40 11 64 c0 0a 01 01 01 0a 01 01 02 04 d2 04 d2 00 14 00 00 aa aa aa aa aa aa aa aa aa aa aa aa > /dev/null]) That hex string translates to this packet which doesn't use cksum: 12:57:40.065353 IP (tos 0x0, ttl 64, id 1, offset 0, flags [none], proto UDP (17), length 40) 10.1.1.1.search-agent > 10.1.1.2.search-agent: [no cksum] UDP, length 12 > + > +sleep 1 Can we use OVS_WAIT_UNTIL() or OVS_WAIT_WHILE() ? > + > +dnl ensure CT picked up the packet > +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1)], [0], [dnl > +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>) > +]) > + > +AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION_IDLE], > + [0], [dnl > + cookie=0x0, duration=<cleared>, table=2, n_packets=1, n_bytes=54, idle_age=<cleared>, priority=10,ct_state=+new+trk,in_port=1 actions=output:2 > + cookie=0x0, duration=<cleared>, table=2, n_packets=0, n_bytes=0, idle_age=<cleared>, priority=10,ct_state=+est+trk actions=output:2 > +]) > + > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP > + > AT_SETUP([conntrack - Multiple ICMP traverse]) > dnl This tracks sending ICMP packets via conntrack multiple times for the > dnl same packet > @@ -5971,6 +6013,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION_IDLE > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > + There mixed styles. Some use two lines, some use single line and some use two lines with a dnl ----- or ^L. I guess we should stick to single line, but I don't have a strong preference. Otherwise the patch works for me. Thanks, fbl > AT_BANNER([802.1ad]) > > AT_SETUP([802.1ad - vlan_limit]) > -- > 2.25.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Flavio Leitner <fbl@sysclose.org> writes: > On Wed, Feb 10, 2021 at 11:49:33AM -0500, Aaron Conole wrote: >> Recently, during some conntrack testing a bug was uncovered in a DPDK >> PMD, which doesn't support an IPv4 packet with a zero checksum value. >> In order to show that the connection tracking code in userspace >> supports IPv4 UDP with a zero checksum, add a test case to enforce >> this behavior. >> >> Reported-at: http://mails.dpdk.org/archives/dev/2021-January/198528.html >> Reported-by: Paolo Valerio <pvalerio@redhat.com> >> Signed-off-by: Aaron Conole <aconole@redhat.com> >> --- >> tests/system-traffic.at | 43 +++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 43 insertions(+) >> >> diff --git a/tests/system-traffic.at b/tests/system-traffic.at >> index fb5b9a36d2..4971ccc966 100644 >> --- a/tests/system-traffic.at >> +++ b/tests/system-traffic.at >> @@ -5927,6 +5927,48 @@ ovs-appctl dpif/dump-flows br0 >> OVS_TRAFFIC_VSWITCHD_STOP >> AT_CLEANUP >> >> + >> +AT_SETUP([conntrack - IPv4 UDP zero checksum]) >> +dnl This tracks sending zero checksum packets for udp over ipv4 >> +CHECK_CONNTRACK() >> +OVS_TRAFFIC_VSWITCHD_START() >> +OVS_CHECK_CT_CLEAR() >> + >> +ADD_NAMESPACES(at_ns0, at_ns1) >> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "f0:00:00:01:01:01") >> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "f0:00:00:01:01:02") >> +dnl setup ct flows >> +AT_DATA([flows.txt], [dnl >> +table=0,priority=10 ip,udp,ct_state=-trk action=ct(zone=1,table=1) >> +table=0,priority=0 action=drop >> +table=1,priority=10 ct_state=-est+trk+new,ip,ct_zone=1,in_port=1 action=ct(commit,table=2) >> +table=1,priority=10 ct_state=+est-new+trk,ct_zone=1,in_port=1 action=resubmit(,2) >> +table=1,priority=0 action=drop >> +table=2,priority=10 ct_state=+trk+new,in_port=1 action=2 >> +table=2,priority=10 ct_state=+trk+est action=2 >> +]) >> + >> +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) >> + >> +# sending udp pkt >> +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f0 00 00 01 >> 01 02 f0 00 00 01 01 01 08 00 45 00 00 28 00 01 00 00 40 11 64 c0 0a >> 01 01 01 0a 01 01 02 04 d2 04 d2 00 14 00 00 aa aa aa aa aa aa aa aa >> aa aa aa aa > /dev/null]) > > That hex string translates to this packet which doesn't use cksum: > 12:57:40.065353 IP (tos 0x0, ttl 64, id 1, offset 0, flags [none], > proto UDP (17), length 40) > 10.1.1.1.search-agent > 10.1.1.2.search-agent: [no cksum] UDP, > length 12 Yes, there should be no UDP checksum. This is the point of this test. >> + >> +sleep 1 > > Can we use OVS_WAIT_UNTIL() or OVS_WAIT_WHILE() ? Good idea. >> + >> +dnl ensure CT picked up the packet >> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1)], [0], [dnl >> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>) >> +]) >> + >> +AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION_IDLE], >> + [0], [dnl >> + cookie=0x0, duration=<cleared>, table=2, n_packets=1, n_bytes=54, >> idle_age=<cleared>, priority=10,ct_state=+new+trk,in_port=1 >> actions=output:2 >> + cookie=0x0, duration=<cleared>, table=2, n_packets=0, n_bytes=0, >> idle_age=<cleared>, priority=10,ct_state=+est+trk actions=output:2 >> +]) >> + >> +OVS_TRAFFIC_VSWITCHD_STOP >> +AT_CLEANUP >> + >> AT_SETUP([conntrack - Multiple ICMP traverse]) >> dnl This tracks sending ICMP packets via conntrack multiple times for the >> dnl same packet >> @@ -5971,6 +6013,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION_IDLE >> OVS_TRAFFIC_VSWITCHD_STOP >> AT_CLEANUP >> >> + > > There mixed styles. Some use two lines, some use single line > and some use two lines with a dnl ----- or ^L. > > I guess we should stick to single line, but I don't have a > strong preference. Oops, this snuck in. I will remove it, and we can do something there with a different patch if someone thinks it is needed. > Otherwise the patch works for me. > > Thanks, > fbl > >> AT_BANNER([802.1ad]) >> >> AT_SETUP([802.1ad - vlan_limit]) >> -- >> 2.25.4 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Wed, Feb 10, 2021 at 02:45:06PM -0500, Aaron Conole wrote: > Flavio Leitner <fbl@sysclose.org> writes: > > > On Wed, Feb 10, 2021 at 11:49:33AM -0500, Aaron Conole wrote: > >> Recently, during some conntrack testing a bug was uncovered in a DPDK > >> PMD, which doesn't support an IPv4 packet with a zero checksum value. > >> In order to show that the connection tracking code in userspace > >> supports IPv4 UDP with a zero checksum, add a test case to enforce > >> this behavior. > >> > >> Reported-at: http://mails.dpdk.org/archives/dev/2021-January/198528.html > >> Reported-by: Paolo Valerio <pvalerio@redhat.com> > >> Signed-off-by: Aaron Conole <aconole@redhat.com> > >> --- > >> tests/system-traffic.at | 43 +++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 43 insertions(+) > >> > >> diff --git a/tests/system-traffic.at b/tests/system-traffic.at > >> index fb5b9a36d2..4971ccc966 100644 > >> --- a/tests/system-traffic.at > >> +++ b/tests/system-traffic.at > >> @@ -5927,6 +5927,48 @@ ovs-appctl dpif/dump-flows br0 > >> OVS_TRAFFIC_VSWITCHD_STOP > >> AT_CLEANUP > >> > >> + > >> +AT_SETUP([conntrack - IPv4 UDP zero checksum]) > >> +dnl This tracks sending zero checksum packets for udp over ipv4 > >> +CHECK_CONNTRACK() > >> +OVS_TRAFFIC_VSWITCHD_START() > >> +OVS_CHECK_CT_CLEAR() > >> + > >> +ADD_NAMESPACES(at_ns0, at_ns1) > >> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "f0:00:00:01:01:01") > >> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "f0:00:00:01:01:02") > >> +dnl setup ct flows > >> +AT_DATA([flows.txt], [dnl > >> +table=0,priority=10 ip,udp,ct_state=-trk action=ct(zone=1,table=1) > >> +table=0,priority=0 action=drop > >> +table=1,priority=10 ct_state=-est+trk+new,ip,ct_zone=1,in_port=1 action=ct(commit,table=2) > >> +table=1,priority=10 ct_state=+est-new+trk,ct_zone=1,in_port=1 action=resubmit(,2) > >> +table=1,priority=0 action=drop > >> +table=2,priority=10 ct_state=+trk+new,in_port=1 action=2 > >> +table=2,priority=10 ct_state=+trk+est action=2 > >> +]) > >> + > >> +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) > >> + > >> +# sending udp pkt > >> +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f0 00 00 01 > >> 01 02 f0 00 00 01 01 01 08 00 45 00 00 28 00 01 00 00 40 11 64 c0 0a > >> 01 01 01 0a 01 01 02 04 d2 04 d2 00 14 00 00 aa aa aa aa aa aa aa aa > >> aa aa aa aa > /dev/null]) > > > > That hex string translates to this packet which doesn't use cksum: > > 12:57:40.065353 IP (tos 0x0, ttl 64, id 1, offset 0, flags [none], > > proto UDP (17), length 40) > > 10.1.1.1.search-agent > 10.1.1.2.search-agent: [no cksum] UDP, > > length 12 > > Yes, there should be no UDP checksum. This is the point of this test. Sorry, I was confirming that the hex string does use a packet as this patch says. It's good. > >> + > >> +sleep 1 > > > > Can we use OVS_WAIT_UNTIL() or OVS_WAIT_WHILE() ? > > Good idea. > > >> + > >> +dnl ensure CT picked up the packet > >> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1)], [0], [dnl > >> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>) > >> +]) > >> + > >> +AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION_IDLE], > >> + [0], [dnl > >> + cookie=0x0, duration=<cleared>, table=2, n_packets=1, n_bytes=54, > >> idle_age=<cleared>, priority=10,ct_state=+new+trk,in_port=1 > >> actions=output:2 > >> + cookie=0x0, duration=<cleared>, table=2, n_packets=0, n_bytes=0, > >> idle_age=<cleared>, priority=10,ct_state=+est+trk actions=output:2 > >> +]) > >> + > >> +OVS_TRAFFIC_VSWITCHD_STOP > >> +AT_CLEANUP > >> + > >> AT_SETUP([conntrack - Multiple ICMP traverse]) > >> dnl This tracks sending ICMP packets via conntrack multiple times for the > >> dnl same packet > >> @@ -5971,6 +6013,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION_IDLE > >> OVS_TRAFFIC_VSWITCHD_STOP > >> AT_CLEANUP > >> > >> + > > > > There mixed styles. Some use two lines, some use single line > > and some use two lines with a dnl ----- or ^L. > > > > I guess we should stick to single line, but I don't have a > > strong preference. > > Oops, this snuck in. I will remove it, and we can do something there > with a different patch if someone thinks it is needed. Cool, thanks fbl > > > Otherwise the patch works for me. > > > > Thanks, > > fbl > > > >> AT_BANNER([802.1ad]) > >> > >> AT_SETUP([802.1ad - vlan_limit]) > >> -- > >> 2.25.4 > >> > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi Aaron, Should we also consider the case where udp checksum is 0xffff? I saw in netdev_tnl_calc_udp_csum, we set to 0xffff when a packet has udp checksum = 0. Thanks William On Wed, Feb 10, 2021 at 12:15 PM Flavio Leitner <fbl@sysclose.org> wrote: > > On Wed, Feb 10, 2021 at 02:45:06PM -0500, Aaron Conole wrote: > > Flavio Leitner <fbl@sysclose.org> writes: > > > > > On Wed, Feb 10, 2021 at 11:49:33AM -0500, Aaron Conole wrote: > > >> Recently, during some conntrack testing a bug was uncovered in a DPDK > > >> PMD, which doesn't support an IPv4 packet with a zero checksum value. > > >> In order to show that the connection tracking code in userspace > > >> supports IPv4 UDP with a zero checksum, add a test case to enforce > > >> this behavior. > > >> > > >> Reported-at: http://mails.dpdk.org/archives/dev/2021-January/198528.html > > >> Reported-by: Paolo Valerio <pvalerio@redhat.com> > > >> Signed-off-by: Aaron Conole <aconole@redhat.com> > > >> --- > > >> tests/system-traffic.at | 43 +++++++++++++++++++++++++++++++++++++++++ > > >> 1 file changed, 43 insertions(+) > > >> > > >> diff --git a/tests/system-traffic.at b/tests/system-traffic.at > > >> index fb5b9a36d2..4971ccc966 100644 > > >> --- a/tests/system-traffic.at > > >> +++ b/tests/system-traffic.at > > >> @@ -5927,6 +5927,48 @@ ovs-appctl dpif/dump-flows br0 > > >> OVS_TRAFFIC_VSWITCHD_STOP > > >> AT_CLEANUP > > >> > > >> + > > >> +AT_SETUP([conntrack - IPv4 UDP zero checksum]) > > >> +dnl This tracks sending zero checksum packets for udp over ipv4 > > >> +CHECK_CONNTRACK() > > >> +OVS_TRAFFIC_VSWITCHD_START() > > >> +OVS_CHECK_CT_CLEAR() > > >> + > > >> +ADD_NAMESPACES(at_ns0, at_ns1) > > >> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "f0:00:00:01:01:01") > > >> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "f0:00:00:01:01:02") > > >> +dnl setup ct flows > > >> +AT_DATA([flows.txt], [dnl > > >> +table=0,priority=10 ip,udp,ct_state=-trk action=ct(zone=1,table=1) > > >> +table=0,priority=0 action=drop > > >> +table=1,priority=10 ct_state=-est+trk+new,ip,ct_zone=1,in_port=1 action=ct(commit,table=2) > > >> +table=1,priority=10 ct_state=+est-new+trk,ct_zone=1,in_port=1 action=resubmit(,2) > > >> +table=1,priority=0 action=drop > > >> +table=2,priority=10 ct_state=+trk+new,in_port=1 action=2 > > >> +table=2,priority=10 ct_state=+trk+est action=2 > > >> +]) > > >> + > > >> +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) > > >> + > > >> +# sending udp pkt > > >> +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f0 00 00 01 > > >> 01 02 f0 00 00 01 01 01 08 00 45 00 00 28 00 01 00 00 40 11 64 c0 0a > > >> 01 01 01 0a 01 01 02 04 d2 04 d2 00 14 00 00 aa aa aa aa aa aa aa aa > > >> aa aa aa aa > /dev/null]) > > > > > > That hex string translates to this packet which doesn't use cksum: > > > 12:57:40.065353 IP (tos 0x0, ttl 64, id 1, offset 0, flags [none], > > > proto UDP (17), length 40) > > > 10.1.1.1.search-agent > 10.1.1.2.search-agent: [no cksum] UDP, > > > length 12 > > > > Yes, there should be no UDP checksum. This is the point of this test. > > Sorry, I was confirming that the hex string does use a packet > as this patch says. It's good. > > > > >> + > > >> +sleep 1 > > > > > > Can we use OVS_WAIT_UNTIL() or OVS_WAIT_WHILE() ? > > > > Good idea. > > > > >> + > > >> +dnl ensure CT picked up the packet > > >> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1)], [0], [dnl > > >> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>) > > >> +]) > > >> + > > >> +AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION_IDLE], > > >> + [0], [dnl > > >> + cookie=0x0, duration=<cleared>, table=2, n_packets=1, n_bytes=54, > > >> idle_age=<cleared>, priority=10,ct_state=+new+trk,in_port=1 > > >> actions=output:2 > > >> + cookie=0x0, duration=<cleared>, table=2, n_packets=0, n_bytes=0, > > >> idle_age=<cleared>, priority=10,ct_state=+est+trk actions=output:2 > > >> +]) > > >> + > > >> +OVS_TRAFFIC_VSWITCHD_STOP > > >> +AT_CLEANUP > > >> + > > >> AT_SETUP([conntrack - Multiple ICMP traverse]) > > >> dnl This tracks sending ICMP packets via conntrack multiple times for the > > >> dnl same packet > > >> @@ -5971,6 +6013,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION_IDLE > > >> OVS_TRAFFIC_VSWITCHD_STOP > > >> AT_CLEANUP > > >> > > >> + > > > > > > There mixed styles. Some use two lines, some use single line > > > and some use two lines with a dnl ----- or ^L. > > > > > > I guess we should stick to single line, but I don't have a > > > strong preference. > > > > Oops, this snuck in. I will remove it, and we can do something there > > with a different patch if someone thinks it is needed. > > Cool, > thanks > fbl > > > > > > > Otherwise the patch works for me. > > > > > > Thanks, > > > fbl > > > > > >> AT_BANNER([802.1ad]) > > >> > > >> AT_SETUP([802.1ad - vlan_limit]) > > >> -- > > >> 2.25.4 > > >> > > >> _______________________________________________ > > >> dev mailing list > > >> dev@openvswitch.org > > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > -- > fbl > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
William Tu <u9012063@gmail.com> writes: > Hi Aaron, > > Should we also consider the case where udp checksum is 0xffff? > I saw in netdev_tnl_calc_udp_csum, we set to 0xffff when a packet has > udp checksum = 0. I can add it. This was added because a PMD we caught had a broken firmware that didn't support udp with checksum 0 and would flag it as a bad packet. Thanks for the review! > Thanks > William > > On Wed, Feb 10, 2021 at 12:15 PM Flavio Leitner <fbl@sysclose.org> wrote: >> >> On Wed, Feb 10, 2021 at 02:45:06PM -0500, Aaron Conole wrote: >> > Flavio Leitner <fbl@sysclose.org> writes: >> > >> > > On Wed, Feb 10, 2021 at 11:49:33AM -0500, Aaron Conole wrote: >> > >> Recently, during some conntrack testing a bug was uncovered in a DPDK >> > >> PMD, which doesn't support an IPv4 packet with a zero checksum value. >> > >> In order to show that the connection tracking code in userspace >> > >> supports IPv4 UDP with a zero checksum, add a test case to enforce >> > >> this behavior. >> > >> >> > >> Reported-at: http://mails.dpdk.org/archives/dev/2021-January/198528.html >> > >> Reported-by: Paolo Valerio <pvalerio@redhat.com> >> > >> Signed-off-by: Aaron Conole <aconole@redhat.com> >> > >> --- >> > >> tests/system-traffic.at | 43 +++++++++++++++++++++++++++++++++++++++++ >> > >> 1 file changed, 43 insertions(+) >> > >> >> > >> diff --git a/tests/system-traffic.at b/tests/system-traffic.at >> > >> index fb5b9a36d2..4971ccc966 100644 >> > >> --- a/tests/system-traffic.at >> > >> +++ b/tests/system-traffic.at >> > >> @@ -5927,6 +5927,48 @@ ovs-appctl dpif/dump-flows br0 >> > >> OVS_TRAFFIC_VSWITCHD_STOP >> > >> AT_CLEANUP >> > >> >> > >> + >> > >> +AT_SETUP([conntrack - IPv4 UDP zero checksum]) >> > >> +dnl This tracks sending zero checksum packets for udp over ipv4 >> > >> +CHECK_CONNTRACK() >> > >> +OVS_TRAFFIC_VSWITCHD_START() >> > >> +OVS_CHECK_CT_CLEAR() >> > >> + >> > >> +ADD_NAMESPACES(at_ns0, at_ns1) >> > >> +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "f0:00:00:01:01:01") >> > >> +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "f0:00:00:01:01:02") >> > >> +dnl setup ct flows >> > >> +AT_DATA([flows.txt], [dnl >> > >> +table=0,priority=10 ip,udp,ct_state=-trk action=ct(zone=1,table=1) >> > >> +table=0,priority=0 action=drop >> > >> +table=1,priority=10 >> > >> ct_state=-est+trk+new,ip,ct_zone=1,in_port=1 >> > >> action=ct(commit,table=2) >> > >> +table=1,priority=10 ct_state=+est-new+trk,ct_zone=1,in_port=1 action=resubmit(,2) >> > >> +table=1,priority=0 action=drop >> > >> +table=2,priority=10 ct_state=+trk+new,in_port=1 action=2 >> > >> +table=2,priority=10 ct_state=+trk+est action=2 >> > >> +]) >> > >> + >> > >> +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) >> > >> + >> > >> +# sending udp pkt >> > >> +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f0 00 00 01 >> > >> 01 02 f0 00 00 01 01 01 08 00 45 00 00 28 00 01 00 00 40 11 64 c0 0a >> > >> 01 01 01 0a 01 01 02 04 d2 04 d2 00 14 00 00 aa aa aa aa aa aa aa aa >> > >> aa aa aa aa > /dev/null]) >> > > >> > > That hex string translates to this packet which doesn't use cksum: >> > > 12:57:40.065353 IP (tos 0x0, ttl 64, id 1, offset 0, flags [none], >> > > proto UDP (17), length 40) >> > > 10.1.1.1.search-agent > 10.1.1.2.search-agent: [no cksum] UDP, >> > > length 12 >> > >> > Yes, there should be no UDP checksum. This is the point of this test. >> >> Sorry, I was confirming that the hex string does use a packet >> as this patch says. It's good. >> >> >> > >> + >> > >> +sleep 1 >> > > >> > > Can we use OVS_WAIT_UNTIL() or OVS_WAIT_WHILE() ? >> > >> > Good idea. >> > >> > >> + >> > >> +dnl ensure CT picked up the packet >> > >> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1)], [0], [dnl >> > >> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>) >> > >> +]) >> > >> + >> > >> +AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION_IDLE], >> > >> + [0], [dnl >> > >> + cookie=0x0, duration=<cleared>, table=2, n_packets=1, n_bytes=54, >> > >> idle_age=<cleared>, priority=10,ct_state=+new+trk,in_port=1 >> > >> actions=output:2 >> > >> + cookie=0x0, duration=<cleared>, table=2, n_packets=0, n_bytes=0, >> > >> idle_age=<cleared>, priority=10,ct_state=+est+trk actions=output:2 >> > >> +]) >> > >> + >> > >> +OVS_TRAFFIC_VSWITCHD_STOP >> > >> +AT_CLEANUP >> > >> + >> > >> AT_SETUP([conntrack - Multiple ICMP traverse]) >> > >> dnl This tracks sending ICMP packets via conntrack multiple times for the >> > >> dnl same packet >> > >> @@ -5971,6 +6013,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep >> > >> table=2, | OFPROTO_CLEAR_DURATION_IDLE >> > >> OVS_TRAFFIC_VSWITCHD_STOP >> > >> AT_CLEANUP >> > >> >> > >> + >> > > >> > > There mixed styles. Some use two lines, some use single line >> > > and some use two lines with a dnl ----- or ^L. >> > > >> > > I guess we should stick to single line, but I don't have a >> > > strong preference. >> > >> > Oops, this snuck in. I will remove it, and we can do something there >> > with a different patch if someone thinks it is needed. >> >> Cool, >> thanks >> fbl >> >> >> > >> > > Otherwise the patch works for me. >> > > >> > > Thanks, >> > > fbl >> > > >> > >> AT_BANNER([802.1ad]) >> > >> >> > >> AT_SETUP([802.1ad - vlan_limit]) >> > >> -- >> > >> 2.25.4 >> > >> >> > >> _______________________________________________ >> > >> dev mailing list >> > >> dev@openvswitch.org >> > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > >> > _______________________________________________ >> > dev mailing list >> > dev@openvswitch.org >> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> -- >> fbl >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff --git a/tests/system-traffic.at b/tests/system-traffic.at index fb5b9a36d2..4971ccc966 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -5927,6 +5927,48 @@ ovs-appctl dpif/dump-flows br0 OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([conntrack - IPv4 UDP zero checksum]) +dnl This tracks sending zero checksum packets for udp over ipv4 +CHECK_CONNTRACK() +OVS_TRAFFIC_VSWITCHD_START() +OVS_CHECK_CT_CLEAR() + +ADD_NAMESPACES(at_ns0, at_ns1) +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", "f0:00:00:01:01:01") +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", "f0:00:00:01:01:02") +dnl setup ct flows +AT_DATA([flows.txt], [dnl +table=0,priority=10 ip,udp,ct_state=-trk action=ct(zone=1,table=1) +table=0,priority=0 action=drop +table=1,priority=10 ct_state=-est+trk+new,ip,ct_zone=1,in_port=1 action=ct(commit,table=2) +table=1,priority=10 ct_state=+est-new+trk,ct_zone=1,in_port=1 action=resubmit(,2) +table=1,priority=0 action=drop +table=2,priority=10 ct_state=+trk+new,in_port=1 action=2 +table=2,priority=10 ct_state=+trk+est action=2 +]) + +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) + +# sending udp pkt +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 f0 00 00 01 01 02 f0 00 00 01 01 01 08 00 45 00 00 28 00 01 00 00 40 11 64 c0 0a 01 01 01 0a 01 01 02 04 d2 04 d2 00 14 00 00 aa aa aa aa aa aa aa aa aa aa aa aa > /dev/null]) + +sleep 1 + +dnl ensure CT picked up the packet +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1)], [0], [dnl +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>) +]) + +AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION_IDLE], + [0], [dnl + cookie=0x0, duration=<cleared>, table=2, n_packets=1, n_bytes=54, idle_age=<cleared>, priority=10,ct_state=+new+trk,in_port=1 actions=output:2 + cookie=0x0, duration=<cleared>, table=2, n_packets=0, n_bytes=0, idle_age=<cleared>, priority=10,ct_state=+est+trk actions=output:2 +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([conntrack - Multiple ICMP traverse]) dnl This tracks sending ICMP packets via conntrack multiple times for the dnl same packet @@ -5971,6 +6013,7 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, | OFPROTO_CLEAR_DURATION_IDLE OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP + AT_BANNER([802.1ad]) AT_SETUP([802.1ad - vlan_limit])
Recently, during some conntrack testing a bug was uncovered in a DPDK PMD, which doesn't support an IPv4 packet with a zero checksum value. In order to show that the connection tracking code in userspace supports IPv4 UDP with a zero checksum, add a test case to enforce this behavior. Reported-at: http://mails.dpdk.org/archives/dev/2021-January/198528.html Reported-by: Paolo Valerio <pvalerio@redhat.com> Signed-off-by: Aaron Conole <aconole@redhat.com> --- tests/system-traffic.at | 43 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)