| Message ID | 20230830082822.2168900-7-david.marchand@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v4,1/7] netdev-dpdk: Disable net/tap Tx L4 checksum offloads. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 8/30/23 10:28, David Marchand wrote: > Integrate system-traffic.at tests as part of check-dpdk. > > Some tests that can't work with the userspace datapath are skipped by > overriding some OVS_CHECK_* macros. > > ADD_VETH is implemented with a tap interface in the kernel, directly > polled by OVS with the net/tap DPDK driver. > ADD_VETH_IGNORE_LOGS is provided to filter warning logs that may be > emitted (like for example, lack of DPDK multiprocess feature which OVS > does not care about). > > This driver expects an equal number of rxq and txq. > On the other hand, OVS spawns one PMD thread per numa node, and OVS > reserves one txq for non PMD thread usage. > This means that a net/tap port n_rxq must be set to numa_count + 1. > > Signed-off-by: David Marchand <david.marchand@redhat.com> > --- > Changes since v3: > - reverted --dummy-numa and opted for configuring a number of rxqs > relevant to the number of NUMA sockets, > > Changes since v2: > - added ADD_VETH_IGNORE_LOGS and moved ignored error logs to > OVS_TRAFFIC_VSWITCHD_STOP, > - added --no-pci to DPDK options to avoid failing the tests when > running in a vm with a virtio-net device, > - faked a mono numa/mono core so that OVS requests at max 2 txqs on > the net/tap port, > > --- > tests/system-dpdk-macros.at | 95 ++++++++++++++++++++++++++++++++++ > tests/system-dpdk-testsuite.at | 2 + > tests/system-dpdk.at | 7 +-- > 3 files changed, 99 insertions(+), 5 deletions(-) > > diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at > index f29b3c283f..ce8e98f475 100644 > --- a/tests/system-dpdk-macros.at > +++ b/tests/system-dpdk-macros.at > @@ -124,3 +124,98 @@ m4_define([OVS_DPDK_STOP_TESTPMD], > [AT_CHECK([kill `cat testpmd.pid`]) > OVS_WAIT([kill -0 `cat testpmd.pid`], [kill -9 `cat testpmd.pid`]) > ]) > + > + > +# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [dbinit-aux-args]) > +# > +# Creates a database and starts ovsdb-server, starts ovs-vswitchd > +# connected to that database, calls ovs-vsctl to create a bridge named > +# br0 with predictable settings, passing 'vsctl-args' as additional > +# commands to ovs-vsctl. If 'vsctl-args' causes ovs-vsctl to provide > +# output (e.g. because it includes "create" commands) then 'vsctl-output' > +# specifies the expected output after filtering through uuidfilt. > +# 'dbinit-aux-args' are passed as additional commands to 'ovs-vsctl init' > +# before starting ovs-vswitchd. > +m4_define([OVS_TRAFFIC_VSWITCHD_START], > + [ > + OVS_DPDK_PRE_CHECK() > + OVS_WAIT_WHILE([ip link show ovs-netdev]) > + dnl For functional tests, no need for DPDK PCI probing. > + OVS_DPDK_START([--no-pci], [--disable-system], [$3]) > + dnl Add bridges, ports, etc. > + OVS_WAIT_WHILE([ip link show br0]) > + AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| uuidfilt])], [0], [$2]) > +]) > + > + > +# OVS_TRAFFIC_VSWITCHD_STOP([ALLOWLIST], [extra_cmds]) > +# > +# Gracefully stops ovs-vswitchd and ovsdb-server, checking their log files > +# for messages with severity WARN or higher and signaling an error if any > +# is present. The optional ALLOWLIST may contain shell-quoted "sed" > +# commands to delete any warnings that are actually expected, e.g.: > +# > +# OVS_TRAFFIC_VSWITCHD_STOP(["/expected error/d"]) > +# > +# 'extra_cmds' are shell commands to be executed after OVS_VSWITCHD_STOP() is > +# invoked. They can be used to perform additional cleanups such as name space > +# removal. > +m4_define([OVS_TRAFFIC_VSWITCHD_STOP], > + [OVS_DPDK_STOP_VSWITCHD([$1";"ADD_VETH_IGNORE_LOGS";dnl > +/does not exist. The Open vSwitch kernel module is probably not loaded./d"]) > + AT_CHECK([:; $2]) > +]) > + > + > +m4_define([OVS_CHECK_8021AD], > + [AT_SKIP_IF([:])]) > + > + > +m4_define([OVS_CHECK_TC_QDISC], > + [AT_SKIP_IF([:])]) > + > + > +m4_define([OVS_CHECK_TCPDUMP], > + [AT_SKIP_IF([:])]) > + > + > +# Fake a veth by creating a tap on kernel side and plug it in OVS using the > +# net/tap DPDK driver. Why do we need to fake a veth? It looks strange to me and also error prone as taps do not behave like veths in some cases. Can we just have a veth and open it with af_packet ports like all other testsuites do? Best regards, Ilya Maximets.
On Wed, Aug 30, 2023 at 1:45 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > +# Fake a veth by creating a tap on kernel side and plug it in OVS using the > > +# net/tap DPDK driver. > > Why do we need to fake a veth? It looks strange to me and also error prone > as taps do not behave like veths in some cases. Can we just have a veth and > open it with af_packet ports like all other testsuites do? When you suggest af_packet port, I suppose you mean netdev-afxdp ports. My initial reason for plugging net/tap was to have a way to test DPBUF_DPDK dp-packets through OVS. Testing with netdev-dpdk only ports should also be closer to deployments of ovs-dpdk, rather than a mix of kernel netdev and dpdk ports with the userspace datapath.
On 8/30/23 18:02, David Marchand wrote: > On Wed, Aug 30, 2023 at 1:45 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>> +# Fake a veth by creating a tap on kernel side and plug it in OVS using the >>> +# net/tap DPDK driver. >> >> Why do we need to fake a veth? It looks strange to me and also error prone >> as taps do not behave like veths in some cases. Can we just have a veth and >> open it with af_packet ports like all other testsuites do? > > When you suggest af_packet port, I suppose you mean netdev-afxdp ports. No, I meant DPDK's drivers/net/af_packet/rte_eth_af_packet.c. > > My initial reason for plugging net/tap was to have a way to test > DPBUF_DPDK dp-packets through OVS. > Testing with netdev-dpdk only ports should also be closer to > deployments of ovs-dpdk, rather than a mix of kernel netdev and dpdk > ports with the userspace datapath. > >
On Wed, Aug 30, 2023 at 6:09 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 8/30/23 18:02, David Marchand wrote: > > On Wed, Aug 30, 2023 at 1:45 PM Ilya Maximets <i.maximets@ovn.org> wrote: > >>> +# Fake a veth by creating a tap on kernel side and plug it in OVS using the > >>> +# net/tap DPDK driver. > >> > >> Why do we need to fake a veth? It looks strange to me and also error prone > >> as taps do not behave like veths in some cases. Can we just have a veth and > >> open it with af_packet ports like all other testsuites do? > > > > When you suggest af_packet port, I suppose you mean netdev-afxdp ports. > > No, I meant DPDK's drivers/net/af_packet/rte_eth_af_packet.c. That's something I can investigate, though it's been a long time (in a previous life) since I played with it. In comparison net/tap seems widely used to me. /me opens https://bugs.dpdk.org/enter_bug.cgi in preparation
On 8/30/23 18:17, David Marchand wrote: > On Wed, Aug 30, 2023 at 6:09 PM Ilya Maximets <i.maximets@ovn.org> wrote: >> >> On 8/30/23 18:02, David Marchand wrote: >>> On Wed, Aug 30, 2023 at 1:45 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>>>> +# Fake a veth by creating a tap on kernel side and plug it in OVS using the >>>>> +# net/tap DPDK driver. >>>> >>>> Why do we need to fake a veth? It looks strange to me and also error prone >>>> as taps do not behave like veths in some cases. Can we just have a veth and >>>> open it with af_packet ports like all other testsuites do? >>> >>> When you suggest af_packet port, I suppose you mean netdev-afxdp ports. >> >> No, I meant DPDK's drivers/net/af_packet/rte_eth_af_packet.c. > > That's something I can investigate, though it's been a long time (in a > previous life) since I played with it. > In comparison net/tap seems widely used to me. That didn't save it from having completely broken checksum offloading. I suppose both the tap and af_packet can be tested in DPDK's CI. The net/af_xdp DPDK driver might be a better maintained alternative as well, I guess. Best regards, Ilya Maximets.
On 8/30/23 18:25, Ilya Maximets wrote: > On 8/30/23 18:17, David Marchand wrote: >> On Wed, Aug 30, 2023 at 6:09 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>> >>> On 8/30/23 18:02, David Marchand wrote: >>>> On Wed, Aug 30, 2023 at 1:45 PM Ilya Maximets <i.maximets@ovn.org> wrote: >>>>>> +# Fake a veth by creating a tap on kernel side and plug it in OVS using the >>>>>> +# net/tap DPDK driver. >>>>> >>>>> Why do we need to fake a veth? It looks strange to me and also error prone >>>>> as taps do not behave like veths in some cases. Can we just have a veth and >>>>> open it with af_packet ports like all other testsuites do? >>>> >>>> When you suggest af_packet port, I suppose you mean netdev-afxdp ports. >>> >>> No, I meant DPDK's drivers/net/af_packet/rte_eth_af_packet.c. >> >> That's something I can investigate, though it's been a long time (in a >> previous life) since I played with it. >> In comparison net/tap seems widely used to me. > > That didn't save it from having completely broken checksum offloading. > I suppose both the tap and af_packet can be tested in DPDK's CI. (I meant CI of the DPDK project) > > The net/af_xdp DPDK driver might be a better maintained alternative as well, > I guess. > > Best regards, Ilya Maximets.
On Wed, Aug 30, 2023 at 6:17 PM David Marchand <david.marchand@redhat.com> wrote: > > On Wed, Aug 30, 2023 at 6:09 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > > > On 8/30/23 18:02, David Marchand wrote: > > > On Wed, Aug 30, 2023 at 1:45 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > >>> +# Fake a veth by creating a tap on kernel side and plug it in OVS using the > > >>> +# net/tap DPDK driver. > > >> > > >> Why do we need to fake a veth? It looks strange to me and also error prone > > >> as taps do not behave like veths in some cases. Can we just have a veth and > > >> open it with af_packet ports like all other testsuites do? > > > > > > When you suggest af_packet port, I suppose you mean netdev-afxdp ports. > > > > No, I meant DPDK's drivers/net/af_packet/rte_eth_af_packet.c. > > That's something I can investigate, though it's been a long time (in a > previous life) since I played with it. > In comparison net/tap seems widely used to me. > > /me opens https://bugs.dpdk.org/enter_bug.cgi in preparation - As for net/af_packet, it uses PACKET_FANOUT_FLAG_DEFRAG (with no way to disable this feature). https://git.dpdk.org/dpdk-stable/tree/drivers/net/af_packet/rte_eth_af_packet.c?h=22.11#n763 https://git.dpdk.org/dpdk-stable/tree/drivers/net/af_packet/rte_eth_af_packet.c?h=22.11#n879 This does not play well with OVS tests like when pinging with oversized packets. The reassembled packets are received on the DPDK port and then get dropped when trying to send them because the packets exceed the port mtu. I don't think this is something we can live with. - About net/af_xdp, after some ethtool tweaking on the "kernel-facing" netdev (same as system-afxdp tests) to disable tx offloads, the tests pass on my fedora37 except some random failure on "conntrack - ICMP related" tests with following logs: +2023-08-30T19:41:54.364Z|00001|dpif(revalidator17)|WARN|netdev@ovs-netdev: failed to flow_get (No such file or directory) ufid:3e239a2e-e9ff-42e9-a6b1-bfeeb0b511a9 <empty>, packets:0, bytes:0, used:never +2023-08-30T19:41:54.364Z|00002|ofproto_dpif_upcall(revalidator17)|WARN|Failed to acquire udpif_key corresponding to unexpected flow (No such file or directory): ufid:3e239a2e-e9ff-42e9-a6b1-bfeeb0b511a9 This does not seem related as it goes away on a retest, but I hit it in two different runs, is this something known? As a note, there is no issue with tx checksum offloading: there is no support at all in the pmd itself.
diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at index f29b3c283f..ce8e98f475 100644 --- a/tests/system-dpdk-macros.at +++ b/tests/system-dpdk-macros.at @@ -124,3 +124,98 @@ m4_define([OVS_DPDK_STOP_TESTPMD], [AT_CHECK([kill `cat testpmd.pid`]) OVS_WAIT([kill -0 `cat testpmd.pid`], [kill -9 `cat testpmd.pid`]) ]) + + +# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [dbinit-aux-args]) +# +# Creates a database and starts ovsdb-server, starts ovs-vswitchd +# connected to that database, calls ovs-vsctl to create a bridge named +# br0 with predictable settings, passing 'vsctl-args' as additional +# commands to ovs-vsctl. If 'vsctl-args' causes ovs-vsctl to provide +# output (e.g. because it includes "create" commands) then 'vsctl-output' +# specifies the expected output after filtering through uuidfilt. +# 'dbinit-aux-args' are passed as additional commands to 'ovs-vsctl init' +# before starting ovs-vswitchd. +m4_define([OVS_TRAFFIC_VSWITCHD_START], + [ + OVS_DPDK_PRE_CHECK() + OVS_WAIT_WHILE([ip link show ovs-netdev]) + dnl For functional tests, no need for DPDK PCI probing. + OVS_DPDK_START([--no-pci], [--disable-system], [$3]) + dnl Add bridges, ports, etc. + OVS_WAIT_WHILE([ip link show br0]) + AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| uuidfilt])], [0], [$2]) +]) + + +# OVS_TRAFFIC_VSWITCHD_STOP([ALLOWLIST], [extra_cmds]) +# +# Gracefully stops ovs-vswitchd and ovsdb-server, checking their log files +# for messages with severity WARN or higher and signaling an error if any +# is present. The optional ALLOWLIST may contain shell-quoted "sed" +# commands to delete any warnings that are actually expected, e.g.: +# +# OVS_TRAFFIC_VSWITCHD_STOP(["/expected error/d"]) +# +# 'extra_cmds' are shell commands to be executed after OVS_VSWITCHD_STOP() is +# invoked. They can be used to perform additional cleanups such as name space +# removal. +m4_define([OVS_TRAFFIC_VSWITCHD_STOP], + [OVS_DPDK_STOP_VSWITCHD([$1";"ADD_VETH_IGNORE_LOGS";dnl +/does not exist. The Open vSwitch kernel module is probably not loaded./d"]) + AT_CHECK([:; $2]) +]) + + +m4_define([OVS_CHECK_8021AD], + [AT_SKIP_IF([:])]) + + +m4_define([OVS_CHECK_TC_QDISC], + [AT_SKIP_IF([:])]) + + +m4_define([OVS_CHECK_TCPDUMP], + [AT_SKIP_IF([:])]) + + +# Fake a veth by creating a tap on kernel side and plug it in OVS using the +# net/tap DPDK driver. +m4_define([ADD_VETH], + dnl By default, OVS starts as many PMD threads as NUMA sockets. + dnl One txq is required for each PMD thread. One additional txq is required + dnl for OVS main thread. On the other hand, a tap device expects + dnl n_rxq == n_txq. + [ n_rxq=$(lscpu | awk '/^NUMA node\(s\):/ { print $NF + 1; }') + AT_CHECK([ovs-vsctl add-port $3 ovs-$1 -- \ + set interface ovs-$1 external-ids:iface-id="$1" -- \ + set interface ovs-$1 type=dpdk -- \ + set interface ovs-$1 options:n_rxq=${n_rxq} -- \ + set interface ovs-$1 options:dpdk-devargs=net_tap$1,iface=$1]) + OVS_WAIT_UNTIL([ip link show dev $1 | grep -qw LOWER_UP]) + AT_CHECK([ip link set $1 netns $2]) + NS_CHECK_EXEC([$2], [ip addr add $4 dev $1 $7]) + NS_CHECK_EXEC([$2], [ip link set dev $1 up]) + if test -n "$5"; then + NS_CHECK_EXEC([$2], [ip link set dev $1 address $5]) + fi + if test -n "$6"; then + NS_CHECK_EXEC([$2], [ip route add default via $6]) + fi + ] +) + + +# Using a net/tap port with ADD_VETH can trigger some warning logs that can be +# filtered with the following macro: +m4_define([ADD_VETH_IGNORE_LOGS], + ["dnl +/tap_nl_dump_ext_ack(): Specified qdisc kind is unknown/d +/qdisc_create_multiq(): Could not add multiq qdisc (2): No such file or directory/d +/eth_dev_tap_create(): .*: failed to create multiq qdisc./d +/eth_dev_tap_create(): Disabling rte flow support: No such file or directory/d +/tap_mp_req_on_rxtx(): Failed to send start req to secondary/d"]) + + +m4_define([CONFIGURE_VETH_OFFLOADS], + [AT_CHECK([ethtool -K $1 tx off], [0], [ignore], [ignore])]) diff --git a/tests/system-dpdk-testsuite.at b/tests/system-dpdk-testsuite.at index 382f09e9ff..f61fbf9212 100644 --- a/tests/system-dpdk-testsuite.at +++ b/tests/system-dpdk-testsuite.at @@ -20,6 +20,8 @@ m4_include([tests/ovs-macros.at]) m4_include([tests/ovsdb-macros.at]) m4_include([tests/ofproto-macros.at]) m4_include([tests/system-common-macros.at]) +m4_include([tests/system-userspace-macros.at]) m4_include([tests/system-dpdk-macros.at]) m4_include([tests/system-dpdk.at]) +m4_include([tests/system-traffic.at]) diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at index e8a04d1d86..3c812fa6e3 100644 --- a/tests/system-dpdk.at +++ b/tests/system-dpdk.at @@ -1,6 +1,3 @@ -m4_define([CONFIGURE_VETH_OFFLOADS], - [AT_CHECK([ethtool -K $1 tx off], [0], [ignore], [ignore])]) - AT_BANNER([OVS-DPDK unit tests]) dnl CHECK_MEMPOOL_PARAM([mtu], [numa], [+line]) @@ -140,7 +137,7 @@ OVS_WAIT_UNTIL([grep "vHost Device '$OVS_RUNDIR/dpdkvhostuser0' has been removed dnl Clean up AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuser0], [], [stdout], [stderr]) -OVS_DPDK_STOP_VSWITCHD(["dnl +OVS_DPDK_STOP_VSWITCHD([ADD_VETH_IGNORE_LOGS";dnl /VHOST_CONFIG: (.*dpdkvhostuser0) recvmsg failed/d /VHOST_CONFIG: (.*dpdkvhostuser0) failed to connect: No such file or directory/d /dpdkvhostuser ports are considered deprecated; please migrate to dpdkvhostuserclient ports./d @@ -226,7 +223,7 @@ OVS_DPDK_STOP_TESTPMD() dnl Clean up AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr]) -OVS_DPDK_STOP_VSWITCHD(["dnl +OVS_DPDK_STOP_VSWITCHD([ADD_VETH_IGNORE_LOGS";dnl /VHOST_CONFIG: (.*dpdkvhostclient0) recvmsg failed/d /VHOST_CONFIG: (.*dpdkvhostclient0) failed to connect: No such file or directory/d /dpdkvhostuser ports are considered deprecated; please migrate to dpdkvhostuserclient ports./d
Integrate system-traffic.at tests as part of check-dpdk. Some tests that can't work with the userspace datapath are skipped by overriding some OVS_CHECK_* macros. ADD_VETH is implemented with a tap interface in the kernel, directly polled by OVS with the net/tap DPDK driver. ADD_VETH_IGNORE_LOGS is provided to filter warning logs that may be emitted (like for example, lack of DPDK multiprocess feature which OVS does not care about). This driver expects an equal number of rxq and txq. On the other hand, OVS spawns one PMD thread per numa node, and OVS reserves one txq for non PMD thread usage. This means that a net/tap port n_rxq must be set to numa_count + 1. Signed-off-by: David Marchand <david.marchand@redhat.com> --- Changes since v3: - reverted --dummy-numa and opted for configuring a number of rxqs relevant to the number of NUMA sockets, Changes since v2: - added ADD_VETH_IGNORE_LOGS and moved ignored error logs to OVS_TRAFFIC_VSWITCHD_STOP, - added --no-pci to DPDK options to avoid failing the tests when running in a vm with a virtio-net device, - faked a mono numa/mono core so that OVS requests at max 2 txqs on the net/tap port, --- tests/system-dpdk-macros.at | 95 ++++++++++++++++++++++++++++++++++ tests/system-dpdk-testsuite.at | 2 + tests/system-dpdk.at | 7 +-- 3 files changed, 99 insertions(+), 5 deletions(-)