* Re: Code paths setting gso_size
From: Eric Dumazet @ 2010-03-13 12:09 UTC (permalink / raw)
To: William Allen Simpson
Cc: Linux Kernel Network Developers, Dan Carpenter, David Miller,
Simon Horman
In-Reply-To: <4B9B7591.9080706@gmail.com>
Le samedi 13 mars 2010 à 06:22 -0500, William Allen Simpson a écrit :
> This is a new thread dedicated to a specific topic, spawned by an
> earlier discussion. I've CC'd only those that participated recently.
>
> I've written:
> # In this particular instance, I suggest that you take a look at all the
> # places that gso_size is set, and cross index with all the code paths that
> # place these TCP headers onto the txq without a check of doff -- as I did!
> #
> # I'll specifically mention the tun and virtio_net devices, but I'm also
> # particularly concerned with af_packet.c and skbuff.c -- and the general
> # problem with inet_lro.c, too.
> #
> # Amazingly enough, folks sometimes use Linux for routers....
> #
>
> Eric (and David) have written:
> # Only *locally* generated trafic by linux kernel can enter this path.
> #
>
> So, let us begin with the tun device, and work our way through the others.
>
Thats a really good idea William, I'll start another thread to study all
the paths setting skb->len to a possible wrong value. But I'll do this
in my own time, and if I find something, I'll post a patch.
Sure we'll find some bugs by this studies, since in average we find 100
bugs per month, and introduce 90 new ones, thanks God for this.
^ permalink raw reply
* Re: [PATCH] fix PHY polling system blocking
From: Stefani Seibold @ 2010-03-13 11:54 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, netdev
In-Reply-To: <20100312144248.ade8b700.akpm@linux-foundation.org>
Am Freitag, den 12.03.2010, 14:42 -0800 schrieb Andrew Morton:
> On Sat, 06 Mar 2010 17:50:58 +0100
> Stefani Seibold <stefani@seibold.net> wrote:
>
> > This patch fix the PHY poller, which can block the whole system. On a
> > Freescale PPC 834x this result in a delay of 450 us due the slow
> > communication with the PHY chip.
> >
> > For PHY chips without interrupts, the status of the ethernet will be
> > polled every 2 sec. The poll function will read some register of the MII
> > PHY. The time between the sending the MII_READ_COMMAND and receiving the
> > result is more the 100 us on a PPC 834x.
> >
> > The patch modifies the poller a lit bit. Only a link status state change
> > will result in a successive detection of the connection type. The poll
> > cycle on the other hand will be increased to every seconds.
>
> You didn't tell us how many seconds. That would be important?
>
The old implementation would poll the PHC every 2 seconds, the new one
once per second.
> > Together this patch will prevent a blocking of nearly 400 us every two
> > seconds of the whole system on a PPC 834x.
> >
>
> I can't say that I really understand what you did - what functionality
> did we lose?
>
There is not real drawback, only the detection of a connection type
change without unplugging the cable will not work. But this is more an
esoteric use case.
> Would it not be better to extend the phy state machine a bit? When we
> detect the link status change, issue the MII command then arm the
> delayed-work timer to expire in a millisecond, then go in next time and
> read the result?
>
> That might require fancy locking to prevent other threads of control
> from going in and mucking up the MII state. Possibly leave phydev_lock
> held across that millisecond to keep other people away?
>
You are right, that would be the best solution. But i am currently busy,
so this a fast interim solution ;-) It was also my approach, because it
the PHY communication in the most cases very slow. Maybe i will do this
in the near future.
> >
> > ...
> >
> > diff -u -N -r -p linux-2.6.33.orig/drivers/net/phy/phy.c linux-2.6.33/drivers/net//phy/phy.c
> > --- linux-2.6.33.orig/drivers/net/phy/phy.c 2010-02-24 19:52:17.000000000 +0100
> > +++ linux-2.6.33/drivers/net//phy/phy.c 2010-02-28 22:53:14.725464101 +0100
>
> erp, your weird patch headers ("//") broke my seven-year-old script.
> But I fixed it!
>
Sorry, my fault. But now you have a better script.
> > @@ -871,9 +871,8 @@ void phy_state_machine(struct work_struc
> > case PHY_RUNNING:
> > /* Only register a CHANGE if we are
> > * polling */
> > - if (PHY_POLL == phydev->irq)
> > - phydev->state = PHY_CHANGELINK;
> > - break;
> > + if (PHY_POLL != phydev->irq)
> > + break;
> > case PHY_CHANGELINK:
> > err = phy_read_status(phydev);
> >
> > diff -u -N -r -p linux-2.6.33.orig/drivers/net/phy/phy_device.c linux-2.6.33/drivers/net//phy/phy_device.c
> > --- linux-2.6.33.orig/drivers/net/phy/phy_device.c 2010-02-24 19:52:17.000000000 +0100
> > +++ linux-2.6.33/drivers/net//phy/phy_device.c 2010-02-28 22:53:14.726464145 +0100
> > @@ -161,7 +161,7 @@ struct phy_device* phy_device_create(str
> > dev->speed = 0;
> > dev->duplex = -1;
> > dev->pause = dev->asym_pause = 0;
> > - dev->link = 1;
> > + dev->link = 0;
> > dev->interface = PHY_INTERFACE_MODE_GMII;
> >
> > dev->autoneg = AUTONEG_ENABLE;
> > @@ -694,10 +694,16 @@ int genphy_update_link(struct phy_device
> > if (status < 0)
> > return status;
> >
> > - if ((status & BMSR_LSTATUS) == 0)
> > + if ((status & BMSR_LSTATUS) == 0) {
> > + if (phydev->link==0)
> > + return 1;
> > phydev->link = 0;
> > - else
> > + }
> > + else {
> > + if (phydev->link==1)
> > + return 1;
> > phydev->link = 1;
> > + }
>
> Please don't invent new coding styles. scripts/checkpatch.pl is here
> to help.
>
>
Will be fixed!
^ permalink raw reply
* Re: [PATCH v4 2/7] net: remove old tcp_optlen function
From: Eric Dumazet @ 2010-03-13 11:24 UTC (permalink / raw)
To: William Allen Simpson
Cc: Dan Carpenter, Linus Torvalds, Andrew Morton,
Linux Kernel Developers, Linux Kernel Network Developers,
David Miller, Michael Chan, Simon Horman
In-Reply-To: <4B9B7312.80301@gmail.com>
Le samedi 13 mars 2010 à 06:12 -0500, William Allen Simpson a écrit :
> Since you agree with David, perhaps you could kindly point out exactly
> how your statement is true? Proof by assertion is generally not good
> argumentation.
>
You have to bring proof of your claims, not the reverse.
You started the whole game, adding whole universe into this minor netdev
discussion for an obscure and yet to be aknowledged bug, not me.
Therefore, I wont continue this trolling. You apparently want some
public exposure I have no idea why.
Have a good week end.
^ permalink raw reply
* Code paths setting gso_size
From: William Allen Simpson @ 2010-03-13 11:22 UTC (permalink / raw)
To: Linux Kernel Developers, Linux Kernel Network Developers
Cc: Eric Dumazet, Dan Carpenter, David Miller, Simon Horman
This is a new thread dedicated to a specific topic, spawned by an
earlier discussion. I've CC'd only those that participated recently.
I've written:
# In this particular instance, I suggest that you take a look at all the
# places that gso_size is set, and cross index with all the code paths that
# place these TCP headers onto the txq without a check of doff -- as I did!
#
# I'll specifically mention the tun and virtio_net devices, but I'm also
# particularly concerned with af_packet.c and skbuff.c -- and the general
# problem with inet_lro.c, too.
#
# Amazingly enough, folks sometimes use Linux for routers....
#
Eric (and David) have written:
# Only *locally* generated trafic by linux kernel can enter this path.
#
So, let us begin with the tun device, and work our way through the others.
^ permalink raw reply
* Re: [PATCH v4 2/7] net: remove old tcp_optlen function
From: William Allen Simpson @ 2010-03-13 11:12 UTC (permalink / raw)
To: Eric Dumazet
Cc: Dan Carpenter, Linus Torvalds, Andrew Morton,
Linux Kernel Developers, Linux Kernel Network Developers,
David Miller, Michael Chan, Simon Horman
In-Reply-To: <1268471488.2947.26.camel@edumazet-laptop>
On 3/13/10 4:11 AM, Eric Dumazet wrote:
> David already pointed out fact that this code path is not used in
> forwardind / routing path. Your assumptions are clearly wrong.
>
> Can you sit down and understand this difference ?
>
> Only *locally* generated trafic by linux kernel can enter this path.
>
Since you agree with David, perhaps you could kindly point out exactly
how your statement is true? Proof by assertion is generally not good
argumentation. I'll start a new thread for you to discuss.
More importantly, how is this relevant to this patch?
This patch removes a seldom used function that generates bad code, and
MUST NOT be used elsewhere in the code base -- an attractive nuisance,
speaking in legal terms.
This patch is also more elegant, and generates faster code.
Are you saying this patch is incorrect in any way?
Please point to the bad line of code (not David's comments or diatribe).
Certainly, I'll be happy to correct it!
> And if a bug in linux core network stack can feed any driver a buggy
> skb, bad things can happen, even if a driver is perfect.
>
Certainly. But a driver cannot depend on the entirety of a large
project being perfect. It *should* be perfect in and of itself!
> Please point out _this_ bug _if_ it really exists, so that we can
> correct this bug instead of hiding it in one thousand of drivers.
>
Again, I'll start a new thread for your edification.
> Your attacks make no sense, you know nothing about linux kernel
> internals and assume it was written like other projects you were
> involved to.
>
I've made no "attacks". The above is a personal /ad hominem/ attack,
evidently the raison d'etre for Linux email lists. :-(
Yes, I've been involved in many other projects, and am arguably the
most widely experienced TCP/IP implementer in the world. I've been
programming in C since 1977, and my first TCP/IP implementation was
started in 1979 (in assembly). Probably before you were born.
Why are you so threatened by that?
Why are you insulting and trying to drive me away?
Why, oh why, is there such a lack of community spirit here?!?!
^ permalink raw reply
* ipv6: Send netlink notification when DAD fails
From: Herbert Xu @ 2010-03-13 11:11 UTC (permalink / raw)
To: David S. Miller, netdev
Hi:
ipv6: Send netlink notification when DAD fails
If we are managing IPv6 addresses using DHCP, it would be nice
for user-space to be notified if an address configured through
DHCP fails DAD. Otherwise user-space would have to poll to see
whether DAD succeeds.
This patch uses the existing notification mechanism and simply
hooks it into the DAD failure code path.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 88fd8c5..01d142e 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1380,6 +1380,8 @@ static void addrconf_dad_stop(struct inet6_ifaddr *ifp, int dad_failed)
if (dad_failed)
ifp->flags |= IFA_F_DADFAILED;
spin_unlock_bh(&ifp->lock);
+ if (dad_failed)
+ ipv6_ifa_notify(0, ifp);
in6_ifa_put(ifp);
#ifdef CONFIG_IPV6_PRIVACY
} else if (ifp->flags&IFA_F_TEMPORARY) {
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 related
* Re: [PATCH v4 2/7] net: remove old tcp_optlen function
From: Eric Dumazet @ 2010-03-13 9:11 UTC (permalink / raw)
To: William Allen Simpson
Cc: Dan Carpenter, Linus Torvalds, Andrew Morton,
Linux Kernel Developers, Linux Kernel Network Developers,
David Miller, Michael Chan, Simon Horman
In-Reply-To: <4B9AC8C5.2010300@gmail.com>
Le vendredi 12 mars 2010 à 18:05 -0500, William Allen Simpson a écrit :
> In this particular instance, I suggest that you take a look at all the
> places that gso_size is set, and cross index with all the code paths that
> place these TCP headers onto the txq without a check of doff -- as I did!
>
> I'll specifically mention the tun and virtio_net devices, but I'm also
> particularly concerned with af_packet.c and skbuff.c -- and the general
> problem with inet_lro.c, too.
>
> Amazingly enough, folks sometimes use Linux for routers....
> --
David already pointed out fact that this code path is not used in
forwardind / routing path. Your assumptions are clearly wrong.
Can you sit down and understand this difference ?
Only *locally* generated trafic by linux kernel can enter this path.
And if a bug in linux core network stack can feed any driver a buggy
skb, bad things can happen, even if a driver is perfect.
Please point out _this_ bug _if_ it really exists, so that we can
correct this bug instead of hiding it in one thousand of drivers.
Your attacks make no sense, you know nothing about linux kernel
internals and assume it was written like other projects you were
involved to.
^ permalink raw reply
* [PATCH] drivers/net/tg3.c: change the field used with the TG3_FLAG_10_100_ONLY constant
From: Julia Lawall @ 2010-03-13 9:01 UTC (permalink / raw)
To: Matt Carlson, Michael Chan, netdev, linux-kernel, kernel-janitors
From: Julia Lawall <julia@diku.dk>
The constant TG3_FLAG_10_100_ONLY should be used with the tg3_flags field,
not the tg3_flags2 field, as done elsewhere in the same file.
Signed-off-by: Julia Lawall <julia@diku.dk>
---
drivers/net/tg3.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 0fa7688..fa8fb1a 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -9776,7 +9776,7 @@ static int tg3_set_settings(struct net_device *dev, struct ethtool_cmd *cmd)
ADVERTISED_Pause |
ADVERTISED_Asym_Pause;
- if (!(tp->tg3_flags2 & TG3_FLAG_10_100_ONLY))
+ if (!(tp->tg3_flags & TG3_FLAG_10_100_ONLY))
mask |= ADVERTISED_1000baseT_Half |
ADVERTISED_1000baseT_Full;
^ permalink raw reply related
* Re: 2.6.34-rc1: rcu lockdep bug?
From: Américo Wang @ 2010-03-13 5:33 UTC (permalink / raw)
To: Eric Dumazet
Cc: Américo Wang, David Miller, paulmck, peterz, linux-kernel,
netdev
In-Reply-To: <1268401058.3141.9.camel@edumazet-laptop>
On Fri, Mar 12, 2010 at 02:37:38PM +0100, Eric Dumazet wrote:
>Le vendredi 12 mars 2010 à 21:11 +0800, Américo Wang a écrit :
>
>> Oh, but lockdep complains about rcu_read_lock(), it said
>> rcu_read_lock() can't be used in softirq context.
>>
>> Am I missing something?
>
>Well, lockdep might be dumb, I dont know...
>
>I suggest you read rcu_read_lock_bh kernel doc :
>
>/**
> * rcu_read_lock_bh - mark the beginning of a softirq-only RCU critical
>section
> *
> * This is equivalent of rcu_read_lock(), but to be used when updates
> * are being done using call_rcu_bh(). Since call_rcu_bh() callbacks
> * consider completion of a softirq handler to be a quiescent state,
> * a process in RCU read-side critical section must be protected by
> * disabling softirqs. Read-side critical sections in interrupt context
> * can use just rcu_read_lock().
> *
> */
>
>
>Last sentence being perfect :
>
>Read-side critical sections in interrupt context
>can use just rcu_read_lock().
>
Yeah, right, then it is more likely to be a bug of rcu lockdep.
Paul is looking at it.
Thanks!
^ permalink raw reply
* Re: 2.6.34-rc1: rcu lockdep bug?
From: Américo Wang @ 2010-03-13 5:31 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Américo Wang, Eric Dumazet, David Miller, peterz,
linux-kernel, netdev
In-Reply-To: <20100312220319.GC7824@linux.vnet.ibm.com>
On Fri, Mar 12, 2010 at 02:03:19PM -0800, Paul E. McKenney wrote:
>On Fri, Mar 12, 2010 at 09:11:02PM +0800, Américo Wang wrote:
>> On Fri, Mar 12, 2010 at 7:11 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> > Le vendredi 12 mars 2010 à 16:59 +0800, Américo Wang a écrit :
>> >> On Fri, Mar 12, 2010 at 4:07 PM, David Miller <davem@davemloft.net> wrote:
>> >> > From: Américo Wang <xiyou.wangcong@gmail.com>
>> >> > Date: Fri, 12 Mar 2010 15:56:03 +0800
>> >> >
>> >> >> Ok, after decoding the lockdep output, it looks like that
>> >> >> netif_receive_skb() should call rcu_read_lock_bh() instead of rcu_read_lock()?
>> >> >> But I don't know if all callers of netif_receive_skb() are in softirq context.
>> >> >
>> >> > Normally, netif_receive_skb() is invoked from softirq context.
>> >> >
>> >> > However, via netpoll it can be invoked essentially from any context.
>> >> >
>> >> > But, when this happens, the networking receive path makes amends such
>> >> > that this works fine. That's what the netpoll_receive_skb() check in
>> >> > netif_receive_skb() is for. That check makes it bail out early if the
>> >> > call to netif_receive_skb() is via a netpoll invocation.
>> >> >
>> >>
>> >> Oh, I see. This means we should call rcu_read_lock_bh() instead.
>> >> If Paul has no objections, I will send a patch for this.
>> >>
>> >
>> > Nope, its calling rcu_read_lock() from interrupt context and it should
>> > stay as is (we dont need to disable bh, this has a cpu cost)
>> >
>>
>> Oh, but lockdep complains about rcu_read_lock(), it said
>> rcu_read_lock() can't be used in softirq context.
>>
>> Am I missing something?
>
>Hmmm... It is supposed to be OK to use rcu_read_lock() in pretty much
>any context, even NMI. I will take a look.
>
Thanks! Please let me know if you have new progress.
^ permalink raw reply
* Re: [PATCH 0/7] tcp: bugs and cleanup for 2.6.34-rc1
From: Américo Wang @ 2010-03-13 5:29 UTC (permalink / raw)
To: William Allen Simpson
Cc: Eric Dumazet, Linus Torvalds, Andrew Morton,
Linux Kernel Developers, Linux Kernel Network Developers,
David Miller, Penttilä Mika
In-Reply-To: <4B992A89.6040700@gmail.com>
On Thu, Mar 11, 2010 at 12:38:17PM -0500, William Allen Simpson wrote:
> On 3/11/10 10:01 AM, Eric Dumazet wrote:
>> It would be nice if you could update your knowledge of how linux
>> development works these days.
>>
> Perhaps you could supply pointers to the relevant documentation?
>
>
Documentation/development-process/* are still nice documents.
^ permalink raw reply
* [PATCH] timestamping: fix example build
From: Randy Dunlap @ 2010-03-13 5:23 UTC (permalink / raw)
To: Netdev; +Cc: David Miller, Patrick Ohly
From: Randy Dunlap <randy.dunlap@oracle.com>
Fix Makefiles so that Documentation/networking/timestamping/timestamping.c
will build when using the CONFIG_BUILD_DOCSRC kconfig option.
(timestamping.c does not build currently with its simple Makefile.)
Also fix printf format warnings.
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Cc: Patrick Ohly <patrick.ohly@intel.com>
---
Documentation/networking/Makefile | 2 +
Documentation/networking/timestamping/Makefile | 11 ++++++++--
Documentation/networking/timestamping/timestamping.c | 10 ++++-----
3 files changed, 16 insertions(+), 7 deletions(-)
--- lnx-2634-rc1.orig/Documentation/networking/Makefile
+++ lnx-2634-rc1/Documentation/networking/Makefile
@@ -6,3 +6,5 @@ hostprogs-y := ifenslave
# Tell kbuild to always build the programs
always := $(hostprogs-y)
+
+obj-m := timestamping/
--- lnx-2634-rc1.orig/Documentation/networking/timestamping/Makefile
+++ lnx-2634-rc1/Documentation/networking/timestamping/Makefile
@@ -1,6 +1,13 @@
-CPPFLAGS = -I../../../include
+# kbuild trick to avoid linker error. Can be omitted if a module is built.
+obj- := dummy.o
-timestamping: timestamping.c
+# List of programs to build
+hostprogs-y := timestamping
+
+# Tell kbuild to always build the programs
+always := $(hostprogs-y)
+
+HOSTCFLAGS_timestamping.o += -I$(objtree)/usr/include
clean:
rm -f timestamping
--- lnx-2634-rc1.orig/Documentation/networking/timestamping/timestamping.c
+++ lnx-2634-rc1/Documentation/networking/timestamping/timestamping.c
@@ -41,9 +41,9 @@
#include <arpa/inet.h>
#include <net/if.h>
-#include "asm/types.h"
-#include "linux/net_tstamp.h"
-#include "linux/errqueue.h"
+#include <asm/types.h>
+#include <linux/net_tstamp.h>
+#include <linux/errqueue.h>
#ifndef SO_TIMESTAMPING
# define SO_TIMESTAMPING 37
@@ -164,7 +164,7 @@ static void printpacket(struct msghdr *m
gettimeofday(&now, 0);
- printf("%ld.%06ld: received %s data, %d bytes from %s, %d bytes control messages\n",
+ printf("%ld.%06ld: received %s data, %d bytes from %s, %zu bytes control messages\n",
(long)now.tv_sec, (long)now.tv_usec,
(recvmsg_flags & MSG_ERRQUEUE) ? "error" : "regular",
res,
@@ -173,7 +173,7 @@ static void printpacket(struct msghdr *m
for (cmsg = CMSG_FIRSTHDR(msg);
cmsg;
cmsg = CMSG_NXTHDR(msg, cmsg)) {
- printf(" cmsg len %d: ", cmsg->cmsg_len);
+ printf(" cmsg len %zu: ", cmsg->cmsg_len);
switch (cmsg->cmsg_level) {
case SOL_SOCKET:
printf("SOL_SOCKET ");
^ permalink raw reply
* Re: Linux support for 802.3af?
From: Ben Hutchings @ 2010-03-13 4:40 UTC (permalink / raw)
To: Philip A. Prindeville; +Cc: netdev
In-Reply-To: <4B9AE62B.6050502@redfish-solutions.com>
On Fri, 2010-03-12 at 18:11 -0700, Philip A. Prindeville wrote:
> On 03/12/2010 05:43 PM, Ben Hutchings wrote:
> > On Fri, 2010-03-12 at 17:01 -0700, Philip A. Prindeville wrote:
> >> I was looking at this Marvell-based 4-port PCI card with PoE and
> >> thinking it would be handy for powering cameras and ip-phones, but don't
> >> remember seeing any kernel support for 802.3af...
> >>
> >> http://www.korenix.com/jetcard-PoE_Universal_PCI_Card-2215-overview.htm
> >>
> >> There's no support for 802.3af, right?
> >
> > The standard MDIO ioctls can be used to access registers 11 and 12 on
> > PHYs with integrated PSE functionality. There is no way of indicating
> > whether a PHY has such functionality, but a flag for this could be added
> > to the mdio_support field of struct ethtool_cmd.
> >
> > Ben.
>
> Well, there's more to it than that, right?
>
> There's a software state machine that's required to go through the
> handshake, there's prioritization and power budgeting, etc. Yes?
>
> What am I missing?
We seem to have contrary expectations of how a PSE would be implemented.
You assume that the CPU would be involved at quite a low level. I
assume that a PSE in a Linux system would be implemented on a peripheral
device and that it would be managed from the host via MDIO (though this
is specified as an optional feature).
Looking at the specs, that particular card is using a Marvell switch
chip which doesn't have any PoE support, so management via MDIO seems
unlikely. If you're curious I suppose you should try to get the driver
from this vendor.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* sungem on PowerMac G4: RX MAC fifo overflow
From: Alexei Colin @ 2010-03-13 4:17 UTC (permalink / raw)
To: netdev
Hi,
Any help on this issue would be greatly appreciated. I am emailing
netdev because I couldn't find anything relevant online (except
ancient inconclusive reports of the same error message).
Issue: system loses Ethernet connectivity after several hours usually
or sooner occasionally. This message gets written to the log multiple
times:
eht0: RX MAC fifo overflow smac[<a mem address>]
Machine: PowerMac G4 (no more firmware updates applicable)
Ethernet: 0002:20:0f.0 Ethernet controller: Apple Computer Inc.
UniNorth GMAC (Sun GEM) (rev 01)
System: Linux 2.6.31-19-powerpc #56-Ubuntu Thu Jan 28 00:50:48 UTC
2010 GNU/Linux
Driver: sungem (srcversion: 9C4DDFB8D2201E852413F4F)
I'd like to think that it started after I updated to Ubuntu 9.10
(different version of sungem), but booting into 2.6.28-6-powerpc
(after the dist upgrade, though) the same is observed.
This does bring back the network:
ifdown eth0; rmmod sungem; modprobe sungem; ifup eth0
(I never tried without reloading the module)
Please let me know if you would like me to get debugging info and
which. Thank you in advance!
Regards,
Alexei
^ permalink raw reply
* Re: Linux support for 802.3af?
From: Philip A. Prindeville @ 2010-03-13 1:11 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev
In-Reply-To: <1268440985.7439.23.camel@localhost>
On 03/12/2010 05:43 PM, Ben Hutchings wrote:
> On Fri, 2010-03-12 at 17:01 -0700, Philip A. Prindeville wrote:
>> I was looking at this Marvell-based 4-port PCI card with PoE and
>> thinking it would be handy for powering cameras and ip-phones, but don't
>> remember seeing any kernel support for 802.3af...
>>
>> http://www.korenix.com/jetcard-PoE_Universal_PCI_Card-2215-overview.htm
>>
>> There's no support for 802.3af, right?
>
> The standard MDIO ioctls can be used to access registers 11 and 12 on
> PHYs with integrated PSE functionality. There is no way of indicating
> whether a PHY has such functionality, but a flag for this could be added
> to the mdio_support field of struct ethtool_cmd.
>
> Ben.
Well, there's more to it than that, right?
There's a software state machine that's required to go through the handshake, there's prioritization and power budgeting, etc. Yes?
What am I missing?
^ permalink raw reply
* [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
From: John Linn @ 2010-03-13 1:05 UTC (permalink / raw)
To: netdev, linuxppc-dev, grant.likely, jwboyer
Cc: michal.simek, John Tyner, John Linn, john.williams
This patch adds support for using the LL TEMAC Ethernet driver on
non-Virtex 5 platforms by adding support for accessing the Soft DMA
registers as if they were memory mapped instead of solely through the
DCR's (available on the Virtex 5).
The patch also updates the driver so that it runs on the MicroBlaze.
The changes were tested on the PowerPC 440, PowerPC 405, and the
MicroBlaze platforms.
Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
Signed-off-by: John Linn <john.linn@xilinx.com>
---
V2 - Incorporated comments from Grant and added more logic to allow the driver
to work on MicroBlaze.
drivers/net/Kconfig | 1 -
drivers/net/ll_temac.h | 17 +++++-
drivers/net/ll_temac_main.c | 124 ++++++++++++++++++++++++++++++++++---------
3 files changed, 113 insertions(+), 29 deletions(-)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 9b6efe1..5402105 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -2443,7 +2443,6 @@ config MV643XX_ETH
config XILINX_LL_TEMAC
tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC) driver"
select PHYLIB
- depends on PPC_DCR_NATIVE
help
This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
core used in Xilinx Spartan and Virtex FPGAs
diff --git a/drivers/net/ll_temac.h b/drivers/net/ll_temac.h
index 1af66a1..915aa34 100644
--- a/drivers/net/ll_temac.h
+++ b/drivers/net/ll_temac.h
@@ -5,8 +5,11 @@
#include <linux/netdevice.h>
#include <linux/of.h>
#include <linux/spinlock.h>
+
+#ifdef CONFIG_PPC_DCR
#include <asm/dcr.h>
#include <asm/dcr-regs.h>
+#endif
/* packet size info */
#define XTE_HDR_SIZE 14 /* size of Ethernet header */
@@ -290,8 +293,12 @@ This option defaults to enabled (set) */
#define TX_CONTROL_CALC_CSUM_MASK 1
+/* Align the IP data in the packet on word boundaries as MicroBlaze
+ * needs it.
+ */
+
#define XTE_ALIGN 32
-#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN)
+#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN)
#define MULTICAST_CAM_TABLE_NUM 4
@@ -335,9 +342,15 @@ struct temac_local {
struct mii_bus *mii_bus; /* MII bus reference */
int mdio_irqs[PHY_MAX_ADDR]; /* IRQs table for MDIO bus */
- /* IO registers and IRQs */
+ /* IO registers, dma functions and IRQs */
void __iomem *regs;
+ void __iomem *sdma_regs;
+#ifdef CONFIG_PPC_DCR
dcr_host_t sdma_dcrs;
+#endif
+ u32 (*dma_in)(struct temac_local *, int);
+ void (*dma_out)(struct temac_local *, int, u32);
+
int tx_irq;
int rx_irq;
int emac_num;
diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index a18e348..9aedf9b 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -20,9 +20,6 @@
* or rx, so this should be okay.
*
* TODO:
- * - Fix driver to work on more than just Virtex5. Right now the driver
- * assumes that the locallink DMA registers are accessed via DCR
- * instructions.
* - Factor out locallink DMA code into separate driver
* - Fix multicast assignment.
* - Fix support for hardware checksumming.
@@ -115,17 +112,86 @@ void temac_indirect_out32(struct temac_local *lp, int reg, u32 value)
temac_iow(lp, XTE_CTL0_OFFSET, CNTLREG_WRITE_ENABLE_MASK | reg);
}
+/**
+ * temac_dma_in32 - Memory mapped DMA read, this function expects a
+ * register input that is based on DCR word addresses which
+ * are then converted to memory mapped byte addresses
+ */
static u32 temac_dma_in32(struct temac_local *lp, int reg)
{
- return dcr_read(lp->sdma_dcrs, reg);
+ return in_be32((u32 *)(lp->sdma_regs + (reg << 2)));
}
+/**
+ * temac_dma_out32 - Memory mapped DMA read, this function expects a
+ * register input that is based on DCR word addresses which
+ * are then converted to memory mapped byte addresses
+ */
static void temac_dma_out32(struct temac_local *lp, int reg, u32 value)
{
+ out_be32((u32 *)(lp->sdma_regs + (reg << 2)), value);
+}
+
+/* DMA register access functions can be DCR based or memory mapped.
+ * The PowerPC 440 is DCR based, the PowerPC 405 and MicroBlaze are both
+ * memory mapped.
+ */
+#ifdef CONFIG_PPC_DCR
+
+/**
+ * temac_dma_dcr_in32 - DCR based DMA read
+ */
+static u32 temac_dma_dcr_in(struct temac_local *lp, int reg)
+{
+ return dcr_read(lp->sdma_dcrs, reg);
+}
+
+/**
+ * temac_dma_dcr_out32 - DCR based DMA write
+ */
+static void temac_dma_dcr_out(struct temac_local *lp, int reg, u32 value)
+{
dcr_write(lp->sdma_dcrs, reg, value);
}
/**
+ * temac_dcr_setup - If the DMA is DCR based, then setup the address and
+ * I/O functions
+ */
+static int temac_dcr_setup(struct temac_local *lp, struct of_device *op,
+ struct device_node *np)
+{
+ unsigned int dcrs;
+
+ /* setup the dcr address mapping if it's in the device tree */
+
+ dcrs = dcr_resource_start(np, 0);
+ if (dcrs != 0) {
+ lp->sdma_dcrs = dcr_map(np, dcrs, dcr_resource_len(np, 0));
+ lp->dma_in = temac_dma_dcr_in;
+ lp->dma_out = temac_dma_dcr_out;
+ dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
+ return 0;
+ }
+ /* no DCR in the device tree, indicate a failure */
+ return -1;
+}
+
+#else
+
+/*
+ * temac_dcr_setup - This is a stub for when DCR is not supported,
+ * such as with MicroBlaze
+ */
+static int temac_dcr_setup(struct temac_local *lp, struct of_device *op,
+ struct device_node *np)
+{
+ return -1;
+}
+
+#endif
+
+/**
* temac_dma_bd_init - Setup buffer descriptor rings
*/
static int temac_dma_bd_init(struct net_device *ndev)
@@ -172,23 +238,23 @@ static int temac_dma_bd_init(struct net_device *ndev)
lp->rx_bd_v[i].app0 = STS_CTRL_APP0_IRQONEND;
}
- temac_dma_out32(lp, TX_CHNL_CTRL, 0x10220400 |
+ lp->dma_out(lp, TX_CHNL_CTRL, 0x10220400 |
CHNL_CTRL_IRQ_EN |
CHNL_CTRL_IRQ_DLY_EN |
CHNL_CTRL_IRQ_COAL_EN);
/* 0x10220483 */
/* 0x00100483 */
- temac_dma_out32(lp, RX_CHNL_CTRL, 0xff010000 |
+ lp->dma_out(lp, RX_CHNL_CTRL, 0xff010000 |
CHNL_CTRL_IRQ_EN |
CHNL_CTRL_IRQ_DLY_EN |
CHNL_CTRL_IRQ_COAL_EN |
CHNL_CTRL_IRQ_IOE);
/* 0xff010283 */
- temac_dma_out32(lp, RX_CURDESC_PTR, lp->rx_bd_p);
- temac_dma_out32(lp, RX_TAILDESC_PTR,
+ lp->dma_out(lp, RX_CURDESC_PTR, lp->rx_bd_p);
+ lp->dma_out(lp, RX_TAILDESC_PTR,
lp->rx_bd_p + (sizeof(*lp->rx_bd_v) * (RX_BD_NUM - 1)));
- temac_dma_out32(lp, TX_CURDESC_PTR, lp->tx_bd_p);
+ lp->dma_out(lp, TX_CURDESC_PTR, lp->tx_bd_p);
return 0;
}
@@ -426,9 +492,9 @@ static void temac_device_reset(struct net_device *ndev)
temac_indirect_out32(lp, XTE_RXC1_OFFSET, val & ~XTE_RXC1_RXEN_MASK);
/* Reset Local Link (DMA) */
- temac_dma_out32(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
+ lp->dma_out(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
timeout = 1000;
- while (temac_dma_in32(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
+ while (lp->dma_in(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
udelay(1);
if (--timeout == 0) {
dev_err(&ndev->dev,
@@ -436,7 +502,7 @@ static void temac_device_reset(struct net_device *ndev)
break;
}
}
- temac_dma_out32(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
+ lp->dma_out(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
temac_dma_bd_init(ndev);
@@ -597,7 +663,7 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
lp->tx_bd_tail = 0;
/* Kick off the transfer */
- temac_dma_out32(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
+ lp->dma_out(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
return NETDEV_TX_OK;
}
@@ -663,7 +729,7 @@ static void ll_temac_recv(struct net_device *ndev)
cur_p = &lp->rx_bd_v[lp->rx_bd_ci];
bdstat = cur_p->app0;
}
- temac_dma_out32(lp, RX_TAILDESC_PTR, tail_p);
+ lp->dma_out(lp, RX_TAILDESC_PTR, tail_p);
spin_unlock_irqrestore(&lp->rx_lock, flags);
}
@@ -674,8 +740,8 @@ static irqreturn_t ll_temac_tx_irq(int irq, void *_ndev)
struct temac_local *lp = netdev_priv(ndev);
unsigned int status;
- status = temac_dma_in32(lp, TX_IRQ_REG);
- temac_dma_out32(lp, TX_IRQ_REG, status);
+ status = lp->dma_in(lp, TX_IRQ_REG);
+ lp->dma_out(lp, TX_IRQ_REG, status);
if (status & (IRQ_COAL | IRQ_DLY))
temac_start_xmit_done(lp->ndev);
@@ -692,8 +758,8 @@ static irqreturn_t ll_temac_rx_irq(int irq, void *_ndev)
unsigned int status;
/* Read and clear the status registers */
- status = temac_dma_in32(lp, RX_IRQ_REG);
- temac_dma_out32(lp, RX_IRQ_REG, status);
+ status = lp->dma_in(lp, RX_IRQ_REG);
+ lp->dma_out(lp, RX_IRQ_REG, status);
if (status & (IRQ_COAL | IRQ_DLY))
ll_temac_recv(lp->ndev);
@@ -794,7 +860,7 @@ static ssize_t temac_show_llink_regs(struct device *dev,
int i, len = 0;
for (i = 0; i < 0x11; i++)
- len += sprintf(buf + len, "%.8x%s", temac_dma_in32(lp, i),
+ len += sprintf(buf + len, "%.8x%s", lp->dma_in(lp, i),
(i % 8) == 7 ? "\n" : " ");
len += sprintf(buf + len, "\n");
@@ -820,7 +886,6 @@ temac_of_probe(struct of_device *op, const struct of_device_id *match)
struct net_device *ndev;
const void *addr;
int size, rc = 0;
- unsigned int dcrs;
/* Init network device structure */
ndev = alloc_etherdev(sizeof(*lp));
@@ -870,13 +935,20 @@ temac_of_probe(struct of_device *op, const struct of_device_id *match)
goto nodev;
}
- dcrs = dcr_resource_start(np, 0);
- if (dcrs == 0) {
- dev_err(&op->dev, "could not get DMA register address\n");
- goto nodev;
+ /* Setup the DMA register accesses, could be DCR or memory mapped */
+ if (temac_dcr_setup(lp, op, np)) {
+
+ /* no DCR in the device tree, try non-DCR */
+ lp->sdma_regs = of_iomap(np, 0);
+ if (lp->sdma_regs) {
+ lp->dma_in = temac_dma_in32;
+ lp->dma_out = temac_dma_out32;
+ dev_dbg(&op->dev, "MEM base: %p\n", lp->sdma_regs);
+ } else {
+ dev_err(&op->dev, "unable to map DMA registers\n");
+ goto nodev;
+ }
}
- lp->sdma_dcrs = dcr_map(np, dcrs, dcr_resource_len(np, 0));
- dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
lp->rx_irq = irq_of_parse_and_map(np, 0);
lp->tx_irq = irq_of_parse_and_map(np, 1);
--
1.6.2.1
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
^ permalink raw reply related
* Re: [PATCH 0/3] pci: fix/cleanup pcix get and set mmrbc functions
From: Jesse Barnes @ 2010-03-13 1:00 UTC (permalink / raw)
To: Dean Nelson; +Cc: netdev, linux-pci
In-Reply-To: <20100310032632.6331.15414.send-patch@aqua>
On Tue, 9 Mar 2010 22:26:33 -0500
Dean Nelson <dnelson@redhat.com> wrote:
> A customer running RHEL4.8 encountered
>
> "e1000: eth0: e1000_clean_tx_irq: Detected Tx Unit Hang"
>
> type errors, which were determined to be the result of a bad return value
> from e1000_pcix_get_mmrbc() causing the call to e1000_pcix_set_mmrbc() to
> be skipped in the following snippet of code from e1000_init_hw().
>
> switch (hw->mac_type) {
> case e1000_82545_rev_3:
> case e1000_82546_rev_3:
> break;
> default:
> /* Workaround for PCI-X problem when BIOS sets MMRBC incorrectly. */
> if (hw->bus_type == e1000_bus_type_pcix
> && e1000_pcix_get_mmrbc(hw) > 2048)
> e1000_pcix_set_mmrbc(hw, 2048);
> break;
> }
>
> e1000_pcix_get_mmrbc() is basically a wrapper for a call to pcix_get_mmrbc().
> e1000_pcix_set_mmrbc() is the same for pcix_set_mmrbc().
>
> The following three patches are a response to the problems that were found to
> exist with pcix_get_max_mmrbc(), pcix_get_mmrbc() and pcix_set_mmrbc().
>
> Versions of these patches applicable to RHEL4 were verified by the customer to
> solve their problem.
Thanks Dean, I'll pull these in and send them to Linus for 2.6.34. I
assume they should also be included in the stable kernel series? If
so, I'll add a cc: stable@kernel.org when I commit them.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply
* Re: Linux support for 802.3af?
From: Ben Hutchings @ 2010-03-13 0:43 UTC (permalink / raw)
To: Philip A. Prindeville; +Cc: netdev
In-Reply-To: <4B9AD5CA.7000303@redfish-solutions.com>
On Fri, 2010-03-12 at 17:01 -0700, Philip A. Prindeville wrote:
> I was looking at this Marvell-based 4-port PCI card with PoE and
> thinking it would be handy for powering cameras and ip-phones, but don't
> remember seeing any kernel support for 802.3af...
>
> http://www.korenix.com/jetcard-PoE_Universal_PCI_Card-2215-overview.htm
>
> There's no support for 802.3af, right?
The standard MDIO ioctls can be used to access registers 11 and 12 on
PHYs with integrated PSE functionality. There is no way of indicating
whether a PHY has such functionality, but a flag for this could be added
to the mdio_support field of struct ethtool_cmd.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] vsprintf.c: Use noinline_for_stack
From: Andrew Morton @ 2010-03-13 0:25 UTC (permalink / raw)
To: Joe Perches
Cc: Linus Torvalds, Nick Andrew, Linux Kernel Mailing List,
Greg Kroah-Hartman, netdev, Bjorn Helgaas
In-Reply-To: <1267929215.1937.18.camel@Joe-Laptop.home>
On Sat, 06 Mar 2010 18:33:35 -0800
Joe Perches <joe@perches.com> wrote:
> On Sat, 2010-03-06 at 18:03 -0800, Linus Torvalds wrote:
> > A few noinlines might be appropriate.
>
> Mark static functions with noinline_for_stack
>
-ENOTESTINGRESULTS.
Before:
akpm:/usr/src/25> objdump -d lib/vsprintf.o | perl scripts/checkstack.pl
0x00000e82 pointer [vsprintf.o]: 344
0x0000198c pointer [vsprintf.o]: 344
0x000025d6 scnprintf [vsprintf.o]: 216
0x00002648 scnprintf [vsprintf.o]: 216
0x00002565 snprintf [vsprintf.o]: 208
0x0000267c sprintf [vsprintf.o]: 208
0x000030a3 bprintf [vsprintf.o]: 208
0x00003b1e sscanf [vsprintf.o]: 208
0x00000608 number [vsprintf.o]: 136
0x00000937 number [vsprintf.o]: 136
After:
akpm:/usr/src/25> objdump -d lib/vsprintf.o | perl scripts/checkstack.pl
0x00000a7c symbol_string [vsprintf.o]: 248
0x00000ae8 symbol_string [vsprintf.o]: 248
0x00002310 scnprintf [vsprintf.o]: 216
0x00002382 scnprintf [vsprintf.o]: 216
0x0000229f snprintf [vsprintf.o]: 208
0x000023b6 sprintf [vsprintf.o]: 208
0x00002ddd bprintf [vsprintf.o]: 208
0x00003858 sscanf [vsprintf.o]: 208
0x00000625 number [vsprintf.o]: 136
0x00000954 number [vsprintf.o]: 136
nice.
^ permalink raw reply
* Linux support for 802.3af?
From: Philip A. Prindeville @ 2010-03-13 0:01 UTC (permalink / raw)
To: netdev
I was looking at this Marvell-based 4-port PCI card with PoE and
thinking it would be handy for powering cameras and ip-phones, but don't
remember seeing any kernel support for 802.3af...
http://www.korenix.com/jetcard-PoE_Universal_PCI_Card-2215-overview.htm
There's no support for 802.3af, right?
Thanks,
-Philip
^ permalink raw reply
* Re: [PATCH] TI DaVinci EMAC: Convert to dev_pm_ops
From: Kevin Hilman @ 2010-03-12 23:28 UTC (permalink / raw)
To: Chaithrika U S; +Cc: netdev, davinci-linux-open-source, davem
In-Reply-To: <1268296676-12514-1-git-send-email-chaithrika@ti.com>
Chaithrika U S <chaithrika@ti.com> writes:
> Migrate from the legacy PM hooks to use dev_pm_ops structure.
>
> Signed-off-by: Chaithrika U S <chaithrika@ti.com>
Acked-by: Kevin Hilman <khilman@deeprootsystems.com>
^ permalink raw reply
* Re: [PATCH 3/4] OMAP3 : clock data: Update name string for EMAC clocks.
From: Kevin Hilman @ 2010-03-12 23:26 UTC (permalink / raw)
To: Sriramakrishnan; +Cc: linux-omap, netdev, davinci-linux-open-source
In-Reply-To: <1268320422-32656-4-git-send-email-srk@ti.com>
Sriramakrishnan <srk@ti.com> writes:
> The emac driver uses generic name for the module and phy
> clocks. Updated the omap3xxx_clks table to match the names
> used by the Davinci emac driver.
>
> Signed-off-by: Sriramakrishnan <srk@ti.com>
> ---
> arch/arm/mach-omap2/clock3xxx_data.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/clock3xxx_data.c b/arch/arm/mach-omap2/clock3xxx_data.c
> index d5153b6..989da2e 100644
> --- a/arch/arm/mach-omap2/clock3xxx_data.c
> +++ b/arch/arm/mach-omap2/clock3xxx_data.c
> @@ -3472,8 +3472,8 @@ static struct omap_clk omap3xxx_clks[] = {
> CLK(NULL, "ipss_ick", &ipss_ick, CK_AM35XX),
> CLK(NULL, "rmii_ck", &rmii_ck, CK_AM35XX),
> CLK(NULL, "pclk_ck", &pclk_ck, CK_AM35XX),
> - CLK("davinci_emac", "ick", &emac_ick, CK_AM35XX),
> - CLK("davinci_emac", "fck", &emac_fck, CK_AM35XX),
> + CLK("davinci_emac", "emac_clk", &emac_ick, CK_AM35XX),
> + CLK("davinci_emac", "phy_clk", &emac_fck, CK_AM35XX),
As I mentioned in the review of the equivalent davinci changes,
I'm not crazy abou the _clk suffixes on the con_id. I think
these should be called "main" (or "emac") and "phy".
Kevin
^ permalink raw reply
* [PATCH] sky2: Avoid rtnl_unlock without rtnl_lock
From: Mike McCormack @ 2010-03-12 23:11 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Make sure we always call rtnl_lock before going down the
error path in sky2_resume, which unlocks the rtnl lock.
Signed-off-by: Mike McCormack <mikem@ring3k.org>
---
drivers/net/sky2.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 653bdd7..d8ec4c1 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -4863,6 +4863,7 @@ static int sky2_resume(struct pci_dev *pdev)
if (!hw)
return 0;
+ rtnl_lock();
err = pci_set_power_state(pdev, PCI_D0);
if (err)
goto out;
@@ -4884,7 +4885,6 @@ static int sky2_resume(struct pci_dev *pdev)
sky2_write32(hw, B0_IMSK, Y2_IS_BASE);
napi_enable(&hw->napi);
- rtnl_lock();
for (i = 0; i < hw->ports; i++) {
err = sky2_reattach(hw->dev[i]);
if (err)
--
1.5.6.5
^ permalink raw reply related
* Re: [PATCH v7] rps: Receive Packet Steering
From: Tom Herbert @ 2010-03-12 23:08 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev
In-Reply-To: <1268429319.2947.10.camel@edumazet-laptop>
> One last point about placement of rxhash in struct sk_buff, that I
> missed in my previous review, sorry...
>
> You put it right before cb[48] which is now aligned to 8 bytes (since
> commit da3f5cf1 skbuff: align sk_buff::cb to 64 bit and close some
> potential holes), so this adds a 4 bytes hole.
>
> Please put it elsewhere, possibly close to fields that are read in
> get_rps_cpu() (skb->queue_mapping, skb->protocol, skb->data, ...) to
> minimize number of cache lines that dispatcher cpu has to bring into its
> cache, before giving skb to another cpu for IP/TCP processing.
>
Looks like it will fit right before queue_mapping, I'll put it there.
Tom
^ permalink raw reply
* Re: [PATCH v4 2/7] net: remove old tcp_optlen function
From: William Allen Simpson @ 2010-03-12 23:05 UTC (permalink / raw)
To: Dan Carpenter, Linus Torvalds, Andrew Morton,
Linux Kernel Developers
In-Reply-To: <20100312174656.GA12175@bicker>
All the drama is beside the point. This patch merely removes a *rarely*
used function (2 drivers). Not complicated....
There's a reason that this function isn't used much. It doesn't work.
On 3/12/10 12:46 PM, Dan Carpenter wrote:
> So after you removed the checks this change includes:
I didn't remove any *existing* checks. I had added *new* checks in my
earlier patch, then removed *my* checks from this patch as required by
David Miller.
> 1) Random slagging on the networking guys
I had to look up that "random slagging on" colloquialism. Apparently,
the "random slagging" target would be *me* -- calling me "anal" and my
code "rediculious bloat" [sic] probably qualifies....
(Admittedly, I'm rather careful and may be overly cautious at times, after
some 30+ years of writing network drivers. Once it's in half a billion
cell phones, it's hard/impossible to update.)
Since my first unpleasant interactions with David Miller on my very
earliest (October) netdev posts, I've conspicuously avoided contradicting
him. I've merely *obeyed* his injunction here, and moved on....
The patch itself neutrally documents a coding requirement decision by that
networking maintainer by name.
> 2) u32 => int to ameliorate your static checker's complaints
Good idea. Actually, I simply looked at the code and its history.
> 3) cleanups
>
Removing this function is really a *bug* fix (in several places), with
cleanups in the vicinity for obviously poor coding (variants in 3 places):
- mss = 0;
- if ((mss = skb_shinfo(skb)->gso_size) != 0) {
- int tcp_opt_len, ip_tcp_len;
Cleaner as:
+ mss = skb_shinfo(skb)->gso_size;
+ if (mss != 0) {
+ struct tcphdr *th;
But I wouldn't have bothered had I not been changing that immediately
following line. 30+ years of experience with collaborative projects
informs me that it's best to make minor cleanups only where I'm already
improving the code nearby. Otherwise, it creates patch conflicts.
> People have already explained that tcp_optlen() doesn't return
> negative values.
People? The fact that the calculation itself can be negative appeared
the very first time I tested my own code using this bad function!
> negative values. It would really help us if you could show how
> tcp_hdr(skb)->doff can be less than 5?
>
Oh, I've long since given up on lengthy explanations. Both Eric and
Ilpo have repeatedly castigated me for being too wordy.
In this particular instance, I suggest that you take a look at all the
places that gso_size is set, and cross index with all the code paths that
place these TCP headers onto the txq without a check of doff -- as I did!
I'll specifically mention the tun and virtio_net devices, but I'm also
particularly concerned with af_packet.c and skbuff.c -- and the general
problem with inet_lro.c, too.
Amazingly enough, folks sometimes use Linux for routers....
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox