Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next-2.6] tcp: tso_fragment() might avoid GFP_ATOMIC
From: David Miller @ 2010-06-29  6:38 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1277377222.2816.296.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 24 Jun 2010 13:00:22 +0200

> We can pass a gfp argument to tso_fragment() and avoid GFP_ATOMIC
> allocations sometimes.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

Slow down Eric, you're on fire :-)

^ permalink raw reply

* Re: [PATCH net-next-2.6 4/4] vlan: 64 bit rx counters
From: David Miller @ 2010-06-29  6:37 UTC (permalink / raw)
  To: eric.dumazet; +Cc: kaber, netdev
In-Reply-To: <1277376906.2816.287.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 24 Jun 2010 12:55:06 +0200

> Use u64_stats_sync infrastructure to implement 64bit rx stats.
> 
> (tx stats are addressed later)
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next-2.6 3/4] macvlan: 64 bit rx counters
From: David Miller @ 2010-06-29  6:37 UTC (permalink / raw)
  To: eric.dumazet; +Cc: kaber, netdev
In-Reply-To: <1277376861.2816.284.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 24 Jun 2010 12:54:21 +0200

> Use u64_stats_sync infrastructure to implement 64bit stats.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next-2.6 2/4] net: u64_stats_fetch_begin_bh() and u64_stats_fetch_retry_bh()
From: David Miller @ 2010-06-29  6:37 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1277376846.2816.283.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 24 Jun 2010 12:54:06 +0200

> - Must disable preemption in case of 32bit UP in u64_stats_fetch_begin()
> and u64_stats_fetch_retry()
> 
> - Add new u64_stats_fetch_begin_bh() and u64_stats_fetch_retry_bh() for
> network usage, disabling BH on 32bit UP only.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: use this_cpu_ptr()
From: David Miller @ 2010-06-29  6:37 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1277376757.2816.272.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 24 Jun 2010 12:52:37 +0200

> use this_cpu_ptr(p) instead of per_cpu_ptr(p, smp_processor_id())
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: u64_stats_sync improvements
From: David Miller @ 2010-06-29  6:37 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1277373878.2816.177.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 24 Jun 2010 12:04:38 +0200

> - Add a comment about interrupts:
> 
> 6) If counter might be written by an interrupt, readers should block
> interrupts.
> 
> - Fix a typo in sample of use.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [iproute2] iproute2: Allow 'ip addr flush' to loop more than 10 times.
From: David Miller @ 2010-06-29  6:36 UTC (permalink / raw)
  To: greearb; +Cc: greearb, netdev
In-Reply-To: <4C29925B.9090008@candelatech.com>

From: Ben Greear <greearb@candelatech.com>
Date: Mon, 28 Jun 2010 23:27:39 -0700

> I'm not sure I understand how this loop could have run forever
> anyway, unless some other process(es) was constantly adding
> addresses at the same time?  Or maybe some ipv6 auto config thing?
> 
> It appears there is already code to detect when the loop
> is done (flushing ~70 IPv4 addresses with -l 0 was one of my
> test cases, and worked as expected).

What happens is that we are simply limited by how many addresses
we can delete in one go, and that limit is 4096 bytes of netlink
message size.

So we have to iterate, reusing that buffer each time, to get them all
done.

The limit exists because meanwhile it is possible that some other
entity could add addresses and thus cause us to loop forever and
never actually delete all of the addresses because every time we
delete a bunch the other entity adds more.

I can understand the reasoning behind the limit, because if this is
run by something automated it's not like someone is at the command
line and hit Ctrl-C to break out of a looping instance.

But practically speaking I bet this never happens.

So what makes sense to me is:

1) Loop forever by default.

2) When the number of loops exceeds a threshold (calculated by the
   number of addresses we see the first dump, divided by the number
   of deletes we can squeeze into the 4096 byte message), we emit
   a warning.

3) A hard limit, off by default, it available via your "-l" new option.

But seriously we can determine forward progress quite easily I think.

Each loop, we see if the dump returns a smaller number of addresses
than the last iteration.  If so, we just keep going.

If the number of addresses increases, I think we can bail in this
case.

This logic would only ever trigger iff another entity is adding a
large number of addresses simultaneously with our flush.  And frankly
speaking the person doing the flush probably doesn't expect that to be
happening.  You're flushing all of the addresses so you can start with
a clean slate and then add specific addresses back, or whatever.


^ permalink raw reply

* Re: [iproute2] iproute2: Allow 'ip addr flush' to loop more than 10 times.
From: Ben Greear @ 2010-06-29  6:27 UTC (permalink / raw)
  To: David Miller; +Cc: greearb, netdev
In-Reply-To: <20100628.231204.229752207.davem@davemloft.net>

On 06/28/2010 11:12 PM, David Miller wrote:
> From: greearb@gmail.com
> Date: Mon, 28 Jun 2010 22:55:59 -0700
>
>> From: Ben Greear<greearb@candelatech.com>
>>
>> The default remains at 10 for backwards compatibility.
>>
>> For instance:
>>   # ip addr flush dev eth2
>>   *** Flush remains incomplete after 10 rounds. ***
>>   # ip -l 20 addr flush dev eth2
>>   *** Flush remains incomplete after 20 rounds. ***
>>   # ip -loops 0 addr flush dev eth2
>>   #
>>
>> This is useful for getting rid of large numbers of IP
>> addresses in scripts.
>>
>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>
> I would suggest to instead add some logic to this code to detect that
> forward progress is being made.
>
> I really don't see any value in having a hard limit that triggers on a
> bulk delete when no other address changing activity is happening in
> the system.

I'm not sure I understand how this loop could have run forever
anyway, unless some other process(es) was constantly adding addresses at
the same time?  Or maybe some ipv6 auto config thing?

It appears there is already code to detect when the loop
is done (flushing ~70 IPv4 addresses with -l 0 was one of my
test cases, and worked as expected).

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

^ permalink raw reply

* Re: [PATCH net-next-2.6 1/2] 3c59x: Specify window explicitly for access to windowed registers
From: David Miller @ 2010-06-29  6:20 UTC (permalink / raw)
  To: ben; +Cc: netdev, chase.douglas, nordmark
In-Reply-To: <1277337271.26161.17.camel@localhost>

From: Ben Hutchings <ben@decadent.org.uk>
Date: Thu, 24 Jun 2010 00:54:31 +0100

> Currently much of the code assumes that a specific window has been
> selected, while a few functions save and restore the window.  This
> makes it impossible to introduce fine-grained locking.
> 
> Make those assumptions explicit by introducing wrapper functions
> to set the window and read/write a register.  Use these everywhere
> except vortex_interrupt(), vortex_start_xmit() and vortex_rx().
> These set the window just once, or not at all in the case of
> vortex_rx() as it should always be called from vortex_interrupt().
> 
> Cache the current window in struct vortex_private to avoid
> unnecessary hardware writes.
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Tested-by: Arne Nordmark <nordmark@mech.kth.se> [against 2.6.32]

Applied.

^ permalink raw reply

* Re: [PATCH net-next-2.6 2/2] 3c59x: Use fine-grained locks for MII and windowed register access
From: David Miller @ 2010-06-29  6:18 UTC (permalink / raw)
  To: steffen.klassert; +Cc: ben, netdev, chase.douglas, nordmark
In-Reply-To: <20100625082447.GK5570@secunet.com>

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Fri, 25 Jun 2010 10:24:47 +0200

> These locks are not needed, why you want to have them arround?

Steffen I think you are being overly picky of Ben's changes.

I'd rather have too much locking during device probe and
initialization than a subtle bug that occurs because later on someone
decides to move IRQ enabling earlier in the chip init path and now
we get strange hangs that take forever to diagnose.

