DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jeff Daly <jeffd@silicom-usa.com>
To: dev@dpdk.org
Cc: Stephen Douthit <stephend@silicom-usa.com>,
	Qiming Yang <qiming.yang@intel.com>,
	Wenjun Wu <wenjun1.wu@intel.com>
Subject: [PATCH 3/3] ixgbe: make hotplug detection aware of changed SFPs
Date: Thu, 19 May 2022 15:25:15 -0400	[thread overview]
Message-ID: <67248a6302d70bbba39d9a4ba40d171b9e4cd220.1652987887.git.jeffd@silicom-usa.com> (raw)
In-Reply-To: <cover.1652987887.git.jeffd@silicom-usa.com>

SFP hotplug may require setup/configuration of the SFP in the event that
the SFP is different from the previously installed SFP.  Ensure that the
periodic service task can handle this situation.  Teach the interrupt
path that an SFP removal means that an SFP insertion will need to be
setup again.

Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
Suggested-by: Stephen Douthit <stephend@silicom-usa.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 272 ++++++++++++++++++++++++++++++-
 drivers/net/ixgbe/ixgbe_ethdev.h |   8 +
 2 files changed, 272 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index f1694ef76b14..e1d66fecd8d5 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -135,6 +135,8 @@ static const char * const ixgbevf_valid_arguments[] = {
 	NULL
 };
 
+static s32 eth_ixgbe_check_mac_link_generic(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
+					    bool *link_up, bool link_up_wait_to_complete);
 static int eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params);
 static int eth_ixgbe_dev_uninit(struct rte_eth_dev *eth_dev);
 static int ixgbe_fdir_filter_init(struct rte_eth_dev *eth_dev);
@@ -764,6 +766,33 @@ static const struct rte_ixgbe_xstats_name_off rte_ixgbevf_stats_strings[] = {
 #define IXGBEVF_NB_XSTATS (sizeof(rte_ixgbevf_stats_strings) /	\
 		sizeof(rte_ixgbevf_stats_strings[0]))
 
+/**
+ * This function is the same as ixgbe_need_crosstalk_fix() in base/ixgbe_common.c
+ *
+ * ixgbe_need_crosstalk_fix - Determine if we need to do cross talk fix
+ * @hw: pointer to hardware structure
+ *
+ * Contains the logic to identify if we need to verify link for the
+ * crosstalk fix
+ **/
+static bool ixgbe_need_crosstalk_fix(struct ixgbe_hw *hw)
+{
+	/* Does FW say we need the fix */
+	if (!hw->need_crosstalk_fix)
+		return false;
+
+	/* Only consider SFP+ PHYs i.e. media type fiber */
+	switch (ixgbe_get_media_type(hw)) {
+	case ixgbe_media_type_fiber:
+	case ixgbe_media_type_fiber_qsfp:
+		break;
+	default:
+	return false;
+}
+
+return true;
+}
+
 /*
  * This function is the same as ixgbe_is_sfp() in base/ixgbe.h.
  */
@@ -1051,6 +1080,7 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 		IXGBE_DEV_PRIVATE_TO_FILTER_INFO(eth_dev->data->dev_private);
 	struct ixgbe_bw_conf *bw_conf =
 		IXGBE_DEV_PRIVATE_TO_BW_CONF(eth_dev->data->dev_private);
+	struct ixgbe_mac_info *mac = &hw->mac;
 	uint32_t ctrl_ext;
 	uint16_t csum;
 	int diag, i, ret;
@@ -1123,6 +1153,18 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev, void *init_params __rte_unused)
 	/* pick up the PCI bus settings for reporting later */
 	ixgbe_get_bus_info(hw);
 
+	/* override mac_link_check to check for sfp cage full/empty */
+	switch (hw->mac.type) {
+	case ixgbe_mac_X550EM_x:
+	case ixgbe_mac_X550EM_a:
+	case ixgbe_mac_82599EB:
+		mac->ops.check_link = eth_ixgbe_check_mac_link_generic;
+		break;
+	default:
+		break;
+	}
+
+
 	/* Unlock any pending hardware semaphore */
 	ixgbe_swfw_lock_reset(hw);
 
