Re: [PATCH] libertas: skb dereferenced after netif_rx

2007-05-20 Thread David Miller
From: Jeff Garzik <[EMAIL PROTECTED]>
Date: Sat, 19 May 2007 21:47:00 -0400

> Dan Williams wrote:
> > On Fri, 2007-05-18 at 14:09 -0400, John W. Linville wrote:
> >> On Wed, May 16, 2007 at 05:01:27PM -0400, Florin Malita wrote:
> >>> In libertas_process_rxed_packet() and process_rxed_802_11_packet() the 
> >>> skb is dereferenced after being passed to netif_rx (called from 
> >>> libertas_upload_rx_packet). Spotted by Coverity (1658, 1659).
> >>  
> >> Relocating the libertas_upload_rx_packet call is fine, but...
> >>
> >>> Also, libertas_upload_rx_packet() unconditionally returns 0 so the error 
> >>> check is dead code - might as well take it out.
> >> Is this merely an implementation detail?  Or an absolute fact?
> >> If the former is true, then we should preserve the error
> >> checking.  If the latter, then we should change the signature of
> >> libertas_upload_rx_packet to return void.
> > 
> > According to the comments, netif_rx always succeeds.  I think we should
> > just change the return type to void since there's nothing else in that
> > function that can fail.
> 
> According to the implementation, netif_rx() can fail.

It doesn't exactly "fail", but it does give return values
which indicate RX congestion.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Bugme-new] [Bug 8491] New: OOPS triggered by ip(8) deconfiguring a network interface

2007-05-20 Thread Herbert Xu
Andrew Morton <[EMAIL PROTECTED]> wrote:
>
> [43092.388000] Stack: e30f433c cab6 c028608e  e2e8ae40  
> e2e8a5c0 dfe0aa58 
> [43092.388000]c826ca00 c826ca00 f10a72ff f1088989 00a33d71 80fe 
>  0001 
> [43092.388000]cab6 e2e8a5c0 dfe0aa58  c826ca00 f1089594 
> c826ca54 0040 
> [43092.388000] Call Trace:
> [43092.388000]  [] pneigh_queue_purge+0x1e/0x30
> [43092.388000]  [] snmp6_unregister_dev+0x2f/0x40 [ipv6]

Looks like a problem with your resume.  The second argument to
remove_proc_entry, parent, has the value 00a33d71 which is clearly
bogus.  That value in turn comes from proc_net_devsnmp6.  So somehow
during the suspend/resume that pointer has been corrupted.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[NET] napi: Call __netif_rx_complete in netif_rx_complete

2007-05-20 Thread Herbert Xu
Hi Dave:

[NET] napi: Call __netif_rx_complete in netif_rx_complete

This patch kills a little bit of code duplication between the two
variants of netif_rx_complete.

Signed-off-by: Herbert Xu <[EMAIL PROTECTED]>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f671cd2..3a70f55 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -910,6 +910,17 @@ static inline int netif_rx_reschedule(struct net_device 
*dev, int undo)
return 0;
 }
 
