Skip to content

Commit

Permalink
netdev-dpdk: Trigger port reconfiguration in main thread for resets.
Browse files Browse the repository at this point in the history
When OVS (main thread) configures a DPDK netdev, it holds a netdev_dpdk
mutex lock.
As part of this configure operation, the net/iavf driver (used with i40e
VF devices) triggers a queue count change. The PF entity (serviced by a
kernel PF driver for example) handles this change and requests back that
the VF driver resets the VF device. The driver then completes the VF reset
operation on its side and waits for completion of the iavf-event thread
responsible for handling various VF device events.

On the other hand, handling of the VF reset request in this iavf-event
thread results in notifying the application with a port reset request
(RTE_ETH_EVENT_INTR_RESET). The OVS reset callback tries to take a hold
of the same netdev_dpdk mutex and blocks the iavf-event thread.

As a result, the net/iavf driver (still running on OVS main thread) is
unable to complete as it is waiting for iavf-event to complete.

To break from this situation, the OVS reset callback now won't take a
netdev_dpdk mutex. Instead, the port reset request is stored in a simple
RTE_ETH_MAXPORTS array associated to a seq object.
This is enough to let the VF driver complete this port initialization.
The OVS main thread later handles the port reset request.

More details in the DPDK upstream bz as this issue appeared following a
change in DPDK.

Link: https://bugs.dpdk.org/show_bug.cgi?id=1337
Signed-off-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
  • Loading branch information
david-marchand authored and igsilya committed Jan 19, 2024
1 parent 9ca8d3a commit 3eb91a8
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 22 deletions.
7 changes: 0 additions & 7 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,6 @@ v3.3.0 - xx xxx xxxx
- Support for multicast snooping to show the protocol responsible for
adding/updating the entry.

Known issues:
- DPDK: v23.11 has a change in behavior in handling i40e VF devices. This
may block and prevent OVS from adding such devices as ports in a netdev
datapath bridge.
For the details, see https://bugs.dpdk.org/show_bug.cgi?id=1337 which
describes the issue first detected in the 21.11 LTS branch.


v3.2.0 - 17 Aug 2023
--------------------
Expand Down
76 changes: 61 additions & 15 deletions lib/netdev-dpdk.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
#include "openvswitch/match.h"
#include "openvswitch/ofp-parse.h"
#include "openvswitch/ofp-print.h"
#include "openvswitch/poll-loop.h"
#include "openvswitch/shash.h"
#include "openvswitch/vlog.h"
#include "ovs-numa.h"
Expand Down Expand Up @@ -2101,32 +2102,73 @@ netdev_dpdk_process_devargs(struct netdev_dpdk *dev,
return new_port_id;
}

static struct seq *netdev_dpdk_reset_seq;
static uint64_t netdev_dpdk_last_reset_seq;
static atomic_bool netdev_dpdk_pending_reset[RTE_MAX_ETHPORTS];

static void
netdev_dpdk_wait(const struct netdev_class *netdev_class OVS_UNUSED)
{
uint64_t last_reset_seq = seq_read(netdev_dpdk_reset_seq);

if (netdev_dpdk_last_reset_seq == last_reset_seq) {
seq_wait(netdev_dpdk_reset_seq, netdev_dpdk_last_reset_seq);
} else {
poll_immediate_wake();
}
}

static void
netdev_dpdk_run(const struct netdev_class *netdev_class OVS_UNUSED)
{
uint64_t reset_seq = seq_read(netdev_dpdk_reset_seq);

if (reset_seq != netdev_dpdk_last_reset_seq) {
dpdk_port_t port_id;

netdev_dpdk_last_reset_seq = reset_seq;

for (port_id = 0; port_id < RTE_MAX_ETHPORTS; port_id++) {
struct netdev_dpdk *dev;
bool pending_reset;

atomic_read_relaxed(&netdev_dpdk_pending_reset[port_id],
&pending_reset);
if (!pending_reset) {
continue;
}
atomic_store_relaxed(&netdev_dpdk_pending_reset[port_id], false);

ovs_mutex_lock(&dpdk_mutex);
dev = netdev_dpdk_lookup_by_port_id(port_id);
if (dev) {
ovs_mutex_lock(&dev->mutex);
dev->reset_needed = true;
netdev_request_reconfigure(&dev->up);
VLOG_DBG_RL(&rl, "%s: Device reset requested.",
netdev_get_name(&dev->up));
ovs_mutex_unlock(&dev->mutex);
}
ovs_mutex_unlock(&dpdk_mutex);
}
}
}

static int
dpdk_eth_event_callback(dpdk_port_t port_id, enum rte_eth_event_type type,
void *param OVS_UNUSED, void *ret_param OVS_UNUSED)
{
struct netdev_dpdk *dev;

switch ((int) type) {
case RTE_ETH_EVENT_INTR_RESET:
ovs_mutex_lock(&dpdk_mutex);
dev = netdev_dpdk_lookup_by_port_id(port_id);
if (dev) {
ovs_mutex_lock(&dev->mutex);
dev->reset_needed = true;
netdev_request_reconfigure(&dev->up);
VLOG_DBG_RL(&rl, "%s: Device reset requested.",
netdev_get_name(&dev->up));
ovs_mutex_unlock(&dev->mutex);
}
ovs_mutex_unlock(&dpdk_mutex);
atomic_store_relaxed(&netdev_dpdk_pending_reset[port_id], true);
seq_change(netdev_dpdk_reset_seq);
break;

default:
/* Ignore all other types. */
break;
}
return 0;
}
return 0;
}

static void
Expand Down Expand Up @@ -5001,6 +5043,8 @@ netdev_dpdk_class_init(void)
"[netdev]", 0, 1,
netdev_dpdk_get_mempool_info, NULL);

netdev_dpdk_reset_seq = seq_create();
netdev_dpdk_last_reset_seq = seq_read(netdev_dpdk_reset_seq);
ret = rte_eth_dev_callback_register(RTE_ETH_ALL,
RTE_ETH_EVENT_INTR_RESET,
dpdk_eth_event_callback, NULL);
Expand Down Expand Up @@ -6593,6 +6637,8 @@ parse_vhost_config(const struct smap *ovs_other_config)
#define NETDEV_DPDK_CLASS_BASE \
NETDEV_DPDK_CLASS_COMMON, \
.init = netdev_dpdk_class_init, \
.run = netdev_dpdk_run, \
.wait = netdev_dpdk_wait, \
.destruct = netdev_dpdk_destruct, \
.set_tx_multiq = netdev_dpdk_set_tx_multiq, \
.get_carrier = netdev_dpdk_get_carrier, \
Expand Down

0 comments on commit 3eb91a8

Please sign in to comment.