[dpdk-dev] [PATCH 0/2] support mailbox interruption on ixgbe/igb VF

[dpdk-dev] [PATCH 0/2] support mailbox interruption on ixgbe/igb VF

[Wenzhuo Lu]

This patch set addes the support of the mailbox interruption on VF.
So, VF can receice the messges for physical link down/up.

PS: This patch set is splitted from a previous patch set, *automatic
link recovery on ixgbe/igb VF*.

Wenzhuo Lu (2):
  ixgbe: VF supports mailbox interruption for PF link up/down
  igb: VF supports mailbox interruption for PF link up/down

 doc/guides/rel_notes/release_16_07.rst |   6 ++
 drivers/net/e1000/igb_ethdev.c         | 159 +++++++++++++++++++++++++++++++++
 drivers/net/ixgbe/ixgbe_ethdev.c       |  85 +++++++++++++++++-
 3 files changed, 247 insertions(+), 3 deletions(-)

-- 
1.9.3

[dpdk-dev] [PATCH 1/2] ixgbe: VF supports mailbox interruption for PF link up/down

[Wenzhuo Lu]

In this scenario, kernel PF + DPDK VF, when PF finds the link
state is changed, up -> down or down -> up, it will send a
mailbox message to VF.
This patch enables the support of the interruption of mailbox,
so VF can receive the message for link up/down.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 doc/guides/rel_notes/release_16_07.rst |  6 +++
 drivers/net/ixgbe/ixgbe_ethdev.c       | 85 ++++++++++++++++++++++++++++++++--
 2 files changed, 88 insertions(+), 3 deletions(-)

diff --git a/doc/guides/rel_notes/release_16_07.rst b/doc/guides/rel_notes/release_16_07.rst
index 83c841b..be702fd 100644
--- a/doc/guides/rel_notes/release_16_07.rst
+++ b/doc/guides/rel_notes/release_16_07.rst
@@ -34,6 +34,12 @@ This section should contain new features added in this release. Sample format:
 
   Refer to the previous release notes for examples.
 
+* **Added mailbox interruption support for ixgbe VF.**
+
+  When the link becomes down or up, PF will use mailbox message to notice
+  VF. To handle this link up/down event, add the mailbox interruption
+  support to receive the message.
+
 
 Resolved Issues
 ---------------
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index eec607c..8e5f64f 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -151,6 +151,8 @@
 #define IXGBE_VMTIR(_i) (0x00017000 + ((_i) * 4)) /* 64 of these (0-63) */
 #define IXGBE_QDE_STRIP_TAG                    0x00000004
 
+#define IXGBE_VTEICR_MASK        0x07
+
 enum ixgbevf_xcast_modes {
 	IXGBEVF_XCAST_MODE_NONE = 0,
 	IXGBEVF_XCAST_MODE_MULTI,
@@ -361,6 +363,8 @@ static int ixgbe_timesync_read_time(struct rte_eth_dev *dev,
 				   struct timespec *timestamp);
 static int ixgbe_timesync_write_time(struct rte_eth_dev *dev,
 				   const struct timespec *timestamp);
+static void ixgbevf_dev_interrupt_handler(struct rte_intr_handle *handle,
+					  void *param);
 
 static int ixgbe_dev_l2_tunnel_eth_type_conf
 	(struct rte_eth_dev *dev, struct rte_eth_l2_tunnel_conf *l2_tunnel);
@@ -1441,6 +1445,12 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 			return -EIO;
 	}
 
+	rte_intr_callback_register(&pci_dev->intr_handle,
+				   ixgbevf_dev_interrupt_handler,
+				   (void *)eth_dev);
+	rte_intr_enable(&pci_dev->intr_handle);
+	ixgbevf_intr_enable(hw);
+
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id, "ixgbe_mac_82599_vf");
@@ -1454,6 +1464,7 @@ static int
 eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
 {
 	struct ixgbe_hw *hw;
+	struct rte_pci_device *pci_dev = eth_dev->pci_dev;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -1475,6 +1486,11 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
 	rte_free(eth_dev->data->mac_addrs);
 	eth_dev->data->mac_addrs = NULL;
 
+	rte_intr_disable(&pci_dev->intr_handle);
+	rte_intr_callback_unregister(&pci_dev->intr_handle,
+				     ixgbevf_dev_interrupt_handler,
+				     (void *)eth_dev);
+
 	return 0;
 }
 
@@ -4073,6 +4089,8 @@ ixgbevf_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
+	ixgbevf_intr_disable(hw);
+
 	hw->adapter_stopped = 1;
 	ixgbe_stop_adapter(hw);
 
@@ -4796,6 +4814,9 @@ ixgbevf_configure_msix(struct rte_eth_dev *dev)
 	uint32_t q_idx;
 	uint32_t vector_idx = IXGBE_MISC_VEC_ID;
 
+	/* Configure VF other cause ivar */
+	ixgbevf_set_ivar_map(hw, -1, 1, vector_idx);
+
 	/* won't configure msix register if no mapping is done
 	 * between intr vector and event fd.
 	 */
@@ -4810,9 +4831,6 @@ ixgbevf_configure_msix(struct rte_eth_dev *dev)
 		ixgbevf_set_ivar_map(hw, 0, q_idx, vector_idx);
 		intr_handle->intr_vec[q_idx] = vector_idx;
 	}