+/* same as netif_rx_complete, except that local_irq_save(flags)
+ * has already been issued
+ */
+static inline void __netif_rx_complete(struct net_device *dev)
+{
+   BUG_ON(!test_bit(__LINK_STATE_RX_SCHED, &dev->state));
+   list_del(&dev->poll_list);
+   smp_mb__before_clear_bit();
+   clear_bit(__LINK_STATE_RX_SCHED, &dev->state);
+}
+
 /* Remove interface from poll list: it must be in the poll list
  * on current cpu. This primitive is called by dev->poll(), when
  * it completes the work. The device cannot be out of poll list at this
@@ -920,10 +931,7 @@ static inline void netif_rx_complete(struct net_device 
*dev)
unsigned long flags;
 
local_irq_save(flags);
-   BUG_ON(!test_bit(__LINK_STATE_RX_SCHED, &dev->state));
-   list_del(&dev->poll_list);
-   smp_mb__before_clear_bit();
-   clear_bit(__LINK_STATE_RX_SCHED, &dev->state);
+   __netif_rx_complete(dev);
local_irq_restore(flags);
 }
 
@@ -940,17 +948,6 @@ static inline void netif_poll_enable(struct net_device 
*dev)
clear_bit(__LINK_STATE_RX_SCHED, &dev->state);
 }
 
-/* same as netif_rx_complete, except that local_irq_save(flags)
- * has already been issued
- */
-static inline void __netif_rx_complete(struct net_device *dev)
-{
-   BUG_ON(!test_bit(__LINK_STATE_RX_SCHED, &dev->state));
-   list_del(&dev->poll_list);
-   smp_mb__before_clear_bit();
-   clear_bit(__LINK_STATE_RX_SCHED, &dev->state);
-}
-
 static inline void netif_tx_lock(struct net_device *dev)
 {
spin_lock(&dev->_xmit_lock);
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: e1000: assertion hit in e1000_clean(), kernel 2.6.21.1

2007-05-20 Thread Herbert Xu
Kok, Auke <[EMAIL PROTECTED]> wrote:
>
>> The source file has four extra lines at the top because of a
>> trivial wireless patch, so 898 in that code is really 894 in
>> the stock kernel.
> 
> please shared that code then.

I've had a look and e1000 is definitely buggy.

The problem is that you're calling netif_poll_enable on startup.
This is *wrong*.

netif_poll_enable can only be called if you've previously called
netif_poll_disable.  Otherwise a poll might already be in action
and you may get a crash like this.

So perhaps you should divide e1000_up into two sections, one that
is called on both start and restart and another which is only
called on restart (i.e., after e1000_down).

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [5/5] 2.6.22-rc2: known regressions

2007-05-20 Thread Herbert Xu
Stephen Hemminger <[EMAIL PROTECTED]> wrote:
> 
> Looks more like the IPV6 SNMP6 OOPS, I saw and fixed with:
> 
> commit 5632c5152aa621885d87ea0b8fdd5a6bb9f69c6f
> Author: Stephen Hemminger <[EMAIL PROTECTED]>
> Date:   Sat Apr 28 21:16:39 2007 -0700
> 
>[IPV6]: Track device renames in snmp6.
>
>When network device's are renamed, the IPV6 snmp6 code
>gets confused. It doesn't track name changes so it will OOPS
>when network device's are removed.
>
>The fix is trivial, just unregister/re-register in notify handler.
>
>Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]>
>Signed-off-by: David S. Miller <[EMAIL PROTECTED]>

It's quite possible that this fixes it, especially since the version
in Ubuntu's bug report came out the day before your fix went in :)

However, I don't really get why it's crashing in the first place.
Without your patch, if the device is renamed the proc entry just
keeps the original name.  When it goes down it should still get
removed correctly.

Could you explain this in a bit more detail?

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: STRANGE ERROR

2007-05-20 Thread Vitaly Bordug
On Sat, May 19, 2007 at 16:34 -0700, Andrew Morton wrote:
> On Sun, 20 May 2007 00:30:55 +0200 "Sasa Ostrouska" <[EMAIL PROTECTED]> wrote:
> 
> > Hi everybody,
> > 
> > I tried today to upgrade the kernel to 2.6.21.1 and i got the same
> > error during the boot time.
> > Here is the dmesg of the 2.6.20.2, can somebody tell me what this is ?
> > 
> > ...
> >
> > Marvell 88E1101: Registered new driver
> > Fixed PHY: Registered new driver
> > driver_bound: device [EMAIL PROTECTED]:1 already bound
> 
> I don't know what caused that one.
>

this is because of issue in fixed phy driver initialisation
- have a patch but need to test it a bit more.
 
> > Device '[EMAIL PROTECTED]:1' does not have a release() function, it is 
> > broken
> > and must be fixed.
> > BUG: at drivers/base/core.c:104 device_release()
> > 
> > Call Trace:
> >  [] kobject_cleanup+0x53/0x7e
> >  [] kobject_release+0x0/0x9
> >  [] kref_put+0x74/0x81
> >  [] fixed_mdio_register_device+0x230/0x265
> >  [] fixed_init+0x1f/0x35
> >  [] init+0x147/0x2fb
> >  [] schedule_tail+0x36/0x92
> >  [] child_rip+0xa/0x12
> >  [] acpi_ds_init_one_object+0x0/0x83
> >  [] init+0x0/0x2fb
> >  [] child_rip+0x0/0x12
> 
> This appears to have happened because fixed_mdio_register_device() (or
> phy_device_create) didn't suitably initialise phy_device.dev.
> 
> But I don't immediately see why this doesn't affect all phy drivers. 
> Presumably it's the fixed driver which is at fault.  Jeff, how is this
> supposed to work?
> 
the fixed phy used to have "specific" bus bound stuff 
but I've reworked this point.

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: e1000: assertion hit in e1000_clean(), kernel 2.6.21.1

