Currently, the architecture of the 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 these
functions from being executed in e1000 driver.

Fixes: 805803445a02 ("e1000: support EM devices (also known as e1000/e1000e)")
Cc: sta...@dpdk.org

Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com>
---
 doc/guides/nics/e1000em.rst   | 18 ++++++++
 drivers/net/e1000/em_ethdev.c | 80 +++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+)

diff --git a/doc/guides/nics/e1000em.rst b/doc/guides/nics/e1000em.rst
index 5e752a29e5..95ff04451a 100644
--- a/doc/guides/nics/e1000em.rst
+++ b/doc/guides/nics/e1000em.rst
@@ -153,3 +153,21 @@ The following are known limitations:
 #.  Qemu e1000 only supports one interrupt source, so link and Rx interrupt 
should be exclusive.
 
 #.  Qemu e1000 does not support interrupt auto-clear, application should 
disable interrupt immediately when woken up.
+
+Secondary Process Support
+-------------------------
+
+The following ethdev API's are currently not supported for use in secondary 
processes:
+
+* ``rte_eth_dev_start``
+* ``rte_eth_dev_stop``
+* ``rte_eth_dev_rx_intr_enable``
+* ``rte_eth_dev_rx_intr_disable``
+* ``rte_eth_link_get``
+* ``rte_eth_dev_led_on``
+* ``rte_eth_dev_led_off``
+* ``rte_eth_dev_flow_ctrl_set``
+* ``rte_eth_dev_default_mac_addr_set``
+* ``rte_eth_dev_mac_addr_add``
+* ``rte_eth_dev_mac_addr_remove``
+* ``rte_eth_dev_set_mc_addr_list``
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index f6875b0762..e26930f1d6 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -543,6 +543,14 @@ eth_em_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;
+
        ret = eth_em_stop(dev);
        if (ret != 0)
                return ret;
@@ -727,6 +735,14 @@ eth_em_stop(struct rte_eth_dev *dev)
        struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
        struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 
+       /*
+        * 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;
 
        eth_em_rxtx_control(dev, false);
@@ -1016,6 +1032,10 @@ eth_em_rx_queue_intr_enable(struct rte_eth_dev *dev, 
__rte_unused uint16_t queue
        struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
        struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
 
+       /* device interrupts are only subscribed to in primary processes */
+       if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+               return -E_RTE_SECONDARY;
+
        em_rxq_intr_enable(hw);
        rte_intr_ack(intr_handle);
 
@@ -1027,6 +1047,10 @@ eth_em_rx_queue_intr_disable(struct rte_eth_dev *dev, 
__rte_unused uint16_t queu
 {
        struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
+       /* device interrupts are only subscribed to in primary processes */
+       if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+               return -E_RTE_SECONDARY;
+
        em_rxq_intr_disable(hw);
 
        return 0;
@@ -1654,6 +1678,14 @@ eth_em_led_on(struct rte_eth_dev *dev)
 {
        struct e1000_hw *hw;
 
+       /*
+        * 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;
+
        hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
        return e1000_led_on(hw) == E1000_SUCCESS ? 0 : -ENOTSUP;
 }
@@ -1663,6 +1695,14 @@ eth_em_led_off(struct rte_eth_dev *dev)
 {
        struct e1000_hw *hw;
 
+       /*
+        * 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;
+
        hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
        return e1000_led_off(hw) == E1000_SUCCESS ? 0 : -ENOTSUP;
 }
@@ -1724,6 +1764,14 @@ eth_em_flow_ctrl_set(struct rte_eth_dev *dev, struct 
rte_eth_fc_conf *fc_conf)
        uint32_t max_high_water;
        uint32_t rctl;
 
+       /*
+        * 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;
+
        hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
        if (fc_conf->autoneg != hw->mac.autoneg)
                return -ENOTSUP;
@@ -1775,6 +1823,14 @@ eth_em_rar_set(struct rte_eth_dev *dev, struct 
rte_ether_addr *mac_addr,
 {
        struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
+       /*
+        * 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_rar_set(hw, mac_addr->addr_bytes, index);
 }
 
@@ -1784,6 +1840,14 @@ eth_em_rar_clear(struct rte_eth_dev *dev, uint32_t index)
        uint8_t addr[RTE_ETHER_ADDR_LEN];
        struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
+       /*
+        * 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;
+
        memset(addr, 0, sizeof(addr));
 
        e1000_rar_set(hw, addr, index);
@@ -1793,6 +1857,14 @@ static int
 eth_em_default_mac_addr_set(struct rte_eth_dev *dev,
                            struct rte_ether_addr *addr)
 {
+       /*
+        * 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;
+
        eth_em_rar_clear(dev, 0);
 
        return eth_em_rar_set(dev, (void *)addr, 0, 0);
@@ -1837,6 +1909,14 @@ eth_em_set_mc_addr_list(struct rte_eth_dev *dev,
 {
        struct e1000_hw *hw;
 
+       /*
+        * 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;
+
        hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
        e1000_update_mc_addr_list(hw, (u8 *)mc_addr_set, nb_mc_addr);
        return 0;
-- 
2.43.5

Reply via email to