RE: [PATCH] failsafe: fix segfault on hotplug event

[Konstantin Ananyev <konstantin.ananyev@huawei.com>]

 
> 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 7a0935239b, 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.

 
It is not recommended way to update rte_eth_fp_ops[] contents directly.
There are eth_dev_fp_ops_setup()/ eth_dev_fp_ops_reset() that supposed
to be used for that.
About the fix itself - while it might help till some extent,
I think it will not remove the problem completely.
There still remain a race-condition between rte_eth_rx_burst() and failsafe_eth_rmv_event_callback().
Right now DPDK doesn't support switching PMD fast-ops functions (or updating rxq/txq data)
on the fly.
  
> Fixes: 7a0935239b ("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>
> ---
>  drivers/net/failsafe/failsafe_rxtx.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> 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();
>  }
> --
> 2.25.1