On 6/14/24 17:08, David Marchand 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 <david.march...@redhat.com>
> Reviewed-by: Robin Jarry <rja...@redhat.com>
> Acked-by: Mike Pattrick <m...@redhat.com>
> ---
> Changes since v2:
> - fixed typo in NEWS,
>
> Changes since v1:
> - (early) fail when interrupt lsc is requested by user but not supported
> by the driver,
> - otherwise, log a debug message if user did not request interrupt mode,
>
> ---
> Documentation/topics/dpdk/phy.rst | 4 ++--
> NEWS | 3 +++
> lib/netdev-dpdk.c | 13 ++++++++++++-
> vswitchd/vswitch.xml | 8 ++++----
> 4 files changed, 21 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..d05f2d0f89 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 'options:dpdk-lsc-interrupt' to 'false'.
>
>
> v3.3.0 - 16 Feb 2024
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 0fa37d5145..a260bc8485 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2397,7 +2397,18 @@ 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)) {
> + if (smap_get(args, "dpdk-lsc-interrupt")) {
> + VLOG_ERR("interface '%s': link status interrupt is not
> supported.",
> + netdev_get_name(netdev));
Since we're exiting with an error set, the message should be buffered
into errp instead, so it can be visible in the database record and
returned as a result of the ovs-vsctl.
Also, we're using WARN level for all other configuration issues, so we
should do that here as well. ERR is usually some sort of internal error.
And we're usually just using "%s: ..." and not "interface '%s': ...".
> + err = EINVAL;
> + goto out;
> + }
> + VLOG_DBG("interface '%s': not enabling link status interrupt.",
> + netdev_get_name(netdev));
> + lsc_interrupt_mode = false;
> + }
> if (dev->requested_lsc_interrupt_mode != lsc_interrupt_mode) {
> dev->requested_lsc_interrupt_mode = lsc_interrupt_mode;
> netdev_request_reconfigure(netdev);
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 8a1b607d71..e3afb78a4e 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -4647,12 +4647,12 @@ ovs-vsctl add-port br0 p0 -- set Interface p0
> type=patch options:peer=p1 \
> <column name="options" key="dpdk-lsc-interrupt"
> type='{"type": "boolean"}'>
> <p>
> - Set this value to <code>true</code> to configure interrupt mode for
> - Link State Change (LSC) detection instead of poll mode for the DPDK
> - interface.
> + Set this value to <code>false</code> to configure poll mode for
> + Link State Change (LSC) detection instead of interrupt mode for the
> + DPDK interface.
> </p>
> <p>
> - If this value is not set, poll mode is configured.
> + If this value is not set, interrupt mode is configured.
> </p>
> <p>
> This parameter has an effect only on netdev dpdk interfaces.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
