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. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