-
-	/* Configure VF other cause ivar */
-	ixgbevf_set_ivar_map(hw, -1, 1, vector_idx);
 }
 
 /**
@@ -7131,6 +7149,67 @@ ixgbevf_dev_allmulticast_disable(struct rte_eth_dev *dev)
 	ixgbevf_update_xcast_mode(hw, IXGBEVF_XCAST_MODE_NONE);
 }
 
+static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	u32 in_msg = 0;
+
+	if (ixgbe_read_mbx(hw, &in_msg, 1, 0))
+		return;
+
+	/* PF reset VF event */
+	if (in_msg == IXGBE_PF_CONTROL_MSG)
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET);
+}
+
+static int
+ixgbevf_dev_interrupt_get_status(struct rte_eth_dev *dev)
+{
+	uint32_t eicr;
+	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);
+	ixgbevf_intr_disable(hw);
+
+	/* read-on-clear nic registers here */
+	eicr = IXGBE_READ_REG(hw, IXGBE_VTEICR);
+	intr->flags = 0;
+
+	/* only one misc vector supported - mailbox */
+	eicr &= IXGBE_VTEICR_MASK;
+	if (eicr == IXGBE_MISC_VEC_ID)
+		intr->flags |= IXGBE_FLAG_MAILBOX;
+
+	return 0;
+}
+
+static int
+ixgbevf_dev_interrupt_action(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);
+
+	if (intr->flags & IXGBE_FLAG_MAILBOX) {
+		ixgbevf_mbx_process(dev);
+		intr->flags &= ~IXGBE_FLAG_MAILBOX;
+	}
+
+	ixgbevf_intr_enable(hw);
+
+	return 0;
+}
+
+static void
+ixgbevf_dev_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
+			      void *param)
+{
+	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
+
+	ixgbevf_dev_interrupt_get_status(dev);
+	ixgbevf_dev_interrupt_action(dev);
+}
+
 static struct rte_driver rte_ixgbe_driver = {
 	.type = PMD_PDEV,
 	.init = rte_ixgbe_pmd_init,
-- 
1.9.3

[dpdk-dev] [PATCH 2/2] igb: VF supports mailbox interruption for PF link up/down

[Wenzhuo Lu]

In this scenario, kernel PF + DPDK VF, when PF finds the link
state is changed, up -> down or down -> up, it will send a
mailbox message to VF.
This patch enables the support of the interruption of mailbox,
so VF can receive the message for link up/down.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 doc/guides/rel_notes/release_16_07.rst |   2 +-
 drivers/net/e1000/igb_ethdev.c         | 159 +++++++++++++++++++++++++++++++++
 2 files changed, 160 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/release_16_07.rst b/doc/guides/rel_notes/release_16_07.rst
index be702fd..8d45915 100644
--- a/doc/guides/rel_notes/release_16_07.rst
+++ b/doc/guides/rel_notes/release_16_07.rst
@@ -34,7 +34,7 @@ This section should contain new features added in this release. Sample format:
 
   Refer to the previous release notes for examples.
 
-* **Added mailbox interruption support for ixgbe VF.**
+* **Added mailbox interruption support for ixgbe/igb VF.**
 
   When the link becomes down or up, PF will use mailbox message to notice
   VF. To handle this link up/down event, add the mailbox interruption
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index f0921ee..b0e5e6a 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -86,6 +86,12 @@
 #define E1000_INCVALUE_82576         (16 << IGB_82576_TSYNC_SHIFT)
 #define E1000_TSAUXC_DISABLE_SYSTIME 0x80000000
 
+#define E1000_VTIVAR_MISC                0x01740
+#define E1000_VTIVAR_MISC_MASK           0xFF
+#define E1000_VTIVAR_VALID               0x80
+#define E1000_VTIVAR_MISC_MAILBOX        0
+#define E1000_VTIVAR_MISC_INTR_MASK      0x3
+
 static int  eth_igb_configure(struct rte_eth_dev *dev);
 static int  eth_igb_start(struct rte_eth_dev *dev);
 static void eth_igb_stop(struct rte_eth_dev *dev);
@@ -259,6 +265,9 @@ static void eth_igb_assign_msix_vector(struct e1000_hw *hw, int8_t direction,
 static void eth_igb_write_ivar(struct e1000_hw *hw, uint8_t msix_vector,
 			       uint8_t index, uint8_t offset);
 static void eth_igb_configure_msix_intr(struct rte_eth_dev *dev);
+static void eth_igbvf_interrupt_handler(struct rte_intr_handle *handle,
+					void *param);
+static void igbvf_mbx_process(struct rte_eth_dev *dev);
 
 /*
  * Define VF Stats MACRO for Non "cleared on read" register
@@ -554,6 +563,41 @@ igb_intr_disable(struct e1000_hw *hw)
 	E1000_WRITE_FLUSH(hw);
 }
 
+static inline void
+igbvf_intr_enable(struct rte_eth_dev *dev)
+{
+	struct e1000_hw *hw =
+		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	/* only for mailbox */
+	E1000_WRITE_REG(hw, E1000_EIAM, 1 << E1000_VTIVAR_MISC_MAILBOX);
+	E1000_WRITE_REG(hw, E1000_EIAC, 1 << E1000_VTIVAR_MISC_MAILBOX);
+	E1000_WRITE_REG(hw, E1000_EIMS, 1 << E1000_VTIVAR_MISC_MAILBOX);
+	E1000_WRITE_FLUSH(hw);
+}
+
+/* only for mailbox now. If RX/TX needed, should extend this function.  */
+static void
+igbvf_set_ivar_map(struct e1000_hw *hw, uint8_t msix_vector)
+{
+	uint32_t tmp = 0;
+
+	/* mailbox */
+	tmp |= (msix_vector & E1000_VTIVAR_MISC_INTR_MASK);
+	tmp |= E1000_VTIVAR_VALID;
+	E1000_WRITE_REG(hw, E1000_VTIVAR_MISC, tmp);
+}
+
+static void
+eth_igbvf_configure_msix_intr(struct rte_eth_dev *dev)
+{
+	struct e1000_hw *hw =
+		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	/* Configure VF other cause ivar */
+	igbvf_set_ivar_map(hw, E1000_VTIVAR_MISC_MAILBOX);
+}
+
 static inline int32_t
 igb_pf_reset_hw(struct e1000_hw *hw)
 {
@@ -942,6 +986,10 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id, "igb_mac_82576_vf");
 
+	rte_intr_callback_register(&pci_dev->intr_handle,
+				   eth_igbvf_interrupt_handler,
+				   (void *)eth_dev);
+
 	return 0;
 }
 
@@ -950,6 +998,7 @@ eth_igbvf_dev_uninit(struct rte_eth_dev *eth_dev)
 {
 	struct e1000_adapter *adapter =
 		E1000_DEV_PRIVATE(eth_dev->data->dev_private);
+	struct rte_pci_device *pci_dev = eth_dev->pci_dev;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -966,6 +1015,12 @@ eth_igbvf_dev_uninit(struct rte_eth_dev *eth_dev)
 	rte_free(eth_dev->data->mac_addrs);
 	eth_dev->data->mac_addrs = NULL;
 
+	/* disable uio intr before callback unregister */
+	rte_intr_disable(&pci_dev->intr_handle);
+	rte_intr_callback_unregister(&pci_dev->intr_handle,
+				     eth_igbvf_interrupt_handler,
+				     (void *)eth_dev);
+
 	return 0;
 }
 
@@ -2564,6 +2619,69 @@ eth_igb_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
 }
 
 static int
+eth_igbvf_interrupt_get_status(struct rte_eth_dev *dev)
+{
+	uint32_t eicr;
+	struct e1000_hw *hw =
+		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct e1000_interrupt *intr =
+		E1000_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+
+	igbvf_intr_disable(hw);
+
+	/* read-on-clear nic registers here */
+	eicr = E1000_READ_REG(hw, E1000_EICR);
+	intr->flags = 0;
+
+	if (eicr == E1000_VTIVAR_MISC_MAILBOX)
+		intr->flags |= E1000_FLAG_MAILBOX;
+
+	return 0;
+}
+
+void igbvf_mbx_process(struct rte_eth_dev *dev)
+{
+	struct e1000_hw *hw =
+		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct e1000_mbx_info *mbx = &hw->mbx;
+	u32 in_msg = 0;
+
+	if (mbx->ops.read(hw, &in_msg, 1, 0))
+		return;
+
+	/* PF reset VF event */
+	if (in_msg == E1000_PF_CONTROL_MSG)
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET);
+}
+
+static int
+eth_igbvf_interrupt_action(struct rte_eth_dev *dev)
+{
+	struct e1000_interrupt *intr =
+		E1000_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+
+	if (intr->flags & E1000_FLAG_MAILBOX) {
+		igbvf_mbx_process(dev);
+		intr->flags &= ~E1000_FLAG_MAILBOX;
+	}
+
+	igbvf_intr_enable(dev);
+	rte_intr_enable(&dev->pci_dev->intr_handle);
+
+	return 0;
+}
+
+static void
+eth_igbvf_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
+			    void *param)
+{
+	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
+
+	eth_igbvf_interrupt_get_status(dev);
+	eth_igbvf_interrupt_action(dev);
+}
+
+static int
 eth_igb_led_on(struct rte_eth_dev *dev)
 {
 	struct e1000_hw *hw;
@@ -2834,6 +2952,8 @@ igbvf_dev_start(struct rte_eth_dev *dev)
 	struct e1000_adapter *adapter =
 		E1000_DEV_PRIVATE(dev->data->dev_private);
 	int ret;
+	struct rte_intr_handle *intr_handle = &dev->pci_dev->intr_handle;
+	uint32_t intr_vector = 0;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -2853,12 +2973,41 @@ igbvf_dev_start(struct rte_eth_dev *dev)
 		return ret;
 	}
 
