Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e5df49d5 authored by Elad Kanfi's avatar Elad Kanfi Committed by David S. Miller
Browse files

net: nps_enet: Tx handler synchronization 



Below is a description of a possible problematic
sequence. CPU-A is sending a frame and CPU-B handles
the interrupt that indicates the frame was sent. CPU-B
reads an invalid value of tx_packet_sent.

	CPU-A				CPU-B
	-----				-----
	nps_enet_send_frame
	.
	.
	tx_skb = skb
	tx_packet_sent = true
	order HW to start tx
	.
	.
	HW complete tx
			    ------> 	get tx complete interrupt
					.
					.
					if(tx_packet_sent == true)
						handle tx_skb

	end memory transaction
	(tx_packet_sent actually
	 written)

Furthermore there is a dependency between tx_skb and tx_packet_sent.
There is no assurance that tx_skb contains a valid pointer at CPU B
when it sees tx_packet_sent == true.

Solution:

Initialize tx_skb to NULL and use it to indicate that packet was sent,
in this way tx_packet_sent can be removed.
Add a write memory barrier after setting tx_skb in order to make sure
that it is valid before HW is informed and IRQ is fired.

Fixed sequence will be:

       CPU-A                           CPU-B
       -----                           -----

	tx_skb = skb
	wmb()
	.
	.
	order HW to start tx
	.
	.
	HW complete tx
			------>		get tx complete interrupt
					.
					.
					if(tx_skb != NULL)
						handle tx_skb

					tx_skb = NULL

Signed-off-by: default avatarElad Kanfi <eladkan@mellanox.com>
Acked-by: default avatarNoam Camus <noamca@mellanox.com>
Acked-by: default avatarGilad Ben-Yossef <giladby@mellanox.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent d99079e2

drivers/net/ethernet/ezchip/nps_enet.c

+9 −6
Original line number Diff line number Diff line
@@ -145,7 +145,7 @@ static void nps_enet_tx_handler(struct net_device *ndev) 
	u32 tx_ctrl_nt = (tx_ctrl_value & TX_CTL_NT_MASK) >> TX_CTL_NT_SHIFT;



	/* Check if we got TX */

	if (!priv->tx_packet_sent || tx_ctrl_ct)

	if (!priv->tx_skb || tx_ctrl_ct)

		return;



	/* Ack Tx ctrl register */
@@ -160,7 +160,7 @@ static void nps_enet_tx_handler(struct net_device *ndev) 
	}



	dev_kfree_skb(priv->tx_skb);

	priv->tx_packet_sent = false;

	priv->tx_skb = NULL;



	if (netif_queue_stopped(ndev))

		netif_wake_queue(ndev);
@@ -217,7 +217,7 @@ static irqreturn_t nps_enet_irq_handler(s32 irq, void *dev_instance) 
	u32 tx_ctrl_ct = (tx_ctrl_value & TX_CTL_CT_MASK) >> TX_CTL_CT_SHIFT;

	u32 rx_ctrl_cr = (rx_ctrl_value & RX_CTL_CR_MASK) >> RX_CTL_CR_SHIFT;



	if ((!tx_ctrl_ct && priv->tx_packet_sent) || rx_ctrl_cr)

	if ((!tx_ctrl_ct && priv->tx_skb) || rx_ctrl_cr)

		if (likely(napi_schedule_prep(&priv->napi))) {

			nps_enet_reg_set(priv, NPS_ENET_REG_BUF_INT_ENABLE, 0);

			__napi_schedule(&priv->napi);
@@ -387,8 +387,6 @@ static void nps_enet_send_frame(struct net_device *ndev, 
	/* Write the length of the Frame */

	tx_ctrl_value |= length << TX_CTL_NT_SHIFT;



	/* Indicate SW is done */

	priv->tx_packet_sent = true;

	tx_ctrl_value |= NPS_ENET_ENABLE << TX_CTL_CT_SHIFT;

	/* Send Frame */

	nps_enet_reg_set(priv, NPS_ENET_REG_TX_CTL, tx_ctrl_value);
@@ -465,7 +463,7 @@ static s32 nps_enet_open(struct net_device *ndev) 
	s32 err;



	/* Reset private variables */

	priv->tx_packet_sent = false;

	priv->tx_skb = NULL;

	priv->ge_mac_cfg_2_value = 0;

	priv->ge_mac_cfg_3_value = 0;
@@ -534,6 +532,11 @@ static netdev_tx_t nps_enet_start_xmit(struct sk_buff *skb, 


	priv->tx_skb = skb;



	/* make sure tx_skb is actually written to the memory

	 * before the HW is informed and the IRQ is fired.

	 */

	wmb();



	nps_enet_send_frame(ndev, skb);



	return NETDEV_TX_OK;

drivers/net/ethernet/ezchip/nps_enet.h

+0 −2
Original line number Diff line number Diff line
@@ -165,14 +165,12 @@ 
 * struct nps_enet_priv - Storage of ENET's private information.

 * @regs_base:      Base address of ENET memory-mapped control registers.

 * @irq:            For RX/TX IRQ number.

 * @tx_packet_sent: SW indication if frame is being sent.

 * @tx_skb:         socket buffer of sent frame.

 * @napi:           Structure for NAPI.

 */

struct nps_enet_priv {

	void __iomem *regs_base;

	s32 irq;

	bool tx_packet_sent;

	struct sk_buff *tx_skb;

	struct napi_struct napi;

	u32 ge_mac_cfg_2_value;