Netdev List
 help / color / mirror / Atom feed
* [PATCH] usbnet: smsc95xx: dereferencing NULL pointer
From: Sudip Mukherjee @ 2014-11-07 13:22 UTC (permalink / raw)
  To: Steve Glendinning; +Cc: Sudip Mukherjee, netdev, linux-usb, linux-kernel

we were dereferencing dev to initialize pdata. but just after that we
have a BUG_ON(!dev). so we were basically dereferencing the pointer
first and then tesing it for NULL.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/net/usb/smsc95xx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index d07bf4c..3393238 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1670,12 +1670,13 @@ done:
 static int smsc95xx_resume(struct usb_interface *intf)
 {
 	struct usbnet *dev = usb_get_intfdata(intf);
-	struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
+	struct smsc95xx_priv *pdata;
 	u8 suspend_flags = pdata->suspend_flags;
 	int ret;
 	u32 val;
 
 	BUG_ON(!dev);
+	pdata = (struct smsc95xx_priv *)(dev->data[0]);
 
 	netdev_dbg(dev->net, "resume suspend_flags=0x%02x\n", suspend_flags);
 
-- 
1.8.1.2

^ permalink raw reply related

* [PATCH 4/4] net: Kill skb_copy_datagram_const_iovec
From: Herbert Xu @ 2014-11-07 13:22 UTC (permalink / raw)
  To: David Miller, viro, netdev, linux-kernel, bcrl, YOSHIFUJI Hideaki
In-Reply-To: <20141107132135.GA20022@gondor.apana.org.au>

Now that both macvtap and tun are using skb_copy_datagram_iter, we
can kill the abomination that is skb_copy_datagram_const_iovec.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 include/linux/skbuff.h |    3 -
 net/core/datagram.c    |   89 -------------------------------------------------
 2 files changed, 92 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 933cfce..103fbe8 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2651,9 +2651,6 @@ int skb_copy_datagram_from_iovec(struct sk_buff *skb, int offset,
 				 int len);
 int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *frm,
 			   int offset, size_t count);