I mean, extra locking in probe/init paths... ugh, there are so many
more important things to worry about!

Once Ben posts a new version of this second patch with the
proper spin_lock_init() calls added I am going to apply both
of his changes.

^ permalink raw reply

* Re: [iproute2] iproute2: Allow 'ip addr flush' to loop more than 10 times.
From: David Miller @ 2010-06-29  6:12 UTC (permalink / raw)
  To: greearb; +Cc: netdev, greearb
In-Reply-To: <1277790959-28075-1-git-send-email-greearb@candelatech.com>

From: greearb@gmail.com
Date: Mon, 28 Jun 2010 22:55:59 -0700

> From: Ben Greear <greearb@candelatech.com>
> 
> The default remains at 10 for backwards compatibility.
> 
> For instance:
>  # ip addr flush dev eth2
>  *** Flush remains incomplete after 10 rounds. ***
>  # ip -l 20 addr flush dev eth2
>  *** Flush remains incomplete after 20 rounds. ***
>  # ip -loops 0 addr flush dev eth2
>  #
> 
> This is useful for getting rid of large numbers of IP
> addresses in scripts.
> 
> Signed-off-by: Ben Greear <greearb@candelatech.com>

I would suggest to instead add some logic to this code to detect that
forward progress is being made.

I really don't see any value in having a hard limit that triggers on a
bulk delete when no other address changing activity is happening in
the system.

^ permalink raw reply

* Re: [v4 Patch 2/2] mlx4: add dynamic LRO disable support
From: David Miller @ 2010-06-29  6:04 UTC (permalink / raw)
  To: amwang; +Cc: netdev, nhorman, sgruszka, herbert.xu, bhutchings,
	Ramkrishna.Vepa
In-Reply-To: <20100622085426.5566.51436.sendpatchset@localhost.localdomain>

From: Amerigo Wang <amwang@redhat.com>
Date: Tue, 22 Jun 2010 04:50:17 -0400

> 
> This patch adds dynamic LRO diable support for mlx4 net driver.
> It also fixes a bug of mlx4, which checks NETIF_F_LRO flag in rx
> path without rtnl lock.
> 
> (I don't have mlx4 card, so only did compiling test. Anyone who wants
> to test this is more than welcome.)
> 
> This is based on Neil's initial work too, and heavily modified based
> on Stanislaw's suggestions.
> 
> Signed-off-by: WANG Cong <amwang@redhat.com>
> Signed-off-by: Neil Horman <nhorman@redhat.com>
> Acked-by: Neil Horman <nhorman@redhat.com>
> Reviewed-by: Stanislaw Gruszka <sgruszka@redhat.com>
> Cc: Ben Hutchings <bhutchings@solarflare.com>

Applied to net-next-2.6

I'd really like the module options to just die as the dynamic
ethtool mechanism should be the only knob for this for consistency
with the rest of the drivers.

^ permalink raw reply

* Re: [PATCH] s2io: add dynamic LRO disable support
From: David Miller @ 2010-06-29  6:04 UTC (permalink / raw)
  To: jon.mason
  Cc: amwang, netdev, nhorman, sgruszka, herbert.xu, bhutchings,
	Ramkrishna.Vepa
In-Reply-To: <20100625044510.GC2739@exar.com>

From: Jon Mason <jon.mason@exar.com>
Date: Thu, 24 Jun 2010 23:45:10 -0500

> This patch adds dynamic LRO disable support for s2io net driver,
> enables LRO by default, increases the driver version number, and
> corrects the name of the LRO modparm.
> 
> This is mostly Wang's patch based on Neil's initial work, heavily
> modified based on Ramkrishna's suggestions.  This has been tested on
> a Neterion Xframe adapter and verified via adapter LRO statistics.
> 
> Signed-off-by: Jon Mason <jon.mason@exar.com>
> Signed-off-by: WANG Cong <amwang@redhat.com>
> Signed-off-by: Neil Horman <nhorman@redhat.com>
> Acked-by: Neil Horman <nhorman@redhat.com>
> Reviewed-by: Stanislaw Gruszka <sgruszka@redhat.com>
> Cc: Ramkrishna Vepa <Ramkrishna.Vepa@exar.com>

Applied to net-next-2.6

^ permalink raw reply

* [iproute2] iproute2:  Allow 'ip addr flush' to loop more than 10 times.
From: greearb @ 2010-06-29  5:55 UTC (permalink / raw)
  To: netdev; +Cc: Ben Greear

From: Ben Greear <greearb@candelatech.com>

The default remains at 10 for backwards compatibility.

For instance:
 # ip addr flush dev eth2
 *** Flush remains incomplete after 10 rounds. ***
 # ip -l 20 addr flush dev eth2
 *** Flush remains incomplete after 20 rounds. ***
 # ip -loops 0 addr flush dev eth2
 #

This is useful for getting rid of large numbers of IP
addresses in scripts.

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 f7ef939... 3da6998... M	include/utils.h
:100644 100644 9f29533... b127d57... M	ip/ip.c
:100644 100644 3a411b1... 5f0789c... M	ip/ipaddress.c
:100644 100644 1a73efa... d0146a5... M	man/man8/ip.8
 include/utils.h |    1 +
 ip/ip.c         |   11 ++++++++++-
 ip/ipaddress.c  |    6 +++---
 man/man8/ip.8   |    6 ++++++
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/include/utils.h b/include/utils.h
index f7ef939..3da6998 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -17,6 +17,7 @@ extern int resolve_hosts;
 extern int oneline;
 extern int timestamp;
 extern char * _SL_;
+extern int max_flush_loops;
 
 #ifndef IPPROTO_ESP
 #define IPPROTO_ESP	50
diff --git a/ip/ip.c b/ip/ip.c
index 9f29533..b127d57 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -32,6 +32,8 @@ int timestamp = 0;
 char * _SL_ = NULL;
 char *batch_file = NULL;
 int force = 0;
+int max_flush_loops = 10;
+
 struct rtnl_handle rth = { .fd = -1 };
 
 static void usage(void) __attribute__((noreturn));
@@ -45,6 +47,7 @@ static void usage(void)
 "                   tunnel | tuntap | maddr | mroute | mrule | monitor | xfrm }\n"
 "       OPTIONS := { -V[ersion] | -s[tatistics] | -d[etails] | -r[esolve] |\n"
 "                    -f[amily] { inet | inet6 | ipx | dnet | link } |\n"
+"                    -l[oops] { maximum-addr-flush-attempts } |\n"
 "                    -o[neline] | -t[imestamp] | -b[atch] [filename] |\n"
 "                    -rc[vbuf] [size]}\n");
 	exit(-1);
@@ -157,7 +160,13 @@ int main(int argc, char **argv)
 			break;
 		if (opt[1] == '-')
 			opt++;
