On Tue, Jul 9, 2024 at 5:57 PM Ilya Maximets <i.maxim...@ovn.org> wrote:
>
> On 7/5/24 22:44, Mike Pattrick wrote:
> > When sending packets that are flagged as requiring segmentation to an
> > interface that does not support this feature, send the packet to the TSO
> > software fallback instead of dropping it.
> >
> > Signed-off-by: Mike Pattrick <m...@redhat.com>
> > ---
> > v2:
> > - Fixed udp tunnel length
> > - Added test that UDP headers are correct
> > - Split inner and outer ip_id into different counters
> > - Set tunnel flags in reset_tcp_seg
> >
> > v3:
> > - Changed logic of netdev_send() to account for NICs that support
> > tunnel offload but not checksum offload
> > - Adjusted udp tunnel header length during software tso
> >
> > v4:
> > - Moved a bugfix into its own patch
> > - Fixed an indentation issue
> > - Changed the scope of ip_hdr
> > Signed-off-by: Mike Pattrick <m...@redhat.com>
> > ---
> > lib/dp-packet-gso.c | 90 ++++++++++++++++++++++++++++++++---------
> > lib/dp-packet.h | 34 ++++++++++++++++
> > lib/netdev.c | 44 ++++++++++----------
> > tests/system-traffic.at | 87 +++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 214 insertions(+), 41 deletions(-)
>
> Hi, Mike. Not a full review, but see a few comments below.
>
> > diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> > index 3f1a15445..dd30b6bf5 100644
> > --- a/tests/system-traffic.at
> > +++ b/tests/system-traffic.at
> > @@ -351,6 +351,93 @@ OVS_WAIT_UNTIL([diff -q payload.bin udp_data])
> > OVS_TRAFFIC_VSWITCHD_STOP
> > AT_CLEANUP
> >
> > +AT_SETUP([datapath - tcp over vxlan tunnel with software fallback])
> > +AT_SKIP_IF([test $HAVE_NC = no])
> > +AT_SKIP_IF([test $HAVE_TCPDUMP = no])
> > +OVS_CHECK_VXLAN()
> > +
> > +dnl This test is only valid with tso. If the kernel segments the packets,
> > the
> > +dnl packet lengths in the final test will be different.
> > +m4_ifndef([CHECK_SYSTEM_TSO], [AT_SKIP_IF(:)])
> > +
> > +OVS_TRAFFIC_VSWITCHD_START()
> > +ADD_BR([br-underlay])
> > +
> > +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
> > +AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
> > +
> > +ADD_NAMESPACES(at_ns0)
> > +
> > +dnl Set up underlay link from host into the namespace using veth pair.
> > +ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
> > +AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
> > +AT_CHECK([ip link set dev br-underlay up])
> > +
> > +dnl Test the case where only one side has all checksum and tso offload
> > disabled.
> > +AT_CHECK([ethtool -K ovs-p0 tso off], [0], [ignore], [ignore])
> > +AT_CHECK([ethtool -K ovs-p0 sg off], [0], [ignore], [ignore])
> > +
> > +dnl Reinitialize.
> > +AT_CHECK([ovs-vsctl del-port ovs-p0])
> > +AT_CHECK([ovs-vsctl add-port br-underlay ovs-p0])
> > +
> > +dnl Set up tunnel endpoints on OVS outside the namespace and with a native
> > +dnl linux device inside the namespace.
> > +ADD_OVS_TUNNEL([vxlan], [br0], [at_vxlan0], [172.31.1.1], [10.1.1.100/24])
> > +ADD_NATIVE_TUNNEL([vxlan], [at_vxlan1], [at_ns0], [172.31.1.100],
> > [10.1.1.1/24],
> > + [id 0 dstport 4789])
> > +
> > +dnl First, check the underlay.
> > +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -W 2 172.31.1.100 |
> > FORMAT_PING], [0], [dnl
> > +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > +])
> > +
> > +dnl Check that the tunnel is up.
> > +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -W 2 10.1.1.100 |
> > FORMAT_PING], [0], [dnl
> > +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > +])
> > +
> > +dnl Start tcpdump to capture the encapsulated packets.
> > +OVS_DAEMONIZE([tcpdump -i ovs-p0 -w p0.pcap], [tcpdump.pid])
> > +sleep 1
>
> Don't sleep - wait.
The sleep here is to make sure that tcpdump has actually started
listening for packets. I found sometimes in github ci the pcap would
be missing the start of the transfer causing a failure. It's also used
in the conntrack tests.
> > +
> > +dnl Initialize the listener before it is needed.
> > +NETNS_DAEMONIZE([at_ns0], [nc -l 10.1.1.1 1234 > data2], [nc.pid])
> > +
> > +dnl Verify that ncat is ready.
> > +OVS_WAIT_UNTIL([NS_EXEC([at_ns0], [netstat -ln | grep :1234])])
> > +
> > +dnl Large TCP transfer aimed towards ovs-p0, which has TSO disabled.
> > +AT_CHECK([dd if=/dev/urandom of=payload.bin bs=60000 count=1 2> /dev/null])
> > +AT_CHECK([nc $NC_EOF_OPT 10.1.1.1 1234 < payload.bin])
> > +
> > +dnl Wait until transfer completes before checking.
> > +OVS_WAIT_WHILE([kill -0 $(cat nc.pid)])
> > +AT_CHECK([diff -q payload.bin data2], [0])
> > +
> > +dnl Stop OVS and tcpdump and verify the results.
> > +OVS_TRAFFIC_VSWITCHD_STOP
> > +
> > +AT_CHECK([kill -15 $(cat tcpdump.pid)])
> > +
> > +OVS_WAIT_WHILE([kill -0 $(cat tcpdump.pid)])
> > +
> > +ovs-pcap p0.pcap
> > +
> > +dnl The exact number of packets sent will vary, but we check that the
> > largest segments have the correct
> > +dnl lengths and certain other fields.
> > +AT_CHECK([test $(ovs-pcap p0.pcap | grep -Ec dnl
> > +"^.{24}0800"dnl Ethernet
> > +"450005aa....4000..11....ac1f0164ac1f0101"dnl IP(len=1450, DF, UDP,
> > 172.31.1.100->172.31.1.1)
> > +"....12b505960000"dnl UDP(len=1430, dport=4789)
> > +"0800000000000000"dnl VXLAN(gpid=0, vni=0)
> > +".{24}0800"dnl Ethernet
> > +"45000578....4000..06....0a0101640a010101"dnl IP(len=1400, DF, TCP,
> > 10.1.1.100->10.1.1.1)
> > +"....04d2............................0000"dnl TCP(dport=1234
> > +) -ge 20])
> > +
> > +AT_CLEANUP
> > +
> > AT_SETUP([datapath - ping vlan over vxlan tunnel])
> > OVS_CHECK_TUNNEL_TSO()
>
> I forgot why exactly we have this test skipped in TSO case.
> Can we stop skipping with the software fallback available?
Linux kernel issue when combining udp tunnel, vlan, veth, and tso, the
tso start location is incorrect, off by the vlan header.
> Also, it's good to have a system test with a real traffic, but this
> functionality can be tested without involving the real traffic. We
> should have a unit test for it, similar to 'userspace offload - tso'
> or 'userspace offload - ip csum offload'.
Can do!
-M
>
> Best regards, Ilya Maximets.
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