-int skb_copy_datagram_const_iovec(const struct sk_buff *from, int offset,
-				  const struct iovec *to, int to_offset,
-				  int size);
 int skb_copy_datagram_iter(const struct sk_buff *from, int offset,
 			   struct iov_iter *to, int size);
 void skb_free_datagram(struct sock *sk, struct sk_buff *skb);
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 84d90d0..26391a3 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -394,95 +394,6 @@ fault:
 EXPORT_SYMBOL(skb_copy_datagram_iovec);
 
 /**
- *	skb_copy_datagram_const_iovec - Copy a datagram to an iovec.
- *	@skb: buffer to copy
- *	@offset: offset in the buffer to start copying from
- *	@to: io vector to copy to
- *	@to_offset: offset in the io vector to start copying to
- *	@len: amount of data to copy from buffer to iovec
- *
- *	Returns 0 or -EFAULT.
- *	Note: the iovec is not modified during the copy.
- */
-int skb_copy_datagram_const_iovec(const struct sk_buff *skb, int offset,
-				  const struct iovec *to, int to_offset,
-				  int len)
-{
-	int start = skb_headlen(skb);
-	int i, copy = start - offset;
-	struct sk_buff *frag_iter;
-
-	/* Copy header. */
-	if (copy > 0) {
-		if (copy > len)
-			copy = len;
-		if (memcpy_toiovecend(to, skb->data + offset, to_offset, copy))
-			goto fault;
-		if ((len -= copy) == 0)
-			return 0;
-		offset += copy;
-		to_offset += copy;
-	}
-
-	/* Copy paged appendix. Hmm... why does this look so complicated? */
-	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
-		int end;
-		const skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
-
-		WARN_ON(start > offset + len);
-
-		end = start + skb_frag_size(frag);
-		if ((copy = end - offset) > 0) {
-			int err;
-			u8  *vaddr;
-			struct page *page = skb_frag_page(frag);
-
-			if (copy > len)
-				copy = len;
-			vaddr = kmap(page);
-			err = memcpy_toiovecend(to, vaddr + frag->page_offset +
-						offset - start, to_offset, copy);
-			kunmap(page);
-			if (err)
-				goto fault;
-			if (!(len -= copy))
-				return 0;
-			offset += copy;
-			to_offset += copy;
-		}
-		start = end;
-	}
-
-	skb_walk_frags(skb, frag_iter) {
-		int end;
-
-		WARN_ON(start > offset + len);
-
-		end = start + frag_iter->len;
-		if ((copy = end - offset) > 0) {
-			if (copy > len)
-				copy = len;
-			if (skb_copy_datagram_const_iovec(frag_iter,
-							  offset - start,
-							  to, to_offset,
-							  copy))
-				goto fault;
-			if ((len -= copy) == 0)
-				return 0;
-			offset += copy;
-			to_offset += copy;
-		}
-		start = end;
-	}
-	if (!len)
-		return 0;
-
-fault:
-	return -EFAULT;
-}
-EXPORT_SYMBOL(skb_copy_datagram_const_iovec);
-
-/**
  *	skb_copy_datagram_iter - Copy a datagram to an iovec iterator.
  *	@skb: buffer to copy
  *	@offset: offset in the buffer to start copying from

^ permalink raw reply related

* [PATCH 3/4] macvtap: Use iovec iterators
From: Herbert Xu @ 2014-11-07 13:22 UTC (permalink / raw)
  To: David Miller, viro, netdev, linux-kernel, bcrl, YOSHIFUJI Hideaki
In-Reply-To: <20141107132135.GA20022@gondor.apana.org.au>

This patch removes the use of skb_copy_datagram_const_iovec in
favour of the iovec iterator-based skb_copy_datagram_iter.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 drivers/net/macvtap.c |   46 +++++++++++++++++++++-------------------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 880cc09..cea99d4 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -15,6 +15,7 @@
 #include <linux/cdev.h>
 #include <linux/idr.h>
 #include <linux/fs.h>
+#include <linux/uio.h>
 
 #include <net/ipv6.h>
 #include <net/net_namespace.h>
@@ -778,31 +779,29 @@ static ssize_t macvtap_aio_write(struct kiocb *iocb, const struct iovec *iv,
 /* Put packet to the user space buffer */
 static ssize_t macvtap_put_user(struct macvtap_queue *q,
 				const struct sk_buff *skb,
-				const struct iovec *iv, int len)
+				struct iov_iter *iter)
 {
 	int ret;
 	int vnet_hdr_len = 0;
 	int vlan_offset = 0;
-	int copied, total;
+	int total;
 
 	if (q->flags & IFF_VNET_HDR) {
 		struct virtio_net_hdr vnet_hdr;
 		vnet_hdr_len = q->vnet_hdr_sz;
-		if ((len -= vnet_hdr_len) < 0)
+		if (iov_iter_count(iter) < vnet_hdr_len)
 			return -EINVAL;
 
 		macvtap_skb_to_vnet_hdr(skb, &vnet_hdr);
 
-		if (memcpy_toiovecend(iv, (void *)&vnet_hdr, 0, sizeof(vnet_hdr)))
+		if (copy_to_iter(&vnet_hdr, sizeof(vnet_hdr), iter) !=
+		    sizeof(vnet_hdr))
 			return -EFAULT;
 	}
-	total = copied = vnet_hdr_len;
+	total = vnet_hdr_len;
 	total += skb->len;
 
-	if (!vlan_tx_tag_present(skb))
-		len = min_t(int, skb->len, len);
-	else {
-		int copy;
+	if (vlan_tx_tag_present(skb)) {
 		struct {
 			__be16 h_vlan_proto;
 			__be16 h_vlan_TCI;
@@ -811,37 +810,33 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
 		veth.h_vlan_TCI = htons(vlan_tx_tag_get(skb));
 
 		vlan_offset = offsetof(struct vlan_ethhdr, h_vlan_proto);
-		len = min_t(int, skb->len + VLAN_HLEN, len);
 		total += VLAN_HLEN;
 
-		copy = min_t(int, vlan_offset, len);
-		ret = skb_copy_datagram_const_iovec(skb, 0, iv, copied, copy);
-		len -= copy;
-		copied += copy;
-		if (ret || !len)
+		ret = skb_copy_datagram_iter(skb, 0, iter, vlan_offset);
+		if (ret || !iov_iter_count(iter))
 			goto done;
 
-		copy = min_t(int, sizeof(veth), len);
-		ret = memcpy_toiovecend(iv, (void *)&veth, copied, copy);
-		len -= copy;
-		copied += copy;
-		if (ret || !len)
+		ret = copy_to_iter(&veth, sizeof(veth), iter);
+		if (ret != sizeof(veth) || !iov_iter_count(iter))
 			goto done;
 	}
 
-	ret = skb_copy_datagram_const_iovec(skb, vlan_offset, iv, copied, len);
+	ret = skb_copy_datagram_iter(skb, vlan_offset, iter,
+				     skb->len - vlan_offset);
 
 done:
 	return ret ? ret : total;
 }
 
 static ssize_t macvtap_do_read(struct macvtap_queue *q,
-			       const struct iovec *iv, unsigned long len,
+			       const struct iovec *iv, unsigned long segs,
+			       unsigned long len,
 			       int noblock)
 {
 	DEFINE_WAIT(wait);
 	struct sk_buff *skb;
 	ssize_t ret = 0;
+	struct iov_iter iter;
 
 	while (len) {
 		if (!noblock)
@@ -863,7 +858,8 @@ static ssize_t macvtap_do_read(struct macvtap_queue *q,
 			schedule();
 			continue;
 		}
-		ret = macvtap_put_user(q, skb, iv, len);
+		iov_iter_init(&iter, READ, iv, segs, len);
+		ret = macvtap_put_user(q, skb, &iter);
 		kfree_skb(skb);
 		break;
 	}
@@ -886,7 +882,7 @@ static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
 		goto out;
 	}
 
-	ret = macvtap_do_read(q, iv, len, file->f_flags & O_NONBLOCK);
+	ret = macvtap_do_read(q, iv, count, len, file->f_flags & O_NONBLOCK);
 	ret = min_t(ssize_t, ret, len);
 	if (ret > 0)
 		iocb->ki_pos = ret;
@@ -1117,7 +1113,7 @@ static int macvtap_recvmsg(struct kiocb *iocb, struct socket *sock,
 	int ret;
 	if (flags & ~(MSG_DONTWAIT|MSG_TRUNC))
 		return -EINVAL;
-	ret = macvtap_do_read(q, m->msg_iov, total_len,
+	ret = macvtap_do_read(q, m->msg_iov, m->msg_iovlen, total_len,
 			  flags & MSG_DONTWAIT);
 	if (ret > total_len) {
 		m->msg_flags |= MSG_TRUNC;

^ permalink raw reply related

* [PATCH 2/4] tun: Use iovec iterators
From: Herbert Xu @ 2014-11-07 13:22 UTC (permalink / raw)
  To: David Miller, viro, netdev, linux-kernel, bcrl, YOSHIFUJI Hideaki
In-Reply-To: <20141107132135.GA20022@gondor.apana.org.au>

This patch removes the use of skb_copy_datagram_const_iovec in
favour of the iovec iterator-based skb_copy_datagram_iter.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

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

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 9dd3746..2ff769b 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -71,6 +71,7 @@
 #include <net/rtnetlink.h>
 #include <net/sock.h>
 #include <linux/seq_file.h>
+#include <linux/uio.h>
 
 #include <asm/uaccess.h>
 
@@ -1230,11 +1231,11 @@ static ssize_t tun_chr_aio_write(struct kiocb *iocb, const struct iovec *iv,
 static ssize_t tun_put_user(struct tun_struct *tun,
 			    struct tun_file *tfile,
 			    struct sk_buff *skb,
-			    const struct iovec *iv, int len)
+			    struct iov_iter *iter)
 {
 	struct tun_pi pi = { 0, skb->protocol };
-	ssize_t total = 0;
-	int vlan_offset = 0, copied;
+	ssize_t total;
+	int vlan_offset;
 	int vlan_hlen = 0;
 	int vnet_hdr_sz = 0;
 
@@ -1244,23 +1245,25 @@ static ssize_t tun_put_user(struct tun_struct *tun,
 	if (tun->flags & TUN_VNET_HDR)
 		vnet_hdr_sz = tun->vnet_hdr_sz;
 
+	total = skb->len + vlan_hlen + vnet_hdr_sz;
+
 	if (!(tun->flags & TUN_NO_PI)) {
-		if ((len -= sizeof(pi)) < 0)
+		if (iov_iter_count(iter) < sizeof(pi))
 			return -EINVAL;
 
-		if (len < skb->len + vlan_hlen + vnet_hdr_sz) {
+		total += sizeof(pi);
+		if (iov_iter_count(iter) < total) {
 			/* Packet will be striped */
 			pi.flags |= TUN_PKT_STRIP;
 		}
 
-		if (memcpy_toiovecend(iv, (void *) &pi, 0, sizeof(pi)))
+		if (copy_to_iter(&pi, sizeof(pi), iter) != sizeof(pi))
 			return -EFAULT;
-		total += sizeof(pi);
 	}
 
 	if (vnet_hdr_sz) {
 		struct virtio_net_hdr gso = { 0 }; /* no info leak */
-		if ((len -= vnet_hdr_sz) < 0)
+		if (iov_iter_count(iter) < vnet_hdr_sz)
 			return -EINVAL;
 
 		if (skb_is_gso(skb)) {
@@ -1299,17 +1302,12 @@ static ssize_t tun_put_user(struct tun_struct *tun,
 			gso.flags = VIRTIO_NET_HDR_F_DATA_VALID;
 		} /* else everything is zero */
 
-		if (unlikely(memcpy_toiovecend(iv, (void *)&gso, total,
-					       sizeof(gso))))
+		if (copy_to_iter(&gso, sizeof(gso), iter) != sizeof(gso))
 			return -EFAULT;
-		total += vnet_hdr_sz;
 	}
 
-	copied = total;
-	len = min_t(int, skb->len + vlan_hlen, len);
-	total += skb->len + vlan_hlen;
 	if (vlan_hlen) {
-		int copy, ret;
+		int ret;
 		struct {
 			__be16 h_vlan_proto;
 			__be16 h_vlan_TCI;
@@ -1320,36 +1318,32 @@ static ssize_t tun_put_user(struct tun_struct *tun,
 
 		vlan_offset = offsetof(struct vlan_ethhdr, h_vlan_proto);
 
-		copy = min_t(int, vlan_offset, len);
-		ret = skb_copy_datagram_const_iovec(skb, 0, iv, copied, copy);
-		len -= copy;
-		copied += copy;
-		if (ret || !len)
+		ret = skb_copy_datagram_iter(skb, 0, iter, vlan_offset);
+		if (ret || !iov_iter_count(iter))
 			goto done;
 
-		copy = min_t(int, sizeof(veth), len);
-		ret = memcpy_toiovecend(iv, (void *)&veth, copied, copy);
-		len -= copy;
-		copied += copy;
-		if (ret || !len)
+		ret = copy_to_iter(&veth, sizeof(veth), iter);
+		if (ret != sizeof(veth) || !iov_iter_count(iter))
 			goto done;
 	}
 
-	skb_copy_datagram_const_iovec(skb, vlan_offset, iv, copied, len);
+	skb_copy_datagram_iter(skb, vlan_offset, iter, skb->len - vlan_offset);
 
 done:
 	tun->dev->stats.tx_packets++;
-	tun->dev->stats.tx_bytes += len;
+	tun->dev->stats.tx_bytes += skb->len + vlan_hlen;
 
 	return total;
 }
 
 static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
-			   const struct iovec *iv, ssize_t len, int noblock)
+			   const struct iovec *iv, unsigned long segs,
+			   ssize_t len, int noblock)
 {
 	struct sk_buff *skb;
 	ssize_t ret = 0;
 	int peeked, err, off = 0;
+	struct iov_iter iter;
 
 	tun_debug(KERN_INFO, tun, "tun_do_read\n");
 
@@ -1362,11 +1356,12 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
 	/* Read frames from queue */
 	skb = __skb_recv_datagram(tfile->socket.sk, noblock ? MSG_DONTWAIT : 0,
 				  &peeked, &off, &err);
-	if (skb) {
-		ret = tun_put_user(tun, tfile, skb, iv, len);
-		kfree_skb(skb);
-	} else
-		ret = err;
+	if (!skb)
+		return ret;
+
+	iov_iter_init(&iter, READ, iv, segs, len);
+	ret = tun_put_user(tun, tfile, skb, &iter);
+	kfree_skb(skb);
 
 	return ret;
 }
@@ -1387,7 +1382,7 @@ static ssize_t tun_chr_aio_read(struct kiocb *iocb, const struct iovec *iv,
 		goto out;
 	}
 
-	ret = tun_do_read(tun, tfile, iv, len,
+	ret = tun_do_read(tun, tfile, iv, count, len,
 			  file->f_flags & O_NONBLOCK);
 	ret = min_t(ssize_t, ret, len);
 	if (ret > 0)
@@ -1488,7 +1483,7 @@ static int tun_recvmsg(struct kiocb *iocb, struct socket *sock,
 					 SOL_PACKET, TUN_TX_TIMESTAMP);
 		goto out;
 	}
-	ret = tun_do_read(tun, tfile, m->msg_iov, total_len,
+	ret = tun_do_read(tun, tfile, m->msg_iov, m->msg_iovlen, total_len,
 			  flags & MSG_DONTWAIT);
 	if (ret > total_len) {
 		m->msg_flags |= MSG_TRUNC;

^ permalink raw reply related

* [PATCH 1/4] inet: Add skb_copy_datagram_iter
From: Herbert Xu @ 2014-11-07 13:22 UTC (permalink / raw)
  To: David Miller, viro, netdev, linux-kernel, bcrl, YOSHIFUJI Hideaki
In-Reply-To: <20141107132135.GA20022@gondor.apana.org.au>

This patch adds skb_copy_datagram_iter, which is identical to
skb_copy_datagram_iovec except that it operates on iov_iter
instead of iovec.

Eventually all users of skb_copy_datagram_iovec should switch
over to iov_iter and then we can remove skb_copy_datagram_iovec.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 include/linux/skbuff.h |    3 +
 net/core/datagram.c    |   87 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 53f4f6c..933cfce 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -150,6 +150,7 @@
 struct net_device;
 struct scatterlist;
 struct pipe_inode_info;
+struct iov_iter;
 
 #if defined(CONFIG_NF_CONNTRACK) || defined(CONFIG_NF_CONNTRACK_MODULE)
 struct nf_conntrack {
@@ -2653,6 +2654,8 @@ int zerocopy_sg_from_iovec(struct sk_buff *skb, const struct iovec *frm,
 int skb_copy_datagram_const_iovec(const struct sk_buff *from, int offset,
 				  const struct iovec *to, int to_offset,
 				  int size);
+int skb_copy_datagram_iter(const struct sk_buff *from, int offset,
+			   struct iov_iter *to, int size);
 void skb_free_datagram(struct sock *sk, struct sk_buff *skb);
 void skb_free_datagram_locked(struct sock *sk, struct sk_buff *skb);
 int skb_kill_datagram(struct sock *sk, struct sk_buff *skb, unsigned int flags);
diff --git a/net/core/datagram.c b/net/core/datagram.c
index fdbc9a8..84d90d0 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -49,6 +49,7 @@
 #include <linux/spinlock.h>
 #include <linux/slab.h>
 #include <linux/pagemap.h>
+#include <linux/uio.h>
 
 #include <net/protocol.h>
 #include <linux/skbuff.h>
@@ -482,6 +483,92 @@ fault:
 EXPORT_SYMBOL(skb_copy_datagram_const_iovec);
 
 /**
+ *	skb_copy_datagram_iter - Copy a datagram to an iovec iterator.
+ *	@skb: buffer to copy
+ *	@offset: offset in the buffer to start copying from
+ *	@to: iovec iterator to copy to
+ *	@len: amount of data to copy from buffer to iovec
+ */
+int skb_copy_datagram_iter(const struct sk_buff *skb, int offset,
+			   struct iov_iter *to, int len)
+{
+	int start = skb_headlen(skb);
+	int i, copy = start - offset;
+	struct sk_buff *frag_iter;
+
+	trace_skb_copy_datagram_iovec(skb, len);
+
+	/* Copy header. */
+	if (copy > 0) {
+		if (copy > len)
+			copy = len;
+		if (copy_to_iter(skb->data + offset, copy, to) != copy)
+			goto short_copy;
+		if ((len -= copy) == 0)
+			return 0;
+		offset += copy;
+	}
+
+	/* Copy paged appendix. Hmm... why does this look so complicated? */
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+		int end;
+		const skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+
+		WARN_ON(start > offset + len);
+
+		end = start + skb_frag_size(frag);
+		if ((copy = end - offset) > 0) {
+			if (copy > len)
+				copy = len;
+			if (copy_page_to_iter(skb_frag_page(frag),
+					      frag->page_offset + offset -
+					      start, copy, to) != copy)
+				goto short_copy;
+			if (!(len -= copy))
+				return 0;
+			offset += copy;
+		}
+		start = end;
+	}
+
+	skb_walk_frags(skb, frag_iter) {
+		int end;
+
+		WARN_ON(start > offset + len);
+
+		end = start + frag_iter->len;
+		if ((copy = end - offset) > 0) {
+			if (copy > len)
+				copy = len;
+			if (skb_copy_datagram_iter(frag_iter, offset - start,
+						   to, copy))
+				goto fault;
+			if ((len -= copy) == 0)
+				return 0;
+			offset += copy;
+		}
+		start = end;
+	}
+	if (!len)
+		return 0;
+
+	/* This is not really a user copy fault, but rather someone
+	 * gave us a bogus length on the skb.  We should probably
+	 * print a warning here as it may indicate a kernel bug.
+	 */
+
+fault:
+	return -EFAULT;
+
+short_copy:
+	if (iov_iter_count(to))
+		goto fault;
+
+	return 0;
+}
+EXPORT_SYMBOL(skb_copy_datagram_iter);
+
+/**
  *	skb_copy_datagram_from_iovec - Copy a datagram from an iovec.
  *	@skb: buffer to copy
  *	@offset: offset in the buffer to start copying to

^ permalink raw reply related

* Re: [REGRESSION] in 3.18-rc1: ppp crashes kernel
From: Takashi Iwai @ 2014-11-07 13:22 UTC (permalink / raw)
  To: Stefan Seyfried; +Cc: Paul Mackerras, linux-ppp, netdev, LKML
In-Reply-To: <545CA8B6.2060608@message-id.googlemail.com>

At Fri, 07 Nov 2014 12:10:46 +0100,
Stefan Seyfried wrote:
> 
> Hi all,
> 
> since 3.18-rc1, setting up a PPP interface kills my kernel with
> 
> [  163.433251] PPP generic driver version 2.4.2
> [  164.452474] ------------[ cut here ]------------
> [  164.453327] kernel BUG at ../mm/vmalloc.c:1316!
> [  164.453327] invalid opcode: 0000 [#1] PREEMPT SMP 
> [  164.453327] Modules linked in: ppp_async crc_ccitt ppp_generic slhc af_packet xfs libcrc32c coretemp kvm_intel 
> snd_hda_codec_conexant iTCO_wdt snd_hda_codec_generic iTCO_vendor_support uvcvideo snd_hda_intel snd_hda_controller mac80211 videobuf2_vmalloc snd_hda_codec kvm e1000e videobuf2_memops cfg80211 videobuf2_core v4l2_common snd_hwdep i2c_i801 videodev snd_pcm pcspkr thinkpad_acpi serio_raw wmi lpc_ich snd_timer thermal snd rfkill mfd_core tpm_tis shpchp mei_me soundcore ptp mei pps_core acpi_cpufreq tpm battery processor ac dm_mod btrfs xor raid6_pq i915 i2c_algo_bit drm_kms_helper drm video button sg
> [  164.453327] CPU: 0 PID: 6927 Comm: pppd Not tainted 3.18.0-rc3-3.ge706e91-desktop #1
> [  164.453327] Hardware name: LENOVO 7470E36/7470E36, BIOS 6DET61WW (3.11 ) 11/10/2009
> 
> This is easy to reproduce with:
> 
> linux:~ # cat bin/crashme.sh 
> ----
> #!/bin/bash -x
> pppd local pty "netcat -l 1234" &
> sleep 1
> pppd local pty "netcat localhost 1234" &
> sleep 1
> ----
> 
> 3.17 works fine.
> I bisected the issue multiple times and always arrived at
> 
> # first bad commit: [d6dd50e07c5bec00db2005969b1a01f8ca3d25ef] Merge branch 'core-rcu-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
> 
> which is a merge commit unfortunately.
> 
> The BUG encountered above is in:
> 
> 1309 static struct vm_struct *__get_vm_area_node(unsigned long size,
> 1310                 unsigned long align, unsigned long flags, unsigned long start,
> 1311                 unsigned long end, int node, gfp_t gfp_mask, const void *caller)
> 1312 {
> 1313         struct vmap_area *va;
> 1314         struct vm_struct *area;
> 1315 
> 1316         BUG_ON(in_interrupt());
> 1317         if (flags & VM_IOREMAP)
> 1318                 align = 1ul << clamp(fls(size), PAGE_SHIFT, IOREMAP_MAX_ORDER);
> 1319 
> 
> the call trace is:
> [  164.453327] Call Trace:
> [  164.453327]  [<ffffffff811974bd>] __vmalloc_node_range+0x6d/0x290
> [  164.453327]  [<ffffffff8119771e>] __vmalloc+0x3e/0x50
> [  164.453327]  [<ffffffff81146950>] bpf_prog_alloc+0x30/0xa0
> [  164.453327]  [<ffffffff8157b716>] bpf_prog_create+0x46/0xb0
> [  164.453327]  [<ffffffffa07ecb90>] ppp_ioctl+0x420/0xe9a [ppp_generic]
> [  164.453327]  [<ffffffff811df1c7>] do_vfs_ioctl+0x2e7/0x4c0
> [  164.453327]  [<ffffffff811df421>] SyS_ioctl+0x81/0xa0
> [  164.453327]  [<ffffffff8165ee2d>] system_call_fastpath+0x16/0x1b
> [  164.453327]  [<00007f4502d87397>] 0x7f4502d87397

bpf_prog_create() is called inside spin_lock_bh(), and the BUG_ON()
hits.  Below is a quick fix.


Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] net: ppp: Don't call bpf_prog_create() in ppp_lock

In ppp_ioctl(), bpf_prog_create() is called inside ppp_lock, which
eventually calls vmalloc() and hits BUG_ON() in vmalloc.c.  This patch
works around the problem by moving the allocation outside the lock.

Reported-by: Stefan Seyfried <stefan.seyfried@googlemail.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/net/ppp/ppp_generic.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 68c3a3f4e0ab..794a47329368 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -755,23 +755,23 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 		err = get_filter(argp, &code);
 		if (err >= 0) {
+			struct bpf_prog *pass_filter = NULL;
 			struct sock_fprog_kern fprog = {
 				.len = err,
 				.filter = code,
 			};
 
-			ppp_lock(ppp);
-			if (ppp->pass_filter) {
-				bpf_prog_destroy(ppp->pass_filter);
-				ppp->pass_filter = NULL;
+			err = 0;
+			if (fprog.filter)
+				err = bpf_prog_create(&pass_filter, &fprog);
+			if (!err) {
+				ppp_lock(ppp);
+				if (ppp->pass_filter)
+					bpf_prog_destroy(ppp->pass_filter);
+				ppp->pass_filter = pass_filter;
+				ppp_unlock(ppp);
 			}
-			if (fprog.filter != NULL)
-				err = bpf_prog_create(&ppp->pass_filter,
-						      &fprog);
-			else
-				err = 0;
 			kfree(code);
-			ppp_unlock(ppp);
 		}
 		break;
 	}
@@ -781,23 +781,23 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 
 		err = get_filter(argp, &code);
 		if (err >= 0) {
+			struct bpf_prog *active_filter = NULL;
 			struct sock_fprog_kern fprog = {
 				.len = err,
 				.filter = code,
 			};
 
-			ppp_lock(ppp);
-			if (ppp->active_filter) {
-				bpf_prog_destroy(ppp->active_filter);
-				ppp->active_filter = NULL;
+			err = 0;
+			if (fprog.filter)
+				err = bpf_prog_create(&active_filter, &fprog);
+			if (!err) {
+				ppp_lock(ppp);
+				if (ppp->active_filter)
+					bpf_prog_destroy(ppp->active_filter);
+				ppp->active_filter = active_filter;
+				ppp_unlock(ppp);
 			}
-			if (fprog.filter != NULL)
-				err = bpf_prog_create(&ppp->active_filter,
-						      &fprog);
-			else
-				err = 0;
 			kfree(code);
-			ppp_unlock(ppp);
 		}
 		break;
 	}
-- 
2.1.3

^ permalink raw reply related

* [PATCH 0/4] Replace skb_copy_datagram_const_iovec with iterator version
From: Herbert Xu @ 2014-11-07 13:21 UTC (permalink / raw)
  To: David Miller; +Cc: viro, netdev, linux-kernel, bcrl, YOSHIFUJI Hideaki
In-Reply-To: <20141106.221313.1375322315446025123.davem@davemloft.net>

Hi Dave:

This patch series adds the helper skb_copy_datagram_iter, which
is meant to replace both skb_copy_datagram_iovec and its evil
twin skb_copy_datagram_const_iovec.

It then converts tun and macvtap over to the new helper and finally
removes skb_copy_datagram_const_iovec which is only used by tun
and macvtap.

The copy_to_iter return value issue pointed out by Al has now been
fixed.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [Xen-devel] BUG in xennet_make_frags with paged skb data
From: Stefan Bader @ 2014-11-07 12:28 UTC (permalink / raw)
  To: Zoltan Kiss, Eric Dumazet
  Cc: netdev, David S. Miller, Konrad Rzeszutek Wilk, Boris Ostrovsky,
	David Vrabel, Jay Vosburgh, linux-kernel, xen-devel
In-Reply-To: <545CB92E.9050106@linaro.org>

[-- Attachment #1: Type: text/plain, Size: 1357 bytes --]

On 07.11.2014 13:21, Zoltan Kiss wrote:
> 
> 
> On 07/11/14 12:15, Stefan Bader wrote:
>> On 07.11.2014 12:22, Eric Dumazet wrote:
>>> On Fri, 2014-11-07 at 09:25 +0000, Zoltan Kiss wrote:
>>>
>>> Please do not top post.
>>>
>>>> Hi,
>>>>
>>>> AFAIK in this scenario your skb frag is wrong. The page pointer should
>>>> point to the original compound page (not a member of it), and offset
>>>> should be set accordingly.
>>>> For example, if your compound page is 16K (4 page), then the page
>>>> pointer should point to the first page, and if the data starts at the
>>>> 3rd page, then offset should be >8K
>>>
>>> This is not accurate.
>>>
>>> This BUG_ON() is wrong.
>>>
>>> It should instead be :
>>>
>>> BUG_ON(len + offset > PAGE_SIZE<<compound_order(compound_head(page)));
>>
>> would that not have to be
>>
>> BUG_ON((page-compound_head(page)*PAGE_SIZE)+offset+len >
>> PAGE_SIZE<<compound_order(compound_head(page)));
> 
> There should be a parentheses around "page-compound_head(page)".

*sigh* before submitting anything I'll have to make sure I run it past the
compiler at least. I never manager to type the beast without some error.

But you got the drift...

>>
>> since offset is adjusted to start from the tail page in that case.
>>>
>>> splice() code can generate such cases.
>>>
>>>
>>
>>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCHv4 1/1] net: fec: fix regression on i.MX28 introduced by rx_copybreak support
From: Fabio Estevam @ 2014-11-07 12:26 UTC (permalink / raw)
  To: Lothar Waßmann
  Cc: netdev@vger.kernel.org, Fabio Estevam, Frank Li, linux-kernel,
	Russell King, David S. Miller,
	linux-arm-kernel@lists.infradead.org, Stefan Wahren
In-Reply-To: <1415350967-2238-2-git-send-email-LW@KARO-electronics.de>

On Fri, Nov 7, 2014 at 7:02 AM, Lothar Waßmann <LW@karo-electronics.de> wrote:
> commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> introduced a regression for i.MX28. The swap_buffer() function doing
> the endian conversion of the received data on i.MX28 may access memory
> beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> does not copy those bytes, so that the last bytes of a packet may be
> filled with invalid data after swapping.
> This will likely lead to checksum errors on received packets.
> E.g. when trying to mount an NFS rootfs:
> UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
>
> Do the byte swapping and copying to the new skb in one go if
> necessary.
>
> Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>

With this patch I am able to NFS mount on mx28 again. Thanks, Lothar!

Tested-by: Fabio Estevam <fabio.estevam@freescale.com>

^ permalink raw reply

* Re: [Xen-devel] BUG in xennet_make_frags with paged skb data
From: Zoltan Kiss @ 2014-11-07 12:21 UTC (permalink / raw)
  To: Stefan Bader, Eric Dumazet
  Cc: netdev, David S. Miller, Konrad Rzeszutek Wilk, Boris Ostrovsky,
	David Vrabel, Jay Vosburgh, linux-kernel, xen-devel
In-Reply-To: <545CB7FF.8080003@canonical.com>



On 07/11/14 12:15, Stefan Bader wrote:
> On 07.11.2014 12:22, Eric Dumazet wrote:
>> On Fri, 2014-11-07 at 09:25 +0000, Zoltan Kiss wrote:
>>
>> Please do not top post.
>>
>>> Hi,
>>>
>>> AFAIK in this scenario your skb frag is wrong. The page pointer should
>>> point to the original compound page (not a member of it), and offset
>>> should be set accordingly.
>>> For example, if your compound page is 16K (4 page), then the page
>>> pointer should point to the first page, and if the data starts at the
>>> 3rd page, then offset should be >8K
>>
>> This is not accurate.
>>
>> This BUG_ON() is wrong.
>>
>> It should instead be :
>>
>> BUG_ON(len + offset > PAGE_SIZE<<compound_order(compound_head(page)));
>
> would that not have to be
>
> BUG_ON((page-compound_head(page)*PAGE_SIZE)+offset+len >
> PAGE_SIZE<<compound_order(compound_head(page)));

There should be a parentheses around "page-compound_head(page)".
>
> since offset is adjusted to start from the tail page in that case.
>>
>> splice() code can generate such cases.
>>
>>
>
>

^ permalink raw reply

* Re: [Xen-devel] BUG in xennet_make_frags with paged skb data
From: Stefan Bader @ 2014-11-07 12:15 UTC (permalink / raw)
  To: Eric Dumazet, Zoltan Kiss
  Cc: netdev, David S. Miller, Konrad Rzeszutek Wilk, Boris Ostrovsky,
	David Vrabel, Jay Vosburgh, linux-kernel, xen-devel
In-Reply-To: <1415359320.13896.105.camel@edumazet-glaptop2.roam.corp.google.com>

[-- Attachment #1: Type: text/plain, Size: 948 bytes --]

On 07.11.2014 12:22, Eric Dumazet wrote:
> On Fri, 2014-11-07 at 09:25 +0000, Zoltan Kiss wrote:
> 
> Please do not top post.
> 
>> Hi,
>>
>> AFAIK in this scenario your skb frag is wrong. The page pointer should 
>> point to the original compound page (not a member of it), and offset 
>> should be set accordingly.
>> For example, if your compound page is 16K (4 page), then the page 
>> pointer should point to the first page, and if the data starts at the 
>> 3rd page, then offset should be >8K
> 
> This is not accurate.
> 
> This BUG_ON() is wrong.
> 
> It should instead be :
> 
> BUG_ON(len + offset > PAGE_SIZE<<compound_order(compound_head(page)));

would that not have to be

BUG_ON((page-compound_head(page)*PAGE_SIZE)+offset+len >
PAGE_SIZE<<compound_order(compound_head(page)));

since offset is adjusted to start from the tail page in that case.
> 
> splice() code can generate such cases.
> 
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH V4 3/3] can: m_can: add missing message RAM initialization
From: Oliver Hartkopp @ 2014-11-07 11:53 UTC (permalink / raw)
  To: Dong Aisheng, linux-can; +Cc: mkl, wg, varkabhadram, netdev, linux-arm-kernel
In-Reply-To: <1415349914-9145-3-git-send-email-b29396@freescale.com>

Thanks!

Hopefully we can manage to push the patch series via can/master ;-)

Regards,
Oliver

On 07.11.2014 09:45, Dong Aisheng wrote:
> The M_CAN message RAM is usually equipped with a parity or ECC functionality.
> But RAM cells suffer a hardware reset and can therefore hold arbitrary
> content at startup - including parity and/or ECC bits.
>
> To prevent the M_CAN controller detecting checksum errors when reading
> potentially uninitialized TX message RAM content to transmit CAN
> frames the TX message RAM has to be written with (any kind of) initial
> data.
>
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> ---
> ChangeLog:
> v3-v4:
>   * initialize the entire Message RAM in use instead of only 8 bytes
>     since this is a valid and needed initialization process.
>     Patch title also changed.
> ---
>   drivers/net/can/m_can/m_can.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index eee1533..de742d0 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -1114,7 +1114,7 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
>   	struct resource *res;
>   	void __iomem *addr;
>   	u32 out_val[MRAM_CFG_LEN];
> -	int ret;
> +	int i, start, end, ret;
>
>   	/* message ram could be shared */
>   	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
> @@ -1165,6 +1165,15 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
>   		priv->mcfg[MRAM_TXE].off, priv->mcfg[MRAM_TXE].num,
>   		priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num);
>
> +	/* initialize the entire Message RAM in use to avoid possible
> +	 * ECC/parity checksum errors when reading an uninitialized buffer
> +	 */
> +	start = priv->mcfg[MRAM_SIDF].off;
> +	end = priv->mcfg[MRAM_TXB].off +
> +		priv->mcfg[MRAM_TXB].num * TXB_ELEMENT_SIZE;
> +	for (i = start; i < end; i += 4)
> +		writel(0x0, priv->mram_base + i);
> +
>   	return 0;
>   }
>
>

^ permalink raw reply

* [PATCH net 3/3] cxgb4vf: FL Starvation Threshold needs to be larger than the SGE's Egress Congestion Threshold
From: Hariprasad Shenai @ 2014-11-07 11:36 UTC (permalink / raw)
  To: netdev; +Cc: davem, leedom, nirranjan, Hariprasad Shenai
In-Reply-To: <1415360191-25395-1-git-send-email-hariprasad@chelsio.com>

Free List Starvation Threshold needs to be larger than the SGE's Egress
Congestion Threshold or we'll end up in a mutual stall where the driver waits
for Ingress Packets to drive replacing Free List Pointers and the SGE waits for
Free List Pointers before pushing Ingress Packets to the host.

Based on original work by Casey Leedom <leedom@chelsio.com>

Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4vf/sge.c         |   16 ++++++++++------
 drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h |    1 +
 drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c     |    5 ++++-
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
index cd5b789..fdd078d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
@@ -94,12 +94,6 @@ enum {
 	MAX_TIMER_TX_RECLAIM = 100,
 
 	/*
-	 * An FL with <= FL_STARVE_THRES buffers is starving and a periodic
-	 * timer will attempt to refill it.
-	 */
-	FL_STARVE_THRES = 4,
-
-	/*
 	 * Suspend an Ethernet TX queue with fewer available descriptors than
 	 * this.  We always want to have room for a maximum sized packet:
 	 * inline immediate data + MAX_SKB_FRAGS. This is the same as
@@ -2490,6 +2484,16 @@ int t4vf_sge_init(struct adapter *adapter)
 		s->fl_align = max(ingpadboundary, ingpackboundary);
 	}
 
+	/* A FL with <= fl_starve_thres buffers is starving and a periodic
+	 * timer will attempt to refill it.  This needs to be larger than the
+	 * SGE's Egress Congestion Threshold.  If it isn't, then we can get
+	 * stuck waiting for new packets while the SGE is waiting for us to
+	 * give it more Free List entries.  (Note that the SGE's Egress
+	 * Congestion Threshold is in units of 2 Free List pointers.)
+	 */
+	s->fl_starve_thres
+		= EGRTHRESHOLD_GET(sge_params->sge_congestion_control)*2 + 1;
+
 	/*
 	 * Set up tasklet timers.
 	 */
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h
index b5c301d..4b6a6d1 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h
@@ -140,6 +140,7 @@ struct sge_params {
 	u32 sge_user_mode_limits;	/* limits for BAR2 user mode accesses */
 	u32 sge_fl_buffer_size[16];	/* free list buffer sizes */
 	u32 sge_ingress_rx_threshold;	/* RX counter interrupt threshold[4] */
+	u32 sge_congestion_control;     /* congestion thresholds, etc. */
 	u32 sge_timer_value_0_and_1;	/* interrupt coalescing timer values */
 	u32 sge_timer_value_2_and_3;
 	u32 sge_timer_value_4_and_5;
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
index dc30d28..1e896b9 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
@@ -493,10 +493,13 @@ int t4vf_get_sge_params(struct adapter *adapter)
 
 	params[0] = (FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
 		     FW_PARAMS_PARAM_XYZ(SGE_INGRESS_RX_THRESHOLD));
-	v = t4vf_query_params(adapter, 1, params, vals);
+	params[1] = (FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
+		     FW_PARAMS_PARAM_XYZ(SGE_CONM_CTRL));
+	v = t4vf_query_params(adapter, 2, params, vals);
 	if (v)
 		return v;
 	sge_params->sge_ingress_rx_threshold = vals[0];
+	sge_params->sge_congestion_control = vals[1];
 
 	return 0;
 }
-- 
1.7.1

^ permalink raw reply related

* [PATCH net 2/3] cxgb4/cxgb4vf: For T5 use Packing and Padding Boundaries for SGE DMA transfers
From: Hariprasad Shenai @ 2014-11-07 11:36 UTC (permalink / raw)
  To: netdev; +Cc: davem, leedom, nirranjan, Hariprasad Shenai
In-Reply-To: <1415360191-25395-1-git-send-email-hariprasad@chelsio.com>

T5 introduces the ability to have separate Packing and Padding Boundaries
for SGE DMA transfers from the chip to Host Memory. This change set takes
advantage of that to set up a smaller Padding Boundary to conserve PCI Link
and Memory Bandwidth with T5.

Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4/sge.c           |   30 ++++++++++-
 drivers/net/ethernet/chelsio/cxgb4/t4_hw.c         |   51 +++++++++++++++++--
 drivers/net/ethernet/chelsio/cxgb4/t4_regs.h       |   10 ++++
 drivers/net/ethernet/chelsio/cxgb4vf/sge.c         |   31 +++++++++++-
 drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h |    1 +
 drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c     |   23 +++++++++
 6 files changed, 135 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c b/drivers/net/ethernet/chelsio/cxgb4/sge.c
index 5e1b314..39f2b13 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -2914,7 +2914,8 @@ static int t4_sge_init_hard(struct adapter *adap)
 int t4_sge_init(struct adapter *adap)
 {
 	struct sge *s = &adap->sge;
-	u32 sge_control, sge_conm_ctrl;
+	u32 sge_control, sge_control2, sge_conm_ctrl;
+	unsigned int ingpadboundary, ingpackboundary;
 	int ret, egress_threshold;
 
 	/*
@@ -2924,8 +2925,31 @@ int t4_sge_init(struct adapter *adap)
 	sge_control = t4_read_reg(adap, SGE_CONTROL);
 	s->pktshift = PKTSHIFT_GET(sge_control);
 	s->stat_len = (sge_control & EGRSTATUSPAGESIZE_MASK) ? 128 : 64;
-	s->fl_align = 1 << (INGPADBOUNDARY_GET(sge_control) +
-			    X_INGPADBOUNDARY_SHIFT);
+
+	/* T4 uses a single control field to specify both the PCIe Padding and
+	 * Packing Boundary.  T5 introduced the ability to specify these
+	 * separately.  The actual Ingress Packet Data alignment boundary
+	 * within Packed Buffer Mode is the maximum of these two
+	 * specifications.
+	 */
+	ingpadboundary = 1 << (INGPADBOUNDARY_GET(sge_control) +
+			       X_INGPADBOUNDARY_SHIFT);
+	if (is_t4(adap->params.chip)) {
+		s->fl_align = ingpadboundary;
+	} else {
+		/* T5 has a different interpretation of one of the PCIe Packing
+		 * Boundary values.
+		 */
+		sge_control2 = t4_read_reg(adap, SGE_CONTROL2_A);
+		ingpackboundary = INGPACKBOUNDARY_G(sge_control2);
+		if (ingpackboundary == INGPACKBOUNDARY_16B_X)
+			ingpackboundary = 16;
+		else
+			ingpackboundary = 1 << (ingpackboundary +
+						INGPACKBOUNDARY_SHIFT_X);
+
+		s->fl_align = max(ingpadboundary, ingpackboundary);
+	}
 
 	if (adap->flags & USING_SOFT_PARAMS)
 		ret = t4_sge_init_soft(adap);
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
index a9d9d74..163a2a1 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
@@ -3129,12 +3129,51 @@ int t4_fixup_host_params(struct adapter *adap, unsigned int page_size,
 		     HOSTPAGESIZEPF6(sge_hps) |
 		     HOSTPAGESIZEPF7(sge_hps));
 
-	t4_set_reg_field(adap, SGE_CONTROL,
-			 INGPADBOUNDARY_MASK |
-			 EGRSTATUSPAGESIZE_MASK,
-			 INGPADBOUNDARY(fl_align_log - 5) |
-			 EGRSTATUSPAGESIZE(stat_len != 64));
-
+	if (is_t4(adap->params.chip)) {
+		t4_set_reg_field(adap, SGE_CONTROL,
+				 INGPADBOUNDARY_MASK |
+				 EGRSTATUSPAGESIZE_MASK,
+				 INGPADBOUNDARY(fl_align_log - 5) |
+				 EGRSTATUSPAGESIZE(stat_len != 64));
+	} else {
+		/* T5 introduced the separation of the Free List Padding and
+		 * Packing Boundaries.  Thus, we can select a smaller Padding
+		 * Boundary to avoid uselessly chewing up PCIe Link and Memory
+		 * Bandwidth, and use a Packing Boundary which is large enough
+		 * to avoid false sharing between CPUs, etc.
+		 *
+		 * For the PCI Link, the smaller the Padding Boundary the
+		 * better.  For the Memory Controller, a smaller Padding
+		 * Boundary is better until we cross under the Memory Line
+		 * Size (the minimum unit of transfer to/from Memory).  If we
+		 * have a Padding Boundary which is smaller than the Memory
+		 * Line Size, that'll involve a Read-Modify-Write cycle on the
+		 * Memory Controller which is never good.  For T5 the smallest
+		 * Padding Boundary which we can select is 32 bytes which is
+		 * larger than any known Memory Controller Line Size so we'll
+		 * use that.
+		 *
+		 * T5 has a different interpretation of the "0" value for the
+		 * Packing Boundary.  This corresponds to 16 bytes instead of
+		 * the expected 32 bytes.  We never have a Packing Boundary
+		 * less than 32 bytes so we can't use that special value but
+		 * on the other hand, if we wanted 32 bytes, the best we can
+		 * really do is 64 bytes.
+		*/
+		if (fl_align <= 32) {
+			fl_align = 64;
+			fl_align_log = 6;
+		}
+		t4_set_reg_field(adap, SGE_CONTROL,
+				 INGPADBOUNDARY_MASK |
+				 EGRSTATUSPAGESIZE_MASK,
+				 INGPADBOUNDARY(INGPCIEBOUNDARY_32B_X) |
+				 EGRSTATUSPAGESIZE(stat_len != 64));
+		t4_set_reg_field(adap, SGE_CONTROL2_A,
+				 INGPACKBOUNDARY_V(INGPACKBOUNDARY_M),
+				 INGPACKBOUNDARY_V(fl_align_log -
+						 INGPACKBOUNDARY_SHIFT_X));
+	}
 	/*
 	 * Adjust various SGE Free List Host Buffer Sizes.
 	 *
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_regs.h b/drivers/net/ethernet/chelsio/cxgb4/t4_regs.h
index a1024db..8d2de10 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_regs.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_regs.h
@@ -95,6 +95,7 @@
 #define X_INGPADBOUNDARY_SHIFT 5
 
 #define SGE_CONTROL 0x1008
+#define SGE_CONTROL2_A		0x1124
 #define  DCASYSTYPE             0x00080000U
 #define  RXPKTCPLMODE_MASK      0x00040000U
 #define  RXPKTCPLMODE_SHIFT     18
@@ -106,6 +107,7 @@
 #define  PKTSHIFT_SHIFT         10
 #define  PKTSHIFT(x)            ((x) << PKTSHIFT_SHIFT)
 #define  PKTSHIFT_GET(x)	(((x) & PKTSHIFT_MASK) >> PKTSHIFT_SHIFT)
+#define  INGPCIEBOUNDARY_32B_X	0
 #define  INGPCIEBOUNDARY_MASK   0x00000380U
 #define  INGPCIEBOUNDARY_SHIFT  7
 #define  INGPCIEBOUNDARY(x)     ((x) << INGPCIEBOUNDARY_SHIFT)
@@ -114,6 +116,14 @@
 #define  INGPADBOUNDARY(x)      ((x) << INGPADBOUNDARY_SHIFT)
 #define  INGPADBOUNDARY_GET(x)	(((x) & INGPADBOUNDARY_MASK) \
 				 >> INGPADBOUNDARY_SHIFT)
+#define  INGPACKBOUNDARY_16B_X	0
+#define  INGPACKBOUNDARY_SHIFT_X 5
+
+#define  INGPACKBOUNDARY_S	16
+#define  INGPACKBOUNDARY_M	0x7U
+#define  INGPACKBOUNDARY_V(x)	((x) << INGPACKBOUNDARY_S)
+#define  INGPACKBOUNDARY_G(x)	(((x) >> INGPACKBOUNDARY_S) \
+				 & INGPACKBOUNDARY_M)
 #define  EGRPCIEBOUNDARY_MASK   0x0000000eU
 #define  EGRPCIEBOUNDARY_SHIFT  1
 #define  EGRPCIEBOUNDARY(x)     ((x) << EGRPCIEBOUNDARY_SHIFT)
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
index a18830d..cd5b789 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
@@ -2436,6 +2436,7 @@ int t4vf_sge_init(struct adapter *adapter)
 	u32 fl0 = sge_params->sge_fl_buffer_size[0];
 	u32 fl1 = sge_params->sge_fl_buffer_size[1];
 	struct sge *s = &adapter->sge;
+	unsigned int ingpadboundary, ingpackboundary;
 
 	/*
 	 * Start by vetting the basic SGE parameters which have been set up by
@@ -2460,8 +2461,34 @@ int t4vf_sge_init(struct adapter *adapter)
 	s->stat_len = ((sge_params->sge_control & EGRSTATUSPAGESIZE_MASK)
 			? 128 : 64);
 	s->pktshift = PKTSHIFT_GET(sge_params->sge_control);
-	s->fl_align = 1 << (INGPADBOUNDARY_GET(sge_params->sge_control) +
-			    SGE_INGPADBOUNDARY_SHIFT);
+
+	/* T4 uses a single control field to specify both the PCIe Padding and
+	 * Packing Boundary.  T5 introduced the ability to specify these
+	 * separately.  The actual Ingress Packet Data alignment boundary
+	 * within Packed Buffer Mode is the maximum of these two
+	 * specifications.  (Note that it makes no real practical sense to
+	 * have the Pading Boudary be larger than the Packing Boundary but you
+	 * could set the chip up that way and, in fact, legacy T4 code would
+	 * end doing this because it would initialize the Padding Boundary and
+	 * leave the Packing Boundary initialized to 0 (16 bytes).)
+	 */
+	ingpadboundary = 1 << (INGPADBOUNDARY_GET(sge_params->sge_control) +
+			       X_INGPADBOUNDARY_SHIFT);
+	if (is_t4(adapter->params.chip)) {
+		s->fl_align = ingpadboundary;
+	} else {
+		/* T5 has a different interpretation of one of the PCIe Packing
+		 * Boundary values.
+		 */
+		ingpackboundary = INGPACKBOUNDARY_G(sge_params->sge_control2);
+		if (ingpackboundary == INGPACKBOUNDARY_16B_X)
+			ingpackboundary = 16;
+		else
+			ingpackboundary = 1 << (ingpackboundary +
+						INGPACKBOUNDARY_SHIFT_X);
+
+		s->fl_align = max(ingpadboundary, ingpackboundary);
+	}
 
 	/*
 	 * Set up tasklet timers.
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h
index 95df61d..b5c301d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h
@@ -134,6 +134,7 @@ struct dev_params {
  */
 struct sge_params {
 	u32 sge_control;		/* padding, boundaries, lengths, etc. */
+	u32 sge_control2;		/* T5: more of the same */
 	u32 sge_host_page_size;		/* RDMA page sizes */
 	u32 sge_queues_per_page;	/* RDMA queues/page */
 	u32 sge_user_mode_limits;	/* limits for BAR2 user mode accesses */
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
index e984fdc..dc30d28 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c
@@ -468,6 +468,29 @@ int t4vf_get_sge_params(struct adapter *adapter)
 	sge_params->sge_timer_value_2_and_3 = vals[5];
 	sge_params->sge_timer_value_4_and_5 = vals[6];
 
+	/* T4 uses a single control field to specify both the PCIe Padding and
+	 * Packing Boundary.  T5 introduced the ability to specify these
+	 * separately with the Padding Boundary in SGE_CONTROL and and Packing
+	 * Boundary in SGE_CONTROL2.  So for T5 and later we need to grab
+	 * SGE_CONTROL in order to determine how ingress packet data will be
+	 * laid out in Packed Buffer Mode.  Unfortunately, older versions of
+	 * the firmware won't let us retrieve SGE_CONTROL2 so if we get a
+	 * failure grabbing it we throw an error since we can't figure out the
+	 * right value.
+	 */
+	if (!is_t4(adapter->params.chip)) {
+		params[0] = (FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
+			     FW_PARAMS_PARAM_XYZ(SGE_CONTROL2_A));
+		v = t4vf_query_params(adapter, 1, params, vals);
+		if (v != FW_SUCCESS) {
+			dev_err(adapter->pdev_dev,
+				"Unable to get SGE Control2; "
+				"probably old firmware.\n");
+			return v;
+		}
+		sge_params->sge_control2 = vals[0];
+	}
+
 	params[0] = (FW_PARAMS_MNEM(FW_PARAMS_MNEM_REG) |
 		     FW_PARAMS_PARAM_XYZ(SGE_INGRESS_RX_THRESHOLD));
 	v = t4vf_query_params(adapter, 1, params, vals);
-- 
1.7.1

^ permalink raw reply related

* [PATCH net 1/3] cxgb4vf: Move fl_starv_thres into adapter->sge data structure
From: Hariprasad Shenai @ 2014-11-07 11:36 UTC (permalink / raw)
  To: netdev; +Cc: davem, leedom, nirranjan, Hariprasad Shenai
In-Reply-To: <1415360191-25395-1-git-send-email-hariprasad@chelsio.com>

Move fl_starv_thres into adapter->sge data structure since it
_could_ be different from adapter to adapter.  Also move other per-adapter
SGE values which had been treated as driver globals into adapter->sge.

Based on original work by Casey Leedom <leedom@chelsio.com>

Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4vf/adapter.h |    8 ++
 drivers/net/ethernet/chelsio/cxgb4vf/sge.c     |   93 ++++++++++++++----------
 2 files changed, 61 insertions(+), 40 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/adapter.h b/drivers/net/ethernet/chelsio/cxgb4vf/adapter.h
index 68eaa9c..3d06e77 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/adapter.h
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/adapter.h
@@ -299,6 +299,14 @@ struct sge {
 	u16 timer_val[SGE_NTIMERS];	/* interrupt holdoff timer array */
 	u8 counter_val[SGE_NCOUNTERS];	/* interrupt RX threshold array */
 
+	/* Decoded Adapter Parameters.
+	 */
+	u32 fl_pg_order;		/* large page allocation size */
+	u32 stat_len;			/* length of status page at ring end */
+	u32 pktshift;			/* padding between CPL & packet data */
+	u32 fl_align;			/* response queue message alignment */
+	u32 fl_starve_thres;		/* Free List starvation threshold */
+
 	/*
 	 * Reverse maps from Absolute Queue IDs to associated queue pointers.
 	 * The absolute Queue IDs are in a compact range which start at a
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
index 85036e6..a18830d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/sge.c
@@ -51,14 +51,6 @@
 #include "../cxgb4/t4_msg.h"
 
 /*
- * Decoded Adapter Parameters.
- */
-static u32 FL_PG_ORDER;		/* large page allocation size */
-static u32 STAT_LEN;		/* length of status page at ring end */
-static u32 PKTSHIFT;		/* padding between CPL and packet data */
-static u32 FL_ALIGN;		/* response queue message alignment */
-
-/*
  * Constants ...
  */
 enum {
@@ -264,15 +256,19 @@ static inline unsigned int fl_cap(const struct sge_fl *fl)
 
 /**
  *	fl_starving - return whether a Free List is starving.
+ *	@adapter: pointer to the adapter
  *	@fl: the Free List
  *
  *	Tests specified Free List to see whether the number of buffers
  *	available to the hardware has falled below our "starvation"
  *	threshold.
  */
-static inline bool fl_starving(const struct sge_fl *fl)
+static inline bool fl_starving(const struct adapter *adapter,
+			       const struct sge_fl *fl)
 {
-	return fl->avail - fl->pend_cred <= FL_STARVE_THRES;
+	const struct sge *s = &adapter->sge;
+
+	return fl->avail - fl->pend_cred <= s->fl_starve_thres;
 }
 
 /**
@@ -457,13 +453,16 @@ static inline void reclaim_completed_tx(struct adapter *adapter,
 
 /**
  *	get_buf_size - return the size of an RX Free List buffer.
+ *	@adapter: pointer to the associated adapter
  *	@sdesc: pointer to the software buffer descriptor
  */
-static inline int get_buf_size(const struct rx_sw_desc *sdesc)
+static inline int get_buf_size(const struct adapter *adapter,
+			       const struct rx_sw_desc *sdesc)
 {
-	return FL_PG_ORDER > 0 && (sdesc->dma_addr & RX_LARGE_BUF)
-		? (PAGE_SIZE << FL_PG_ORDER)
-		: PAGE_SIZE;
+	const struct sge *s = &adapter->sge;
+
+	return (s->fl_pg_order > 0 && (sdesc->dma_addr & RX_LARGE_BUF)
+		? (PAGE_SIZE << s->fl_pg_order) : PAGE_SIZE);
 }
 
 /**
@@ -483,7 +482,8 @@ static void free_rx_bufs(struct adapter *adapter, struct sge_fl *fl, int n)
 
 		if (is_buf_mapped(sdesc))
 			dma_unmap_page(adapter->pdev_dev, get_buf_addr(sdesc),
-				       get_buf_size(sdesc), PCI_DMA_FROMDEVICE);
+				       get_buf_size(adapter, sdesc),
+				       PCI_DMA_FROMDEVICE);
 		put_page(sdesc->page);
 		sdesc->page = NULL;
 		if (++fl->cidx == fl->size)
@@ -511,7 +511,8 @@ static void unmap_rx_buf(struct adapter *adapter, struct sge_fl *fl)
 
 	if (is_buf_mapped(sdesc))
 		dma_unmap_page(adapter->pdev_dev, get_buf_addr(sdesc),
-			       get_buf_size(sdesc), PCI_DMA_FROMDEVICE);
+			       get_buf_size(adapter, sdesc),
+			       PCI_DMA_FROMDEVICE);
 	sdesc->page = NULL;
 	if (++fl->cidx == fl->size)
 		fl->cidx = 0;
@@ -589,6 +590,7 @@ static inline void poison_buf(struct page *page, size_t sz)
 static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
 			      int n, gfp_t gfp)
 {
+	struct sge *s = &adapter->sge;
 	struct page *page;
 	dma_addr_t dma_addr;
 	unsigned int cred = fl->avail;
@@ -608,12 +610,12 @@ static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
 	 * If we don't support large pages, drop directly into the small page
 	 * allocation code.
 	 */
-	if (FL_PG_ORDER == 0)
+	if (s->fl_pg_order == 0)
 		goto alloc_small_pages;
 
 	while (n) {
 		page = alloc_pages(gfp | __GFP_COMP | __GFP_NOWARN,
-				   FL_PG_ORDER);
+				   s->fl_pg_order);
 		if (unlikely(!page)) {
 			/*
 			 * We've failed inour attempt to allocate a "large
@@ -623,10 +625,10 @@ static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
 			fl->large_alloc_failed++;
 			break;
 		}
-		poison_buf(page, PAGE_SIZE << FL_PG_ORDER);
+		poison_buf(page, PAGE_SIZE << s->fl_pg_order);
 
 		dma_addr = dma_map_page(adapter->pdev_dev, page, 0,
-					PAGE_SIZE << FL_PG_ORDER,
+					PAGE_SIZE << s->fl_pg_order,
 					PCI_DMA_FROMDEVICE);
 		if (unlikely(dma_mapping_error(adapter->pdev_dev, dma_addr))) {
 			/*
@@ -637,7 +639,7 @@ static unsigned int refill_fl(struct adapter *adapter, struct sge_fl *fl,
 			 * because DMA mapping resources are typically
 			 * critical resources once they become scarse.
 			 */
-			__free_pages(page, FL_PG_ORDER);
+			__free_pages(page, s->fl_pg_order);
 			goto out;
 		}
 		dma_addr |= RX_LARGE_BUF;
@@ -693,7 +695,7 @@ out:
 	fl->pend_cred += cred;
 	ring_fl_db(adapter, fl);
 
-	if (unlikely(fl_starving(fl))) {
+	if (unlikely(fl_starving(adapter, fl))) {
 		smp_wmb();
 		set_bit(fl->cntxt_id, adapter->sge.starving_fl);
 	}
@@ -1468,6 +1470,8 @@ static void t4vf_pktgl_free(const struct pkt_gl *gl)
 static void do_gro(struct sge_eth_rxq *rxq, const struct pkt_gl *gl,
 		   const struct cpl_rx_pkt *pkt)
 {
+	struct adapter *adapter = rxq->rspq.adapter;
+	struct sge *s = &adapter->sge;
 	int ret;
 	struct sk_buff *skb;
 
@@ -1478,8 +1482,8 @@ static void do_gro(struct sge_eth_rxq *rxq, const struct pkt_gl *gl,
 		return;
 	}
 
-	copy_frags(skb, gl, PKTSHIFT);
-	skb->len = gl->tot_len - PKTSHIFT;
+	copy_frags(skb, gl, s->pktshift);
+	skb->len = gl->tot_len - s->pktshift;
 	skb->data_len = skb->len;
 	skb->truesize += skb->data_len;
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
@@ -1516,6 +1520,8 @@ int t4vf_ethrx_handler(struct sge_rspq *rspq, const __be64 *rsp,
 	bool csum_ok = pkt->csum_calc && !pkt->err_vec &&
 		       (rspq->netdev->features & NETIF_F_RXCSUM);
 	struct sge_eth_rxq *rxq = container_of(rspq, struct sge_eth_rxq, rspq);
+	struct adapter *adapter = rspq->adapter;
+	struct sge *s = &adapter->sge;
 
 	/*
 	 * If this is a good TCP packet and we have Generic Receive Offload
@@ -1537,7 +1543,7 @@ int t4vf_ethrx_handler(struct sge_rspq *rspq, const __be64 *rsp,
 		rxq->stats.rx_drops++;
 		return 0;
 	}
-	__skb_pull(skb, PKTSHIFT);
+	__skb_pull(skb, s->pktshift);
 	skb->protocol = eth_type_trans(skb, rspq->netdev);
 	skb_record_rx_queue(skb, rspq->idx);
 	rxq->stats.pkts++;
@@ -1648,6 +1654,8 @@ static inline void rspq_next(struct sge_rspq *rspq)
 static int process_responses(struct sge_rspq *rspq, int budget)
 {
 	struct sge_eth_rxq *rxq = container_of(rspq, struct sge_eth_rxq, rspq);
+	struct adapter *adapter = rspq->adapter;
+	struct sge *s = &adapter->sge;
 	int budget_left = budget;
 
 	while (likely(budget_left)) {
@@ -1697,7 +1705,7 @@ static int process_responses(struct sge_rspq *rspq, int budget)
 				BUG_ON(frag >= MAX_SKB_FRAGS);
 				BUG_ON(rxq->fl.avail == 0);
 				sdesc = &rxq->fl.sdesc[rxq->fl.cidx];
-				bufsz = get_buf_size(sdesc);
+				bufsz = get_buf_size(adapter, sdesc);
 				fp->page = sdesc->page;
 				fp->offset = rspq->offset;
 				fp->size = min(bufsz, len);
@@ -1726,7 +1734,7 @@ static int process_responses(struct sge_rspq *rspq, int budget)
 			 */
 			ret = rspq->handler(rspq, rspq->cur_desc, &gl);
 			if (likely(ret == 0))
-				rspq->offset += ALIGN(fp->size, FL_ALIGN);
+				rspq->offset += ALIGN(fp->size, s->fl_align);
 			else
 				restore_rx_bufs(&gl, &rxq->fl, frag);
 		} else if (likely(rsp_type == RSP_TYPE_CPL)) {
@@ -1963,7 +1971,7 @@ static void sge_rx_timer_cb(unsigned long data)
 			 * schedule napi but the FL is no longer starving.
 			 * No biggie.
 			 */
-			if (fl_starving(fl)) {
+			if (fl_starving(adapter, fl)) {
 				struct sge_eth_rxq *rxq;
 
 				rxq = container_of(fl, struct sge_eth_rxq, fl);
@@ -2047,6 +2055,7 @@ int t4vf_sge_alloc_rxq(struct adapter *adapter, struct sge_rspq *rspq,
 		       int intr_dest,
 		       struct sge_fl *fl, rspq_handler_t hnd)
 {
+	struct sge *s = &adapter->sge;
 	struct port_info *pi = netdev_priv(dev);
 	struct fw_iq_cmd cmd, rpl;
 	int ret, iqandst, flsz = 0;
@@ -2117,7 +2126,7 @@ int t4vf_sge_alloc_rxq(struct adapter *adapter, struct sge_rspq *rspq,
 		fl->size = roundup(fl->size, FL_PER_EQ_UNIT);
 		fl->desc = alloc_ring(adapter->pdev_dev, fl->size,
 				      sizeof(__be64), sizeof(struct rx_sw_desc),
-				      &fl->addr, &fl->sdesc, STAT_LEN);
+				      &fl->addr, &fl->sdesc, s->stat_len);
 		if (!fl->desc) {
 			ret = -ENOMEM;
 			goto err;
@@ -2129,7 +2138,7 @@ int t4vf_sge_alloc_rxq(struct adapter *adapter, struct sge_rspq *rspq,
 		 * free list ring) in Egress Queue Units.
 		 */
 		flsz = (fl->size / FL_PER_EQ_UNIT +
-			STAT_LEN / EQ_UNIT);
+			s->stat_len / EQ_UNIT);
 
 		/*
 		 * Fill in all the relevant firmware Ingress Queue Command
@@ -2217,6 +2226,7 @@ int t4vf_sge_alloc_eth_txq(struct adapter *adapter, struct sge_eth_txq *txq,
 			   struct net_device *dev, struct netdev_queue *devq,
 			   unsigned int iqid)
 {
+	struct sge *s = &adapter->sge;
 	int ret, nentries;
 	struct fw_eq_eth_cmd cmd, rpl;
 	struct port_info *pi = netdev_priv(dev);
@@ -2225,7 +2235,7 @@ int t4vf_sge_alloc_eth_txq(struct adapter *adapter, struct sge_eth_txq *txq,
 	 * Calculate the size of the hardware TX Queue (including the Status
 	 * Page on the end of the TX Queue) in units of TX Descriptors.
 	 */
-	nentries = txq->q.size + STAT_LEN / sizeof(struct tx_desc);
+	nentries = txq->q.size + s->stat_len / sizeof(struct tx_desc);
 
 	/*
 	 * Allocate the hardware ring for the TX ring (with space for its
@@ -2234,7 +2244,7 @@ int t4vf_sge_alloc_eth_txq(struct adapter *adapter, struct sge_eth_txq *txq,
 	txq->q.desc = alloc_ring(adapter->pdev_dev, txq->q.size,
 				 sizeof(struct tx_desc),
 				 sizeof(struct tx_sw_desc),
-				 &txq->q.phys_addr, &txq->q.sdesc, STAT_LEN);
+				 &txq->q.phys_addr, &txq->q.sdesc, s->stat_len);
 	if (!txq->q.desc)
 		return -ENOMEM;
 
@@ -2307,8 +2317,10 @@ int t4vf_sge_alloc_eth_txq(struct adapter *adapter, struct sge_eth_txq *txq,
  */
 static void free_txq(struct adapter *adapter, struct sge_txq *tq)
 {
+	struct sge *s = &adapter->sge;
+
 	dma_free_coherent(adapter->pdev_dev,
-			  tq->size * sizeof(*tq->desc) + STAT_LEN,
+			  tq->size * sizeof(*tq->desc) + s->stat_len,
 			  tq->desc, tq->phys_addr);
 	tq->cntxt_id = 0;
 	tq->sdesc = NULL;
@@ -2322,6 +2334,7 @@ static void free_txq(struct adapter *adapter, struct sge_txq *tq)
 static void free_rspq_fl(struct adapter *adapter, struct sge_rspq *rspq,
 			 struct sge_fl *fl)
 {
+	struct sge *s = &adapter->sge;
 	unsigned int flid = fl ? fl->cntxt_id : 0xffff;
 
 	t4vf_iq_free(adapter, FW_IQ_TYPE_FL_INT_CAP,
@@ -2337,7 +2350,7 @@ static void free_rspq_fl(struct adapter *adapter, struct sge_rspq *rspq,
 	if (fl) {
 		free_rx_bufs(adapter, fl, fl->avail);
 		dma_free_coherent(adapter->pdev_dev,
-				  fl->size * sizeof(*fl->desc) + STAT_LEN,
+				  fl->size * sizeof(*fl->desc) + s->stat_len,
 				  fl->desc, fl->addr);
 		kfree(fl->sdesc);
 		fl->sdesc = NULL;
@@ -2443,12 +2456,12 @@ int t4vf_sge_init(struct adapter *adapter)
 	 * Now translate the adapter parameters into our internal forms.
 	 */
 	if (fl1)
-		FL_PG_ORDER = ilog2(fl1) - PAGE_SHIFT;
-	STAT_LEN = ((sge_params->sge_control & EGRSTATUSPAGESIZE_MASK)
-		    ? 128 : 64);
-	PKTSHIFT = PKTSHIFT_GET(sge_params->sge_control);
-	FL_ALIGN = 1 << (INGPADBOUNDARY_GET(sge_params->sge_control) +
-			 SGE_INGPADBOUNDARY_SHIFT);
+		s->fl_pg_order = ilog2(fl1) - PAGE_SHIFT;
+	s->stat_len = ((sge_params->sge_control & EGRSTATUSPAGESIZE_MASK)
+			? 128 : 64);
+	s->pktshift = PKTSHIFT_GET(sge_params->sge_control);
+	s->fl_align = 1 << (INGPADBOUNDARY_GET(sge_params->sge_control) +
+			    SGE_INGPADBOUNDARY_SHIFT);
 
 	/*
 	 * Set up tasklet timers.
-- 
1.7.1

^ permalink raw reply related

* [PATCH net 0/3] cxgb4/cxgb4vf: Misc. fixes for cxgb4vf
From: Hariprasad Shenai @ 2014-11-07 11:36 UTC (permalink / raw)
  To: netdev; +Cc: davem, leedom, nirranjan, Hariprasad Shenai

Hi,

For T5 use Packing and Padding Boundaries for SGE DMA transfers, move
fl_starve_thres to adpater structure, since they are different for each
adapter. The cxgb4vf driver's Free List Starvation Threshold needs to be larger
than the SGE's Egress Congestion Threshold or we'll end up in a mutual stall
where the driver waits for Ingress Packets to drive replacing Free List
Pointers and the SGE waits for Free List Pointers before pushing Ingress
Packets to the host.

The patches series is created against 'net' tree.
And includes patches on cxgb4 and cxgb4vf driver.

We have included all the maintainers of respective drivers. Kindly review the
change and let us know in case of any review comments.

Thanks

Hariprasad Shenai (3):
  cxgb4vf: Move fl_starv_thres into adapter->sge data structure
  cxgb4/cxgb4vf: For T5 use Packing and Padding Boundaries for SGE DMA
    transfers
  cxgb4vf: FL Starvation Threshold needs to be larger than the SGE's
    Egress Congestion Threshold

 drivers/net/ethernet/chelsio/cxgb4/sge.c           |   30 ++++-
 drivers/net/ethernet/chelsio/cxgb4/t4_hw.c         |   51 +++++++-
 drivers/net/ethernet/chelsio/cxgb4/t4_regs.h       |   10 ++
 drivers/net/ethernet/chelsio/cxgb4vf/adapter.h     |    8 +
 drivers/net/ethernet/chelsio/cxgb4vf/sge.c         |  136 +++++++++++++-------
 drivers/net/ethernet/chelsio/cxgb4vf/t4vf_common.h |    2 +
 drivers/net/ethernet/chelsio/cxgb4vf/t4vf_hw.c     |   28 ++++-
 7 files changed, 209 insertions(+), 56 deletions(-)

^ permalink raw reply

* Re: [Xen-devel] BUG in xennet_make_frags with paged skb data
From: Eric Dumazet @ 2014-11-07 11:22 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: netdev, David S. Miller, Konrad Rzeszutek Wilk, Boris Ostrovsky,
	David Vrabel, Stefan Bader, Jay Vosburgh, linux-kernel, xen-devel
In-Reply-To: <545C9013.1090406@linaro.org>

On Fri, 2014-11-07 at 09:25 +0000, Zoltan Kiss wrote:

Please do not top post.

> Hi,
> 
> AFAIK in this scenario your skb frag is wrong. The page pointer should 
> point to the original compound page (not a member of it), and offset 
> should be set accordingly.
> For example, if your compound page is 16K (4 page), then the page 
> pointer should point to the first page, and if the data starts at the 
> 3rd page, then offset should be >8K

This is not accurate.

This BUG_ON() is wrong.

It should instead be :

BUG_ON(len + offset > PAGE_SIZE<<compound_order(compound_head(page)));

splice() code can generate such cases.

^ permalink raw reply

* Re: BUG in xennet_make_frags with paged skb data
From: Stefan Bader @ 2014-11-07 11:08 UTC (permalink / raw)
  To: David Vrabel, netdev, David S. Miller, Konrad Rzeszutek Wilk,
	Boris Ostrovsky, Jay Vosburgh, linux-kernel, xen-devel
In-Reply-To: <545CA27F.4070400@citrix.com>

[-- Attachment #1: Type: text/plain, Size: 2802 bytes --]

On 07.11.2014 11:44, David Vrabel wrote:
> On 06/11/14 21:49, Seth Forshee wrote:
>> We've had several reports of hitting the following BUG_ON in
>> xennet_make_frags with 3.2 and 3.13 kernels (I'm currently awaiting
>> results of testing with 3.17):
>>
>>         /* Grant backend access to each skb fragment page. */
>>         for (i = 0; i < frags; i++) {
>>                 skb_frag_t *frag = skb_shinfo(skb)->frags + i;
>>                 struct page *page = skb_frag_page(frag);
>>
>>                 len = skb_frag_size(frag);
>>                 offset = frag->page_offset;
>>
>>                 /* Data must not cross a page boundary. */
>>                 BUG_ON(len + offset > PAGE_SIZE<<compound_order(page));
>>
>> When this happens the page in question is a "middle" page in a compound
>> page (i.e. it's a tail page but not the last tail page), and the data is
>> fully contained within the compound page. The data does however cross
>> the hardware page boundary, and since compound_order evaluates to 0 for
>> tail pages the check fails.
>>
>> In going over this I've been unable to determine whether the BUG_ON in
>> xennet_make_frags is incorrect or the paged skb data is wrong. I can't
>> find that it's documented anywhere, and the networking code itself is a
>> bit ambiguous when it comes to compound pages. On the one hand
>> __skb_fill_page_desc specifically handles adding tail pages as paged
>> data, but on the other hand skb_copy_bits kmaps frag->page.p which could
>> fail with data that extends into another page.
> 
> netfront will safely handle this case so you can remove this BUG_ON()
> (and the one later on).  But it would be better to find out were these
> funny-looking skbs are coming from and (if necessary) fixing the bug there.

Right, in fact it does ignore pages from the start of a compound page in case
the offset is big enough. So there is no difference between that case and the
one where the page pointer from the frag is adjusted the way it was observed.

It really boils down to the question what the real rules for the frags are. If
it is "only" that data may not cross the boundary of a hw or compound page this
leaves room for interpretation. The odd skb does not violate that but a check
for conformance would become a lot more complex. And netfront is not the only
place that expects the frag page to be pointing to the compound head (there is
igb for example, though it does only a warn_on upon failing).

On the other hand the __skb_fill_page_desc is written in a way that seems to
assume the frag page pointer may be pointing to a tail page. So before "blaming"
one piece of code or the other we need an authoritative answer to what we are
supposed to expect.

-Stefan
> 
> David
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: BUG in xennet_make_frags with paged skb data
From: David Vrabel @ 2014-11-07 10:44 UTC (permalink / raw)
  To: netdev, David S. Miller, Konrad Rzeszutek Wilk, Boris Ostrovsky,
	Stefan Bader, Jay Vosburgh, linux-kernel, xen-devel
In-Reply-To: <20141106214940.GD44162@ubuntu-hedt>

On 06/11/14 21:49, Seth Forshee wrote:
> We've had several reports of hitting the following BUG_ON in
> xennet_make_frags with 3.2 and 3.13 kernels (I'm currently awaiting
> results of testing with 3.17):
> 
>         /* Grant backend access to each skb fragment page. */
>         for (i = 0; i < frags; i++) {
>                 skb_frag_t *frag = skb_shinfo(skb)->frags + i;
>                 struct page *page = skb_frag_page(frag);
> 
>                 len = skb_frag_size(frag);
>                 offset = frag->page_offset;
> 
>                 /* Data must not cross a page boundary. */
>                 BUG_ON(len + offset > PAGE_SIZE<<compound_order(page));
> 
> When this happens the page in question is a "middle" page in a compound
> page (i.e. it's a tail page but not the last tail page), and the data is
> fully contained within the compound page. The data does however cross
> the hardware page boundary, and since compound_order evaluates to 0 for
> tail pages the check fails.
> 
> In going over this I've been unable to determine whether the BUG_ON in
> xennet_make_frags is incorrect or the paged skb data is wrong. I can't
> find that it's documented anywhere, and the networking code itself is a
> bit ambiguous when it comes to compound pages. On the one hand
> __skb_fill_page_desc specifically handles adding tail pages as paged
> data, but on the other hand skb_copy_bits kmaps frag->page.p which could
> fail with data that extends into another page.

netfront will safely handle this case so you can remove this BUG_ON()
(and the one later on).  But it would be better to find out were these
funny-looking skbs are coming from and (if necessary) fixing the bug there.

David

^ permalink raw reply

* Re: [PATCH V4 3/3] can: m_can: add missing message RAM initialization
From: Marc Kleine-Budde @ 2014-11-07 10:30 UTC (permalink / raw)
  To: Dong Aisheng, linux-can
  Cc: wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <1415349914-9145-3-git-send-email-b29396@freescale.com>

[-- Attachment #1: Type: text/plain, Size: 2285 bytes --]

On 11/07/2014 09:45 AM, Dong Aisheng wrote:
> The M_CAN message RAM is usually equipped with a parity or ECC functionality.
> But RAM cells suffer a hardware reset and can therefore hold arbitrary
> content at startup - including parity and/or ECC bits.
> 
> To prevent the M_CAN controller detecting checksum errors when reading
> potentially uninitialized TX message RAM content to transmit CAN
> frames the TX message RAM has to be written with (any kind of) initial
> data.
> 
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> ---
> ChangeLog:
> v3-v4:
>  * initialize the entire Message RAM in use instead of only 8 bytes
>    since this is a valid and needed initialization process.
>    Patch title also changed.
> ---
>  drivers/net/can/m_can/m_can.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index eee1533..de742d0 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -1114,7 +1114,7 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
>  	struct resource *res;
>  	void __iomem *addr;
>  	u32 out_val[MRAM_CFG_LEN];
> -	int ret;
> +	int i, start, end, ret;
>  
>  	/* message ram could be shared */
>  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
> @@ -1165,6 +1165,15 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
>  		priv->mcfg[MRAM_TXE].off, priv->mcfg[MRAM_TXE].num,
>  		priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num);
>  
> +	/* initialize the entire Message RAM in use to avoid possible
> +	 * ECC/parity checksum errors when reading an uninitialized buffer
> +	 */
> +	start = priv->mcfg[MRAM_SIDF].off;
> +	end = priv->mcfg[MRAM_TXB].off +
> +		priv->mcfg[MRAM_TXB].num * TXB_ELEMENT_SIZE;
> +	for (i = start; i < end; i += 4)
> +		writel(0x0, priv->mram_base + i);

Can we access the mram without turning on the clock?

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH V4 3/3] can: m_can: add missing message RAM initialization
From: Dong Aisheng @ 2014-11-07 10:21 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: linux-can, wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <545C9F4E.3040901@pengutronix.de>

On Fri, Nov 07, 2014 at 11:30:38AM +0100, Marc Kleine-Budde wrote:
> On 11/07/2014 09:45 AM, Dong Aisheng wrote:
> > The M_CAN message RAM is usually equipped with a parity or ECC functionality.
> > But RAM cells suffer a hardware reset and can therefore hold arbitrary
> > content at startup - including parity and/or ECC bits.
> > 
> > To prevent the M_CAN controller detecting checksum errors when reading
> > potentially uninitialized TX message RAM content to transmit CAN
> > frames the TX message RAM has to be written with (any kind of) initial
> > data.
> > 
> > Signed-off-by: Dong Aisheng <b29396@freescale.com>
> > ---
> > ChangeLog:
> > v3-v4:
> >  * initialize the entire Message RAM in use instead of only 8 bytes
> >    since this is a valid and needed initialization process.
> >    Patch title also changed.
> > ---
> >  drivers/net/can/m_can/m_can.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > index eee1533..de742d0 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -1114,7 +1114,7 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> >  	struct resource *res;
> >  	void __iomem *addr;
> >  	u32 out_val[MRAM_CFG_LEN];
> > -	int ret;
> > +	int i, start, end, ret;
> >  
> >  	/* message ram could be shared */
> >  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
> > @@ -1165,6 +1165,15 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
> >  		priv->mcfg[MRAM_TXE].off, priv->mcfg[MRAM_TXE].num,
> >  		priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num);
> >  
> > +	/* initialize the entire Message RAM in use to avoid possible
> > +	 * ECC/parity checksum errors when reading an uninitialized buffer
> > +	 */
> > +	start = priv->mcfg[MRAM_SIDF].off;
> > +	end = priv->mcfg[MRAM_TXB].off +
> > +		priv->mcfg[MRAM_TXB].num * TXB_ELEMENT_SIZE;
> > +	for (i = start; i < end; i += 4)
> > +		writel(0x0, priv->mram_base + i);
> 
> Can we access the mram without turning on the clock?
> 

Yes, we can.
Since it's CPU access, it seems it does not depend on M_CAN clocks.
What i see from M_CAN subsystem block diagram of MX6SX,
it requires CPU clock which should always be enabled.

Regards
Dong Aisheng

> Marc
> 
> -- 
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
> 



^ permalink raw reply

* Re: How to make stack send broadcast ARP request when entry is STALE?
From: Ulf samuelsson @ 2014-11-07 10:11 UTC (permalink / raw)
  To: Brian Haley; +Cc: Netdev
In-Reply-To: <545C96D6.5020204@hp.com>

The HP router is configured by a customer, and they intentionally limit replies
to broadcast, and that is how they want it.

In the previous version of the build system, the Interpeak stack was used
and this would in PROBE state send unicast ARP request, and if that failed
send broadcast ARP.

The native linux stack, when in PROBE state sends only unicast until it decides
that it should enter FAILED state.

The 'mcast_probes' variable seems to be totally ignored, except the first  time,
so I do not see why it is there.

Best Regards
Ulf Samuelsson
ulf@emagii.com
+46  (722) 427 437


> 7 nov 2014 kl. 10:54 skrev Brian Haley <brian.haley@hp.com>:
> 
>> On 11/05/2014 07:48 AM, Ulf samuelsson wrote:
>> Have a problem with an HP router at a certain location, which
>> is configured to only answer to broadcast ARP requests.
>> That cannot be changed.
> 
> Sorry to hear about the problem, but my only suggestions would be to try the latest firmware and/or put a call in to support.  I don't happen work in the division that makes routers...
> 
>> The first ARP request the kernel sends out, is a broadcast request,
>> which is fine, but after the reply, the kernel sends unicast requests,
>> which will not get any replies.
> 
> You might be able to hack this by inserting an ebtables rule - check the dnat target section of the man page - don't know the exact syntax but it would probably end in '-j dnat --to-destination ff:ff:ff:ff:ff:ff'
> 
> -Brian
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [PATCH net-next v2 2/3] r8152: clear theflagofSCHEDULE_TASKLETin tasklet
From: Hayes Wang @ 2014-11-07 10:05 UTC (permalink / raw)
  To: Francois Romieu
  Cc: David Miller, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	nic_swsd, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20141102225307.GA19900-lmTtMILVy1jWQcoT9B9Ug5SCg42XY1Uw0E9HWUfgJXw@public.gmane.org>

 Francois Romieu [mailto:romieu-W8zweXLXuWQS+FvcfC7Uqw@public.gmane.org] 
> Sent: Monday, November 03, 2014 6:53 AM
[...]
> test_and_clear_bit (dense) or clear_bit would be more idiomatic.

Excuse me. Any suggestion?
Should I use clear_bit directly, or something else?
Or, do I have to remove this patch?
 
Best Regards,
Hayes
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH net-next 2/2] r8152: adjust rtl_start_rx
From: Hayes Wang @ 2014-11-07  9:55 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-88-Taiwan-albertk@realtek.com>

Submit all the rx buffers, even though a error occurs. Otherwise
the buffers which are not submitted would be lost until next
rtl_start_rx() is called. Besides, the fail buffer could be
re-submitted later.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ad62994..5e0386f 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1993,10 +1993,16 @@ static int rtl_start_rx(struct r8152 *tp)
 
 	INIT_LIST_HEAD(&tp->rx_done);
 	for (i = 0; i < RTL8152_MAX_RX; i++) {
+		int rr;
+
 		INIT_LIST_HEAD(&tp->rx_info[i].list);
-		ret = r8152_submit_rx(tp, &tp->rx_info[i], GFP_KERNEL);
-		if (ret)
-			break;
+
+		rr = r8152_submit_rx(tp, &tp->rx_info[i], GFP_KERNEL);
+		if (rr)
+			netif_err(tp, rx_err, tp->netdev,
+				  "Couldn't submit rx[%d], ret = %d\n", i, rr);
+		if (!ret)
+			ret = rr;
 	}
 
 	return ret;
-- 
1.9.3

^ permalink raw reply related

* [PATCH net-next 1/2] r8152: adjust r8152_submit_rx
From: Hayes Wang @ 2014-11-07  9:55 UTC (permalink / raw)
  To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1394712342-15778-88-Taiwan-albertk@realtek.com>

The behavior of handling the returned status from r8152_submit_rx()
is almost same, so let r8152_submit_rx() deal with the error
directly. This could avoid the duplicate code.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 39 +++++++++++++++++++--------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 66b139a..ad62994 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1032,7 +1032,6 @@ static void read_bulk_callback(struct urb *urb)
 	int status = urb->status;
 	struct rx_agg *agg;
 	struct r8152 *tp;
-	int result;
 
 	agg = urb->context;
 	if (!agg)
@@ -1083,16 +1082,7 @@ static void read_bulk_callback(struct urb *urb)
 		break;
 	}
 
-	result = r8152_submit_rx(tp, agg, GFP_ATOMIC);
-	if (result == -ENODEV) {
-		set_bit(RTL8152_UNPLUG, &tp->flags);
-		netif_device_detach(tp->netdev);
-	} else if (result) {
-		spin_lock(&tp->rx_lock);
-		list_add_tail(&agg->list, &tp->rx_done);
-		spin_unlock(&tp->rx_lock);
-		tasklet_schedule(&tp->tl);
-	}
+	r8152_submit_rx(tp, agg, GFP_ATOMIC);
 }
 
 static void write_bulk_callback(struct urb *urb)
@@ -1681,7 +1671,6 @@ static void rx_bottom(struct r8152 *tp)
 		int len_used = 0;
 		struct urb *urb;
 		u8 *rx_data;
-		int ret;
 
 		list_del_init(cursor);
 
@@ -1734,13 +1723,7 @@ find_next_rx:
 		}
 
 submit:
-		ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
-		if (ret && ret != -ENODEV) {
-			spin_lock_irqsave(&tp->rx_lock, flags);
-			list_add_tail(&agg->list, &tp->rx_done);
-			spin_unlock_irqrestore(&tp->rx_lock, flags);
-			tasklet_schedule(&tp->tl);
-		}
+		r8152_submit_rx(tp, agg, GFP_ATOMIC);
 	}
 }
 
@@ -1805,11 +1788,27 @@ static void bottom_half(unsigned long data)
 static
 int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags)
 {
+	int ret = 0;
+
 	usb_fill_bulk_urb(agg->urb, tp->udev, usb_rcvbulkpipe(tp->udev, 1),
 			  agg->head, agg_buf_sz,
 			  (usb_complete_t)read_bulk_callback, agg);
 
-	return usb_submit_urb(agg->urb, mem_flags);
+	ret = usb_submit_urb(agg->urb, mem_flags);
+
+	if (ret == -ENODEV) {
+		set_bit(RTL8152_UNPLUG, &tp->flags);
+		netif_device_detach(tp->netdev);
+	} else if (ret) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&tp->rx_lock, flags);
+		list_add_tail(&agg->list, &tp->rx_done);
+		spin_unlock_irqrestore(&tp->rx_lock, flags);
+		tasklet_schedule(&tp->tl);
+	}
+
+	return ret;
 }
 
 static void rtl_drop_queued_tx(struct r8152 *tp)
-- 
1.9.3

^ 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