On Wed, Oct 04, 2023 at 02:27:17PM +0200, Jakob Meng wrote: > On 04.10.23 12:18, Ilya Maximets wrote: > > On 10/2/23 16:44, David Marchand wrote: > >> On Mon, Oct 2, 2023 at 1:52 PM Simon Horman <[email protected]> wrote: > >>> On Wed, Sep 27, 2023 at 03:24:07PM +0200, [email protected] wrote: > >>>> From: Jakob Meng <[email protected]> > >>>> > >>>> For better usability, the function pairs get_config() and > >>>> set_config() for each netdev should be symmetric: Options which are > >>>> accepted by set_config() should be returned by get_config() and the > >>>> latter should output valid options for set_config() only. > >>>> > >>>> This patch moves key-value pairs which are no valid options from > >>>> get_config() to the get_status() callback. For example, get_config() > >>>> in lib/netdev-dpdk.c returned {configured,requested}_{rx,tx}_queues > >>>> previously. For requested rx queues the proper option name is n_rxq, > >>>> so requested_rx_queues has been renamed respectively. Tx queues > >>>> cannot be changed by the user, hence requested_tx_queues has been > >>>> dropped. Both configured_{rx,tx}_queues will be returned as > >>>> n_{r,t}xq in the get_status() callback. > >>>> > >>>> The documentation in vswitchd/vswitch.xml for status columns as well > >>>> as tests have been updated accordingly. > >>>> > >>>> Reported-at: https://bugzilla.redhat.com/1949855 > >>>> Signed-off-by: Jakob Meng <[email protected]> > >>> Hi Jacob, > >>> > >>> I see some CI failures with this change, > >>> but I'm unsure if they are related to the change or not. > >>> > >>> Could you take a look? > >> I can reproduce issues on vhost-user port mtu tests. > >> > >> I understand we have a race now. > >> > >> Previously, the test was calling an explicit dpctl/show which > >> synchronuously invoked each port get_config(), then matching this > >> command output against mtu=9000. > >> Now with this patch, the test looks at the interface status field in ovsdb. > >> My understanding is that this status field may not have been refreshed > >> *yet* (or there is another issue and I missed some sync point..). > >> > >> > >> Adding a sleep 1 before "ovs-vsctl get Interface dpdkvhostuserclient0 > >> mtu" does the trick for me... > >> > >> I tested adding a check on the vhost-user port link status (see below). > >> Link status of a vhu port is supposed to be up once the port has been > >> reconfigured (vhost_reconfigured == which means that > >> netdev_dpdk_mempool_configure() has been called). > >> > >> This seems to work, though I never noticed a "down" status in my runs. > >> So maybe adding one ovs-vsctl command was enough to put some delay in the > >> test.. > >> > >> $ git diff > >> diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at > >> index 95aa3e906b..59fec6903f 100644 > >> --- a/tests/system-dpdk.at > >> +++ b/tests/system-dpdk.at > >> @@ -749,6 +749,7 @@ tail -f /dev/null | dpdk-testpmd > >> --socket-mem="$(cat NUMA_NODE)" --no-pci\ > >> --single-file-segments -- -a > >>> $OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log 2>&1 & > >> OVS_WAIT_UNTIL([grep "virtio is now ready for processing" > >> ovs-vswitchd.log]) > >> +OVS_WAIT_UNTIL([ovs-vsctl get Interface dpdkvhostuserclient0 > >> link_state | grep -w up]) > >> > >> dnl Check MTU value in the datapath > >> AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl > >> @@ -887,6 +888,7 @@ tail -f /dev/null | dpdk-testpmd > >> --socket-mem="$(cat NUMA_NODE)" --no-pci\ > >> --single-file-segments -- -a > >>> $OVS_RUNDIR/testpmd-dpdkvhostuserclient0.log 2>&1 & > >> OVS_WAIT_UNTIL([grep "virtio is now ready for processing" > >> ovs-vswitchd.log]) > >> +OVS_WAIT_UNTIL([ovs-vsctl get Interface dpdkvhostuserclient0 > >> link_state | grep -w up]) > >> > >> dnl Check MTU value in the datapath > >> AT_CHECK([ovs-vsctl get Interface dpdkvhostuserclient0 mtu], [0], [dnl > > Thanks, David. Yeah, there is a slight delay for the database update. > > Waiting for the port to come 'up' seems reasonable. > > > > Best regards, Ilya Maximets. > > > > Thank you all for your suggestions and David for finding a fix for the DPDK > tests ☺️ > > I have incorporated everything into patch v4 which is out now: > > https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/ >
Thanks, I concur that v4 addresses the issues discussed here. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
