DPDK patches and discussions
 help / color / mirror / Atom feed
From: Anatoly Burakov <anatoly.burakov@intel.com>
To: dev@dpdk.org, Alvin Zhang <alvinx.zhang@intel.com>,
	Ferruh Yigit <ferruh.yigit@intel.com>
Subject: [PATCH v2 3/4] net/e1000: prevent crashes in secondary processes
Date: Mon, 17 Feb 2025 13:54:07 +0000	[thread overview]
Message-ID: <421d8b0458f2ab9912413705247cef7b1c593ff4.1739800426.git.anatoly.burakov@intel.com> (raw)
In-Reply-To: <cd3ef1f143afdef81cf7086bf0e6a3e49c43fbff.1739800426.git.anatoly.burakov@intel.com>

Currently, the architecture of e1000 base driver is such that it uses
function pointers internally. These are not guaranteed to be valid in
secondary processes, which can lead to crashes. This patch prevents IGC
ethdev driver from calling into these functions.

Fixes: 4f09bc55ac3d ("net/igc: implement device base operations")
Cc: stable@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
 doc/guides/nics/igc.rst              |  5 ++
 drivers/net/intel/e1000/igc_ethdev.c | 97 ++++++++++++++++++++++++++++
 2 files changed, 102 insertions(+)

diff --git a/doc/guides/nics/igc.rst b/doc/guides/nics/igc.rst
index c267431c5f..9790b58102 100644
--- a/doc/guides/nics/igc.rst
+++ b/doc/guides/nics/igc.rst
@@ -104,3 +104,8 @@ Add a rule to enable ipv4-udp RSS:
 .. code-block:: console
 
    testpmd> flow create 0 ingress pattern end actions rss types ipv4-udp end / end
+
+Secondary Process Support
+-------------------------
+
+Control plane operations are currently not supported in secondary processes.
diff --git a/drivers/net/intel/e1000/igc_ethdev.c b/drivers/net/intel/e1000/igc_ethdev.c
index 136f5af2a0..e712cfcf7c 100644
--- a/drivers/net/intel/e1000/igc_ethdev.c
+++ b/drivers/net/intel/e1000/igc_ethdev.c
@@ -12,6 +12,7 @@
 #include <ethdev_pci.h>
 #include <rte_malloc.h>
 #include <rte_alarm.h>
+#include <rte_errno.h>
 
 #include "e1000_logs.h"
 #include "igc_txrx.h"
@@ -397,6 +398,14 @@ eth_igc_set_link_up(struct rte_eth_dev *dev)
 {
 	struct e1000_hw *hw = IGC_DEV_PRIVATE_HW(dev);
 
+	/*
+	 * This function calls into the base driver, which in turn will use
+	 * function pointers, which are not guaranteed to be valid in secondary
+	 * processes, so avoid using this function in secondary processes.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -E_RTE_SECONDARY;
+
 	if (hw->phy.media_type == e1000_media_type_copper)
 		e1000_power_up_phy(hw);
 	else
@@ -409,6 +418,14 @@ eth_igc_set_link_down(struct rte_eth_dev *dev)
 {
 	struct e1000_hw *hw = IGC_DEV_PRIVATE_HW(dev);
 
+	/*
+	 * This function calls into the base driver, which in turn will use
+	 * function pointers, which are not guaranteed to be valid in secondary
+	 * processes, so avoid using this function in secondary processes.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -E_RTE_SECONDARY;
+
 	if (hw->phy.media_type == e1000_media_type_copper)
 		e1000_power_down_phy(hw);
 	else
@@ -482,6 +499,14 @@ eth_igc_link_update(struct rte_eth_dev *dev, int wait_to_complete)
 	struct rte_eth_link link;
 	int link_check, count;
 
+	/*
+	 * This function calls into the base driver, which in turn will use
+	 * function pointers, which are not guaranteed to be valid in secondary
+	 * processes, so avoid using this function in secondary processes.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -E_RTE_SECONDARY;
+
 	link_check = 0;
 	hw->mac.get_link_status = 1;
 
@@ -659,6 +684,14 @@ eth_igc_stop(struct rte_eth_dev *dev)
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 	struct rte_eth_link link;
 
+	/*
+	 * This function calls into the base driver, which in turn will use
+	 * function pointers, which are not guaranteed to be valid in secondary
+	 * processes, so avoid using this function in secondary processes.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -E_RTE_SECONDARY;
+
 	dev->data->dev_started = 0;
 	adapter->stopped = 1;
 
@@ -970,6 +1003,14 @@ eth_igc_start(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
+	/*
+	 * This function calls into the base driver, which in turn will use
+	 * function pointers, which are not guaranteed to be valid in secondary
+	 * processes, so avoid using this function in secondary processes.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -E_RTE_SECONDARY;
+
 	/* disable all MSI-X interrupts */
 	E1000_WRITE_REG(hw, E1000_EIMC, 0x1f);
 	E1000_WRITE_FLUSH(hw);
@@ -1553,6 +1594,14 @@ eth_igc_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
 	struct e1000_fw_version fw;
 	int ret;
 
+	/*
+	 * This function calls into the base driver, which in turn will use
+	 * function pointers, which are not guaranteed to be valid in secondary
+	 * processes, so avoid using this function in secondary processes.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -E_RTE_SECONDARY;
+
 	e1000_get_fw_version(hw, &fw);
 
 	/* if option rom is valid, display its version too */
@@ -1643,6 +1692,14 @@ eth_igc_led_on(struct rte_eth_dev *dev)
 {
 	struct e1000_hw *hw = IGC_DEV_PRIVATE_HW(dev);
 
+	/*
+	 * This function calls into the base driver, which in turn will use
+	 * function pointers, which are not guaranteed to be valid in secondary
+	 * processes, so avoid using this function in secondary processes.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -E_RTE_SECONDARY;
+
 	return e1000_led_on(hw) == E1000_SUCCESS ? 0 : -ENOTSUP;
 }
 
@@ -1651,6 +1708,14 @@ eth_igc_led_off(struct rte_eth_dev *dev)
 {
 	struct e1000_hw *hw = IGC_DEV_PRIVATE_HW(dev);
 
+	/*
+	 * This function calls into the base driver, which in turn will use
+	 * function pointers, which are not guaranteed to be valid in secondary
+	 * processes, so avoid using this function in secondary processes.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -E_RTE_SECONDARY;
+
 	return e1000_led_off(hw) == E1000_SUCCESS ? 0 : -ENOTSUP;
 }
 
@@ -2194,6 +2259,10 @@ eth_igc_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id)
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 	uint32_t vec = IGC_MISC_VEC_ID;
 
+	/* device interrupts are only subscribed to in primary processes */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -E_RTE_SECONDARY;
+
 	if (rte_intr_allow_others(intr_handle))
 		vec = IGC_RX_VEC_START;
 
@@ -2213,6 +2282,10 @@ eth_igc_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id)
 	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 	uint32_t vec = IGC_MISC_VEC_ID;
 
+	/* device interrupts are only subscribed to in primary processes */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -E_RTE_SECONDARY;
+
 	if (rte_intr_allow_others(intr_handle))
 		vec = IGC_RX_VEC_START;
 
@@ -2276,6 +2349,14 @@ eth_igc_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf)
 	uint32_t rctl;
 	int err;
 
