[v2] failsafe: fix segfault on hotplug event

Message ID 20221110174241.16061-1-lucp.at.work@gmail.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v2] failsafe: fix segfault on hotplug event |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Luc Pelletier Nov. 10, 2022, 5:42 p.m. UTC
  When the failsafe PMD encounters a hotplug event, it switches its rx/tx
functions to "safe" ones that validate the sub-device's rx/tx functions
before calling them. It switches the rx/tx functions by changing the
function pointers in the rte_eth_dev structure.

Following commit 7a0935239b9e, the rx/tx functions of PMDs are no longer
called through the function pointers in the rte_eth_dev structure. They
are rather called through a flat array named rte_eth_fp_ops. The
function pointers in that array are initialized when the devices start
and are initialized.

When a hotplug event occurs, the function pointers in rte_eth_fp_ops
still point to the "unsafe" rx/tx functions in the failsafe PMD since
they haven't been updated. This results in a segmentation fault because
it ends up using the "unsafe" functions, when the "safe" functions
should have been used.

To fix the problem, the failsafe PMD code was changed to update the
function pointers in the rte_eth_fp_ops array when a hotplug event
occurs.

Fixes: 7a0935239b9e ("ethdev: make fast-path functions to use new flat array")
Cc: Konstantin Ananyev <konstantin.ananyev@intel.com>
Cc: stable@dpdk.org

Signed-off-by: Luc Pelletier <lucp.at.work@gmail.com>
---

v2:
* fixed git commit hashes in commit message

 drivers/net/failsafe/failsafe_rxtx.c | 9 +++++++++
 1 file changed, 9 insertions(+)
  

Comments

Stephen Hemminger Nov. 10, 2022, 11:33 p.m. UTC | #1
On Thu, 10 Nov 2022 12:42:43 -0500
Luc Pelletier <lucp.at.work@gmail.com> wrote:

> When the failsafe PMD encounters a hotplug event, it switches its rx/tx
> functions to "safe" ones that validate the sub-device's rx/tx functions
> before calling them. It switches the rx/tx functions by changing the
> function pointers in the rte_eth_dev structure.
> 
> Following commit 7a0935239b9e, the rx/tx functions of PMDs are no longer
> called through the function pointers in the rte_eth_dev structure. They
> are rather called through a flat array named rte_eth_fp_ops. The
> function pointers in that array are initialized when the devices start
> and are initialized.
> 
> When a hotplug event occurs, the function pointers in rte_eth_fp_ops
> still point to the "unsafe" rx/tx functions in the failsafe PMD since
> they haven't been updated. This results in a segmentation fault because
> it ends up using the "unsafe" functions, when the "safe" functions
> should have been used.
> 
> To fix the problem, the failsafe PMD code was changed to update the
> function pointers in the rte_eth_fp_ops array when a hotplug event
> occurs.

Have it in both places might be breaking other drivers as well.
Shouldn't there be a ethdev function when changing rx/tx burst.

Also, changing a variable used by another thread needs to be
using __atomic_store and __atomic_load to guarantee that CPU
or compiler will no that it changed.
  

Patch

diff --git a/drivers/net/failsafe/failsafe_rxtx.c b/drivers/net/failsafe/failsafe_rxtx.c
index fe67293299..34d59dfbb1 100644
--- a/drivers/net/failsafe/failsafe_rxtx.c
+++ b/drivers/net/failsafe/failsafe_rxtx.c
@@ -5,6 +5,7 @@ 
 
 #include <rte_atomic.h>
 #include <rte_debug.h>
+#include <rte_ethdev.h>
 #include <rte_mbuf.h>
 #include <ethdev_driver.h>
 
@@ -44,9 +45,13 @@  failsafe_set_burst_fn(struct rte_eth_dev *dev, int force_safe)
 		DEBUG("Using safe RX bursts%s",
 		      (force_safe ? " (forced)" : ""));
 		dev->rx_pkt_burst = &failsafe_rx_burst;
+		rte_eth_fp_ops[dev->data->port_id].rx_pkt_burst =
+			&failsafe_rx_burst;
 	} else if (!need_safe && safe_set) {
 		DEBUG("Using fast RX bursts");
 		dev->rx_pkt_burst = &failsafe_rx_burst_fast;
+		rte_eth_fp_ops[dev->data->port_id].rx_pkt_burst =
+			&failsafe_rx_burst_fast;
 	}
 	need_safe = force_safe || fs_tx_unsafe(TX_SUBDEV(dev));
 	safe_set = (dev->tx_pkt_burst == &failsafe_tx_burst);
@@ -54,9 +59,13 @@  failsafe_set_burst_fn(struct rte_eth_dev *dev, int force_safe)
 		DEBUG("Using safe TX bursts%s",
 		      (force_safe ? " (forced)" : ""));
 		dev->tx_pkt_burst = &failsafe_tx_burst;
+		rte_eth_fp_ops[dev->data->port_id].tx_pkt_burst =
+			&failsafe_tx_burst;
 	} else if (!need_safe && safe_set) {
 		DEBUG("Using fast TX bursts");
 		dev->tx_pkt_burst = &failsafe_tx_burst_fast;
+		rte_eth_fp_ops[dev->data->port_id].tx_pkt_burst =
+			&failsafe_tx_burst_fast;
 	}
 	rte_wmb();
 }