Netdev List
 help / color / mirror / Atom feed
* [RFC][PATCH 2/2] inet6: sysctl pointers shadow global pointer
From: Gerrit Renker @ 2009-07-19 18:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, Gerrit Renker
In-Reply-To: <1248028721-24244-2-git-send-email-gerrit@erg.abdn.ac.uk>

net/ipv6/sysctl_net_ipv6.c:64:19:  warning: symbol 'ipv6_table' shadows an earlier one
net/ipv6/sysctl_net_ipv6.c:107:19: warning: symbol 'ipv6_table' shadows an earlier one
net/ipv6/sysctl_net_ipv6.c:43:18:  originally declared here

The only place where 'ipv6_table' is not shadowed is in ipv6_sysctl_register().

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/ipv6/sysctl_net_ipv6.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -40,7 +40,7 @@ static ctl_table ipv6_table_template[] =
 	{ .ctl_name = 0 }
 };
 
-static ctl_table ipv6_table[] = {
+static ctl_table ipv6_rotable[] = {
 	{
 		.ctl_name	= NET_IPV6_MLD_MAX_MSF,
 		.procname	= "mld_max_msf",
@@ -130,7 +130,7 @@ int ipv6_sysctl_register(void)
 {
 	int err = -ENOMEM;
 
-	ip6_header = register_net_sysctl_rotable(net_ipv6_ctl_path, ipv6_table);
+	ip6_header = register_net_sysctl_rotable(net_ipv6_ctl_path, ipv6_rotable);
 	if (ip6_header == NULL)
 		goto out;
 

^ permalink raw reply

* [RFC][PATCH 0/2] inet6: two cases of shadowing other variables
From: Gerrit Renker @ 2009-07-19 18:38 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <cases_of_shadowing_other_variables>

Two cases of shadowing variables. 

Patches are not meant to be applied immediately, especially in the first
one I am not sure if the code is ok without adding a lock.

Can the IPv6 developers please have a look through:

Patch #1: Local variable shadows function argument. After renaming,
          the question arose whether the code is correct wrt locking.

          Please take a look at this code part, no lock is taken on
	  the pmc->idev. (There is similar code in ip6_mc_del_src()).

Patch #2: Several functions shadow a global variable.
	  Can be applied directly, but better to review wrt:
	  given that ipv6_sysctl_register() calls register_pernet_subsys(),
	  does it make sense to merge the one-row table with
	  ipv6_table_template[], or are there namespace issues?


^ permalink raw reply

* [PATCH 1/4] inet6: Return convention in datagram_send_ctl
From: Gerrit Renker @ 2009-07-19 18:23 UTC (permalink / raw)
  To: davem; +Cc: netdev, Gerrit Renker
In-Reply-To: <1248027819-23959-1-git-send-email-gerrit@erg.abdn.ac.uk>

The current code has both return conventions for errors
 * set err = 'errval' and then goto exit_f,
 * return 'errval' directly.
This patch reduces the number of alternatives to one.

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/ipv6/datagram.c |  156 ++++++++++++++++++---------------------------------
 1 files changed, 55 insertions(+), 101 deletions(-)

--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -505,15 +505,12 @@ int datagram_send_ctl(struct net *net,
 	struct ipv6_rt_hdr *rthdr;
 	struct ipv6_opt_hdr *hdr;
 	int len;
-	int err = 0;
 
 	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
 		int addr_type;
 
-		if (!CMSG_OK(msg, cmsg)) {
-			err = -EINVAL;
-			goto exit_f;
-		}
+		if (!CMSG_OK(msg, cmsg))
+			return -EINVAL;
 
 		if (cmsg->cmsg_level != SOL_IPV6)
 			continue;
@@ -524,10 +521,8 @@ int datagram_send_ctl(struct net *net,
 		    {
 			struct net_device *dev = NULL;
 
-			if (cmsg->cmsg_len < CMSG_LEN(sizeof(struct in6_pktinfo))) {
-				err = -EINVAL;
-				goto exit_f;
-			}
+			if (cmsg->cmsg_len < CMSG_LEN(sizeof(struct in6_pktinfo)))
+				return -EINVAL;
 
 			src_info = (struct in6_pktinfo *)CMSG_DATA(cmsg);
 
@@ -548,99 +543,76 @@ int datagram_send_ctl(struct net *net,
 
 			if (addr_type != IPV6_ADDR_ANY) {
 				int strict = __ipv6_addr_src_scope(addr_type) <= IPV6_ADDR_SCOPE_LINKLOCAL;
-				if (!ipv6_chk_addr(net, &src_info->ipi6_addr,
-						   strict ? dev : NULL, 0))
-					err = -EINVAL;
-				else
-					ipv6_addr_copy(&fl->fl6_src, &src_info->ipi6_addr);
+				if (!ipv6_chk_addr(net, &src_info->ipi6_addr, strict ? dev : NULL, 0))
+					return -EINVAL;
+				ipv6_addr_copy(&fl->fl6_src, &src_info->ipi6_addr);
 			}
 
 			if (dev)
 				dev_put(dev);
 
-			if (err)
-				goto exit_f;
-
 			break;
 		    }
 
 		case IPV6_FLOWINFO:
-			if (cmsg->cmsg_len < CMSG_LEN(4)) {
-				err = -EINVAL;
-				goto exit_f;
-			}
+			if (cmsg->cmsg_len < CMSG_LEN(4))
+				return -EINVAL;
 
 			if (fl->fl6_flowlabel&IPV6_FLOWINFO_MASK) {
-				if ((fl->fl6_flowlabel^*(__be32 *)CMSG_DATA(cmsg))&~IPV6_FLOWINFO_MASK) {
-					err = -EINVAL;
-					goto exit_f;
-				}
+				if ((fl->fl6_flowlabel^*(__be32 *)CMSG_DATA(cmsg))&~IPV6_FLOWINFO_MASK)
+					return -EINVAL;
 			}
 			fl->fl6_flowlabel = IPV6_FLOWINFO_MASK & *(__be32 *)CMSG_DATA(cmsg);
 			break;
 
 		case IPV6_2292HOPOPTS:
 		case IPV6_HOPOPTS:
-			if (opt->hopopt || cmsg->cmsg_len < CMSG_LEN(sizeof(struct ipv6_opt_hdr))) {
-				err = -EINVAL;
-				goto exit_f;
-			}
+			if (opt->hopopt || cmsg->cmsg_len < CMSG_LEN(sizeof(struct ipv6_opt_hdr)))
+				return -EINVAL;
 
 			hdr = (struct ipv6_opt_hdr *)CMSG_DATA(cmsg);
 			len = ((hdr->hdrlen + 1) << 3);
-			if (cmsg->cmsg_len < CMSG_LEN(len)) {
-				err = -EINVAL;
-				goto exit_f;
-			}
-			if (!capable(CAP_NET_RAW)) {
-				err = -EPERM;
-				goto exit_f;
-			}
+
+			if (cmsg->cmsg_len < CMSG_LEN(len))
+				return -EINVAL;
+			if (!capable(CAP_NET_RAW))
+				return -EPERM;
+
 			opt->opt_nflen += len;
 			opt->hopopt = hdr;
 			break;
 
 		case IPV6_2292DSTOPTS:
-			if (cmsg->cmsg_len < CMSG_LEN(sizeof(struct ipv6_opt_hdr))) {
-				err = -EINVAL;
-				goto exit_f;
-			}
+			if (cmsg->cmsg_len < CMSG_LEN(sizeof(struct ipv6_opt_hdr)))
+				return -EINVAL;
 
 			hdr = (struct ipv6_opt_hdr *)CMSG_DATA(cmsg);
 			len = ((hdr->hdrlen + 1) << 3);
-			if (cmsg->cmsg_len < CMSG_LEN(len)) {
-				err = -EINVAL;
-				goto exit_f;
-			}
-			if (!capable(CAP_NET_RAW)) {
-				err = -EPERM;
-				goto exit_f;
-			}
-			if (opt->dst1opt) {
-				err = -EINVAL;
-				goto exit_f;
-			}
+
+			if (cmsg->cmsg_len < CMSG_LEN(len))
+				return -EINVAL;
+			if (!capable(CAP_NET_RAW))
+				return -EPERM;
+			if (opt->dst1opt)
+				return -EINVAL;
+
 			opt->opt_flen += len;
 			opt->dst1opt = hdr;
 			break;
 
 		case IPV6_DSTOPTS:
 		case IPV6_RTHDRDSTOPTS:
-			if (cmsg->cmsg_len < CMSG_LEN(sizeof(struct ipv6_opt_hdr))) {
-				err = -EINVAL;
-				goto exit_f;
-			}
+			if (cmsg->cmsg_len < CMSG_LEN(sizeof(struct ipv6_opt_hdr)))
+				return -EINVAL;
 
 			hdr = (struct ipv6_opt_hdr *)CMSG_DATA(cmsg);
 			len = ((hdr->hdrlen + 1) << 3);
-			if (cmsg->cmsg_len < CMSG_LEN(len)) {
-				err = -EINVAL;
-				goto exit_f;
-			}
-			if (!capable(CAP_NET_RAW)) {
-				err = -EPERM;
-				goto exit_f;
-			}
+			if (cmsg->cmsg_len < CMSG_LEN(len))
+				return -EINVAL;
+
+			if (!capable(CAP_NET_RAW))
+				return -EPERM;
+
 			if (cmsg->cmsg_type == IPV6_DSTOPTS) {
 				opt->opt_flen += len;
 				opt->dst1opt = hdr;
@@ -652,10 +624,8 @@ int datagram_send_ctl(struct net *net,
 
 		case IPV6_2292RTHDR:
 		case IPV6_RTHDR:
-			if (cmsg->cmsg_len < CMSG_LEN(sizeof(struct ipv6_rt_hdr))) {
-				err = -EINVAL;
-				goto exit_f;
-			}
+			if (cmsg->cmsg_len < CMSG_LEN(sizeof(struct ipv6_rt_hdr)))
+				return -EINVAL;
 
 			rthdr = (struct ipv6_rt_hdr *)CMSG_DATA(cmsg);
 
@@ -663,29 +633,22 @@ int datagram_send_ctl(struct net *net,
 #if defined(CONFIG_IPV6_MIP6) || defined(CONFIG_IPV6_MIP6_MODULE)
 			case IPV6_SRCRT_TYPE_2:
 				if (rthdr->hdrlen != 2 ||
-				    rthdr->segments_left != 1) {
-					err = -EINVAL;
-					goto exit_f;
-				}
+				    rthdr->segments_left != 1)
+					return -EINVAL;
 				break;
 #endif
 			default:
-				err = -EINVAL;
-				goto exit_f;
+				return -EINVAL;
 			}
 
 			len = ((rthdr->hdrlen + 1) << 3);
 
-			if (cmsg->cmsg_len < CMSG_LEN(len)) {
-				err = -EINVAL;
-				goto exit_f;
-			}
+			if (cmsg->cmsg_len < CMSG_LEN(len))
+				return -EINVAL;
 
 			/* segments left must also match */
-			if ((rthdr->hdrlen >> 1) != rthdr->segments_left) {
-				err = -EINVAL;
-				goto exit_f;
-			}
+			if ((rthdr->hdrlen >> 1) != rthdr->segments_left)
+				return -EINVAL;
 
 			opt->opt_nflen += len;
 			opt->srcrt = rthdr;
@@ -703,16 +666,12 @@ int datagram_send_ctl(struct net *net,
 
 		case IPV6_2292HOPLIMIT:
 		case IPV6_HOPLIMIT:
-			if (cmsg->cmsg_len != CMSG_LEN(sizeof(int))) {
-				err = -EINVAL;
-				goto exit_f;
-			}
+			if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)))
+				return -EINVAL;
 
 			*hlimit = *(int *)CMSG_DATA(cmsg);
-			if (*hlimit < -1 || *hlimit > 0xff) {
-				err = -EINVAL;
-				goto exit_f;
-			}
+			if (*hlimit < -1 || *hlimit > 0xff)
+				return -EINVAL;
 
 			break;
 
@@ -720,16 +679,13 @@ int datagram_send_ctl(struct net *net,
 		    {
 			int tc;
 
-			err = -EINVAL;
-			if (cmsg->cmsg_len != CMSG_LEN(sizeof(int))) {
-				goto exit_f;
-			}
+			if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)))
+				return -EINVAL;
 
 			tc = *(int *)CMSG_DATA(cmsg);
 			if (tc < -1 || tc > 0xff)
-				goto exit_f;
+				return -EINVAL;
 
-			err = 0;
 			*tclass = tc;
 
 			break;
@@ -737,11 +693,9 @@ int datagram_send_ctl(struct net *net,
 		default:
 			LIMIT_NETDEBUG(KERN_DEBUG "invalid cmsg type: %d\n",
 				       cmsg->cmsg_type);
-			err = -EINVAL;
-			goto exit_f;
+			return -EINVAL;
 		}
 	}
 
-exit_f:
-	return err;
+	return 0;
 }

^ permalink raw reply

* [PATCH 3/4] inet6: Conversion from u8 to int
From: Gerrit Renker @ 2009-07-19 18:23 UTC (permalink / raw)
  To: davem; +Cc: netdev, Gerrit Renker
In-Reply-To: <1248027819-23959-3-git-send-email-gerrit@erg.abdn.ac.uk>

This replaces assignments of the type "int on LHS" = "u8 on RHS" with
simpler code. The LHS can express all of the unsigned right hand side
values, hence the assigned value can not be negative.

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/ipv6/icmp.c          |   17 ++++-------------
 net/ipv6/ip6_output.c    |   15 +++++----------
 net/ipv6/ipv6_sockglue.c |    2 --
 net/ipv6/raw.c           |    5 +----
 net/ipv6/udp.c           |    5 +----
 5 files changed, 11 insertions(+), 33 deletions(-)

--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -323,7 +323,7 @@ void icmpv6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info,
 	int iif = 0;
 	int addr_type = 0;
 	int len;
-	int hlimit, tclass;
+	int hlimit;
 	int err = 0;
 
 	if ((u8 *)hdr < skb->head ||
@@ -469,10 +469,6 @@ route_done:
 	if (hlimit < 0)
 		hlimit = ip6_dst_hoplimit(dst);
 
-	tclass = np->tclass;
-	if (tclass < 0)
-		tclass = 0;
-
 	msg.skb = skb;
 	msg.offset = skb_network_offset(skb);
 	msg.type = type;
@@ -488,8 +484,8 @@ route_done:
 
 	err = ip6_append_data(sk, icmpv6_getfrag, &msg,
 			      len + sizeof(struct icmp6hdr),
-			      sizeof(struct icmp6hdr),
-			      hlimit, tclass, NULL, &fl, (struct rt6_info*)dst,
+			      sizeof(struct icmp6hdr), hlimit,
+			      np->tclass, NULL, &fl, (struct rt6_info*)dst,
 			      MSG_DONTWAIT);
 	if (err) {
 		ip6_flush_pending_frames(sk);
@@ -522,7 +518,6 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
 	struct dst_entry *dst;
 	int err = 0;
 	int hlimit;
-	int tclass;
 
 	saddr = &ipv6_hdr(skb)->daddr;
 
@@ -562,10 +557,6 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
 	if (hlimit < 0)
 		hlimit = ip6_dst_hoplimit(dst);
 
-	tclass = np->tclass;
-	if (tclass < 0)
-		tclass = 0;
-
 	idev = in6_dev_get(skb->dev);
 
 	msg.skb = skb;
@@ -573,7 +564,7 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
 	msg.type = ICMPV6_ECHO_REPLY;
 
 	err = ip6_append_data(sk, icmpv6_getfrag, &msg, skb->len + sizeof(struct icmp6hdr),
-				sizeof(struct icmp6hdr), hlimit, tclass, NULL, &fl,
+				sizeof(struct icmp6hdr), hlimit, np->tclass, NULL, &fl,
 				(struct rt6_info*)dst, MSG_DONTWAIT);
 
 	if (err) {
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -206,7 +206,8 @@ int ip6_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl,
 	struct ipv6hdr *hdr;
 	u8  proto = fl->proto;
 	int seg_len = skb->len;
-	int hlimit, tclass;
+	int hlimit = -1;
+	int tclass = 0;
 	u32 mtu;
 
 	if (opt) {
@@ -249,19 +250,13 @@ int ip6_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl,
 	/*
 	 *	Fill in the IPv6 header
 	 */
-
-	hlimit = -1;
-	if (np)
+	if (np) {
+		tclass = np->tclass;
 		hlimit = np->hop_limit;
+	}
 	if (hlimit < 0)
 		hlimit = ip6_dst_hoplimit(dst);
 
-	tclass = -1;
-	if (np)
-		tclass = np->tclass;
-	if (tclass < 0)
-		tclass = 0;
-
 	*(__be32 *)hdr = htonl(0x60000000 | (tclass << 20)) | fl->fl6_flowlabel;
 
 	hdr->payload_len = htons(seg_len);
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -1037,8 +1037,6 @@ static int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 
 	case IPV6_TCLASS:
 		val = np->tclass;
-		if (val < 0)
-			val = 0;
 		break;
 
 	case IPV6_RECVTCLASS:
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -877,11 +877,8 @@ static int rawv6_sendmsg(struct kiocb *iocb, struct sock *sk,
 			hlimit = ip6_dst_hoplimit(dst);
 	}
 
-	if (tclass < 0) {
+	if (tclass < 0)
 		tclass = np->tclass;
-		if (tclass < 0)
-			tclass = 0;
-	}
 
 	if (msg->msg_flags&MSG_CONFIRM)
 		goto do_confirm;
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -900,11 +900,8 @@ do_udp_sendmsg:
 			hlimit = ip6_dst_hoplimit(dst);
 	}
 
-	if (tclass < 0) {
+	if (tclass < 0)
 		tclass = np->tclass;
-		if (tclass < 0)
-			tclass = 0;
-	}
 
 	if (msg->msg_flags&MSG_CONFIRM)
 		goto do_confirm;

^ permalink raw reply

* [PATCH 2/4] inet6: Consolidate common code for IPv6 Hop Limit / Traffic Class
From: Gerrit Renker @ 2009-07-19 18:23 UTC (permalink / raw)
  To: davem; +Cc: netdev, Gerrit Renker
In-Reply-To: <1248027819-23959-2-git-send-email-gerrit@erg.abdn.ac.uk>

The RFC 3542 definitions for Hop Limit (6.3) and Traffic Class (6.5) cmsg
values differ only in the names of the cmsg type. So does the code.
The patch combines these commonalities.

Further changes:
----------------
Replaced other use of temporary 'int' variable with 'val' variable.

Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
---
 net/ipv6/datagram.c |   34 ++++++++++++----------------------
 1 files changed, 12 insertions(+), 22 deletions(-)

--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -504,10 +504,10 @@ int datagram_send_ctl(struct net *net,
 	struct cmsghdr *cmsg;
 	struct ipv6_rt_hdr *rthdr;
 	struct ipv6_opt_hdr *hdr;
+	int val;
 	int len;
 
 	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
-		int addr_type;
 
 		if (!CMSG_OK(msg, cmsg))
 			return -EINVAL;
@@ -532,17 +532,17 @@ int datagram_send_ctl(struct net *net,
 				fl->oif = src_info->ipi6_ifindex;
 			}
 
-			addr_type = __ipv6_addr_type(&src_info->ipi6_addr);
+			val = __ipv6_addr_type(&src_info->ipi6_addr);
 
 			if (fl->oif) {
 				dev = dev_get_by_index(net, fl->oif);
 				if (!dev)
 					return -ENODEV;
-			} else if (addr_type & IPV6_ADDR_LINKLOCAL)
+			} else if (val & IPV6_ADDR_LINKLOCAL)
 				return -EINVAL;
 
-			if (addr_type != IPV6_ADDR_ANY) {
-				int strict = __ipv6_addr_src_scope(addr_type) <= IPV6_ADDR_SCOPE_LINKLOCAL;
+			if (val != IPV6_ADDR_ANY) {
+				int strict = __ipv6_addr_src_scope(val) <= IPV6_ADDR_SCOPE_LINKLOCAL;
 				if (!ipv6_chk_addr(net, &src_info->ipi6_addr, strict ? dev : NULL, 0))
 					return -EINVAL;
 				ipv6_addr_copy(&fl->fl6_src, &src_info->ipi6_addr);
@@ -666,30 +666,20 @@ int datagram_send_ctl(struct net *net,
 
 		case IPV6_2292HOPLIMIT:
 		case IPV6_HOPLIMIT:
-			if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)))
-				return -EINVAL;
-
-			*hlimit = *(int *)CMSG_DATA(cmsg);
-			if (*hlimit < -1 || *hlimit > 0xff)
-				return -EINVAL;
-
-			break;
-
 		case IPV6_TCLASS:
-		    {
-			int tc;
-
 			if (cmsg->cmsg_len != CMSG_LEN(sizeof(int)))
 				return -EINVAL;
 
-			tc = *(int *)CMSG_DATA(cmsg);
-			if (tc < -1 || tc > 0xff)
+			val = *(int *)CMSG_DATA(cmsg);
+			if (val < -1 || val > 0xff)
 				return -EINVAL;
 
-			*tclass = tc;
-
+			if (cmsg->cmsg_type == IPV6_TCLASS)
+				*tclass = val;
+			else
+				*hlimit = val;
 			break;
-		    }
+
 		default:
 			LIMIT_NETDEBUG(KERN_DEBUG "invalid cmsg type: %d\n",
 				       cmsg->cmsg_type);

^ permalink raw reply

* [PATCH 0/4] inet6: minor cleanups
From: Gerrit Renker @ 2009-07-19 18:23 UTC (permalink / raw)
  To: davem; +Cc: netdev
In-Reply-To: <inet6_minor_cleanups>

This is a small set of minor-cleanup patches, found while
studying the tclass code.

Patch #1: Unifies return convention of datagram_send_ctl().
Patch #2: Consolidates cmsg code for Traffic Class / Hop Limit.
Patch #3: Simplifies assignments from u8 to int (no conversion).
Patch #4: Finds that RT_TOS() redefines IPTOS_TOS().

^ permalink raw reply

* Re: [PATCH] drivers/net/mlx4: Adjust constant
From: Joe Perches @ 2009-07-19 18:24 UTC (permalink / raw)
  To: Julia Lawall; +Cc: netdev, linux-kernel, kernel-janitors
In-Reply-To: <Pine.LNX.4.64.0907191809050.16542@ask.diku.dk>

On Sun, 2009-07-19 at 18:09 +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
> 
> The values in the advertising field are typically ADVERTISED_xxx, not
> SUPPORTED_xxx.  Both SUPPORTED_10000baseT_Full and
> ADVERTISED_1000baseT_Full have the same value.

other possibles:

drivers/net/sungem.c:#define ADVERTISE_MASK	(SUPPORTED_10baseT_Half | SUPPORTED_10baseT_Full | \
			 SUPPORTED_100baseT_Half | SUPPORTED_100baseT_Full | \
			 SUPPORTED_1000baseT_Half | SUPPORTED_1000baseT_Full | \
			 SUPPORTED_Pause | SUPPORTED_Autoneg)
drivers/net/s2io.c:	info->advertising = (SUPPORTED_10000baseT_Full | SUPPORTED_FIBRE);
drivers/net/spider_net.c:		advertise |= SUPPORTED_1000baseT_Full;
drivers/net/spider_net.c:		advertise |= SUPPORTED_1000baseT_Half;



^ permalink raw reply

* [PATCH] New device ID for sc92031 [1088:2031]
From: Cesar Eduardo Barros @ 2009-07-19 18:03 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Cesar Eduardo Barros, rain_maker

rain_maker@root-forum.org wrote:
> Hello cesar,
>
> In a recent thread in a german linux forum, a user reported his PIC
> NIC not being recognized by the kernel.
>
> Fortunately he provided enough information and I was able to help him
> and get the device working with the sc92031 driver.
>
> The device ID is [1088:2031] (Vendor is called "Microcomputer Systems
> (M) Son"), here is the respective thread in "ubuntuusers.de"
>
> http://forum.ubuntuusers.de/topic/lankarte-unter-xubuntu-wird-nicht-erkannt/
>
> (Although you might not speak german, the code provided will show
> you, that the device is actually working with your driver).
>
> It would be nice, if you include this new device ID to the
> sc92031-driver.
>
> Regards,
>
> Axel Köllhofer (aka Rain_Maker)

Cc: rain_maker@root-forum.org
Signed-off-by: Cesar Eduardo Barros <cesarb@cesarb.net>
---
 drivers/net/sc92031.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/sc92031.c b/drivers/net/sc92031.c
index 18821f2..e3156c9 100644
--- a/drivers/net/sc92031.c
+++ b/drivers/net/sc92031.c
@@ -1593,6 +1593,7 @@ out:
 static struct pci_device_id sc92031_pci_device_id_table[] __devinitdata = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_SILAN, 0x2031) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_SILAN, 0x8139) },
+	{ PCI_DEVICE(0x1088, 0x2031) },
 	{ 0, }
 };
 MODULE_DEVICE_TABLE(pci, sc92031_pci_device_id_table);
-- 
1.6.3.3


^ permalink raw reply related

* r8169 not advertising 1000mbit after suspend anymore
From: Benjamin S. @ 2009-07-19 17:09 UTC (permalink / raw)
  To: romieu; +Cc: netdev


Hello,

I have a Gigabyte GA-MA78G-DS3H mainboard with a RTL8111/8168B Realtek 
Gigabit Ethernet Controller. Until first suspend everything works as 
expected, but after suspend only 100mbit are used instead of 1000.


Interesting part of lspci -v:
02:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168B PCI Express Gigabit Ethernet controller (rev 02)
	Subsystem: Giga-byte Technology Device e000
	Flags: bus master, fast devsel, latency 0, IRQ 26
	I/O ports at de00 [size=256]
	Memory at fdaff000 (64-bit, prefetchable) [size=4K]
	Memory at fdae0000 (64-bit, prefetchable) [size=64K]
	[virtual] Expansion ROM at fda00000 [disabled] [size=64K]
	Capabilities: [40] Power Management version 3
	Capabilities: [50] Message Signalled Interrupts: Mask- 64bit+ Queue=0/1 Enable+
	Capabilities: [70] Express Endpoint, MSI 01
	Capabilities: [b0] MSI-X: Enable- Mask- TabSize=2
	Capabilities: [d0] Vital Product Data <?>
	Capabilities: [100] Advanced Error Reporting <?>
	Capabilities: [140] Virtual Channel <?>
	Capabilities: [160] Device Serial Number 78-56-34-12-78-56-34-12
	Kernel driver in use: r8169
	Kernel modules: r8169

mii-tool after suspend:
eth0: negotiated 100baseTx-FD flow-control, link ok
  product info: vendor 00:07:32, model 17 rev 2
  basic mode:   autonegotiation enabled
  basic status: autonegotiation complete, link ok
  capabilities: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
  advertising:  100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD flow-control
  link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD flow-control

mii-tool after setting speed to 1000 with "ethtool -s eth0 speed 1000":
eth0: negotiated 1000baseT-FD flow-control, link ok
  product info: vendor 00:07:32, model 17 rev 2
  basic mode:   autonegotiation enabled
  basic status: autonegotiation complete, link ok
  capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
  advertising:  1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD flow-control
  link partner: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD flow-control


I did not knew if the Switch could be the problem and thus I connected the 
card to my laptop which also has a gigabit controller, but the result was 
the same as above.


regards,
Benjamin

^ permalink raw reply

* [PATCH 10/10, corrected diffstat] drivers/net: Move a dereference below a NULL test
From: Julia Lawall @ 2009-07-19 16:13 UTC (permalink / raw)
  To: netdev, linux-kernel, kernel-janitors

From: Julia Lawall <julia@diku.dk>

If the NULL test is necessary, then the dereferences should be moved below
the NULL test.

The semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@@
type T;
expression E,E1;
identifier i,fld;
statement S;
@@

- T i = E->fld;
+ T i;
  ... when != E=E1
      when != i
  BUG_ON (E == NULL||...);
+ i = E->fld;
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/net/ibm_newemac/rgmii.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ibm_newemac/rgmii.c b/drivers/net/ibm_newemac/rgmii.c
index 1d5379d..8d76cb8 100644
--- a/drivers/net/ibm_newemac/rgmii.c
+++ b/drivers/net/ibm_newemac/rgmii.c
@@ -188,11 +188,12 @@ void rgmii_put_mdio(struct of_device *ofdev, int input)
 void rgmii_detach(struct of_device *ofdev, int input)
 {
 	struct rgmii_instance *dev = dev_get_drvdata(&ofdev->dev);
-	struct rgmii_regs __iomem *p = dev->base;
-
-	mutex_lock(&dev->lock);
+	struct rgmii_regs __iomem *p;
 
 	BUG_ON(!dev || dev->users == 0);
+	p = dev->base;
+
+	mutex_lock(&dev->lock);
 
 	RGMII_DBG(dev, "detach(%d)" NL, input);
 

^ permalink raw reply related

* [PATCH] drivers/net/mlx4: Adjust constant
From: Julia Lawall @ 2009-07-19 16:09 UTC (permalink / raw)
  To: netdev, linux-kernel, kernel-janitors

From: Julia Lawall <julia@diku.dk>

The values in the advertising field are typically ADVERTISED_xxx, not
SUPPORTED_xxx.  Both SUPPORTED_10000baseT_Full and
ADVERTISED_1000baseT_Full have the same value.

The semantic match that finds this problem is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@@
struct ethtool_cmd E;
@@
*E.advertising = SUPPORTED_10000baseT_Full
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/net/mlx4/en_ethtool.c       |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/mlx4/en_ethtool.c b/drivers/net/mlx4/en_ethtool.c
index 091f990..86467b4 100644
--- a/drivers/net/mlx4/en_ethtool.c
+++ b/drivers/net/mlx4/en_ethtool.c
@@ -220,7 +220,7 @@ static int mlx4_en_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
 	cmd->autoneg = AUTONEG_DISABLE;
 	cmd->supported = SUPPORTED_10000baseT_Full;
-	cmd->advertising = SUPPORTED_10000baseT_Full;
+	cmd->advertising = ADVERTISED_1000baseT_Full;
 	if (netif_carrier_ok(dev)) {
 		cmd->speed = SPEED_10000;
 		cmd->duplex = DUPLEX_FULL;

^ permalink raw reply related

* [PATCH 10/10] drivers/net: Move a dereference below a NULL test
From: Julia Lawall @ 2009-07-19 15:31 UTC (permalink / raw)
  To: netdev, linux-kernel, kernel-janitors

From: Julia Lawall <julia@diku.dk>

If the NULL test is necessary, then the dereferences should be moved below
the NULL test.

The semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@@
type T;
expression E,E1;
identifier i,fld;
statement S;
@@

- T i = E->fld;
+ T i;
  ... when != E=E1
      when != i
  BUG_ON (E == NULL||...);
+ i = E->fld;
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 arch/arm/plat-omap/iovmm.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ibm_newemac/rgmii.c b/drivers/net/ibm_newemac/rgmii.c
index 1d5379d..8d76cb8 100644
--- a/drivers/net/ibm_newemac/rgmii.c
+++ b/drivers/net/ibm_newemac/rgmii.c
@@ -188,11 +188,12 @@ void rgmii_put_mdio(struct of_device *ofdev, int input)
 void rgmii_detach(struct of_device *ofdev, int input)
 {
 	struct rgmii_instance *dev = dev_get_drvdata(&ofdev->dev);
-	struct rgmii_regs __iomem *p = dev->base;
-
-	mutex_lock(&dev->lock);
+	struct rgmii_regs __iomem *p;
 
 	BUG_ON(!dev || dev->users == 0);
+	p = dev->base;
+
+	mutex_lock(&dev->lock);
 
 	RGMII_DBG(dev, "detach(%d)" NL, input);
 

^ permalink raw reply related

* [PATCH 4/10] drivers/net: Move a dereference below a NULL test
From: Julia Lawall @ 2009-07-19 15:26 UTC (permalink / raw)
  To: eilong, netdev, linux-kernel, kernel-janitors

From: Julia Lawall <julia@diku.dk>

If the NULL test is necessary, then the dereference should be moved below
the NULL test.

The semantic patch that makes this change is as follows:
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@@
type T;
expression E,E1;
identifier i,fld;
statement S;
@@

- T i = E->fld;
+ T i;
  ... when != E=E1
      when != i
  if (E == NULL||...) S
+ i = E->fld;
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 drivers/net/bnx2x_link.c            |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bnx2x_link.c b/drivers/net/bnx2x_link.c
index ed648ac..2ee581a 100644
--- a/drivers/net/bnx2x_link.c
+++ b/drivers/net/bnx2x_link.c
@@ -4212,13 +4212,14 @@ static void bnx2x_turn_off_sf(struct bnx2x *bp, u8 port)
 u8 bnx2x_get_ext_phy_fw_version(struct link_params *params, u8 driver_loaded,
 			      u8 *version, u16 len)
 {
-	struct bnx2x *bp = params->bp;
+	struct bnx2x *bp;
 	u32 ext_phy_type = 0;
 	u32 spirom_ver = 0;
 	u8 status = 0 ;
 
 	if (version == NULL || params == NULL)
 		return -EINVAL;
+	bp = params->bp;
 
 	spirom_ver = REG_RD(bp, params->shmem_base +
 		   offsetof(struct shmem_region,

^ permalink raw reply related

* [net-next-2.6 PATCH 3/3] mlx4_en: Not using Shared Receive Queues
From: Yevgeny Petrilin @ 2009-07-19 13:07 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

We use 1:1 mapping between QPs and SRQs on receive side,
so additional indirection level not required. Allocated the receive
buffers for the RSS QPs.

Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
---
 drivers/net/mlx4/en_netdev.c    |    3 +-
 drivers/net/mlx4/en_resources.c |    9 ++---
 drivers/net/mlx4/en_rx.c        |   77 +++++++++------------------------------
 drivers/net/mlx4/en_tx.c        |    4 +-
 drivers/net/mlx4/mlx4_en.h      |    6 +--
 5 files changed, 26 insertions(+), 73 deletions(-)

diff --git a/drivers/net/mlx4/en_netdev.c b/drivers/net/mlx4/en_netdev.c
index f8bbc5a..c48b0f4 100644
--- a/drivers/net/mlx4/en_netdev.c
+++ b/drivers/net/mlx4/en_netdev.c
@@ -622,8 +622,7 @@ int mlx4_en_start_port(struct net_device *dev)
 
 		/* Configure ring */
 		tx_ring = &priv->tx_ring[i];
-		err = mlx4_en_activate_tx_ring(priv, tx_ring, cq->mcq.cqn,
-					       priv->rx_ring[0].srq.srqn);
+		err = mlx4_en_activate_tx_ring(priv, tx_ring, cq->mcq.cqn);
 		if (err) {
 			en_err(priv, "Failed allocating Tx ring\n");
 			mlx4_en_deactivate_cq(priv, cq);
diff --git a/drivers/net/mlx4/en_resources.c b/drivers/net/mlx4/en_resources.c
index 65ca706..1625678 100644
--- a/drivers/net/mlx4/en_resources.c
+++ b/drivers/net/mlx4/en_resources.c
@@ -37,7 +37,7 @@
 #include "mlx4_en.h"
 
 void mlx4_en_fill_qp_context(struct mlx4_en_priv *priv, int size, int stride,
-			     int is_tx, int rss, int qpn, int cqn, int srqn,
+			     int is_tx, int rss, int qpn, int cqn,
 			     struct mlx4_qp_context *context)
 {
 	struct mlx4_en_dev *mdev = priv->mdev;
@@ -46,11 +46,12 @@ void mlx4_en_fill_qp_context(struct mlx4_en_priv *priv, int size, int stride,
 	context->flags = cpu_to_be32(7 << 16 | rss << 13);
 	context->pd = cpu_to_be32(mdev->priv_pdn);
 	context->mtu_msgmax = 0xff;
-	context->rq_size_stride = 0;
+	if (!is_tx && !rss)
+		context->rq_size_stride = ilog2(size) << 3 | (ilog2(stride) - 4);
 	if (is_tx)
 		context->sq_size_stride = ilog2(size) << 3 | (ilog2(stride) - 4);
 	else
-		context->sq_size_stride = 1;
+		context->sq_size_stride = ilog2(TXBB_SIZE) - 4;
 	context->usr_page = cpu_to_be32(mdev->priv_uar.index);
 	context->local_qpn = cpu_to_be32(qpn);
 	context->pri_path.ackto = 1 & 0x07;
@@ -59,8 +60,6 @@ void mlx4_en_fill_qp_context(struct mlx4_en_priv *priv, int size, int stride,
 	context->cqn_send = cpu_to_be32(cqn);
 	context->cqn_recv = cpu_to_be32(cqn);
 	context->db_rec_addr = cpu_to_be64(priv->res.db.dma << 2);
-	if (!rss)
-		context->srqn = cpu_to_be32(MLX4_EN_USE_SRQ | srqn);
 }
 
 
diff --git a/drivers/net/mlx4/en_rx.c b/drivers/net/mlx4/en_rx.c
index 47b178e..cd084de 100644
--- a/drivers/net/mlx4/en_rx.c
+++ b/drivers/net/mlx4/en_rx.c
@@ -40,16 +40,6 @@
 
 #include "mlx4_en.h"
 
-static void *get_wqe(struct mlx4_en_rx_ring *ring, int n)
-{
-	int offset = n << ring->srq.wqe_shift;
-	return ring->buf + offset;
-}
-
-static void mlx4_en_srq_event(struct mlx4_srq *srq, enum mlx4_event type)
-{
-	return;
-}
 
 static int mlx4_en_get_frag_header(struct skb_frag_struct *frags, void **mac_hdr,
 				   void **ip_hdr, void **tcpudp_hdr,
@@ -154,9 +144,6 @@ static void mlx4_en_init_rx_desc(struct mlx4_en_priv *priv,
 	int possible_frags;
 	int i;
 
-	/* Pre-link descriptor */
-	rx_desc->next.next_wqe_index = cpu_to_be16((index + 1) & ring->size_mask);
-
 	/* Set size and memtype fields */
 	for (i = 0; i < priv->num_frags; i++) {
 		skb_frags[i].size = priv->frag_info[i].frag_size;
@@ -294,9 +281,6 @@ int mlx4_en_create_rx_ring(struct mlx4_en_priv *priv,
 	int err;
 	int tmp;
 
-	/* Sanity check SRQ size before proceeding */
-	if (size >= mdev->dev->caps.max_srq_wqes)
-		return -EINVAL;
 
 	ring->prod = 0;
 	ring->cons = 0;
@@ -304,7 +288,7 @@ int mlx4_en_create_rx_ring(struct mlx4_en_priv *priv,
 	ring->size_mask = size - 1;
 	ring->stride = stride;
 	ring->log_stride = ffs(ring->stride) - 1;
-	ring->buf_size = ring->size * ring->stride;
+	ring->buf_size = ring->size * ring->stride + TXBB_SIZE;
 
 	tmp = size * roundup_pow_of_two(MLX4_EN_MAX_RX_FRAGS *
 					sizeof(struct skb_frag_struct));
@@ -360,15 +344,12 @@ err_ring:
 
 int mlx4_en_activate_rx_rings(struct mlx4_en_priv *priv)
 {
-	struct mlx4_en_dev *mdev = priv->mdev;
-	struct mlx4_wqe_srq_next_seg *next;
 	struct mlx4_en_rx_ring *ring;
 	int i;
 	int ring_ind;
 	int err;
 	int stride = roundup_pow_of_two(sizeof(struct mlx4_en_rx_desc) +
 					DS_SIZE * priv->num_frags);
-	int max_gs = (stride - sizeof(struct mlx4_wqe_srq_next_seg)) / DS_SIZE;
 
 	for (ring_ind = 0; ring_ind < priv->rx_ring_num; ring_ind++) {
 		ring = &priv->rx_ring[ring_ind];
@@ -379,6 +360,9 @@ int mlx4_en_activate_rx_rings(struct mlx4_en_priv *priv)
 		ring->cqn = priv->rx_cq[ring_ind].mcq.cqn;
 
 		ring->stride = stride;
+		if (ring->stride <= TXBB_SIZE)
+			ring->buf += TXBB_SIZE;
+
 		ring->log_stride = ffs(ring->stride) - 1;
 		ring->buf_size = ring->size * ring->stride;
 
@@ -405,37 +389,10 @@ int mlx4_en_activate_rx_rings(struct mlx4_en_priv *priv)
 		ring = &priv->rx_ring[ring_ind];
 
 		mlx4_en_update_rx_prod_db(ring);
-
-		/* Configure SRQ representing the ring */
-		ring->srq.max    = ring->actual_size;
-		ring->srq.max_gs = max_gs;
-		ring->srq.wqe_shift = ilog2(ring->stride);
-
-		for (i = 0; i < ring->srq.max; ++i) {
-			next = get_wqe(ring, i);
-			next->next_wqe_index =
-			cpu_to_be16((i + 1) & (ring->srq.max - 1));
-		}
-
-		err = mlx4_srq_alloc(mdev->dev, mdev->priv_pdn, &ring->wqres.mtt,
-				     ring->wqres.db.dma, &ring->srq);
-		if (err){
-			en_err(priv, "Failed to allocate srq\n");
-			ring_ind--;
-			goto err_srq;
-		}
-		ring->srq.event = mlx4_en_srq_event;
 	}
 
 	return 0;
 
-err_srq:
-	while (ring_ind >= 0) {
-		ring = &priv->rx_ring[ring_ind];
-		mlx4_srq_free(mdev->dev, &ring->srq);
-		ring_ind--;
-	}
-
 err_buffers:
 	for (ring_ind = 0; ring_ind < priv->rx_ring_num; ring_ind++)
 		mlx4_en_free_rx_buf(priv, &priv->rx_ring[ring_ind]);
@@ -456,7 +413,7 @@ void mlx4_en_destroy_rx_ring(struct mlx4_en_priv *priv,
 
 	kfree(ring->lro.lro_arr);
 	mlx4_en_unmap_buffer(&ring->wqres.buf);
-	mlx4_free_hwq_res(mdev->dev, &ring->wqres, ring->buf_size);
+	mlx4_free_hwq_res(mdev->dev, &ring->wqres, ring->buf_size + TXBB_SIZE);
 	vfree(ring->rx_info);
 	ring->rx_info = NULL;
 }
@@ -464,10 +421,9 @@ void mlx4_en_destroy_rx_ring(struct mlx4_en_priv *priv,
 void mlx4_en_deactivate_rx_ring(struct mlx4_en_priv *priv,
 				struct mlx4_en_rx_ring *ring)
 {
-	struct mlx4_en_dev *mdev = priv->mdev;
-
-	mlx4_srq_free(mdev->dev, &ring->srq);
 	mlx4_en_free_rx_buf(priv, ring);
+	if (ring->stride <= TXBB_SIZE)
+		ring->buf -= TXBB_SIZE;
 	mlx4_en_destroy_allocator(priv, ring);
 }
 
@@ -835,8 +791,8 @@ void mlx4_en_calc_rx_buf(struct net_device *dev)
 
 /* RSS related functions */
 
-static int mlx4_en_config_rss_qp(struct mlx4_en_priv *priv,
-				 int qpn, int srqn, int cqn,
+static int mlx4_en_config_rss_qp(struct mlx4_en_priv *priv, int qpn,
+				 struct mlx4_en_rx_ring *ring,
 				 enum mlx4_qp_state *state,
 				 struct mlx4_qp *qp)
 {
@@ -858,13 +814,16 @@ static int mlx4_en_config_rss_qp(struct mlx4_en_priv *priv,
 	qp->event = mlx4_en_sqp_event;
 
 	memset(context, 0, sizeof *context);
-	mlx4_en_fill_qp_context(priv, 0, 0, 0, 0, qpn, cqn, srqn, context);
+	mlx4_en_fill_qp_context(priv, ring->size, ring->stride, 0, 0,
+				qpn, ring->cqn, context);
+	context->db_rec_addr = cpu_to_be64(ring->wqres.db.dma);
 
-	err = mlx4_qp_to_ready(mdev->dev, &priv->res.mtt, context, qp, state);
+	err = mlx4_qp_to_ready(mdev->dev, &ring->wqres.mtt, context, qp, state);
 	if (err) {
 		mlx4_qp_remove(mdev->dev, qp);
 		mlx4_qp_free(mdev->dev, qp);
 	}
+	mlx4_en_update_rx_prod_db(ring);
 out:
 	kfree(context);
 	return err;
@@ -880,7 +839,7 @@ int mlx4_en_config_rss_steer(struct mlx4_en_priv *priv)
 	void *ptr;
 	int rss_xor = mdev->profile.rss_xor;
 	u8 rss_mask = mdev->profile.rss_mask;
-	int i, srqn, qpn, cqn;
+	int i, qpn;
 	int err = 0;
 	int good_qps = 0;
 
@@ -894,10 +853,8 @@ int mlx4_en_config_rss_steer(struct mlx4_en_priv *priv)
 	}
 
 	for (i = 0; i < priv->rx_ring_num; i++) {
-		cqn = priv->rx_ring[i].cqn;
-		srqn = priv->rx_ring[i].srq.srqn;
 		qpn = rss_map->base_qpn + i;
-		err = mlx4_en_config_rss_qp(priv, qpn, srqn, cqn,
+		err = mlx4_en_config_rss_qp(priv, qpn, &priv->rx_ring[i],
 					    &rss_map->state[i],
 					    &rss_map->qps[i]);
 		if (err)
@@ -920,7 +877,7 @@ int mlx4_en_config_rss_steer(struct mlx4_en_priv *priv)
 	}
 	rss_map->indir_qp.event = mlx4_en_sqp_event;
 	mlx4_en_fill_qp_context(priv, 0, 0, 0, 1, priv->base_qpn,
-				priv->rx_ring[0].cqn, 0, &context);
+				priv->rx_ring[0].cqn, &context);
 
 	ptr = ((void *) &context) + 0x3c;
 	rss_context = (struct mlx4_en_rss_context *) ptr;
diff --git a/drivers/net/mlx4/en_tx.c b/drivers/net/mlx4/en_tx.c
index 3e8a404..0ecc1e1 100644
--- a/drivers/net/mlx4/en_tx.c
+++ b/drivers/net/mlx4/en_tx.c
@@ -150,7 +150,7 @@ void mlx4_en_destroy_tx_ring(struct mlx4_en_priv *priv,
 
 int mlx4_en_activate_tx_ring(struct mlx4_en_priv *priv,
 			     struct mlx4_en_tx_ring *ring,
-			     int cq, int srqn)
+			     int cq)
 {
 	struct mlx4_en_dev *mdev = priv->mdev;
 	int err;
@@ -168,7 +168,7 @@ int mlx4_en_activate_tx_ring(struct mlx4_en_priv *priv,
 	ring->doorbell_qpn = swab32(ring->qp.qpn << 8);
 
 	mlx4_en_fill_qp_context(priv, ring->size, ring->stride, 1, 0, ring->qpn,
-				ring->cqn, srqn, &ring->context);
+				ring->cqn, &ring->context);
 
 	err = mlx4_qp_to_ready(mdev->dev, &ring->wqres.mtt, &ring->context,
 			       &ring->qp, &ring->qp_state);
diff --git a/drivers/net/mlx4/mlx4_en.h b/drivers/net/mlx4/mlx4_en.h
index 2d76ff4..4513fb4 100644
--- a/drivers/net/mlx4/mlx4_en.h
+++ b/drivers/net/mlx4/mlx4_en.h
@@ -274,13 +274,11 @@ struct mlx4_en_tx_ring {
 };
 
 struct mlx4_en_rx_desc {
-	struct mlx4_wqe_srq_next_seg next;
 	/* actual number of entries depends on rx ring stride */
 	struct mlx4_wqe_data_seg data[0];
 };
 
 struct mlx4_en_rx_ring {
-	struct mlx4_srq srq;
 	struct mlx4_hwq_resources wqres;
 	struct mlx4_en_rx_alloc page_alloc[MLX4_EN_MAX_RX_FRAGS];
 	struct net_lro_mgr lro;
@@ -527,7 +525,7 @@ int mlx4_en_create_tx_ring(struct mlx4_en_priv *priv, struct mlx4_en_tx_ring *ri
 void mlx4_en_destroy_tx_ring(struct mlx4_en_priv *priv, struct mlx4_en_tx_ring *ring);
 int mlx4_en_activate_tx_ring(struct mlx4_en_priv *priv,
 			     struct mlx4_en_tx_ring *ring,
-			     int cq, int srqn);
+			     int cq);
 void mlx4_en_deactivate_tx_ring(struct mlx4_en_priv *priv,
 				struct mlx4_en_tx_ring *ring);
 
@@ -544,7 +542,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev,
 			  int budget);
 int mlx4_en_poll_rx_cq(struct napi_struct *napi, int budget);
 void mlx4_en_fill_qp_context(struct mlx4_en_priv *priv, int size, int stride,
-			     int is_tx, int rss, int qpn, int cqn, int srqn,
+			     int is_tx, int rss, int qpn, int cqn,
 			     struct mlx4_qp_context *context);
 void mlx4_en_sqp_event(struct mlx4_qp *qp, enum mlx4_event event);
 int mlx4_en_map_buffer(struct mlx4_buf *buf);
-- 
1.6.0


^ permalink raw reply related

* [net-next-2.6 PATCH 2/3] mlx4_en: Using real number of rings as RSS map size
From: Yevgeny Petrilin @ 2009-07-19 13:07 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

There is no point in using more QPs then actual number of receive rings.
If the RSS function for two streams gives the same result modulo number
of rings, they will arrive to the same RX ring anyway.

Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
---
 drivers/net/mlx4/en_main.c   |    5 +++--
 drivers/net/mlx4/en_netdev.c |    3 ---
 drivers/net/mlx4/en_rx.c     |   38 +++++++++++---------------------------
 drivers/net/mlx4/mlx4_en.h   |   11 ++---------
 4 files changed, 16 insertions(+), 41 deletions(-)

diff --git a/drivers/net/mlx4/en_main.c b/drivers/net/mlx4/en_main.c
index 9ed4a15..507e11f 100644
--- a/drivers/net/mlx4/en_main.c
+++ b/drivers/net/mlx4/en_main.c
@@ -218,8 +218,9 @@ static void *mlx4_en_add(struct mlx4_dev *dev)
 	mlx4_foreach_port(i, dev, MLX4_PORT_TYPE_ETH) {
 		mlx4_info(mdev, "Using %d tx rings for port:%d\n",
 			  mdev->profile.prof[i].tx_ring_num, i);
-		mdev->profile.prof[i].rx_ring_num =
-			min_t(int, dev->caps.num_comp_vectors, MAX_RX_RINGS);
+		mdev->profile.prof[i].rx_ring_num = min_t(int,
+			roundup_pow_of_two(dev->caps.num_comp_vectors),
+			MAX_RX_RINGS);
 		mlx4_info(mdev, "Defaulting to %d rx rings for port:%d\n",
 			  mdev->profile.prof[i].rx_ring_num, i);
 	}
diff --git a/drivers/net/mlx4/en_netdev.c b/drivers/net/mlx4/en_netdev.c
index c8a24dc..f8bbc5a 100644
--- a/drivers/net/mlx4/en_netdev.c
+++ b/drivers/net/mlx4/en_netdev.c
@@ -1011,9 +1011,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
 	if (err)
 		goto out;
 
-	/* Populate Rx default RSS mappings */
-	mlx4_en_set_default_rss_map(priv, &priv->rss_map, priv->rx_ring_num *
-						RSS_FACTOR, priv->rx_ring_num);
 	/* Allocate page for receive rings */
 	err = mlx4_alloc_hwq_res(mdev->dev, &priv->res,
 				MLX4_EN_PAGE_SIZE, MLX4_EN_PAGE_SIZE);
diff --git a/drivers/net/mlx4/en_rx.c b/drivers/net/mlx4/en_rx.c
index 91bdfdf..47b178e 100644
--- a/drivers/net/mlx4/en_rx.c
+++ b/drivers/net/mlx4/en_rx.c
@@ -835,23 +835,6 @@ void mlx4_en_calc_rx_buf(struct net_device *dev)
 
 /* RSS related functions */
 
-/* Calculate rss size and map each entry in rss table to rx ring */
-void mlx4_en_set_default_rss_map(struct mlx4_en_priv *priv,
-				 struct mlx4_en_rss_map *rss_map,
-				 int num_entries, int num_rings)
-{
-	int i;
-
-	rss_map->size = roundup_pow_of_two(num_entries);
-	en_dbg(DRV, priv, "Setting default RSS map of %d entires\n",
-	       rss_map->size);
-
-	for (i = 0; i < rss_map->size; i++) {
-		rss_map->map[i] = i % num_rings;
-		en_dbg(DRV, priv, "Entry %d ---> ring %d\n", i, rss_map->map[i]);
-	}
-}
-
 static int mlx4_en_config_rss_qp(struct mlx4_en_priv *priv,
 				 int qpn, int srqn, int cqn,
 				 enum mlx4_qp_state *state,
@@ -902,16 +885,17 @@ int mlx4_en_config_rss_steer(struct mlx4_en_priv *priv)
 	int good_qps = 0;
 
 	en_dbg(DRV, priv, "Configuring rss steering\n");
-	err = mlx4_qp_reserve_range(mdev->dev, rss_map->size,
-				    rss_map->size, &rss_map->base_qpn);
+	err = mlx4_qp_reserve_range(mdev->dev, priv->rx_ring_num,
+				    priv->rx_ring_num,
+				    &rss_map->base_qpn);
 	if (err) {
-		en_err(priv, "Failed reserving %d qps\n", rss_map->size);
+		en_err(priv, "Failed reserving %d qps\n", priv->rx_ring_num);
 		return err;
 	}
 
-	for (i = 0; i < rss_map->size; i++) {
-		cqn = priv->rx_ring[rss_map->map[i]].cqn;
-		srqn = priv->rx_ring[rss_map->map[i]].srq.srqn;
+	for (i = 0; i < priv->rx_ring_num; i++) {
+		cqn = priv->rx_ring[i].cqn;
+		srqn = priv->rx_ring[i].srq.srqn;
 		qpn = rss_map->base_qpn + i;
 		err = mlx4_en_config_rss_qp(priv, qpn, srqn, cqn,
 					    &rss_map->state[i],
@@ -940,7 +924,7 @@ int mlx4_en_config_rss_steer(struct mlx4_en_priv *priv)
 
 	ptr = ((void *) &context) + 0x3c;
 	rss_context = (struct mlx4_en_rss_context *) ptr;
-	rss_context->base_qpn = cpu_to_be32(ilog2(rss_map->size) << 24 |
+	rss_context->base_qpn = cpu_to_be32(ilog2(priv->rx_ring_num) << 24 |
 					    (rss_map->base_qpn));
 	rss_context->default_qpn = cpu_to_be32(rss_map->base_qpn);
 	rss_context->hash_fn = rss_xor & 0x3;
@@ -967,7 +951,7 @@ rss_err:
 		mlx4_qp_remove(mdev->dev, &rss_map->qps[i]);
 		mlx4_qp_free(mdev->dev, &rss_map->qps[i]);
 	}
-	mlx4_qp_release_range(mdev->dev, rss_map->base_qpn, rss_map->size);
+	mlx4_qp_release_range(mdev->dev, rss_map->base_qpn, priv->rx_ring_num);
 	return err;
 }
 
@@ -983,13 +967,13 @@ void mlx4_en_release_rss_steer(struct mlx4_en_priv *priv)
 	mlx4_qp_free(mdev->dev, &rss_map->indir_qp);
 	mlx4_qp_release_range(mdev->dev, priv->base_qpn, 1);
 
-	for (i = 0; i < rss_map->size; i++) {
+	for (i = 0; i < priv->rx_ring_num; i++) {
 		mlx4_qp_modify(mdev->dev, NULL, rss_map->state[i],
 			       MLX4_QP_STATE_RST, NULL, 0, 0, &rss_map->qps[i]);
 		mlx4_qp_remove(mdev->dev, &rss_map->qps[i]);
 		mlx4_qp_free(mdev->dev, &rss_map->qps[i]);
 	}
-	mlx4_qp_release_range(mdev->dev, rss_map->base_qpn, rss_map->size);
+	mlx4_qp_release_range(mdev->dev, rss_map->base_qpn, priv->rx_ring_num);
 }
 
 
diff --git a/drivers/net/mlx4/mlx4_en.h b/drivers/net/mlx4/mlx4_en.h
index c7c5e86..2d76ff4 100644
--- a/drivers/net/mlx4/mlx4_en.h
+++ b/drivers/net/mlx4/mlx4_en.h
@@ -95,8 +95,6 @@
 #define MLX4_EN_PAGE_SIZE	(1 << MLX4_EN_PAGE_SHIFT)
 #define MAX_TX_RINGS		16
 #define MAX_RX_RINGS		16
-#define MAX_RSS_MAP_SIZE	64
-#define RSS_FACTOR		2
 #define TXBB_SIZE		64
 #define HEADROOM		(2048 / TXBB_SIZE + 1)
 #define STAMP_STRIDE		64
@@ -377,11 +375,9 @@ struct mlx4_en_dev {
 
 
 struct mlx4_en_rss_map {
-	int size;
 	int base_qpn;
-	u16 map[MAX_RSS_MAP_SIZE];
-	struct mlx4_qp qps[MAX_RSS_MAP_SIZE];
-	enum mlx4_qp_state state[MAX_RSS_MAP_SIZE];
+	struct mlx4_qp qps[MAX_RX_RINGS];
+	enum mlx4_qp_state state[MAX_RX_RINGS];
 	struct mlx4_qp indir_qp;
 	enum mlx4_qp_state indir_state;
 };
@@ -555,9 +551,6 @@ int mlx4_en_map_buffer(struct mlx4_buf *buf);
 void mlx4_en_unmap_buffer(struct mlx4_buf *buf);
 
 void mlx4_en_calc_rx_buf(struct net_device *dev);
-void mlx4_en_set_default_rss_map(struct mlx4_en_priv *priv,
-				 struct mlx4_en_rss_map *rss_map,
-				 int num_entries, int num_rings);
 int mlx4_en_config_rss_steer(struct mlx4_en_priv *priv);
 void mlx4_en_release_rss_steer(struct mlx4_en_priv *priv);
 int mlx4_en_free_tx_buf(struct net_device *dev, struct mlx4_en_tx_ring *ring);
-- 
1.6.0


^ permalink raw reply related

* [net-next-2.6 PATCH 1/3] mlx4_en: Adaptive moderation policy change
From: Yevgeny Petrilin @ 2009-07-19 13:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

If the net device is identified as "sender" (number of sent packets
is higher then the number of received packets and the incoming packets are
small), set the moderation time to its low limit.
We do it because the incoming packets are acks, and we don't want to delay them

Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
---
 drivers/net/mlx4/en_netdev.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx4/en_netdev.c b/drivers/net/mlx4/en_netdev.c
index 93f4abd..c8a24dc 100644
--- a/drivers/net/mlx4/en_netdev.c
+++ b/drivers/net/mlx4/en_netdev.c
@@ -414,6 +414,7 @@ static void mlx4_en_auto_moderation(struct mlx4_en_priv *priv)
 	unsigned long avg_pkt_size;
 	unsigned long rx_packets;
 	unsigned long rx_bytes;
+	unsigned long rx_byte_diff;
 	unsigned long tx_packets;
 	unsigned long tx_pkt_diff;
 	unsigned long rx_pkt_diff;
@@ -437,6 +438,8 @@ static void mlx4_en_auto_moderation(struct mlx4_en_priv *priv)
 	rx_pkt_diff = ((unsigned long) (rx_packets -
 					priv->last_moder_packets));
 	packets = max(tx_pkt_diff, rx_pkt_diff);
+	rx_byte_diff = rx_bytes - priv->last_moder_bytes;
+	rx_byte_diff = rx_byte_diff ? rx_byte_diff : 1;
 	rate = packets * HZ / period;
 	avg_pkt_size = packets ? ((unsigned long) (rx_bytes -
 				 priv->last_moder_bytes)) / packets : 0;
@@ -447,10 +450,13 @@ static void mlx4_en_auto_moderation(struct mlx4_en_priv *priv)
 		/* If tx and rx packet rates are not balanced, assume that
 		 * traffic is mainly BW bound and apply maximum moderation.
 		 * Otherwise, moderate according to packet rate */
-		if (2 * tx_pkt_diff > 3 * rx_pkt_diff ||
-		    2 * rx_pkt_diff > 3 * tx_pkt_diff) {
+		if (2 * tx_pkt_diff > 3 * rx_pkt_diff &&
+		    rx_pkt_diff / rx_byte_diff <
+		    MLX4_EN_SMALL_PKT_SIZE)
+			moder_time = priv->rx_usecs_low;
+		else if (2 * rx_pkt_diff > 3 * tx_pkt_diff)
 			moder_time = priv->rx_usecs_high;
-		} else {
+		else {
 			if (rate < priv->pkt_rate_low)
 				moder_time = priv->rx_usecs_low;
 			else if (rate > priv->pkt_rate_high)
-- 
1.6.0


^ permalink raw reply related

* dl2k: potential null dereference in recieve_packet()
From: Dan Carpenter @ 2009-07-19 11:59 UTC (permalink / raw)
  To: netdev

Hello,

Smatch (http://repo.or.cz/w/smatch.git) found a potential null dereference 
in drivers/net/dl2k.c receive_packet().  If the allocation on line 874 
fails we still dereference skb on line 890.

drivers/net/dl2k.c 
   874                          } else if ((skb = netdev_alloc_skb(dev, pkt_len + 2))) {
   875                                  pci_dma_sync_single_for_cpu(np->pdev,
   876                                                              desc_to_dma(desc),
   877                                                              np->rx_buf_sz,
   878                                                              PCI_DMA_FROMDEVICE);
   879                                  /* 16 byte align the IP header */
   880                                  skb_reserve (skb, 2);
   881                                  skb_copy_to_linear_data (skb,
   882                                                    np->rx_skbuff[entry]->data,
   883                                                    pkt_len);
   884                                  skb_put (skb, pkt_len);
   885                                  pci_dma_sync_single_for_device(np->pdev,
   886                                                                 desc_to_dma(desc),
   887                                                                 np->rx_buf_sz,
   888                                                                 PCI_DMA_FROMDEVICE);
   889                          }
   890                          skb->protocol = eth_type_trans (skb, dev);

regards,
dan carpenter


^ permalink raw reply

* Re: [PATCH] Add mac driver for w90p910
From: Russell King - ARM Linux @ 2009-07-19 10:16 UTC (permalink / raw)
  To: Wan ZongShun
  Cc: Trilok Soni, David S. Miller, linux-netdev, linux-arm-kernel,
	Eric.miao
In-Reply-To: <4A605EC1.1070203@gmail.com>

On Fri, Jul 17, 2009 at 07:21:37PM +0800, Wan ZongShun wrote:
> Some locks seems useless, So I remove them, and regarding the casting,
> I have to keep "(dma_addr_t *)" here, as I remove it, a "warning occurs, as following:
> "warning: passing argument 3 of 'dma_alloc_coherent' from incompatible pointer type"

This tends to mean you don't have things quite right then.  Below are
some suggestions which should resolve this.

> +struct  w90p910_ether {
> +	struct recv_pdesc *rdesc;
> +	struct recv_pdesc *rdesc_phys;

	dma_addr_t rdesc_phys;

> +	struct tran_pdesc *tdesc;
> +	struct tran_pdesc *tdesc_phys;

	dma_addr_t tdesc_phys;

> +	struct net_device_stats stats;
> +	struct platform_device *pdev;
> +	struct sk_buff *skb;
> +	struct clk *clk;
> +	struct clk *rmiiclk;
> +	struct mii_if_info mii;
> +	struct timer_list check_timer;
> +	void __iomem *reg;
> +	unsigned int rxirq;
> +	unsigned int txirq;
> +	unsigned int cur_tx;
> +	unsigned int cur_rx;
> +	unsigned int finish_tx;
> +	unsigned int rx_packets;
> +	unsigned int rx_bytes;
> +	unsigned int start_tx_ptr;
> +	unsigned int start_rx_ptr;
> +	unsigned int linkflag;
> +};
...
> +static void w90p910_init_desc(struct net_device *dev)
> +{
> +	struct w90p910_ether *ether;
> +	struct w90p910_txbd  *tdesc, *tdesc_phys;
> +	struct w90p910_rxbd  *rdesc, *rdesc_phys;
> +	unsigned int i, j;
> +
> +	ether = netdev_priv(dev);
> +
> +	ether->tdesc = (struct tran_pdesc *)
> +			dma_alloc_coherent(NULL, sizeof(struct tran_pdesc),
> +				(dma_addr_t *)&ether->tdesc_phys, GFP_KERNEL);
> +
> +	ether->rdesc = (struct recv_pdesc *)
> +			dma_alloc_coherent(NULL, sizeof(struct recv_pdesc),
> +				(dma_addr_t *)&ether->rdesc_phys, GFP_KERNEL);

	ether->tdesc = dma_alloc_cohernet(NULL, sizeof(struct tran_pdesc),
				&ether->tdesc_phys, GFP_KERNEL);
	ether->rdesc = dma_alloc_cohernet(NULL, sizeof(struct recv_pdesc),
				&ether->rdesc_phys, GFP_KERNEL);

Note that these functions can fail and return a NULL pointer - you need
to check for that.  Also I wonder why you're not passing &ether->pdev->dev
as the first argument.

> +
> +	for (i = 0; i < TX_DESC_SIZE; i++) {
> +		tdesc = &(ether->tdesc->desclist[i]);
> +
> +		j = ((i + 1) / TX_DESC_SIZE);
> +
> +		if (j != 0) {

This is a weird way of detecting the last interation in the loop.
Wouldn't:

		if (i == TX_DESC_SIZE - 1) {

be sufficient?

> +			tdesc_phys = &(ether->tdesc_phys->desclist[0]);
> +			ether->start_tx_ptr = (unsigned int)tdesc_phys;
> +			tdesc->next = (unsigned int)ether->start_tx_ptr;
> +		} else {
> +			tdesc_phys = &(ether->tdesc_phys->desclist[i+1]);
> +			tdesc->next = (unsigned int)tdesc_phys;
> +		}
> +
> +		tdesc->buffer = (unsigned int)ether->tdesc_phys->tran_buf[i];
> +		tdesc->sl = 0;
> +		tdesc->mode = 0;
> +	}

How about this instead:

	for (i = 0; i < TX_DESC_SIZE; i++) {
		unsigned int offset;

		tdesc = &(ether->tdesc->desclist[i]);

		if (i == TX_DESC_SIZE - 1)
			offset = offsetof(struct tran_pdesc, desclist[0]);
		else
			offset = offsetof(struct tran_pdesc, desclist[i + 1]);

		tdesc->next = ether->tdesc_phys + offset;
		tdesc->buffer = ether->tdesc_phys + 
			offsetof(struct tran_pdesc, tran_buf[i]);
		tdesc->sl = 0;
		tdesc->mode = 0;
	}

	ether->start_tx_ptr = ether->tdesc_phys;

> +
> +	for (i = 0; i < RX_DESC_SIZE; i++) {
> +		rdesc = &(ether->rdesc->desclist[i]);
> +
> +		j = ((i + 1) / RX_DESC_SIZE);
> +
> +		if (j != 0) {
> +			rdesc_phys = &(ether->rdesc_phys->desclist[0]);
> +			ether->start_rx_ptr = (unsigned int)rdesc_phys;
> +			rdesc->next = (unsigned int)ether->start_rx_ptr;
> +		} else {
> +			rdesc_phys = &(ether->rdesc_phys->desclist[i+1]);
> +			rdesc->next = (unsigned int)rdesc_phys;
> +		}
> +
> +		rdesc->sl = RX_OWEN_DMA;
> +		rdesc->buffer = (unsigned int)ether->rdesc_phys->recv_buf[i];
> +	  }

Same kind of fixup here.

> +}
...
> +static int w90p910_ether_close(struct net_device *dev)
> +{
> +	struct w90p910_ether *ether = netdev_priv(dev);
> +
> +	dma_free_writecombine(NULL, sizeof(struct w90p910_rxbd),
> +				ether->rdesc, (dma_addr_t)ether->rdesc_phys);
> +	dma_free_writecombine(NULL, sizeof(struct w90p910_txbd),
> +				ether->tdesc, (dma_addr_t)ether->tdesc_phys);

With rdesc_phys and tdesc_phys correctly typed, these casts can go away.
These functions should also be dma_free_cohernet(), and also note comment
about first argument for dma_alloc_coherent() also applies here.

> +
> +	netif_stop_queue(dev);
> +
> +	del_timer_sync(&ether->check_timer);
> +	clk_disable(ether->rmiiclk);
> +	clk_disable(ether->clk);
> +
> +	free_irq(ether->txirq, dev);
> +	free_irq(ether->rxirq, dev);
> +
> +	return 0;
> +}
...
> +static irqreturn_t w90p910_tx_interrupt(int irq, void *dev_id)
> +{
> +	struct w90p910_ether *ether;
> +	struct w90p910_txbd  *txbd;
> +	struct platform_device *pdev;
> +	struct tran_pdesc *tran_pdesc;
> +	struct net_device *dev;
> +	unsigned int cur_entry, entry, status;
> +
> +	dev = dev_id;
> +	ether = netdev_priv(dev);
> +	pdev = ether->pdev;
> +
> +	w90p910_get_and_clear_int(dev, &status);
> +
> +	cur_entry = __raw_readl(ether->reg + REG_CTXDSA);
> +
> +	tran_pdesc = ether->tdesc_phys;
> +	entry = (unsigned int)(&tran_pdesc->desclist[ether->finish_tx]);

	entry = ether->tdesc_phys +
		offsetof(struct tran_pdesc, desclist[ether->finish_tx]);

> +
> +	while (entry != cur_entry) {
> +		txbd = &ether->tdesc->desclist[ether->finish_tx];
> +
> +		ether->finish_tx = (ether->finish_tx + 1) % TX_DESC_SIZE;

Division and modulus can be expensive when not powers of two.  Would:

		if (++ether->finish_tx >= TX_DESC_SIZE)
			ether->finish_tx = 0;

be better?

> +
> +		if (txbd->sl & TXDS_TXCP) {
> +			ether->stats.tx_packets++;
> +			ether->stats.tx_bytes += txbd->sl & 0xFFFF;
> +		} else {
> +			ether->stats.tx_errors++;
> +		}
> +
> +		txbd->sl = 0x0;
> +		txbd->mode = 0x0;
> +
> +		if (netif_queue_stopped(dev))
> +			netif_wake_queue(dev);
> +
> +		entry = (unsigned int)(&tran_pdesc->desclist[ether->finish_tx]);

		entry = ether->tdesc_phys +
			offsetof(struct tran_pdesc, desclist[ether->finish_tx]);

> +	}
> +
> +	if (status & MISTA_EXDEF) {
> +		dev_err(&pdev->dev, "emc defer exceed interrupt\n");
> +	} else if (status & MISTA_TXBERR) {
> +		dev_err(&pdev->dev, "emc bus error interrupt\n");
> +		w90p910_reset_mac(dev);
> +	} else if (status & MISTA_TDU) {
> +		if (netif_queue_stopped(dev))
> +			netif_wake_queue(dev);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void netdev_rx(struct net_device *dev)
> +{
> +	struct w90p910_ether *ether;
> +	struct w90p910_rxbd *rxbd;
> +	struct platform_device *pdev;
> +	struct recv_pdesc *rdesc_phys;
> +	struct sk_buff *skb;
> +	unsigned char *data;
> +	unsigned int length, status, val, entry;
> +
> +	ether = netdev_priv(dev);
> +	pdev = ether->pdev;
> +	rdesc_phys = ether->rdesc_phys;
> +
> +	rxbd = &ether->rdesc->desclist[ether->cur_rx];
> +
> +	do {
> +		val = __raw_readl(ether->reg + REG_CRXDSA);
> +		entry = (unsigned int)&rdesc_phys->desclist[ether->cur_rx];

Same kind of fixups as above.

> +
> +		if (val == entry)
> +			break;
> +
> +		status = rxbd->sl;
> +		length = status & 0xFFFF;
> +
> +		if (status & RXDS_RXGD) {
> +			data = ether->rdesc->recv_buf[ether->cur_rx];
> +			skb = dev_alloc_skb(length+2);
> +			if (!skb) {
> +				dev_err(&pdev->dev, "get skb buffer error\n");
> +				ether->stats.rx_dropped++;
> +				return;
> +			}
> +
> +			skb->dev = dev;
> +			skb_reserve(skb, 2);
> +			skb_put(skb, length);
> +			skb_copy_to_linear_data(skb, data, length);
> +			skb->protocol = eth_type_trans(skb, dev);
> +			ether->stats.rx_packets++;
> +			ether->stats.rx_bytes += length;
> +			netif_rx(skb);
> +		} else {
> +			ether->stats.rx_errors++;
> +
> +			if (status & RXDS_RP) {
> +				dev_err(&pdev->dev, "rx runt err\n");
> +				ether->stats.rx_length_errors++;
> +			} else if (status & RXDS_CRCE) {
> +				dev_err(&pdev->dev, "rx crc err\n");
> +				ether->stats.rx_crc_errors++;
> +			} else if (status & RXDS_ALIE) {
> +				dev_err(&pdev->dev, "rx aligment err\n");
> +				ether->stats.rx_frame_errors++;
> +			} else if (status & RXDS_PTLE) {
> +				dev_err(&pdev->dev, "rx longer err\n");
> +				ether->stats.rx_over_errors++;
> +			}
> +		}
> +
> +		rxbd->sl = RX_OWEN_DMA;
> +		rxbd->reserved = 0x0;
> +		ether->cur_rx = (ether->cur_rx+1) % RX_DESC_SIZE;
> +		rxbd = &ether->rdesc->desclist[ether->cur_rx];
> +
> +		dev->last_rx = jiffies;
> +	} while (1);
> +}
...
> +static int __devexit w90p910_ether_remove(struct platform_device *pdev)
> +{
> +	struct net_device *dev = platform_get_drvdata(pdev);
> +	struct w90p910_ether *ether = netdev_priv(dev);
> +
> +	unregister_netdev(dev);
> +	clk_put(ether->rmiiclk);
> +	clk_put(ether->clk);
> +	del_timer_sync(&ether->check_timer);
> +	platform_set_drvdata(pdev, NULL);
> +	free_irq(ether->txirq, dev);
> +	free_irq(ether->rxirq, dev);

iounmap?  release_mem_region?

> +	free_netdev(dev);
> +	return 0;
> +}

^ permalink raw reply

* a problem with tap port insertion/deletion from bridge
From: Deepjyoti Kakati @ 2009-07-19  9:56 UTC (permalink / raw)
  To: netdev

am facing a issue with bridge that apparently stops fwding and
hoping someone can point out what is going wrong or give some
clues.

am using linux 2.6.27 and corresponding bridgeutils pkg.

my topology inside the machine is like this

(UDP Server daemon)
|
|
socket
|
|
[my_bridge 172.16.0.3]----tap1---Virtual machine2
|
|
tap0
|
virtual machine1
172.16.0.2

I create a few virtual machines using qemu and have their tap
interfaces hook into the bridge.

virtual machine1 internally assigns 172.16.0.2 to its virtual nic

the bridge itself has address 172.16.0.3

my server binds a UDP socket to a {fixed port, 172.16.0.3} and takes
heartbeats and messages from virtual machine1 who is UDP client
and binds to 172.16.0.2

things work fine and packets flow both ways

STP is turned off by default as per "brctl show"

trouble starts when I shut down virtual machine2 or 3 and detach the
tap1 or tap2 from the bridge. the detach sequence I used is:

ifconfig tap1 down
brctl delif br_internal tap1
ifconfig tap1 up

I would expect the bridge to keep on forwarding pkts to and from tap0
port which is still attached, however i see sometimes the bridge receives
frames from tap0 (heartbeats from virtual machine1) but my socket never
gets it and is blocked in recvfrom()

this condition lasts from 10-40 seconds before things get back to normal.

I think my socket is ok because I sent a message to it from another
thread while this condition was happening and this dummy message
instantly came to recvfrom().

tried turning on STP and setting fwd delay of 0 to no avail. also
tried explicitly setting the promiscuous flag with ifconfig.

/var/log/messages does not show tap0 going out of forwarding state. it
appears normal.

the port thats detached goes into disabled state which is expected.

if there is anything wrong with my approach or there is some known
issue with bridge forwarding during topology change, kindly let me know.

I need to be able to attach and detach taps dynamically without
affecting traffic on other ports as the VLANs used by the virtual machines
can be changed on the fly. multiple virtual machine tap's could be hooked
onto the same bridge (vlan) hence the issue.

thanks
Deepjyoti Kakati

^ permalink raw reply

* [PATCH] mlx4_en: Fix double pci unmapping.
From: Yevgeny Petrilin @ 2009-07-19  7:23 UTC (permalink / raw)
  To: David Miller; +Cc: netdev


In cases of fragmented skb, with the data pointers being wrapped around
the TX buffer, the completion handling code would not forward the data
pointer and the firs fragment was unmapped several times, while others
were not unmapped at all.

Signed-off-by: Yevgeny Petrilin <yevgenyp@mellanox.co.il>
---
 drivers/net/mlx4/en_tx.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/mlx4/en_tx.c b/drivers/net/mlx4/en_tx.c
index d5c18c6..3e8a404 100644
--- a/drivers/net/mlx4/en_tx.c
+++ b/drivers/net/mlx4/en_tx.c
@@ -249,6 +249,7 @@ static u32 mlx4_en_free_tx_desc(struct mlx4_en_priv *priv,
 				pci_unmap_page(mdev->pdev,
 					(dma_addr_t) be64_to_cpu(data->addr),
 					 frag->size, PCI_DMA_TODEVICE);
+				++data;
 			}
 		}
 		/* Stamp the freed descriptor */
-- 
1.6.0


^ permalink raw reply related

* Re: [1/1] connector maintainer/mail update.
From: Mike Frysinger @ 2009-07-19  5:38 UTC (permalink / raw)
  To: Evgeniy Polyakov; +Cc: Andrew Morton, David Miller, netdev
In-Reply-To: <8bd0f97a0907181217y81e9025mcd59c9741eb6ce96@mail.gmail.com>

On Sat, Jul 18, 2009 at 15:17, Mike Frysinger wrote:
> 2009/7/18 Evgeniy Polyakov
>> On Sat, Jul 18, 2009 at 02:51:38PM -0400, Mike Frysinger wrote:
>>> >> i dont think open-ended copyrights are acceptable.  copyrights arent
>>> >> something that extend forever which is why they need explicit updating
>>> >> (i.e. you'd want 2004-<last year you made a change>).
>>> >
>>> > It can be always replaced back if I will step aside.
>>>
>>> copyright doesnt disappear or go away if the maintainer changes.
>>> there's nothing wrong with multiple people claiming copyright on a
>>> file.
>>
>> But apparently you do not like that there is no 'finish' date.
>> And this can be changed when I step aside :)
>
> *i* dont personally give two sh*ts about copyright.  i'm only pointing
> out common accepted practice with regard to maintaining copyright
> information.  accepted practice is that open ended markings like this
> are not legally applicable (ask the SFLC).

btw, dont take my comments as a NAK.  if someone merges it anyways, i
dont really care.  i'm just giving [what i think is] some useful
feedback.
-mike

^ permalink raw reply

* Hello
From: aliyu_anita @ 2009-07-19  0:15 UTC (permalink / raw)


Hello my lovely dear friend,
Hope your enjoying the good atmosphere of the day accordingly,i know it might surprised how i got your email address,it was while surfing the net that i discovered it,then i derived interest and decided to drop few couple of mine
words to you.first and foremost, i want to use this medium express myself once again to you,my name is miss.Aliyu Aliyu,single young girl,searching for a mature man with good sense of humour ,also will appreciate each other start having a good relationship with a real love,caring, understand and honest man,please i appreciate you endeavours contact me directly in the above email address so as to enable me sending my pictures directly to your email box for further introduction,shall be very glad reading good news from you soonest and God bless as you do comply.
kiss,
miss Anita

^ permalink raw reply

* Re: [1/1] connector maintainer/mail update.
From: Mike Frysinger @ 2009-07-18 19:17 UTC (permalink / raw)
  To: Evgeniy Polyakov; +Cc: Andrew Morton, David Miller, netdev
In-Reply-To: <20090718185928.GA7474@ioremap.net>

2009/7/18 Evgeniy Polyakov
> On Sat, Jul 18, 2009 at 02:51:38PM -0400, Mike Frysinger wrote:
>> >> i dont think open-ended copyrights are acceptable.  copyrights arent
>> >> something that extend forever which is why they need explicit updating
>> >> (i.e. you'd want 2004-<last year you made a change>).
>> >
>> > It can be always replaced back if I will step aside.
>>
>> copyright doesnt disappear or go away if the maintainer changes.
>> there's nothing wrong with multiple people claiming copyright on a
>> file.
>
> But apparently you do not like that there is no 'finish' date.
> And this can be changed when I step aside :)

*i* dont personally give two sh*ts about copyright.  i'm only pointing
out common accepted practice with regard to maintaining copyright
information.  accepted practice is that open ended markings like this
are not legally applicable (ask the SFLC).
-mike

^ permalink raw reply

* Re: [1/1] connector maintainer/mail update.
From: Evgeniy Polyakov @ 2009-07-18 18:59 UTC (permalink / raw)
  To: Mike Frysinger; +Cc: Andrew Morton, David Miller, netdev
In-Reply-To: <8bd0f97a0907181151i460fbc83t41e0c7de4d5cadc@mail.gmail.com>

On Sat, Jul 18, 2009 at 02:51:38PM -0400, Mike Frysinger (vapier.adi@gmail.com) wrote:
> >> i dont think open-ended copyrights are acceptable.  copyrights arent
> >> something that extend forever which is why they need explicit updating
> >> (i.e. you'd want 2004-<last year you made a change>).
> >
> > It can be always replaced back if I will step aside.
> 
> copyright doesnt disappear or go away if the maintainer changes.
> there's nothing wrong with multiple people claiming copyright on a
> file.

But apparently you do not like that there is no 'finish' date.
And this can be changed when I step aside :)

-- 
	Evgeniy Polyakov

^ permalink raw reply

* Re: [1/1] connector maintainer/mail update.
From: Mike Frysinger @ 2009-07-18 18:51 UTC (permalink / raw)
  To: Evgeniy Polyakov; +Cc: Andrew Morton, David Miller, netdev
In-Reply-To: <20090718184329.GA7103@ioremap.net>

On Sat, Jul 18, 2009 at 14:43, Evgeniy Polyakov wrote:
> On Sat, Jul 18, 2009 at 02:35:55PM -0400, Mike Frysinger wrote:
>> On Sat, Jul 18, 2009 at 10:25, Evgeniy Polyakov wrote:
>> > - * 2004-2005 Copyright (c) Evgeniy Polyakov <johnpol@2ka.mipt.ru>
>> > + * 2004+ Copyright (c) Evgeniy Polyakov <zbr@ioremap.net>
>>
>> i dont think open-ended copyrights are acceptable.  copyrights arent
>> something that extend forever which is why they need explicit updating
>> (i.e. you'd want 2004-<last year you made a change>).
>
> It can be always replaced back if I will step aside.

copyright doesnt disappear or go away if the maintainer changes.
there's nothing wrong with multiple people claiming copyright on a
file.
-mike

^ permalink raw reply


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