* Re: dhcp client packet sniffing...
From: Patrick McHardy @ 2010-04-08 12:49 UTC (permalink / raw)
To: Herbert Xu; +Cc: David Miller, netdev
In-Reply-To: <20100408123025.GA23762@gondor.apana.org.au>
Herbert Xu wrote:
> On Thu, Apr 08, 2010 at 05:11:44AM -0700, David Miller wrote:
>> We have the socket pointer etc. already in dev_queue_xmit_nit(), so we
>> can easily do it.
>
> It doesn't look trivial to me since AF_PACKET fiddles with packet
> headers before running the filter...
Yes, that looks difficult. What might work is to pass the skb->data
offsets resulting from those modifications to sk_run_filter to
adjust the postition when loading data from the packet. That would
allow to run the filter on the original packet before cloning it.
Regarding your idea of only receiving incoming packets, userspace could
use the SKF_AD_PKTTYPE filter with PACKET_HOST. During filter attachment
and checks, we could mark the socket as only interested in incoming or
outgoing packets.
This would require userspace changes of course, but we should be able
to avoid passing outgoing packets to af_packet with very low overhead.
^ permalink raw reply
* Re: netfilter: Only do skb_checksum_help on CHECKSUM_PARTIAL in ip_queue
From: Patrick McHardy @ 2010-04-08 12:53 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev
In-Reply-To: <20100408123545.GA23829@gondor.apana.org.au>
Herbert Xu wrote:
> Hi:
>
> netfilter: Only do skb_checksum_help on CHECKSUM_PARTIAL in ip_queue
>
> While doing yet another audit on ip_summed I noticed ip_queue
> calling skb_checksum_help unnecessarily. As we will set ip_summed
> to CHECKSUM_NONE when necessary in ipq_mangle_ipv4, there is no
> need to zap CHECKSUM_COMPLETE in ipq_build_packet_message.
Applied, thanks Herbert.
^ permalink raw reply
* Re: netfilter: Only do skb_checksum_help on CHECKSUM_PARTIAL in ip6_queue
From: Patrick McHardy @ 2010-04-08 12:54 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev
In-Reply-To: <20100408123930.GA23910@gondor.apana.org.au>
Herbert Xu wrote:
> On Thu, Apr 08, 2010 at 08:35:45PM +0800, Herbert Xu wrote:
>> netfilter: Only do skb_checksum_help on CHECKSUM_PARTIAL in ip_queue
>
> Doh! There are three of these queue beasts.
;)
> netfilter: Only do skb_checksum_help on CHECKSUM_PARTIAL in ip6_queue
>
> As we will set ip_summed to CHECKSUM_NONE when necessary in
> ipq_mangle_ipv6, there is no need to zap CHECKSUM_COMPLETE in
> ipq_build_packet_message.
Also applied, thanks.
^ permalink raw reply
* Re: netfilter: Only do skb_checksum_help on CHECKSUM_PARTIAL in nfnetlink_queue
From: Patrick McHardy @ 2010-04-08 12:55 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev
In-Reply-To: <20100408124158.GB23910@gondor.apana.org.au>
Herbert Xu wrote:
> netfilter: Only do skb_checksum_help on CHECKSUM_PARTIAL in nfnetlink_queue
>
> As we will set ip_summed to CHECKSUM_NONE when necessary in
> nfqnl_mangle, there is no need to zap CHECKSUM_COMPLETE in
> nfqnl_build_packet_message.
Applied, thanks.
^ permalink raw reply
* Re: dhcp client packet sniffing...
From: Herbert Xu @ 2010-04-08 13:12 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David Miller, netdev
In-Reply-To: <4BBDD0ED.1010203@trash.net>
On Thu, Apr 08, 2010 at 02:49:49PM +0200, Patrick McHardy wrote:
>
> Yes, that looks difficult. What might work is to pass the skb->data
> offsets resulting from those modifications to sk_run_filter to
> adjust the postition when loading data from the packet. That would
> allow to run the filter on the original packet before cloning it.
The thing is we can't express those offsets as constants so we'll
need to run protocol-specific code (i.e., an indirect function
call) prior to calling the filter.
At that point I'd just give up and go back to the skb_share idea :)
If we're concerned about atomic counter performance, we could even
do a non-atomic version of skb_share and use it here. We're the
sole owner of the skb at this point.
> Regarding your idea of only receiving incoming packets, userspace could
> use the SKF_AD_PKTTYPE filter with PACKET_HOST. During filter attachment
> and checks, we could mark the socket as only interested in incoming or
> outgoing packets.
>
> This would require userspace changes of course, but we should be able
> to avoid passing outgoing packets to af_packet with very low overhead.
As kernel programmers, we reject in principle any solution that
involves user-space coding :)
Seriously, with the number of DHCP clients out there, any solution
that requires changing the client is not going to achieve the
objective in a reasonable time-frame.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: FEC driver: rcv is not +last
From: Sascha Hauer @ 2010-04-08 13:20 UTC (permalink / raw)
To: Matthias Kaehlcke; +Cc: netdev
In-Reply-To: <20100408104033.GI3787@darwin>
On Thu, Apr 08, 2010 at 12:40:33PM +0200, Matthias Kaehlcke wrote:
> hi,
>
> i have problems with the FEC on a i.MX25 3-Stack board. the kernel is
> v2.6.34-rc2 plus the following patch:
> http://patchwork.ozlabs.org/patch/41235/
>
> the following traces are generated at boot time:
>
> FEC Ethernet Driver
> fec: PHY @ 0x1, ID 0x20005ce1 -- unknown PHY!
> ...
> eth0: config: auto-negotiation on, 100FDX, 100HDX, 10FDX, 10HDX.
> ...
> FEC ENET: rcv is not +last
> FEC ENET: rcv is not +last
> FEC ENET: rcv is not +last
> FEC ENET: rcv is not +last
> ...
No idea, I have never seen this message. Does the controller work
besides these messages?
>
> the PHY of the board is a DP83840, which is not supported by the
> driver. could this be the problem? i tried to make the kernel think
> the DP83840 is a DP83848, which is supported, but the behaviour is the
> same except the 'unknown PHY' warning.
This should be solved by the phylib patches recently posted for the fec
driver.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: dhcp client packet sniffing...
From: Patrick McHardy @ 2010-04-08 13:23 UTC (permalink / raw)
To: Herbert Xu; +Cc: David Miller, netdev
In-Reply-To: <20100408131254.GA24262@gondor.apana.org.au>
Herbert Xu wrote:
> On Thu, Apr 08, 2010 at 02:49:49PM +0200, Patrick McHardy wrote:
>> Yes, that looks difficult. What might work is to pass the skb->data
>> offsets resulting from those modifications to sk_run_filter to
>> adjust the postition when loading data from the packet. That would
>> allow to run the filter on the original packet before cloning it.
>
> The thing is we can't express those offsets as constants so we'll
> need to run protocol-specific code (i.e., an indirect function
> call) prior to calling the filter.
Well, we already have some AF_PACKET specific code in
dev_queue_xmit_nit(). It wouldn't be pretty, but for this special
case we could just calculate the offsets in a non-generic way
without indirect function calls. It basically comes down to:
if (ptype->af_packet_priv &&
dev->header_ops) {
if (((struct sock *)ptype->af_packet_priv)->sk_type != SOCK_DGRAM)
offset = skb->data - skb_mac_header(skb);
else /* always PACKET_OUTGOING in dev_queue_xmit_nit */
offset = skb_network_offset(skb);
}
> At that point I'd just give up and go back to the skb_share idea :)
That does sound better :)
> If we're concerned about atomic counter performance, we could even
> do a non-atomic version of skb_share and use it here. We're the
> sole owner of the skb at this point.
>
>> Regarding your idea of only receiving incoming packets, userspace could
>> use the SKF_AD_PKTTYPE filter with PACKET_HOST. During filter attachment
>> and checks, we could mark the socket as only interested in incoming or
>> outgoing packets.
>>
>> This would require userspace changes of course, but we should be able
>> to avoid passing outgoing packets to af_packet with very low overhead.
>
> As kernel programmers, we reject in principle any solution that
> involves user-space coding :)
>
> Seriously, with the number of DHCP clients out there, any solution
> that requires changing the client is not going to achieve the
> objective in a reasonable time-frame.
That's true.
^ permalink raw reply
* Re: [PATCH] xfrm: Fix double dst_release() in xfrm_lookup() -EREMOTE case
From: Mark Brown @ 2010-04-08 13:43 UTC (permalink / raw)
To: Timo Teräs; +Cc: David S. Miller, netdev
In-Reply-To: <4BBDCE8A.7040106@iki.fi>
On Thu, Apr 08, 2010 at 03:39:38PM +0300, Timo Teräs wrote:
> Mark Brown wrote:
> >I'm not sure if this is correct or not - there may a reference been
> >taken earlier in __xfrm_lookup() that's being dropped but I didn't spot
> >it.
> This is not correct.
So I was correct when I said that there might've been a reference taken
earlier :)
> This semantics is important because __xfrm_lookup() is also called
> from other places, that do other things when they get -EREMOTE.
Right, it was the fact that this was the only place doing the free that
made this unclear. Some comments might make this rather more obvious,
the fact that the release was added to __xfrm_lookup() as part of the
recent patch made it unclear if the release that was still there in
xfrm_lookup() was still needed or an oversight.
^ permalink raw reply
* Re: [PATCH] tcp: Set CHECKSUM_UNNECESSARY in tcp_init_nondata_skb
From: Herbert Xu @ 2010-04-08 13:57 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100408.012617.98660541.davem@davemloft.net>
On Thu, Apr 08, 2010 at 01:26:17AM -0700, David Miller wrote:
>
> Back in commit 04a0551c87363f100b04d28d7a15a632b70e18e7
> ("loopback: Drop obsolete ip_summed setting") we stopped
> setting CHECKSUM_UNNECESSARY in the loopback xmit.
>
> This is because such a setting was a lie since it implies that the
> checksum field of the packet is properly filled in.
>
> Instead what happens normally is that CHECKSUM_PARTIAL is set and
> skb->csum is calculated as needed.
>
> But this was only happening for TCP data packets (via the
> skb->ip_summed assignment done in tcp_sendmsg()). It doesn't
> happen for non-data packets like ACKs etc.
Actually, the checksum is still calculated in this case as otherwise
people would've screamed murder since their TCP connections would
have stopped working :)
However, it is suboptimal for loopback because we'll end up doing
an unnecessary verification for non-data packets.
> Fix this by setting skb->ip_summed in the common non-data packet
> constructor. It already is setting skb->csum to zero.
The problem here is that for non-data packets CHECKSUM_PARTIAL
can actually end up being worse if we wind up going out through
an interface that doesn't support checksums.
The crux of the matter is to how to distinguish between those
packets where we have computed the checksum ourselves (such as
these non-data TCP packets), vs. packets from other sources where
we don't trust the checksum to be correct.
Since using CHECKSUM_PARTIAL is problematic, another possibility
is to use CHECKSUM_UNNECESSARY to indicate this.
I've gone through the core TX paths and it would appear that
all of them can handle packets with CHECKSUM_UNNECESSARY. They
will be treated in the same way as CHECKSUM_NONE.
In fact most drivers can handle this too, since they just do
skb->ip_summed == CHECKSUM_PARTIAL and treat everything else
as the same. Unfortunately, some drivers do the opposite check.
So for the time being, we need to ensure that by the time the skb
hits the driver ip_summed is either CHECKSUM_PARTIAL or CHECKSUM_NONE.
net: Sanitise ip_summed before ndo_start_xmit
We would like to use CHECKSUM_UNNECESSARY on the TX path to mark
packets that are known to have a correct checksum. However, some
drivers cannot handle this value.
This patch ensures that the value of ip_summed is one that all
drivers can handle before calling ndo_start_xmit.
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/net/core/dev.c b/net/core/dev.c
index 1c8a0ce..2784298 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1877,6 +1877,13 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
if (dev->priv_flags & IFF_XMIT_DST_RELEASE)
skb_dst_drop(skb);
+ /*
+ * When drivers are ready for all possible values of
+ * ip_summed we can remove this.
+ */
+ if (skb->ip_summed != CHECKSUM_PARTIAL)
+ skb->ip_summed = CHECKSUM_NONE;
+
rc = ops->ndo_start_xmit(skb, dev);
if (rc == NETDEV_TX_OK)
txq_trans_update(txq);
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: linux-next: build failure after merge of the final tree
From: John Linn @ 2010-04-08 14:15 UTC (permalink / raw)
To: David Miller, sfr; +Cc: netdev, linux-next, linux-kernel, jtyner, grant.likely
In-Reply-To: <20100407.232500.184442638.davem@davemloft.net>
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Thursday, April 08, 2010 12:25 AM
> To: sfr@canb.auug.org.au
> Cc: netdev@vger.kernel.org; linux-next@vger.kernel.org;
linux-kernel@vger.kernel.org; John Linn;
> jtyner@cs.ucr.edu
> Subject: Re: linux-next: build failure after merge of the final tree
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Thu, 8 Apr 2010 15:35:05 +1000
>
> > After merging the final tree, today's linux-next build (powerpc
> > allyesconfig) failed like this:
> >
> > drivers/net/ll_temac_main.c: In function 'll_temac_recv':
> > drivers/net/ll_temac_main.c:695: error: implicit declaration of
function 'virt_to_bus'
> >
> > Caused by commit 459569145516f7967b916c57445feb02c600668c ("Add
> > non-Virtex5 support for LL TEMAC driver") from the net tree.
> >
> > I have reverted that commit for today.
>
> And I'm reverting it too, it's illegal to use virt_to_bus()
> in new code in this day and age.
I'm not pushing back here, just trying to make sure I understand and do
it better next time :)
I don't see that my patch has touched that part of the driver as that
call was already in the driver before my patch (but maybe I'm just
missing it).
My patch did change the dependency in the Kconfig so that it only
depends on powerpc rather than powerpc DCR and maybe that exposed
something that wasn't previously exposed.
Maybe this is an issue in linux-next and I need to test against that,
I'm pulling it now. I was testing against Linus's tree.
>
> John, you'll need to use the DMA mapping APIs in this change when you
> resubmit it.
Thanks, will spin it again and get that fixed. My apologies for the
hassles, appreciate the help.
-- John
>
> Thanks.
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
* Re: dhcp client packet sniffing...
From: Herbert Xu @ 2010-04-08 14:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100408114738.GA23329@gondor.apana.org.au>
On Thu, Apr 08, 2010 at 07:47:38PM +0800, Herbert Xu wrote:
>
> The main problem is that it needs to be able to receive packets
> destined to an address which is not yet a local address. IOW,
> it needs to be able to bypass the routing table and receive
> non-local traffic.
Oh and if anybody is doing any user-space hacking, this is what you
could do: AF_PACKET is only necessary when you're waiting for a
response from the server. So you could close the socket after a
lease has been acquired, and reopen it the next time you need
to communicate with a server.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: dhcp client packet sniffing...
From: Maxime Bizon @ 2010-04-08 14:37 UTC (permalink / raw)
To: Herbert Xu; +Cc: David Miller, netdev
In-Reply-To: <20100408142722.GA25392@gondor.apana.org.au>
On Thu, 2010-04-08 at 22:27 +0800, Herbert Xu wrote:
> Oh and if anybody is doing any user-space hacking, this is what you
> could do: AF_PACKET is only necessary when you're waiting for a
> response from the server. So you could close the socket after a
> lease has been acquired, and reopen it the next time you need
> to communicate with a server.
That's already what some clients do (at least udhcpc from busybox)
The packet socket is only needed during "init_selecting", "init_reboot",
"requesting" and "rebinding" state.
For "bound" or "requesting" state (in which you spend most of the time),
the packet socket is not needed and a standard AF_INET dgram socket can
be used instead.
You can't close the socket while bound since you can receive DHCPINFORM
anytime.
--
Maxime
^ permalink raw reply
* Re: FEC driver: rcv is not +last
From: Matthias Kaehlcke @ 2010-04-08 14:53 UTC (permalink / raw)
To: Sascha Hauer; +Cc: netdev
In-Reply-To: <20100408132033.GJ3688@pengutronix.de>
El Thu, Apr 08, 2010 at 03:20:33PM +0200 Sascha Hauer ha dit:
> On Thu, Apr 08, 2010 at 12:40:33PM +0200, Matthias Kaehlcke wrote:
> > hi,
> >
> > i have problems with the FEC on a i.MX25 3-Stack board. the kernel is
> > v2.6.34-rc2 plus the following patch:
> > http://patchwork.ozlabs.org/patch/41235/
> >
> > the following traces are generated at boot time:
> >
> > FEC Ethernet Driver
> > fec: PHY @ 0x1, ID 0x20005ce1 -- unknown PHY!
> > ...
> > eth0: config: auto-negotiation on, 100FDX, 100HDX, 10FDX, 10HDX.
> > ...
> > FEC ENET: rcv is not +last
> > FEC ENET: rcv is not +last
> > FEC ENET: rcv is not +last
> > FEC ENET: rcv is not +last
> > ...
>
> No idea, I have never seen this message. Does the controller work
> besides these messages?
nope
> > the PHY of the board is a DP83840, which is not supported by the
> > driver. could this be the problem? i tried to make the kernel think
> > the DP83840 is a DP83848, which is supported, but the behaviour is the
> > same except the 'unknown PHY' warning.
>
> This should be solved by the phylib patches recently posted for the fec
> driver.
thanks for the pointer!
i just applied the patch. it actually makes the 'unkown PHY' and even
the 'FEC ENET: rcv is not +last' messages disappear, but networking
still doesn't work
--
Matthias Kaehlcke
Embedded Linux Developer
Barcelona
Control over the use of one's ideas really constitutes control over other
people's lives; and it is usually used to make their lives more difficult.
(Richard Stallman)
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
^ permalink raw reply
* [PATCH] udp: fix for unicast RX path optimization
From: Jorge Boncompte [DTI2] @ 2010-04-08 14:56 UTC (permalink / raw)
To: Linux Network Development list
Commits 5051ebd275de672b807c28d93002c2fb0514a3c9 and
5051ebd275de672b807c28d93002c2fb0514a3c9 ("ipv[46]: udp: optimize unicast RX
path") broke some programs.
After upgrading a L2TP server to 2.6.33 it started to fail, tunnels going up an
down, after the 10th tunnel came up. My modified rp-l2tp uses a global
unconnected socket bound to (INADDR_ANY, 1701) and one connected socket per
tunnel after parameter negotiation.
After ten sockets were open and due to mixed parameters to
udp[46]_lib_lookup2() kernel started to drop packets.
Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net>
---
diff -uNrp linux-2.6.33/net/ipv4/udp.c kernel-netns/net/ipv4/udp.c
--- linux-2.6.33/net/ipv4/udp.c 2010-02-24 19:52:17.000000000 +0100
+++ kernel-netns/net/ipv4/udp.c 2010-04-08 15:07:21.888717449 +0200
@@ -471,8 +471,8 @@ static struct sock *__udp4_lib_lookup(st
if (hslot->count < hslot2->count)
goto begin;
- result = udp4_lib_lookup2(net, INADDR_ANY, sport,
- daddr, hnum, dif,
+ result = udp4_lib_lookup2(net, saddr, sport,
+ INADDR_ANY, hnum, dif,
hslot2, slot2);
}
rcu_read_unlock();
diff -uNrp linux-2.6.33/net/ipv6/udp.c kernel-netns/net/ipv6/udp.c
--- linux-2.6.33/net/ipv6/udp.c 2010-02-24 19:52:17.000000000 +0100
+++ kernel-netns/net/ipv6/udp.c 2010-04-08 15:07:57.081396901 +0200
@@ -258,8 +258,8 @@ static struct sock *__udp6_lib_lookup(st
if (hslot->count < hslot2->count)
goto begin;
- result = udp6_lib_lookup2(net, &in6addr_any, sport,
- daddr, hnum, dif,
+ result = udp6_lib_lookup2(net, saddr, sport,
+ &in6addr_any, hnum, dif,
hslot2, slot2);
}
rcu_read_unlock();
^ permalink raw reply
* Re: dhcp client packet sniffing...
From: Herbert Xu @ 2010-04-08 15:07 UTC (permalink / raw)
To: Maxime Bizon; +Cc: David Miller, netdev
In-Reply-To: <1270737440.1872.93.camel@sakura.staff.proxad.net>
On Thu, Apr 08, 2010 at 04:37:20PM +0200, Maxime Bizon wrote:
>
> You can't close the socket while bound since you can receive DHCPINFORM
> anytime.
Right, as long as you don't use an AF_PACKET socket most of the
time it should be fine.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] udp: fix for unicast RX path optimization
From: Eric Dumazet @ 2010-04-08 15:12 UTC (permalink / raw)
To: jorge, David Miller; +Cc: Linux Network Development list
In-Reply-To: <4BBDEEB0.7060003@dti2.net>
Le jeudi 08 avril 2010 à 16:56 +0200, Jorge Boncompte [DTI2] a écrit :
> Commits 5051ebd275de672b807c28d93002c2fb0514a3c9 and
> 5051ebd275de672b807c28d93002c2fb0514a3c9 ("ipv[46]: udp: optimize unicast RX
> path") broke some programs.
>
> After upgrading a L2TP server to 2.6.33 it started to fail, tunnels going up an
> down, after the 10th tunnel came up. My modified rp-l2tp uses a global
> unconnected socket bound to (INADDR_ANY, 1701) and one connected socket per
> tunnel after parameter negotiation.
>
> After ten sockets were open and due to mixed parameters to
> udp[46]_lib_lookup2() kernel started to drop packets.
>
> Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net>
Oops...
Thank you for finding this bug, this is indeed the right fix.
This is a stable candidate. David, can you catch it even if Jorge didnt
sent you a copy ?
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
^ permalink raw reply
* Re: mmotm 2010-04-05-16-09 uploaded
From: Valdis.Kletnieks @ 2010-04-08 15:23 UTC (permalink / raw)
To: Patrick McHardy
Cc: Andrew Morton, Peter Zijlstra, Ingo Molnar, David S. Miller,
linux-kernel, netfilter-devel, netdev
In-Reply-To: <4BBDC0CC.7080305@trash.net>
[-- Attachment #1: Type: text/plain, Size: 3174 bytes --]
On Thu, 08 Apr 2010 13:41:00 +0200, Patrick McHardy said:
> Valdis.Kletnieks@vt.edu wrote:
> > On Mon, 05 Apr 2010 16:09:45 PDT, akpm@linux-foundation.org said:
> >> The mm-of-the-moment snapshot 2010-04-05-16-09 has been uploaded to
> >>
> >> http://userweb.kernel.org/~akpm/mmotm/
> >
> > Seen in dmesg, 2.6.34-rc2-mmotm0323 didn't do this. Tossing it at all the
> > likely suspects, hopefully somebody will recognize it and save me the
> > bisecting. ;)
> >
> > [ 11.488535] ctnetlink v0.93: registering with nfnetlink.
> > [ 11.488579]
> > [ 11.488579] ===================================================
> > [ 11.489529] [ INFO: suspicious rcu_dereference_check() usage. ]
> > [ 11.489988] ---------------------------------------------------
> > [ 11.490494] net/netfilter/nf_conntrack_ecache.c:88 invoked rcu_dereference_check() without protection!
> > [ 11.491024]
> > [ 11.491024] other info that might help us debug this:
> > [ 11.491025]
> > [ 11.492834]
> > [ 11.492835] rcu_scheduler_active = 1, debug_locks = 0
> > [ 11.494124] 1 lock held by swapper/1:
> > [ 11.494776] #0: (nf_ct_ecache_mutex){+.+...}, at: [<ffffffff8148c606>] nf_conntrack_register_notifier+0x1a/0x76
> > [ 11.495505]
>
> There are some unnecessary rcu_dereference() calls in the conntrack
> notifier registration and unregistration functions.
>
> Does this fix it?
Well, it *changed* it. Does the rcu_defererence_check() only fire on the
first time it hits something, so we've fixed the first one and now we get to
see the second one?
(For what it's worth, if this is going to be one-at-a-time whack-a-mole, I'm
OK on that, just want to know up front.)
[ 9.299425] ip_tables: (C) 2000-2006 Netfilter Core Team
[ 9.299486]
[ 9.299486] ===================================================
[ 9.300499] [ INFO: suspicious rcu_dereference_check() usage. ]
[ 9.301001] ---------------------------------------------------
[ 9.301523] net/netfilter/nf_log.c:55 invoked rcu_dereference_check() without protection!
[ 9.302066]
[ 9.302066] other info that might help us debug this:
[ 9.302067]
[ 9.303748]
[ 9.303748] rcu_scheduler_active = 1, debug_locks = 0
[ 9.304990] 1 lock held by swapper/1:
[ 9.305645] #0: (nf_log_mutex){+.+...}, at: [<ffffffff8148427a>] nf_log_register+0x57/0x111
[ 9.306342]
[ 9.306343] stack backtrace:
[ 9.307729] Pid: 1, comm: swapper Not tainted 2.6.34-rc3-mmotm0405 #2
[ 9.308447] Call Trace:
[ 9.309170] [<ffffffff810638c5>] lockdep_rcu_dereference+0xaa/0xb2
[ 9.309935] [<ffffffff81484301>] nf_log_register+0xde/0x111
[ 9.310688] [<ffffffff81b6064b>] ? log_tg_init+0x0/0x29
[ 9.311465] [<ffffffff81b60670>] log_tg_init+0x25/0x29
[ 9.312233] [<ffffffff810001ef>] do_one_initcall+0x59/0x14e
[ 9.313030] [<ffffffff81b3268a>] kernel_init+0x144/0x1ce
[ 9.313819] [<ffffffff81003514>] kernel_thread_helper+0x4/0x10
[ 9.314625] [<ffffffff8159cb80>] ? restore_args+0x0/0x30
[ 9.315434] [<ffffffff81b32546>] ? kernel_init+0x0/0x1ce
[ 9.316224] [<ffffffff81003510>] ? kernel_thread_helper+0x0/0x10
[ 9.317037] TCP bic registered
[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]
^ permalink raw reply
* Re: hackbench regression due to commit 9dfc6e68bfe6e
From: Christoph Lameter @ 2010-04-08 15:34 UTC (permalink / raw)
To: Eric Dumazet
Cc: Zhang, Yanmin, Pekka Enberg, netdev, Tejun Heo, alex.shi,
linux-kernel@vger.kernel.org, Ma, Ling, Chen, Tim C,
Andrew Morton
In-Reply-To: <1270705153.8141.58.camel@edumazet-laptop>
On Thu, 8 Apr 2010, Eric Dumazet wrote:
> I suspect NUMA is completely out of order on current kernel, or my
> Nehalem machine NUMA support is a joke
>
> # numactl --hardware
> available: 2 nodes (0-1)
> node 0 size: 3071 MB
> node 0 free: 2637 MB
> node 1 size: 3062 MB
> node 1 free: 2909 MB
How do the cpus map to the nodes? cpu 0 and 1 both on the same node?
> # ./try.sh
> Running with 50*40 (== 2000) tasks.
> Time: 16.865
> node0 results
> Running with 25*40 (== 1000) tasks.
> Time: 16.767
> node1 results
> Running with 25*40 (== 1000) tasks.
> Time: 16.564
> node0 on mem1 results
> Running with 25*40 (== 1000) tasks.
> Time: 16.814
> node1 on mem0 results
> Running with 25*40 (== 1000) tasks.
> Time: 16.896
>
>
>
^ permalink raw reply
* Re: mmotm 2010-04-05-16-09 uploaded
From: Patrick McHardy @ 2010-04-08 15:36 UTC (permalink / raw)
To: Valdis.Kletnieks
Cc: Andrew Morton, Peter Zijlstra, Ingo Molnar, David S. Miller,
linux-kernel, netfilter-devel, netdev
In-Reply-To: <6795.1270740197@localhost>
[-- Attachment #1: Type: text/plain, Size: 1543 bytes --]
Valdis.Kletnieks@vt.edu wrote:
> On Thu, 08 Apr 2010 13:41:00 +0200, Patrick McHardy said:
>>> [ 11.488579] ===================================================
>>> [ 11.489529] [ INFO: suspicious rcu_dereference_check() usage. ]
>>> [ 11.489988] ---------------------------------------------------
>>> [ 11.490494] net/netfilter/nf_conntrack_ecache.c:88 invoked rcu_dereference_check() without protection!
>>> [ 11.491024]
>> There are some unnecessary rcu_dereference() calls in the conntrack
>> notifier registration and unregistration functions.
>>
>> Does this fix it?
>
> Well, it *changed* it. Does the rcu_defererence_check() only fire on the
> first time it hits something, so we've fixed the first one and now we get to
> see the second one?
It appears that way, otherwise you should have seen a second warning in
nf_conntrack_ecache the last time.
> (For what it's worth, if this is going to be one-at-a-time whack-a-mole, I'm
> OK on that, just want to know up front.)
I went through the other files and I believe this should be it.
We already removed most of these incorrect rcu_dereference()
calls a while back.
> [ 9.299425] ip_tables: (C) 2000-2006 Netfilter Core Team
> [ 9.299486]
> [ 9.299486] ===================================================
> [ 9.300499] [ INFO: suspicious rcu_dereference_check() usage. ]
> [ 9.301001] ---------------------------------------------------
> [ 9.301523] net/netfilter/nf_log.c:55 invoked rcu_dereference_check() without protection!
> [ 9.302066]
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 3053 bytes --]
diff --git a/net/netfilter/nf_conntrack_ecache.c b/net/netfilter/nf_conntrack_ecache.c
index d5a9bcd..849614a 100644
--- a/net/netfilter/nf_conntrack_ecache.c
+++ b/net/netfilter/nf_conntrack_ecache.c
@@ -81,11 +81,9 @@ EXPORT_SYMBOL_GPL(nf_ct_deliver_cached_events);
int nf_conntrack_register_notifier(struct nf_ct_event_notifier *new)
{
int ret = 0;
- struct nf_ct_event_notifier *notify;
mutex_lock(&nf_ct_ecache_mutex);
- notify = rcu_dereference(nf_conntrack_event_cb);
- if (notify != NULL) {
+ if (nf_conntrack_event_cb != NULL) {
ret = -EBUSY;
goto out_unlock;
}
@@ -101,11 +99,8 @@ EXPORT_SYMBOL_GPL(nf_conntrack_register_notifier);
void nf_conntrack_unregister_notifier(struct nf_ct_event_notifier *new)
{
- struct nf_ct_event_notifier *notify;
-
mutex_lock(&nf_ct_ecache_mutex);
- notify = rcu_dereference(nf_conntrack_event_cb);
- BUG_ON(notify != new);
+ BUG_ON(nf_conntrack_event_cb != new);
rcu_assign_pointer(nf_conntrack_event_cb, NULL);
mutex_unlock(&nf_ct_ecache_mutex);
}
@@ -114,11 +109,9 @@ EXPORT_SYMBOL_GPL(nf_conntrack_unregister_notifier);
int nf_ct_expect_register_notifier(struct nf_exp_event_notifier *new)
{
int ret = 0;
- struct nf_exp_event_notifier *notify;
mutex_lock(&nf_ct_ecache_mutex);
- notify = rcu_dereference(nf_expect_event_cb);
- if (notify != NULL) {
+ if (nf_expect_event_cb != NULL) {
ret = -EBUSY;
goto out_unlock;
}
@@ -134,11 +127,8 @@ EXPORT_SYMBOL_GPL(nf_ct_expect_register_notifier);
void nf_ct_expect_unregister_notifier(struct nf_exp_event_notifier *new)
{
- struct nf_exp_event_notifier *notify;
-
mutex_lock(&nf_ct_ecache_mutex);
- notify = rcu_dereference(nf_expect_event_cb);
- BUG_ON(notify != new);
+ BUG_ON(nf_expect_event_cb != new);
rcu_assign_pointer(nf_expect_event_cb, NULL);
mutex_unlock(&nf_ct_ecache_mutex);
}
diff --git a/net/netfilter/nf_log.c b/net/netfilter/nf_log.c
index 015725a..908f599 100644
--- a/net/netfilter/nf_log.c
+++ b/net/netfilter/nf_log.c
@@ -35,7 +35,6 @@ static struct nf_logger *__find_logger(int pf, const char *str_logger)
/* return EEXIST if the same logger is registred, 0 on success. */
int nf_log_register(u_int8_t pf, struct nf_logger *logger)
{
- const struct nf_logger *llog;
int i;
if (pf >= ARRAY_SIZE(nf_loggers))
@@ -52,8 +51,7 @@ int nf_log_register(u_int8_t pf, struct nf_logger *logger)
} else {
/* register at end of list to honor first register win */
list_add_tail(&logger->list[pf], &nf_loggers_l[pf]);
- llog = rcu_dereference(nf_loggers[pf]);
- if (llog == NULL)
+ if (nf_loggers[pf] == NULL)
rcu_assign_pointer(nf_loggers[pf], logger);
}
@@ -65,13 +63,11 @@ EXPORT_SYMBOL(nf_log_register);
void nf_log_unregister(struct nf_logger *logger)
{
- const struct nf_logger *c_logger;
int i;
mutex_lock(&nf_log_mutex);
for (i = 0; i < ARRAY_SIZE(nf_loggers); i++) {
- c_logger = rcu_dereference(nf_loggers[i]);
- if (c_logger == logger)
+ if (nf_loggers[i] == logger)
rcu_assign_pointer(nf_loggers[i], NULL);
list_del(&logger->list[i]);
}
^ permalink raw reply related
* Re: [v3 Patch 2/3] bridge: make bridge support netpoll
From: Stephen Hemminger @ 2010-04-08 15:37 UTC (permalink / raw)
To: Amerigo Wang
Cc: linux-kernel, netdev, bridge, Andy Gospodarek, Neil Horman,
Jeff Moyer, Matt Mackall, bonding-devel, Jay Vosburgh,
David Miller
In-Reply-To: <20100408062246.4499.5670.sendpatchset@localhost.localdomain>
On Thu, 8 Apr 2010 02:18:58 -0400
Amerigo Wang <amwang@redhat.com> wrote:
>
> Based on the previous patch, make bridge support netpoll by:
>
> 1) implement the 2 methods to support netpoll for bridge;
>
> 2) modify netpoll during forwarding packets via bridge;
>
> 3) disable netpoll support of bridge when a netpoll-unabled device
> is added to bridge;
>
> 4) enable netpoll support when all underlying devices support netpoll.
>
> Cc: David Miller <davem@davemloft.net>
> Cc: Neil Horman <nhorman@tuxdriver.com>
> Cc: Stephen Hemminger <shemminger@linux-foundation.org>
> Cc: Matt Mackall <mpm@selenic.com>
> Signed-off-by: WANG Cong <amwang@redhat.com>
>
> ---
>
> Index: linux-2.6/net/bridge/br_device.c
> ===================================================================
> --- linux-2.6.orig/net/bridge/br_device.c
> +++ linux-2.6/net/bridge/br_device.c
> @@ -13,8 +13,10 @@
>
> #include <linux/kernel.h>
> #include <linux/netdevice.h>
> +#include <linux/netpoll.h>
> #include <linux/etherdevice.h>
> #include <linux/ethtool.h>
> +#include <linux/list.h>
>
> #include <asm/uaccess.h>
> #include "br_private.h"
> @@ -162,6 +164,59 @@ static int br_set_tx_csum(struct net_dev
> return 0;
> }
>
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> +bool br_devices_support_netpoll(struct net_bridge *br)
> +{
> + struct net_bridge_port *p;
> + bool ret = true;
> + int count = 0;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&br->lock, flags);
> + list_for_each_entry(p, &br->port_list, list) {
> + count++;
> + if (p->dev->priv_flags & IFF_DISABLE_NETPOLL
> + || !p->dev->netdev_ops->ndo_poll_controller)
> + ret = false;
> + }
> + spin_unlock_irqrestore(&br->lock, flags);
> + return count != 0 && ret;
> +}
> +
> +static void br_poll_controller(struct net_device *br_dev)
> +{
> + struct netpoll *np = br_dev->npinfo->netpoll;
> +
> + if (np->real_dev != br_dev)
> + netpoll_poll_dev(np->real_dev);
> +}
> +
> +void br_netpoll_cleanup(struct net_device *br_dev)
> +{
> + struct net_bridge *br = netdev_priv(br_dev);
> + struct net_bridge_port *p, *n;
> + const struct net_device_ops *ops;
> +
> + br->dev->npinfo = NULL;
> + list_for_each_entry_safe(p, n, &br->port_list, list) {
> + if (p->dev) {
> + ops = p->dev->netdev_ops;
> + if (ops->ndo_netpoll_cleanup)
> + ops->ndo_netpoll_cleanup(p->dev);
> + else
> + p->dev->npinfo = NULL;
> + }
> + }
> +}
> +
> +#else
> +
> +void br_netpoll_cleanup(struct net_device *br_dev)
> +{
> +}
> +
> +#endif
Could you use more stub functions to eliminate #ifdef's in code.
> static const struct ethtool_ops br_ethtool_ops = {
> .get_drvinfo = br_getinfo,
> .get_link = ethtool_op_get_link,
> @@ -184,6 +239,10 @@ static const struct net_device_ops br_ne
> .ndo_set_multicast_list = br_dev_set_multicast_list,
> .ndo_change_mtu = br_change_mtu,
> .ndo_do_ioctl = br_dev_ioctl,
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + .ndo_netpoll_cleanup = br_netpoll_cleanup,
> + .ndo_poll_controller = br_poll_controller,
> +#endif
> };
>
> void br_dev_setup(struct net_device *dev)
> Index: linux-2.6/net/bridge/br_forward.c
> ===================================================================
> --- linux-2.6.orig/net/bridge/br_forward.c
> +++ linux-2.6/net/bridge/br_forward.c
> @@ -15,6 +15,7 @@
> #include <linux/slab.h>
> #include <linux/kernel.h>
> #include <linux/netdevice.h>
> +#include <linux/netpoll.h>
> #include <linux/skbuff.h>
> #include <linux/if_vlan.h>
> #include <linux/netfilter_bridge.h>
> @@ -50,7 +51,13 @@ int br_dev_queue_push_xmit(struct sk_buf
> else {
> skb_push(skb, ETH_HLEN);
>
> - dev_queue_xmit(skb);
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + if (skb->dev->priv_flags & IFF_IN_NETPOLL) {
> + netpoll_send_skb(skb->dev->npinfo->netpoll, skb);
> + skb->dev->priv_flags &= ~IFF_IN_NETPOLL;
> + } else
> +#endif
There is no protection on dev->priv_flags for SMP access.
It would better bit value in dev->state if you are using it as control flag.
Then you could use
if (unlikely(test_and_clear_bit(__IN_NETPOLL, &skb->dev->state)))
netpoll_send_skb(...)
> + dev_queue_xmit(skb);
> }
> }
>
> @@ -66,9 +73,23 @@ int br_forward_finish(struct sk_buff *sk
>
> static void __br_deliver(const struct net_bridge_port *to, struct sk_buff *skb)
> {
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + struct net_bridge *br = to->br;
> + if (br->dev->priv_flags & IFF_IN_NETPOLL) {
> + struct netpoll *np;
> + to->dev->npinfo = skb->dev->npinfo;
> + np = skb->dev->npinfo->netpoll;
> + np->real_dev = np->dev = to->dev;
> + to->dev->priv_flags |= IFF_IN_NETPOLL;
> + }
> +#endif
This is n hot path, so use unlikely()
> skb->dev = to->dev;
> NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_OUT, skb, NULL, skb->dev,
> br_forward_finish);
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + if (skb->dev->npinfo)
> + skb->dev->npinfo->netpoll->dev = br->dev;
> +#endif
> }
>
> static void __br_forward(const struct net_bridge_port *to, struct sk_buff *skb)
> Index: linux-2.6/net/bridge/br_if.c
> ===================================================================
> --- linux-2.6.orig/net/bridge/br_if.c
> +++ linux-2.6/net/bridge/br_if.c
> @@ -13,6 +13,7 @@
>
> #include <linux/kernel.h>
> #include <linux/netdevice.h>
> +#include <linux/netpoll.h>
> #include <linux/ethtool.h>
> #include <linux/if_arp.h>
> #include <linux/module.h>
> @@ -153,6 +154,14 @@ static void del_nbp(struct net_bridge_po
> kobject_uevent(&p->kobj, KOBJ_REMOVE);
> kobject_del(&p->kobj);
>
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + if (br_devices_support_netpoll(br))
> + br->dev->priv_flags &= ~IFF_DISABLE_NETPOLL;
> + if (dev->netdev_ops->ndo_netpoll_cleanup)
> + dev->netdev_ops->ndo_netpoll_cleanup(dev);
> + else
> + dev->npinfo = NULL;
> +#endif
> call_rcu(&p->rcu, destroy_nbp_rcu);
> }
>
> @@ -165,6 +174,8 @@ static void del_br(struct net_bridge *br
> del_nbp(p);
> }
>
> + br_netpoll_cleanup(br->dev);
> +
> del_timer_sync(&br->gc_timer);
>
> br_sysfs_delbr(br->dev);
> @@ -438,6 +449,20 @@ int br_add_if(struct net_bridge *br, str
>
> kobject_uevent(&p->kobj, KOBJ_ADD);
>
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> + if (br_devices_support_netpoll(br)) {
> + br->dev->priv_flags &= ~IFF_DISABLE_NETPOLL;
> + if (br->dev->npinfo)
> + dev->npinfo = br->dev->npinfo;
> + } else if (!(br->dev->priv_flags & IFF_DISABLE_NETPOLL)) {
> + br->dev->priv_flags |= IFF_DISABLE_NETPOLL;
> + printk(KERN_INFO "New device %s does not support netpoll\n",
> + dev->name);
> + printk(KERN_INFO "Disabling netpoll for %s\n",
> + br->dev->name);
One message is sufficient.
--
^ permalink raw reply
* Re: [PATCH] tcp: Set CHECKSUM_UNNECESSARY in tcp_init_nondata_skb
From: Herbert Xu @ 2010-04-08 15:39 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100408135715.GA24890@gondor.apana.org.au>
On Thu, Apr 08, 2010 at 09:57:15PM +0800, Herbert Xu wrote:
>
> The problem here is that for non-data packets CHECKSUM_PARTIAL
> can actually end up being worse if we wind up going out through
> an interface that doesn't support checksums.
I don't know what I was thinking but the above is totally wrong.
CHECKSUM_PARTIAL should be just fine on non-checksuming interfaces
as we'll checksum everything once just as the CHECKSUM_NONE case
would.
So with that in mind, we don't need my CHECKSUM_UNNECESSARY patch
at all and your CHECKSUM_PARTIAL path is the right solution after
all :)
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: hackbench regression due to commit 9dfc6e68bfe6e
From: Eric Dumazet @ 2010-04-08 15:52 UTC (permalink / raw)
To: Christoph Lameter
Cc: Zhang, Yanmin, Pekka Enberg, netdev, Tejun Heo, alex.shi,
linux-kernel@vger.kernel.org, Ma, Ling, Chen, Tim C,
Andrew Morton
In-Reply-To: <alpine.DEB.2.00.1004081033420.6321@router.home>
Le jeudi 08 avril 2010 à 10:34 -0500, Christoph Lameter a écrit :
> On Thu, 8 Apr 2010, Eric Dumazet wrote:
>
> > I suspect NUMA is completely out of order on current kernel, or my
> > Nehalem machine NUMA support is a joke
> >
> > # numactl --hardware
> > available: 2 nodes (0-1)
> > node 0 size: 3071 MB
> > node 0 free: 2637 MB
> > node 1 size: 3062 MB
> > node 1 free: 2909 MB
>
> How do the cpus map to the nodes? cpu 0 and 1 both on the same node?
one socket maps to 0 2 4 6 8 10 12 14 (Node 0)
one socket maps to 1 3 5 7 9 11 13 15 (Node 1)
# numactl --cpubind=0 --membind=0 numactl --show
policy: bind
preferred node: 0
interleavemask:
interleavenode: 0
nodebind: 0
membind: 0
cpubind: 1 3 5 7 9 11 13 15 1024
(strange 1024 report...)
# numactl --cpubind=1 --membind=1 numactl --show
policy: bind
preferred node: 1
interleavemask:
interleavenode: 0
nodebind:
membind: 1
cpubind: 0 2 4 6 8 10 12 14
[ 0.161170] Booting Node 0, Processors #1
[ 0.248995] CPU 1 MCA banks CMCI:2 CMCI:3 CMCI:5 CMCI:6 SHD:8
[ 0.269177] Ok.
[ 0.269453] Booting Node 1, Processors #2
[ 0.356965] CPU 2 MCA banks CMCI:2 CMCI:3 CMCI:5 SHD:6 SHD:8
[ 0.377207] Ok.
[ 0.377485] Booting Node 0, Processors #3
[ 0.464935] CPU 3 MCA banks CMCI:2 CMCI:3 CMCI:5 SHD:6 SHD:8
[ 0.485065] Ok.
[ 0.485217] Booting Node 1, Processors #4
[ 0.572906] CPU 4 MCA banks CMCI:2 CMCI:3 CMCI:5 SHD:6 SHD:8
[ 0.593044] Ok.
...
grep "physical id" /proc/cpuinfo
physical id : 1
physical id : 0
physical id : 1
physical id : 0
physical id : 1
physical id : 0
physical id : 1
physical id : 0
physical id : 1
physical id : 0
physical id : 1
physical id : 0
physical id : 1
physical id : 0
physical id : 1
physical id : 0
^ permalink raw reply
* Re: FEC driver: rcv is not +last
From: Sascha Hauer @ 2010-04-08 16:05 UTC (permalink / raw)
To: Matthias Kaehlcke; +Cc: netdev
In-Reply-To: <20100408145324.GH29538@darwin>
On Thu, Apr 08, 2010 at 04:53:24PM +0200, Matthias Kaehlcke wrote:
> El Thu, Apr 08, 2010 at 03:20:33PM +0200 Sascha Hauer ha dit:
>
> > On Thu, Apr 08, 2010 at 12:40:33PM +0200, Matthias Kaehlcke wrote:
> > > hi,
> > >
> > > i have problems with the FEC on a i.MX25 3-Stack board. the kernel is
> > > v2.6.34-rc2 plus the following patch:
> > > http://patchwork.ozlabs.org/patch/41235/
> > >
> > > the following traces are generated at boot time:
> > >
> > > FEC Ethernet Driver
> > > fec: PHY @ 0x1, ID 0x20005ce1 -- unknown PHY!
> > > ...
> > > eth0: config: auto-negotiation on, 100FDX, 100HDX, 10FDX, 10HDX.
> > > ...
> > > FEC ENET: rcv is not +last
> > > FEC ENET: rcv is not +last
> > > FEC ENET: rcv is not +last
> > > FEC ENET: rcv is not +last
> > > ...
> >
> > No idea, I have never seen this message. Does the controller work
> > besides these messages?
>
> nope
>
> > > the PHY of the board is a DP83840, which is not supported by the
> > > driver. could this be the problem? i tried to make the kernel think
> > > the DP83840 is a DP83848, which is supported, but the behaviour is the
> > > same except the 'unknown PHY' warning.
> >
> > This should be solved by the phylib patches recently posted for the fec
> > driver.
>
> thanks for the pointer!
>
> i just applied the patch. it actually makes the 'unkown PHY' and even
> the 'FEC ENET: rcv is not +last' messages disappear, but networking
> still doesn't work
AFAIK there is nothing special about the fec in the i.MX25 except the
RMII mode, so just some more things you can check:
- Does it work in the bootloader? (Are you using barebox?)
- Are packets transmitted/received at all?
- Have you configured the MAC address? currently the fec driver relies
on finding a valid MAC address in the FEC_ADDR_LOW/FEC_ADDR_HIGH
registers
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: FEC driver: rcv is not +last
From: Matthias Kaehlcke @ 2010-04-08 16:04 UTC (permalink / raw)
To: Sascha Hauer; +Cc: netdev
In-Reply-To: <20100408145324.GH29538@darwin>
El Thu, Apr 08, 2010 at 04:53:24PM +0200 Matthias Kaehlcke ha dit:
> El Thu, Apr 08, 2010 at 03:20:33PM +0200 Sascha Hauer ha dit:
>
> > On Thu, Apr 08, 2010 at 12:40:33PM +0200, Matthias Kaehlcke wrote:
> > > hi,
> > >
> > > i have problems with the FEC on a i.MX25 3-Stack board. the kernel is
> > > v2.6.34-rc2 plus the following patch:
> > > http://patchwork.ozlabs.org/patch/41235/
> > >
> > > the following traces are generated at boot time:
> > >
> > > FEC Ethernet Driver
> > > fec: PHY @ 0x1, ID 0x20005ce1 -- unknown PHY!
> > > ...
> > > eth0: config: auto-negotiation on, 100FDX, 100HDX, 10FDX, 10HDX.
> > > ...
> > > FEC ENET: rcv is not +last
> > > FEC ENET: rcv is not +last
> > > FEC ENET: rcv is not +last
> > > FEC ENET: rcv is not +last
> > > ...
> >
> > No idea, I have never seen this message. Does the controller work
> > besides these messages?
>
> nope
>
> > > the PHY of the board is a DP83840, which is not supported by the
> > > driver. could this be the problem? i tried to make the kernel think
> > > the DP83840 is a DP83848, which is supported, but the behaviour is the
> > > same except the 'unknown PHY' warning.
> >
> > This should be solved by the phylib patches recently posted for the fec
> > driver.
>
> thanks for the pointer!
>
> i just applied the patch. it actually makes the 'unkown PHY' and even
> the 'FEC ENET: rcv is not +last' messages disappear, but networking
> still doesn't work
i just received a mail from our distributor:
the PDK board they gave us for evaluation is a preliminary version,
with a slightly different hardware than the official one. they suppose
that's the reason of our problem
sorry for the noise
--
Matthias Kaehlcke
Embedded Linux Developer
Barcelona
Debugging is like alien abduction. Large blocks of time disappear,
for which you have no explanation
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
^ permalink raw reply
* Re: [PATCH net-next-2.6 1/3 (TAKE 3 RESENT)] ipv6 mcast: Introduce include/net/mld.h for MLD definitions.
From: Stephen Hemminger @ 2010-04-08 16:47 UTC (permalink / raw)
To: YOSHIFUJI Hideaki; +Cc: davem, netdev
In-Reply-To: <201004080659.o386xIXP019382@94.43.138.210.xn.2iij.net>
Is there a piece missing. I tried this against net-next-2.6
--
CC [M] net/ipv6/mcast.o
net/ipv6/mcast.c: In function ‘igmp6_event_query’:
net/ipv6/mcast.c:1134: error: implicit declaration of function ‘mld_msg’
net/ipv6/mcast.c:1134: warning: assignment makes pointer from integer without a cast
net/ipv6/mcast.c: In function ‘igmp6_event_report’:
net/ipv6/mcast.c:1254: warning: assignment makes pointer from integer without a cast
^ 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