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