+	/* check and configure queue intr-vector mapping */
+	if (dev->data->dev_conf.intr_conf.rxq != 0) {
+		intr_vector = dev->data->nb_rx_queues;
+		ret = rte_intr_efd_enable(intr_handle, intr_vector);
+		if (ret)
+			return ret;
+	}
+
+	if (rte_intr_dp_is_en(intr_handle) && !intr_handle->intr_vec) {
+		intr_handle->intr_vec =
+			rte_zmalloc("intr_vec",
+				    dev->data->nb_rx_queues * sizeof(int), 0);
+		if (!intr_handle->intr_vec) {
+			PMD_INIT_LOG(ERR, "Failed to allocate %d rx_queues"
+				     " intr_vec\n", dev->data->nb_rx_queues);
+			return -ENOMEM;
+		}
+	}
+
+	eth_igbvf_configure_msix_intr(dev);
+
+	/* enable uio/vfio intr/eventfd mapping */
+	rte_intr_enable(intr_handle);
+
+	/* resume enabled intr since hw reset */
+	igbvf_intr_enable(dev);
+
 	return 0;
 }
 
 static void
 igbvf_dev_stop(struct rte_eth_dev *dev)
 {
+	struct rte_intr_handle *intr_handle = &dev->pci_dev->intr_handle;
+
 	PMD_INIT_FUNC_TRACE();
 
 	igbvf_stop_adapter(dev);
@@ -2870,6 +3019,16 @@ igbvf_dev_stop(struct rte_eth_dev *dev)
 	igbvf_set_vfta_all(dev,0);
 
 	igb_dev_clear_queues(dev);
+
+	/* disable intr eventfd mapping */
+	rte_intr_disable(intr_handle);
+
+	/* Clean datapath event and queue/vec mapping */
+	rte_intr_efd_disable(intr_handle);
+	if (intr_handle->intr_vec) {
+		rte_free(intr_handle->intr_vec);
+		intr_handle->intr_vec = NULL;
+	}
 }
 
 static void
-- 
1.9.3

Re: [dpdk-dev] [PATCH 1/2] ixgbe: VF supports mailbox interruption for PF link up/down

[Wu, Jingjing]

> +static void ixgbevf_mbx_process(struct rte_eth_dev *dev) {
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
> +	u32 in_msg = 0;
> +
> +	if (ixgbe_read_mbx(hw, &in_msg, 1, 0))
> +		return;
> +
> +	/* PF reset VF event */
> +	if (in_msg == IXGBE_PF_CONTROL_MSG)
> +		_rte_eth_dev_callback_process(dev,
> RTE_ETH_EVENT_INTR_RESET); }
> +

RTE_ETH_EVENT_INTR_RESET is used for PF reset event reporting,
and this patch is to support PF link change. Maybe
RTE_ETH_EVENT_INTR_LSC should be used here instead.
Or you need to distinguish which control message is coming from PF.

> +static int
> +ixgbevf_dev_interrupt_get_status(struct rte_eth_dev *dev) {
> +	uint32_t eicr;
> +	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);
> +	ixgbevf_intr_disable(hw);
> +
> +	/* read-on-clear nic registers here */
> +	eicr = IXGBE_READ_REG(hw, IXGBE_VTEICR);
> +	intr->flags = 0;
> +
> +	/* only one misc vector supported - mailbox */
> +	eicr &= IXGBE_VTEICR_MASK;
> +	if (eicr == IXGBE_MISC_VEC_ID)
> +		intr->flags |= IXGBE_FLAG_MAILBOX;
> +
> +	return 0;
> +}
> +
> +static int
> +ixgbevf_dev_interrupt_action(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);
> +
> +	if (intr->flags & IXGBE_FLAG_MAILBOX) {
> +		ixgbevf_mbx_process(dev);
> +		intr->flags &= ~IXGBE_FLAG_MAILBOX;
> +	}
> +
> +	ixgbevf_intr_enable(hw);
> +
> +	return 0;
> +}


For the readability, it's better to put ixgbevf_intr_disable and ixgbevf_intr_enable
in the same function,  for example, at the beginning and ending of
ixgbevf_dev_interrupt_handler.

Thanks
Jingjing

Re: [dpdk-dev] [PATCH 1/2] ixgbe: VF supports mailbox interruption for PF link up/down

[Lu, Wenzhuo]

Hi Jingjing,

Thanks for the comments. Please see inline.

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Friday, May 27, 2016 4:31 PM
> To: Lu, Wenzhuo; dev@dpdk.org
> Cc: Lu, Wenzhuo
> Subject: RE: [dpdk-dev] [PATCH 1/2] ixgbe: VF supports mailbox interruption for
> PF link up/down
> 
> > +static void ixgbevf_mbx_process(struct rte_eth_dev *dev) {
> > +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data-
> > >dev_private);
> > +	u32 in_msg = 0;
> > +
> > +	if (ixgbe_read_mbx(hw, &in_msg, 1, 0))
> > +		return;
> > +
> > +	/* PF reset VF event */
> > +	if (in_msg == IXGBE_PF_CONTROL_MSG)
> > +		_rte_eth_dev_callback_process(dev,
> > RTE_ETH_EVENT_INTR_RESET); }
> > +
> 
> RTE_ETH_EVENT_INTR_RESET is used for PF reset event reporting, and this
> patch is to support PF link change. Maybe RTE_ETH_EVENT_INTR_LSC should be
> used here instead.
> Or you need to distinguish which control message is coming from PF.
Ixgbe PF driver use IXGBE_PF_CONTROL_MSG to cover LSC, configuration, link up/down.
After these events, VF cannot work. I want the user to reset the VF port to make it work again.
Let me explain more in commit log to make it clearer.

> 
> > +static int
> > +ixgbevf_dev_interrupt_get_status(struct rte_eth_dev *dev) {
> > +	uint32_t eicr;
> > +	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);
> > +	ixgbevf_intr_disable(hw);
> > +
> > +	/* read-on-clear nic registers here */
> > +	eicr = IXGBE_READ_REG(hw, IXGBE_VTEICR);
> > +	intr->flags = 0;
> > +
> > +	/* only one misc vector supported - mailbox */
> > +	eicr &= IXGBE_VTEICR_MASK;
> > +	if (eicr == IXGBE_MISC_VEC_ID)
> > +		intr->flags |= IXGBE_FLAG_MAILBOX;
> > +
> > +	return 0;
> > +}
> > +
> > +static int
> > +ixgbevf_dev_interrupt_action(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);
> > +
> > +	if (intr->flags & IXGBE_FLAG_MAILBOX) {
> > +		ixgbevf_mbx_process(dev);
> > +		intr->flags &= ~IXGBE_FLAG_MAILBOX;
> > +	}
> > +
> > +	ixgbevf_intr_enable(hw);
> > +
> > +	return 0;
> > +}
> 
> 
> For the readability, it's better to put ixgbevf_intr_disable and
> ixgbevf_intr_enable in the same function,  for example, at the beginning and
> ending of ixgbevf_dev_interrupt_handler.
O, I follow the style of ixgbe. It's a good suggestion, I may create a new patch to enhance it later.