-		if (matches(opt, "-family") == 0) {
+		if (matches(opt, "-loops") == 0) {
+			argc--;
+			argv++;
+			if (argc <= 1)
+				usage();
+                        max_flush_loops = atoi(argv[1]);
+                } else if (matches(opt, "-family") == 0) {
 			argc--;
 			argv++;
 			if (argc <= 1)
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 3a411b1..5f0789c 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -33,7 +33,6 @@
 #include "ll_map.h"
 #include "ip_common.h"
 
-#define MAX_ROUNDS 10
 
 static struct
 {
@@ -818,7 +817,7 @@ static int ipaddr_list_or_flush(int argc, char **argv, int flush)
 		filter.flushp = 0;
 		filter.flushe = sizeof(flushb);
 
-		while (round < MAX_ROUNDS) {
+		while ((max_flush_loops == 0) || (round < max_flush_loops)) {
 			const struct rtnl_dump_filter_arg a[3] = {
 				{
 					.filter = print_addrinfo_secondary,
@@ -867,7 +866,8 @@ static int ipaddr_list_or_flush(int argc, char **argv, int flush)
 				fflush(stdout);
 			}
 		}
-		fprintf(stderr, "*** Flush remains incomplete after %d rounds. ***\n", MAX_ROUNDS); fflush(stderr);
+		fprintf(stderr, "*** Flush remains incomplete after %d rounds. ***\n", max_flush_loops);
+		fflush(stderr);
 		return 1;
 	}
 
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index 1a73efa..d0146a5 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -730,6 +730,12 @@ appears twice or more, the amount of information increases.
 As a rule, the information is statistics or some time values.
 
 .TP
+.BR "\-l" , " \-loops"
+Specify maximum number of loops the 'ip addr flush' logic
+will attempt before giving up.  The default is 10.
+Zero (0) means loop until all addresses are removed.
+
+.TP
 .BR "\-f" , " \-family"
 followed by protocol family identifier:
 .BR "inet" , " inet6"
-- 
1.7.0.1


^ permalink raw reply related

* Re: [PATCH net-next-2.6] xfrm: remove export of xfrm4_rcv_encap.
From: Eric Dumazet @ 2010-06-29  5:37 UTC (permalink / raw)
  To: Rami Rosen; +Cc: davem, netdev
In-Reply-To: <AANLkTimIJ_vhe7wwCqTrfjj4HUdrbpyrlAwnQGrtAiOk@mail.gmail.com>

Le mardi 29 juin 2010 à 08:05 +0300, Rami Rosen a écrit :
> Hi,
>  The patch removes EXPORT_SYMBOL of xfrm4_rcv_encap() method
>  as it is unneeded.
> 
> Regards,
> Rami Rosen
> 

This claim seems wrong. I wonder how you came to it.

CONFIG_INET_XFRM_TUNNEL=m
CONFIG_XFRM_IPCOMP=m
CONFIG_INET_IPCOMP=m

ERROR: "xfrm4_rcv_encap" [net/ipv4/xfrm4_tunnel.ko] undefined!




^ permalink raw reply

* Re: b44: Reset due to FIFO overflow.
From: Eric Dumazet @ 2010-06-29  5:17 UTC (permalink / raw)
  To: Mitchell Erblich; +Cc: James Courtier-Dutton, netdev
In-Reply-To: <ED315045-4A5D-4ECA-99C8-06B4714D8FA0@earthlink.net>

Le lundi 28 juin 2010 à 14:21 -0700, Mitchell Erblich a écrit :
> On Jun 28, 2010, at 4:09 AM, Eric Dumazet wrote:
> 
> > Le lundi 28 juin 2010 à 11:17 +0100, James Courtier-Dutton a écrit :
> >> On 28 June 2010 11:00, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >>> 
> >>> Problem is we receive a spike of RX network frames (possibly UDP or some
> >>> other RX only trafic), and chip raises an RX fifo overflow _error_
> >>> indication.
> >>> 
> 
> IMO, spikes are a normal behaviour.

Yes, this is why I said NIC is buggy, if it requires a reset (lasting a
_very_ long time) on a normal condition.

> 
> >> 
> >> The cause of the RX overflow is in my case is TCP.
> >> It is reproducible in mythtv.
> >> While watching LiveTV, press "s" for the program guide.
> >> The program guide is implemented into mythtv by a SQL query that
> >> results in a large response.
> >> The kernel is probably not servicing the RX FIFO quickly enough due to
> >> it being busy doing something else. In this case, probably a video
> >> mode switch.
> >> 
> > 
> > Thats strange, b44 has a big RX ring... and tcp sender should wait for
> > ACK...
> > 
> 
> Slow start, etc SHOULD/CAN  double the number of in-flight segments in each
> next round-trip, placing them back to back.
> 

rx ring buffer is about 200 frames on b44. One single tcp flow should
fit.

Limit is 511. James, did you try to increase rx ring ?

ethtool -G eth0 rx 511

> IMO,  a stress test, would be a large number/wirespeed set of pings?
> 

Better is to use frames that are going to slow down receiver.
Say multicast trafic with 100 receivers on same multicast group.
Send 1000 consecutive frames, last ones will trigger RX overflow,
because softirq handler cannot be fast enough.

Ping is answered by kernel, its pretty fast.

> >>> Some hardware are buggy enough that such error indication is fatal and
> >>> _require_ hardware reset. Thats life. I suspect b44 driver doing a full
> >>> reset is not a random guess from driver author, but to avoid a complete
> >>> NIC lockup.
> >>> 
> >> 
> >> Interesting, which hardware, apart from the b44, is it that "requires"
> >> a hardware reset after a RX FIFO overflow.
> > 
> > Just take a look at some net drivers and you'll see some of them have
> > this requirement.
> > 
> > rtl8169_rx_interrupt()
> > ...
> > 	if (status & RxFOVF) {
> > 		rtl8169_schedule_work(dev, rtl8169_reset_task);
> > 		dev->stats.rx_fifo_errors++;
> > 	}
> > 
> > 
> > 
> > 
> 
> 
> If they can reset in say X frame loss units, then why not reset if
> X is an acceptable number?
> 

Because a reset is an exception. While card is reset, we lose many tx
and rx frames and this should be the very last thing to consider.

Why not a complete reboot of the host while we are at it ?

> And a hammer may fix the dent, while I may be more
> interested in preventing the dent in the first place.

So ? Please submit an alternative firmware for this NIC, or provide
another NIC on thousand of machines that are stuck with it.




^ permalink raw reply

* [PATCH net-next-2.6] xfrm: remove export of xfrm4_rcv_encap.
From: Rami Rosen @ 2010-06-29  5:05 UTC (permalink / raw)
  To: davem, netdev

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

Hi,
 The patch removes EXPORT_SYMBOL of xfrm4_rcv_encap() method
 as it is unneeded.

Regards,
Rami Rosen


Signed-off-by: Rami Rosen <ramirose@gmail.com>

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 464 bytes --]

diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
index ad8fbb8..d85336c 100644
--- a/net/ipv4/xfrm4_input.c
+++ b/net/ipv4/xfrm4_input.c
@@ -44,7 +44,6 @@ int xfrm4_rcv_encap(struct sk_buff *skb, int nexthdr, __be32 spi,
 	XFRM_SPI_SKB_CB(skb)->daddroff = offsetof(struct iphdr, daddr);
 	return xfrm_input(skb, nexthdr, spi, encap_type);
 }
-EXPORT_SYMBOL(xfrm4_rcv_encap);
 
 int xfrm4_transport_finish(struct sk_buff *skb, int async)
 {

^ permalink raw reply related

* Re: [PATCH 3/3] pm_qos: get rid of the allocation in pm_qos_add_request()
From: mark gross @ 2010-06-29  4:39 UTC (permalink / raw)
  To: James Bottomley; +Cc: Linux PM, markgross, netdev, Takashi Iwai
In-Reply-To: <1277747088.10879.201.camel@mulgrave.site>

On Mon, Jun 28, 2010 at 12:44:48PM -0500, James Bottomley wrote:
> Since every caller has to squirrel away the returned pointer anyway,
> they might as well supply the memory area.  This fixes a bug in a few of
> the call sites where the returned pointer was dereferenced without
> checking it for NULL (which gets returned if the kzalloc failed).
> 
> I'd like to hear how sound and netdev feels about this: it will add
> about two more pointers worth of data to struct netdev and struct
> snd_pcm_substream .. but I think it's worth it.  If you're OK, I'll add
> your acks and send through the pm tree.
> 
> This also looks to me like an android independent clean up (even though
> it renders the request_add atomically callable).  I also added include
> guards to include/linux/pm_qos_params.h
> 
> cc: netdev@vger.kernel.org
> cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: James Bottomley <James.Bottomley@suse.de>
Thank you for doing this!, I'll integrate it into some testing targets
in the morning!

Signed-off-by: mark gross <markgross@thegnar.org>

--mgross



> ---
>  drivers/net/e1000e/netdev.c            |   17 +++-----
>  drivers/net/igbvf/netdev.c             |    9 ++--
>  drivers/net/wireless/ipw2x00/ipw2100.c |   12 +++---
>  include/linux/netdevice.h              |    2 +-
>  include/linux/pm_qos_params.h          |   13 +++++-
>  include/sound/pcm.h                    |    2 +-
>  kernel/pm_qos_params.c                 |   67 +++++++++++++++++++-------------
>  sound/core/pcm_native.c                |   13 ++----
>  8 files changed, 74 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index 24507f3..47ea62f 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -2901,10 +2901,10 @@ static void e1000_configure_rx(struct e1000_adapter *adapter)
>  			 * dropped transactions.
>  			 */
>  			pm_qos_update_request(
> -				adapter->netdev->pm_qos_req, 55);
> +				&adapter->netdev->pm_qos_req, 55);
>  		} else {
>  			pm_qos_update_request(
> -				adapter->netdev->pm_qos_req,
> +				&adapter->netdev->pm_qos_req,
>  				PM_QOS_DEFAULT_VALUE);
>  		}
>  	}
> @@ -3196,9 +3196,9 @@ int e1000e_up(struct e1000_adapter *adapter)
>  
>  	/* DMA latency requirement to workaround early-receive/jumbo issue */
>  	if (adapter->flags & FLAG_HAS_ERT)
> -		adapter->netdev->pm_qos_req =
> -			pm_qos_add_request(PM_QOS_CPU_DMA_LATENCY,
> -				       PM_QOS_DEFAULT_VALUE);
> +		pm_qos_add_request(&adapter->netdev->pm_qos_req,
> +				   PM_QOS_CPU_DMA_LATENCY,
> +				   PM_QOS_DEFAULT_VALUE);
>  
>  	/* hardware has been reset, we need to reload some things */
>  	e1000_configure(adapter);
> @@ -3263,11 +3263,8 @@ void e1000e_down(struct e1000_adapter *adapter)
>  	e1000_clean_tx_ring(adapter);
>  	e1000_clean_rx_ring(adapter);
>  
> -	if (adapter->flags & FLAG_HAS_ERT) {
> -		pm_qos_remove_request(
> -			      adapter->netdev->pm_qos_req);
> -		adapter->netdev->pm_qos_req = NULL;
> -	}
> +	if (adapter->flags & FLAG_HAS_ERT)
> +		pm_qos_remove_request(&adapter->netdev->pm_qos_req);
>  
>  	/*
>  	 * TODO: for power management, we could drop the link and
> diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c
> index 5e2b2a8..add6197 100644
> --- a/drivers/net/igbvf/netdev.c
> +++ b/drivers/net/igbvf/netdev.c
> @@ -48,7 +48,7 @@
>  #define DRV_VERSION "1.0.0-k0"
>  char igbvf_driver_name[] = "igbvf";
>  const char igbvf_driver_version[] = DRV_VERSION;
> -struct pm_qos_request_list *igbvf_driver_pm_qos_req;
> +static struct pm_qos_request_list igbvf_driver_pm_qos_req;
>  static const char igbvf_driver_string[] =
>  				"Intel(R) Virtual Function Network Driver";
>  static const char igbvf_copyright[] = "Copyright (c) 2009 Intel Corporation.";
> @@ -2902,8 +2902,8 @@ static int __init igbvf_init_module(void)
>  	printk(KERN_INFO "%s\n", igbvf_copyright);
>  
>  	ret = pci_register_driver(&igbvf_driver);
> -	igbvf_driver_pm_qos_req = pm_qos_add_request(PM_QOS_CPU_DMA_LATENCY,
> -	                       PM_QOS_DEFAULT_VALUE);
> +	pm_qos_add_request(&igbvf_driver_pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
> +			   PM_QOS_DEFAULT_VALUE);
>  
>  	return ret;
>  }
> @@ -2918,8 +2918,7 @@ module_init(igbvf_init_module);
>  static void __exit igbvf_exit_module(void)
>  {
>  	pci_unregister_driver(&igbvf_driver);
> -	pm_qos_remove_request(igbvf_driver_pm_qos_req);
> -	igbvf_driver_pm_qos_req = NULL;
> +	pm_qos_remove_request(&igbvf_driver_pm_qos_req);
>  }
>  module_exit(igbvf_exit_module);
>  
> diff --git a/drivers/net/wireless/ipw2x00/ipw2100.c b/drivers/net/wireless/ipw2x00/ipw2100.c
> index 0bd4dfa..7f0d98b 100644
> --- a/drivers/net/wireless/ipw2x00/ipw2100.c
> +++ b/drivers/net/wireless/ipw2x00/ipw2100.c
> @@ -174,7 +174,7 @@ that only one external action is invoked at a time.
>  #define DRV_DESCRIPTION	"Intel(R) PRO/Wireless 2100 Network Driver"
>  #define DRV_COPYRIGHT	"Copyright(c) 2003-2006 Intel Corporation"
>  
> -struct pm_qos_request_list *ipw2100_pm_qos_req;
> +struct pm_qos_request_list ipw2100_pm_qos_req;
>  
>  /* Debugging stuff */
>  #ifdef CONFIG_IPW2100_DEBUG
> @@ -1741,7 +1741,7 @@ static int ipw2100_up(struct ipw2100_priv *priv, int deferred)
>  	/* the ipw2100 hardware really doesn't want power management delays
>  	 * longer than 175usec
>  	 */
> -	pm_qos_update_request(ipw2100_pm_qos_req, 175);
> +	pm_qos_update_request(&ipw2100_pm_qos_req, 175);
>  
>  	/* If the interrupt is enabled, turn it off... */
>  	spin_lock_irqsave(&priv->low_lock, flags);
> @@ -1889,7 +1889,7 @@ static void ipw2100_down(struct ipw2100_priv *priv)
>  	ipw2100_disable_interrupts(priv);
>  	spin_unlock_irqrestore(&priv->low_lock, flags);
>  
> -	pm_qos_update_request(ipw2100_pm_qos_req, PM_QOS_DEFAULT_VALUE);
> +	pm_qos_update_request(&ipw2100_pm_qos_req, PM_QOS_DEFAULT_VALUE);
>  
>  	/* We have to signal any supplicant if we are disassociating */
>  	if (associated)
> @@ -6669,8 +6669,8 @@ static int __init ipw2100_init(void)
>  	if (ret)
>  		goto out;
>  
> -	ipw2100_pm_qos_req = pm_qos_add_request(PM_QOS_CPU_DMA_LATENCY,
> -			PM_QOS_DEFAULT_VALUE);
> +	pm_qos_add_request(&ipw2100_pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
> +			   PM_QOS_DEFAULT_VALUE);
>  #ifdef CONFIG_IPW2100_DEBUG
>  	ipw2100_debug_level = debug;
>  	ret = driver_create_file(&ipw2100_pci_driver.driver,
> @@ -6692,7 +6692,7 @@ static void __exit ipw2100_exit(void)
>  			   &driver_attr_debug_level);
>  #endif
>  	pci_unregister_driver(&ipw2100_pci_driver);
> -	pm_qos_remove_request(ipw2100_pm_qos_req);
> +	pm_qos_remove_request(&ipw2100_pm_qos_req);
>  }
>  
>  module_init(ipw2100_init);
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 40291f3..393555a 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -779,7 +779,7 @@ struct net_device {
>  	 */
>  	char			name[IFNAMSIZ];
>  
> -	struct pm_qos_request_list *pm_qos_req;
> +	struct pm_qos_request_list pm_qos_req;
>  
>  	/* device name hash chain */
>  	struct hlist_node	name_hlist;
> diff --git a/include/linux/pm_qos_params.h b/include/linux/pm_qos_params.h
> index 8ba440e..77cbddb 100644
> --- a/include/linux/pm_qos_params.h
> +++ b/include/linux/pm_qos_params.h
> @@ -1,8 +1,10 @@
> +#ifndef _LINUX_PM_QOS_PARAMS_H
> +#define _LINUX_PM_QOS_PARAMS_H
>  /* interface for the pm_qos_power infrastructure of the linux kernel.
>   *
>   * Mark Gross <mgross@linux.intel.com>
>   */
> -#include <linux/list.h>
> +#include <linux/plist.h>
>  #include <linux/notifier.h>
>  #include <linux/miscdevice.h>
>  
> @@ -14,9 +16,12 @@
>  #define PM_QOS_NUM_CLASSES 4
>  #define PM_QOS_DEFAULT_VALUE -1
>  
> -struct pm_qos_request_list;
> +struct pm_qos_request_list {
> +	struct plist_node list;
> +	int pm_qos_class;
> +};
>  
> -struct pm_qos_request_list *pm_qos_add_request(int pm_qos_class, s32 value);
> +void pm_qos_add_request(struct pm_qos_request_list *l, int pm_qos_class, s32 value);
>  void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
>  		s32 new_value);
>  void pm_qos_remove_request(struct pm_qos_request_list *pm_qos_req);
> @@ -24,4 +29,6 @@ void pm_qos_remove_request(struct pm_qos_request_list *pm_qos_req);
>  int pm_qos_request(int pm_qos_class);
>  int pm_qos_add_notifier(int pm_qos_class, struct notifier_block *notifier);
>  int pm_qos_remove_notifier(int pm_qos_class, struct notifier_block *notifier);
> +int pm_qos_request_active(struct pm_qos_request_list *req);
>  
> +#endif
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index dd76cde..6e3a297 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -366,7 +366,7 @@ struct snd_pcm_substream {
>  	int number;
>  	char name[32];			/* substream name */
>  	int stream;			/* stream (direction) */
> -	struct pm_qos_request_list *latency_pm_qos_req; /* pm_qos request */
> +	struct pm_qos_request_list latency_pm_qos_req; /* pm_qos request */
>  	size_t buffer_bytes_max;	/* limit ring buffer size */
>  	struct snd_dma_buffer dma_buffer;
>  	unsigned int dma_buf_id;
> diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> index b130b97..bff4053 100644
> --- a/kernel/pm_qos_params.c
> +++ b/kernel/pm_qos_params.c
> @@ -30,7 +30,6 @@
>  /*#define DEBUG*/
>  
>  #include <linux/pm_qos_params.h>
> -#include <linux/plist.h>
>  #include <linux/sched.h>
>  #include <linux/spinlock.h>
>  #include <linux/slab.h>
> @@ -49,11 +48,6 @@
>   * or pm_qos_object list and pm_qos_objects need to happen with pm_qos_lock
>   * held, taken with _irqsave.  One lock to rule them all
>   */
> -struct pm_qos_request_list {
> -	struct plist_node list;
> -	int pm_qos_class;
> -};
> -
>  enum pm_qos_type {
>  	PM_QOS_MAX,		/* return the largest value */
>  	PM_QOS_MIN		/* return the smallest value */
> @@ -210,6 +204,12 @@ int pm_qos_request(int pm_qos_class)
>  }
>  EXPORT_SYMBOL_GPL(pm_qos_request);
>  
> +int pm_qos_request_active(struct pm_qos_request_list *req)
> +{
> +	return req->pm_qos_class != 0;
> +}
> +EXPORT_SYMBOL_GPL(pm_qos_request_active);
> +
>  /**
>   * pm_qos_add_request - inserts new qos request into the list
>   * @pm_qos_class: identifies which list of qos request to us
> @@ -221,25 +221,23 @@ EXPORT_SYMBOL_GPL(pm_qos_request);
>   * element as a handle for use in updating and removal.  Call needs to save
>   * this handle for later use.
>   */
> -struct pm_qos_request_list *pm_qos_add_request(int pm_qos_class, s32 value)
> +void pm_qos_add_request(struct pm_qos_request_list *dep,
> +			int pm_qos_class, s32 value)
>  {
> -	struct pm_qos_request_list *dep;
> -
> -	dep = kzalloc(sizeof(struct pm_qos_request_list), GFP_KERNEL);
> -	if (dep) {
> -		struct pm_qos_object *o =  pm_qos_array[pm_qos_class];
> -		int new_value;
> -
> -		if (value == PM_QOS_DEFAULT_VALUE)
> -			new_value = o->default_value;
> -		else
> -			new_value = value;
> -		plist_node_init(&dep->list, new_value);
> -		dep->pm_qos_class = pm_qos_class;
> -		update_target(o, &dep->list, 0, PM_QOS_DEFAULT_VALUE);
> -	}
> +	struct pm_qos_object *o =  pm_qos_array[pm_qos_class];
> +	int new_value;
>  
> -	return dep;
> +	if (pm_qos_request_active(dep)) {
> +		WARN(1, KERN_ERR "pm_qos_add_request() called for already added request\n");
> +		return;
> +	}
> +	if (value == PM_QOS_DEFAULT_VALUE)
> +		new_value = o->default_value;
> +	else
> +		new_value = value;
> +	plist_node_init(&dep->list, new_value);
> +	dep->pm_qos_class = pm_qos_class;
> +	update_target(o, &dep->list, 0, PM_QOS_DEFAULT_VALUE);
>  }
>  EXPORT_SYMBOL_GPL(pm_qos_add_request);
>  
> @@ -262,6 +260,11 @@ void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
>  	if (!pm_qos_req) /*guard against callers passing in null */
>  		return;
>  
> +	if (pm_qos_request_active(pm_qos_req)) {
> +		WARN(1, KERN_ERR "pm_qos_update_request() called for unknown object\n");
> +		return;
> +	}
> +
>  	o = pm_qos_array[pm_qos_req->pm_qos_class];
>  
>  	if (new_value == PM_QOS_DEFAULT_VALUE)
> @@ -290,9 +293,14 @@ void pm_qos_remove_request(struct pm_qos_request_list *pm_qos_req)
>  		return;
>  		/* silent return to keep pcm code cleaner */
>  
> +	if (!pm_qos_request_active(pm_qos_req)) {
> +		WARN(1, KERN_ERR "pm_qos_remove_request() called for unknown object\n");
> +		return;
> +	}
> +
>  	o = pm_qos_array[pm_qos_req->pm_qos_class];
>  	update_target(o, &pm_qos_req->list, 1, PM_QOS_DEFAULT_VALUE);
> -	kfree(pm_qos_req);
> +	memset(pm_qos_req, 0, sizeof(*pm_qos_req));
>  }
>  EXPORT_SYMBOL_GPL(pm_qos_remove_request);
>  
> @@ -340,8 +348,12 @@ static int pm_qos_power_open(struct inode *inode, struct file *filp)
>  
>  	pm_qos_class = find_pm_qos_object_by_minor(iminor(inode));
>  	if (pm_qos_class >= 0) {
> -		filp->private_data = (void *) pm_qos_add_request(pm_qos_class,
> -				PM_QOS_DEFAULT_VALUE);
> +		struct pm_qos_request_list *req = kzalloc(GFP_KERNEL, sizeof(*req));
> +		if (!req)
> +			return -ENOMEM;
> +
> +		pm_qos_add_request(req, pm_qos_class, PM_QOS_DEFAULT_VALUE);
> +		filp->private_data = req;
>  
>  		if (filp->private_data)
>  			return 0;
> @@ -353,8 +365,9 @@ static int pm_qos_power_release(struct inode *inode, struct file *filp)
>  {
>  	struct pm_qos_request_list *req;
>  
> -	req = (struct pm_qos_request_list *)filp->private_data;
> +	req = filp->private_data;
>  	pm_qos_remove_request(req);
> +	kfree(req);
>  
>  	return 0;
>  }
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 303ac04..a3b2a64 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -451,13 +451,11 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
>  	snd_pcm_timer_resolution_change(substream);
>  	runtime->status->state = SNDRV_PCM_STATE_SETUP;
>  
> -	if (substream->latency_pm_qos_req) {
> -		pm_qos_remove_request(substream->latency_pm_qos_req);
> -		substream->latency_pm_qos_req = NULL;
> -	}
> +	if (pm_qos_request_active(&substream->latency_pm_qos_req))
> +		pm_qos_remove_request(&substream->latency_pm_qos_req);
>  	if ((usecs = period_to_usecs(runtime)) >= 0)
> -		substream->latency_pm_qos_req = pm_qos_add_request(
> -					PM_QOS_CPU_DMA_LATENCY, usecs);
> +		pm_qos_add_request(&substream->latency_pm_qos_req,
> +				   PM_QOS_CPU_DMA_LATENCY, usecs);
>  	return 0;
>   _error:
>  	/* hardware might be unuseable from this time,
> @@ -512,8 +510,7 @@ static int snd_pcm_hw_free(struct snd_pcm_substream *substream)
>  	if (substream->ops->hw_free)
>  		result = substream->ops->hw_free(substream);
>  	runtime->status->state = SNDRV_PCM_STATE_OPEN;
> -	pm_qos_remove_request(substream->latency_pm_qos_req);
> -	substream->latency_pm_qos_req = NULL;
> +	pm_qos_remove_request(&substream->latency_pm_qos_req);
>  	return result;
>  }
>  
> -- 
> 1.6.4.2
> 
> 
> 

^ permalink raw reply

* Re: PATCH: uninitialized memory access in tcp_parse_options
From: David Miller @ 2010-06-29  4:22 UTC (permalink / raw)
  To: eric.dumazet; +Cc: mathieu.lacage, netdev
In-Reply-To: <1277531884.2481.22.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 26 Jun 2010 07:58:04 +0200

> If you want to avoid valgrind false positive at this point, without
> introducing bug for other tcp_parse_options() callers, a better fix
> would be following patch.
> 
> Thanks
> 
> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> index 794c2e1..4e758ac 100644
> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -520,14 +520,13 @@ struct sock *tcp_check_req(struct sock *sk, struct sk_buff *skb,
>  			   struct request_sock *req,
>  			   struct request_sock **prev)
>  {
> -	struct tcp_options_received tmp_opt;
> +	struct tcp_options_received tmp_opt = {0};
>  	u8 *hash_location;
>  	struct sock *child;

That's a 28 byte memset() in the connect fast-path.  We shouldn't eat this
just to placate a valgrind miscue. :-)



^ permalink raw reply

* Re: [PATCH] cpmac: do not leak struct net_device on phy_connect errors
From: David Miller @ 2010-06-29  4:20 UTC (permalink / raw)
  To: florian; +Cc: netdev
In-Reply-To: <201006271905.26506.florian@openwrt.org>

From: Florian Fainelli <florian@openwrt.org>
Date: Sun, 27 Jun 2010 19:05:24 +0200

> Forgot to mention that this is relevant for -stable and current net-next-2.6

I put it into net-2.6 and since you CC:'d stable it should show up there
automatically.


^ permalink raw reply

* Re: [RFC net-2.6 PATCH] ixgbe: need to purge skb qdisc lists when changing real_num_tx_queues
From: David Miller @ 2010-06-29  4:19 UTC (permalink / raw)
  To: john.r.fastabend; +Cc: netdev, jeffrey.t.kirsher
In-Reply-To: <20100621052520.2417.65984.stgit@jf-dev1-dcblab>

From: John Fastabend <john.r.fastabend@intel.com>
Date: Sun, 20 Jun 2010 22:25:20 -0700

> This patch exports dev_deactivate() symbol.
> 
> The qdisc needs to be purged when real_num_tx_queues is
> changed so that skbs do not hit ndo_start_xmit with
> queue_mappings corresponding to tx_rings already freed.
> 
> real_num_tx_queues is changed when dynamically enabling or
> disabling DCB or FCoE.  To purge the qdisc use dev_deactivate().
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>

Whatever it is that needs to happen when real_num_tx_queues changes
outght to be transparent to drivers.

Therefore we should hide all of these details behind a well named
interface drivers can call when they need to change real_num_tx_queues.

^ permalink raw reply

* Re: [PATCH net-next-2.6] ipv4: sysctl to block responding on down interface
From: David Miller @ 2010-06-29  3:01 UTC (permalink / raw)
  To: joakim.tjernlund; +Cc: eric.dumazet, netdev, shemminger
In-Reply-To: <OFA9AE9CDE.69224382-ONC1257750.007F44EA-C1257750.0081210A@transmode.se>

From: Joakim Tjernlund <joakim.tjernlund@transmode.se>
Date: Tue, 29 Jun 2010 01:30:26 +0200

> This is an strict interpretation of the weak host model and does not
> answer my questions. Mind to elaborate why such a strict view and
> what is gained by answering on an IP address which has been "downed"?

IP addresses are never "downed" just as your default route is not
"downed" when you take down an interface.

Rather, hosts are configured with an IP address and when they are so
configured they respond to it and can generate local application
sourced packets with that IP address as a source.

And what this means is that even in situations where hosts are
slightly mis-configured communication between them can still be
possible.  That's the goal of the weak host model, to get a host
respond to IP datagrams in every situation where such an act is
plausible.

All of the design decisions we've made in the networking in this area
are meant to increase the likelyhood of successful communication
between two hosts.

And in the 10+ years this behavior has existed, I know for sure that
people have ended up with a working networking because of the way we
do things.

So from that perspective it doesn't matter one iota what you or any
other particular entity wish things to be, since 10+ years of having
this behavior is ingrained enough that changing it is guarenteed to
break someone's setup so we absolutely can't do it.

This topic comes up at least once every few months, therefore someone
should post a FAQ somewhere because it's tiring to explain over and
over again why this is a good design decision and why the default
behavior is never going to change.

The RFCs allow both models equally, and just because many other
system does things the other way doesn't make it any better or more
valid than what Linux is doing.

^ permalink raw reply

* [PATCH linux-2.6.35-rc3] micrel phy driver - updated(1)
From: Choi, David @ 2010-06-29  1:23 UTC (permalink / raw)
  To: netdev; +Cc: bhutchings


Hello all:

This patch fixes what Ben mentioned, namely duplicated ids.

From: David J. Choi <david.choi@micrel.com>

Body of the explanation: This patch has changes as followings;
 -support the interrupt from phy devices from Micrel Inc.
 -support more phy devices, ks8737, ks8721, ks8041, ks8051 from Micrel.
 -remove vsc8201 because this device was used only internal test at Micrel.

Signed-off-by: David J. Choi <david.choi@micrel.com>

---
--- ./linux-2.6.35-rc3/drivers/net/phy/micrel.c.orig	2010-06-28 17:55:30.000000000 -0700
+++ ./linux-2.6.35-rc3/drivers/net/phy/micrel.c	2010-06-28 18:06:00.000000000 -0700
@@ -12,7 +12,8 @@
  * Free Software Foundation;  either version 2 of the  License, or (at your
  * option) any later version.
  *
- * Support : ksz9021 , vsc8201, ks8001
+ * Support : ksz9021 1000/100/10 phy from Micrel
+ *		ks8001, ks8737, ks8721, ks8041, ks8051 100/10 phy
  */
 
 #include <linux/kernel.h>
@@ -20,37 +21,146 @@
 #include <linux/phy.h>
 
 #define	PHY_ID_KSZ9021			0x00221611
-#define	PHY_ID_VSC8201			0x000FC413
+#define	PHY_ID_KS8737			0x00221720
+#define	PHY_ID_KS8041			0x00221510
+#define	PHY_ID_KS8051			0x00221550
+/* both for ks8001 Rev. A/B, and for ks8721 Rev 3. */
 #define	PHY_ID_KS8001			0x0022161A
 
+/* general Interrupt control/status reg in vendor specific block. */
+#define MII_KSZPHY_INTCS			0x1B
+#define	KSZPHY_INTCS_JABBER			(1 << 15)
+#define	KSZPHY_INTCS_RECEIVE_ERR		(1 << 14)
+#define	KSZPHY_INTCS_PAGE_RECEIVE		(1 << 13)
+#define	KSZPHY_INTCS_PARELLEL			(1 << 12)
+#define	KSZPHY_INTCS_LINK_PARTNER_ACK		(1 << 11)
+#define	KSZPHY_INTCS_LINK_DOWN			(1 << 10)
+#define	KSZPHY_INTCS_REMOTE_FAULT		(1 << 9)
+#define	KSZPHY_INTCS_LINK_UP			(1 << 8)
+#define	KSZPHY_INTCS_ALL			(KSZPHY_INTCS_LINK_UP |\
+						KSZPHY_INTCS_LINK_DOWN)
+
+/* general PHY control reg in vendor specific block. */
+#define	MII_KSZPHY_CTRL			0x1F
+/* bitmap of PHY register to set interrupt mode */
+#define KSZPHY_CTRL_INT_ACTIVE_HIGH		(1 << 9)
+#define KSZ9021_CTRL_INT_ACTIVE_HIGH		(1 << 14)
+#define KS8737_CTRL_INT_ACTIVE_HIGH		(1 << 14)
+
+static int kszphy_ack_interrupt(struct phy_device *phydev)
+{
+	/* bit[7..0] int status, which is a read and clear register. */
+	int rc;
+
+	rc = phy_read(phydev, MII_KSZPHY_INTCS);
+
+	return (rc < 0) ? rc : 0;
+}
+
+static int kszphy_set_interrupt(struct phy_device *phydev)
+{
+	int temp;
+	temp = (PHY_INTERRUPT_ENABLED == phydev->interrupts) ?
+		KSZPHY_INTCS_ALL : 0;
+	return phy_write(phydev, MII_KSZPHY_INTCS, temp);
+}
+
+static int kszphy_config_intr(struct phy_device *phydev)
+{
+	int temp, rc;
+
+	/* set the interrupt pin active low */
+	temp = phy_read(phydev, MII_KSZPHY_CTRL);
+	temp &= ~KSZPHY_CTRL_INT_ACTIVE_HIGH;
+	phy_write(phydev, MII_KSZPHY_CTRL, temp);
+	rc = kszphy_set_interrupt(phydev);
+	return rc < 0 ? rc : 0;
+}
+
+static int ksz9021_config_intr(struct phy_device *phydev)
+{
+	int temp, rc;
+
+	/* set the interrupt pin active low */
+	temp = phy_read(phydev, MII_KSZPHY_CTRL);
+	temp &= ~KSZ9021_CTRL_INT_ACTIVE_HIGH;
+	phy_write(phydev, MII_KSZPHY_CTRL, temp);
+	rc = kszphy_set_interrupt(phydev);
+	return rc < 0 ? rc : 0;
+}
+
+static int ks8737_config_intr(struct phy_device *phydev)
+{
+	int temp, rc;
+
+	/* set the interrupt pin active low */
+	temp = phy_read(phydev, MII_KSZPHY_CTRL);
+	temp &= ~KS8737_CTRL_INT_ACTIVE_HIGH;
+	phy_write(phydev, MII_KSZPHY_CTRL, temp);
+	rc = kszphy_set_interrupt(phydev);
+	return rc < 0 ? rc : 0;
+}
 
 static int kszphy_config_init(struct phy_device *phydev)
 {
 	return 0;
 }
 