2007-05-20 Thread Kok, Auke

Herbert Xu wrote:

Kok, Auke <[EMAIL PROTECTED]> wrote:

The source file has four extra lines at the top because of a
trivial wireless patch, so 898 in that code is really 894 in
the stock kernel.

please shared that code then.


I've had a look and e1000 is definitely buggy.

The problem is that you're calling netif_poll_enable on startup.
This is *wrong*.

netif_poll_enable can only be called if you've previously called
netif_poll_disable.  Otherwise a poll might already be in action
and you may get a crash like this.

So perhaps you should divide e1000_up into two sections, one that
is called on both start and restart and another which is only
called on restart (i.e., after e1000_down).


OK, that would explain the recent frenzy of reports in this matter. That code 
was only recently merged. I will dig into this and get a patch out as soon as I 
can so you can test this.


Thanks Herbert.

Auke
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[IPV6] ADDRCONF: Fix conflicts in DEVCONF_xxx constant.

2007-05-20 Thread YOSHIFUJI Hideaki / 吉藤英明
Signed-off-by: YOSHIFUJI Hideaki <[EMAIL PROTECTED]>

--
diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 09ea01a..648bd1f 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -209,9 +209,8 @@ enum {
DEVCONF_RTR_PROBE_INTERVAL,
DEVCONF_ACCEPT_RA_RT_INFO_MAX_PLEN,
DEVCONF_PROXY_NDP,
-   __DEVCONF_OPTIMISTIC_DAD,
-   DEVCONF_ACCEPT_SOURCE_ROUTE,
DEVCONF_OPTIMISTIC_DAD,
+   DEVCONF_ACCEPT_SOURCE_ROUTE,
DEVCONF_MAX
 };
 

-- 
YOSHIFUJI Hideaki @ USAGI Project  <[EMAIL PROTECTED]>
GPG-FP  : 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Sending ipv6 packets from a kernel module

2007-05-20 Thread Anton

Hi all,

Please excuse me if the following question has already been asked on the 
mailing list, I am a little new to this.


I am trying to send IPv6 packets from a kernel module. As far as I 
understand, I would have to use the ip6_output() function, and this 
function is certainly present in the net/ipv6/ip6_output.c file (on my

2.6.18 kernel). Unfortunately, the function is not exported from the
IPv6 module and so my kernel module is unable to use it. Has anyone 
encountered such a problem before, and if not, does anyone know what is 
required for me to push a sk_buff into the IP stack for transmission 
(from a kernel module)?

Any ideas would be appreciated!

Regards,
Anton



--

IMPORTANT: This email remains the property of the Australian Defence 
Organisation and is subject to the jurisdiction of section 70 of the 
CRIMES ACT 1914.  If you have received this email in error, you are 
requested to contact the sender and delete the email.



-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] ibm_emac: Correctly detect old link speed

2007-05-20 Thread David Gibson
On Wed, May 16, 2007 at 08:44:47PM +0200, Stefan Roese wrote:
> On Wednesday 16 May 2007, Eugene Surovegin wrote:
> > On Wed, May 16, 2007 at 01:00:08PM +0200, Stefan Roese wrote:
> > > This patch fixes a bug where the link speed change was not
> > > detected correctly. This occured on a 440SPe (EMAC4) system
> > > where the old link speed was 100Mbps and the new link speed
> > > is 1000Mbps.
> >
> > Good catch, Stefan. Unfortunately, I have to NACK your patch - you
> > broke non EMAC4 builds.
> 
> Yes, you're right of course.
> 
> > Correct fix is just to remove EMAC_MR1_MF_1000GPCS from the first
> > if condition.
> 
> Yep.
> 
> > I'll send correct fix shortly along with other queued patches.
> 
> Thanks.

I've merged essentially the same fix into the device tree aware
"new_emac" driver.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html