From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7B24945E87; Thu, 12 Dec 2024 17:19:26 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 88F3D402EE; Thu, 12 Dec 2024 17:19:22 +0100 (CET) Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.9]) by mails.dpdk.org (Postfix) with ESMTP id 0B87740269 for ; Thu, 12 Dec 2024 17:19:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1734020360; x=1765556360; h=from:to:subject:date:message-id:in-reply-to:references: mime-version:content-transfer-encoding; bh=X5u1DS6MUxB3XFOcqJysQEcYoaExsv/XbBiHZn9R/nw=; b=KzZw4dEVc95WGH4PvYr6StT7fCjcVvlT9i58BFt11G5Cc+lZ9pdXnlT7 bcjtSLpWSxz6yCw6zH7BzBxz/RqfTH2uGvt+SpWLxL+OB+V3moX68+XoY +JTr/Sq8vllzgHJOaB9PwYQEv9WXTO8YH1in5/0VpVj5t1bzyBc2uh10H ijHDZdEGrGbqok3iXFpCKMvFigvFcLpdr17IJg2jLOio1ief419X91ZZq +C8yqKG9pwCjR9bLrNeQt9rzHajRJGiruf++v8DiMdlXGgN8WSWl2BDaA 9dL8ASXU261/FfpqMBGEsSw7oK/HPIcp5XuP/6pNJlItn04dUj1lr8gSr A==; X-CSE-ConnectionGUID: CkAklOcJQN+HARTBqF4Yig== X-CSE-MsgGUID: esMdOHlhTXmJ+17Q8WgJ3w== X-IronPort-AV: E=McAfee;i="6700,10204,11284"; a="45135271" X-IronPort-AV: E=Sophos;i="6.12,229,1728975600"; d="scan'208";a="45135271" Received: from fmviesa001.fm.intel.com ([10.60.135.141]) by fmvoesa103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 Dec 2024 08:19:10 -0800 X-CSE-ConnectionGUID: tstVzCHkQR+DD9+rTJ+XMw== X-CSE-MsgGUID: QQBR5/SZTKyMb41bXholjA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.12,224,1728975600"; d="scan'208";a="127269220" Received: from unknown (HELO silpixa00401119.ir.intel.com) ([10.55.129.167]) by fmviesa001.fm.intel.com with ESMTP; 12 Dec 2024 08:19:09 -0800 From: Anatoly Burakov To: dev@dpdk.org Subject: [PATCH v1 2/4] net/igb: prevent crashes in secondary processes Date: Thu, 12 Dec 2024 16:19:04 +0000 Message-ID: <71b556aa1eb4e272c375e65e2f7255760fde3e44.1734020337.git.anatoly.burakov@intel.com> X-Mailer: git-send-email 2.43.5 In-Reply-To: <3c323577ce36cf4425d2c2def85d0d6644b87dc8.1734020337.git.anatoly.burakov@intel.com> References: <3c323577ce36cf4425d2c2def85d0d6644b87dc8.1734020337.git.anatoly.burakov@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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 igb driver. Fixes: af75078fece3 ("first public release") Cc: stable@dpdk.org Signed-off-by: Anatoly Burakov --- doc/guides/nics/igb.rst | 47 +++++++++ drivers/net/e1000/igb_ethdev.c | 176 +++++++++++++++++++++++++++++++++ 2 files changed, 223 insertions(+) diff --git a/doc/guides/nics/igb.rst b/doc/guides/nics/igb.rst index e3a91c316b..fbb3730354 100644 --- a/doc/guides/nics/igb.rst +++ b/doc/guides/nics/igb.rst @@ -31,3 +31,50 @@ Features of the IGB PMD are: * Checksum offload * TCP segmentation offload * Jumbo frames supported + +Secondary Process Support +------------------------- + +IGB Physical Function Driver +~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +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_set_link_up`` +* ``rte_eth_dev_set_link_down`` +* ``rte_eth_dev_rx_intr_enable`` +* ``rte_eth_dev_rx_intr_disable`` +* ``rte_eth_link_get`` +* ``rte_eth_dev_fw_version_get`` +* ``rte_eth_dev_rx_intr_enable`` +* ``rte_eth_dev_rx_intr_disable`` +* ``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`` +* ``rte_eth_dev_get_eeprom`` +* ``rte_eth_dev_set_eeprom`` + +IGB Virtual Function Driver +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The following ethdev API's are currently not supported for use in secondary processes: + +* ``rte_eth_dev_start`` +* ``rte_eth_dev_stop`` +* ``rte_eth_promiscuous_enable`` +* ``rte_eth_promiscuous_disable`` +* ``rte_eth_allmulticast_enable`` +* ``rte_eth_allmulticast_disable`` +* ``rte_eth_dev_set_link_up`` +* ``rte_eth_dev_set_link_down`` +* ``rte_eth_link_get`` +* ``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/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index c695f44c4c..1d966d67eb 100644 --- a/drivers/net/e1000/igb_ethdev.c +++ b/drivers/net/e1000/igb_ethdev.c @@ -1247,6 +1247,14 @@ eth_igb_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 uio/vfio intr/eventfd mapping */ rte_intr_disable(intr_handle); @@ -1470,6 +1478,14 @@ eth_igb_stop(struct rte_eth_dev *dev) struct e1000_adapter *adapter = E1000_DEV_PRIVATE(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; + if (adapter->stopped) return 0; @@ -1523,6 +1539,14 @@ eth_igb_dev_set_link_up(struct rte_eth_dev *dev) { 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; + if (hw->phy.media_type == e1000_media_type_copper) e1000_power_up_phy(hw); else @@ -1536,6 +1560,14 @@ eth_igb_dev_set_link_down(struct rte_eth_dev *dev) { 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; + if (hw->phy.media_type == e1000_media_type_copper) e1000_power_down_phy(hw); else @@ -2157,6 +2189,14 @@ eth_igb_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); switch (hw->mac.type) { @@ -2405,6 +2445,14 @@ eth_igb_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; @@ -3027,6 +3075,14 @@ eth_igb_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; } @@ -3036,6 +3092,14 @@ eth_igb_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; } @@ -3098,6 +3162,14 @@ eth_igb_flow_ctrl_set(struct rte_eth_dev *dev, struct rte_eth_fc_conf *fc_conf) uint32_t rctl; uint32_t ctrl; + /* + * 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; @@ -3184,6 +3256,14 @@ eth_igb_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); uint32_t rah; + /* + * 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_rar_set(hw, mac_addr->addr_bytes, index); rah = E1000_READ_REG(hw, E1000_RAH(index)); rah |= (0x1 << (E1000_RAH_POOLSEL_SHIFT + pool)); @@ -3197,6 +3277,14 @@ eth_igb_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); @@ -3206,6 +3294,14 @@ static int eth_igb_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_igb_rar_clear(dev, 0); eth_igb_rar_set(dev, (void *)addr, 0, 0); @@ -3339,6 +3435,14 @@ igbvf_dev_start(struct rte_eth_dev *dev) int ret; uint32_t intr_vector = 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; + PMD_INIT_FUNC_TRACE(); hw->mac.ops.reset_hw(hw); @@ -3395,6 +3499,14 @@ igbvf_dev_stop(struct rte_eth_dev *dev) struct e1000_adapter *adapter = E1000_DEV_PRIVATE(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; + if (adapter->stopped) return 0; @@ -3467,6 +3579,14 @@ igbvf_promiscuous_enable(struct rte_eth_dev *dev) { 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; + /* Set both unicast and multicast promisc */ e1000_promisc_set_vf(hw, e1000_promisc_enabled); @@ -3478,6 +3598,14 @@ igbvf_promiscuous_disable(struct rte_eth_dev *dev) { 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; + /* If in allmulticast mode leave multicast promisc */ if (dev->data->all_multicast == 1) e1000_promisc_set_vf(hw, e1000_promisc_multicast); @@ -3492,6 +3620,14 @@ igbvf_allmulticast_enable(struct rte_eth_dev *dev) { 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; + /* In promiscuous mode multicast promisc already set */ if (dev->data->promiscuous == 0) e1000_promisc_set_vf(hw, e1000_promisc_multicast); @@ -3504,6 +3640,14 @@ igbvf_allmulticast_disable(struct rte_eth_dev *dev) { 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; + /* In promiscuous mode leave multicast promisc enabled */ if (dev->data->promiscuous == 0) e1000_promisc_set_vf(hw, e1000_promisc_disabled); @@ -4607,6 +4751,14 @@ eth_igb_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; @@ -5055,6 +5207,14 @@ eth_igb_get_eeprom(struct rte_eth_dev *dev, uint16_t *data = in_eeprom->data; int first, length; + /* + * 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; + first = in_eeprom->offset >> 1; length = in_eeprom->length >> 1; if ((first >= hw->nvm.word_size) || @@ -5079,6 +5239,14 @@ eth_igb_set_eeprom(struct rte_eth_dev *dev, uint16_t *data = in_eeprom->data; int first, length; + /* + * 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; + first = in_eeprom->offset >> 1; length = in_eeprom->length >> 1; if ((first >= hw->nvm.word_size) || @@ -5179,6 +5347,10 @@ eth_igb_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 = E1000_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 = E1000_RX_VEC_START; @@ -5199,6 +5371,10 @@ eth_igb_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 = E1000_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 = E1000_RX_VEC_START; -- 2.43.5