+static struct phy_driver ks8737_driver = {
+	.phy_id		= PHY_ID_KS8737,
+	.phy_id_mask	= 0x00fffff0,
+	.name		= "Micrel KS8737",
+	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
+	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.config_init	= kszphy_config_init,
+	.config_aneg	= genphy_config_aneg,
+	.read_status	= genphy_read_status,
+	.ack_interrupt	= kszphy_ack_interrupt,
+	.config_intr	= ks8737_config_intr,
+	.driver		= { .owner = THIS_MODULE,},
+};
+
+static struct phy_driver ks8041_driver = {
+	.phy_id		= PHY_ID_KS8041,
+	.phy_id_mask	= 0x00fffff0,
+	.name		= "Micrel KS8041",
+	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause
+				| SUPPORTED_Asym_Pause),
+	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
+	.config_init	= kszphy_config_init,
+	.config_aneg	= genphy_config_aneg,
+	.read_status	= genphy_read_status,
+	.ack_interrupt	= kszphy_ack_interrupt,
+	.config_intr	= kszphy_config_intr,
+	.driver		= { .owner = THIS_MODULE,},
+};
 
-static struct phy_driver ks8001_driver = {
-	.phy_id		= PHY_ID_KS8001,
-	.name		= "Micrel KS8001",
+static struct phy_driver ks8051_driver = {
+	.phy_id		= PHY_ID_KS8051,
 	.phy_id_mask	= 0x00fffff0,
-	.features	= PHY_BASIC_FEATURES,
-	.flags		= PHY_POLL,
+	.name		= "Micrel KS8051",
+	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause
+				| SUPPORTED_Asym_Pause),
+	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
+	.ack_interrupt	= kszphy_ack_interrupt,
+	.config_intr	= kszphy_config_intr,
 	.driver		= { .owner = THIS_MODULE,},
 };
 