> 
> Thanks
> Jingjing

[dpdk-dev] [PATCH v2 0/2] support mailbox interruption on ixgbe/igb VF

[Wenzhuo Lu]

This patch set addes the support of the mailbox interruption on VF.
So, VF can receice the messges for physical link down/up. And
register a reset callback in the handler.

PS: This patch set is splitted from a previous patch set, *automatic
link recovery on ixgbe/igb VF*.

v2:
- Reword the commit log and add more details.
- Rebase the patches on the latest code.

Wenzhuo Lu (2):
  ixgbe: VF supports mailbox interruption for PF link up/down
  igb: VF supports mailbox interruption for PF link up/down

 doc/guides/rel_notes/release_16_07.rst |   6 ++
 drivers/net/e1000/igb_ethdev.c         | 159 +++++++++++++++++++++++++++++++++
 drivers/net/ixgbe/ixgbe_ethdev.c       |  84 ++++++++++++++++-
 3 files changed, 246 insertions(+), 3 deletions(-)

-- 
1.9.3

[dpdk-dev] [PATCH v2 1/2] ixgbe: VF supports mailbox interruption for PF link up/down

[Wenzhuo Lu]

In this scenario, kernel PF + DPDK VF, when PF finds the link
state changes, up -> down or down -> up, it will send a
message to VF by mailbox. This link state change may be
triggered by PHY disconnection/reconnection, configuration
like *ifconfig down/up* or interface parameter, like MTU,
change.
This patch enables the support of the mailbox interruption,
so VF can receive the message of link up/down.
After VF receives this message, VF port need to be reset to
recover. So the handler of this message registers a reset
callback to let APP reset the VF port.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 doc/guides/rel_notes/release_16_07.rst |  6 +++
 drivers/net/ixgbe/ixgbe_ethdev.c       | 84 ++++++++++++++++++++++++++++++++--
 2 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/doc/guides/rel_notes/release_16_07.rst b/doc/guides/rel_notes/release_16_07.rst
index 30e78d4..990bd46 100644
--- a/doc/guides/rel_notes/release_16_07.rst
+++ b/doc/guides/rel_notes/release_16_07.rst
@@ -47,6 +47,12 @@ New Features
   * Dropped specific Xen Dom0 code.
   * Dropped specific anonymous mempool code in testpmd.
 
+* **Added mailbox interruption support for ixgbe VF.**
+
+  When the link becomes down or up, PF will use mailbox message to notice
+  VF. To handle this link up/down event, add the mailbox interruption
+  support to receive the message.
+
 
 Resolved Issues
 ---------------
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index a2b170b..05f4f29 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -150,6 +150,7 @@
 #define IXGBE_VMVIR_TAGA_ETAG_INSERT           0x08000000
 #define IXGBE_VMTIR(_i) (0x00017000 + ((_i) * 4)) /* 64 of these (0-63) */
 #define IXGBE_QDE_STRIP_TAG                    0x00000004
+#define IXGBE_VTEICR_MASK                      0x07
 
 enum ixgbevf_xcast_modes {
 	IXGBEVF_XCAST_MODE_NONE = 0,
@@ -361,6 +362,8 @@ static int ixgbe_timesync_read_time(struct rte_eth_dev *dev,
 				   struct timespec *timestamp);
 static int ixgbe_timesync_write_time(struct rte_eth_dev *dev,
 				   const struct timespec *timestamp);
+static void ixgbevf_dev_interrupt_handler(struct rte_intr_handle *handle,
+					  void *param);
 
 static int ixgbe_dev_l2_tunnel_eth_type_conf
 	(struct rte_eth_dev *dev, struct rte_eth_l2_tunnel_conf *l2_tunnel);
@@ -1442,6 +1445,12 @@ eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 		return -EIO;
 	}
 
+	rte_intr_callback_register(&pci_dev->intr_handle,
+				   ixgbevf_dev_interrupt_handler,
+				   (void *)eth_dev);
+	rte_intr_enable(&pci_dev->intr_handle);
+	ixgbevf_intr_enable(hw);
+
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id, "ixgbe_mac_82599_vf");
@@ -1455,6 +1464,7 @@ static int
 eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
 {
 	struct ixgbe_hw *hw;
+	struct rte_pci_device *pci_dev = eth_dev->pci_dev;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -1476,6 +1486,11 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
 	rte_free(eth_dev->data->mac_addrs);
 	eth_dev->data->mac_addrs = NULL;
 
+	rte_intr_disable(&pci_dev->intr_handle);
+	rte_intr_callback_unregister(&pci_dev->intr_handle,
+				     ixgbevf_dev_interrupt_handler,
+				     (void *)eth_dev);
+
 	return 0;
 }
 
@@ -4074,6 +4089,8 @@ ixgbevf_dev_stop(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
+	ixgbevf_intr_disable(hw);
+
 	hw->adapter_stopped = 1;
 	ixgbe_stop_adapter(hw);
 
@@ -4818,6 +4835,9 @@ ixgbevf_configure_msix(struct rte_eth_dev *dev)
 	uint32_t q_idx;
 	uint32_t vector_idx = IXGBE_MISC_VEC_ID;
 
+	/* Configure VF other cause ivar */
+	ixgbevf_set_ivar_map(hw, -1, 1, vector_idx);
+
 	/* won't configure msix register if no mapping is done
 	 * between intr vector and event fd.
 	 */
@@ -4832,9 +4852,6 @@ ixgbevf_configure_msix(struct rte_eth_dev *dev)
 		ixgbevf_set_ivar_map(hw, 0, q_idx, vector_idx);
 		intr_handle->intr_vec[q_idx] = vector_idx;
 	}