@@ -2386,6 +2428,8 @@ ixgbe_dev_configure(struct rte_eth_dev *dev)
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	struct ixgbe_adapter *adapter = dev->data->dev_private;
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	int ret;
 
 	PMD_INIT_FUNC_TRACE();
@@ -2404,6 +2448,10 @@ ixgbe_dev_configure(struct rte_eth_dev *dev)
 	/* set flag to update link status after init */
 	intr->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
 
+	/* set flag to setup SFP after init */
+	if (ixgbe_is_sfp(hw))
+		intr->flags |= IXGBE_FLAG_NEED_SFP_SETUP;
+
 	/*
 	 * Initialize to TRUE. If any of Rx queues doesn't meet the bulk
 	 * allocation or vector Rx preconditions we will reset it.
@@ -2423,13 +2471,24 @@ ixgbe_dev_phy_intr_setup(struct rte_eth_dev *dev)
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	uint32_t gpie;
 
-	/* only set up it on X550EM_X */
+	/* only set up it on X550EM_X (external PHY interrupt)
+	 * or on X550EM_a_* for SFP_PRSNT# de-assertion (SFP removal)
+	 */
 	if (hw->mac.type == ixgbe_mac_X550EM_x) {
 		gpie = IXGBE_READ_REG(hw, IXGBE_GPIE);
 		gpie |= IXGBE_SDP0_GPIEN_X550EM_x;
 		IXGBE_WRITE_REG(hw, IXGBE_GPIE, gpie);
 		if (hw->phy.type == ixgbe_phy_x550em_ext_t)
 			intr->mask |= IXGBE_EICR_GPI_SDP0_X550EM_x;
+	} else if (hw->mac.type == ixgbe_mac_X550EM_a) {
+		gpie = IXGBE_READ_REG(hw, IXGBE_GPIE);
+		gpie |= IXGBE_SDP0_GPIEN_X550EM_a;
+		IXGBE_WRITE_REG(hw, IXGBE_GPIE, gpie);
+		intr->mask |= IXGBE_EICR_GPI_SDP0_X550EM_a;
+	} else {
+		PMD_DRV_LOG(DEBUG,
+			    "No PHY/SFP interrupt for MAC %d, PHY %d\n",
+			    hw->mac.type, hw->phy.type);
 	}
 }
 
@@ -2692,9 +2751,9 @@ ixgbe_dev_start(struct rte_eth_dev *dev)
 		}
 	}
 