-static struct phy_driver vsc8201_driver = {
-	.phy_id		= PHY_ID_VSC8201,
-	.name		= "Micrel VSC8201",
+static struct phy_driver ks8001_driver = {
+	.phy_id		= PHY_ID_KS8001,
+	.name		= "Micrel KS8001 or KS8721",
 	.phy_id_mask	= 0x00fffff0,
-	.features	= PHY_BASIC_FEATURES,
-	.flags		= PHY_POLL,
+	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause),
+	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
+	.ack_interrupt	= kszphy_ack_interrupt,
+	.config_intr	= kszphy_config_intr,
 	.driver		= { .owner = THIS_MODULE,},
 };
 
@@ -58,11 +168,14 @@ static struct phy_driver ksz9021_driver 
 	.phy_id		= PHY_ID_KSZ9021,
 	.phy_id_mask	= 0x000fff10,
 	.name		= "Micrel KSZ9021 Gigabit PHY",
-	.features	= PHY_GBIT_FEATURES | SUPPORTED_Pause,
-	.flags		= PHY_POLL,
+	.features	= (PHY_GBIT_FEATURES | SUPPORTED_Pause
+				| SUPPORTED_Asym_Pause),
+	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
+	.ack_interrupt	= kszphy_ack_interrupt,
+	.config_intr	= ksz9021_config_intr,
 	.driver		= { .owner = THIS_MODULE, },
 };
 
