Netdev List
 help / color / mirror / Atom feed
* [2.6 patch] drivers/net/irda/mcs7780.c: make struct mcs_driver static
From: Adrian Bunk @ 2006-06-21 23:20 UTC (permalink / raw)
  To: Samuel Ortiz; +Cc: netdev, linux-kernel

This patch makes a needlessly global struct static.

Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- linux-2.6.17-mm1-full/drivers/net/irda/mcs7780.c.old	2006-06-22 00:38:41.000000000 +0200
+++ linux-2.6.17-mm1-full/drivers/net/irda/mcs7780.c	2006-06-22 00:38:50.000000000 +0200
@@ -101,7 +101,7 @@
 module_param(transceiver_type, int, 0444);
 MODULE_PARM_DESC(transceiver_type, "IR transceiver type, see mcs7780.h.");
 
-struct usb_driver mcs_driver = {
+static struct usb_driver mcs_driver = {
 	.name = "mcs7780",
 	.probe = mcs_probe,
 	.disconnect = mcs_disconnect,


^ permalink raw reply

* Re: [1/5] [NET]: Merge TSO/UFO fields in sk_buff
From: Herbert Xu @ 2006-06-21 23:27 UTC (permalink / raw)
  To: Michael Chan; +Cc: David S. Miller, netdev
In-Reply-To: <1150926495.3475.5.camel@rh4>

Hi Michael:

On Wed, Jun 21, 2006 at 02:48:15PM -0700, Michael Chan wrote:
> 
> We have some hardware that supports TSO and ECN.  Is something like the
> patch below what you had in mind to support NETIF_F_TSO_ECN?  Or are you
> thinking about something more generic that works with or without
> hardware support?

Yeah I was thinking of something more generic because packets with CWR
set should be rare.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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

* [PATCH] Export accept queue len of a TCP listening socket via /proc/net/tcp{6}
From: Sridhar Samudrala @ 2006-06-21 23:56 UTC (permalink / raw)
  To: davem; +Cc: netdev

While debugging a TCP server hang issue, we noticed that currently there is 
no way for a user to find the acceptq backlog value for a TCP listen socket.

All the standard networking utilities that display socket info like netstat,
ss and /proc/net/tcp have 2 fields called rx_queue and tx_queue. I think these
fields do not mean much for listening sockets and they are always zero. So why
not use one of these fields(rx_queue) to export the accept queue len for listening
sockets?
The attached patch implements this, but it only works with /proc/net/tcp and
/proc/net/tcp6.

In order to support this with netstat/ss that use netlink mechanism to
get the socket info, i think we need to extend tcp_info to add this field.
Can this be done in a backward compatible way?

Anyway, i think it is useful to have this value exported via /proc.

Thanks
Sridhar


Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 25ecc6e..4c6ef47 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1726,7 +1726,8 @@ static void get_tcp4_sock(struct sock *s
 	sprintf(tmpbuf, "%4d: %08X:%04X %08X:%04X %02X %08X:%08X %02X:%08lX "
 			"%08X %5d %8d %lu %d %p %u %u %u %u %d",
 		i, src, srcp, dest, destp, sp->sk_state,
-		tp->write_seq - tp->snd_una, tp->rcv_nxt - tp->copied_seq,
+		tp->write_seq - tp->snd_una,
+		(sp->sk_state == TCP_LISTEN) ? sp->sk_ack_backlog : (tp->rcv_nxt - tp->copied_seq),
 		timer_active,
 		jiffies_to_clock_t(timer_expires - jiffies),
 		icsk->icsk_retransmits,
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index a50eb30..b36d5b2 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1469,7 +1469,8 @@ static void get_tcp6_sock(struct seq_fil
 		   dest->s6_addr32[0], dest->s6_addr32[1],
 		   dest->s6_addr32[2], dest->s6_addr32[3], destp,
 		   sp->sk_state, 
-		   tp->write_seq-tp->snd_una, tp->rcv_nxt-tp->copied_seq,
+		   tp->write_seq-tp->snd_una,
+		   (sp->sk_state == TCP_LISTEN) ? sp->sk_ack_backlog : (tp->rcv_nxt - tp->copied_seq),
 		   timer_active,
 		   jiffies_to_clock_t(timer_expires - jiffies),
 		   icsk->icsk_retransmits,



^ permalink raw reply related

* Re: [PATCH] Export accept queue len of a TCP listening socket via /proc/net/tcp{6}
From: Herbert Xu @ 2006-06-22  0:21 UTC (permalink / raw)
  To: Sridhar Samudrala; +Cc: davem, netdev
In-Reply-To: <1150934207.2868.24.camel@w-sridhar2.beaverton.ibm.com>

Sridhar Samudrala <sri@us.ibm.com> wrote:
> 
> In order to support this with netstat/ss that use netlink mechanism to
> get the socket info, i think we need to extend tcp_info to add this field.
> Can this be done in a backward compatible way?

What about using the same fields (rqueue/wqueue) as you did for /proc?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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: ipv6 source address selection in addrconf.c (2.6.17)
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2006-06-22  0:26 UTC (permalink / raw)
  To: stlman; +Cc: netdev, yoshfuji
In-Reply-To: <4499CEF4.2050308@poczta.fm>

In article <4499CEF4.2050308@poczta.fm> (at Thu, 22 Jun 2006 00:57:56 +0200), Lukasz Stelmach <stlman@poczta.fm> says:

> Lukasz Stelmach wrote:
> > Lukasz Stelmach wrote:
> > 
> >> [...] when trying to connect to
> >>
> >> 2001:200:0:8002:203:47ff:fea5:3085 (www.kame.net)
> >>
> >> with two global addresses assigned to the ethernet card
> >>
> >> fd24:6f44:46bd:face::254
> >> 2002:531f:d667:face::254
> >>
> >> rule 8 does not work and the first address is chosen.
> > 
> > The answer is that fc00::/7 matches 2001:: better because it gets the same
> > label (ipv6_saddr_label()). Although fc00::/7 addresses are defined as global
> > unicast IMHO they should be treated *slightly* different. This is the patch.
> > Since 6to4 has its own label I have decided to assign one to Teredo too.
> 
> There still, however, remains one issue. Aditional labels prevent kernel from
> selecting fc00::/7 prematurly. But there is no way to stop it from selecting
> it in rule 7. A wrong assumption has been taken that there is only one
> "private" address per interface and it is always the best choice. If there are
> four addresses on the interface:
> 
> fd24:6f44:46bd:face:EUI64 fd24:6f44:46bd:face:RANDOM
> and
> 2002:531f:d667:face:EUI64 2002:531f:d667:face::RANDOM
> 
> there seem to be no way to prefere 2002:: over fc00:: in rule 7 and it will be
> selected as long as it is before 2002:: on the list. I can see here that an
> implicit assumption has been made that an interface either is multihomed or
> "private". The seventh rule should not IMHO break the whole process of
> selection but rather mark as selectable all "private" (random) addresses. And
> it should rather be done before rule 6.

Hmm? We do not have such intention.
In above case, when you connect to 2001:200:0:8002:203:47ff:fea5:3085,
either 2002:531f:d667:face:EUI64 or 2002:531f:d667:face::RANDOM
should be selected (depending on if use_tempaddr >= 2),
by the longest matching rule (Rule 8).

--yoshfuji

^ permalink raw reply

* Re: [PATCH] Export accept queue len of a TCP listening socket via /proc/net/tcp{6}
From: Sridhar Samudrala @ 2006-06-22  0:35 UTC (permalink / raw)
  To: Herbert Xu; +Cc: davem, netdev
In-Reply-To: <E1FtCws-0005in-00@gondolin.me.apana.org.au>

On Thu, 2006-06-22 at 10:21 +1000, Herbert Xu wrote:
> Sridhar Samudrala <sri@us.ibm.com> wrote:
> > 
> > In order to support this with netstat/ss that use netlink mechanism to
> > get the socket info, i think we need to extend tcp_info to add this field.
> > Can this be done in a backward compatible way?
> 
> What about using the same fields (rqueue/wqueue) as you did for /proc?

I meant extending tcp_info structure to add new fields. I think the user
space also uses this structure.

Thanks
Sridhar


^ permalink raw reply

* Re: PATCHv3 2.6.17-rc5 tulip free_irq() called too late
From: Valerie Henson @ 2006-06-22  0:43 UTC (permalink / raw)
  To: Grant Grundler; +Cc: Jeff Garzik, Andrew Morton, netdev
In-Reply-To: <20060613235531.GA4191@colo.lackof.org>

On Tue, Jun 13, 2006 at 05:55:31PM -0600, Grant Grundler wrote:
> On Thu, Jun 08, 2006 at 11:01:20AM -0600, Grant Grundler wrote:
> > Here is a new patch that moves free_irq() into tulip_down().
> > The resulting code is structured the same as cp_close().
> 
> Val,
> Two details are wrong in version 2 and are fixed in v3 (appended below):
> 
> o we don't need synchronize_irq() before calling free_irq().
>   (It should be removed from cp_close() too)
>   Thanks to willy for pointing me at kernel/irq/manage.c.
> 
> o tulip_stop_rxtx() has to be called _after_ free_irq().
>   ie. v2 patch didn't fix the original race condition
>   and when under test, dies about as fast as the original code.
> 
> Tested on rx4640 (HP IA64) for several hours.
> Please apply.

Hi folks,

The quick summary of my thoughts on this patch is that it isn't the
ideal patch, but it works and it's well-tested.  Doing my preferred
fix (adding a shutdown flag) would be invasive and take many weeks to
reach the level of stability of Grant's patch.  So I'm taking this
patch but adding a comment describing my preferred fix.

Here's the long version.  The obvious ordering of bringing up the card
is:

request_irq()
setup DMA resources
enable interrupts
start DMA engine

And the obvious ordering of shutting it down is:

stop DMA engine
disable interrupts
remove DMA resources
free_irq()

The problem with the above shutdown order is that we can receive an
interrupt in between stopping DMA and disabling interrupts, and the
tulip irq handler will reenable DMA =><= Boom!  The solution I prefer
is to make the irq handler aware of whether we are shutting down or
not, and not reenable DMA in that case.  However, it is a non-trivial
patch to get right, and I would rather have the bug fixed short-term
with the ordering Grant uses:

disable interrupts
free_irq()
stop rxtx
remove DMA resources

Make sense to everyone?  I'll redo the patch with the comment and get
Grant's sign-off.

-VAL

^ permalink raw reply

* Re: [PATCH] Export accept queue len of a TCP listening socket via /proc/net/tcp{6}
From: Herbert Xu @ 2006-06-22  0:50 UTC (permalink / raw)
  To: Sridhar Samudrala; +Cc: herbert, davem, netdev
In-Reply-To: <1150936509.2868.30.camel@w-sridhar2.beaverton.ibm.com>

Sridhar Samudrala <sri@us.ibm.com> wrote:
>> 
>> What about using the same fields (rqueue/wqueue) as you did for /proc?
> 
> I meant extending tcp_info structure to add new fields. I think the user
> space also uses this structure.

What about putting it into inet_idiag_msg.idiag_[rw]queue instead?

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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: Memory corruption in 8390.c ? (was Re: Possible leaks in network drivers)
From: Herbert Xu @ 2006-06-22  0:55 UTC (permalink / raw)
  To: Alan Cox; +Cc: snakebyte, linux-kernel, jgarzik, netdev, davem
In-Reply-To: <1150909982.15275.100.camel@localhost.localdomain>

Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> 
> skb_padto() returns either a new buffer or the existing one depending
> upon the space situation. If it returns a new buffer then it frees the
> old one.

I think skb_padto simply shouldn't allocate a new skb.  It only needs
to extend the data area.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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: [1/5] [NET]: Merge TSO/UFO fields in sk_buff
From: Michael Chan @ 2006-06-22  0:46 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David S. Miller, netdev
In-Reply-To: <20060621232741.GA21559@gondor.apana.org.au>

On Thu, 2006-06-22 at 09:27 +1000, Herbert Xu wrote:
> Hi Michael:
> 
> On Wed, Jun 21, 2006 at 02:48:15PM -0700, Michael Chan wrote:
> > 
> > We have some hardware that supports TSO and ECN.  Is something like the
> > patch below what you had in mind to support NETIF_F_TSO_ECN?  Or are you
> > thinking about something more generic that works with or without
> > hardware support?
> 
> Yeah I was thinking of something more generic because packets with CWR
> set should be rare.
> 
OK, if time permits, I'll cook up some patches to support generic TSO
ECN with or without hardware support.  Without hardware ECN, it will use
GSO to split up the packet with CWR.  Can we assume that all hardware
will handle ECE properly?


^ permalink raw reply

* Re: [1/5] [NET]: Merge TSO/UFO fields in sk_buff
From: Herbert Xu @ 2006-06-22  1:09 UTC (permalink / raw)
  To: Michael Chan; +Cc: David S. Miller, netdev
In-Reply-To: <1150937184.4069.4.camel@rh4>

On Wed, Jun 21, 2006 at 05:46:24PM -0700, Michael Chan wrote:
>
> OK, if time permits, I'll cook up some patches to support generic TSO
> ECN with or without hardware support.  Without hardware ECN, it will use
> GSO to split up the packet with CWR.  Can we assume that all hardware
> will handle ECE properly?

ECE just needs to be replicated so it would seem to be a safe bet unless
Dave knows some really broken hardware out there? If not I'd say that
we should just assume that it works and add a new bit it if said broken
stuff does turn up.

Thanks a lot for looking into this!

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <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: [1/5] [NET]: Merge TSO/UFO fields in sk_buff
From: David Miller @ 2006-06-22  1:14 UTC (permalink / raw)
  To: herbert; +Cc: mchan, netdev
In-Reply-To: <20060622010925.GA22448@gondor.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 22 Jun 2006 11:09:25 +1000

> ECE just needs to be replicated so it would seem to be a safe bet unless
> Dave knows some really broken hardware out there? If not I'd say that
> we should just assume that it works and add a new bit it if said broken
> stuff does turn up.

ECE simply needs to persist while the ECE condition is true.
If it is true when we build the TSO frame, it would have
thus been true during the time in which we had built each
individual sub-frame.

I don't anticipate any problems if you just mirror the ECE
bit in each chopped up frame.

> Thanks a lot for looking into this!

Yes, indeed, thanks Michael.

^ permalink raw reply

* Re: [PATCH 1/3] Rough VJ Channel Implementation - vj_core.patch
From: Kelly Daly @ 2006-06-22  2:05 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, rusty
In-Reply-To: <20060515.221637.90861876.davem@davemloft.net>

> The hash table bits look good, just as they did last time :-)
> So I'll put this part into my vj-2.6 tree now, thanks.
Rockin' - thanks...

Sorry for the massive delay - here's the next attempt.

-------

diff -urp davem/include/linux/netchannel.h kelly_new/include/linux/netchannel.h
--- davem/include/linux/netchannel.h	2006-06-16 15:14:15.000000000 +1000
+++ kelly_new/include/linux/netchannel.h	2006-06-22 11:47:04.000000000 +1000
@@ -19,6 +19,7 @@ struct netchannel {
 	void 			(*netchan_callb)(struct netchannel *);
 	void			*netchan_callb_data;
 	unsigned long		netchan_head;
+	wait_queue_head_t	wq;
 };
 
 extern void netchannel_init(struct netchannel *,
@@ -56,6 +57,11 @@ static inline unsigned char *netchan_buf
 	return netchan_buf_base(bp) + bp->netchan_buf_offset;
 }
 
+static inline int netchan_data_len(const struct netchannel_buftrailer *bp)
+{
+	return bp->netchan_buf_len - bp->netchan_buf_offset;
+}
+
 extern int netchannel_enqueue(struct netchannel *, struct netchannel_buftrailer *);
 extern struct netchannel_buftrailer *__netchannel_dequeue(struct netchannel *);
 static inline struct netchannel_buftrailer *netchannel_dequeue(struct netchannel *np)
@@ -65,6 +71,7 @@ static inline struct netchannel_buftrail
 	return __netchannel_dequeue(np);
 }
 
+extern struct netchannel *find_netchannel(const struct netchannel_buftrailer *bp);
 extern struct sk_buff *skb_netchan_graft(struct netchannel_buftrailer *, gfp_t);
 
 #endif /* _LINUX_NETCHANNEL_H */
diff -urp davem/include/net/inet_hashtables.h kelly_new/include/net/inet_hashtables.h
--- davem/include/net/inet_hashtables.h	2006-06-16 14:34:20.000000000 +1000
+++ kelly_new/include/net/inet_hashtables.h	2006-06-19 10:42:45.000000000 +1000
@@ -418,4 +418,7 @@ static inline struct sock *inet_lookup(s
 
 extern int inet_hash_connect(struct inet_timewait_death_row *death_row,
 			     struct sock *sk);
+extern void  inet_hash_register(u8 proto, struct inet_hashinfo *hashinfo);
+extern struct sock *inet_lookup_proto(u8 protocol, u32 saddr, u16 sport, u32 daddr, u16 dport, int ifindex);
+
 #endif /* _INET_HASHTABLES_H */
diff -urp davem/include/net/sock.h kelly_new/include/net/sock.h
--- davem/include/net/sock.h	2006-06-16 15:14:16.000000000 +1000
+++ kelly_new/include/net/sock.h	2006-06-19 10:42:45.000000000 +1000
@@ -196,6 +196,7 @@ struct sock {
 	unsigned short		sk_type;
 	int			sk_rcvbuf;
 	socket_lock_t		sk_lock;
+	struct netchannel	*sk_channel;
 	wait_queue_head_t	*sk_sleep;
 	struct dst_entry	*sk_dst_cache;
 	struct xfrm_policy	*sk_policy[2];
diff -urp davem/net/core/dev.c kelly_new/net/core/dev.c
--- davem/net/core/dev.c	2006-06-16 15:14:16.000000000 +1000
+++ kelly_new/net/core/dev.c	2006-06-22 11:45:55.000000000 +1000
@@ -113,9 +113,12 @@
 #include <linux/delay.h>
 #include <linux/wireless.h>
 #include <linux/netchannel.h>
+#include <linux/kthread.h>
+#include <linux/wait.h>
 #include <net/iw_handler.h>
 #include <asm/current.h>
 #include <linux/audit.h>
+#include <net/inet_hashtables.h>
 
 /*
  *	The list of packet types we will receive (as opposed to discard)
@@ -190,6 +193,8 @@ static inline struct hlist_head *dev_ind
 	return &dev_index_head[ifindex & ((1<<NETDEV_HASHBITS)-1)];
 }
 
+static struct netchannel default_netchannel;
+
 /*
  *	Our notifier list
  */
@@ -1854,11 +1859,18 @@ softnet_break:
 	goto out;
 }
 
+void netchannel_wake(struct netchannel *np)
+{
+	wake_up(&np->wq);
+}
+
 void netchannel_init(struct netchannel *np,
 		     void (*callb)(struct netchannel *), void *callb_data)
 {
 	memset(np, 0, sizeof(*np));
 
+	init_waitqueue_head(&np->wq);
+
 	np->netchan_callb	= callb;
 	np->netchan_callb_data	= callb_data;
 }
@@ -1912,6 +1924,76 @@ struct netchannel_buftrailer *__netchann
 }
 EXPORT_SYMBOL_GPL(__netchannel_dequeue);
 
+/* Find the channel for a packet, or return default channel. */
+struct netchannel *find_netchannel(const struct netchannel_buftrailer *bp)
+{
+	struct sock *sk = NULL;
+	int datalen = netchan_data_len(bp);
+
+	switch (bp->netchan_buf_proto) {
+	case __constant_htons(ETH_P_IP): {
+		struct iphdr *ip = (void *)bp - datalen;
+		int iphl = ip->ihl * 4;
+
+		/* FIXME: Do sanity checks, parse packet. */
+
+		if (datalen >+ (iphl + 4) && iphl == sizeof(struct iphdr)) {
+			u16 *ports = (u16 *)ip + 1;
+			sk = inet_lookup_proto(ip->protocol, 
+					 ip->saddr, ports[0],
+					 ip->daddr, ports[1],
+					 bp->netchan_buf_dev->ifindex);
+		}
+		break;
+	}
+	}
+
+	if (sk && sk->sk_channel)
+		return sk->sk_channel;
+	return &default_netchannel;
+}
+EXPORT_SYMBOL_GPL(find_netchannel);
+
+static int sock_add_netchannel(struct sock *sk)
+{
+	struct netchannel *np;
+
+	np = kmalloc(sizeof(struct netchannel), GFP_KERNEL);
+	if (!np)
+		return -ENOMEM;
+	netchannel_init(np, netchannel_wake, (void *)np);
+	sk->sk_channel = np;
+
+	return 0;
+}
+
+/* deal with packets coming to default thread */
+static int netchannel_default_thread(void *unused)
+{
+	struct netchannel *np = &default_netchannel;
+	struct netchannel_buftrailer *nbp;
+	struct sk_buff *skbp;
+	DECLARE_WAITQUEUE(wait, current);
+
+	add_wait_queue(&np->wq, &wait);
+	set_current_state(TASK_UNINTERRUPTIBLE);
+
+	while (!kthread_should_stop()) {
+		while (np->netchan_tail != np->netchan_head) {
+			nbp = netchannel_dequeue(np);
+			skbp = skb_netchan_graft(nbp, GFP_KERNEL);
+			netif_receive_skb(skbp);
+		}
+		schedule();
+		set_current_state(TASK_INTERRUPTIBLE);
+	}
+	
+	remove_wait_queue(&np->wq, &wait);
+	__set_current_state(TASK_RUNNING);
+
+	return 0;
+}
+
 static gifconf_func_t * gifconf_list [NPROTO];
 
 /**
@@ -3426,6 +3508,10 @@ static int __init net_dev_init(void)
 	hotcpu_notifier(dev_cpu_callback, 0);
 	dst_init();
 	dev_mcast_init();
+
+	netchannel_init(&default_netchannel, netchannel_wake, (void *)&default_netchannel);
+	kthread_run(netchannel_default_thread, NULL, "nc_def");
+
 	rc = 0;
 out:
 	return rc;

^ permalink raw reply

* New and hot Do you have life experience?
From: Johanna @ 2006-06-22  2:25 UTC (permalink / raw)
  To: netdev

Degree - higher pay

Fas t Trac k De gree Progr am

Obtain the deg ree you deserve, based on your present knowledge and life
experience. A prosperous future, money earning power, and the Admiration of all.

Degr ees from an Established, Prestig ious, Leading Institution.
Your deg ree will show exactly what you really can do.

Get the Job, Promotion, Business Opportunity and Social Advancement you Desire!
Eliminates classrooms and traveling.

Achieve your Ba chelors, M asters, M BA, or Ph D
in the field of your expertise.

Professional and affordable! Call now - your Graduat ion is a phone call away.

Please call:
 1-215-689-7 379 
Calls returned promptly



See Naples and die Life is like a good book, the more you get into it the more it makes sense Those who play the game do not see it as clearly as those who watch A clear conscience is usually the sign of a bad memory.  The early worm enjoys a leisurely breakfast  The company makes the feast A gossip betrays a confidence, but a trustworthy man keeps a secret.






^ permalink raw reply

* Re: Memory corruption in 8390.c ? (was Re: Possible leaks in network drivers)
From: Herbert Xu @ 2006-06-22  2:30 UTC (permalink / raw)
  To: Alan Cox; +Cc: snakebyte, linux-kernel, jgarzik, netdev, davem
In-Reply-To: <E1FtDU0-0005nd-00@gondolin.me.apana.org.au>

On Thu, Jun 22, 2006 at 10:55:44AM +1000, Herbert Xu wrote:
> 
> I think skb_padto simply shouldn't allocate a new skb.  It only needs
> to extend the data area.

OK, here is a patch to make it do that.

[NET]: Avoid allocating skb in skb_pad

First of all it is unnecessary to allocate a new skb in skb_pad since
the existing one is not shared.  More importantly, our hard_start_xmit
interface does not allow a new skb to be allocated since that breaks
requeueing.

This patch uses pskb_expand_head to expand the existing skb and linearize
it if needed.  Actually, someone should sift through every instance of
skb_pad on a non-linear skb as they do not fit the reasons why this was
originally created.

Incidentally, this fixes a minor bug when the skb is cloned (tcpdump,
TCP, etc.).  As it is skb_pad will simply write over a cloned skb.  Because
of the position of the write it is unlikely to cause problems but still
it's best if we don't do it.

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

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/drivers/net/3c527.c b/drivers/net/3c527.c
index 1b1cb00..157eda5 100644
--- a/drivers/net/3c527.c
+++ b/drivers/net/3c527.c
@@ -1031,8 +1031,7 @@ static int mc32_send_packet(struct sk_bu
 		return 1;
 	}
 
-	skb = skb_padto(skb, ETH_ZLEN);
-	if (skb == NULL) {
+	if (skb_padto(skb, ETH_ZLEN)) {
 		netif_wake_queue(dev);
 		return 0;
 	}
diff --git a/drivers/net/82596.c b/drivers/net/82596.c
index da0c878..8a9f7d6 100644
--- a/drivers/net/82596.c
+++ b/drivers/net/82596.c
@@ -1070,8 +1070,7 @@ static int i596_start_xmit(struct sk_buf
 				skb->len, (unsigned int)skb->data));
 
 	if (skb->len < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
+		if (skb_padto(skb, ETH_ZLEN))
 			return 0;
 		length = ETH_ZLEN;
 	}
diff --git a/drivers/net/8390.c b/drivers/net/8390.c
index f870274..00abe83 100644
--- a/drivers/net/8390.c
+++ b/drivers/net/8390.c
@@ -277,8 +277,7 @@ static int ei_start_xmit(struct sk_buff 
 	unsigned long flags;
 
 	if (skb->len < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
+		if (skb_padto(skb, ETH_ZLEN))
 			return 0;
 		send_length = ETH_ZLEN;
 	}
diff --git a/drivers/net/a2065.c b/drivers/net/a2065.c
index 79bb56b..71165ac 100644
--- a/drivers/net/a2065.c
+++ b/drivers/net/a2065.c
@@ -573,8 +573,7 @@ static int lance_start_xmit (struct sk_b
 	
 	if (len < ETH_ZLEN) {
 		len = ETH_ZLEN;
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
+		if (skb_padto(skb, ETH_ZLEN))
 			return 0;
 	}
 
diff --git a/drivers/net/ariadne.c b/drivers/net/ariadne.c
index d1b6b1f..a9bb7a4 100644
--- a/drivers/net/ariadne.c
+++ b/drivers/net/ariadne.c
@@ -607,8 +607,7 @@ #endif
     /* FIXME: is the 79C960 new enough to do its own padding right ? */
     if (skb->len < ETH_ZLEN)
     {
-    	skb = skb_padto(skb, ETH_ZLEN);
-    	if (skb == NULL)
+    	if (skb_padto(skb, ETH_ZLEN))
     	    return 0;
     	len = ETH_ZLEN;
     }
diff --git a/drivers/net/arm/ether1.c b/drivers/net/arm/ether1.c
index 36475eb..312955d 100644
--- a/drivers/net/arm/ether1.c
+++ b/drivers/net/arm/ether1.c
@@ -700,8 +700,7 @@ ether1_sendpacket (struct sk_buff *skb, 
 	}
 
 	if (skb->len < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
+		if (skb_padto(skb, ETH_ZLEN))
 			goto out;
 	}
 
diff --git a/drivers/net/arm/ether3.c b/drivers/net/arm/ether3.c
index f1d5b10..0810741 100644
--- a/drivers/net/arm/ether3.c
+++ b/drivers/net/arm/ether3.c
@@ -518,8 +518,7 @@ ether3_sendpacket(struct sk_buff *skb, s
 
 	length = (length + 1) & ~1;
 	if (length != skb->len) {
-		skb = skb_padto(skb, length);
-		if (skb == NULL)
+		if (skb_padto(skb, length))
 			goto out;
 	}
 
diff --git a/drivers/net/atarilance.c b/drivers/net/atarilance.c
index 442b2cb..91783a8 100644
--- a/drivers/net/atarilance.c
+++ b/drivers/net/atarilance.c
@@ -804,8 +804,7 @@ static int lance_start_xmit( struct sk_b
 		++len;
 		
 	if (len > skb->len) {
-		skb = skb_padto(skb, len);
-		if (skb == NULL)
+		if (skb_padto(skb, len))
 			return 0;
 	}
 		
diff --git a/drivers/net/cassini.c b/drivers/net/cassini.c
index 39f36aa..565a54f 100644
--- a/drivers/net/cassini.c
+++ b/drivers/net/cassini.c
@@ -2915,8 +2915,7 @@ static int cas_start_xmit(struct sk_buff
 	 */
 	static int ring; 
 
-	skb = skb_padto(skb, cp->min_frame_size);
-	if (!skb)
+	if (skb_padto(skb, cp->min_frame_size))
 		return 0;
 
 	/* XXX: we need some higher-level QoS hooks to steer packets to
diff --git a/drivers/net/declance.c b/drivers/net/declance.c
index f130bda..d3d958e 100644
--- a/drivers/net/declance.c
+++ b/drivers/net/declance.c
@@ -885,8 +885,7 @@ static int lance_start_xmit(struct sk_bu
 	len = skblen;
 	
 	if (len < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
+		if (skb_padto(skb, ETH_ZLEN))
 			return 0;
 		len = ETH_ZLEN;
 	}
diff --git a/drivers/net/depca.c b/drivers/net/depca.c
index 0941d40..e946c43 100644
--- a/drivers/net/depca.c
+++ b/drivers/net/depca.c
@@ -938,11 +938,8 @@ static int depca_start_xmit(struct sk_bu
 	if (skb->len < 1)
 		goto out;
 
-	if (skb->len < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
-			goto out;
-	}
+	if (skb_padto(skb, ETH_ZLEN))
+		goto out;
 	
 	netif_stop_queue(dev);
 
diff --git a/drivers/net/eepro.c b/drivers/net/eepro.c
index a806dfe..e70f172 100644
--- a/drivers/net/eepro.c
+++ b/drivers/net/eepro.c
@@ -1154,8 +1154,7 @@ static int eepro_send_packet(struct sk_b
 		printk(KERN_DEBUG  "%s: entering eepro_send_packet routine.\n", dev->name);
 
 	if (length < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
+		if (skb_padto(skb, ETH_ZLEN))
 			return 0;
 		length = ETH_ZLEN;
 	}
diff --git a/drivers/net/eexpress.c b/drivers/net/eexpress.c
index 82bd356..a74b207 100644
--- a/drivers/net/eexpress.c
+++ b/drivers/net/eexpress.c
@@ -677,8 +677,7 @@ #if NET_DEBUG > 6
 #endif
 
 	if (buf->len < ETH_ZLEN) {
-		buf = skb_padto(buf, ETH_ZLEN);
-		if (buf == NULL)
+		if (skb_padto(buf, ETH_ZLEN))
 			return 0;
 		length = ETH_ZLEN;
 	}
diff --git a/drivers/net/epic100.c b/drivers/net/epic100.c
index 8d680ce..724d7dc 100644
--- a/drivers/net/epic100.c
+++ b/drivers/net/epic100.c
@@ -1027,11 +1027,8 @@ static int epic_start_xmit(struct sk_buf
 	u32 ctrl_word;
 	unsigned long flags;
 
-	if (skb->len < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
-			return 0;
-	}
+	if (skb_padto(skb, ETH_ZLEN))
+		return 0;
 
 	/* Caution: the write order is important here, set the field with the
 	   "ownership" bit last. */
diff --git a/drivers/net/eth16i.c b/drivers/net/eth16i.c
index b67545b..4bf76f8 100644
--- a/drivers/net/eth16i.c
+++ b/drivers/net/eth16i.c
@@ -1064,8 +1064,7 @@ static int eth16i_tx(struct sk_buff *skb
 	unsigned long flags;
 
 	if (length < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
+		if (skb_padto(skb, ETH_ZLEN))
 			return 0;
 		length = ETH_ZLEN;
 	}
diff --git a/drivers/net/hp100.c b/drivers/net/hp100.c
index 247c8ca..dd1dc32 100644
--- a/drivers/net/hp100.c
+++ b/drivers/net/hp100.c
@@ -1487,11 +1487,8 @@ #endif
 	if (skb->len <= 0)
 		return 0;
 		
-	if (skb->len < ETH_ZLEN && lp->chip == HP100_CHIPID_SHASTA) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
-			return 0;
-	}
+	if (lp->chip == HP100_CHIPID_SHASTA && skb_padto(skb, ETH_ZLEN))
+		return 0;
 
 	/* Get Tx ring tail pointer */
 	if (lp->txrtail->next == lp->txrhead) {
diff --git a/drivers/net/lance.c b/drivers/net/lance.c
index bb5ad47..c1c3452 100644
--- a/drivers/net/lance.c
+++ b/drivers/net/lance.c
@@ -968,8 +968,7 @@ static int lance_start_xmit(struct sk_bu
 	/* The old LANCE chips doesn't automatically pad buffers to min. size. */
 	if (chip_table[lp->chip_version].flags & LANCE_MUST_PAD) {
 		if (skb->len < ETH_ZLEN) {
-			skb = skb_padto(skb, ETH_ZLEN);
-			if (skb == NULL)
+			if (skb_padto(skb, ETH_ZLEN))
 				goto out;
 			lp->tx_ring[entry].length = -ETH_ZLEN;
 		}
diff --git a/drivers/net/lasi_82596.c b/drivers/net/lasi_82596.c
index 957888d..1ab0944 100644
--- a/drivers/net/lasi_82596.c
+++ b/drivers/net/lasi_82596.c
@@ -1083,8 +1083,7 @@ static int i596_start_xmit(struct sk_buf
 				skb->len, skb->data));
 
 	if (length < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
+		if (skb_padto(skb, ETH_ZLEN))
 			return 0;
 		length = ETH_ZLEN;
 	}
diff --git a/drivers/net/lp486e.c b/drivers/net/lp486e.c
index 94d5ea1..bf3f343 100644
--- a/drivers/net/lp486e.c
+++ b/drivers/net/lp486e.c
@@ -877,8 +877,7 @@ static int i596_start_xmit (struct sk_bu
 	length = skb->len;
 	
 	if (length < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
+		if (skb_padto(skb, ETH_ZLEN))
 			return 0;
 		length = ETH_ZLEN;
 	}
diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index e1feb58..b245476 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -1939,8 +1939,7 @@ #endif				/*NETIF_F_TSO */
 
 		/* pad frames to at least ETH_ZLEN bytes */
 		if (unlikely(skb->len < ETH_ZLEN)) {
-			skb = skb_padto(skb, ETH_ZLEN);
-			if (skb == NULL) {
+			if (skb_padto(skb, ETH_ZLEN)) {
 				/* The packet is gone, so we must
 				 * return 0 */
 				mgp->stats.tx_dropped += 1;
diff --git a/drivers/net/pcmcia/fmvj18x_cs.c b/drivers/net/pcmcia/fmvj18x_cs.c
index 09b1176..ea93b8f 100644
--- a/drivers/net/pcmcia/fmvj18x_cs.c
+++ b/drivers/net/pcmcia/fmvj18x_cs.c
@@ -831,8 +831,7 @@ static int fjn_start_xmit(struct sk_buff
     
     if (length < ETH_ZLEN)
     {
-    	skb = skb_padto(skb, ETH_ZLEN);
-    	if (skb == NULL)
+    	if (skb_padto(skb, ETH_ZLEN))
     		return 0;
     	length = ETH_ZLEN;
     }
diff --git a/drivers/net/pcmcia/xirc2ps_cs.c b/drivers/net/pcmcia/xirc2ps_cs.c
index 71f4505..54bbfda 100644
--- a/drivers/net/pcmcia/xirc2ps_cs.c
+++ b/drivers/net/pcmcia/xirc2ps_cs.c
@@ -1374,8 +1374,7 @@ do_start_xmit(struct sk_buff *skb, struc
      */
     if (pktlen < ETH_ZLEN)
     {
-        skb = skb_padto(skb, ETH_ZLEN);
-        if (skb == NULL)
+        if (skb_padto(skb, ETH_ZLEN))
         	return 0;
 	pktlen = ETH_ZLEN;
     }
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 9945cc6..985afe0 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -2222,8 +2222,7 @@ static int rtl8169_start_xmit(struct sk_
 		len = skb->len;
 
 		if (unlikely(len < ETH_ZLEN)) {
-			skb = skb_padto(skb, ETH_ZLEN);
-			if (!skb)
+			if (skb_padto(skb, ETH_ZLEN))
 				goto err_update_stats;
 			len = ETH_ZLEN;
 		}
diff --git a/drivers/net/seeq8005.c b/drivers/net/seeq8005.c
index bcef03f..efd0f23 100644
--- a/drivers/net/seeq8005.c
+++ b/drivers/net/seeq8005.c
@@ -396,8 +396,7 @@ static int seeq8005_send_packet(struct s
 	unsigned char *buf;
 
 	if (length < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
+		if (skb_padto(skb, ETH_ZLEN))
 			return 0;
 		length = ETH_ZLEN;
 	}
diff --git a/drivers/net/sis190.c b/drivers/net/sis190.c
index 31dd3f0..df39f34 100644
--- a/drivers/net/sis190.c
+++ b/drivers/net/sis190.c
@@ -1156,8 +1156,7 @@ static int sis190_start_xmit(struct sk_b
 	dma_addr_t mapping;
 
 	if (unlikely(skb->len < ETH_ZLEN)) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (!skb) {
+		if (skb_padto(skb, ETH_ZLEN)) {
 			tp->stats.tx_dropped++;
 			goto out;
 		}
diff --git a/drivers/net/sk98lin/skge.c b/drivers/net/sk98lin/skge.c
index 38a26df..f3efbd1 100644
--- a/drivers/net/sk98lin/skge.c
+++ b/drivers/net/sk98lin/skge.c
@@ -1525,7 +1525,7 @@ #endif
 	** This is to resolve faulty padding by the HW with 0xaa bytes.
 	*/
 	if (BytesSend < C_LEN_ETHERNET_MINSIZE) {
-		if ((pMessage = skb_padto(pMessage, C_LEN_ETHERNET_MINSIZE)) == NULL) {
+		if (skb_padto(pMessage, C_LEN_ETHERNET_MINSIZE)) {
 			spin_unlock_irqrestore(&pTxPort->TxDesRingLock, Flags);
 			return 0;
 		}
diff --git a/drivers/net/skge.c b/drivers/net/skge.c
index 536dd1c..19a4a16 100644
--- a/drivers/net/skge.c
+++ b/drivers/net/skge.c
@@ -2310,8 +2310,7 @@ static int skge_xmit_frame(struct sk_buf
 	u64 map;
 	unsigned long flags;
 
-	skb = skb_padto(skb, ETH_ZLEN);
-	if (!skb)
+	if (skb_padto(skb, ETH_ZLEN))
 		return NETDEV_TX_OK;
 
 	if (!spin_trylock_irqsave(&skge->tx_lock, flags))
diff --git a/drivers/net/smc9194.c b/drivers/net/smc9194.c
index 6cf16f3..8b0321f 100644
--- a/drivers/net/smc9194.c
+++ b/drivers/net/smc9194.c
@@ -523,8 +523,7 @@ static int smc_wait_to_send_packet( stru
 	length = skb->len;
 
 	if (length < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL) {
+		if (skb_padto(skb, ETH_ZLEN)) {
 			netif_wake_queue(dev);
 			return 0;
 		}
diff --git a/drivers/net/sonic.c b/drivers/net/sonic.c
index 90b818a..cab0dd9 100644
--- a/drivers/net/sonic.c
+++ b/drivers/net/sonic.c
@@ -231,8 +231,7 @@ static int sonic_send_packet(struct sk_b
 
 	length = skb->len;
 	if (length < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
+		if (skb_padto(skb, ETH_ZLEN))
 			return 0;
 		length = ETH_ZLEN;
 	}
diff --git a/drivers/net/starfire.c b/drivers/net/starfire.c
index 9b7805b..c158eed 100644
--- a/drivers/net/starfire.c
+++ b/drivers/net/starfire.c
@@ -1349,8 +1349,7 @@ static int start_tx(struct sk_buff *skb,
 
 #if defined(ZEROCOPY) && defined(HAS_BROKEN_FIRMWARE)
 	if (skb->ip_summed == CHECKSUM_HW) {
-		skb = skb_padto(skb, (skb->len + PADDING_MASK) & ~PADDING_MASK);
-		if (skb == NULL)
+		if (skb_padto(skb, (skb->len + PADDING_MASK) & ~PADDING_MASK))
 			return NETDEV_TX_OK;
 	}
 #endif /* ZEROCOPY && HAS_BROKEN_FIRMWARE */
diff --git a/drivers/net/via-rhine.c b/drivers/net/via-rhine.c
index fdc2103..c80a4f1 100644
--- a/drivers/net/via-rhine.c
+++ b/drivers/net/via-rhine.c
@@ -1284,11 +1284,8 @@ static int rhine_start_tx(struct sk_buff
 	/* Calculate the next Tx descriptor entry. */
 	entry = rp->cur_tx % TX_RING_SIZE;
 
-	if (skb->len < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
-			return 0;
-	}
+	if (skb_padto(skb, ETH_ZLEN))
+		return 0;
 
 	rp->tx_skbuff[entry] = skb;
 
diff --git a/drivers/net/wireless/ray_cs.c b/drivers/net/wireless/ray_cs.c
index 879eb42..a915fe6 100644
--- a/drivers/net/wireless/ray_cs.c
+++ b/drivers/net/wireless/ray_cs.c
@@ -924,8 +924,7 @@ static int ray_dev_start_xmit(struct sk_
 
     if (length < ETH_ZLEN)
     {
-    	skb = skb_padto(skb, ETH_ZLEN);
-    	if (skb == NULL)
+    	if (skb_padto(skb, ETH_ZLEN))
     		return 0;
     	length = ETH_ZLEN;
     }
diff --git a/drivers/net/wireless/wavelan.c b/drivers/net/wireless/wavelan.c
index dade4b9..aba56af 100644
--- a/drivers/net/wireless/wavelan.c
+++ b/drivers/net/wireless/wavelan.c
@@ -2936,11 +2936,8 @@ #endif
 	 * and we don't have the Ethernet specific requirement of beeing
 	 * able to detect collisions, therefore in theory we don't really
 	 * need to pad. Jean II */
-	if (skb->len < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
-			return 0;
-	}
+	if (skb_padto(skb, ETH_ZLEN))
+		return 0;
 
 	/* Write packet on the card */
 	if(wv_packet_write(dev, skb->data, skb->len))
diff --git a/drivers/net/wireless/wavelan_cs.c b/drivers/net/wireless/wavelan_cs.c
index f7724eb..561250f 100644
--- a/drivers/net/wireless/wavelan_cs.c
+++ b/drivers/net/wireless/wavelan_cs.c
@@ -3194,11 +3194,8 @@ #endif
 	 * and we don't have the Ethernet specific requirement of beeing
 	 * able to detect collisions, therefore in theory we don't really
 	 * need to pad. Jean II */
-	if (skb->len < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
-			return 0;
-	}
+	if (skb_padto(skb, ETH_ZLEN))
+		return 0;
 
   wv_packet_write(dev, skb->data, skb->len);
 
diff --git a/drivers/net/yellowfin.c b/drivers/net/yellowfin.c
index fd0f43b..ecec8e5 100644
--- a/drivers/net/yellowfin.c
+++ b/drivers/net/yellowfin.c
@@ -862,13 +862,11 @@ static int yellowfin_start_xmit(struct s
 		/* Fix GX chipset errata. */
 		if (cacheline_end > 24  || cacheline_end == 0) {
 			len = skb->len + 32 - cacheline_end + 1;
-			if (len != skb->len)
-				skb = skb_padto(skb, len);
-		}
-		if (skb == NULL) {
-			yp->tx_skbuff[entry] = NULL;
-			netif_wake_queue(dev);
-			return 0;
+			if (skb_padto(skb, len)) {
+				yp->tx_skbuff[entry] = NULL;
+				netif_wake_queue(dev);
+				return 0;
+			}
 		}
 	}
 	yp->tx_skbuff[entry] = skb;
diff --git a/drivers/net/znet.c b/drivers/net/znet.c
index 3ac047b..a7c089d 100644
--- a/drivers/net/znet.c
+++ b/drivers/net/znet.c
@@ -544,8 +544,7 @@ static int znet_send_packet(struct sk_bu
 		printk(KERN_DEBUG "%s: ZNet_send_packet.\n", dev->name);
 
 	if (length < ETH_ZLEN) {
-		skb = skb_padto(skb, ETH_ZLEN);
-		if (skb == NULL)
+		if (skb_padto(skb, ETH_ZLEN))
 			return 0;
 		length = ETH_ZLEN;
 	}
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 66f8819..f8c7eb7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -345,7 +345,7 @@ extern struct sk_buff *skb_realloc_headr
 extern struct sk_buff *skb_copy_expand(const struct sk_buff *skb,
 				       int newheadroom, int newtailroom,
 				       gfp_t priority);
-extern struct sk_buff *		skb_pad(struct sk_buff *skb, int pad);
+extern int	       skb_pad(struct sk_buff *skb, int pad);
 #define dev_kfree_skb(a)	kfree_skb(a)
 extern void	      skb_over_panic(struct sk_buff *skb, int len,
 				     void *here);
@@ -1122,16 +1122,15 @@ static inline int skb_cow(struct sk_buff
  *
  *	Pads up a buffer to ensure the trailing bytes exist and are
  *	blanked. If the buffer already contains sufficient data it
- *	is untouched. Returns the buffer, which may be a replacement
- *	for the original, or NULL for out of memory - in which case
- *	the original buffer is still freed.
+ *	is untouched. Otherwise it is extended. Returns zero on
+ *	success. The skb is freed on error.
  */
  
-static inline struct sk_buff *skb_padto(struct sk_buff *skb, unsigned int len)
+static inline int skb_padto(struct sk_buff *skb, unsigned int len)
 {
 	unsigned int size = skb->len;
 	if (likely(size >= len))
-		return skb;
+		return 0;
 	return skb_pad(skb, len-size);
 }
 
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index bb7210f..fe63d4e 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -781,24 +781,40 @@ struct sk_buff *skb_copy_expand(const st
  *	filled. Used by network drivers which may DMA or transfer data
  *	beyond the buffer end onto the wire.
  *
- *	May return NULL in out of memory cases.
+ *	May return error in out of memory cases. The skb is freed on error.
  */
  
-struct sk_buff *skb_pad(struct sk_buff *skb, int pad)
+int skb_pad(struct sk_buff *skb, int pad)
 {
-	struct sk_buff *nskb;
+	int err;
+	int ntail;
 	
 	/* If the skbuff is non linear tailroom is always zero.. */
-	if (skb_tailroom(skb) >= pad) {
+	if (!skb_cloned(skb) && skb_tailroom(skb) >= pad) {
 		memset(skb->data+skb->len, 0, pad);
-		return skb;
+		return 0;
 	}
-	
-	nskb = skb_copy_expand(skb, skb_headroom(skb), skb_tailroom(skb) + pad, GFP_ATOMIC);
+
+	ntail = skb->data_len + pad - (skb->end - skb->tail);
+	if (likely(skb_cloned(skb) || ntail > 0)) {
+		err = pskb_expand_head(skb, 0, ntail, GFP_ATOMIC);
+		if (unlikely(err))
+			goto free_skb;
+	}
+
+	/* FIXME: The use of this function with non-linear skb's really needs
+	 * to be audited.
+	 */
+	err = skb_linearize(skb);
+	if (unlikely(err))
+		goto free_skb;
+
+	memset(skb->data + skb->len, 0, pad);
+	return 0;
+
+free_skb:
 	kfree_skb(skb);
-	if (nskb)
-		memset(nskb->data+nskb->len, 0, pad);
-	return nskb;
+	return err;
 }	
  
 /* Trims skb to length len. It can change skb pointers.

^ permalink raw reply related

* Re: [PATCH 1/3] Rough VJ Channel Implementation - vj_core.patch
From: James Morris @ 2006-06-22  3:58 UTC (permalink / raw)
  To: Kelly Daly; +Cc: David S. Miller, netdev, rusty
In-Reply-To: <200606221205.35161.kelly@au.ibm.com>

On Thu, 22 Jun 2006, Kelly Daly wrote:

> +	switch (bp->netchan_buf_proto) {
> +	case __constant_htons(ETH_P_IP): {

__constant_htons and friends should not be used in runtime code, only for 
data being initialized at compile time.



- James
-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply

* Re: [patch 2.6.17] s2io driver irq fix
From: Andrew Morton @ 2006-06-22  4:15 UTC (permalink / raw)
  To: Ananda Raju
  Cc: netdev, linux-kernel, linux-fsdevel, dgc, balbir, viro, neilb,
	jblunck, tglx, ananda.raju, leonid.grossman, ravinandan.arakali,
	alicia.pena
In-Reply-To: <Pine.GSO.4.10.10606211537540.23949-100000@guinness>

On Wed, 21 Jun 2006 15:50:49 -0400 (EDT)
Ananda Raju <Ananda.Raju@neterion.com> wrote:

> +	if (sp->intr_type == MSI_X) {
> +		int i;
>  
> -				free_irq(vector, arg);
> +		for (i=1; (sp->s2io_entries[i].in_use == MSIX_FLG); i++) {
> +			if (sp->s2io_entries[i].type == MSIX_FIFO_TYPE) {
> +				sprintf(sp->desc[i], "%s:MSI-X-%d-TX",
> +					dev->name, i);
> +				err = request_irq(sp->entries[i].vector,
> +					  s2io_msix_fifo_handle, 0, sp->desc[i],
> +						  sp->s2io_entries[i].arg);

Is it usual to prohibit IRQ sharing with msix?


^ permalink raw reply

* Re: [PATCH 1/3] Rough VJ Channel Implementation - vj_core.patch
From: Arnaldo Carvalho de Melo @ 2006-06-22  4:31 UTC (permalink / raw)
  To: James Morris; +Cc: Kelly Daly, David S. Miller, netdev, rusty
In-Reply-To: <Pine.LNX.4.64.0606212354420.15426@d.namei>

On 6/22/06, James Morris <jmorris@namei.org> wrote:
> On Thu, 22 Jun 2006, Kelly Daly wrote:
>
> > +     switch (bp->netchan_buf_proto) {
> > +     case __constant_htons(ETH_P_IP): {
>
> __constant_htons and friends should not be used in runtime code, only for
> data being initialized at compile time.

... because they generate the same code, so, to make source code less
cluttered  ... :-)

- Arnaldo

^ permalink raw reply

* Re: [PATCH 1/3] Rough VJ Channel Implementation - vj_core.patch
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2006-06-22  4:36 UTC (permalink / raw)
  To: jmorris; +Cc: kelly, davem, netdev, rusty, yoshfuji
In-Reply-To: <Pine.LNX.4.64.0606212354420.15426@d.namei>

In article <Pine.LNX.4.64.0606212354420.15426@d.namei> (at Wed, 21 Jun 2006 23:58:56 -0400 (EDT)), James Morris <jmorris@namei.org> says:

> On Thu, 22 Jun 2006, Kelly Daly wrote:
> 
> > +	switch (bp->netchan_buf_proto) {
> > +	case __constant_htons(ETH_P_IP): {
> 
> __constant_htons and friends should not be used in runtime code, only for 
> data being initialized at compile time.

I disagree.  For "case," use __constant_{hton,ntoh}{s,l}(), please.

--yoshfuji

^ permalink raw reply

* Re: Locking validator output on DCCP
From: Ian McDonald @ 2006-06-22  4:51 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Herbert Xu, Arnaldo Carvalho de Melo, DCCP Mailing List, netdev,
	Ingo Molnar
In-Reply-To: <1150885091.3057.30.camel@laptopd505.fenrus.org>

On 6/21/06, Arjan van de Ven <arjan@linux.intel.com> wrote:
> On Wed, 2006-06-21 at 10:34 +1000, Herbert Xu wrote:
> > > As I read this it is not a recursive lock as sk_clone is occurring
> > > second and is actually creating a new socket so they are trying to
> > > lock on different sockets.
> > >
> > > Can someone tell me whether I am correct in my thinking or not? If I
> > > am then I will work out how to tell the lock validator not to worry
> > > about it.
> >
> > I agree, this looks bogus.  Ingo, could you please take a look?
>
> Fix is relatively easy:
>
>
> sk_clone creates a new socket, and thus can never deadlock, and in fact
> can be called with the original socket locked. This therefore is a
> legitimate nesting case; mark it as such.
>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
>
>
> ---
>  net/core/sock.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-2.6.17-rc6-mm2/net/core/sock.c
> ===================================================================
> --- linux-2.6.17-rc6-mm2.orig/net/core/sock.c
> +++ linux-2.6.17-rc6-mm2/net/core/sock.c
> @@ -846,7 +846,7 @@ struct sock *sk_clone(const struct sock
>                 /* SANITY */
>                 sk_node_init(&newsk->sk_node);
>                 sock_lock_init(newsk);
> -               bh_lock_sock(newsk);
> +               bh_lock_sock_nested(newsk);
>
>                 atomic_set(&newsk->sk_rmem_alloc, 0);
>                 atomic_set(&newsk->sk_wmem_alloc, 0);
>
>
When I do this it now shifts around. I'll investigate further
(probably tomorrow).

Now get

Jun 22 14:20:48 localhost kernel: [ 1276.424531]
=============================================
Jun 22 14:20:48 localhost kernel: [ 1276.424541] [ INFO: possible
recursive locking detected ]
Jun 22 14:20:48 localhost kernel: [ 1276.424546]
---------------------------------------------
Jun 22 14:20:48 localhost kernel: [ 1276.424553] idle/0 is trying to
acquire lock:
Jun 22 14:20:48 localhost kernel: [ 1276.424559]
(&sk->sk_lock.slock#5/1){-+..}, at: [<c024594e>] sk_clone+0x5f/0x195
Jun 22 14:20:48 localhost kernel: [ 1276.424585]
Jun 22 14:20:48 localhost kernel: [ 1276.424587] but task is already
holding lock:
Jun 22 14:20:48 localhost kernel: [ 1276.424592]
(&sk->sk_lock.slock#5/1){-+..}, at: [<c027cd87>]
tcp_v4_rcv+0x42e/0x9b3
Jun 22 14:20:48 localhost kernel: [ 1276.424616]
Jun 22 14:20:48 localhost kernel: [ 1276.424618] other info that might
help us debug this:
Jun 22 14:20:48 localhost kernel: [ 1276.424624] 2 locks held by idle/0:
Jun 22 14:20:48 localhost kernel: [ 1276.424628]  #0:
(&tp->rx_lock){-+..}, at: [<e0898915>] rtl8139_poll+0x42/0x41c
[8139too]
Jun 22 14:20:48 localhost kernel: [ 1276.424666]  #1:
(&sk->sk_lock.slock#5/1){-+..}, at: [<c027cd87>]
tcp_v4_rcv+0x42e/0x9b3
Jun 22 14:20:48 localhost kernel: [ 1276.424685]
Jun 22 14:20:48 localhost kernel: [ 1276.424686] stack backtrace:
Jun 22 14:20:48 localhost kernel: [ 1276.425002]  [<c0103a2a>]
show_trace_log_lvl+0x53/0xff
Jun 22 14:20:48 localhost kernel: [ 1276.425038]  [<c0104078>]
show_trace+0x16/0x19
Jun 22 14:20:48 localhost kernel: [ 1276.425068]  [<c010411e>]
dump_stack+0x1a/0x1f
Jun 22 14:20:48 localhost kernel: [ 1276.425099]  [<c012d6cb>]
__lock_acquire+0x8e6/0x902
Jun 22 14:20:48 localhost kernel: [ 1276.425311]  [<c012d879>]
lock_acquire+0x4e/0x66
Jun 22 14:20:48 localhost kernel: [ 1276.425510]  [<c02989e1>]
_spin_lock_nested+0x26/0x36
Jun 22 14:20:48 localhost kernel: [ 1276.425726]  [<c024594e>]
sk_clone+0x5f/0x195
Jun 22 14:20:48 localhost kernel: [ 1276.427191]  [<c026d10f>]
inet_csk_clone+0xf/0x67
Jun 22 14:20:48 localhost kernel: [ 1276.428879]  [<c027d3d0>]
tcp_create_openreq_child+0x15/0x32b
Jun 22 14:20:48 localhost kernel: [ 1276.430598]  [<c027b383>]
tcp_v4_syn_recv_sock+0x47/0x29c
Jun 22 14:20:48 localhost kernel: [ 1276.432313]  [<e0fcf440>]
tcp_v6_syn_recv_sock+0x37/0x534 [ipv6]
Jun 22 14:20:48 localhost kernel: [ 1276.432482]  [<c027d886>]
tcp_check_req+0x1a0/0x2db
Jun 22 14:20:48 localhost kernel: [ 1276.434198]  [<c027aecc>]
tcp_v4_do_rcv+0x9f/0x2fe
Jun 22 14:20:48 localhost kernel: [ 1276.435911]  [<c027d28b>]
tcp_v4_rcv+0x932/0x9b3
Jun 22 14:20:48 localhost kernel: [ 1276.437632]  [<c0265980>]
ip_local_deliver+0x159/0x1f1
Jun 22 14:20:48 localhost kernel: [ 1276.439305]  [<c02657fa>]
ip_rcv+0x3e9/0x416
Jun 22 14:20:48 localhost kernel: [ 1276.440977]  [<c024bba4>]
netif_receive_skb+0x287/0x317
Jun 22 14:20:48 localhost kernel: [ 1276.442542]  [<e0898b67>]
rtl8139_poll+0x294/0x41c [8139too]
Jun 22 14:20:48 localhost kernel: [ 1276.442590]  [<c024d585>]
net_rx_action+0x8b/0x17c
Jun 22 14:20:48 localhost kernel: [ 1276.444160]  [<c011adf6>]
__do_softirq+0x54/0xb3
Jun 22 14:20:48 localhost kernel: [ 1276.444335]  [<c011ae84>]
do_softirq+0x2f/0x47
Jun 22 14:20:48 localhost kernel: [ 1276.444460]  [<c011b0a5>]
irq_exit+0x39/0x46
Jun 22 14:20:48 localhost kernel: [ 1276.444585]  [<c0104f73>] do_IRQ+0x77/0x84
Jun 22 14:20:48 localhost kernel: [ 1276.444621]  [<c0103561>]
common_interrupt+0x25/0x2c



-- 
Ian McDonald
Web: http://wand.net.nz/~iam4
Blog: http://imcdnzl.blogspot.com
WAND Network Research Group
Department of Computer Science
University of Waikato
New Zealand

^ permalink raw reply

* Re: Locking validator output on DCCP
From: Ian McDonald @ 2006-06-22  4:55 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Herbert Xu, netdev, DCCP Mailing List, Arnaldo Carvalho de Melo
In-Reply-To: <20060621105256.GA21015@elte.hu>

On 6/21/06, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> > > Can someone tell me whether I am correct in my thinking or not? If I
> > > am then I will work out how to tell the lock validator not to worry
> > > about it.
> >
> > I agree, this looks bogus.  Ingo, could you please take a look?
>
> sure - Ian, could you try Arjan's fix below?
>
>         Ingo
>
> ------------------------------------------------------------
> Subject: lock validator: annotate vlan "master" device locks
> From: Arjan van de Ven <arjan@linux.intel.com>
>
The fix you sent here was the incorrect one but I did test Arjan's as
per previous e-mail.

Real dumb question time. The lock validator is testing for recursive
lock holding. Given that this is a lock at a different address can we
eliminate all such cases? Or are you trying to detect code here that
keeps on locking same type of lock in case of error and we should
explicitly flag...

Ian
-- 
Ian McDonald
Web: http://wand.net.nz/~iam4
Blog: http://imcdnzl.blogspot.com
WAND Network Research Group
Department of Computer Science
University of Waikato
New Zealand

^ permalink raw reply

* Re: New Qlogic qla3xxx NIC Driver v2.02.00k31 for upstream inclusion
From: Andrew Morton @ 2006-06-22  4:55 UTC (permalink / raw)
  To: Ron Mercer; +Cc: jeff, linux-driver, netdev
In-Reply-To: <0BB3E5E7462EEA4295BC02D49691DC07176751@AVEXCH1.qlogic.org>

On Wed, 21 Jun 2006 14:24:27 -0700
"Ron Mercer" <ron.mercer@qlogic.com> wrote:

> Please add the qla3xxx NIC driver to the next netdev-2.6 GIT tree. 

- We won't be able to include this withot a Signed-off-by: as per section
  11 of Documentation/SubmittingPatches.

- The driver does a lot of:

static void ql_write_page0_reg(struct ql3_adapter *qdev,
			       volatile u32 * pRegister, u32 value)
{
	...
	writel(value, (u32 *) pRegister);

The volatile is undesirable (and, for writel, unneeded) (and ity has been
typecast away anwyay).

And the arg to writel is of the type `void __iomem *'.  Really, this
function should take an arg of type `void __iomem *pRegister' or `u32
__iomem *pRegister'.

Treat __iomem as a C type, and propagate it correctly from top to bottom
and you cannot go wrong.

- What's going on here?

static void fm93c56a_deselect(struct ql3_adapter *qdev)
{
	struct ql3xxx_port_registers *port_regs =
	    (struct ql3xxx_port_registers *)qdev->mem_map_registers;

  ->mem_map_registers is already of type `struct ql3xxx_port_registers
  __iomem *', so all the cast here does is to remove the __iomem, making
  the code incorrect...

  (many instances)

- Is there a better way of doing this?

static void ql_swap_mac_addr(u8 * macAddress)
{
#ifdef __BIG_ENDIAN
	u8 temp;
	temp = macAddress[0];
	macAddress[0] = macAddress[1];
	macAddress[1] = temp;
	temp = macAddress[2];
	macAddress[2] = macAddress[3];
	macAddress[3] = temp;
	temp = macAddress[4];
	macAddress[4] = macAddress[5];
	macAddress[5] = temp;
#endif
}

  It seems like a common sort of thing to do?

- ql_mii_write_reg_ex() has two up-to-10-millisecond busywaits.

- In fact, those up-to-10-millisecond busywaits are all over the place.

- The driver would be cleaner if it had a helper function rather than
  open-coding all those up-to-10-millisecond busywaits.


Have some random tweaks:


From: Andrew Morton <akpm@osdl.org>

- coding style

- use cpu_relax()

- use msleep()

Cc: "Ron Mercer" <ron.mercer@qlogic.com>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: Stephen Hemminger <shemminger@osdl.org>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 drivers/net/qla3xxx.c |  164 +++++++++++++++-------------------------
 1 file changed, 65 insertions(+), 99 deletions(-)

diff -puN drivers/net/qla3xxx.c~qla3xxx-NIC-driver-tidy drivers/net/qla3xxx.c
--- a/drivers/net/qla3xxx.c~qla3xxx-NIC-driver-tidy
+++ a/drivers/net/qla3xxx.c
@@ -57,7 +57,7 @@ static int debug = -1;		/* defaults abov
 module_param(debug, int, 0);
 MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
 
-static int msi = 0;
+static int msi;
 module_param(msi, int, 0);
 MODULE_PARM_DESC(msi, "Turn on Message Signaled Interrupts.");
 
@@ -84,6 +84,7 @@ static void ql_sem_spinlock(struct ql3_a
 		spin_unlock_irqrestore(&qdev->hw_lock, hw_flags);
 		if ((value & (sem_mask >> 16)) == sem_bits)
 			break;
+		cpu_relax();
 	}
 }
 
@@ -121,12 +122,11 @@ static int ql_wait_for_drvr_lock(struct 
 				 (QL_RESOURCE_BITS_BASE_CODE | (qdev->mac_index)
 				  * 2) << 1)) {
 			if (i < 10) {
-				set_current_state(TASK_UNINTERRUPTIBLE);
-				schedule_timeout(1 * HZ);
+				msleep(1000);
 				i++;
 			} else {
-				printk(KERN_ERR PFX
-				       "%s: Timed out waiting for driver lock...\n",
+				printk(KERN_ERR PFX "%s: Timed out waiting for "
+				       "driver lock...\n",
 				       qdev->ndev->name);
 				return 0;
 			}
@@ -149,7 +149,7 @@ static u32 ql_read_common_reg(struct ql3
 	value = readl(pRegister);
 	spin_unlock_irqrestore(&qdev->hw_lock, hw_flags);
 
-	return (value);
+	return value;
 }
 
 static u32 ql_read_page0_reg(struct ql3_adapter *qdev, volatile u32 * pRegister)
@@ -170,7 +170,7 @@ static u32 ql_read_page0_reg(struct ql3_
 	value = readl(pRegister);
 
 	spin_unlock_irqrestore(&qdev->hw_lock, hw_flags);
-	return (value);
+	return value;
 }
 
 static void ql_write_common_reg(struct ql3_adapter *qdev,
@@ -324,7 +324,7 @@ static struct ql_rcv_buf_cb *ql_get_from
 		qdev->lrg_buf_free_count--;
 	}
 
-	return (lrg_buf_cb);
+	return lrg_buf_cb;
 }
 
 static u32 addrBits = EEPROM_NO_ADDR_BITS;
@@ -439,7 +439,7 @@ static void fm93c56a_deselect(struct ql3
 {
 	struct ql3xxx_port_registers *port_regs =
 	    (struct ql3xxx_port_registers *)qdev->mem_map_registers;
-	qdev->eeprom_cmd_data = AUBURN_EEPROM_CS_0;
+  	qdev->eeprom_cmd_data = AUBURN_EEPROM_CS_0;
 	ql_write_common_reg(qdev, &port_regs->CommonRegs.serialPortInterfaceReg,
 			    ISP_NVRAM_MASK | qdev->eeprom_cmd_data);
 }
@@ -618,12 +618,11 @@ static int ql_mii_write_reg_ex(struct ql
 	count = 1000;
 	while (count) {
 		temp = ql_read_page0_reg(qdev, &port_regs->macMIIStatusReg);
-		if (!(temp & MAC_MII_STATUS_BSY)) {
+		if (!(temp & MAC_MII_STATUS_BSY))
 			break;
-		}
 		udelay(10);
 		count--;
-	};
+	}
 	if (!count) {
 		if (netif_msg_link(qdev))
 			printk(KERN_WARNING PFX
@@ -643,12 +642,11 @@ static int ql_mii_write_reg_ex(struct ql
 	count = 1000;
 	while (count) {
 		temp = ql_read_page0_reg(qdev, &port_regs->macMIIStatusReg);
-		if (!(temp & MAC_MII_STATUS_BSY)) {
+		if (!(temp & MAC_MII_STATUS_BSY))
 			break;
-		}
 		udelay(10);
 		count--;
-	};
+	}
 	if (!count) {
 		if (netif_msg_link(qdev))
 			printk(KERN_WARNING PFX
@@ -684,12 +682,11 @@ static int ql_mii_read_reg_ex(struct ql3
 	count = 1000;
 	while (count) {
 		temp = ql_read_page0_reg(qdev, &port_regs->macMIIStatusReg);
-		if (!(temp & MAC_MII_STATUS_BSY)) {
+		if (!(temp & MAC_MII_STATUS_BSY))
 			break;
-		}
 		udelay(10);
 		count--;
-	};
+	}
 	if (!count) {
 		if (netif_msg_link(qdev))
 			printk(KERN_WARNING PFX
@@ -713,12 +710,11 @@ static int ql_mii_read_reg_ex(struct ql3
 	count = 1000;
 	while (count) {
 		temp = ql_read_page0_reg(qdev, &port_regs->macMIIStatusReg);
-		if (!(temp & MAC_MII_STATUS_BSY)) {
+		if (!(temp & MAC_MII_STATUS_BSY))
 			break;
-		}
 		udelay(10);
 		count--;
-	};
+	}
 	if (!count) {
 		if (netif_msg_link(qdev))
 			printk(KERN_WARNING PFX
@@ -756,12 +752,11 @@ static int ql_mii_write_reg(struct ql3_a
 	count = 1000;
 	while (count) {
 		temp = ql_read_page0_reg(qdev, &port_regs->macMIIStatusReg);
-		if (!(temp & MAC_MII_STATUS_BSY)) {
+		if (!(temp & MAC_MII_STATUS_BSY))
 			break;
-		}
 		udelay(10);
 		count--;
-	};
+	}
 	if (!count) {
 		if (netif_msg_link(qdev))
 			printk(KERN_WARNING PFX
@@ -781,12 +776,11 @@ static int ql_mii_write_reg(struct ql3_a
 	count = 1000;
 	while (count) {
 		temp = ql_read_page0_reg(qdev, &port_regs->macMIIStatusReg);
-		if (!(temp & MAC_MII_STATUS_BSY)) {
+		if (!(temp & MAC_MII_STATUS_BSY))
 			break;
-		}
 		udelay(10);
 		count--;
-	};
+	}
 	if (!count) {
 		if (netif_msg_link(qdev))
 			printk(KERN_WARNING PFX
@@ -803,7 +797,7 @@ static int ql_mii_write_reg(struct ql3_a
 	return 0;
 }
 
-static int ql_mii_read_reg(struct ql3_adapter *qdev, u16 regAddr, u16 * value)
+static int ql_mii_read_reg(struct ql3_adapter *qdev, u16 regAddr, u16 *value)
 {
 	u32 temp;
 	struct ql3xxx_port_registers *port_regs =
@@ -819,12 +813,11 @@ static int ql_mii_read_reg(struct ql3_ad
 	count = 1000;
 	while (count) {
 		temp = ql_read_page0_reg(qdev, &port_regs->macMIIStatusReg);
-		if (!(temp & MAC_MII_STATUS_BSY)) {
+		if (!(temp & MAC_MII_STATUS_BSY))
 			break;
-		}
 		udelay(10);
 		count--;
-	};
+	}
 	if (!count) {
 		if (netif_msg_link(qdev))
 			printk(KERN_WARNING PFX
@@ -848,12 +841,11 @@ static int ql_mii_read_reg(struct ql3_ad
 	count = 1000;
 	while (count) {
 		temp = ql_read_page0_reg(qdev, &port_regs->macMIIStatusReg);
-		if (!(temp & MAC_MII_STATUS_BSY)) {
+		if (!(temp & MAC_MII_STATUS_BSY))
 			break;
-		}
 		udelay(10);
 		count--;
-	};
+	}
 	if (!count) {
 		if (netif_msg_link(qdev))
 			printk(KERN_WARNING PFX
@@ -992,11 +984,10 @@ static void ql_mac_enable(struct ql3_ada
 	else
 		value = (MAC_CONFIG_REG_PE << 16);
 
-	if (qdev->mac_index) {
+	if (qdev->mac_index)
 		ql_write_page0_reg(qdev, &port_regs->mac1ConfigReg, value);
-	} else {
+	else
 		ql_write_page0_reg(qdev, &port_regs->mac0ConfigReg, value);
-	}
 }
 
 static void ql_mac_cfg_soft_reset(struct ql3_adapter *qdev, u32 enable)
@@ -1010,11 +1001,10 @@ static void ql_mac_cfg_soft_reset(struct
 	else
 		value = (MAC_CONFIG_REG_SR << 16);
 
-	if (qdev->mac_index) {
+	if (qdev->mac_index)
 		ql_write_page0_reg(qdev, &port_regs->mac1ConfigReg, value);
-	} else {
+	else
 		ql_write_page0_reg(qdev, &port_regs->mac0ConfigReg, value);
-	}
 }
 
 static void ql_mac_cfg_gig(struct ql3_adapter *qdev, u32 enable)
@@ -1028,11 +1018,10 @@ static void ql_mac_cfg_gig(struct ql3_ad
 	else
 		value = (MAC_CONFIG_REG_GM << 16);
 
-	if (qdev->mac_index) {
+	if (qdev->mac_index)
 		ql_write_page0_reg(qdev, &port_regs->mac1ConfigReg, value);
-	} else {
+	else
 		ql_write_page0_reg(qdev, &port_regs->mac0ConfigReg, value);
-	}
 }
 
 static void ql_mac_cfg_full_dup(struct ql3_adapter *qdev, u32 enable)
@@ -1046,11 +1035,10 @@ static void ql_mac_cfg_full_dup(struct q
 	else
 		value = (MAC_CONFIG_REG_FD << 16);
 
-	if (qdev->mac_index) {
+	if (qdev->mac_index)
 		ql_write_page0_reg(qdev, &port_regs->mac1ConfigReg, value);
-	} else {
+	else
 		ql_write_page0_reg(qdev, &port_regs->mac0ConfigReg, value);
-	}
 }
 
 static void ql_mac_cfg_pause(struct ql3_adapter *qdev, u32 enable)
@@ -1066,11 +1054,10 @@ static void ql_mac_cfg_pause(struct ql3_
 	else
 		value = ((MAC_CONFIG_REG_TF | MAC_CONFIG_REG_RF) << 16);
 
-	if (qdev->mac_index) {
+	if (qdev->mac_index)
 		ql_write_page0_reg(qdev, &port_regs->mac1ConfigReg, value);
-	} else {
+	else
 		ql_write_page0_reg(qdev, &port_regs->mac0ConfigReg, value);
-	}
 }
 
 static int ql_is_fiber(struct ql3_adapter *qdev)
@@ -1166,18 +1153,16 @@ static u32 ql_get_link_speed(struct ql3_
 {
 	if (ql_is_fiber(qdev))
 		return SPEED_1000;
-	else {
+	else
 		return ql_phy_get_speed(qdev);
-	}
 }
 
 static int ql_is_link_full_dup(struct ql3_adapter *qdev)
 {
 	if (ql_is_fiber(qdev))
 		return 1;
-	else {
+	else
 		return ql_is_full_dup(qdev);
-	}
 }
 
 static int ql_link_down_detect(struct ql3_adapter *qdev)
@@ -1442,11 +1427,10 @@ static void ql_get_phy_owner(struct ql3_
 			(QL_RESOURCE_BITS_BASE_CODE | (qdev->mac_index) *
 			 2) << 7);
 
-	if (ql_this_adapter_controls_port(qdev, qdev->mac_index)) {
+	if (ql_this_adapter_controls_port(qdev, qdev->mac_index))
 		set_bit(QL_LINK_MASTER,&qdev->flags);
-	} else {
+	else
 		clear_bit(QL_LINK_MASTER,&qdev->flags);
-	}
 	ql_sem_unlock(qdev, QL_PHY_GIO_SEM_MASK);
 }
 
@@ -1461,13 +1445,11 @@ static void ql_init_scan_mode(struct ql3
 	ql_sem_unlock(qdev, QL_PHY_GIO_SEM_MASK);
 
 	if (test_bit(QL_LINK_OPTICAL,&qdev->flags)) {
-		if (ql_this_adapter_controls_port(qdev, qdev->mac_index)) {
+		if (ql_this_adapter_controls_port(qdev, qdev->mac_index))
 			ql_petbi_init_ex(qdev, qdev->mac_index);
-		}
 	} else {
-		if (ql_this_adapter_controls_port(qdev, qdev->mac_index)) {
+		if (ql_this_adapter_controls_port(qdev, qdev->mac_index))
 			ql_phy_init_ex(qdev, qdev->mac_index);
-		}
 	}
 }
 
@@ -1646,11 +1628,10 @@ static irqreturn_t ql3xxx_isr(int irq, v
 	} else if (value & ISP_IMR_DISABLE_CMPL_INT) {
 #ifdef CONFIG_QLA3XXX_NAPI
 		ql_disable_interrupts(qdev);
-		if (likely(netif_rx_schedule_prep(ndev))) {
+		if (likely(netif_rx_schedule_prep(ndev)))
 			__netif_rx_schedule(ndev);
-		} else {
+		else
 			ql_enable_interrupts(qdev);
-		}
 #else
 		handled = ql_intr_handler((unsigned long)qdev);
 #endif
@@ -1694,9 +1675,8 @@ static int ql_populate_free_queue(struct
 						  qdev->lrg_buffer_len -
 						  QL_HEADER_SPACE);
 				--qdev->lrg_buf_skb_check;
-				if (!qdev->lrg_buf_skb_check) {
+				if (!qdev->lrg_buf_skb_check)
 					return 1;
-				}
 			}
 		}
 		lrg_buf_cb = lrg_buf_cb->next;
@@ -1715,9 +1695,8 @@ static void ql_update_lrg_bufq_prod_inde
 	    && (qdev->lrg_buf_release_cnt >= 16)) {
 
 		if (qdev->lrg_buf_skb_check) {
-			if (!ql_populate_free_queue(qdev)) {
+			if (!ql_populate_free_queue(qdev))
 				return;
-			}
 		}
 
 		lrg_buf_q_ele = qdev->lrg_buf_next_free;
@@ -1726,7 +1705,6 @@ static void ql_update_lrg_bufq_prod_inde
 		       && (qdev->lrg_buf_free_count >= 8)) {
 
 			for (i = 0; i < 8; i++) {
-
 				lrg_buf_cb =
 				    ql_get_from_lrg_buf_free_list(qdev);
 				lrg_buf_q_ele->addr_high =
@@ -1799,9 +1777,8 @@ static void ql_process_mac_rx_intr(struc
 	 * Get the inbound address list (small buffer).
 	 */
 	offset = qdev->small_buf_index * QL_SMALL_BUFFER_SIZE;
-	if (++qdev->small_buf_index == NUM_SMALL_BUFFERS) {
+	if (++qdev->small_buf_index == NUM_SMALL_BUFFERS)
 		qdev->small_buf_index = 0;
-	}
 
 	curr_ial_ptr = (u32 *) (qdev->small_buf_virt_addr + offset);
 	qdev->last_rsp_offset = qdev->small_buf_phy_addr_low + offset;
@@ -1811,9 +1788,8 @@ static void ql_process_mac_rx_intr(struc
 	lrg_buf_phy_addr_low = le32_to_cpu(*curr_ial_ptr);
 	lrg_buf_cb1 = &qdev->lrg_buf[qdev->lrg_buf_index];
 	qdev->lrg_buf_release_cnt++;
-	if (++qdev->lrg_buf_index == NUM_LARGE_BUFFERS) {
+	if (++qdev->lrg_buf_index == NUM_LARGE_BUFFERS)
 		qdev->lrg_buf_index = 0;
-	}
 	curr_ial_ptr++;		/* 64-bit pointers require two incs. */
 	curr_ial_ptr++;
 
@@ -1825,9 +1801,8 @@ static void ql_process_mac_rx_intr(struc
 	 * Second buffer gets sent up the stack.
 	 */
 	qdev->lrg_buf_release_cnt++;
-	if (++qdev->lrg_buf_index == NUM_LARGE_BUFFERS) {
+	if (++qdev->lrg_buf_index == NUM_LARGE_BUFFERS)
 		qdev->lrg_buf_index = 0;
-	}
 	skb = lrg_buf_cb2->skb;
 
 	qdev->stats.rx_packets++;
@@ -1873,9 +1848,8 @@ static void ql_process_macip_rx_intr(str
 	 */
 
 	offset = qdev->small_buf_index * QL_SMALL_BUFFER_SIZE;
-	if (++qdev->small_buf_index == NUM_SMALL_BUFFERS) {
+	if (++qdev->small_buf_index == NUM_SMALL_BUFFERS)
 		qdev->small_buf_index = 0;
-	}
 	curr_ial_ptr = (u32 *) (qdev->small_buf_virt_addr + offset);
 	qdev->last_rsp_offset = qdev->small_buf_phy_addr_low + offset;
 	qdev->small_buf_release_cnt++;
@@ -1885,9 +1859,8 @@ static void ql_process_macip_rx_intr(str
 	lrg_buf_cb1 = &qdev->lrg_buf[qdev->lrg_buf_index];
 
 	qdev->lrg_buf_release_cnt++;
-	if (++qdev->lrg_buf_index == NUM_LARGE_BUFFERS) {
+	if (++qdev->lrg_buf_index == NUM_LARGE_BUFFERS)
 		qdev->lrg_buf_index = 0;
-	}
 	skb1 = lrg_buf_cb1->skb;
 	curr_ial_ptr++;		/* 64-bit pointers require two incs. */
 	curr_ial_ptr++;
@@ -1897,9 +1870,8 @@ static void ql_process_macip_rx_intr(str
 	lrg_buf_cb2 = &qdev->lrg_buf[qdev->lrg_buf_index];
 	skb2 = lrg_buf_cb2->skb;
 	qdev->lrg_buf_release_cnt++;
-	if (++qdev->lrg_buf_index == NUM_LARGE_BUFFERS) {
+	if (++qdev->lrg_buf_index == NUM_LARGE_BUFFERS)
 		qdev->lrg_buf_index = 0;
-	}
 
 	qdev->stats.rx_packets++;
 	qdev->stats.rx_bytes += length;
@@ -1908,11 +1880,10 @@ static void ql_process_macip_rx_intr(str
 	 * Copy the ethhdr from first buffer to second. This
 	 * is necessary for IP completions.
 	 */
-	if (*((u16 *) skb1->data) != 0xFFFF) {
+	if (*((u16 *) skb1->data) != 0xFFFF)
 		size = VLAN_ETH_HLEN;
-	} else {
+	else
 		size = ETH_HLEN;
-	}
 
 	skb_put(skb2, length);	/* Just the second buffer length here. */
 	pci_unmap_single(qdev->pdev,
@@ -2010,9 +1981,8 @@ static int ql_intr_handler(unsigned long
 			qdev->small_buf_q_producer_index++;
 
 			if (qdev->small_buf_q_producer_index ==
-			    NUM_SBUFQ_ENTRIES) {
+			    NUM_SBUFQ_ENTRIES)
 				qdev->small_buf_q_producer_index = 0;
-			}
 			qdev->small_buf_release_cnt -= 8;
 		}
 
@@ -2027,7 +1997,7 @@ static int ql_intr_handler(unsigned long
 			    qdev->rsp_consumer_index);
 
 	spin_unlock(&qdev->adapter_lock);
-	return (10 - max_ios_per_intr);
+	return 10 - max_ios_per_intr;
 }
 
 #else				/* !CONFIG_QLA3XXX_NAPI */
@@ -2098,9 +2068,8 @@ static int ql_tx_rx_clean(struct ql3_ada
 			qdev->small_buf_q_producer_index++;
 
 			if (qdev->small_buf_q_producer_index ==
-			    NUM_SBUFQ_ENTRIES) {
+			    NUM_SBUFQ_ENTRIES)
 				qdev->small_buf_q_producer_index = 0;
-			}
 			qdev->small_buf_release_cnt -= 8;
 		}
 
@@ -2125,7 +2094,7 @@ static int ql_tx_rx_clean(struct ql3_ada
 		spin_unlock(&qdev->tx_lock);
 	}
 
-	return (*tx_cleaned + *rx_cleaned);
+	return *tx_cleaned + *rx_cleaned;
 }
 
 static int ql_poll(struct net_device *ndev, int *budget)
@@ -2210,6 +2179,7 @@ static int ql3xxx_send(struct sk_buff *s
 
 	return NETDEV_TX_OK;
 }
+
 static int ql_alloc_net_req_rsp_queues(struct ql3_adapter *qdev)
 {
 	qdev->req_q_size =
@@ -2797,7 +2767,7 @@ static int ql_adapter_initialize(struct 
 	ql_sem_spinlock(qdev, QL_DDR_RAM_SEM_MASK,
 			(QL_RESOURCE_BITS_BASE_CODE | (qdev->mac_index) *
 			 2) << 4);
-	clear_bit(QL_LINK_MASTER,&qdev->flags);
+	clear_bit(QL_LINK_MASTER, &qdev->flags);
 	value = ql_read_page0_reg(qdev, &port_regs->portStatus);
 	if ((value & PORT_STATUS_IC) == 0) {
 		/* Chip has not been configured yet, so let it rip. */
@@ -2971,15 +2941,13 @@ static int ql_adapter_reset(struct ql3_a
 			    ql_read_common_reg(qdev,
 					       &port_regs->CommonRegs.
 					       ispControlStatus);
-			if ((value & ISP_CONTROL_FSR) == 0) {
+			if ((value & ISP_CONTROL_FSR) == 0)
 				break;
-			}
 			msleep(1000);
 		} while ((--max_wait_time));
 	}
-	if (max_wait_time == 0) {
+	if (max_wait_time == 0)
 		status = 1;
-	}
 
 	clear_bit(QL_RESET_ACTIVE,&qdev->flags);
 	set_bit(QL_RESET_DONE,&qdev->flags);
@@ -3211,9 +3179,8 @@ static int ql3xxx_close(struct net_devic
 	 * Wait for device to recover from a reset.
 	 * (Rarely happens, but possible.)
 	 */
-	while (!test_bit(QL_ADAPTER_UP,&qdev->flags)) {
+	while (!test_bit(QL_ADAPTER_UP,&qdev->flags))
 		msleep(50);
-	}
 
 	ql_adapter_down(qdev,QL_DO_RESET);
 	return 0;
@@ -3644,10 +3611,9 @@ static void __devexit ql3xxx_remove(stru
 	/*
 	 * Wait for any resets to complete...
 	 */
-	while (test_bit((QL_RESET_ACTIVE | QL_RESET_START |	QL_RESET_PER_SCSI),
-					&qdev->flags)) {
+	while (test_bit((QL_RESET_ACTIVE | QL_RESET_START | QL_RESET_PER_SCSI),
+					&qdev->flags))
 		msleep(1000);
-	}
 
 	index = qdev->index;
 	if (qdev->workqueue) {
_


^ permalink raw reply

* [PATCH 00/21] e1000: driver update to 7.1.9-k2
From: Kok, Auke @ 2006-06-22  5:18 UTC (permalink / raw)
  To: Garzik, Jeff
  Cc: netdev, Brandeburg, Jesse, Kok, Auke, Kok, Auke, Ronciak, John


Hi,

A large number of e1000 patches this update, including the release of
the integrated NIC in the ICH8 motherboard chipset, which has been
officially released into the public. Also known as i965, the lan
chipset on this motherboard is fully supported by this driver.

The motherboard actually comes with two different PHY's, one that
supports gigabit speeds and one that only does 10/100. This driver
supports both of them.

All of these changes correspond with the 7.1.9 release on SF this
week, and have been tested and validated by our team, who did and
an excellent job ;^).

Furthermore, the patches included also fixes included in the 7.0.38
and 7.0.41 releases, which were not yet merged into the kernel. This
patch series brings the kernel driver up to current.


---

Summary of patches:

[01]: fix loopback ethtool test
[02]: rework driver hardware reset locking
[03]: Make PHY powerup/down a function
[04]: fix CONFIG_PM blocks
[05]: small performance tweak by removing double code
[06]: add smart power down code
[07]: change printk into DPRINTK
[08]: recycle skb
[09]: rework module param code with uninitialized values
[10]: force register write flushes to circumvent broken platforms
[11]: disable CRC stripping workaround
[12]: fix adapter led blinking inconsistency
[13]: add E1000_BIG_ENDIAN symbol
[14]: M88 PHY workaround
[15]: check return value of _get_speed_and_duplex
[16]: disable ERT
[17]: add ich8lan core functions
[18]: integrate ich8 support into driver
[19]: allow user to disable ich8 lock loss workaround
[20]: add ich8lan device ID's
[21]: increase version to 7.1.9-k2


---

Since these patches are rather large together, they are also available
over http here:
    http://foo-projects.org/~sofar/e1000-7.1.9-k2/
And in a tarball:
    http://foo-projects.org/~sofar/e1000-7.1.9-k2/e1000-patches-7.1.9-k2.tar.bz2
---

Jeff, please pull from our git repository:

git://lost.foo-projects.org/~ahkok/git/netdev-2.6 upstream


These patches are against
    netdev-2.6#upstream 612eff0e3715a6faff5ba1b74873b99e036c59fe
(Brian Haley <brian.haley@hp.com> / [PATCH] s2io: netpoll support)

Cheers,

Auke


---
 drivers/net/e1000/e1000.h         |   10 
 drivers/net/e1000/e1000_ethtool.c |  143 +--
 drivers/net/e1000/e1000_hw.c      | 1770 +++++++++++++++++++++++++++++++++++---
 drivers/net/e1000/e1000_hw.h      |  400 ++++++++
 drivers/net/e1000/e1000_main.c    |  386 +++++---
 drivers/net/e1000/e1000_osdep.h   |   16 
 drivers/net/e1000/e1000_param.c   |  213 ++--
 7 files changed, 2539 insertions(+), 399 deletions(-)


--
Auke Kok <auke-jan.h.kok@intel.com>

^ permalink raw reply

* [PATCH 01/21] e1000: fix loopback ethtool test
From: Kok, Auke @ 2006-06-22  5:20 UTC (permalink / raw)
  To: Garzik, Jeff
  Cc: netdev, Brandeburg, Jesse, Kok, Auke, Kok, Auke, Ronciak, John
In-Reply-To: <20060622051815.25497.89192.stgit@gitlost.site>


Ethtool was reporting that loopback failed randomly on esb2
systems. Upon study it was found that the phy manual was changed
with respect to the loopback mode bits. The new value fixes it.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
---

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

diff --git a/drivers/net/e1000/e1000_ethtool.c b/drivers/net/e1000/e1000_ethtool.c
index 6ed7f59..845d293 100644
--- a/drivers/net/e1000/e1000_ethtool.c
+++ b/drivers/net/e1000/e1000_ethtool.c
@@ -1246,7 +1246,7 @@ e1000_integrated_phy_loopback(struct e10
 	} else if (adapter->hw.phy_type == e1000_phy_gg82563) {
 		e1000_write_phy_reg(&adapter->hw,
 		                    GG82563_PHY_KMRN_MODE_CTRL,
-		                    0x1CE);
+		                    0x1CC);
 	}
 	/* force 1000, set loopback */
 	e1000_write_phy_reg(&adapter->hw, PHY_CTRL, 0x4140);



--
Auke Kok <auke-jan.h.kok@intel.com>

^ permalink raw reply related

* [PATCH 02/21] e1000: rework driver hardware reset locking
From: Kok, Auke @ 2006-06-22  5:20 UTC (permalink / raw)
  To: Garzik, Jeff
  Cc: netdev, Brandeburg, Jesse, Kok, Auke, Kok, Auke, Ronciak, John
In-Reply-To: <20060622051815.25497.89192.stgit@gitlost.site>


After studying the driver mac reset code it was found that there
were multiple race conditions possible to reset the unit twice or
bring it e1000_up() double. This fixes all occurences where the
driver needs to reset the mac.

We also remove irq requesting/releasing into _open and _close so
that while the device is _up we will never touch the irq's. This fixes
the double free irq bug that people saw.

To make sure that the watchdog task doesn't cause another race we let
it run as a non-scheduled task.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
---

 drivers/net/e1000/e1000.h         |    8 ++
 drivers/net/e1000/e1000_ethtool.c |   46 ++++++++------
 drivers/net/e1000/e1000_main.c    |  126 +++++++++++++++++++++----------------
 3 files changed, 105 insertions(+), 75 deletions(-)

diff --git a/drivers/net/e1000/e1000.h b/drivers/net/e1000/e1000.h
index 2bc34fb..2b96ad0 100644
--- a/drivers/net/e1000/e1000.h
+++ b/drivers/net/e1000/e1000.h
@@ -69,7 +69,6 @@
 #ifdef NETIF_F_TSO
 #include <net/checksum.h>
 #endif
-#include <linux/workqueue.h>
 #include <linux/mii.h>
 #include <linux/ethtool.h>
 #include <linux/if_vlan.h>
@@ -255,7 +254,6 @@ struct e1000_adapter {
 	spinlock_t tx_queue_lock;
 #endif
 	atomic_t irq_sem;
-	struct work_struct watchdog_task;
 	struct work_struct reset_task;
 	uint8_t fc_autoneg;
 
@@ -340,8 +338,13 @@ struct e1000_adapter {
 #ifdef NETIF_F_TSO
 	boolean_t tso_force;
 #endif
+	unsigned long flags;
 };
 
+enum e1000_state_t {
+	__E1000_DRIVER_TESTING,
+	__E1000_RESETTING,
+};
 
 /*  e1000_main.c  */
 extern char e1000_driver_name[];
@@ -349,6 +352,7 @@ extern char e1000_driver_version[];
 int e1000_up(struct e1000_adapter *adapter);
 void e1000_down(struct e1000_adapter *adapter);
 void e1000_reset(struct e1000_adapter *adapter);
+void e1000_reinit_locked(struct e1000_adapter *adapter);
 int e1000_setup_all_tx_resources(struct e1000_adapter *adapter);
 void e1000_free_all_tx_resources(struct e1000_adapter *adapter);
 int e1000_setup_all_rx_resources(struct e1000_adapter *adapter);
diff --git a/drivers/net/e1000/e1000_ethtool.c b/drivers/net/e1000/e1000_ethtool.c
index 845d293..cf5c5f4 100644
--- a/drivers/net/e1000/e1000_ethtool.c
+++ b/drivers/net/e1000/e1000_ethtool.c
@@ -203,11 +203,9 @@ e1000_set_settings(struct net_device *ne
 
 	/* reset the link */
 
-	if (netif_running(adapter->netdev)) {
-		e1000_down(adapter);
-		e1000_reset(adapter);
-		e1000_up(adapter);
-	} else
+	if (netif_running(adapter->netdev))
+		e1000_reinit_locked(adapter);
+	else
 		e1000_reset(adapter);
 
 	return 0;
@@ -254,10 +252,9 @@ e1000_set_pauseparam(struct net_device *
 	hw->original_fc = hw->fc;
 
 	if (adapter->fc_autoneg == AUTONEG_ENABLE) {
-		if (netif_running(adapter->netdev)) {
-			e1000_down(adapter);
-			e1000_up(adapter);
-		} else
+		if (netif_running(adapter->netdev))
+			e1000_reinit_locked(adapter);
+		else
 			e1000_reset(adapter);
 	} else
 		return ((hw->media_type == e1000_media_type_fiber) ?
@@ -279,10 +276,9 @@ e1000_set_rx_csum(struct net_device *net
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	adapter->rx_csum = data;
 
-	if (netif_running(netdev)) {
-		e1000_down(adapter);
-		e1000_up(adapter);
-	} else
+	if (netif_running(netdev))
+		e1000_reinit_locked(adapter);
+	else
 		e1000_reset(adapter);
 	return 0;
 }
@@ -631,6 +627,9 @@ e1000_set_ringparam(struct net_device *n
 	tx_ring_size = sizeof(struct e1000_tx_ring) * adapter->num_tx_queues;
 	rx_ring_size = sizeof(struct e1000_rx_ring) * adapter->num_rx_queues;
 
+	while (test_and_set_bit(__E1000_RESETTING, &adapter->flags))
+		msleep(1);
+
 	if (netif_running(adapter->netdev))
 		e1000_down(adapter);
 
@@ -691,9 +690,11 @@ e1000_set_ringparam(struct net_device *n
 		adapter->rx_ring = rx_new;
 		adapter->tx_ring = tx_new;
 		if ((err = e1000_up(adapter)))
-			return err;
+			goto err_setup;
 	}
 
+	clear_bit(__E1000_RESETTING, &adapter->flags);
+
 	return 0;
 err_setup_tx:
 	e1000_free_all_rx_resources(adapter);
@@ -701,6 +702,8 @@ err_setup_rx:
 	adapter->rx_ring = rx_old;
 	adapter->tx_ring = tx_old;
 	e1000_up(adapter);
+err_setup:
+	clear_bit(__E1000_RESETTING, &adapter->flags);
 	return err;
 }
 
@@ -1568,6 +1571,7 @@ e1000_diag_test(struct net_device *netde
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	boolean_t if_running = netif_running(netdev);
 
+	set_bit(__E1000_DRIVER_TESTING, &adapter->flags);
 	if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
 		/* Offline tests */
 
@@ -1582,7 +1586,8 @@ e1000_diag_test(struct net_device *netde
 			eth_test->flags |= ETH_TEST_FL_FAILED;
 
 		if (if_running)
-			e1000_down(adapter);
+			/* indicate we're in test mode */
+			dev_close(netdev);
 		else
 			e1000_reset(adapter);
 
@@ -1607,8 +1612,9 @@ e1000_diag_test(struct net_device *netde
 		adapter->hw.autoneg = autoneg;
 
 		e1000_reset(adapter);
+		clear_bit(__E1000_DRIVER_TESTING, &adapter->flags);
 		if (if_running)
-			e1000_up(adapter);
+			dev_open(netdev);
 	} else {
 		/* Online tests */
 		if (e1000_link_test(adapter, &data[4]))
@@ -1619,6 +1625,8 @@ e1000_diag_test(struct net_device *netde
 		data[1] = 0;
 		data[2] = 0;
 		data[3] = 0;
+
+		clear_bit(__E1000_DRIVER_TESTING, &adapter->flags);
 	}
 	msleep_interruptible(4 * 1000);
 }
@@ -1807,10 +1815,8 @@ static int
 e1000_nway_reset(struct net_device *netdev)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
-	if (netif_running(netdev)) {
-		e1000_down(adapter);
-		e1000_up(adapter);
-	}
+	if (netif_running(netdev))
+		e1000_reinit_locked(adapter);
 	return 0;
 }
 
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index a373ccb..52d698b 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -133,7 +133,6 @@ static void e1000_clean_rx_ring(struct e
 static void e1000_set_multi(struct net_device *netdev);
 static void e1000_update_phy_info(unsigned long data);
 static void e1000_watchdog(unsigned long data);
-static void e1000_watchdog_task(struct e1000_adapter *adapter);
 static void e1000_82547_tx_fifo_stall(unsigned long data);
 static int e1000_xmit_frame(struct sk_buff *skb, struct net_device *netdev);
 static struct net_device_stats * e1000_get_stats(struct net_device *netdev);
@@ -261,6 +260,44 @@ e1000_exit_module(void)
 
 module_exit(e1000_exit_module);
 
+static int e1000_request_irq(struct e1000_adapter *adapter)
+{
+	struct net_device *netdev = adapter->netdev;
+	int flags, err = 0;
+
+	flags = SA_SHIRQ | SA_SAMPLE_RANDOM;
+#ifdef CONFIG_PCI_MSI
+	if (adapter->hw.mac_type > e1000_82547_rev_2) {
+		adapter->have_msi = TRUE;
+		if ((err = pci_enable_msi(adapter->pdev))) {
+			DPRINTK(PROBE, ERR,
+			 "Unable to allocate MSI interrupt Error: %d\n", err);
+			adapter->have_msi = FALSE;
+		}
+	}
+	if (adapter->have_msi)
+		flags &= ~SA_SHIRQ;
+#endif
+	if ((err = request_irq(adapter->pdev->irq, &e1000_intr, flags,
+	                       netdev->name, netdev)))
+		DPRINTK(PROBE, ERR,
+		        "Unable to allocate interrupt Error: %d\n", err);
+
+	return err;
+}
+
+static void e1000_free_irq(struct e1000_adapter *adapter)
+{
+	struct net_device *netdev = adapter->netdev;
+
+	free_irq(adapter->pdev->irq, netdev);
+
+#ifdef CONFIG_PCI_MSI
+	if (adapter->have_msi)
+		pci_disable_msi(adapter->pdev);
+#endif
+}
+
 /**
  * e1000_irq_disable - Mask off interrupt generation on the NIC
  * @adapter: board private structure
@@ -387,7 +424,7 @@ int
 e1000_up(struct e1000_adapter *adapter)
 {
 	struct net_device *netdev = adapter->netdev;
-	int i, err;
+	int i;
 
 	/* hardware has been reset, we need to reload some things */
 
@@ -415,24 +452,6 @@ e1000_up(struct e1000_adapter *adapter)
 		                      E1000_DESC_UNUSED(ring));
 	}
 
-#ifdef CONFIG_PCI_MSI
-	if (adapter->hw.mac_type > e1000_82547_rev_2) {
-		adapter->have_msi = TRUE;
-		if ((err = pci_enable_msi(adapter->pdev))) {
-			DPRINTK(PROBE, ERR,
-			 "Unable to allocate MSI interrupt Error: %d\n", err);
-			adapter->have_msi = FALSE;
-		}
-	}
-#endif
-	if ((err = request_irq(adapter->pdev->irq, &e1000_intr,
-		              SA_SHIRQ | SA_SAMPLE_RANDOM,
-		              netdev->name, netdev))) {
-		DPRINTK(PROBE, ERR,
-		    "Unable to allocate interrupt Error: %d\n", err);
-		return err;
-	}
-
 	adapter->tx_queue_len = netdev->tx_queue_len;
 
 	mod_timer(&adapter->watchdog_timer, jiffies);
@@ -450,16 +469,10 @@ e1000_down(struct e1000_adapter *adapter
 {
 	struct net_device *netdev = adapter->netdev;
 	boolean_t mng_mode_enabled = (adapter->hw.mac_type >= e1000_82571) &&
-				     e1000_check_mng_mode(&adapter->hw);
+	                              e1000_check_mng_mode(&adapter->hw);
 
 	e1000_irq_disable(adapter);
 
-	free_irq(adapter->pdev->irq, netdev);
-#ifdef CONFIG_PCI_MSI
-	if (adapter->hw.mac_type > e1000_82547_rev_2 &&
-	   adapter->have_msi == TRUE)
-		pci_disable_msi(adapter->pdev);
-#endif
 	del_timer_sync(&adapter->tx_fifo_stall_timer);
 	del_timer_sync(&adapter->watchdog_timer);
 	del_timer_sync(&adapter->phy_info_timer);
@@ -496,6 +509,17 @@ e1000_down(struct e1000_adapter *adapter
 }
 
 void
+e1000_reinit_locked(struct e1000_adapter *adapter)
+{
+	WARN_ON(in_interrupt());
+	while (test_and_set_bit(__E1000_RESETTING, &adapter->flags))
+		msleep(1);
+	e1000_down(adapter);
+	e1000_up(adapter);
+	clear_bit(__E1000_RESETTING, &adapter->flags);
+}
+
+void
 e1000_reset(struct e1000_adapter *adapter)
 {
 	uint32_t pba, manc;
@@ -758,9 +782,6 @@ e1000_probe(struct pci_dev *pdev,
 	adapter->watchdog_timer.function = &e1000_watchdog;
 	adapter->watchdog_timer.data = (unsigned long) adapter;
 
-	INIT_WORK(&adapter->watchdog_task,
-		(void (*)(void *))e1000_watchdog_task, adapter);
-
 	init_timer(&adapter->phy_info_timer);
 	adapter->phy_info_timer.function = &e1000_update_phy_info;
 	adapter->phy_info_timer.data = (unsigned long) adapter;
@@ -1078,6 +1099,10 @@ e1000_open(struct net_device *netdev)
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 	int err;
 
+	/* disallow open during test */
+	if (test_bit(__E1000_DRIVER_TESTING, &adapter->flags))
+		return -EBUSY;
+
 	/* allocate transmit descriptors */
 
 	if ((err = e1000_setup_all_tx_resources(adapter)))
@@ -1088,6 +1113,10 @@ e1000_open(struct net_device *netdev)
 	if ((err = e1000_setup_all_rx_resources(adapter)))
 		goto err_setup_rx;
 
+	err = e1000_request_irq(adapter);
+	if (err)
+		goto err_up;
+
 	if ((err = e1000_up(adapter)))
 		goto err_up;
 	adapter->mng_vlan_id = E1000_MNG_VLAN_NONE;
@@ -1131,7 +1160,9 @@ e1000_close(struct net_device *netdev)
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 
+	WARN_ON(test_bit(__E1000_RESETTING, &adapter->flags));
 	e1000_down(adapter);
+	e1000_free_irq(adapter);
 
 	e1000_free_all_tx_resources(adapter);
 	e1000_free_all_rx_resources(adapter);
@@ -2201,14 +2232,6 @@ static void
 e1000_watchdog(unsigned long data)
 {
 	struct e1000_adapter *adapter = (struct e1000_adapter *) data;
-
-	/* Do the rest outside of interrupt context */
-	schedule_work(&adapter->watchdog_task);
-}
-
-static void
-e1000_watchdog_task(struct e1000_adapter *adapter)
-{
 	struct net_device *netdev = adapter->netdev;
 	struct e1000_tx_ring *txdr = adapter->tx_ring;
 	uint32_t link, tctl;
@@ -2919,8 +2942,7 @@ e1000_reset_task(struct net_device *netd
 {
 	struct e1000_adapter *adapter = netdev_priv(netdev);
 
-	e1000_down(adapter);
-	e1000_up(adapter);
+	e1000_reinit_locked(adapter);
 }
 
 /**
@@ -3026,10 +3048,8 @@ e1000_change_mtu(struct net_device *netd
 
 	netdev->mtu = new_mtu;
 
-	if (netif_running(netdev)) {
-		e1000_down(adapter);
-		e1000_up(adapter);
-	}
+	if (netif_running(netdev))
+		e1000_reinit_locked(adapter);
 
 	adapter->hw.max_frame_size = max_frame;
 
@@ -4180,10 +4200,9 @@ e1000_mii_ioctl(struct net_device *netde
 						return retval;
 					}
 				}
-				if (netif_running(adapter->netdev)) {
-					e1000_down(adapter);
-					e1000_up(adapter);
-				} else
+				if (netif_running(adapter->netdev))
+					e1000_reinit_locked(adapter);
+				else
 					e1000_reset(adapter);
 				break;
 			case M88E1000_PHY_SPEC_CTRL:
@@ -4200,10 +4219,9 @@ e1000_mii_ioctl(struct net_device *netde
 			case PHY_CTRL:
 				if (mii_reg & MII_CR_POWER_DOWN)
 					break;
-				if (netif_running(adapter->netdev)) {
-					e1000_down(adapter);
-					e1000_up(adapter);
-				} else
+				if (netif_running(adapter->netdev))
+					e1000_reinit_locked(adapter);
+				else
 					e1000_reset(adapter);
 				break;
 			}
@@ -4462,8 +4480,10 @@ e1000_suspend(struct pci_dev *pdev, pm_m
 
 	netif_device_detach(netdev);
 
-	if (netif_running(netdev))
+	if (netif_running(netdev)) {
+		WARN_ON(test_bit(__E1000_RESETTING, &adapter->flags));
 		e1000_down(adapter);
+	}
 
 #ifdef CONFIG_PM
 	/* Implement our own version of pci_save_state(pdev) because pci-



--
Auke Kok <auke-jan.h.kok@intel.com>

^ 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