On Thu, Jun 13, 2024 at 4:32 PM Mike Pattrick <[email protected]> wrote: > > On Thu, Jun 13, 2024 at 5:59 AM David Marchand > <[email protected]> wrote: > > > > Querying link status may get delayed for an undeterministic (long) time > > with mlx5 ports. This is a consequence of the mlx5 driver calling ethtool > > kernel API and getting stuck on the kernel RTNL lock while some other > > operation is in progress under this lock. > > > > One impact for long link status query is that it is called under the bond > > lock taken in write mode periodically in bond_run(). > > In parallel, datapath threads may block requesting to read bonding related > > info (like for example in bond_check_admissibility()). > > > > The LSC interrupt mode is available with many DPDK drivers and is used by > > default with testpmd. > > > > It seems safe enough to switch on this feature by default in OVS. > > We keep the per interface option to disable this feature in case of an > > unforeseen bug. > > > > Signed-off-by: David Marchand <[email protected]> > > --- > > Documentation/topics/dpdk/phy.rst | 4 ++-- > > NEWS | 3 +++ > > lib/netdev-dpdk.c | 7 ++++++- > > vswitchd/vswitch.xml | 8 ++++---- > > 4 files changed, 15 insertions(+), 7 deletions(-) > > > > diff --git a/Documentation/topics/dpdk/phy.rst > > b/Documentation/topics/dpdk/phy.rst > > index efd168cba8..eefc25613d 100644 > > --- a/Documentation/topics/dpdk/phy.rst > > +++ b/Documentation/topics/dpdk/phy.rst > > @@ -546,8 +546,8 @@ the firmware every time to fulfil this request. > > > > Note that not all PMD drivers support LSC interrupts. > > > > -The default configuration is polling mode. To set interrupt mode, option > > -``dpdk-lsc-interrupt`` has to be set to ``true``. > > +The default configuration is interrupt mode. To set polling mode, option > > +``dpdk-lsc-interrupt`` has to be set to ``false``. > > > > Command to set interrupt mode for a specific interface:: > > $ ovs-vsctl set interface <iface_name> options:dpdk-lsc-interrupt=true > > diff --git a/NEWS b/NEWS > > index 5ae0108d55..1e19beb793 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -9,6 +9,9 @@ Post-v3.3.0 > > https://github.com/openvswitch/ovs.git > > - DPDK: > > * OVS validated with DPDK 23.11.1. > > + * Link status changes are now handled via interrupt mode if the DPDK > > + driver supports it. It is possible to revert to polling mode by > > setting > > + per interface 'dpdk-lsc-interrupt' other_config to 'false'. > > > > > > v3.3.0 - 16 Feb 2024 > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > > index 0fa37d5145..833d852319 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -2397,7 +2397,12 @@ netdev_dpdk_set_config(struct netdev *netdev, const > > struct smap *args, > > } > > } > > > > - lsc_interrupt_mode = smap_get_bool(args, "dpdk-lsc-interrupt", false); > > + lsc_interrupt_mode = smap_get_bool(args, "dpdk-lsc-interrupt", true); > > + if (lsc_interrupt_mode && !(*info.dev_flags & RTE_ETH_DEV_INTR_LSC)) { > > + VLOG_INFO("interface '%s': link status interrupt is not > > supported.", > > + netdev_get_name(netdev)); > > + lsc_interrupt_mode = false; > > + } > > I did a quick grep of the dpdk source tree and noticed the following > drivers had this define: > > bnxt dpaa dpaa2 ena failsafe hns3 mlx4 mlx5 netvsc tap vhost virtio
For "normal" PCI drivers, the flag to look for is RTE_PCI_DRV_INTR_LSC $ git grep -A1 RTE_PCI_DRV_INTR_LSC lib/ethdev/ lib/ethdev/ethdev_pci.h: if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC) lib/ethdev/ethdev_pci.h- eth_dev->data->dev_flags |= RTE_ETH_DEV_INTR_LSC; > > This may have been an invalid methodology, but I'm noticing some > drivers for very common network cards are missing. Is there a risk > here of starting to print these info messages all the time for a large > number of users who never set this feature in the first place? $ for dir in drivers/net/*; do git grep -q -E 'RTE_PCI_DRV_INTR_LSC|RTE_ETH_DEV_INTR_LSC' $dir && echo "OK for "$dir || echo "==== Missing for "$dir; done | grep -c OK 34 $ for dir in drivers/net/*; do git grep -q -E 'RTE_PCI_DRV_INTR_LSC|RTE_ETH_DEV_INTR_LSC' $dir && echo "OK for "$dir || echo "==== Missing for "$dir; done | grep -c Missing 25 The majority of net drivers has this feature. > > It may be preferable to continue defaulting to true, but only print > the error if the flag was explicitly set. I had considered this option, but I went with the simpler approach :-). Ok since you thought of it too, let me update for v2. -- David Marchand _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