-
-	/* Configure VF other cause ivar */
-	ixgbevf_set_ivar_map(hw, -1, 1, vector_idx);
 }
 
 /**
@@ -7154,6 +7171,67 @@ ixgbevf_dev_allmulticast_disable(struct rte_eth_dev *dev)
 	ixgbevf_update_xcast_mode(hw, IXGBEVF_XCAST_MODE_NONE);
 }
 
+static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
+{
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	u32 in_msg = 0;
+
+	if (ixgbe_read_mbx(hw, &in_msg, 1, 0))
+		return;
+
+	/* PF reset VF event */
+	if (in_msg == IXGBE_PF_CONTROL_MSG)
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET);
+}
+
+static int
+ixgbevf_dev_interrupt_get_status(struct rte_eth_dev *dev)
+{
+	uint32_t eicr;
+	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);
+	ixgbevf_intr_disable(hw);
+
+	/* read-on-clear nic registers here */
+	eicr = IXGBE_READ_REG(hw, IXGBE_VTEICR);
+	intr->flags = 0;
+
+	/* only one misc vector supported - mailbox */
+	eicr &= IXGBE_VTEICR_MASK;
+	if (eicr == IXGBE_MISC_VEC_ID)
+		intr->flags |= IXGBE_FLAG_MAILBOX;
+
+	return 0;
+}
+
+static int
+ixgbevf_dev_interrupt_action(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);
+
+	if (intr->flags & IXGBE_FLAG_MAILBOX) {
+		ixgbevf_mbx_process(dev);
+		intr->flags &= ~IXGBE_FLAG_MAILBOX;
+	}
+
+	ixgbevf_intr_enable(hw);
+
+	return 0;
+}
+
+static void
+ixgbevf_dev_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
+			      void *param)
+{
+	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
+
+	ixgbevf_dev_interrupt_get_status(dev);
+	ixgbevf_dev_interrupt_action(dev);
+}
+
 static struct rte_driver rte_ixgbe_driver = {
 	.type = PMD_PDEV,
 	.init = rte_ixgbe_pmd_init,
-- 
1.9.3

[dpdk-dev] [PATCH v2 2/2] igb: VF supports mailbox interruption for PF link up/down

[Wenzhuo Lu]

In this scenario, kernel PF + DPDK VF, when PF finds the link
state changes, up -> down or down -> up, it will send a
message to VF by mailbox. This link state change may be
triggered by PHY disconnection/reconnection, configuration
like *ifconfig down/up* or interface parameter, like MTU,
change.
This patch enables the support of the mailbox interruption,
so VF can receive the message of link up/down.
After VF receives this message, VF port need to be reset to
recover. So the handler of this message registers a reset
callback to let APP reset the VF port.

Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
---
 doc/guides/rel_notes/release_16_07.rst |   2 +-
 drivers/net/e1000/igb_ethdev.c         | 159 +++++++++++++++++++++++++++++++++
 2 files changed, 160 insertions(+), 1 deletion(-)

diff --git a/doc/guides/rel_notes/release_16_07.rst b/doc/guides/rel_notes/release_16_07.rst
index 990bd46..a761e3c 100644
--- a/doc/guides/rel_notes/release_16_07.rst
+++ b/doc/guides/rel_notes/release_16_07.rst
@@ -47,7 +47,7 @@ New Features
   * Dropped specific Xen Dom0 code.
   * Dropped specific anonymous mempool code in testpmd.
 
-* **Added mailbox interruption support for ixgbe VF.**
+* **Added mailbox interruption support for ixgbe/igb VF.**
 
   When the link becomes down or up, PF will use mailbox message to notice
   VF. To handle this link up/down event, add the mailbox interruption
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index f0921ee..b0e5e6a 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -86,6 +86,12 @@
 #define E1000_INCVALUE_82576         (16 << IGB_82576_TSYNC_SHIFT)
 #define E1000_TSAUXC_DISABLE_SYSTIME 0x80000000
 
+#define E1000_VTIVAR_MISC                0x01740
+#define E1000_VTIVAR_MISC_MASK           0xFF
+#define E1000_VTIVAR_VALID               0x80
+#define E1000_VTIVAR_MISC_MAILBOX        0
+#define E1000_VTIVAR_MISC_INTR_MASK      0x3
+
 static int  eth_igb_configure(struct rte_eth_dev *dev);
 static int  eth_igb_start(struct rte_eth_dev *dev);
 static void eth_igb_stop(struct rte_eth_dev *dev);
@@ -259,6 +265,9 @@ static void eth_igb_assign_msix_vector(struct e1000_hw *hw, int8_t direction,
 static void eth_igb_write_ivar(struct e1000_hw *hw, uint8_t msix_vector,
 			       uint8_t index, uint8_t offset);
 static void eth_igb_configure_msix_intr(struct rte_eth_dev *dev);
+static void eth_igbvf_interrupt_handler(struct rte_intr_handle *handle,
+					void *param);
+static void igbvf_mbx_process(struct rte_eth_dev *dev);
 
 /*
  * Define VF Stats MACRO for Non "cleared on read" register
@@ -554,6 +563,41 @@ igb_intr_disable(struct e1000_hw *hw)
 	E1000_WRITE_FLUSH(hw);
 }
 
+static inline void
+igbvf_intr_enable(struct rte_eth_dev *dev)
+{
+	struct e1000_hw *hw =
+		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	/* only for mailbox */
+	E1000_WRITE_REG(hw, E1000_EIAM, 1 << E1000_VTIVAR_MISC_MAILBOX);
+	E1000_WRITE_REG(hw, E1000_EIAC, 1 << E1000_VTIVAR_MISC_MAILBOX);
+	E1000_WRITE_REG(hw, E1000_EIMS, 1 << E1000_VTIVAR_MISC_MAILBOX);
+	E1000_WRITE_FLUSH(hw);
+}
+
+/* only for mailbox now. If RX/TX needed, should extend this function.  */
+static void
+igbvf_set_ivar_map(struct e1000_hw *hw, uint8_t msix_vector)
+{
+	uint32_t tmp = 0;
+
+	/* mailbox */
+	tmp |= (msix_vector & E1000_VTIVAR_MISC_INTR_MASK);
+	tmp |= E1000_VTIVAR_VALID;
+	E1000_WRITE_REG(hw, E1000_VTIVAR_MISC, tmp);
+}
+
+static void
+eth_igbvf_configure_msix_intr(struct rte_eth_dev *dev)
+{
+	struct e1000_hw *hw =
+		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	/* Configure VF other cause ivar */
+	igbvf_set_ivar_map(hw, E1000_VTIVAR_MISC_MAILBOX);
+}
+
 static inline int32_t
 igb_pf_reset_hw(struct e1000_hw *hw)
 {
@@ -942,6 +986,10 @@ eth_igbvf_dev_init(struct rte_eth_dev *eth_dev)
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id, "igb_mac_82576_vf");
 
+	rte_intr_callback_register(&pci_dev->intr_handle,
+				   eth_igbvf_interrupt_handler,
+				   (void *)eth_dev);
+
 	return 0;
 }
 
@@ -950,6 +998,7 @@ eth_igbvf_dev_uninit(struct rte_eth_dev *eth_dev)
 {
 	struct e1000_adapter *adapter =
 		E1000_DEV_PRIVATE(eth_dev->data->dev_private);
+	struct rte_pci_device *pci_dev = eth_dev->pci_dev;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -966,6 +1015,12 @@ eth_igbvf_dev_uninit(struct rte_eth_dev *eth_dev)
 	rte_free(eth_dev->data->mac_addrs);
 	eth_dev->data->mac_addrs = NULL;
 
+	/* disable uio intr before callback unregister */
+	rte_intr_disable(&pci_dev->intr_handle);
+	rte_intr_callback_unregister(&pci_dev->intr_handle,
+				     eth_igbvf_interrupt_handler,
+				     (void *)eth_dev);
+
 	return 0;
 }
 
@@ -2564,6 +2619,69 @@ eth_igb_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
 }
 
 static int