-	if (ixgbe_is_sfp(hw) && hw->phy.multispeed_fiber) {
+	if (ixgbe_is_sfp(hw)) {
 		err = hw->mac.ops.setup_sfp(hw);
-		intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
+		intr->flags |= IXGBE_FLAG_NEED_SFP_SETUP;
 		err = rte_ctrl_thread_create(&ad->link_thread_tid,
 					     "ixgbe-service-tid",
 					     NULL,
@@ -4131,6 +4190,181 @@ ixgbevf_check_link(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
 	return ret_val;
 }
 
+/**
+ * ixgbe_check_sfp_cage - Find present status of SFP module
+ * @hw: pointer to hardware structure
+ *
+ * Find if a SFP module is present and if this device supports SFPs
+ **/
+enum ixgbe_sfp_cage_status ixgbe_check_sfp_cage(struct ixgbe_hw *hw)
+{
+	enum ixgbe_sfp_cage_status sfp_cage_status;
+
+	/* If we're not a fiber/fiber_qsfp, no cage to check */
+	switch (hw->mac.ops.get_media_type(hw)) {
+	case ixgbe_media_type_fiber:
+	case ixgbe_media_type_fiber_qsfp:
+		break;
+	default:
+		return IXGBE_SFP_CAGE_NOCAGE;
+	}
+
+	switch (hw->mac.type) {
+	case ixgbe_mac_82599EB:
+		sfp_cage_status = !!(IXGBE_READ_REG(hw, IXGBE_ESDP) &
+			     IXGBE_ESDP_SDP2);
+		break;
+	case ixgbe_mac_X550EM_x:
+	case ixgbe_mac_X550EM_a:
+		/* SDP0 is the active low signal PRSNT#, so invert this */
+		sfp_cage_status = !(IXGBE_READ_REG(hw, IXGBE_ESDP) &
+				  IXGBE_ESDP_SDP0);
+		break;
+	default:
+		/* Don't know how to check this device type yet */
+		sfp_cage_status = IXGBE_SFP_CAGE_UNKNOWN;
+		DEBUGOUT("IXGBE_SFP_CAGE_UNKNOWN, unknown mac type %d\n",
+			 hw->mac.type);
+		break;
+	}
+
+	DEBUGOUT("sfp status %d for mac type %d\n", sfp_cage_status, hw->mac.type);
+	return sfp_cage_status;
+}
+
+static s32
+ixgbe_sfp_id_and_setup(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	enum ixgbe_sfp_cage_status sfp_cage_status;
+	s32 err;
+
+	/* Can't ID or setup SFP if it's not plugged in */
+	sfp_cage_status = ixgbe_check_sfp_cage(hw);
+	if (sfp_cage_status == IXGBE_SFP_CAGE_EMPTY ||
+	    sfp_cage_status == IXGBE_SFP_CAGE_NOCAGE)
+		return IXGBE_ERR_SFP_NOT_PRESENT;
+
+	/* Something's in the cage, ID it */
+	hw->phy.ops.identify_sfp(hw);
+
+	/* Unknown module type, give up */
+	if (hw->phy.sfp_type == ixgbe_sfp_type_unknown) {
+		PMD_DRV_LOG(ERR, "unknown SFP type, giving up");
+		return IXGBE_ERR_SFP_NOT_SUPPORTED;
+	}
+
+	/* This should be a redundant check, since we looked at the
+	 * PRSNT# signal from the cage above, but just in case this is
+	 * an SFP that's slow to respond to I2C pokes correctly, try it
+	 * again later
+	 */
+	if (hw->phy.sfp_type == ixgbe_sfp_type_not_present) {
+		PMD_DRV_LOG(ERR, "IDed SFP as absent but cage PRSNT# active!?");
+		return IXGBE_ERR_SFP_NOT_PRESENT;
+	}
+
+	/* SFP is present and identified, try to set it up */
+	err = hw->mac.ops.setup_sfp(hw);
+	if (err)
+		PMD_DRV_LOG(ERR, "setup_sfp() failed %d", err);
+
+	return err;
+}
+
+static void
+ixgbe_sfp_service(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+	enum ixgbe_sfp_cage_status sfp_cage_status;
+	s32 err;
+	u8 sff_id;
+	bool have_int = false;
+
+	/* If there's no module cage, then there's nothing to service */
+	sfp_cage_status = ixgbe_check_sfp_cage(hw);
+	if (sfp_cage_status == IXGBE_SFP_CAGE_NOCAGE) {
+		PMD_DRV_LOG(DEBUG, "No SFP to service\n");
+		return;
+	}
+
+	/* Even for platforms where ixgbe_check_sfp_cage() gives a clear
+	 * status result, if there's no interrupts, or no interrupt for the SFP
+	 * cage present pin, even if other interrupts exist, then we still need
+	 * to poll here to set the flag.
+	 */
+#ifndef RTE_EXEC_ENV_FREEBSD
+	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
+	struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
+	if (rte_intr_allow_others(intr_handle)) {
+		/* check if lsc interrupt is enabled */
+		if (dev->data->dev_conf.intr_conf.lsc)
+			have_int = true;
+	}
+#endif /* #ifdef RTE_EXEC_ENV_FREEBSD */
+
+	if (!have_int && sfp_cage_status == IXGBE_SFP_CAGE_EMPTY) {
+		intr->flags |= IXGBE_FLAG_NEED_SFP_SETUP;
+		PMD_DRV_LOG(DEBUG, "No SFP, no LSC, set NEED_SFP_SETUP\n");
+	}
+
+	/* For platforms that don't have a way to read the PRESENT# signal from
+	 * the SFP cage, fallback to doing an I2C read and seeing if it's ACKed
+	 * to determine if a module is present
+	 */
+	if (sfp_cage_status == IXGBE_SFP_CAGE_UNKNOWN) {
+		PMD_DRV_LOG(DEBUG,
+			    "SFP present unknown (int? %d), try I2C read\n",
+			    have_int);
+
+		/* Rather than calling identify_sfp, which will read a lot of I2C
+		 * registers (and in a slow processor intensive fashion due to
+		 * bit-banging, just read the SFF ID register, which is at a
+		 * common address across SFP/SFP+/QSFP modules and see if
+		 * there's a NACK.  This works since we only expect a NACK if no
+		 * module is present
+		 */
+		err = ixgbe_read_i2c_eeprom(hw, IXGBE_SFF_IDENTIFIER, &sff_id);
+		if (err != IXGBE_SUCCESS) {
+			PMD_DRV_LOG(DEBUG, "Received I2C NAK from SFP, set NEED_SFP_SETUP flag\n");
+			intr->flags |= IXGBE_FLAG_NEED_SFP_SETUP;
+			sfp_cage_status = IXGBE_SFP_CAGE_EMPTY;
+		} else {
+			PMD_DRV_LOG(DEBUG, "SFP ID read ACKed");
+			sfp_cage_status = IXGBE_SFP_CAGE_FULL;
+		}
+	}
+
+	if (sfp_cage_status == IXGBE_SFP_CAGE_EMPTY) {
+		PMD_DRV_LOG(DEBUG, "SFP absent, cage_status %d\n", sfp_cage_status);
+		return;
+	}
+
+	/* No setup requested?	Nothing to do */
+	if (!(intr->flags & IXGBE_FLAG_NEED_SFP_SETUP))
+		return;
+
+	err = ixgbe_sfp_id_and_setup(dev);
+	if (err) {
+		PMD_DRV_LOG(DEBUG, "failed to ID & setup SFP %d", err);
+		return;
+	}
+
+	/* Setup is done, clear the flag, but make sure link config runs for new SFP */
+	intr->flags &= ~IXGBE_FLAG_NEED_SFP_SETUP;
+	intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
+
+	/*
+	 * Since this is a new SFP, clear the old advertised speed mask so we don't
+	 * end up using an old slower rate
+	 */
+	hw->phy.autoneg_advertised = 0;
+}
+
 static void
 ixgbe_link_service(struct rte_eth_dev *dev)
 {
@@ -4189,13 +4423,13 @@ ixgbe_dev_setup_link_thread_handler(void *param)
 {
 	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	u32 speed, start, ticks, service_ms;
+	uint64_t start, ticks, service_ms;
+	uint32_t speed;
 	s32 err;
 	bool link_up  = false;
 
-	pthread_detach(pthread_self());
-
 	while (1) {
+		ixgbe_sfp_service(dev);
 		ixgbe_link_service(dev);
 
 		if (!ixgbe_dev_link_update(dev, 0)) {
@@ -4227,6 +4461,25 @@ ixgbe_dev_setup_link_thread_handler(void *param)
 	return NULL;
 }
 
+static s32
+eth_ixgbe_check_mac_link_generic(struct ixgbe_hw *hw, ixgbe_link_speed *speed,
+				 bool *link_up, bool link_up_wait_to_complete)
+{
+	if (ixgbe_need_crosstalk_fix(hw)) {
+		enum ixgbe_sfp_cage_status sfp_cage_status;
+
+		sfp_cage_status = ixgbe_check_sfp_cage(hw);
+		if (sfp_cage_status != IXGBE_SFP_CAGE_FULL) {
+			*link_up = false;
+			*speed = IXGBE_LINK_SPEED_UNKNOWN;
+			return IXGBE_SUCCESS;
+		}
+	}
+
+	return ixgbe_check_mac_link_generic(hw, speed, link_up, link_up_wait_to_complete);
+}
+
+
 /*
  * In freebsd environment, nic_uio drivers do not support interrupts,
  * rte_intr_callback_register() will fail to register interrupts.
@@ -4513,8 +4766,6 @@ ixgbe_dev_interrupt_get_status(struct rte_eth_dev *dev)
 	eicr = IXGBE_READ_REG(hw, IXGBE_EICR);
 	PMD_DRV_LOG(DEBUG, "eicr %x", eicr);
 
-	intr->flags = 0;
-
 	/* set flag for async link update */
 	if (eicr & IXGBE_EICR_LSC)
 		intr->flags |= IXGBE_FLAG_NEED_LINK_UPDATE;
@@ -4530,6 +4781,11 @@ ixgbe_dev_interrupt_get_status(struct rte_eth_dev *dev)
 	    (eicr & IXGBE_EICR_GPI_SDP0_X550EM_x))
 		intr->flags |= IXGBE_FLAG_PHY_INTERRUPT;
 
+	/* Check for loss of SFP */
+	if (hw->mac.type ==  ixgbe_mac_X550EM_a &&
+	    (eicr & IXGBE_EICR_GPI_SDP0_X550EM_a))
+		intr->flags |= IXGBE_FLAG_NEED_SFP_SETUP;
+
 	return 0;
 }
 
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index f61706c9eae6..2037cdaa621e 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -517,6 +517,14 @@ struct ixgbe_vf_representor {
 	struct rte_eth_dev *pf_ethdev;
 };
 
+enum ixgbe_sfp_cage_status {
+	IXGBE_SFP_CAGE_EMPTY = 0,
+	IXGBE_SFP_CAGE_FULL,
+	IXGBE_SFP_CAGE_UNKNOWN = -1,
+	IXGBE_SFP_CAGE_NOCAGE = -2,
+};
+enum ixgbe_sfp_cage_status ixgbe_check_sfp_cage(struct ixgbe_hw *hw);
+
 int ixgbe_vf_representor_init(struct rte_eth_dev *ethdev, void *init_params);
 int ixgbe_vf_representor_uninit(struct rte_eth_dev *ethdev);
 
-- 
2.25.1


  parent reply	other threads:[~2022-05-19 19:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-19 19:25 [PATCH 0/3] ixgbe: fix SFP hotplug detection Jeff Daly
2022-05-19 19:25 ` [PATCH 1/3] ixgbe: make link update thread periodic Jeff Daly
2022-05-19 19:25 ` [PATCH 2/3] ixgbe: move periodic link service work into separate function Jeff Daly
2022-05-19 19:25 ` Jeff Daly [this message]
  -- strict thread matches above, loose matches on Subject: below --
2022-05-19 18:02 [PATCH 0/3] ixgbe: Fix SFP hotplug detection Jeff Daly
2022-05-19 18:02 ` [PATCH 3/3] ixgbe: make hotplug detection aware of changed SFPs Jeff Daly
2022-05-19 17:43 [PATCH 0/3] ixgbe: Fix SFP hotplug detection/removal Jeff Daly
2022-05-19 17:43 ` [PATCH 3/3] ixgbe: make hotplug detection aware of changed SFPs Jeff Daly

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=67248a6302d70bbba39d9a4ba40d171b9e4cd220.1652987887.git.jeffd@silicom-usa.com \
    --to=jeffd@silicom-usa.com \
    --cc=dev@dpdk.org \
    --cc=qiming.yang@intel.com \
    --cc=stephend@silicom-usa.com \
    --cc=wenjun1.wu@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).