@@ -73,17 +186,29 @@ static int __init ksphy_init(void)
 	ret = phy_driver_register(&ks8001_driver);
 	if (ret)
 		goto err1;
-	ret = phy_driver_register(&vsc8201_driver);
+
+	ret = phy_driver_register(&ksz9021_driver);
 	if (ret)
 		goto err2;
 
-	ret = phy_driver_register(&ksz9021_driver);
+	ret = phy_driver_register(&ks8737_driver);
 	if (ret)
 		goto err3;
+	ret = phy_driver_register(&ks8041_driver);
+	if (ret)
+		goto err4;
+	ret = phy_driver_register(&ks8051_driver);
+	if (ret)
+		goto err5;
+
 	return 0;
 
+err5:
+	phy_driver_unregister(&ks8041_driver);
+err4:
+	phy_driver_unregister(&ks8737_driver);
 err3:
-	phy_driver_unregister(&vsc8201_driver);
+	phy_driver_unregister(&ksz9021_driver);
 err2:
 	phy_driver_unregister(&ks8001_driver);
 err1:
@@ -93,8 +218,10 @@ err1:
 static void __exit ksphy_exit(void)
 {
 	phy_driver_unregister(&ks8001_driver);
-	phy_driver_unregister(&vsc8201_driver);
+	phy_driver_unregister(&ks8737_driver);
 	phy_driver_unregister(&ksz9021_driver);
+	phy_driver_unregister(&ks8041_driver);
+	phy_driver_unregister(&ks8051_driver);
 }
 
 module_init(ksphy_init);
