[ovs-dev,v2,5/7] system-dpdk: Refactor OVS daemons helpers.

Align system-dpdk existing helpers to other common OVS helpers so they
can accept some optional arguments.

Introduce a OVS_DPDK_STOP_VSWITCHD wrapper around OVS_VSWITCHD_STOP to
catch dpdk related logs in a centralised fashion.

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 tests/system-dpdk-macros.at |  21 ++++-
 tests/system-dpdk.at        | 159 +++++++++++++++---------------------
 2 files changed, 83 insertions(+), 97 deletions(-)

On 23 Aug 2023, at 17:34, David Marchand wrote:

> Align system-dpdk existing helpers to other common OVS helpers so they
> can accept some optional arguments.
>
> Introduce a OVS_DPDK_STOP_VSWITCHD wrapper around OVS_VSWITCHD_STOP to
> catch dpdk related logs in a centralised fashion.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>

For this one I do have a comments :)

//Eelco

> ---
>  tests/system-dpdk-macros.at |  21 ++++-
>  tests/system-dpdk.at        | 159 +++++++++++++++---------------------
>  2 files changed, 83 insertions(+), 97 deletions(-)
>
> diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at
> index 108280f70d..a72133fae1 100644
> --- a/tests/system-dpdk-macros.at
> +++ b/tests/system-dpdk-macros.at
> @@ -36,12 +36,13 @@ m4_define([OVS_DPDK_PRE_PHY_SKIP],
>  #
>  m4_define([OVS_DPDK_START],
>    [dnl start ovs dpdk
> -   OVS_DPDK_START_OVSDB()
> +   OVS_DPDK_START_OVSDB($3)
>     dnl Enable DPDK functionality
>     AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-init=true])
> -   OVS_DPDK_START_VSWITCHD($1)
> +   OVS_DPDK_START_VSWITCHD([$1], [$2])
>  ])
>
> +
>  # OVS_DPDK_START_OVSDB()
>  #
>  # Create an empty database and start ovsdb-server.
> @@ -60,9 +61,10 @@ m4_define([OVS_DPDK_START_OVSDB],
>     AT_CAPTURE_FILE([ovsdb-server.log])
>
>     dnl Initialize database.
> -   AT_CHECK([ovs-vsctl --no-wait init])
> +   AT_CHECK([ovs-vsctl --no-wait init $1])
>  ])
>
> +
>  # OVS_DPDK_START_VSWITCHD()
>  #
>  # Add special configuration for dpdk-init. Start ovs-vswitchd.
> @@ -72,12 +74,23 @@ m4_define([OVS_DPDK_START_VSWITCHD],
>     AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-extra="--log-level=pmd.*:error $1"])
>
>     dnl Start ovs-vswitchd.
> -   AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif -vunixctl], [0], [stdout], [stderr])
> +   AT_CHECK([ovs-vswitchd $2 --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif -vunixctl], [0], [stdout], [stderr])
>     AT_CAPTURE_FILE([ovs-vswitchd.log])
>     on_exit "kill_ovs_vswitchd `cat ovs-vswitchd.pid`"
>  ])
>
>
> +m4_define([OVS_DPDK_STOP_VSWITCHD],
> +  [OVS_VSWITCHD_STOP([dnl
> +$1";/does not exist. The Open vSwitch kernel module is probably not loaded./d
> +/does not support MTU configuration,/d
> +/EAL: No \(available\|free\) .*hugepages reported/d
> +/Failed to enable flow control/d
> +/Rx checksum offload is not supported on/d
> +/TELEMETRY: No legacy callbacks, legacy socket not created/d"])

On my VMs (with all patches applied) all the existing dpdk tests and new ones are failing due to the following error messages:

+2023-08-25T10:09:42.252Z|00016|dpdk|ERR|eth_virtio_pci_init(): Failed to init PCI device
+2023-08-25T10:09:42.252Z|00017|dpdk|ERR|EAL: Requested device 0000:00:05.0 cannot be used
+2023-08-25T10:09:42Z|00016|dpdk|ERR|eth_virtio_pci_init(): Failed to init PCI device
+2023-08-25T10:09:42Z|00017|dpdk|ERR|EAL: Requested device 0000:00:05.0 cannot be used

Maybe we could exclude them here (this is what I did) or add some --no-pci option (and make sure the ones that do need them have it).

@@ -93,6 +93,8 @@ $1";/does not exist. The Open vSwitch kernel module is probably not loaded./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
+/eth_virtio_pci_init(): Failed to init PCI device/d
+/EAL: Requested device .* cannot be used/d
 /does not exist. The Open vSwitch kernel module is probably not loaded./d"])
   ])

> +
> +
>  # OVS_DPDK_CHECK_TESTPMD()
>  #
>  # Check dpdk-testpmd availability.
> diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
> index 270587e2c0..e8a04d1d86 100644
> --- a/tests/system-dpdk.at
> +++ b/tests/system-dpdk.at
> @@ -3,15 +3,6 @@ m4_define([CONFIGURE_VETH_OFFLOADS],
>
>  AT_BANNER([OVS-DPDK unit tests])
>
> -m4_define([SYSTEM_DPDK_ALLOWED_LOGS],[
> -\@does not exist. The Open vSwitch kernel module is probably not loaded.@d
> -\@does not support MTU configuration,@d
> -\@EAL: No \(available\|free\) .*hugepages reported@d
> -\@Failed to enable flow control@d
> -\@Rx checksum offload is not supported on@d
> -\@TELEMETRY: No legacy callbacks, legacy socket not created@d
> -])
> -
>  dnl CHECK_MEMPOOL_PARAM([mtu], [numa], [+line])
>  dnl
>  dnl Waits for logs to indicate that the user has configured a mempool
> @@ -36,7 +27,7 @@ OVS_DPDK_START([--no-pci])
>  AT_CHECK([grep "DPDK Enabled - initializing..." ovs-vswitchd.log], [], [stdout])
>  AT_CHECK([grep "EAL" ovs-vswitchd.log], [], [stdout])
>  AT_CHECK([grep "DPDK Enabled - initialized" ovs-vswitchd.log], [], [stdout])
> -OVS_VSWITCHD_STOP("[SYSTEM_DPDK_ALLOWED_LOGS]")
> +OVS_DPDK_STOP_VSWITCHD
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -58,7 +49,7 @@ sleep 2
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 phy0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("[SYSTEM_DPDK_ALLOWED_LOGS]")
> +OVS_DPDK_STOP_VSWITCHD
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -84,9 +75,8 @@ AT_CHECK([grep "VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) reconnecting..." ov
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/VHOST_CONFIG: (.*dpdkvhostclient0) failed to connect: No such file or directory/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -150,12 +140,11 @@ 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_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostuser0) recvmsg failed@d
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostuser0) failed to connect: No such file or directory@d
> -\@dpdkvhostuser ports are considered deprecated;  please migrate to dpdkvhostuserclient ports.@d
> -\@failed to enumerate system datapaths: No such file or directory@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["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
> +/failed to enumerate system datapaths: No such file or directory/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -237,12 +226,11 @@ OVS_DPDK_STOP_TESTPMD()
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) recvmsg failed@d
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
> -\@dpdkvhostuser ports are considered deprecated;  please migrate to dpdkvhostuserclient ports.@d
> -\@failed to enumerate system datapaths: No such file or directory@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["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
> +/failed to enumerate system datapaths: No such file or directory/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -278,7 +266,7 @@ AT_CHECK([grep -E 'ingress_policing_rate: 0' stdout], [], [stdout])
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 phy0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("[SYSTEM_DPDK_ALLOWED_LOGS]")
> +OVS_DPDK_STOP_VSWITCHD
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -319,9 +307,8 @@ AT_CHECK([grep "VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) reconnecting..." ov
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/VHOST_CONFIG: (.*dpdkvhostclient0) failed to connect: No such file or directory/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -360,9 +347,8 @@ AT_CHECK([grep "VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) reconnecting..." ov
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/VHOST_CONFIG: (.*dpdkvhostclient0) failed to connect: No such file or directory/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -400,9 +386,8 @@ AT_CHECK([grep "VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) reconnecting..." ov
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/VHOST_CONFIG: (.*dpdkvhostclient0) failed to connect: No such file or directory/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -434,7 +419,7 @@ AT_CHECK([grep -E 'QoS not configured on phy0' stdout], [], [stdout])
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 phy0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("[SYSTEM_DPDK_ALLOWED_LOGS]")
> +OVS_DPDK_STOP_VSWITCHD
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -473,9 +458,8 @@ AT_CHECK([grep -E 'QoS not configured on dpdkvhostuserclient0' stdout], [], [std
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/VHOST_CONFIG: (.*dpdkvhostclient0) failed to connect: No such file or directory/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -506,11 +490,10 @@ AT_CHECK([grep -E 'QoS not configured on dpdkvhostuserclient0' stdout], [], [std
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
> -\@Could not create rte meter for egress policer@d
> -\@Failed to set QoS type egress-policer on port dpdkvhostuserclient0: Invalid argument@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/VHOST_CONFIG: (.*dpdkvhostclient0) failed to connect: No such file or directory/d
> +/Could not create rte meter for egress policer/d
> +/Failed to set QoS type egress-policer on port dpdkvhostuserclient0: Invalid argument/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -541,11 +524,10 @@ AT_CHECK([grep -E 'QoS not configured on dpdkvhostuserclient0' stdout], [], [std
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
> -\@Could not create rte meter for egress policer@d
> -\@Failed to set QoS type egress-policer on port dpdkvhostuserclient0: Invalid argument@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/VHOST_CONFIG: (.*dpdkvhostclient0) failed to connect: No such file or directory/d
> +/Could not create rte meter for egress policer/d
> +/Failed to set QoS type egress-policer on port dpdkvhostuserclient0: Invalid argument/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -587,7 +569,7 @@ AT_CHECK([grep -E 'mtu=9000' stdout], [], [stdout])
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 phy0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("[SYSTEM_DPDK_ALLOWED_LOGS]")
> +OVS_DPDK_STOP_VSWITCHD
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -629,7 +611,7 @@ AT_CHECK([grep -E 'mtu=2000' stdout], [], [stdout])
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 phy0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("[SYSTEM_DPDK_ALLOWED_LOGS]")
> +OVS_DPDK_STOP_VSWITCHD
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -674,9 +656,8 @@ OVS_DPDK_STOP_TESTPMD()
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/VHOST_CONFIG: (.*dpdkvhostclient0) failed to connect: No such file or directory/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -722,9 +703,8 @@ OVS_DPDK_STOP_TESTPMD()
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/VHOST_CONFIG: (.*dpdkvhostclient0) failed to connect: No such file or directory/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -762,10 +742,9 @@ AT_CHECK([grep "phy0: unsupported MTU 9711" ovs-vswitchd.log], [], [stdout])
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 phy0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@phy0: unsupported MTU 9711@d
> -\@failed to set MTU for network device phy0: Invalid argument@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/phy0: unsupported MTU 9711/d
> +/failed to set MTU for network device phy0: Invalid argument/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -803,10 +782,9 @@ AT_CHECK([grep "phy0: unsupported MTU 67" ovs-vswitchd.log], [], [stdout])
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 phy0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@phy0: unsupported MTU 67@d
> -\@failed to set MTU for network device phy0: Invalid argument@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/phy0: unsupported MTU 67/d
> +/failed to set MTU for network device phy0: Invalid argument/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -845,11 +823,10 @@ OVS_DPDK_STOP_TESTPMD()
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
> -\@dpdkvhostuserclient0: unsupported MTU 9711@d
> -\@failed to set MTU for network device dpdkvhostuserclient0: Invalid argument@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/VHOST_CONFIG: (.*dpdkvhostclient0) failed to connect: No such file or directory/d
> +/dpdkvhostuserclient0: unsupported MTU 9711/d
> +/failed to set MTU for network device dpdkvhostuserclient0: Invalid argument/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -893,11 +870,10 @@ OVS_DPDK_STOP_TESTPMD()
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@VHOST_CONFIG: ($OVS_RUNDIR/dpdkvhostclient0) failed to connect: No such file or directory@d
> -\@dpdkvhostuserclient0: unsupported MTU 67@d
> -\@failed to set MTU for network device dpdkvhostuserclient0: Invalid argument@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/VHOST_CONFIG: (.*dpdkvhostclient0) failed to connect: No such file or directory/d
> +/dpdkvhostuserclient0: unsupported MTU 67/d
> +/failed to set MTU for network device dpdkvhostuserclient0: Invalid argument/d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -932,7 +908,7 @@ OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics | grep -oP 'rx_packe
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br0 p1], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("[SYSTEM_DPDK_ALLOWED_LOGS]")
> +OVS_DPDK_STOP_VSWITCHD
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -965,9 +941,8 @@ OVS_WAIT_UNTIL([test `ovs-vsctl get interface p1 statistics | grep -oP 'rx_packe
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br0 p1], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@upcall: datapath reached the dynamic limit of .* flows.@d
> -])")
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/upcall: datapath reached the dynamic limit of .* flows./d"])
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
>
> @@ -1078,17 +1053,16 @@ ovs-appctl: ovs-vswitchd: server returned an error
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br0 p1], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -\@Error: unknown argument 1.@d
> -\@Error: invalid study_pkt_cnt value: xyz.@d
> -\@Error: unknown argument abcd.@d
> -\@Error: -pmd option requires a thread id argument.@d
> -\@Error: invalid study_pkt_cnt value: abcd.@d
> -\@Error: miniflow extract parser not changed, PMD thread passed is not valid: 'zero'. Pass a valid pmd thread ID.@d
> -\@Error: no miniflow extract name provided. Output of miniflow-parser-get shows implementation list.@d
> -\@Error: unknown miniflow extract implementation superstudy.@d
> -\@Error: invalid study_pkt_cnt value: -pmd.@d
> -])")
>
> +OVS_DPDK_STOP_VSWITCHD(["dnl
> +/Error: unknown argument 1./d
> +/Error: invalid study_pkt_cnt value: xyz./d
> +/Error: unknown argument abcd./d
> +/Error: -pmd option requires a thread id argument./d
> +/Error: invalid study_pkt_cnt value: abcd./d
> +/Error: miniflow extract parser not changed, PMD thread passed is not valid: 'zero'. Pass a valid pmd thread ID./d
> +/Error: no miniflow extract name provided. Output of miniflow-parser-get shows implementation list./d
> +/Error: unknown miniflow extract implementation superstudy./d
> +/Error: invalid study_pkt_cnt value: -pmd./d"])
>  AT_CLEANUP dnl
>  dnl --------------------------------------------------------------------------
>
> @@ -1139,7 +1113,6 @@ OVS_WAIT_UNTIL([tail -n +$TMP ovs-vswitchd.log | grep "Port p1: Requesting a mem
>
>  dnl Clean up
>  AT_CHECK([ovs-vsctl del-port br10 p1], [], [stdout], [stderr])
> -OVS_VSWITCHD_STOP("m4_join([], [SYSTEM_DPDK_ALLOWED_LOGS], [
> -])")
> +OVS_DPDK_STOP_VSWITCHD
>  AT_CLEANUP
>  dnl --------------------------------------------------------------------------
> -- 
> 2.41.0

On Fri, Aug 25, 2023 at 12:57 PM Eelco Chaudron <echaudro@redhat.com> wrote:
> >
> > +m4_define([OVS_DPDK_STOP_VSWITCHD],
> > +  [OVS_VSWITCHD_STOP([dnl
> > +$1";/does not exist. The Open vSwitch kernel module is probably not loaded./d
> > +/does not support MTU configuration,/d
> > +/EAL: No \(available\|free\) .*hugepages reported/d
> > +/Failed to enable flow control/d
> > +/Rx checksum offload is not supported on/d
> > +/TELEMETRY: No legacy callbacks, legacy socket not created/d"])
>
> On my VMs (with all patches applied) all the existing dpdk tests and new ones are failing due to the following error messages:

As you mention, the issue predates this current patch and it points at
a different problem than the logs matching.

>
> +2023-08-25T10:09:42.252Z|00016|dpdk|ERR|eth_virtio_pci_init(): Failed to init PCI device
> +2023-08-25T10:09:42.252Z|00017|dpdk|ERR|EAL: Requested device 0000:00:05.0 cannot be used
> +2023-08-25T10:09:42Z|00016|dpdk|ERR|eth_virtio_pci_init(): Failed to init PCI device
> +2023-08-25T10:09:42Z|00017|dpdk|ERR|EAL: Requested device 0000:00:05.0 cannot be used
>
> Maybe we could exclude them here (this is what I did) or add some --no-pci option (and make sure the ones that do need them have it).
>
> @@ -93,6 +93,8 @@ $1";/does not exist. The Open vSwitch kernel module is probably not loaded./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
> +/eth_virtio_pci_init(): Failed to init PCI device/d
> +c
>  /does not exist. The Open vSwitch kernel module is probably not loaded./d"])
>    ])
>

Those warnings are due to the fact that OVS lets DPDK try to grab all
devices by default.
And here, we are affected by a port that you don't seem to care about.


Besides, about log matching, in the context of the phy port tests, we
do want to catch errors like "EAL: Requested device .* cannot be
used".
You could also imagine someone passing the PCI device of a virtio-net
for those tests to run so I would not filter the virtio errors.
So I don't think we should ignore those specific logs in the common macro.


Passing --no-pci in the tests that wants to probe one pci device is not doable.


What I think instead is that having DPDK probes all devices by default
is not desirable, though it has been working like this since OVS
integrated DPDK.
I have a patch in store for a long time, I just never found an
explicit need for it and it had some drawback in one usecase (multiple
ports for a single pci device), maybe I could send a rfc.

On 25 Aug 2023, at 13:34, David Marchand wrote:

> On Fri, Aug 25, 2023 at 12:57 PM Eelco Chaudron <echaudro@redhat.com> wrote:
>>>
>>> +m4_define([OVS_DPDK_STOP_VSWITCHD],
>>> +  [OVS_VSWITCHD_STOP([dnl
>>> +$1";/does not exist. The Open vSwitch kernel module is probably not loaded./d
>>> +/does not support MTU configuration,/d
>>> +/EAL: No \(available\|free\) .*hugepages reported/d
>>> +/Failed to enable flow control/d
>>> +/Rx checksum offload is not supported on/d
>>> +/TELEMETRY: No legacy callbacks, legacy socket not created/d"])
>>
>> On my VMs (with all patches applied) all the existing dpdk tests and new ones are failing due to the following error messages:
>
> As you mention, the issue predates this current patch and it points at
> a different problem than the logs matching.

I was wrong, it are only the new system tests that are failing, the existing ones have the --no-pci option.

So I guess you should add the same for the system traffic one, i.e. the following seems to work:

--- a/tests/system-dpdk-macros.at
+++ b/tests/system-dpdk-macros.at
@@ -147,7 +147,7 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START],
   [
    OVS_DPDK_PRE_CHECK()
    OVS_WAIT_WHILE([ip link show ovs-netdev])
-   OVS_DPDK_START([], [--disable-system --no-pci], [$3])
+   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])



>>
>> +2023-08-25T10:09:42.252Z|00016|dpdk|ERR|eth_virtio_pci_init(): Failed to init PCI device
>> +2023-08-25T10:09:42.252Z|00017|dpdk|ERR|EAL: Requested device 0000:00:05.0 cannot be used
>> +2023-08-25T10:09:42Z|00016|dpdk|ERR|eth_virtio_pci_init(): Failed to init PCI device
>> +2023-08-25T10:09:42Z|00017|dpdk|ERR|EAL: Requested device 0000:00:05.0 cannot be used
>>
>> Maybe we could exclude them here (this is what I did) or add some --no-pci option (and make sure the ones that do need them have it).
>>
>> @@ -93,6 +93,8 @@ $1";/does not exist. The Open vSwitch kernel module is probably not loaded./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
>> +/eth_virtio_pci_init(): Failed to init PCI device/d
>> +c
>>  /does not exist. The Open vSwitch kernel module is probably not loaded./d"])
>>    ])
>>
>
> Those warnings are due to the fact that OVS lets DPDK try to grab all
> devices by default.
> And here, we are affected by a port that you don't seem to care about.
>
>
> Besides, about log matching, in the context of the phy port tests, we
> do want to catch errors like "EAL: Requested device .* cannot be
> used".
> You could also imagine someone passing the PCI device of a virtio-net
> for those tests to run so I would not filter the virtio errors.
> So I don't think we should ignore those specific logs in the common macro.
>
>
> Passing --no-pci in the tests that wants to probe one pci device is not doable.
>
>
> What I think instead is that having DPDK probes all devices by default
> is not desirable, though it has been working like this since OVS
> integrated DPDK.
> I have a patch in store for a long time, I just never found an
> explicit need for it and it had some drawback in one usecase (multiple
> ports for a single pci device), maybe I could send a rfc.

See above, as it’s a false alarm and we do not need to make this work ;)

On Fri, Aug 25, 2023 at 2:06 PM Eelco Chaudron <echaudro@redhat.com> wrote:
> > What I think instead is that having DPDK probes all devices by default
> > is not desirable, though it has been working like this since OVS
> > integrated DPDK.
> > I have a patch in store for a long time, I just never found an
> > explicit need for it and it had some drawback in one usecase (multiple
> > ports for a single pci device), maybe I could send a rfc.
>
> See above, as it’s a false alarm and we do not need to make this work ;)

Ah.. too bad, for next time :-).