+eth_igbvf_interrupt_get_status(struct rte_eth_dev *dev)
+{
+	uint32_t eicr;
+	struct e1000_hw *hw =
+		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct e1000_interrupt *intr =
+		E1000_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+
+	igbvf_intr_disable(hw);
+
+	/* read-on-clear nic registers here */
+	eicr = E1000_READ_REG(hw, E1000_EICR);
+	intr->flags = 0;
+
+	if (eicr == E1000_VTIVAR_MISC_MAILBOX)
+		intr->flags |= E1000_FLAG_MAILBOX;
+
+	return 0;
+}
+
+void igbvf_mbx_process(struct rte_eth_dev *dev)
+{
+	struct e1000_hw *hw =
+		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct e1000_mbx_info *mbx = &hw->mbx;
+	u32 in_msg = 0;
+
+	if (mbx->ops.read(hw, &in_msg, 1, 0))
+		return;
+
+	/* PF reset VF event */
+	if (in_msg == E1000_PF_CONTROL_MSG)
+		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_RESET);
+}
+
+static int
+eth_igbvf_interrupt_action(struct rte_eth_dev *dev)
+{
+	struct e1000_interrupt *intr =
+		E1000_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+
+	if (intr->flags & E1000_FLAG_MAILBOX) {
+		igbvf_mbx_process(dev);
+		intr->flags &= ~E1000_FLAG_MAILBOX;
+	}
+
+	igbvf_intr_enable(dev);
+	rte_intr_enable(&dev->pci_dev->intr_handle);
+
+	return 0;
+}
+
+static void
+eth_igbvf_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
+			    void *param)
+{
+	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
+
+	eth_igbvf_interrupt_get_status(dev);
+	eth_igbvf_interrupt_action(dev);
+}
+
+static int
 eth_igb_led_on(struct rte_eth_dev *dev)
 {
 	struct e1000_hw *hw;
@@ -2834,6 +2952,8 @@ igbvf_dev_start(struct rte_eth_dev *dev)
 	struct e1000_adapter *adapter =
 		E1000_DEV_PRIVATE(dev->data->dev_private);
 	int ret;
+	struct rte_intr_handle *intr_handle = &dev->pci_dev->intr_handle;
+	uint32_t intr_vector = 0;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -2853,12 +2973,41 @@ igbvf_dev_start(struct rte_eth_dev *dev)
 		return ret;
 	}
 
+	/* check and configure queue intr-vector mapping */
+	if (dev->data->dev_conf.intr_conf.rxq != 0) {
+		intr_vector = dev->data->nb_rx_queues;
+		ret = rte_intr_efd_enable(intr_handle, intr_vector);
+		if (ret)
+			return ret;
+	}
+
+	if (rte_intr_dp_is_en(intr_handle) && !intr_handle->intr_vec) {
+		intr_handle->intr_vec =
+			rte_zmalloc("intr_vec",
+				    dev->data->nb_rx_queues * sizeof(int), 0);
+		if (!intr_handle->intr_vec) {
+			PMD_INIT_LOG(ERR, "Failed to allocate %d rx_queues"
+				     " intr_vec\n", dev->data->nb_rx_queues);
+			return -ENOMEM;
+		}
+	}
+
+	eth_igbvf_configure_msix_intr(dev);
+
+	/* enable uio/vfio intr/eventfd mapping */
+	rte_intr_enable(intr_handle);
+
+	/* resume enabled intr since hw reset */
+	igbvf_intr_enable(dev);
+
 	return 0;
 }
 
 static void
 igbvf_dev_stop(struct rte_eth_dev *dev)
 {
+	struct rte_intr_handle *intr_handle = &dev->pci_dev->intr_handle;
+
 	PMD_INIT_FUNC_TRACE();
 
 	igbvf_stop_adapter(dev);
@@ -2870,6 +3019,16 @@ igbvf_dev_stop(struct rte_eth_dev *dev)
 	igbvf_set_vfta_all(dev,0);
 
 	igb_dev_clear_queues(dev);
+
+	/* disable intr eventfd mapping */
+	rte_intr_disable(intr_handle);
+
+	/* Clean datapath event and queue/vec mapping */
+	rte_intr_efd_disable(intr_handle);
+	if (intr_handle->intr_vec) {
+		rte_free(intr_handle->intr_vec);
+		intr_handle->intr_vec = NULL;
+	}
 }
 
 static void
-- 
1.9.3

Re: [dpdk-dev] [PATCH v2 1/2] ixgbe: VF supports mailbox interruption for PF link up/down

[Wu, Jingjing]

> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Wednesday, June 01, 2016 9:53 AM
> To: dev@dpdk.org
> Cc: Wu, Jingjing; Lu, Wenzhuo
> Subject: [PATCH v2 1/2] ixgbe: VF supports mailbox interruption for PF link
> up/down
> 
> In this scenario, kernel PF + DPDK VF, when PF finds the link state changes,
> up -> down or down -> up, it will send a message to VF by mailbox. This link
> state change may be triggered by PHY disconnection/reconnection,
> configuration like *ifconfig down/up* or interface parameter, like MTU,
> change.
> This patch enables the support of the mailbox interruption, so VF can receive
> the message of link up/down.
> After VF receives this message, VF port need to be reset to recover. So the
> handler of this message registers a reset callback to let APP reset the VF port.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Acked-by: Jingjing Wu <jingjing.wu@intel.com>

Re: [dpdk-dev] [PATCH v2 2/2] igb: VF supports mailbox interruption for PF link up/down

[Wu, Jingjing]

> -----Original Message-----
> From: Lu, Wenzhuo
> Sent: Wednesday, June 01, 2016 9:53 AM
> To: dev@dpdk.org
> Cc: Wu, Jingjing; Lu, Wenzhuo
> Subject: [PATCH v2 2/2] igb: VF supports mailbox interruption for PF link
> up/down
> 
> In this scenario, kernel PF + DPDK VF, when PF finds the link state changes,
> up -> down or down -> up, it will send a message to VF by mailbox. This link
> state change may be triggered by PHY disconnection/reconnection,
> configuration like *ifconfig down/up* or interface parameter, like MTU,
> change.
> This patch enables the support of the mailbox interruption, so VF can receive
> the message of link up/down.
> After VF receives this message, VF port need to be reset to recover. So the
> handler of this message registers a reset callback to let APP reset the VF port.
> 
> Signed-off-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>

Re: [dpdk-dev] [PATCH v2 1/2] ixgbe: VF supports mailbox interruption for PF link up/down

[Bruce Richardson]

On Wed, Jun 01, 2016 at 09:53:08AM +0800, Wenzhuo Lu wrote:
> In this scenario, kernel PF + DPDK VF, when PF finds the link
> state changes, up -> down or down -> up, it will send a
> message to VF by mailbox. This link state change may be
> triggered by PHY disconnection/reconnection, configuration
> like *ifconfig down/up* or interface parameter, like MTU,
> change.
> This patch enables the support of the mailbox interruption,
> so VF can receive the message of link up/down.
> After VF receives this message, VF port need to be reset to
> recover. So the handler of this message registers a reset
> callback to let APP reset the VF port.
> 
Hi Wenzhuo,

I'm a little unclear as to the last paragraph of this message. Does the
app configure the callback to handle the reset, or does the driver set
up a callback automatically and handle the event itself? [In other words,
who/what is the "handler" in the final sentence, the driver or the app?]

