Netdev List
 help / color / mirror / Atom feed
* [PATCH 1/1 net-next] NFC: llcp: use sizeof(*sdres) instead of nfc_llcp_sdp_tlv
From: Fabian Frederick @ 2014-10-15 19:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Fabian Frederick, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, David S. Miller, linux-wireless, netdev

 See Documentation/CodingStyle Chapter 14

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/nfc/llcp_commands.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
index a3ad69a..a450075 100644
--- a/net/nfc/llcp_commands.c
+++ b/net/nfc/llcp_commands.c
@@ -120,7 +120,7 @@ struct nfc_llcp_sdp_tlv *nfc_llcp_build_sdres_tlv(u8 tid, u8 sap)
 	struct nfc_llcp_sdp_tlv *sdres;
 	u8 value[2];
 
-	sdres = kzalloc(sizeof(struct nfc_llcp_sdp_tlv), GFP_KERNEL);
+	sdres = kzalloc(sizeof(*sdres), GFP_KERNEL);
 	if (sdres == NULL)
 		return NULL;
 
@@ -149,7 +149,7 @@ struct nfc_llcp_sdp_tlv *nfc_llcp_build_sdreq_tlv(u8 tid, char *uri,
 
 	pr_debug("uri: %s, len: %zu\n", uri, uri_len);
 
-	sdreq = kzalloc(sizeof(struct nfc_llcp_sdp_tlv), GFP_KERNEL);
+	sdreq = kzalloc(sizeof(*sdreq), GFP_KERNEL);
 	if (sdreq == NULL)
 		return NULL;
 
-- 
1.9.3

^ permalink raw reply related

* [PATCH 1/1 net-next] NFC: llcp: remove bool comparison on device
From: Fabian Frederick @ 2014-10-15 19:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Fabian Frederick, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, David S. Miller, linux-wireless, netdev

Fixes coccinelle warning:
net/nfc/llcp_core.c:132:5-11: WARNING: Comparison to bool

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/nfc/llcp_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index 51e7887..978399d 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -129,7 +129,7 @@ static void nfc_llcp_socket_release(struct nfc_llcp_local *local, bool device,
 	write_unlock(&local->sockets.lock);
 
 	/* If we still have a device, we keep the RAW sockets alive */
-	if (device == true)
+	if (device)
 		return;
 
 	write_lock(&local->raw_sockets.lock);
-- 
1.9.3

^ permalink raw reply related

* [PATCH 1/1 net-next] NFC: digital: use sizeof(*target) instead of nfc_target
From: Fabian Frederick @ 2014-10-15 19:01 UTC (permalink / raw)
  To: linux-kernel
  Cc: Fabian Frederick, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, David S. Miller, linux-wireless, netdev

See Documentation/CodingStyle Chapter 14

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/nfc/digital_technology.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/nfc/digital_technology.c b/net/nfc/digital_technology.c
index fb58ed2d..5e94b6f 100644
--- a/net/nfc/digital_technology.c
+++ b/net/nfc/digital_technology.c
@@ -494,7 +494,7 @@ static void digital_in_recv_sens_res(struct nfc_digital_dev *ddev, void *arg,
 		goto exit;
 	}
 
-	target = kzalloc(sizeof(struct nfc_target), GFP_KERNEL);
+	target = kzalloc(sizeof(*target), GFP_KERNEL);
 	if (!target) {
 		rc = -ENOMEM;
 		goto exit;
@@ -693,7 +693,7 @@ static void digital_in_recv_sensb_res(struct nfc_digital_dev *ddev, void *arg,
 	else
 		ddev->target_fsc = digital_ats_fsc[fsci];
 
-	target = kzalloc(sizeof(struct nfc_target), GFP_KERNEL);
+	target = kzalloc(sizeof(*target), GFP_KERNEL);
 	if (!target) {
 		rc = -ENOMEM;
 		goto exit;
-- 
1.9.3

^ permalink raw reply related

* [PATCH 1/1 net-next] NFC: digital: remove redundant memory message
From: Fabian Frederick @ 2014-10-15 19:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: Fabian Frederick, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, David S. Miller, linux-wireless, netdev

Let MM subsystem display out of memory messages.

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/nfc/digital_core.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/nfc/digital_core.c b/net/nfc/digital_core.c
index 009bcf3..41ba70c 100644
--- a/net/nfc/digital_core.c
+++ b/net/nfc/digital_core.c
@@ -697,10 +697,8 @@ static int digital_in_send(struct nfc_dev *nfc_dev, struct nfc_target *target,
 	int rc;
 
 	data_exch = kzalloc(sizeof(struct digital_data_exch), GFP_KERNEL);
-	if (!data_exch) {
-		pr_err("Failed to allocate data_exch struct\n");
+	if (!data_exch)
 		return -ENOMEM;
-	}
 
 	data_exch->cb = cb;
 	data_exch->cb_context = cb_context;
-- 
1.9.3

^ permalink raw reply related

* [PATCH 1/1 net-next] HCI: remove comparisons on shdlc boolean members.
From: Fabian Frederick @ 2014-10-15 19:00 UTC (permalink / raw)
  To: linux-kernel
  Cc: Fabian Frederick, Lauro Ramos Venancio, Aloisio Almeida Jr,
	Samuel Ortiz, David S. Miller, linux-wireless, netdev

Fixes coccinelles warnings:
net/nfc/hci/llc_shdlc.c:539:7-17: WARNING: Comparison to bool
net/nfc/hci/llc_shdlc.c:544:9-19: WARNING: Comparison to bool
net/nfc/hci/llc_shdlc.c:574:6-22: WARNING: Comparison to bool
net/nfc/hci/llc_shdlc.c:250:5-21: WARNING: Comparison to bool
net/nfc/hci/llc_shdlc.c:333:6-16: WARNING: Comparison to bool
net/nfc/hci/llc_shdlc.c:431:31-43: WARNING: Comparison to bool

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/nfc/hci/llc_shdlc.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/nfc/hci/llc_shdlc.c b/net/nfc/hci/llc_shdlc.c
index 401c7e2..76e6fa2 100644
--- a/net/nfc/hci/llc_shdlc.c
+++ b/net/nfc/hci/llc_shdlc.c
@@ -247,7 +247,7 @@ static void llc_shdlc_rcv_i_frame(struct llc_shdlc *shdlc,
 		goto exit;
 	}
 
-	if (shdlc->t1_active == false) {
+	if (!shdlc->t1_active) {
 		shdlc->t1_active = true;
 		mod_timer(&shdlc->t1_timer, jiffies +
 			  msecs_to_jiffies(SHDLC_T1_VALUE_MS(shdlc->w)));
@@ -330,7 +330,7 @@ static void llc_shdlc_rcv_s_frame(struct llc_shdlc *shdlc,
 	switch (s_frame_type) {
 	case S_FRAME_RR:
 		llc_shdlc_rcv_ack(shdlc, nr);
-		if (shdlc->rnr == true) {	/* see SHDLC 10.7.7 */
+		if (shdlc->rnr) {	/* see SHDLC 10.7.7 */
 			shdlc->rnr = false;
 			if (shdlc->send_q.qlen == 0) {
 				skb = llc_shdlc_alloc_skb(shdlc, 0);
@@ -428,7 +428,7 @@ static void llc_shdlc_rcv_u_frame(struct llc_shdlc *shdlc,
 					       false;
 
 			if ((w <= SHDLC_MAX_WINDOW) &&
-			    (SHDLC_SREJ_SUPPORT || (srej_support == false))) {
+			    (SHDLC_SREJ_SUPPORT || (!srej_support))) {
 				shdlc->w = w;
 				shdlc->srej_support = srej_support;
 				r = llc_shdlc_connect_send_ua(shdlc);
@@ -536,12 +536,12 @@ static void llc_shdlc_handle_send_queue(struct llc_shdlc *shdlc)
 		pr_debug
 		    ("sendQlen=%d ns=%d dnr=%d rnr=%s w_room=%d unackQlen=%d\n",
 		     shdlc->send_q.qlen, shdlc->ns, shdlc->dnr,
-		     shdlc->rnr == false ? "false" : "true",
+		     !shdlc->rnr ? "false" : "true",
 		     shdlc->w - llc_shdlc_w_used(shdlc->ns, shdlc->dnr),
 		     shdlc->ack_pending_q.qlen);
 
 	while (shdlc->send_q.qlen && shdlc->ack_pending_q.qlen < shdlc->w &&
-	       (shdlc->rnr == false)) {
+	       (!shdlc->rnr)) {
 
 		if (shdlc->t1_active) {
 			del_timer_sync(&shdlc->t1_timer);
@@ -571,7 +571,7 @@ static void llc_shdlc_handle_send_queue(struct llc_shdlc *shdlc)
 
 		skb_queue_tail(&shdlc->ack_pending_q, skb);
 
-		if (shdlc->t2_active == false) {
+		if (!shdlc->t2_active) {
 			shdlc->t2_active = true;
 			mod_timer(&shdlc->t2_timer, time_sent +
 				  msecs_to_jiffies(SHDLC_T2_VALUE_MS));
-- 
1.9.3

^ permalink raw reply related

* Re: ixgbe: Question about Flow Control on 10G
From: dom @ 2014-10-15 18:56 UTC (permalink / raw)
  To: Tantilov, Emil S, Skidmore, Donald C, netdev@vger.kernel.org
In-Reply-To: <87618083B2453E4A8714035B62D67992500E1865@FMSMSX105.amr.corp.intel.com>

On 10/14/2014 01:54 PM, Tantilov, Emil S wrote:
>> -----Original Message-----
>> From: netdev-owner@vger.kernel.org [mailto:netdev-
>> owner@vger.kernel.org] On Behalf Of dom
>> Sent: Tuesday, October 14, 2014 12:01 PM
>> To: Skidmore, Donald C; netdev@vger.kernel.org
>> Subject: ixgbe: Question about Flow Control on 10G
>>
>> Hi
>>
>> I have a question about the ixgbe driver's handling of
>> 'ethtool -a ethX'
>> when the NIC is using fibre.
>>
>> Specifically I don't understand the code introduced by this
>> commit:
>>
>> commit 73d80953dfd1d5a92948005798c857c311c2834b
>> Author: Don Skidmore <donald.c.skidmore@intel.com>
>> Date:   Wed Jul 31 02:19:24 2013 +0000
>> Subject: ixgbe: fix fc autoneg ethtool reporting.
>>
>> The function introduced the function:
>>         ixgbe_device_supports_autoneg_fc()
>>
>> which gets called by
>> ixgbe_get_pauseparam()/ixgbe_set_pauseparam().
>>
>> specifically there is a  case in
>> ixgbe_device_supports_autoneg_fc()
>>
>>      case ixgbe_media_type_fiber_qsfp:
>>      case ixgbe_media_type_fiber:
>>          hw->mac.ops.check_link(hw, &speed, &link_up,
>> false);
>>          /* if link is down, assume supported */
>>          if (link_up)
>>              supported = speed == IXGBE_LINK_SPEED_1GB_FULL ?
>>                  true : false;
>>
>> If link_up=1 then why is supported only true for a
>> speed=IXGBE_LINK_SPEED_1GB_FULL ?
>>
>> Why is Flow Control not supported for IXGBE_LINK_SPEED_10GB_FULL ?
> For SFP modules (media_type_fiber) flow control autoneg is not supported at 10gig. You can still set flow control manually to enabled/disabled, just not autoneg.
>
> Thanks,
> Emil
Hi Emil

Thank you for the quick answer.  I have a one follow-up question if I may...

We noticed that back in 3.2.9 (before 73d80953dfd basically) the 
behaviour was different for 10G fibre.  i.e.  autonegotiate showed 'on'.

# ethtool -a eth1
Pause parameters for eth1:
Autonegotiate:  on
RX:             on
TX:             on


The code:
     if (hw->fc.disable_fc_autoneg ||
         (hw->fc.current_mode == ixgbe_fc_none))
         pause->autoneg = 0;
     else
         pause->autoneg = 1;

So I assume this old output from 'ethtool -a' for autogen was just 
wrong, is that correct ?

[I'm asking cos I _know_ my h/w collegues are going to ask why the change.]

Thanks again
dom

^ permalink raw reply

* Re: [PATCHv4 RFC net-next 1/4] sunvnet: NAPIfy sunvnet
From: David L Stevens @ 2014-10-15 18:51 UTC (permalink / raw)
  To: Sowmini Varadhan, davem, bob.picco, dwight.engen,
	raghuram.kothakota; +Cc: netdev
In-Reply-To: <20141015180115.GH11840@oracle.com>



On 10/15/2014 02:01 PM, Sowmini Varadhan wrote:

> @@ -482,13 +485,26 @@ static int vnet_walk_rx(struct vnet_port *port, struct vio_dring_state *dr,
>  				return err;
>  			ack_start = -1;
>  		}
> +		if ((*npkts) >= budget) {
> +			send_ack = false;
> +			break;
> +		}
>  	}

I still don't like this-- it's virtually "boolean_variable = true;if (condition) boolean_variable = false".
Yes, it has the extra "break;", but I think it's clearer to say "send an ACK when
we're under budget; don't send an ACK and break when we're over," or:

		send_ack = *npkts < budget;
		if (!send_ack)
			break;

I can live with either way, though.

Acked-by: David L Stevens <david.stevens@oracle.com>

						+-DLS

^ permalink raw reply

* Re: [net-next.git 0/3] stmmac: review driver Koptions
From: David Miller @ 2014-10-15 18:36 UTC (permalink / raw)
  To: peppe.cavallaro; +Cc: netdev
In-Reply-To: <1413361531-9182-1-git-send-email-peppe.cavallaro@st.com>

From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Date: Wed, 15 Oct 2014 10:25:28 +0200

> These patches are to rework some Koption used for configuring
> the driver. Recently many options have been added and the main
> goal behind this work is to guarantee that the driver built fine
> on all the branches where it is present. All the Koption for
> platform drivers can be safely removed (for example never added
> for SPEAr). In that case, it will be possible to validate the
> build for all the platforms like STi avoiding failures.
> The patches have been tested on STi Boxes w/o issue at runtime.

Please resubmit these when the net-next tree re-opens.

Thank you.

^ permalink raw reply

* [PATCHv4 RFC net-next 4/4] sunvnet: Remove irqsave/irqrestore on vio.lock
From: Sowmini Varadhan @ 2014-10-15 18:03 UTC (permalink / raw)
  To: davem, sowmini.varadhan; +Cc: netdev


After the  NAPIfication of sunvnet, we no longer need to
synchronize by doing irqsave/restore on vio.lock in the
I/O fastpath.

NAPI ->poll() is non-reentrant, so all RX processing occurs
strictly in a serialized environment. TX reclaim is done in NAPI
context, so the netif_tx_lock can be used to serialize
critical sections between Tx and Rx paths.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
 drivers/net/ethernet/sun/sunvnet.c | 30 +++++-------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index 055061d..c1c5820 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -838,18 +838,6 @@ struct vnet_port *__tx_port_find(struct vnet *vp, struct sk_buff *skb)
 	return NULL;
 }
 
-struct vnet_port *tx_port_find(struct vnet *vp, struct sk_buff *skb)
-{
-	struct vnet_port *ret;
-	unsigned long flags;
-
-	spin_lock_irqsave(&vp->lock, flags);
-	ret = __tx_port_find(vp, skb);
-	spin_unlock_irqrestore(&vp->lock, flags);
-
-	return ret;
-}
-
 static struct sk_buff *vnet_clean_tx_ring(struct vnet_port *port,
 					  unsigned *pending)
 {
@@ -910,11 +898,10 @@ static void vnet_clean_timer_expire(unsigned long port0)
 	struct vnet_port *port = (struct vnet_port *)port0;
 	struct sk_buff *freeskbs;
 	unsigned pending;
-	unsigned long flags;
 
-	spin_lock_irqsave(&port->vio.lock, flags);
+	netif_tx_lock(port->vp->dev);
 	freeskbs = vnet_clean_tx_ring(port, &pending);
-	spin_unlock_irqrestore(&port->vio.lock, flags);
+	netif_tx_unlock(port->vp->dev);
 
 	vnet_free_skbs(freeskbs);
 
@@ -967,7 +954,6 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct vnet_port *port = NULL;
 	struct vio_dring_state *dr;
 	struct vio_net_desc *d;
-	unsigned long flags;
 	unsigned int len;
 	struct sk_buff *freeskbs = NULL;
 	int i, err, txi;
@@ -980,7 +966,7 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		goto out_dropped;
 
 	rcu_read_lock();
-	port = tx_port_find(vp, skb);
+	port = __tx_port_find(vp, skb);
 	if (unlikely(!port))
 		goto out_dropped;
 
@@ -1017,8 +1003,6 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		goto out_dropped;
 	}
 
-	spin_lock_irqsave(&port->vio.lock, flags);
-
 	dr = &port->vio.drings[VIO_DRIVER_TX_RING];
 	if (unlikely(vnet_tx_dring_avail(dr) < 2)) {
 		if (!netif_queue_stopped(dev)) {
@@ -1052,7 +1036,7 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			     (LDC_MAP_SHADOW | LDC_MAP_DIRECT | LDC_MAP_RW));
 	if (err < 0) {
 		netdev_info(dev, "tx buffer map error %d\n", err);
-		goto out_dropped_unlock;
+		goto out_dropped;
 	}
 	port->tx_bufs[txi].ncookies = err;
 
@@ -1105,7 +1089,7 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		netdev_info(dev, "TX trigger error %d\n", err);
 		d->hdr.state = VIO_DESC_FREE;
 		dev->stats.tx_carrier_errors++;
-		goto out_dropped_unlock;
+		goto out_dropped;
 	}
 
 ldc_start_done:
@@ -1121,7 +1105,6 @@ ldc_start_done:
 			netif_wake_queue(dev);
 	}
 
-	spin_unlock_irqrestore(&port->vio.lock, flags);
 	(void)mod_timer(&port->clean_timer, jiffies + VNET_CLEAN_TIMEOUT);
 	rcu_read_unlock();
 
@@ -1129,9 +1112,6 @@ ldc_start_done:
 
 	return NETDEV_TX_OK;
 
-out_dropped_unlock:
-	spin_unlock_irqrestore(&port->vio.lock, flags);
-
 out_dropped:
 	if (pending)
 		(void)mod_timer(&port->clean_timer,
-- 
1.8.4.2

^ permalink raw reply related

* [PATCHv4 RFC 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq()
From: Sowmini Varadhan @ 2014-10-15 18:02 UTC (permalink / raw)
  To: davem, sowmini.varadhan; +Cc: netdev, sparclinux

For NAPIfied drivers , there is no need to
synchronize by doing irqsave/restore on vio.lock in the I/O
path.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
 arch/sparc/kernel/viohs.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/sparc/kernel/viohs.c b/arch/sparc/kernel/viohs.c
index 7ef081a..d731586 100644
--- a/arch/sparc/kernel/viohs.c
+++ b/arch/sparc/kernel/viohs.c
@@ -747,10 +747,11 @@ EXPORT_SYMBOL(vio_ldc_free);
 
 void vio_port_up(struct vio_driver_state *vio)
 {
-	unsigned long flags;
+	unsigned long flags = 0;
 	int err, state;
 
-	spin_lock_irqsave(&vio->lock, flags);
+	if (!in_softirq())
+		spin_lock_irqsave(&vio->lock, flags);
 
 	state = ldc_state(vio->lp);
 
@@ -777,7 +778,8 @@ void vio_port_up(struct vio_driver_state *vio)
 		mod_timer(&vio->timer, expires);
 	}
 
-	spin_unlock_irqrestore(&vio->lock, flags);
+	if (!in_softirq())
+		spin_unlock_irqrestore(&vio->lock, flags);
 }
 EXPORT_SYMBOL(vio_port_up);
 
-- 
1.8.4.2


^ permalink raw reply related

* [PATCHv4 RFC net-next 2/4] sunvnet: Use RCU to synchronize port usage with vnet_port_remove()
From: Sowmini Varadhan @ 2014-10-15 18:01 UTC (permalink / raw)
  To: davem, bob.picco, sowmini.varadhan, dwight.engen, david.stevens; +Cc: netdev


A vnet_port_remove could be triggered as a result of an ldm-unbind
operation by the peer, module unload, or other changes to the
inter-vnet-link configuration.  When this is concurrent with
vnet_start_xmit(), there are several race sequences possible,
such as

thread 1                                    thread 2
vnet_start_xmit
-> tx_port_find
   spin_lock_irqsave(&vp->lock..)
   ret = __tx_port_find(..)
   spin_lock_irqrestore(&vp->lock..)
                                           vio_remove -> ..
                                               ->vnet_port_remove
                                           spin_lock_irqsave(&vp->lock..)
                                           cleanup
                                           spin_lock_irqrestore(&vp->lock..)
                                           kfree(port)
/* attempt to use ret will bomb */

This patch adds RCU locking for port access so that vnet_port_remove
will correctly clean up port-related state.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Acked-by: Dwight Engen <dwight.engen@oracle.com>
Acked-by: Bob Picco <bob.picco@oracle.com>
---
changes since v2: use RCU.
changes since v3: incorporate David Stevens feedback

 drivers/net/ethernet/sun/sunvnet.c | 65 ++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index 4a4b482..055061d 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -619,7 +619,8 @@ static void maybe_tx_wakeup(struct vnet *vp)
 		struct vnet_port *port;
 		int wake = 1;
 
-		list_for_each_entry(port, &vp->port_list, list) {
+		rcu_read_lock();
+		list_for_each_entry_rcu(port, &vp->port_list, list) {
 			struct vio_dring_state *dr;
 
 			dr = &port->vio.drings[VIO_DRIVER_TX_RING];
@@ -629,6 +630,7 @@ static void maybe_tx_wakeup(struct vnet *vp)
 				break;
 			}
 		}
+		rcu_read_unlock();
 		if (wake)
 			netif_wake_queue(dev);
 	}
@@ -820,13 +822,13 @@ struct vnet_port *__tx_port_find(struct vnet *vp, struct sk_buff *skb)
 	struct hlist_head *hp = &vp->port_hash[hash];
 	struct vnet_port *port;
 
-	hlist_for_each_entry(port, hp, hash) {
+	hlist_for_each_entry_rcu(port, hp, hash) {
 		if (!port_is_up(port))
 			continue;
 		if (ether_addr_equal(port->raddr, skb->data))
 			return port;
 	}
-	list_for_each_entry(port, &vp->port_list, list) {
+	list_for_each_entry_rcu(port, &vp->port_list, list) {
 		if (!port->switch_port)
 			continue;
 		if (!port_is_up(port))
@@ -962,7 +964,7 @@ static inline struct sk_buff *vnet_skb_shape(struct sk_buff *skb, void **pstart,
 static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct vnet *vp = netdev_priv(dev);
-	struct vnet_port *port = tx_port_find(vp, skb);
+	struct vnet_port *port = NULL;
 	struct vio_dring_state *dr;
 	struct vio_net_desc *d;
 	unsigned long flags;
@@ -973,14 +975,15 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	int nlen = 0;
 	unsigned pending = 0;
 
-	if (unlikely(!port))
-		goto out_dropped;
-
 	skb = vnet_skb_shape(skb, &start, &nlen);
-
 	if (unlikely(!skb))
 		goto out_dropped;
 
+	rcu_read_lock();
+	port = tx_port_find(vp, skb);
+	if (unlikely(!port))
+		goto out_dropped;
+
 	if (skb->len > port->rmtu) {
 		unsigned long localmtu = port->rmtu - ETH_HLEN;
 
@@ -998,6 +1001,7 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			fl4.saddr = ip_hdr(skb)->saddr;
 
 			rt = ip_route_output_key(dev_net(dev), &fl4);
+			rcu_read_unlock();
 			if (!IS_ERR(rt)) {
 				skb_dst_set(skb, &rt->dst);
 				icmp_send(skb, ICMP_DEST_UNREACH,
@@ -1006,8 +1010,9 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			}
 		}
 #if IS_ENABLED(CONFIG_IPV6)
-		else if (skb->protocol == htons(ETH_P_IPV6))
+		else if (skb->protocol == htons(ETH_P_IPV6)) {
 			icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, localmtu);
+		}
 #endif
 		goto out_dropped;
 	}
@@ -1023,7 +1028,7 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			netdev_err(dev, "BUG! Tx Ring full when queue awake!\n");
 			dev->stats.tx_errors++;
 		}
-		spin_unlock_irqrestore(&port->vio.lock, flags);
+		rcu_read_unlock();
 		return NETDEV_TX_BUSY;
 	}
 
@@ -1117,25 +1122,27 @@ ldc_start_done:
 	}
 
 	spin_unlock_irqrestore(&port->vio.lock, flags);
+	(void)mod_timer(&port->clean_timer, jiffies + VNET_CLEAN_TIMEOUT);
+	rcu_read_unlock();
 
 	vnet_free_skbs(freeskbs);
 
-	(void)mod_timer(&port->clean_timer, jiffies + VNET_CLEAN_TIMEOUT);
-
 	return NETDEV_TX_OK;
 
 out_dropped_unlock:
 	spin_unlock_irqrestore(&port->vio.lock, flags);
 
 out_dropped:
-	if (skb)
-		dev_kfree_skb(skb);
-	vnet_free_skbs(freeskbs);
 	if (pending)
 		(void)mod_timer(&port->clean_timer,
 				jiffies + VNET_CLEAN_TIMEOUT);
 	else if (port)
 		del_timer(&port->clean_timer);
+	if (port)
+		rcu_read_unlock();
+	if (skb)
+		dev_kfree_skb(skb);
+	vnet_free_skbs(freeskbs);
 	dev->stats.tx_dropped++;
 	return NETDEV_TX_OK;
 }
@@ -1265,18 +1272,17 @@ static void vnet_set_rx_mode(struct net_device *dev)
 {
 	struct vnet *vp = netdev_priv(dev);
 	struct vnet_port *port;
-	unsigned long flags;
 
-	spin_lock_irqsave(&vp->lock, flags);
-	if (!list_empty(&vp->port_list)) {
-		port = list_entry(vp->port_list.next, struct vnet_port, list);
+	rcu_read_lock();
+	list_for_each_entry_rcu(port, &vp->port_list, list) {
 
 		if (port->switch_port) {
 			__update_mc_list(vp, dev);
 			__send_mc_list(vp, port);
+			break;
 		}
 	}
-	spin_unlock_irqrestore(&vp->lock, flags);
+	rcu_read_unlock();
 }
 
 static int vnet_change_mtu(struct net_device *dev, int new_mtu)
@@ -1629,10 +1635,11 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 
 	spin_lock_irqsave(&vp->lock, flags);
 	if (switch_port)
-		list_add(&port->list, &vp->port_list);
+		list_add_rcu(&port->list, &vp->port_list);
 	else
-		list_add_tail(&port->list, &vp->port_list);
-	hlist_add_head(&port->hash, &vp->port_hash[vnet_hashfn(port->raddr)]);
+		list_add_tail_rcu(&port->list, &vp->port_list);
+	hlist_add_head_rcu(&port->hash,
+			   &vp->port_hash[vnet_hashfn(port->raddr)]);
 	spin_unlock_irqrestore(&vp->lock, flags);
 
 	dev_set_drvdata(&vdev->dev, port);
@@ -1667,18 +1674,16 @@ static int vnet_port_remove(struct vio_dev *vdev)
 	struct vnet_port *port = dev_get_drvdata(&vdev->dev);
 
 	if (port) {
-		struct vnet *vp = port->vp;
-		unsigned long flags;
 
 		del_timer_sync(&port->vio.timer);
-		del_timer_sync(&port->clean_timer);
 
 		napi_disable(&port->napi);
-		spin_lock_irqsave(&vp->lock, flags);
-		list_del(&port->list);
-		hlist_del(&port->hash);
-		spin_unlock_irqrestore(&vp->lock, flags);
 
+		list_del_rcu(&port->list);
+		hlist_del_rcu(&port->hash);
+
+		synchronize_rcu();
+		del_timer_sync(&port->clean_timer);
 		netif_napi_del(&port->napi);
 		vnet_port_free_tx_bufs(port);
 		vio_ldc_free(&port->vio);
-- 
1.8.4.2

^ permalink raw reply related

* [PATCHv4 RFC net-next 1/4] sunvnet: NAPIfy sunvnet
From: Sowmini Varadhan @ 2014-10-15 18:01 UTC (permalink / raw)
  To: davem, bob.picco, sowmini.varadhan, dwight.engen,
	raghuram.kothakota, david.stevens
  Cc: netdev


Move Rx packet procssing to the NAPI poll callback.
Disable VIO interrupt and unconditioanlly go into NAPI
context from vnet_event.

Note that we want to minimize the number of LDC
STOP/START messages sent. Specifically, do not send a STOP
message if vnet_walk_rx does not read all the available descriptors
because of the NAPI budget limitation. Instead, note the end index
as part of port state, and resume from this index when the
next poll callback is triggered.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Acked-by: Raghuram Kothakota <raghuram.kothakota@oracle.com>
Acked-by: Dwight Engen <dwight.engen@oracle.com>
---
changes since v2: use NAPI.
changes since v3: David Stevens comments.

 drivers/net/ethernet/sun/sunvnet.c | 165 ++++++++++++++++++++++++++++---------
 drivers/net/ethernet/sun/sunvnet.h |   6 +-
 2 files changed, 129 insertions(+), 42 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index 1539672..4a4b482 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -33,6 +33,8 @@
 #define DRV_MODULE_VERSION	"1.0"
 #define DRV_MODULE_RELDATE	"June 25, 2007"
 
+#define	NAPI_POLL_WEIGHT	64
+
 static char version[] =
 	DRV_MODULE_NAME ".c:v" DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")\n";
 MODULE_AUTHOR("David S. Miller (davem@davemloft.net)");
@@ -311,9 +313,7 @@ static int vnet_rx_one(struct vnet_port *port, unsigned int len,
 
 	dev->stats.rx_packets++;
 	dev->stats.rx_bytes += len;
-
-	netif_rx(skb);
-
+	napi_gro_receive(&port->napi, skb);
 	return 0;
 
 out_free_skb:
@@ -430,6 +430,7 @@ static int vnet_walk_rx_one(struct vnet_port *port,
 	struct vio_driver_state *vio = &port->vio;
 	int err;
 
+	BUG_ON(desc == NULL);
 	if (IS_ERR(desc))
 		return PTR_ERR(desc);
 
@@ -456,10 +457,11 @@ static int vnet_walk_rx_one(struct vnet_port *port,
 }
 
 static int vnet_walk_rx(struct vnet_port *port, struct vio_dring_state *dr,
-			u32 start, u32 end)
+			u32 start, u32 end, int *npkts, int budget)
 {
 	struct vio_driver_state *vio = &port->vio;
 	int ack_start = -1, ack_end = -1;
+	bool send_ack = true;
 
 	end = (end == (u32) -1) ? prev_idx(start, dr) : next_idx(end, dr);
 
@@ -471,6 +473,7 @@ static int vnet_walk_rx(struct vnet_port *port, struct vio_dring_state *dr,
 			return err;
 		if (err != 0)
 			break;
+		(*npkts)++;
 		if (ack_start == -1)
 			ack_start = start;
 		ack_end = start;
@@ -482,13 +485,26 @@ static int vnet_walk_rx(struct vnet_port *port, struct vio_dring_state *dr,
 				return err;
 			ack_start = -1;
 		}
+		if ((*npkts) >= budget) {
+			send_ack = false;
+			break;
+		}
 	}
 	if (unlikely(ack_start == -1))
 		ack_start = ack_end = prev_idx(start, dr);
-	return vnet_send_ack(port, dr, ack_start, ack_end, VIO_DRING_STOPPED);
+	if (send_ack) {
+		port->napi_resume = false;
+		return vnet_send_ack(port, dr, ack_start, ack_end,
+				     VIO_DRING_STOPPED);
+	} else  {
+		port->napi_resume = true;
+		port->napi_stop_idx = ack_end;
+		return 1;
+	}
 }
 
-static int vnet_rx(struct vnet_port *port, void *msgbuf)
+static int vnet_rx(struct vnet_port *port, void *msgbuf, int *npkts,
+		   int budget)
 {
 	struct vio_dring_data *pkt = msgbuf;
 	struct vio_dring_state *dr = &port->vio.drings[VIO_DRIVER_RX_RING];
@@ -505,11 +521,13 @@ static int vnet_rx(struct vnet_port *port, void *msgbuf)
 		return 0;
 	}
 
-	dr->rcv_nxt++;
+	if (!port->napi_resume)
+		dr->rcv_nxt++;
 
 	/* XXX Validate pkt->start_idx and pkt->end_idx XXX */
 
-	return vnet_walk_rx(port, dr, pkt->start_idx, pkt->end_idx);
+	return vnet_walk_rx(port, dr, pkt->start_idx, pkt->end_idx,
+			    npkts, budget);
 }
 
 static int idx_is_pending(struct vio_dring_state *dr, u32 end)
@@ -542,9 +560,12 @@ static int vnet_ack(struct vnet_port *port, void *msgbuf)
 	if (unlikely(!idx_is_pending(dr, end)))
 		return 0;
 
+	vp = port->vp;
+	dev = vp->dev;
 	/* sync for race conditions with vnet_start_xmit() and tell xmit it
 	 * is time to send a trigger.
 	 */
+	netif_tx_lock(dev);
 	dr->cons = next_idx(end, dr);
 	desc = vio_dring_entry(dr, dr->cons);
 	if (desc->hdr.state == VIO_DESC_READY && port->start_cons) {
@@ -559,10 +580,8 @@ static int vnet_ack(struct vnet_port *port, void *msgbuf)
 	} else {
 		port->start_cons = true;
 	}
+	netif_tx_unlock(dev);
 
-
-	vp = port->vp;
-	dev = vp->dev;
 	if (unlikely(netif_queue_stopped(dev) &&
 		     vnet_tx_dring_avail(dr) >= VNET_TX_WAKEUP_THRESH(dr)))
 		return 1;
@@ -591,9 +610,8 @@ static int handle_mcast(struct vnet_port *port, void *msgbuf)
 	return 0;
 }
 
-static void maybe_tx_wakeup(unsigned long param)
+static void maybe_tx_wakeup(struct vnet *vp)
 {
-	struct vnet *vp = (struct vnet *)param;
 	struct net_device *dev = vp->dev;
 
 	netif_tx_lock(dev);
@@ -617,31 +635,36 @@ static void maybe_tx_wakeup(unsigned long param)
 	netif_tx_unlock(dev);
 }
 
-static void vnet_event(void *arg, int event)
+static inline bool port_is_up(struct vnet_port *vnet)
+{
+	struct vio_driver_state *vio = &vnet->vio;
+
+	return !!(vio->hs_state & VIO_HS_COMPLETE);
+}
+
+static int vnet_event_napi(struct vnet_port *port, int budget)
 {
-	struct vnet_port *port = arg;
 	struct vio_driver_state *vio = &port->vio;
-	unsigned long flags;
 	int tx_wakeup, err;
-
-	spin_lock_irqsave(&vio->lock, flags);
+	int npkts = 0;
+	int event = port->rx_event;
 
 	if (unlikely(event == LDC_EVENT_RESET ||
 		     event == LDC_EVENT_UP)) {
 		vio_link_state_change(vio, event);
-		spin_unlock_irqrestore(&vio->lock, flags);
 
 		if (event == LDC_EVENT_RESET) {
 			port->rmtu = 0;
 			vio_port_up(vio);
 		}
-		return;
+		port->rx_event = 0;
+		return 0;
 	}
 
 	if (unlikely(event != LDC_EVENT_DATA_READY)) {
-		pr_warn("Unexpected LDC event %d\n", event);
-		spin_unlock_irqrestore(&vio->lock, flags);
-		return;
+		/* napi polling can call us without ready data */
+		port->rx_event = 0;
+		return 0;
 	}
 
 	tx_wakeup = err = 0;
@@ -651,6 +674,21 @@ static void vnet_event(void *arg, int event)
 			u64 raw[8];
 		} msgbuf;
 
+		if (port->napi_resume) {
+			struct vio_dring_data *pkt =
+				(struct vio_dring_data *)&msgbuf;
+			struct vio_dring_state *dr =
+				&port->vio.drings[VIO_DRIVER_RX_RING];
+
+			pkt->tag.type = VIO_TYPE_DATA;
+			pkt->tag.stype = VIO_SUBTYPE_INFO;
+			pkt->tag.stype_env = VIO_DRING_DATA;
+			pkt->seq = dr->rcv_nxt;
+			pkt->start_idx = next_idx(port->napi_stop_idx, dr);
+			pkt->end_idx = -1;
+			goto napi_resume;
+		}
+ldc_read:
 		err = ldc_read(vio->lp, &msgbuf, sizeof(msgbuf));
 		if (unlikely(err < 0)) {
 			if (err == -ECONNRESET)
@@ -667,10 +705,22 @@ static void vnet_event(void *arg, int event)
 		err = vio_validate_sid(vio, &msgbuf.tag);
 		if (err < 0)
 			break;
-
+napi_resume:
 		if (likely(msgbuf.tag.type == VIO_TYPE_DATA)) {
 			if (msgbuf.tag.stype == VIO_SUBTYPE_INFO) {
-				err = vnet_rx(port, &msgbuf);
+				if (!port_is_up(port)) {
+					/* failures like handshake_failure()
+					 * may have cleaned up dring, but
+					 * NAPI polling may bring us here.
+					 */
+					err = -ECONNRESET;
+					break;
+				}
+				err = vnet_rx(port, &msgbuf, &npkts, budget);
+				if (npkts >= budget)
+					break;
+				if (npkts == 0 && err != -ECONNRESET)
+					goto ldc_read;
 			} else if (msgbuf.tag.stype == VIO_SUBTYPE_ACK) {
 				err = vnet_ack(port, &msgbuf);
 				if (err > 0)
@@ -691,15 +741,33 @@ static void vnet_event(void *arg, int event)
 		if (err == -ECONNRESET)
 			break;
 	}
-	spin_unlock(&vio->lock);
-	/* Kick off a tasklet to wake the queue.  We cannot call
-	 * maybe_tx_wakeup directly here because we could deadlock on
-	 * netif_tx_lock() with dev_watchdog()
-	 */
 	if (unlikely(tx_wakeup && err != -ECONNRESET))
-		tasklet_schedule(&port->vp->vnet_tx_wakeup);
+		maybe_tx_wakeup(port->vp);
+	return npkts;
+}
+
+static int vnet_poll(struct napi_struct *napi, int budget)
+{
+	struct vnet_port *port = container_of(napi, struct vnet_port, napi);
+	struct vio_driver_state *vio = &port->vio;
+	int processed = vnet_event_napi(port, budget);
+
+	if (processed < budget) {
+		napi_complete(napi);
+		vio_set_intr(vio->vdev->rx_ino, HV_INTR_ENABLED);
+	}
+	return processed;
+}
+
+static void vnet_event(void *arg, int event)
+{
+	struct vnet_port *port = arg;
+	struct vio_driver_state *vio = &port->vio;
+
+	port->rx_event = event;
+	vio_set_intr(vio->vdev->rx_ino, HV_INTR_DISABLED);
+	napi_schedule(&port->napi);
 
-	local_irq_restore(flags);
 }
 
 static int __vnet_tx_trigger(struct vnet_port *port, u32 start)
@@ -746,13 +814,6 @@ static int __vnet_tx_trigger(struct vnet_port *port, u32 start)
 	return err;
 }
 
-static inline bool port_is_up(struct vnet_port *vnet)
-{
-	struct vio_driver_state *vio = &vnet->vio;
-
-	return !!(vio->hs_state & VIO_HS_COMPLETE);
-}
-
 struct vnet_port *__tx_port_find(struct vnet *vp, struct sk_buff *skb)
 {
 	unsigned int hash = vnet_hashfn(skb->data);
@@ -1342,6 +1403,21 @@ err_out:
 	return err;
 }
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+static void vnet_poll_controller(struct net_device *dev)
+{
+	struct vnet *vp = netdev_priv(dev);
+	struct vnet_port *port;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vp->lock, flags);
+	if (!list_empty(&vp->port_list)) {
+		port = list_entry(vp->port_list.next, struct vnet_port, list);
+		napi_schedule(&port->napi);
+	}
+	spin_unlock_irqrestore(&vp->lock, flags);
+}
+#endif
 static LIST_HEAD(vnet_list);
 static DEFINE_MUTEX(vnet_list_mutex);
 
@@ -1354,6 +1430,9 @@ static const struct net_device_ops vnet_ops = {
 	.ndo_tx_timeout		= vnet_tx_timeout,
 	.ndo_change_mtu		= vnet_change_mtu,
 	.ndo_start_xmit		= vnet_start_xmit,
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	.ndo_poll_controller	= vnet_poll_controller,
+#endif
 };
 
 static struct vnet *vnet_new(const u64 *local_mac)
@@ -1374,7 +1453,6 @@ static struct vnet *vnet_new(const u64 *local_mac)
 	vp = netdev_priv(dev);
 
 	spin_lock_init(&vp->lock);
-	tasklet_init(&vp->vnet_tx_wakeup, maybe_tx_wakeup, (unsigned long)vp);
 	vp->dev = dev;
 
 	INIT_LIST_HEAD(&vp->port_list);
@@ -1434,7 +1512,6 @@ static void vnet_cleanup(void)
 		vp = list_first_entry(&vnet_list, struct vnet, list);
 		list_del(&vp->list);
 		dev = vp->dev;
-		tasklet_kill(&vp->vnet_tx_wakeup);
 		/* vio_unregister_driver() should have cleaned up port_list */
 		BUG_ON(!list_empty(&vp->port_list));
 		unregister_netdev(dev);
@@ -1536,6 +1613,8 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 	if (err)
 		goto err_out_free_port;
 
+	netif_napi_add(port->vp->dev, &port->napi, vnet_poll, NAPI_POLL_WEIGHT);
+
 	err = vnet_port_alloc_tx_bufs(port);
 	if (err)
 		goto err_out_free_ldc;
@@ -1564,6 +1643,7 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 	setup_timer(&port->clean_timer, vnet_clean_timer_expire,
 		    (unsigned long)port);
 
+	napi_enable(&port->napi);
 	vio_port_up(&port->vio);
 
 	mdesc_release(hp);
@@ -1571,6 +1651,7 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 	return 0;
 
 err_out_free_ldc:
+	netif_napi_del(&port->napi);
 	vio_ldc_free(&port->vio);
 
 err_out_free_port:
@@ -1592,11 +1673,13 @@ static int vnet_port_remove(struct vio_dev *vdev)
 		del_timer_sync(&port->vio.timer);
 		del_timer_sync(&port->clean_timer);
 
+		napi_disable(&port->napi);
 		spin_lock_irqsave(&vp->lock, flags);
 		list_del(&port->list);
 		hlist_del(&port->hash);
 		spin_unlock_irqrestore(&vp->lock, flags);
 
+		netif_napi_del(&port->napi);
 		vnet_port_free_tx_bufs(port);
 		vio_ldc_free(&port->vio);
 
diff --git a/drivers/net/ethernet/sun/sunvnet.h b/drivers/net/ethernet/sun/sunvnet.h
index c911045..c8a862e 100644
--- a/drivers/net/ethernet/sun/sunvnet.h
+++ b/drivers/net/ethernet/sun/sunvnet.h
@@ -56,6 +56,11 @@ struct vnet_port {
 	struct timer_list	clean_timer;
 
 	u64			rmtu;
+
+	struct napi_struct	napi;
+	u32			napi_stop_idx;
+	bool			napi_resume;
+	int			rx_event;
 };
 
 static inline struct vnet_port *to_vnet_port(struct vio_driver_state *vio)
@@ -97,7 +102,6 @@ struct vnet {
 	struct list_head	list;
 	u64			local_mac;
 
-	struct tasklet_struct	vnet_tx_wakeup;
 };
 
 #endif /* _SUNVNET_H */
-- 
1.8.4.2

^ permalink raw reply related

* [PATCHv4 RFC net-next 0/4] sunvnet: NAPIfy sunvnet
From: Sowmini Varadhan @ 2014-10-15 18:00 UTC (permalink / raw)
  To: davem, bob.picco, sowmini.varadhan, dwight.engen,
	raghuram.kothakota, david.stevens
  Cc: netdev, sparclinux


This patchset converts the sunvnet driver to use the NAPI framework.

Patch 1 in the series addresses the packet-receive path- all
the vnet_event() processing is moved into NAPI context.
This patch is dependant on the sparc-next commit:
  "sparc64: Add vio_set_intr() to enable/disable Rx interrupts"
(Please cherry-pick sparc commit id ca605b7dd740c8909408d67911d8ddd272c2b320)

Patch 2 uses RCU to fix race conditions between vnet_port_remove and
paths that access/modify port-related state, such as vnet_start_xmit.

Patch 3 and Patch 4 in the series leverage from the NAPIfied Rx path, 
dropping superfluous usage of the irqsave/irqrestores on the vio.lock
where possible.

Note: Patch 3 contains changes that target sparc-next, Patch 4 targets
net-next.

Sowmini Varadhan (4):
  NAPIfy sunvnet
  Use RCU to synchronize port usage with vnet_port_remove()
  Avoid irqsave/restore on vio.lock if in_softirq()
  Remove irqsave/irqrestore on vio.lock

 arch/sparc/kernel/viohs.c          |   8 +-
 drivers/net/ethernet/sun/sunvnet.c | 258 +++++++++++++++++++++++--------------
 drivers/net/ethernet/sun/sunvnet.h |   6 +-
 3 files changed, 173 insertions(+), 99 deletions(-)

-- 
1.8.4.2


^ permalink raw reply

* Re: [PATCHv3 RFC net-next 1/4] sunvnet: NAPIfy sunvnet
From: Sowmini Varadhan @ 2014-10-15 17:22 UTC (permalink / raw)
  To: David L Stevens
  Cc: davem, bob.picco, dwight.engen, raghuram.kothakota, netdev
In-Reply-To: <543EAC34.1010502@oracle.com>

On (10/15/14 13:17), David L Stevens wrote:
> > -static void maybe_tx_wakeup(unsigned long param)
> > +static void maybe_tx_wakeup(struct vnet *vp)
> >  {
> > -	struct vnet *vp = (struct vnet *)param;
> >  	struct net_device *dev = vp->dev;
> >  
> >  	netif_tx_lock(dev);
> 
> Isn't this a function type conflict for tasklet_init()?

maybe_tx_wakeup is not a tasklet any more. That was one of the
big benefits of going NAPI, as David Miller pointed out- we
are already in NAPI context, so the original reason (deadlock
with dev_watchdog) does not exist any more.

I'm working on adding the rest of your comments about
removing comments and making an int into a bool.

--Sowmini

^ permalink raw reply

* Re: [PATCHv3 RFC net-next 1/4] sunvnet: NAPIfy sunvnet
From: David L Stevens @ 2014-10-15 17:17 UTC (permalink / raw)
  To: Sowmini Varadhan, davem, bob.picco, dwight.engen,
	raghuram.kothakota; +Cc: netdev
In-Reply-To: <20141015164252.GB11840@oracle.com>



On 10/15/2014 12:42 PM, Sowmini Varadhan wrote:

> @@ -274,6 +276,7 @@ static struct sk_buff *alloc_and_align_skb(struct net_device *dev,
>  	return skb;
>  }
>  
> +/* reads in exactly one sk_buff */
>  static int vnet_rx_one(struct vnet_port *port, unsigned int len,
>  		       struct ldc_trans_cookie *cookies, int ncookies)
>  {

I'm not sure this comment adds any clarity.

	return PTR_ERR(desc);
>  
> @@ -444,6 +446,7 @@ static int vnet_walk_rx_one(struct vnet_port *port,
>  	       desc->cookies[0].cookie_addr,
>  	       desc->cookies[0].cookie_size);
>  
> +	/* read in one packet */
>  	err = vnet_rx_one(port, desc->size, desc->cookies, desc->ncookies);
>  	if (err == -ECONNRESET)
>  		return err;

or this one.

> @@ -456,10 +459,11 @@ static int vnet_walk_rx_one(struct vnet_port *port,
>  }
>  
>  static int vnet_walk_rx(struct vnet_port *port, struct vio_dring_state *dr,
> -			u32 start, u32 end)
> +			u32 start, u32 end, int *npkts, int budget)
>  {
>  	struct vio_driver_state *vio = &port->vio;
>  	int ack_start = -1, ack_end = -1;
> +	int send_ack = 1;
>  
>  	end = (end == (u32) -1) ? prev_idx(start, dr) : next_idx(end, dr);
>  

suggest making this a "bool send_ack;" with below change.

> @@ -471,6 +475,7 @@ static int vnet_walk_rx(struct vnet_port *port, struct vio_dring_state *dr,
>  			return err;
>  		if (err != 0)
>  			break;
> +		(*npkts)++;
>  		if (ack_start == -1)
>  			ack_start = start;
>  		ack_end = start;
> @@ -482,13 +487,26 @@ static int vnet_walk_rx(struct vnet_port *port, struct vio_dring_state *dr,
>  				return err;
>  			ack_start = -1;
>  		}
> +		if ((*npkts) >= budget) {
> +			send_ack = 0;
> +			break;
> +		}
>  	}

suggest:
		send_ack = (*nkpts) < budget;
		if (!send_ack)
			break;


> @@ -591,9 +612,8 @@ static int handle_mcast(struct vnet_port *port, void *msgbuf)
>  	return 0;
>  }
>  
> -static void maybe_tx_wakeup(unsigned long param)
> +static void maybe_tx_wakeup(struct vnet *vp)
>  {
> -	struct vnet *vp = (struct vnet *)param;
>  	struct net_device *dev = vp->dev;
>  
>  	netif_tx_lock(dev);

Isn't this a function type conflict for tasklet_init()?

					+-DLS

^ permalink raw reply

* Re: [PATCHv3 RFC net-next 2/4] sunvnet: Use RCU to synchronize port usage with vnet_port_remove()
From: David L Stevens @ 2014-10-15 16:53 UTC (permalink / raw)
  To: Sowmini Varadhan, davem, bob.picco, dwight.engen; +Cc: netdev
In-Reply-To: <20141015164303.GC11840@oracle.com>



On 10/15/2014 12:43 PM, Sowmini Varadhan wrote:

>  
> @@ -1000,6 +1003,7 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
...
>  #if IS_ENABLED(CONFIG_IPV6)
> -		else if (skb->protocol == htons(ETH_P_IPV6))
> +		else if (skb->protocol == htons(ETH_P_IPV6)) {
> +			rcu_read_unlock();
>  			icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, localmtu);
> +		}
>  #endif
>  		goto out_dropped;
>  	}

I don't think you want this one; "out_dropped" does this already:


>  
>  out_dropped:
...
>  		del_timer(&port->clean_timer);
> +	if (port)
> +		rcu_read_unlock();
> +	if (skb)
> +		dev_kfree_skb(skb);
> +	vnet_free_skbs(freeskbs);
>  	dev->stats.tx_dropped++;
>  	return NETDEV_TX_OK;
>  }


					+-DLS

^ permalink raw reply

* [PATCHv3 RFC net-next 4/4] sunvnet: Remove irqsave/irqrestore on vio.lock
From: Sowmini Varadhan @ 2014-10-15 16:43 UTC (permalink / raw)
  To: davem, sowmini.varadhan; +Cc: netdev


After the  NAPIfication of sunvnet, we no longer need to
synchronize by doing irqsave/restore on vio.lock in the
I/O fastpath.

NAPI ->poll() is non-reentrant, so all RX processing occurs
strictly in a serialized environment. TX reclaim is done in NAPI
context, so the netif_tx_lock can be used to serialize
critical sections between Tx and Rx paths.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
 drivers/net/ethernet/sun/sunvnet.c | 30 +++++-------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index 09dbd07..128547cc 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -840,18 +840,6 @@ struct vnet_port *__tx_port_find(struct vnet *vp, struct sk_buff *skb)
 	return NULL;
 }
 
-struct vnet_port *tx_port_find(struct vnet *vp, struct sk_buff *skb)
-{
-	struct vnet_port *ret;
-	unsigned long flags;
-
-	spin_lock_irqsave(&vp->lock, flags);
-	ret = __tx_port_find(vp, skb);
-	spin_unlock_irqrestore(&vp->lock, flags);
-
-	return ret;
-}
-
 static struct sk_buff *vnet_clean_tx_ring(struct vnet_port *port,
 					  unsigned *pending)
 {
@@ -912,11 +900,10 @@ static void vnet_clean_timer_expire(unsigned long port0)
 	struct vnet_port *port = (struct vnet_port *)port0;
 	struct sk_buff *freeskbs;
 	unsigned pending;
-	unsigned long flags;
 
-	spin_lock_irqsave(&port->vio.lock, flags);
+	netif_tx_lock(port->vp->dev);
 	freeskbs = vnet_clean_tx_ring(port, &pending);
-	spin_unlock_irqrestore(&port->vio.lock, flags);
+	netif_tx_unlock(port->vp->dev);
 
 	vnet_free_skbs(freeskbs);
 
@@ -969,7 +956,6 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	struct vnet_port *port = NULL;
 	struct vio_dring_state *dr;
 	struct vio_net_desc *d;
-	unsigned long flags;
 	unsigned int len;
 	struct sk_buff *freeskbs = NULL;
 	int i, err, txi;
@@ -982,7 +968,7 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		goto out_dropped;
 
 	rcu_read_lock();
-	port = tx_port_find(vp, skb);
+	port = __tx_port_find(vp, skb);
 	if (unlikely(!port))
 		goto out_dropped;
 
@@ -1020,8 +1006,6 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		goto out_dropped;
 	}
 
-	spin_lock_irqsave(&port->vio.lock, flags);
-
 	dr = &port->vio.drings[VIO_DRIVER_TX_RING];
 	if (unlikely(vnet_tx_dring_avail(dr) < 2)) {
 		if (!netif_queue_stopped(dev)) {
@@ -1055,7 +1039,7 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			     (LDC_MAP_SHADOW | LDC_MAP_DIRECT | LDC_MAP_RW));
 	if (err < 0) {
 		netdev_info(dev, "tx buffer map error %d\n", err);
-		goto out_dropped_unlock;
+		goto out_dropped;
 	}
 	port->tx_bufs[txi].ncookies = err;
 
@@ -1108,7 +1092,7 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		netdev_info(dev, "TX trigger error %d\n", err);
 		d->hdr.state = VIO_DESC_FREE;
 		dev->stats.tx_carrier_errors++;
-		goto out_dropped_unlock;
+		goto out_dropped;
 	}
 
 ldc_start_done:
@@ -1124,7 +1108,6 @@ ldc_start_done:
 			netif_wake_queue(dev);
 	}
 
-	spin_unlock_irqrestore(&port->vio.lock, flags);
 	(void)mod_timer(&port->clean_timer, jiffies + VNET_CLEAN_TIMEOUT);
 	rcu_read_unlock();
 
@@ -1132,9 +1115,6 @@ ldc_start_done:
 
 	return NETDEV_TX_OK;
 
-out_dropped_unlock:
-	spin_unlock_irqrestore(&port->vio.lock, flags);
-
 out_dropped:
 	if (pending)
 		(void)mod_timer(&port->clean_timer,
-- 
1.8.4.2

^ permalink raw reply related

* [PATCHv3 RFC 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq()
From: Sowmini Varadhan @ 2014-10-15 16:43 UTC (permalink / raw)
  To: davem, sowmini.varadhan; +Cc: netdev, sparclinux


For NAPIfied drivers , there is no need to
synchronize by doing irqsave/restore on vio.lock in the I/O
path.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
 arch/sparc/kernel/viohs.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/sparc/kernel/viohs.c b/arch/sparc/kernel/viohs.c
index 7ef081a..d731586 100644
--- a/arch/sparc/kernel/viohs.c
+++ b/arch/sparc/kernel/viohs.c
@@ -747,10 +747,11 @@ EXPORT_SYMBOL(vio_ldc_free);
 
 void vio_port_up(struct vio_driver_state *vio)
 {
-	unsigned long flags;
+	unsigned long flags = 0;
 	int err, state;
 
-	spin_lock_irqsave(&vio->lock, flags);
+	if (!in_softirq())
+		spin_lock_irqsave(&vio->lock, flags);
 
 	state = ldc_state(vio->lp);
 
@@ -777,7 +778,8 @@ void vio_port_up(struct vio_driver_state *vio)
 		mod_timer(&vio->timer, expires);
 	}
 
-	spin_unlock_irqrestore(&vio->lock, flags);
+	if (!in_softirq())
+		spin_unlock_irqrestore(&vio->lock, flags);
 }
 EXPORT_SYMBOL(vio_port_up);
 
-- 
1.8.4.2

^ permalink raw reply related

* [PATCHv3 RFC net-next 2/4] sunvnet: Use RCU to synchronize port usage with vnet_port_remove()
From: Sowmini Varadhan @ 2014-10-15 16:43 UTC (permalink / raw)
  To: davem, bob.picco, sowmini.varadhan, dwight.engen; +Cc: netdev


A vnet_port_remove could be triggered as a result of an ldm-unbind
operation by the peer, module unload, or other changes to the
inter-vnet-link configuration.  When this is concurrent with
vnet_start_xmit(), there are several race sequences possible,
such as

thread 1                                    thread 2
vnet_start_xmit
-> tx_port_find
   spin_lock_irqsave(&vp->lock..)
   ret = __tx_port_find(..)
   spin_lock_irqrestore(&vp->lock..)
                                           vio_remove -> ..
                                               ->vnet_port_remove
                                           spin_lock_irqsave(&vp->lock..)
                                           cleanup
                                           spin_lock_irqrestore(&vp->lock..)
                                           kfree(port)
/* attempt to use ret will bomb */

This patch adds RCU locking for port access so that vnet_port_remove
will correctly clean up port-related state.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Acked-by: Dwight Engen <dwight.engen@oracle.com>
Acked-by: Bob Picco <bob.picco@oracle.com>
---
changes since v2: use RCU.

 drivers/net/ethernet/sun/sunvnet.c | 66 +++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index bc59937..09dbd07 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -621,7 +621,8 @@ static void maybe_tx_wakeup(struct vnet *vp)
 		struct vnet_port *port;
 		int wake = 1;
 
-		list_for_each_entry(port, &vp->port_list, list) {
+		rcu_read_lock();
+		list_for_each_entry_rcu(port, &vp->port_list, list) {
 			struct vio_dring_state *dr;
 
 			dr = &port->vio.drings[VIO_DRIVER_TX_RING];
@@ -631,6 +632,7 @@ static void maybe_tx_wakeup(struct vnet *vp)
 				break;
 			}
 		}
+		rcu_read_unlock();
 		if (wake)
 			netif_wake_queue(dev);
 	}
@@ -822,13 +824,13 @@ struct vnet_port *__tx_port_find(struct vnet *vp, struct sk_buff *skb)
 	struct hlist_head *hp = &vp->port_hash[hash];
 	struct vnet_port *port;
 
-	hlist_for_each_entry(port, hp, hash) {
+	hlist_for_each_entry_rcu(port, hp, hash) {
 		if (!port_is_up(port))
 			continue;
 		if (ether_addr_equal(port->raddr, skb->data))
 			return port;
 	}
-	list_for_each_entry(port, &vp->port_list, list) {
+	list_for_each_entry_rcu(port, &vp->port_list, list) {
 		if (!port->switch_port)
 			continue;
 		if (!port_is_up(port))
@@ -964,7 +966,7 @@ static inline struct sk_buff *vnet_skb_shape(struct sk_buff *skb, void **pstart,
 static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct vnet *vp = netdev_priv(dev);
-	struct vnet_port *port = tx_port_find(vp, skb);
+	struct vnet_port *port = NULL;
 	struct vio_dring_state *dr;
 	struct vio_net_desc *d;
 	unsigned long flags;
@@ -975,14 +977,15 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	int nlen = 0;
 	unsigned pending = 0;
 
-	if (unlikely(!port))
-		goto out_dropped;
-
 	skb = vnet_skb_shape(skb, &start, &nlen);
-
 	if (unlikely(!skb))
 		goto out_dropped;
 
+	rcu_read_lock();
+	port = tx_port_find(vp, skb);
+	if (unlikely(!port))
+		goto out_dropped;
+
 	if (skb->len > port->rmtu) {
 		unsigned long localmtu = port->rmtu - ETH_HLEN;
 
@@ -1000,6 +1003,7 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			fl4.saddr = ip_hdr(skb)->saddr;
 
 			rt = ip_route_output_key(dev_net(dev), &fl4);
+			rcu_read_unlock();
 			if (!IS_ERR(rt)) {
 				skb_dst_set(skb, &rt->dst);
 				icmp_send(skb, ICMP_DEST_UNREACH,
@@ -1008,8 +1012,10 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			}
 		}
 #if IS_ENABLED(CONFIG_IPV6)
-		else if (skb->protocol == htons(ETH_P_IPV6))
+		else if (skb->protocol == htons(ETH_P_IPV6)) {
+			rcu_read_unlock();
 			icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, localmtu);
+		}
 #endif
 		goto out_dropped;
 	}
@@ -1025,7 +1031,7 @@ static int vnet_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			netdev_err(dev, "BUG! Tx Ring full when queue awake!\n");
 			dev->stats.tx_errors++;
 		}
-		spin_unlock_irqrestore(&port->vio.lock, flags);
+		rcu_read_unlock();
 		return NETDEV_TX_BUSY;
 	}
 
@@ -1119,25 +1125,27 @@ ldc_start_done:
 	}
 
 	spin_unlock_irqrestore(&port->vio.lock, flags);
+	(void)mod_timer(&port->clean_timer, jiffies + VNET_CLEAN_TIMEOUT);
+	rcu_read_unlock();
 
 	vnet_free_skbs(freeskbs);
 
-	(void)mod_timer(&port->clean_timer, jiffies + VNET_CLEAN_TIMEOUT);
-
 	return NETDEV_TX_OK;
 
 out_dropped_unlock:
 	spin_unlock_irqrestore(&port->vio.lock, flags);
 
 out_dropped:
-	if (skb)
-		dev_kfree_skb(skb);
-	vnet_free_skbs(freeskbs);
 	if (pending)
 		(void)mod_timer(&port->clean_timer,
 				jiffies + VNET_CLEAN_TIMEOUT);
 	else if (port)
 		del_timer(&port->clean_timer);
+	if (port)
+		rcu_read_unlock();
+	if (skb)
+		dev_kfree_skb(skb);
+	vnet_free_skbs(freeskbs);
 	dev->stats.tx_dropped++;
 	return NETDEV_TX_OK;
 }
@@ -1267,18 +1275,17 @@ static void vnet_set_rx_mode(struct net_device *dev)
 {
 	struct vnet *vp = netdev_priv(dev);
 	struct vnet_port *port;
-	unsigned long flags;
 
-	spin_lock_irqsave(&vp->lock, flags);
-	if (!list_empty(&vp->port_list)) {
-		port = list_entry(vp->port_list.next, struct vnet_port, list);
+	rcu_read_lock();
+	list_for_each_entry_rcu(port, &vp->port_list, list) {
 
 		if (port->switch_port) {
 			__update_mc_list(vp, dev);
 			__send_mc_list(vp, port);
+			break;
 		}
 	}
-	spin_unlock_irqrestore(&vp->lock, flags);
+	rcu_read_unlock();
 }
 
 static int vnet_change_mtu(struct net_device *dev, int new_mtu)
@@ -1631,10 +1638,11 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 
 	spin_lock_irqsave(&vp->lock, flags);
 	if (switch_port)
-		list_add(&port->list, &vp->port_list);
+		list_add_rcu(&port->list, &vp->port_list);
 	else
-		list_add_tail(&port->list, &vp->port_list);
-	hlist_add_head(&port->hash, &vp->port_hash[vnet_hashfn(port->raddr)]);
+		list_add_tail_rcu(&port->list, &vp->port_list);
+	hlist_add_head_rcu(&port->hash,
+			   &vp->port_hash[vnet_hashfn(port->raddr)]);
 	spin_unlock_irqrestore(&vp->lock, flags);
 
 	dev_set_drvdata(&vdev->dev, port);
@@ -1669,18 +1677,16 @@ static int vnet_port_remove(struct vio_dev *vdev)
 	struct vnet_port *port = dev_get_drvdata(&vdev->dev);
 
 	if (port) {
-		struct vnet *vp = port->vp;
-		unsigned long flags;
 
 		del_timer_sync(&port->vio.timer);
-		del_timer_sync(&port->clean_timer);
 
 		napi_disable(&port->napi);
-		spin_lock_irqsave(&vp->lock, flags);
-		list_del(&port->list);
-		hlist_del(&port->hash);
-		spin_unlock_irqrestore(&vp->lock, flags);
 
+		list_del_rcu(&port->list);
+		hlist_del_rcu(&port->hash);
+
+		synchronize_rcu();
+		del_timer_sync(&port->clean_timer);
 		netif_napi_del(&port->napi);
 		vnet_port_free_tx_bufs(port);
 		vio_ldc_free(&port->vio);
-- 
1.8.4.2

^ permalink raw reply related

* [PATCHv3 RFC net-next 1/4] sunvnet: NAPIfy sunvnet
From: Sowmini Varadhan @ 2014-10-15 16:42 UTC (permalink / raw)
  To: davem, bob.picco, sowmini.varadhan, dwight.engen,
	raghuram.kothakota; +Cc: netdev


Move Rx packet procssing to the NAPI poll callback.
Disable VIO interrupt and unconditioanlly go into NAPI
context from vnet_event.

Note that we want to minimize the number of LDC
STOP/START messages exchanged. Specifically, do not send a STOP
message if vnet_walk_rx does not read all the available descriptors
because of the NAPI budget limitation. Instead, note the end index
as part of port state, and resume from this index when the
next poll callback is triggered.

Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Acked-by: Raghuram Kothakota <raghuram.kothakota@oracle.com>
Acked-by: Dwight Engen <dwight.engen@oracle.com>
---
changes since v2: use NAPI.

 drivers/net/ethernet/sun/sunvnet.c | 167 ++++++++++++++++++++++++++++---------
 drivers/net/ethernet/sun/sunvnet.h |   6 +-
 2 files changed, 131 insertions(+), 42 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
index 1539672..bc59937 100644
--- a/drivers/net/ethernet/sun/sunvnet.c
+++ b/drivers/net/ethernet/sun/sunvnet.c
@@ -33,6 +33,8 @@
 #define DRV_MODULE_VERSION	"1.0"
 #define DRV_MODULE_RELDATE	"June 25, 2007"
 
+#define	NAPI_POLL_WEIGHT	64
+
 static char version[] =
 	DRV_MODULE_NAME ".c:v" DRV_MODULE_VERSION " (" DRV_MODULE_RELDATE ")\n";
 MODULE_AUTHOR("David S. Miller (davem@davemloft.net)");
@@ -274,6 +276,7 @@ static struct sk_buff *alloc_and_align_skb(struct net_device *dev,
 	return skb;
 }
 
+/* reads in exactly one sk_buff */
 static int vnet_rx_one(struct vnet_port *port, unsigned int len,
 		       struct ldc_trans_cookie *cookies, int ncookies)
 {
@@ -311,9 +314,7 @@ static int vnet_rx_one(struct vnet_port *port, unsigned int len,
 
 	dev->stats.rx_packets++;
 	dev->stats.rx_bytes += len;
-
-	netif_rx(skb);
-
+	napi_gro_receive(&port->napi, skb);
 	return 0;
 
 out_free_skb:
@@ -430,6 +431,7 @@ static int vnet_walk_rx_one(struct vnet_port *port,
 	struct vio_driver_state *vio = &port->vio;
 	int err;
 
+	BUG_ON(desc == NULL);
 	if (IS_ERR(desc))
 		return PTR_ERR(desc);
 
@@ -444,6 +446,7 @@ static int vnet_walk_rx_one(struct vnet_port *port,
 	       desc->cookies[0].cookie_addr,
 	       desc->cookies[0].cookie_size);
 
+	/* read in one packet */
 	err = vnet_rx_one(port, desc->size, desc->cookies, desc->ncookies);
 	if (err == -ECONNRESET)
 		return err;
@@ -456,10 +459,11 @@ static int vnet_walk_rx_one(struct vnet_port *port,
 }
 
 static int vnet_walk_rx(struct vnet_port *port, struct vio_dring_state *dr,
-			u32 start, u32 end)
+			u32 start, u32 end, int *npkts, int budget)
 {
 	struct vio_driver_state *vio = &port->vio;
 	int ack_start = -1, ack_end = -1;
+	int send_ack = 1;
 
 	end = (end == (u32) -1) ? prev_idx(start, dr) : next_idx(end, dr);
 
@@ -471,6 +475,7 @@ static int vnet_walk_rx(struct vnet_port *port, struct vio_dring_state *dr,
 			return err;
 		if (err != 0)
 			break;
+		(*npkts)++;
 		if (ack_start == -1)
 			ack_start = start;
 		ack_end = start;
@@ -482,13 +487,26 @@ static int vnet_walk_rx(struct vnet_port *port, struct vio_dring_state *dr,
 				return err;
 			ack_start = -1;
 		}
+		if ((*npkts) >= budget) {
+			send_ack = 0;
+			break;
+		}
 	}
 	if (unlikely(ack_start == -1))
 		ack_start = ack_end = prev_idx(start, dr);
-	return vnet_send_ack(port, dr, ack_start, ack_end, VIO_DRING_STOPPED);
+	if (send_ack) {
+		port->napi_resume = false;
+		return vnet_send_ack(port, dr, ack_start, ack_end,
+				     VIO_DRING_STOPPED);
+	} else  {
+		port->napi_resume = true;
+		port->napi_stop_idx = ack_end;
+		return 1;
+	}
 }
 
-static int vnet_rx(struct vnet_port *port, void *msgbuf)
+static int vnet_rx(struct vnet_port *port, void *msgbuf, int *npkts,
+		   int budget)
 {
 	struct vio_dring_data *pkt = msgbuf;
 	struct vio_dring_state *dr = &port->vio.drings[VIO_DRIVER_RX_RING];
@@ -505,11 +523,13 @@ static int vnet_rx(struct vnet_port *port, void *msgbuf)
 		return 0;
 	}
 
-	dr->rcv_nxt++;
+	if (!port->napi_resume)
+		dr->rcv_nxt++;
 
 	/* XXX Validate pkt->start_idx and pkt->end_idx XXX */
 
-	return vnet_walk_rx(port, dr, pkt->start_idx, pkt->end_idx);
+	return vnet_walk_rx(port, dr, pkt->start_idx, pkt->end_idx,
+			    npkts, budget);
 }
 
 static int idx_is_pending(struct vio_dring_state *dr, u32 end)
@@ -542,9 +562,12 @@ static int vnet_ack(struct vnet_port *port, void *msgbuf)
 	if (unlikely(!idx_is_pending(dr, end)))
 		return 0;
 
+	vp = port->vp;
+	dev = vp->dev;
 	/* sync for race conditions with vnet_start_xmit() and tell xmit it
 	 * is time to send a trigger.
 	 */
+	netif_tx_lock(dev);
 	dr->cons = next_idx(end, dr);
 	desc = vio_dring_entry(dr, dr->cons);
 	if (desc->hdr.state == VIO_DESC_READY && port->start_cons) {
@@ -559,10 +582,8 @@ static int vnet_ack(struct vnet_port *port, void *msgbuf)
 	} else {
 		port->start_cons = true;
 	}
+	netif_tx_unlock(dev);
 
-
-	vp = port->vp;
-	dev = vp->dev;
 	if (unlikely(netif_queue_stopped(dev) &&
 		     vnet_tx_dring_avail(dr) >= VNET_TX_WAKEUP_THRESH(dr)))
 		return 1;
@@ -591,9 +612,8 @@ static int handle_mcast(struct vnet_port *port, void *msgbuf)
 	return 0;
 }
 
-static void maybe_tx_wakeup(unsigned long param)
+static void maybe_tx_wakeup(struct vnet *vp)
 {
-	struct vnet *vp = (struct vnet *)param;
 	struct net_device *dev = vp->dev;
 
 	netif_tx_lock(dev);
@@ -617,31 +637,36 @@ static void maybe_tx_wakeup(unsigned long param)
 	netif_tx_unlock(dev);
 }
 
-static void vnet_event(void *arg, int event)
+static inline bool port_is_up(struct vnet_port *vnet)
+{
+	struct vio_driver_state *vio = &vnet->vio;
+
+	return !!(vio->hs_state & VIO_HS_COMPLETE);
+}
+
+static int vnet_event_napi(struct vnet_port *port, int budget)
 {
-	struct vnet_port *port = arg;
 	struct vio_driver_state *vio = &port->vio;
-	unsigned long flags;
 	int tx_wakeup, err;
-
-	spin_lock_irqsave(&vio->lock, flags);
+	int npkts = 0;
+	int event = port->rx_event;
 
 	if (unlikely(event == LDC_EVENT_RESET ||
 		     event == LDC_EVENT_UP)) {
 		vio_link_state_change(vio, event);
-		spin_unlock_irqrestore(&vio->lock, flags);
 
 		if (event == LDC_EVENT_RESET) {
 			port->rmtu = 0;
 			vio_port_up(vio);
 		}
-		return;
+		port->rx_event = 0;
+		return 0;
 	}
 
 	if (unlikely(event != LDC_EVENT_DATA_READY)) {
-		pr_warn("Unexpected LDC event %d\n", event);
-		spin_unlock_irqrestore(&vio->lock, flags);
-		return;
+		/* napi polling can call us without ready data */
+		port->rx_event = 0;
+		return 0;
 	}
 
 	tx_wakeup = err = 0;
@@ -651,6 +676,21 @@ static void vnet_event(void *arg, int event)
 			u64 raw[8];
 		} msgbuf;
 
+		if (port->napi_resume) {
+			struct vio_dring_data *pkt =
+				(struct vio_dring_data *)&msgbuf;
+			struct vio_dring_state *dr =
+				&port->vio.drings[VIO_DRIVER_RX_RING];
+
+			pkt->tag.type = VIO_TYPE_DATA;
+			pkt->tag.stype = VIO_SUBTYPE_INFO;
+			pkt->tag.stype_env = VIO_DRING_DATA;
+			pkt->seq = dr->rcv_nxt;
+			pkt->start_idx = next_idx(port->napi_stop_idx, dr);
+			pkt->end_idx = -1;
+			goto napi_resume;
+		}
+ldc_read:
 		err = ldc_read(vio->lp, &msgbuf, sizeof(msgbuf));
 		if (unlikely(err < 0)) {
 			if (err == -ECONNRESET)
@@ -667,10 +707,22 @@ static void vnet_event(void *arg, int event)
 		err = vio_validate_sid(vio, &msgbuf.tag);
 		if (err < 0)
 			break;
-
+napi_resume:
 		if (likely(msgbuf.tag.type == VIO_TYPE_DATA)) {
 			if (msgbuf.tag.stype == VIO_SUBTYPE_INFO) {
-				err = vnet_rx(port, &msgbuf);
+				if (!port_is_up(port)) {
+					/* failures like handshake_failure()
+					 * may have cleaned up dring, but
+					 * NAPI polling may bring us here.
+					 */
+					err = -ECONNRESET;
+					break;
+				}
+				err = vnet_rx(port, &msgbuf, &npkts, budget);
+				if (npkts >= budget)
+					break;
+				if (npkts == 0 && err != -ECONNRESET)
+					goto ldc_read;
 			} else if (msgbuf.tag.stype == VIO_SUBTYPE_ACK) {
 				err = vnet_ack(port, &msgbuf);
 				if (err > 0)
@@ -691,15 +743,33 @@ static void vnet_event(void *arg, int event)
 		if (err == -ECONNRESET)
 			break;
 	}
-	spin_unlock(&vio->lock);
-	/* Kick off a tasklet to wake the queue.  We cannot call
-	 * maybe_tx_wakeup directly here because we could deadlock on
-	 * netif_tx_lock() with dev_watchdog()
-	 */
 	if (unlikely(tx_wakeup && err != -ECONNRESET))
-		tasklet_schedule(&port->vp->vnet_tx_wakeup);
+		maybe_tx_wakeup(port->vp);
+	return npkts;
+}
+
+static int vnet_poll(struct napi_struct *napi, int budget)
+{
+	struct vnet_port *port = container_of(napi, struct vnet_port, napi);
+	struct vio_driver_state *vio = &port->vio;
+	int processed = vnet_event_napi(port, budget);
+
+	if (processed < budget) {
+		napi_complete(napi);
+		vio_set_intr(vio->vdev->rx_ino, HV_INTR_ENABLED);
+	}
+	return processed;
+}
+
+static void vnet_event(void *arg, int event)
+{
+	struct vnet_port *port = arg;
+	struct vio_driver_state *vio = &port->vio;
+
+	port->rx_event = event;
+	vio_set_intr(vio->vdev->rx_ino, HV_INTR_DISABLED);
+	napi_schedule(&port->napi);
 
-	local_irq_restore(flags);
 }
 
 static int __vnet_tx_trigger(struct vnet_port *port, u32 start)
@@ -746,13 +816,6 @@ static int __vnet_tx_trigger(struct vnet_port *port, u32 start)
 	return err;
 }
 
-static inline bool port_is_up(struct vnet_port *vnet)
-{
-	struct vio_driver_state *vio = &vnet->vio;
-
-	return !!(vio->hs_state & VIO_HS_COMPLETE);
-}
-
 struct vnet_port *__tx_port_find(struct vnet *vp, struct sk_buff *skb)
 {
 	unsigned int hash = vnet_hashfn(skb->data);
@@ -1342,6 +1405,21 @@ err_out:
 	return err;
 }
 
+#ifdef CONFIG_NET_POLL_CONTROLLER
+static void vnet_poll_controller(struct net_device *dev)
+{
+	struct vnet *vp = netdev_priv(dev);
+	struct vnet_port *port;
+	unsigned long flags;
+
+	spin_lock_irqsave(&vp->lock, flags);
+	if (!list_empty(&vp->port_list)) {
+		port = list_entry(vp->port_list.next, struct vnet_port, list);
+		napi_schedule(&port->napi);
+	}
+	spin_unlock_irqrestore(&vp->lock, flags);
+}
+#endif
 static LIST_HEAD(vnet_list);
 static DEFINE_MUTEX(vnet_list_mutex);
 
@@ -1354,6 +1432,9 @@ static const struct net_device_ops vnet_ops = {
 	.ndo_tx_timeout		= vnet_tx_timeout,
 	.ndo_change_mtu		= vnet_change_mtu,
 	.ndo_start_xmit		= vnet_start_xmit,
+#ifdef CONFIG_NET_POLL_CONTROLLER
+	.ndo_poll_controller	= vnet_poll_controller,
+#endif
 };
 
 static struct vnet *vnet_new(const u64 *local_mac)
@@ -1374,7 +1455,6 @@ static struct vnet *vnet_new(const u64 *local_mac)
 	vp = netdev_priv(dev);
 
 	spin_lock_init(&vp->lock);
-	tasklet_init(&vp->vnet_tx_wakeup, maybe_tx_wakeup, (unsigned long)vp);
 	vp->dev = dev;
 
 	INIT_LIST_HEAD(&vp->port_list);
@@ -1434,7 +1514,6 @@ static void vnet_cleanup(void)
 		vp = list_first_entry(&vnet_list, struct vnet, list);
 		list_del(&vp->list);
 		dev = vp->dev;
-		tasklet_kill(&vp->vnet_tx_wakeup);
 		/* vio_unregister_driver() should have cleaned up port_list */
 		BUG_ON(!list_empty(&vp->port_list));
 		unregister_netdev(dev);
@@ -1536,6 +1615,8 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 	if (err)
 		goto err_out_free_port;
 
+	netif_napi_add(port->vp->dev, &port->napi, vnet_poll, NAPI_POLL_WEIGHT);
+
 	err = vnet_port_alloc_tx_bufs(port);
 	if (err)
 		goto err_out_free_ldc;
@@ -1564,6 +1645,7 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 	setup_timer(&port->clean_timer, vnet_clean_timer_expire,
 		    (unsigned long)port);
 
+	napi_enable(&port->napi);
 	vio_port_up(&port->vio);
 
 	mdesc_release(hp);
@@ -1571,6 +1653,7 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
 	return 0;
 
 err_out_free_ldc:
+	netif_napi_del(&port->napi);
 	vio_ldc_free(&port->vio);
 
 err_out_free_port:
@@ -1592,11 +1675,13 @@ static int vnet_port_remove(struct vio_dev *vdev)
 		del_timer_sync(&port->vio.timer);
 		del_timer_sync(&port->clean_timer);
 
+		napi_disable(&port->napi);
 		spin_lock_irqsave(&vp->lock, flags);
 		list_del(&port->list);
 		hlist_del(&port->hash);
 		spin_unlock_irqrestore(&vp->lock, flags);
 
+		netif_napi_del(&port->napi);
 		vnet_port_free_tx_bufs(port);
 		vio_ldc_free(&port->vio);
 
diff --git a/drivers/net/ethernet/sun/sunvnet.h b/drivers/net/ethernet/sun/sunvnet.h
index c911045..c8a862e 100644
--- a/drivers/net/ethernet/sun/sunvnet.h
+++ b/drivers/net/ethernet/sun/sunvnet.h
@@ -56,6 +56,11 @@ struct vnet_port {
 	struct timer_list	clean_timer;
 
 	u64			rmtu;
+
+	struct napi_struct	napi;
+	u32			napi_stop_idx;
+	bool			napi_resume;
+	int			rx_event;
 };
 
 static inline struct vnet_port *to_vnet_port(struct vio_driver_state *vio)
@@ -97,7 +102,6 @@ struct vnet {
 	struct list_head	list;
 	u64			local_mac;
 
-	struct tasklet_struct	vnet_tx_wakeup;
 };
 
 #endif /* _SUNVNET_H */
-- 
1.8.4.2

^ permalink raw reply related

* [PATCHv3 RFC net-next 0/4] sunvnet: NAPIfy sunvnet
From: Sowmini Varadhan @ 2014-10-15 16:42 UTC (permalink / raw)
  To: davem, bob.picco, sowmini.varadhan, dwight.engen,
	raghuram.kothakota
  Cc: netdev, sparclinux


This patchset converts the sunvnet driver to use the NAPI framework.

Patch 1 in the series addresses the packet-receive path- all
the vnet_event() processing is moved into NAPI context.
This patch is dependant on the sparc-next commit:
  "sparc64: Add vio_set_intr() to enable/disable Rx interrupts"
(Please cherry-pick sparc commit id ca605b7dd740c8909408d67911d8ddd272c2b320)

Patch 2 uses RCU to fix race conditions between vnet_port_remove and
paths that access/modify port-related state, such as vnet_start_xmit.

Patch 3 and Patch 4 in the series leverage from the NAPIfied Rx path, 
dropping superfluous usage of the irqsave/irqrestores on the vio.lock
where possible.

Note: Patch 3 contains changes that target sparc-next, Patch 4 targets
net-next.

Sowmini Varadhan (4):
  NAPIfy sunvnet
  Use RCU to synchronize port usage with vnet_port_remove()
  Avoid irqsave/restore on vio.lock if in_softirq()
  Remove irqsave/irqrestore on vio.lock

 arch/sparc/kernel/viohs.c          |   8 +-
 drivers/net/ethernet/sun/sunvnet.c | 261 +++++++++++++++++++++++--------------
 drivers/net/ethernet/sun/sunvnet.h |   6 +-
 3 files changed, 176 insertions(+), 99 deletions(-)

-- 
1.8.4.2


^ permalink raw reply

* RE: bnx2: can't disable rx-vlan-offload?
From: Lukas Tribus @ 2014-10-15 16:25 UTC (permalink / raw)
  To: Vlad Yasevich, Sony Chacko, Dept-HSGLinuxNICDev@qlogic.com
  Cc: netdev@vger.kernel.org
In-Reply-To: <543E86A5.8010406@gmail.com>

Hi Vlad,


>> Any ideas or workarounds that may help disabling rx vlan acceleration?
>
> Try bringing the interface down first. BNX2 is a bit strange in how it handles
> vlan feature changes.

I tried, doesn't work.


Anyway, I reproduced the original issue on e1000e with and without
rxvlan offload, so even if I would be able to disable this on bnx2, the
original problem would not go away.

I will do some more troubleshooting on the original issue with e1000e
and will come back in a new thread if it turns out to be a problem.



Thanks,

Lukas

 		 	   		  

^ permalink raw reply

* Re: [PATCH RFC net-next] sfc: add support for skb->xmit_more
From: David Miller @ 2014-10-15 16:20 UTC (permalink / raw)
  To: ecree; +Cc: dborkman, nikolay, netdev, sshah, jcooper, linux-net-drivers
In-Reply-To: <543E54FF.1040304@solarflare.com>

From: Edward Cree <ecree@solarflare.com>
Date: Wed, 15 Oct 2014 12:05:35 +0100

> On 14/10/14 22:15, David Miller wrote:
>> From: Edward Cree <ecree@solarflare.com>
>> Date: Tue, 14 Oct 2014 19:41:37 +0100
>>
>>> Don't ring the doorbell, and don't do PIO.  This will also prevent
>>>  TX Push, because there will be more than one buffer waiting when
>>>  the doorbell is rung.
>>>
>>> Signed-off-by: Edward Cree <ecree@solarflare.com>
>> This looks good to me, mind if I apply this now?
> I'd rather wait until Jon Cooper's had a chance to look at it; he
> understands our TX path better than I.

Ok.

^ permalink raw reply

* Re: [PATCH net] cxgb4i : Fix -Wmaybe-uninitialized warning.
From: David Miller @ 2014-10-15 16:14 UTC (permalink / raw)
  To: anish; +Cc: netdev, kxie
In-Reply-To: <1413358007-16980-1-git-send-email-anish@chelsio.com>

From: Anish Bhatt <anish@chelsio.com>
Date: Wed, 15 Oct 2014 00:26:47 -0700

> Identified by kbuild test robot. csk family is always set to be AF_INET or 
> AF_INET6, so skb will always be initialized to some value but there is no harm 
> in silencing the warning anyways.
> 
> Signed-off-by: Anish Bhatt <anish@chelsio.com>
> Fixes : f42bb57c61fd ('cxgb4i : Fix -Wunused-function warning')

Applied.

^ permalink raw reply

* [net] gianfar: Add FCS to rx buffer size (fix)
From: Claudiu Manoil @ 2014-10-15 16:11 UTC (permalink / raw)
  To: netdev; +Cc: Kristian Otnes, David S. Miller

For each Rx frame the eTSEC writes its FCS (Frame Check Sequence)
to the Rx buffer.

The eTSEC h/w manual states in the "Receive Buffer Descriptor Field
Descriptions" table:
"Data length is the number of octets written by the eTSEC into this BD's
data buffer if L is cleared (the value is equal to MRBLR), or, if L is
set, the length of the frame including *CRC*, FCB (if RCTRL[PRSDEP > 00),
preamble (if MACCFG2[PreAmRxEn]=1), time stamp (if RCTRL[TS] = 1) and
any padding (RCTRL[PAL])."

Though the FCS bytes are removed by the driver before passing the skb
to the net stack, the Rx buffer size computation does not currently
take into account the FCS bytes (4 bytes).
Because the Rx buffer size is multiple of 512 bytes, leaving out the
FCS is not a problem for the default MTU of 1500, as the Rx buffer size
is 1536 in this case.  However, for custom MTUs, where the difference
between the MTU size and the Rx buffer size is less, this can be a
problem as the computed Rx buffer size won't be enough to accomodate
the FCS for a received frame that is big enough (close to MTU size).
In such case the received frame is considered to be incomplete (L flag
not set in the RxBD status) and silently dropped.

Note that the driver does not currently support S/G on Rx, so it has to
compute its Rx buffer size based on the MTU of the device.

Reported-by: Kristian Otnes <kotnes@cisco.com>
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
 drivers/net/ethernet/freescale/gianfar.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 379b1a5..4fdf0aa 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -338,7 +338,7 @@ static void gfar_init_tx_rx_base(struct gfar_private *priv)
 
 static void gfar_rx_buff_size_config(struct gfar_private *priv)
 {
-	int frame_size = priv->ndev->mtu + ETH_HLEN;
+	int frame_size = priv->ndev->mtu + ETH_HLEN + ETH_FCS_LEN;
 
 	/* set this when rx hw offload (TOE) functions are being used */
 	priv->uses_rxfcb = 0;
-- 
1.7.11.7

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox