| Message ID | 20230511161309.4138397-1-frode.nordahl@canonical.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v2] tests dpdk: Allow passing in dpdk-extra arguments. | 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 5/11/23 18:13, Frode Nordahl wrote: > At present, the system-dpdk-testsuite makes assumptions about > environment configuration, and will error out if DPDK compatible > interfaces not configured for DPDK are present in the system with > a message like: > > EAL: Probe PCI driver: net_virtio (1af4:1000) device: 0000:00:03.0 (socket -1) > eth_virtio_pci_init(): Failed to init PCI device > EAL: Requested device 0000:00:03.0 cannot be used Hmm. We should probably pass --no-pci to all tests that do not use physical ports. We might add an argument to OVS_DPDK_START/OVS_DPDK_START_VSWITCHD to indicate phy tests. It should make tests a bit faster, since no unnecessary PCI scans will be performed. Will that solve the issue for you? Best regards, Ilya Maximets. > > The system-dpdk-testsuite is useful even with no DPDK PHY > available, as the tests requiring a PHY will skip gracefully when > none present. > > This patch allows passing in values that will be set in > `other_config:dpdk-extra` before the test runs, which among > other things, would allow to use the DPDK EAL block (-b) and > allow (-a) options. Having those available would make it possible > to run the testsuite unaltered in more environments. > > We will use this patch in a follow-up, enabling more elaborate > Debian autopkgtests for Open vSwitch. > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> > --- > tests/system-dpdk-macros.at | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at > index 53fbc1320..c74e8a0f1 100644 > --- a/tests/system-dpdk-macros.at > +++ b/tests/system-dpdk-macros.at > @@ -72,7 +72,7 @@ m4_define([OVS_DPDK_START_OVSDB], > # > m4_define([OVS_DPDK_START_VSWITCHD], > [dnl Change DPDK drivers log levels so that tests only catch errors > - AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-extra=--log-level=pmd.*:error]) > + AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-extra="--log-level=pmd.*:error $TESTSUITE_DPDK_EXTRA"]) > > dnl Start ovs-vswitchd. > AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif -vunixctl], [0], [stdout], [stderr])
On Thu, May 11, 2023 at 11:07 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > On 5/11/23 18:13, Frode Nordahl wrote: > > At present, the system-dpdk-testsuite makes assumptions about > > environment configuration, and will error out if DPDK compatible > > interfaces not configured for DPDK are present in the system with > > a message like: > > > > EAL: Probe PCI driver: net_virtio (1af4:1000) device: 0000:00:03.0 (socket -1) > > eth_virtio_pci_init(): Failed to init PCI device > > EAL: Requested device 0000:00:03.0 cannot be used > > Hmm. We should probably pass --no-pci to all tests that > do not use physical ports. We might add an argument to > OVS_DPDK_START/OVS_DPDK_START_VSWITCHD to indicate phy tests. > It should make tests a bit faster, since no unnecessary PCI > scans will be performed. > > Will that solve the issue for you? Yes, it would, and thank you for the suggestion/direction. m4 does not appear to be too flexible wrt. conditional processing of macro arguments, so would you be ok with me adding an argument that would end up in the dpdk-extra config directly? At least it would be more declared/defined than passing in arbitrary values from the environment. Something like: --- a/tests/system-dpdk-macros.at +++ b/tests/system-dpdk-macros.at @@ -42,7 +42,7 @@ m4_define([OVS_DPDK_START], OVS_DPDK_START_OVSDB() dnl Enable DPDK functionality AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-init=true]) - OVS_DPDK_START_VSWITCHD() + OVS_DPDK_START_VSWITCHD($1) ]) # OVS_DPDK_START_OVSDB() @@ -72,7 +72,7 @@ m4_define([OVS_DPDK_START_OVSDB], # m4_define([OVS_DPDK_START_VSWITCHD], [dnl Change DPDK drivers log levels so that tests only catch errors - AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-extra=--log-level=pmd.*:error]) + 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]) -- Frode Nordahl > Best regards, Ilya Maximets. > > > > > The system-dpdk-testsuite is useful even with no DPDK PHY > > available, as the tests requiring a PHY will skip gracefully when > > none present. > > > > This patch allows passing in values that will be set in > > `other_config:dpdk-extra` before the test runs, which among > > other things, would allow to use the DPDK EAL block (-b) and > > allow (-a) options. Having those available would make it possible > > to run the testsuite unaltered in more environments. > > > > We will use this patch in a follow-up, enabling more elaborate > > Debian autopkgtests for Open vSwitch. > > > > Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> > > --- > > tests/system-dpdk-macros.at | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at > > index 53fbc1320..c74e8a0f1 100644 > > --- a/tests/system-dpdk-macros.at > > +++ b/tests/system-dpdk-macros.at > > @@ -72,7 +72,7 @@ m4_define([OVS_DPDK_START_OVSDB], > > # > > m4_define([OVS_DPDK_START_VSWITCHD], > > [dnl Change DPDK drivers log levels so that tests only catch errors > > - AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-extra=--log-level=pmd.*:error]) > > + AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-extra="--log-level=pmd.*:error $TESTSUITE_DPDK_EXTRA"]) > > > > dnl Start ovs-vswitchd. > > AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif -vunixctl], [0], [stdout], [stderr]) >
On 5/12/23 11:13, Frode Nordahl wrote: > On Thu, May 11, 2023 at 11:07 PM Ilya Maximets <i.maximets@ovn.org> wrote: >> >> On 5/11/23 18:13, Frode Nordahl wrote: >>> At present, the system-dpdk-testsuite makes assumptions about >>> environment configuration, and will error out if DPDK compatible >>> interfaces not configured for DPDK are present in the system with >>> a message like: >>> >>> EAL: Probe PCI driver: net_virtio (1af4:1000) device: 0000:00:03.0 (socket -1) >>> eth_virtio_pci_init(): Failed to init PCI device >>> EAL: Requested device 0000:00:03.0 cannot be used >> >> Hmm. We should probably pass --no-pci to all tests that >> do not use physical ports. We might add an argument to >> OVS_DPDK_START/OVS_DPDK_START_VSWITCHD to indicate phy tests. >> It should make tests a bit faster, since no unnecessary PCI >> scans will be performed. >> >> Will that solve the issue for you? > > Yes, it would, and thank you for the suggestion/direction. > > m4 does not appear to be too flexible wrt. conditional processing Just FYI, there is an m4_if and m4_if_val. These should be enough. But what you have in v3 should also be fine. I'll take a closer look at it tomorrow. > of macro arguments, so would you be ok with me adding an argument that > would end up in the dpdk-extra config directly? At least it would be more > declared/defined than passing in arbitrary values from the environment. > > Something like: > --- a/tests/system-dpdk-macros.at > +++ b/tests/system-dpdk-macros.at > @@ -42,7 +42,7 @@ m4_define([OVS_DPDK_START], > OVS_DPDK_START_OVSDB() > dnl Enable DPDK functionality > AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . > other_config:dpdk-init=true]) > - OVS_DPDK_START_VSWITCHD() > + OVS_DPDK_START_VSWITCHD($1) > ]) > > # OVS_DPDK_START_OVSDB() > @@ -72,7 +72,7 @@ m4_define([OVS_DPDK_START_OVSDB], > # > m4_define([OVS_DPDK_START_VSWITCHD], > [dnl Change DPDK drivers log levels so that tests only catch errors > - AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . > other_config:dpdk-extra=--log-level=pmd.*:error]) > + 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]) > > -- > Frode Nordahl > >> Best regards, Ilya Maximets. >> >>> >>> The system-dpdk-testsuite is useful even with no DPDK PHY >>> available, as the tests requiring a PHY will skip gracefully when >>> none present. >>> >>> This patch allows passing in values that will be set in >>> `other_config:dpdk-extra` before the test runs, which among >>> other things, would allow to use the DPDK EAL block (-b) and >>> allow (-a) options. Having those available would make it possible >>> to run the testsuite unaltered in more environments. >>> >>> We will use this patch in a follow-up, enabling more elaborate >>> Debian autopkgtests for Open vSwitch. >>> >>> Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> >>> --- >>> tests/system-dpdk-macros.at | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at >>> index 53fbc1320..c74e8a0f1 100644 >>> --- a/tests/system-dpdk-macros.at >>> +++ b/tests/system-dpdk-macros.at >>> @@ -72,7 +72,7 @@ m4_define([OVS_DPDK_START_OVSDB], >>> # >>> m4_define([OVS_DPDK_START_VSWITCHD], >>> [dnl Change DPDK drivers log levels so that tests only catch errors >>> - AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-extra=--log-level=pmd.*:error]) >>> + AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-extra="--log-level=pmd.*:error $TESTSUITE_DPDK_EXTRA"]) >>> >>> dnl Start ovs-vswitchd. >>> AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif -vunixctl], [0], [stdout], [stderr]) >>
diff --git a/tests/system-dpdk-macros.at b/tests/system-dpdk-macros.at index 53fbc1320..c74e8a0f1 100644 --- a/tests/system-dpdk-macros.at +++ b/tests/system-dpdk-macros.at @@ -72,7 +72,7 @@ m4_define([OVS_DPDK_START_OVSDB], # m4_define([OVS_DPDK_START_VSWITCHD], [dnl Change DPDK drivers log levels so that tests only catch errors - AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-extra=--log-level=pmd.*:error]) + AT_CHECK([ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-extra="--log-level=pmd.*:error $TESTSUITE_DPDK_EXTRA"]) dnl Start ovs-vswitchd. AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif -vunixctl], [0], [stdout], [stderr])
At present, the system-dpdk-testsuite makes assumptions about environment configuration, and will error out if DPDK compatible interfaces not configured for DPDK are present in the system with a message like: EAL: Probe PCI driver: net_virtio (1af4:1000) device: 0000:00:03.0 (socket -1) eth_virtio_pci_init(): Failed to init PCI device EAL: Requested device 0000:00:03.0 cannot be used The system-dpdk-testsuite is useful even with no DPDK PHY available, as the tests requiring a PHY will skip gracefully when none present. This patch allows passing in values that will be set in `other_config:dpdk-extra` before the test runs, which among other things, would allow to use the DPDK EAL block (-b) and allow (-a) options. Having those available would make it possible to run the testsuite unaltered in more environments. We will use this patch in a follow-up, enabling more elaborate Debian autopkgtests for Open vSwitch. Signed-off-by: Frode Nordahl <frode.nordahl@canonical.com> --- tests/system-dpdk-macros.at | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)