Thanks,
/Bruce

Re: [dpdk-dev] [PATCH v2 1/2] ixgbe: VF supports mailbox interruption for PF link up/down

[Lu, Wenzhuo]

Hi Bruce,

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Wednesday, June 15, 2016 12:54 AM
> To: Lu, Wenzhuo
> Cc: dev@dpdk.org; Wu, Jingjing
> Subject: Re: [dpdk-dev] [PATCH v2 1/2] ixgbe: VF supports mailbox
> interruption for PF link up/down
> 
> On Wed, Jun 01, 2016 at 09:53:08AM +0800, Wenzhuo Lu wrote:
> > In this scenario, kernel PF + DPDK VF, when PF finds the link state
> > changes, up -> down or down -> up, it will send a message to VF by
> > mailbox. This link state change may be triggered by PHY
> > disconnection/reconnection, configuration like *ifconfig down/up* or
> > interface parameter, like MTU, change.
> > This patch enables the support of the mailbox interruption, so VF can
> > receive the message of link up/down.
> > After VF receives this message, VF port need to be reset to recover.
> > So the handler of this message registers a reset callback to let APP
> > reset the VF port.
> >
> Hi Wenzhuo,
> 
> I'm a little unclear as to the last paragraph of this message. Does the app
> configure the callback to handle the reset, or does the driver set up a
> callback automatically and handle the event itself? [In other words,
> who/what is the "handler" in the final sentence, the driver or the app?]
Sorry, I may not make it clear. The handler is the APP.  In the code, _rte_eth_dev_callback_process is used for the message. It's the APP's responsibility to realize the callback function and decide what to do with the message.

> 
> Thanks,
> /Bruce

Re: [dpdk-dev] [PATCH v2 0/2] support mailbox interruption on ixgbe/igb VF

[Bruce Richardson]

On Wed, Jun 01, 2016 at 09:53:07AM +0800, Wenzhuo Lu wrote:
> This patch set addes the support of the mailbox interruption on VF.
> So, VF can receice the messges for physical link down/up. And
> register a reset callback in the handler.
> 
> PS: This patch set is splitted from a previous patch set, *automatic
> link recovery on ixgbe/igb VF*.
> 
> v2:
> - Reword the commit log and add more details.
> - Rebase the patches on the latest code.
> 
> Wenzhuo Lu (2):
>   ixgbe: VF supports mailbox interruption for PF link up/down
>   igb: VF supports mailbox interruption for PF link up/down

s/interruptions/interrupt/

Patchset applied with some commit message and doc rewording to dpdk-next-net/rel_16_07

/Bruce

Re: [dpdk-dev] [PATCH 0/2] support mailbox interruption on ixgbe/igb VF

[Luca Boccassi]

On Tue, 2016-05-24 at 14:06 +0800, Wenzhuo Lu wrote:
> This patch set addes the support of the mailbox interruption on VF.
> So, VF can receice the messges for physical link down/up.
> 
> PS: This patch set is splitted from a previous patch set, *automatic
> link recovery on ixgbe/igb VF*.
> 
> Wenzhuo Lu (2):
>   ixgbe: VF supports mailbox interruption for PF link up/down
>   igb: VF supports mailbox interruption for PF link up/down
> 
>  doc/guides/rel_notes/release_16_07.rst |   6 ++
>  drivers/net/e1000/igb_ethdev.c         | 159 +++++++++++++++++++++++++++++++++
>  drivers/net/ixgbe/ixgbe_ethdev.c       |  85 +++++++++++++++++-
>  3 files changed, 247 insertions(+), 3 deletions(-)
> 

Hi,

After backporting these patches to 16.04 or 2.2, we get a segmentation
fault when using interface bonding when the interfaces go down. The
scenario is:

- Host has a X540-AT2 10gb card using the ixgbe driver, 2 VFs are
created and passes to the qemu/kvm guest VM via libvirt
- Guest creates a bonded link using the 2 VFs
- Host sets the VFs state to down via ip link
- Guess DPDK app segfaults

Backtrace:

#0  0x0000000000000000 in ?? ()
No symbol table info available.
#1  0x00007ffff5003957 in bond_ethdev_slave_link_status_change_monitor (
    cb_arg=0x727748 <rte_eth_devices@@DPDK_2.2+4168>)
    at /usr/src/packages/BUILD/drivers/net/bonding/rte_eth_bond_pmd.c:1938
        internals = 0x7fffeb8f5ec0
        i = 0
        polling_slave_found = 0
#2  0x00007ffff68ea88c in eal_alarm_callback (hdl=<optimized out>, arg=<optimized out>)
    at /usr/src/packages/BUILD/lib/librte_eal/linuxapp/eal/eal_alarm.c:120
        now = {tv_sec = 356, tv_nsec = 551082574}
        ap = 0x7fffebc22380
#3  0x00007ffff68e926d in eal_intr_process_interrupts (nfds=<optimized out>, events=<optimized out>)
    at /usr/src/packages/BUILD/lib/librte_eal/linuxapp/eal/eal_interrupts.c:752
        bytes_read = <optimized out>
        buf = {uio_intr_count = 1, vfio_intr_count = 1, timerfd_num = 1, 
          charbuf = "\001\000\000\000\000\000\000\000D\260~\363\377\177\000"}
        n = 0
        src = 0x7fffeb8d2640
        cb = 0x7fffeb8d2d80
        next = <optimized out>
        active_cb = <optimized out>
#4  eal_intr_handle_interrupts (totalfds=<optimized out>, pfd=12)
    at /usr/src/packages/BUILD/lib/librte_eal/linuxapp/eal/eal_interrupts.c:800
        events = 0x7fffefb1ba20
        nfds = 1
#5  eal_intr_thread_main (arg=<optimized out>)
    at /usr/src/packages/BUILD/lib/librte_eal/linuxapp/eal/eal_interrupts.c:870
        pipe_event = {events = 3, data = {ptr = 0x6, fd = 6, u32 = 6, u64 = 6}}
        src = <optimized out>
        numfds = <optimized out>
        pfd = 12
        ev = {events = 3, data = {ptr = 0xf7df02e500000005, fd = 5, u32 = 5, 
            u64 = 17860997829745442821}}
        __func__ = "eal_intr_thread_main"
#6  0x00007ffff37eb0a4 in start_thread (arg=0x7fffefb3c700) at pthread_create.c:309
        __res = <optimized out>
        pd = 0x7fffefb3c700
        now = <optimized out>
        unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140737214924544, 2510814068564645188, 1, 
                140737354125408, 140737336548072, 140737214924544, -2510779380161489596, 
                -2510806361332034236}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, 
            data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
        not_first_call = <optimized out>
        pagesize_m1 = <optimized out>
        sp = <optimized out>
        freesize = <optimized out>
        __PRETTY_FUNCTION__ = "start_thread"
#7  0x00007ffff1b8287d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111

It dies in this bit:

/* Update slave link status */
(*slave_ethdev->dev_ops->link_update)(slave_ethdev,
	internals->slaves[i].link_status_wait_to_complete);

