Skip to contentSkip to conversation

net/e1000: always enable receive and transmit

PATCH In review Charles (Chas) Williams 5 participants 5 messages Jul 17, 2017 – Jan 13, 2018 Network drivers

lore
Share & tools
lore

Charles (Chas) Williams

author
#0 lore raw

PATCH 1/1 net/e1000: always enable receive and transmit

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 <[email protected]>
---
 drivers/net/e1000/em_ethdev.c  | 44 ++++++++++++++++++++++++++---------------
 drivers/net/e1000/igb_ethdev.c | 45 ++++++++++++++++++++++++++++--------------
 2 files changed, 58 insertions(+), 31 deletions(-)
drivers/net/e1000/em_ethdev.c +28 -16
index 3d4ab93..11e0bb8 100644
@@ -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;
 }
drivers/net/e1000/igb_ethdev.c +30 -15

75 lines not on this page · raw message

Signature · 1 line
2.1.4
#1 lore raw
Quoted 11 lines · 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.

Hi Charles,

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

Quoted 2 lines
Signed-off-by Chas Williams <[email protected]>

<...>

#2 lore raw

PATCH v2 1/1 net/e1000: always enable receive and transmit

In reply to Charles (Chas) Williams

From: Chas Williams <[email protected]>

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 <[email protected]>
---
 drivers/net/e1000/em_ethdev.c  | 44 ++++++++++++++++++++++++++---------------
 drivers/net/e1000/igb_ethdev.c | 45 ++++++++++++++++++++++++++++--------------
 2 files changed, 58 insertions(+), 31 deletions(-)
drivers/net/e1000/em_ethdev.c +28 -16
index 6ebfa6d..f2493f2 100644
@@ -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;
 }
drivers/net/e1000/igb_ethdev.c +30 -15

75 lines not on this page · raw message

Signature · 1 line
2.9.5
Acked-by on v2 1/1
#3 lore raw

Hi,

Quoted 10 lines

Date: Thu, 19 Oct 2017 23:23:39 -0400

From: Chas Williams <[email protected]>

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.

Quoted 7 lines

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 <[email protected]>
Acked-by Wenzhuo Lu <[email protected]>
Applied
#4 lore raw
Quoted 21 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Wenzhuo Lu
Sent: Thursday, January 11, 2018 4:23 PM
To: [email protected]; [email protected]
Subject: Re: [dpdk-dev] [dpdk-dev, v2] net/e1000: always enable receive and transmit

Hi,

Quoted 10 lines

Date: Thu, 19 Oct 2017 23:23:39 -0400

From: Chas Williams <[email protected]>

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.

Quoted 8 lines
Quoted 7 lines

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 <[email protected]>
Acked-by Wenzhuo Lu <[email protected]>

Applied to dpdk-next-net-intel, thanks!

/Helin