@@ -106,8 +233,10 @@ MODULE_LICENSE("GPL");
 
 static struct mdio_device_id micrel_tbl[] = {
 	{ PHY_ID_KSZ9021, 0x000fff10 },
-	{ PHY_ID_VSC8201, 0x00fffff0 },
 	{ PHY_ID_KS8001, 0x00fffff0 },
+	{ PHY_ID_KS8737, 0x00fffff0 },
+	{ PHY_ID_KS8041, 0x00fffff0 },
+	{ PHY_ID_KS8051, 0x00fffff0 },
 	{ }
 };
 

---

^ permalink raw reply

* RE: [REGRESSION] e1000e stopped working
From: Allan, Bruce W @ 2010-06-29  1:09 UTC (permalink / raw)
  To: Maxim Levitsky; +Cc: netdev@vger.kernel.org
In-Reply-To: <1277745247.12841.1.camel@localhost.localdomain>

On Monday, June 28, 2010 10:14 AM, Maxim Levitsky wrote:
> On Mon, 2010-06-28 at 10:04 -0700, Allan, Bruce W wrote:
>> On Sunday, June 27, 2010 10:47 AM, Maxim Levitsky wrote:
>>> On Sun, 2010-06-27 at 20:43 +0300, Maxim Levitsky wrote:
>>>> On Sun, 2010-06-27 at 20:29 +0300, Maxim Levitsky wrote:
>>>>> On Sun, 2010-06-27 at 20:27 +0300, Maxim Levitsky wrote:
>>>>>> Just that,
>>>>>> 
>>>>>> It doesn't receive anything from my internet router during DHCP.
>>>>>> 
>>>>>> 
>>>>>> 00:19.0 Ethernet controller [0200]: Intel Corporation 82566DC
>>>>>> 	Gigabit Network Connection [8086:104b] (rev 02) Subsystem: Intel
>>>>>> 	Corporation Device [8086:0001] Control: I/O+ Mem+ BusMaster+
>>>>>> 	SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B-
>>>>>> 	DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast
>>>>>> 	>TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx- 	Latency: 0
>>>>>> 	Interrupt: pin A routed to IRQ 47 Region 0: Memory at 50300000
>>>>>> 	(32-bit, non-prefetchable) [size=128K] Region 1: Memory at
>>>>>> 	50324000 (32-bit, non-prefetchable) [size=4K] Region 2: I/O
>>>>>> 		ports at 30e0 [size=32] Capabilities: [c8] Power Management
>>>>>> 		version 2 Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA
>>>>>> 	PME(D0+,D1-,D2-,D3hot+,D3cold+) Status: D0 PME-Enable- DSel=0
>>>>>> 		DScale=1 PME- Capabilities: [d0] Message Signalled Interrupts:
>>>>>> 	Mask- 64bit+ Queue=0/0 Enable+ Address: 00000000fee0100c  Data:
>>>>>> 	41c9 Kernel driver in use: e1000e Kernel modules: e1000e
>>>>>> 
>>>>>> I use vanilla tree, commit
>>>>>> bf2937695fe2330bfd8933a2310e7bdd2581dc2e 
>>>>>> 
>>>>>> 
>>>>>> Best regards,
>>>>>> 	Maxim Levitsky
>>>>>> 
>>>>> 
>>>>> It appears to work now after reboot.
>>>>> Will keep a look for this.
>>>>> 
>>>>> Disregard for now.
>>>> 
>>>> 
>>>> Just s2ram cycle, problem is back.
>>>> Did full reboot (power off then on), same thing card doesn't
>>>> work... 
>>>> 
>>> 
>>> Yep, s2ram sometimes 'fixes', sometimes breaks the card.
>>> Something got broken in device initialization path.
>>> 
>>> Best regards,
>>>  	Maxim Levitsky
>> 
>> What distro are you using?  If RedHat, since you are using DHCP will
>> you please try putting a "LINKDELAY=10" in the
>> /etc/sysconfig/network-scripts/ifcfg-ethX config file.  
>> 
> I use ubuntu 9.10
> 
>> Is there anything in the system log that might help narrow down the
>> issue? 
> 
> Nothing, really nothing.
> It seems to detect link, dhcp client sends requests, but doesn't
> recieve a thing (even tried promisc mode - doesn't help)
> 
> 
> 
> Best regards,
> 	Maxim Levitsky

Since you say this is a regression, when did this last work for you without this problem, i.e. which distro, which kernel?

I have been unable to reproduce similar behavior.

Thanks,
Bruce.

^ permalink raw reply

* Re: [PATCH 4/9] cxgb4vf: Add code to provision T4 PCI-E SR-IOV Virtual Functions with hardware resources
From: Simon Horman @ 2010-06-29  0:51 UTC (permalink / raw)
  To: Casey Leedom; +Cc: netdev
In-Reply-To: <201006280955.07352.leedom@chelsio.com>

On Mon, Jun 28, 2010 at 09:55:07AM -0700, Casey Leedom wrote:
> | From: Simon Horman <horms@verge.net.au>
> | Date: Saturday, June 26, 2010 06:37 am
> | 
> | I wonder if it would be cleaner to move the guts of the last hunk
> | into a function (e.g. adap_init_sriov()) and have that be a dummy
> | function in the case that CONFIG_PCI_IOV in the first hunk is not set.
> 
>   I have no objection to this.  I'd like to do minor tuning work like
> that as a follow on rather than respin the patch.  But, as I said in my
> submission, I'm not familiar with this process so if making changes like
> the above is required I'll definitely jump on it.  I don't know what
> issues are "critical show stoppers" and which ones are "nice to have"
> once things are in place.

I don't regard my suggestion as merge-critical.

^ 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