+	/*
+	 * This function calls into the base driver, which in turn will use
+	 * function pointers, which are not guaranteed to be valid in secondary
+	 * processes, so avoid using this function in secondary processes.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -E_RTE_SECONDARY;
+
 	if (fc_conf->autoneg != hw->mac.autoneg)
 		return -ENOTSUP;
 
@@ -2777,6 +2858,14 @@ eth_igc_timesync_read_rx_timestamp(__rte_unused struct rte_eth_dev *dev,
 	struct igc_rx_queue *rxq;
 	uint64_t rx_timestamp;
 
+	/*
+	 * This function calls into the base driver, which in turn will use
+	 * function pointers, which are not guaranteed to be valid in secondary
+	 * processes, so avoid using this function in secondary processes.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -E_RTE_SECONDARY;
+
 	/* Get current link speed. */
 	eth_igc_link_update(dev, 1);
 	rte_eth_linkstatus_get(dev, &link);
@@ -2813,6 +2902,14 @@ eth_igc_timesync_read_tx_timestamp(struct rte_eth_dev *dev,
 	uint64_t tx_timestamp;
 	int adjust = 0;
 
+	/*
+	 * This function calls into the base driver, which in turn will use
+	 * function pointers, which are not guaranteed to be valid in secondary
+	 * processes, so avoid using this function in secondary processes.
+	 */
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -E_RTE_SECONDARY;
+
 	val = E1000_READ_REG(hw, E1000_TSYNCTXCTL);
 	if (!(val & E1000_TSYNCTXCTL_VALID))
 		return -EINVAL;
-- 
2.43.5


  parent reply	other threads:[~2025-02-17 13:54 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-12 16:19 [PATCH v1 1/4] " Anatoly Burakov
2024-12-12 16:19 ` [PATCH v1 2/4] net/igb: " Anatoly Burakov
2024-12-12 16:19 ` [PATCH v1 3/4] net/igc: " Anatoly Burakov
2024-12-12 16:19 ` [PATCH v1 4/4] net/ixgbe: " Anatoly Burakov
2024-12-12 18:02 ` [PATCH v1 1/4] net/e1000: " Stephen Hemminger
2024-12-13  9:09   ` Burakov, Anatoly
2025-02-17 13:54 ` [PATCH v2 " Anatoly Burakov
2025-02-17 13:54   ` [PATCH v2 2/4] " Anatoly Burakov
2025-02-17 13:54   ` Anatoly Burakov [this message]
2025-02-17 13:54   ` [PATCH v2 4/4] net/ixgbe: " Anatoly Burakov
2025-02-17 17:28     ` Medvedkin, Vladimir
2025-02-17 15:28   ` [PATCH v2 1/4] net/e1000: " Bruce Richardson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=421d8b0458f2ab9912413705247cef7b1c593ff4.1739800426.git.anatoly.burakov@intel.com \
    --to=anatoly.burakov@intel.com \
    --cc=alvinx.zhang@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).