DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/e1000: always enable receive and transmit
@ 2017-07-17 19:18 Charles (Chas) Williams
  2017-07-18 11:50 ` Ferruh Yigit
  2017-10-20  3:23 ` [dpdk-dev] [PATCH v2] " Chas Williams
  0 siblings, 2 replies; 5+ messages in thread
From: Charles (Chas) Williams @ 2017-07-17 19:18 UTC (permalink / raw)
  To: dev; +Cc: wenzhuo.lu, Charles (Chas) Williams

The transmit and receive controller state machines are only enabled after
receiving an interrupt and the link status is now valid.  If an adapter
is being used in conjuction with NC-SI, network controller sideband
interface, the adapter may never get a link state change interrupt since
the adapter's PHY is always link up and never changes state.

To fix this, always enable and disable the transmit and receive with
.dev_start and .dev_stop.  This is a better match for what is typically
done with the other PMD's.  Since we may never get an interrupt to check
the link state, we also poll once at the end of .dev_start to get the
current link status.

Signed-off-by: Chas Williams <ciwillia@brocade.com>
---
 drivers/net/e1000/em_ethdev.c  | 44 ++++++++++++++++++++++++++---------------
 drivers/net/e1000/igb_ethdev.c | 45 ++++++++++++++++++++++++++++--------------
 2 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 3d4ab93..11e0bb8 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -586,6 +586,30 @@ em_set_pba(struct e1000_hw *hw)
 	E1000_WRITE_REG(hw, E1000_PBA, pba);
 }
 