(gdb) print rte_eth_devices[internals->slaves[i].port_id]
$7 = {rx_pkt_burst = 0x0, tx_pkt_burst = 0x0, data = 0x0, driver = 0x0, dev_ops = 0x0, {pci_dev = 0x0, 
    vmbus_dev = 0x0}, link_intr_cbs = {tqh_first = 0x0, tqh_last = 0x0}, post_rx_burst_cbs = {
    0x0 <repeats 256 times>}, pre_tx_burst_cbs = {0x0 <repeats 256 times>}, attached = 0 '\000', 
  dev_type = RTE_ETH_DEV_UNKNOWN}

I'm assuming it's not a simply matter of checking the dev_type or for
nulls. Do you have any suggestions/insight? I'm delving into the issue,
but it's the first time I look at the bonding code so any help or
pointers would be greatly appreciated.

Note that I also tried to backport the additional patches for reset that
are currently under review on top of these, but there's no difference.
But I have not yet used the new reset API in our app though.

Thanks!

-- 
Kind regards,
Luca Boccassi

Re: [dpdk-dev] [PATCH 0/2] support mailbox interruption on ixgbe/igb VF

[Luca Boccassi]

On Tue, 2016-06-28 at 10:16 +0000, Luca Boccassi wrote:
> On Tue, 2016-05-24 at 14:06 +0800, Wenzhuo Lu wrote:
> > This patch set addes the support of the mailbox interruption on VF.
> > So, VF can receice the messges for physical link down/up.
> > 
> > PS: This patch set is splitted from a previous patch set, *automatic
> > link recovery on ixgbe/igb VF*.
> > 
> > Wenzhuo Lu (2):
> >   ixgbe: VF supports mailbox interruption for PF link up/down
> >   igb: VF supports mailbox interruption for PF link up/down
> > 
> >  doc/guides/rel_notes/release_16_07.rst |   6 ++
> >  drivers/net/e1000/igb_ethdev.c         | 159 +++++++++++++++++++++++++++++++++
> >  drivers/net/ixgbe/ixgbe_ethdev.c       |  85 +++++++++++++++++-
> >  3 files changed, 247 insertions(+), 3 deletions(-)
> > 
> 
> Hi,
> 
> After backporting these patches to 16.04 or 2.2, we get a segmentation
> fault when using interface bonding when the interfaces go down. The
> scenario is:
> 
> - Host has a X540-AT2 10gb card using the ixgbe driver, 2 VFs are
> created and passes to the qemu/kvm guest VM via libvirt
> - Guest creates a bonded link using the 2 VFs
> - Host sets the VFs state to down via ip link
> - Guess DPDK app segfaults
> 
> Backtrace:
> 
> #0  0x0000000000000000 in ?? ()
> No symbol table info available.
> #1  0x00007ffff5003957 in bond_ethdev_slave_link_status_change_monitor (
>     cb_arg=0x727748 <rte_eth_devices@@DPDK_2.2+4168>)
>     at /usr/src/packages/BUILD/drivers/net/bonding/rte_eth_bond_pmd.c:1938
>         internals = 0x7fffeb8f5ec0
>         i = 0
>         polling_slave_found = 0
> #2  0x00007ffff68ea88c in eal_alarm_callback (hdl=<optimized out>, arg=<optimized out>)
>     at /usr/src/packages/BUILD/lib/librte_eal/linuxapp/eal/eal_alarm.c:120
>         now = {tv_sec = 356, tv_nsec = 551082574}
>         ap = 0x7fffebc22380
> #3  0x00007ffff68e926d in eal_intr_process_interrupts (nfds=<optimized out>, events=<optimized out>)
>     at /usr/src/packages/BUILD/lib/librte_eal/linuxapp/eal/eal_interrupts.c:752
>         bytes_read = <optimized out>
>         buf = {uio_intr_count = 1, vfio_intr_count = 1, timerfd_num = 1, 
>           charbuf = "\001\000\000\000\000\000\000\000D\260~\363\377\177\000"}
>         n = 0
>         src = 0x7fffeb8d2640
>         cb = 0x7fffeb8d2d80
>         next = <optimized out>
>         active_cb = <optimized out>
> #4  eal_intr_handle_interrupts (totalfds=<optimized out>, pfd=12)
>     at /usr/src/packages/BUILD/lib/librte_eal/linuxapp/eal/eal_interrupts.c:800
>         events = 0x7fffefb1ba20
>         nfds = 1
> #5  eal_intr_thread_main (arg=<optimized out>)
>     at /usr/src/packages/BUILD/lib/librte_eal/linuxapp/eal/eal_interrupts.c:870
>         pipe_event = {events = 3, data = {ptr = 0x6, fd = 6, u32 = 6, u64 = 6}}
>         src = <optimized out>
>         numfds = <optimized out>
>         pfd = 12
>         ev = {events = 3, data = {ptr = 0xf7df02e500000005, fd = 5, u32 = 5, 
>             u64 = 17860997829745442821}}
>         __func__ = "eal_intr_thread_main"
> #6  0x00007ffff37eb0a4 in start_thread (arg=0x7fffefb3c700) at pthread_create.c:309
>         __res = <optimized out>
>         pd = 0x7fffefb3c700
>         now = <optimized out>
>         unwind_buf = {cancel_jmp_buf = {{jmp_buf = {140737214924544, 2510814068564645188, 1, 
>                 140737354125408, 140737336548072, 140737214924544, -2510779380161489596, 
>                 -2510806361332034236}, mask_was_saved = 0}}, priv = {pad = {0x0, 0x0, 0x0, 0x0}, 
>             data = {prev = 0x0, cleanup = 0x0, canceltype = 0}}}
>         not_first_call = <optimized out>
>         pagesize_m1 = <optimized out>
>         sp = <optimized out>
>         freesize = <optimized out>
>         __PRETTY_FUNCTION__ = "start_thread"
> #7  0x00007ffff1b8287d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
> 
> It dies in this bit:
> 
> /* Update slave link status */
> (*slave_ethdev->dev_ops->link_update)(slave_ethdev,
> 	internals->slaves[i].link_status_wait_to_complete);
> 
> (gdb) print rte_eth_devices[internals->slaves[i].port_id]
> $7 = {rx_pkt_burst = 0x0, tx_pkt_burst = 0x0, data = 0x0, driver = 0x0, dev_ops = 0x0, {pci_dev = 0x0, 
>     vmbus_dev = 0x0}, link_intr_cbs = {tqh_first = 0x0, tqh_last = 0x0}, post_rx_burst_cbs = {
>     0x0 <repeats 256 times>}, pre_tx_burst_cbs = {0x0 <repeats 256 times>}, attached = 0 '\000', 
>   dev_type = RTE_ETH_DEV_UNKNOWN}
> 
> I'm assuming it's not a simply matter of checking the dev_type or for
> nulls. Do you have any suggestions/insight? I'm delving into the issue,
> but it's the first time I look at the bonding code so any help or
> pointers would be greatly appreciated.
> 
> Note that I also tried to backport the additional patches for reset that
> are currently under review on top of these, but there's no difference.
> But I have not yet used the new reset API in our app though.
> 
> Thanks!

I noticed that we were used 2 of the patches that were self-nacked, and
they were causing the crash. I'll switch to the new version that is
under review instead.

-- 
Kind regards,
Luca Boccassi