+static void
+eth_em_rxtx_control(struct rte_eth_dev *dev,
+		    bool enable)
+{
+	struct e1000_hw *hw =
+		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t tctl, rctl;
+
+	tctl = E1000_READ_REG(hw, E1000_TCTL);
+	rctl = E1000_READ_REG(hw, E1000_RCTL);
+	if (enable) {
+		/* enable Tx/Rx */
+		tctl |= E1000_TCTL_EN;
+		rctl |= E1000_RCTL_EN;
+	} else {
+		/* disable Tx/Rx */
+		tctl &= ~E1000_TCTL_EN;
+		rctl &= ~E1000_RCTL_EN;
+	}
+	E1000_WRITE_REG(hw, E1000_TCTL, tctl);
+	E1000_WRITE_REG(hw, E1000_RCTL, rctl);
+	E1000_WRITE_FLUSH(hw);
+}
+
 static int
 eth_em_start(struct rte_eth_dev *dev)
 {
@@ -754,6 +778,9 @@ eth_em_start(struct rte_eth_dev *dev)
 
 	adapter->stopped = 0;
 
+	eth_em_rxtx_control(dev, true);
+	eth_em_link_update(dev, 0);
+
 	PMD_INIT_LOG(DEBUG, "<<");
 
 	return 0;
@@ -779,6 +806,7 @@ 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;
 
+	eth_em_rxtx_control(dev, false);
 	em_rxq_intr_disable(hw);
 	em_lsc_intr_disable(hw);
 
@@ -1602,7 +1630,6 @@ eth_em_interrupt_action(struct rte_eth_dev *dev,
 		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct e1000_interrupt *intr =
 		E1000_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
-	uint32_t tctl, rctl;
 	struct rte_eth_link link;
 	int ret;
 
@@ -1634,21 +1661,6 @@ eth_em_interrupt_action(struct rte_eth_dev *dev,
 		     pci_dev->addr.domain, pci_dev->addr.bus,
 		     pci_dev->addr.devid, pci_dev->addr.function);
 
-	tctl = E1000_READ_REG(hw, E1000_TCTL);
-	rctl = E1000_READ_REG(hw, E1000_RCTL);
-	if (link.link_status) {
-		/* enable Tx/Rx */
-		tctl |= E1000_TCTL_EN;
-		rctl |= E1000_RCTL_EN;
-	} else {
-		/* disable Tx/Rx */
-		tctl &= ~E1000_TCTL_EN;
-		rctl &= ~E1000_RCTL_EN;
-	}
-	E1000_WRITE_REG(hw, E1000_TCTL, tctl);
-	E1000_WRITE_REG(hw, E1000_RCTL, rctl);
-	E1000_WRITE_FLUSH(hw);
-
 	return 0;
 }
 
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index da03d9b..7e2d31a 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -1300,6 +1300,31 @@ eth_igb_configure(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static void
+eth_igb_rxtx_control(struct rte_eth_dev *dev,
+		     bool enable)
+{
+	struct e1000_hw *hw =
+		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t tctl, rctl;
+
+	tctl = E1000_READ_REG(hw, E1000_TCTL);
+	rctl = E1000_READ_REG(hw, E1000_RCTL);
+
+	if (enable) {
+		/* enable Tx/Rx */
+		tctl |= E1000_TCTL_EN;
+		rctl |= E1000_RCTL_EN;
+	} else {
+		/* disable Tx/Rx */
+		tctl &= ~E1000_TCTL_EN;
+		rctl &= ~E1000_RCTL_EN;
+	}
+	E1000_WRITE_REG(hw, E1000_TCTL, tctl);
+	E1000_WRITE_REG(hw, E1000_RCTL, rctl);
+	E1000_WRITE_FLUSH(hw);
+}
+
 static int
 eth_igb_start(struct rte_eth_dev *dev)
 {
@@ -1496,6 +1521,9 @@ eth_igb_start(struct rte_eth_dev *dev)
 	/* restore all types filter */
 	igb_filter_restore(dev);
 
+	eth_igb_rxtx_control(dev, true);
+	eth_igb_link_update(dev, 0);
+
 	PMD_INIT_LOG(DEBUG, "<<");
 
 	return 0;
@@ -1521,6 +1549,8 @@ eth_igb_stop(struct rte_eth_dev *dev)
 	struct rte_eth_link link;
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
 
+	eth_igb_rxtx_control(dev, false);
+
 	igb_intr_disable(hw);
 
 	/* disable intr eventfd mapping */
@@ -2842,7 +2872,6 @@ eth_igb_interrupt_action(struct rte_eth_dev *dev,
 	struct e1000_interrupt *intr =
 		E1000_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
-	uint32_t tctl, rctl;
 	struct rte_eth_link link;
 	int ret;
 
@@ -2884,20 +2913,6 @@ eth_igb_interrupt_action(struct rte_eth_dev *dev,
 			     pci_dev->addr.bus,
 			     pci_dev->addr.devid,
 			     pci_dev->addr.function);
-		tctl = E1000_READ_REG(hw, E1000_TCTL);
-		rctl = E1000_READ_REG(hw, E1000_RCTL);
-		if (link.link_status) {
-			/* enable Tx/Rx */
-			tctl |= E1000_TCTL_EN;
-			rctl |= E1000_RCTL_EN;
-		} else {
-			/* disable Tx/Rx */
-			tctl &= ~E1000_TCTL_EN;
-			rctl &= ~E1000_RCTL_EN;
-		}
-		E1000_WRITE_REG(hw, E1000_TCTL, tctl);
-		E1000_WRITE_REG(hw, E1000_RCTL, rctl);
-		E1000_WRITE_FLUSH(hw);
 		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC,
 					      NULL, NULL);
 	}
-- 
2.1.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] net/e1000: always enable receive and transmit
  2017-07-17 19:18 [dpdk-dev] [PATCH] net/e1000: always enable receive and transmit Charles (Chas) Williams
@ 2017-07-18 11:50 ` Ferruh Yigit
  2017-10-20  3:23 ` [dpdk-dev] [PATCH v2] " Chas Williams
  1 sibling, 0 replies; 5+ messages in thread
From: Ferruh Yigit @ 2017-07-18 11:50 UTC (permalink / raw)
  To: Charles (Chas) Williams, dev; +Cc: wenzhuo.lu

On 7/17/2017 8:18 PM, Charles (Chas) Williams wrote:
> The transmit and receive controller state machines are only enabled after
> receiving an interrupt and the link status is now valid.  If an adapter
> is being used in conjuction with NC-SI, network controller sideband
> interface, the adapter may never get a link state change interrupt since
> the adapter's PHY is always link up and never changes state.
> 
> To fix this, always enable and disable the transmit and receive with
> .dev_start and .dev_stop.  This is a better match for what is typically
> done with the other PMD's.  Since we may never get an interrupt to check
> the link state, we also poll once at the end of .dev_start to get the
> current link status.

Hi Charles,

Is this patch target 17.08-rc2?
Since this is not a fix, can this be postponed to 17.11?

> 
> Signed-off-by: Chas Williams <ciwillia@brocade.com>

<...>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [dpdk-dev] [PATCH v2] net/e1000: always enable receive and transmit
  2017-07-17 19:18 [dpdk-dev] [PATCH] net/e1000: always enable receive and transmit Charles (Chas) Williams
  2017-07-18 11:50 ` Ferruh Yigit
@ 2017-10-20  3:23 ` Chas Williams
  2018-01-11  8:22   ` [dpdk-dev] [dpdk-dev, " Wenzhuo Lu
  1 sibling, 1 reply; 5+ messages in thread
From: Chas Williams @ 2017-10-20  3:23 UTC (permalink / raw)
  To: dev; +Cc: wenzhuo.lu, Chas Williams

From: Chas Williams <chas3@att.com>

The transmit and receive controller state machines are only enabled after
receiving an interrupt and the link status is now valid.  If an adapter
is being used in conjunction with NC-SI, network controller sideband
interface, the adapter may never get a link state change interrupt since
the adapter's PHY is always link up and never changes state.

To fix this, always enable and disable the transmit and receive with
.dev_start and .dev_stop.  This is a better match for what is typically
done with the other PMD's.  Since we may never get an interrupt to check
the link state, we also poll once at the end of .dev_start to get the
current link status.

Signed-off-by: Chas Williams <chas3@att.com>
---
 drivers/net/e1000/em_ethdev.c  | 44 ++++++++++++++++++++++++++---------------
 drivers/net/e1000/igb_ethdev.c | 45 ++++++++++++++++++++++++++++--------------
 2 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 6ebfa6d..f2493f2 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -587,6 +587,30 @@ em_set_pba(struct e1000_hw *hw)
 	E1000_WRITE_REG(hw, E1000_PBA, pba);
 }
 
+static void
+eth_em_rxtx_control(struct rte_eth_dev *dev,
+		    bool enable)
+{
+	struct e1000_hw *hw =
+		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t tctl, rctl;
+
+	tctl = E1000_READ_REG(hw, E1000_TCTL);
+	rctl = E1000_READ_REG(hw, E1000_RCTL);
+	if (enable) {
+		/* enable Tx/Rx */
+		tctl |= E1000_TCTL_EN;
+		rctl |= E1000_RCTL_EN;
+	} else {
+		/* disable Tx/Rx */
+		tctl &= ~E1000_TCTL_EN;
+		rctl &= ~E1000_RCTL_EN;
+	}
+	E1000_WRITE_REG(hw, E1000_TCTL, tctl);
+	E1000_WRITE_REG(hw, E1000_RCTL, rctl);
+	E1000_WRITE_FLUSH(hw);
+}
+
 static int
 eth_em_start(struct rte_eth_dev *dev)
 {
@@ -755,6 +779,9 @@ eth_em_start(struct rte_eth_dev *dev)
 
 	adapter->stopped = 0;
 
+	eth_em_rxtx_control(dev, true);
+	eth_em_link_update(dev, 0);
+
 	PMD_INIT_LOG(DEBUG, "<<");
 
 	return 0;
@@ -780,6 +807,7 @@ 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;
 
+	eth_em_rxtx_control(dev, false);
 	em_rxq_intr_disable(hw);
 	em_lsc_intr_disable(hw);
 
@@ -1604,7 +1632,6 @@ eth_em_interrupt_action(struct rte_eth_dev *dev,
 		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct e1000_interrupt *intr =
 		E1000_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
-	uint32_t tctl, rctl;
 	struct rte_eth_link link;
 	int ret;
 
@@ -1636,21 +1663,6 @@ eth_em_interrupt_action(struct rte_eth_dev *dev,
 		     pci_dev->addr.domain, pci_dev->addr.bus,
 		     pci_dev->addr.devid, pci_dev->addr.function);
 
-	tctl = E1000_READ_REG(hw, E1000_TCTL);
-	rctl = E1000_READ_REG(hw, E1000_RCTL);
-	if (link.link_status) {
-		/* enable Tx/Rx */
-		tctl |= E1000_TCTL_EN;
-		rctl |= E1000_RCTL_EN;
-	} else {
-		/* disable Tx/Rx */
-		tctl &= ~E1000_TCTL_EN;
-		rctl &= ~E1000_RCTL_EN;
-	}
-	E1000_WRITE_REG(hw, E1000_TCTL, tctl);
-	E1000_WRITE_REG(hw, E1000_RCTL, rctl);
-	E1000_WRITE_FLUSH(hw);
-
 	return 0;
 }
 
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 003bdf0..391bd3e 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -1303,6 +1303,31 @@ eth_igb_configure(struct rte_eth_dev *dev)
 	return 0;
 }
 
+static void
+eth_igb_rxtx_control(struct rte_eth_dev *dev,
+		     bool enable)
+{
+	struct e1000_hw *hw =
+		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t tctl, rctl;
+
+	tctl = E1000_READ_REG(hw, E1000_TCTL);
+	rctl = E1000_READ_REG(hw, E1000_RCTL);
+
+	if (enable) {
+		/* enable Tx/Rx */
+		tctl |= E1000_TCTL_EN;
+		rctl |= E1000_RCTL_EN;
+	} else {
+		/* disable Tx/Rx */
+		tctl &= ~E1000_TCTL_EN;
+		rctl &= ~E1000_RCTL_EN;
+	}
+	E1000_WRITE_REG(hw, E1000_TCTL, tctl);
+	E1000_WRITE_REG(hw, E1000_RCTL, rctl);
+	E1000_WRITE_FLUSH(hw);
+}
+
 static int
 eth_igb_start(struct rte_eth_dev *dev)
 {
@@ -1501,6 +1526,9 @@ eth_igb_start(struct rte_eth_dev *dev)
 	/* restore all types filter */
 	igb_filter_restore(dev);
 
+	eth_igb_rxtx_control(dev, true);
+	eth_igb_link_update(dev, 0);
+
 	PMD_INIT_LOG(DEBUG, "<<");
 
 	return 0;
@@ -1526,6 +1554,8 @@ eth_igb_stop(struct rte_eth_dev *dev)
 	struct rte_eth_link link;
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
 
+	eth_igb_rxtx_control(dev, false);
+
 	igb_intr_disable(hw);
 
 	/* disable intr eventfd mapping */
@@ -2854,7 +2884,6 @@ eth_igb_interrupt_action(struct rte_eth_dev *dev,
 	struct e1000_interrupt *intr =
 		E1000_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
-	uint32_t tctl, rctl;
 	struct rte_eth_link link;
 	int ret;
 
@@ -2896,20 +2925,6 @@ eth_igb_interrupt_action(struct rte_eth_dev *dev,
 			     pci_dev->addr.bus,
 			     pci_dev->addr.devid,
 			     pci_dev->addr.function);
-		tctl = E1000_READ_REG(hw, E1000_TCTL);
-		rctl = E1000_READ_REG(hw, E1000_RCTL);
-		if (link.link_status) {
-			/* enable Tx/Rx */
-			tctl |= E1000_TCTL_EN;
-			rctl |= E1000_RCTL_EN;
-		} else {
-			/* disable Tx/Rx */
-			tctl &= ~E1000_TCTL_EN;
-			rctl &= ~E1000_RCTL_EN;
-		}
-		E1000_WRITE_REG(hw, E1000_TCTL, tctl);
-		E1000_WRITE_REG(hw, E1000_RCTL, rctl);
-		E1000_WRITE_FLUSH(hw);
 		_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC,
 					      NULL, NULL);
 	}
-- 
2.9.5

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [dpdk-dev, v2] net/e1000: always enable receive and transmit
  2017-10-20  3:23 ` [dpdk-dev] [PATCH v2] " Chas Williams
@ 2018-01-11  8:22   ` Wenzhuo Lu
  2018-01-13  6:33     ` Zhang, Helin
  0 siblings, 1 reply; 5+ messages in thread
From: Wenzhuo Lu @ 2018-01-11  8:22 UTC (permalink / raw)
  To: chas3, dev

Hi,

> Date: Thu, 19 Oct 2017 23:23:39 -0400
>
> From: Chas Williams <chas3@att.com>
>
> The transmit and receive controller state machines are only enabled after
> receiving an interrupt and the link status is now valid.  If an adapter
> is being used in conjunction with NC-SI, network controller sideband
> interface, the adapter may never get a link state change interrupt since
> the adapter's PHY is always link up and never changes state.
>
> To fix this, always enable and disable the transmit and receive with
> .dev_start and .dev_stop.  This is a better match for what is typically
> done with the other PMD's.  Since we may never get an interrupt to check
> the link state, we also poll once at the end of .dev_start to get the
> current link status.
>
> Signed-off-by: Chas Williams <chas3@att.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [dpdk-dev, v2] net/e1000: always enable receive and transmit
  2018-01-11  8:22   ` [dpdk-dev] [dpdk-dev, " Wenzhuo Lu
@ 2018-01-13  6:33     ` Zhang, Helin
  0 siblings, 0 replies; 5+ messages in thread
From: Zhang, Helin @ 2018-01-13  6:33 UTC (permalink / raw)
  To: Lu, Wenzhuo, chas3, dev



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Thursday, January 11, 2018 4:23 PM
> To: chas3@att.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-dev, v2] net/e1000: always enable receive and
> transmit
> 
> Hi,
> 
> > Date: Thu, 19 Oct 2017 23:23:39 -0400
> >
> > From: Chas Williams <chas3@att.com>
> >
> > The transmit and receive controller state machines are only enabled
> > after receiving an interrupt and the link status is now valid.  If an
> > adapter is being used in conjunction with NC-SI, network controller
> > sideband interface, the adapter may never get a link state change
> > interrupt since the adapter's PHY is always link up and never changes state.
> >
> > To fix this, always enable and disable the transmit and receive with
> > .dev_start and .dev_stop.  This is a better match for what is
> > typically done with the other PMD's.  Since we may never get an
> > interrupt to check the link state, we also poll once at the end of
> > .dev_start to get the current link status.
> >
> > Signed-off-by: Chas Williams <chas3@att.com>
> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Applied to dpdk-next-net-intel, thanks!

/Helin

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-01-13  6:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-17 19:18 [dpdk-dev] [PATCH] net/e1000: always enable receive and transmit Charles (Chas) Williams
2017-07-18 11:50 ` Ferruh Yigit
2017-10-20  3:23 ` [dpdk-dev] [PATCH v2] " Chas Williams
2018-01-11  8:22   ` [dpdk-dev] [dpdk-dev, " Wenzhuo Lu
2018-01-13  6:33     ` Zhang, Helin

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).