Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Olivier Sobrie @ 2012-08-02 12:16 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: Wolfgang Grandegger, linux-can, netdev
In-Reply-To: <501A6AE7.9060508@pengutronix.de>

On Thu, Aug 02, 2012 at 01:56:23PM +0200, Marc Kleine-Budde wrote:
> BTW: is it possible to update the firmware on these devices?

Yes it possible. But I don't know how to do it... I never did it.
I saw on their website that they propose a zipfile with new firmwares.
Inside the zipfile there is a tool 'Update.exe' which, I suppose, does
the upgrade.

Kr,

-- 
Olivier

^ permalink raw reply

* Re: [PATCH] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Marc Kleine-Budde @ 2012-08-02 11:56 UTC (permalink / raw)
  To: Olivier Sobrie; +Cc: Wolfgang Grandegger, linux-can, netdev
In-Reply-To: <20120802105358.GA23787@hposo>

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

On 08/02/2012 12:53 PM, Olivier Sobrie wrote:
>>> 1) With the short circuit:
>>>
>>> I perform the test you described. It showed that the Kvaser passes from
>>> ERROR-WARNING to ERROR-PASSIVE and then BUS-OFF. But after going to the
>>> state BUS-OFF it comes back to ERROR-WARNING.
>>>
>>>   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
>>>   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
>>>   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
>>>   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
>>>   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
>>>   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
>>>   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
>>>   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
>>>   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
>>
>> Why don't we have any rx/tx numbers in the error frame?
> 
> Because the hardware seems to not update the tx/rx_errors_count
> fields :-(

Okay.

>> From the hardware point of view the short circuit and open end tests
>> look good. Please adjust the driver to turn off the CAN interface in
>> case of a bus off if restart_ms is 0.
> 
> And in the case where restart_ms is not 0? Don't I've to put it off so
> and drop the frame?

No, don't drop the frame. restart-ms != 0 means the controller is
automatically restarted after the specified time (if the controller
supports). Or in your and the at91 case, automatically.

> I actually implemeted it as you said and here is what I observed in
> candump output with restart_ms set to 100 ms:
> 
> t0: Short circuit between CAN-H and CAN-L + cansend can1 123#1122
>   can1  2000008C  [8] 00 04 90 00 00 00 00 00   ERRORFRAME
> 	controller-problem{rx-error-warning}
> 	protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> 	bus-error
>   can1  2000008C  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
> 	controller-problem{rx-error-passive}
> 	protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> 	bus-error
>   can1  200000C8  [8] 00 00 90 00 00 00 00 00   ERRORFRAME
> 	protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> 	bus-off
> 	bus-error
> ...
>   can1  2000008C  [8] 00 04 90 00 00 00 00 00   ERRORFRAME
> 	controller-problem{rx-error-warning}
> 	protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> 	bus-error
>   can1  2000008C  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
> 	controller-problem{rx-error-passive}
> 	protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> 	bus-error
>   can1  200000C8  [8] 00 00 90 00 00 00 00 00   ERRORFRAME
> 	protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
> 	bus-off
> 	bus-error
> 
> t1: short circuit removed
>   can1  123  [2] 11 22
>   can1  20000100  [8] 00 00 00 00 00 00 00 00   ERRORFRAME
> 	restarted-after-bus-of
> 
> The echo coming before the restart looks weird? No?
> Shouldn't we drop the frame once BUF-OFF is reached?

No, I don't think so. But wait for Wolfgang, here's more into error
handling then me.

> 
>>>>>>> +		if ((priv->can.state == CAN_STATE_ERROR_WARNING) ||
>>>>>>> +		    (priv->can.state == CAN_STATE_ERROR_PASSIVE)) {
>>>>>>> +			cf->data[1] = (txerr > rxerr) ?
>>>>>>> +				CAN_ERR_CRTL_TX_PASSIVE
>>>>>>> +				: CAN_ERR_CRTL_RX_PASSIVE;
>>
>> Please use CAN_ERR_CRTL_RX_WARNING, CAN_ERR_CRTL_TX_WARNING where
>> appropriate.

> Ok. As the hardware doesn't report good values for txerr and rxerr, I'll
> also remove the tests on txerr and rxerr.
> I observed the same behavior with the original driver.
> I asked Kvaser for this problem. I've to wait before their developer is
> back (same for the GPL issue).

Okay.

>>>>>>> +static int kvaser_usb_get_berr_counter(const struct net_device *netdev,
>>>>>>> +				       struct can_berr_counter *bec)
>>>>>>> +{
>>>>>>> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
>>>>>>> +
>>>>>>> +	bec->txerr = priv->bec.txerr;
>>>>>>> +	bec->rxerr = priv->bec.rxerr;
>>
>> I think you can copy the struct like this:
>>
>> 	*bec = priv->bec;
> 
> Thanks. I'll remove the function kvaser_usb_get_berr_counter as the
> hardware seems to never report txerr and rxerr.

Sounds reasonable.

BTW: is it possible to update the firmware on these devices?

> I'll look deeper at this driver during the week-end if possible...

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] net: Allow to create links with given ifindex
From: Eric W. Biederman @ 2012-08-02 11:09 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Pavel Emelyanov, Linux Netdev List, David Miller
In-Reply-To: <1343903310.9299.184.camel@edumazet-glaptop>

Eric Dumazet <eric.dumazet@gmail.com> writes:

> On Tue, 2012-07-31 at 04:58 -0700, Eric W. Biederman wrote:
>
>> Making lo the particularly interesting case.
>
>
> BTW, I noticed in my benchmarks, that once I remove the contention on
> dst refcnt (using a percpu cache of dsts), I have a strange performance
> cost accessing net->loopback_dev->ifindex in ip_route_output_key.
>
> Strange because I see no false sharing on this ifindex location for
> loopback device.
>
> So we probably can save some cycles adding a net->loopback_ifindex
> to remove one dereference.

I am going to let Pavel tackle the actual work because only migration
really cares and he is working on migration right now.

But assuming we merge the per network namespace ifindex counter we
can change net->loopback_dev->ifindex to LOOPBACK_IFINDEX and
define "#define LOOPBACK_IFINDEX 1"

Certainly that works in the initial network namespace today and might be
worth testing.

> If ifindex are per network space, I guess we'll need to change
> arp_hashfn() or else we'll use some slots more than others.

Darn.  I hate being right about there being a few places to fix
up.

ndisc_hashfn also has the same limitation.

Eric

> diff --git a/include/net/arp.h b/include/net/arp.h
> index 7f7df93..37aac58 100644
> --- a/include/net/arp.h
> +++ b/include/net/arp.h
> @@ -10,7 +10,7 @@ extern struct neigh_table arp_tbl;
>  
>  static inline u32 arp_hashfn(u32 key, const struct net_device *dev, u32 hash_rnd)
>  {
> -	u32 val = key ^ dev->ifindex;
> +	u32 val = key ^ (u32)(unsigned long)dev;
>  
>  	return val * hash_rnd;
>  }

^ permalink raw reply

* [PATCH] isdnloop: fix and simplify isdnloop_init()
From: Fengguang Wu @ 2012-08-02 11:05 UTC (permalink / raw)
  To: open list:ISDN SUBSYSTEM
  Cc: Dan Carpenter, Greg KH, devel@driverdev.osuosl.org, Joe Perches,
	Karsten Keil, LKML

Fix a buffer overflow bug by removing the revision transform code.

[   22.016214] isdnloop-ISDN-driver Rev 1.11.6.7 
[   22.097508] isdnloop: (loop0) virtual card added
[   22.174400] Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: ffffffff83244972
[   22.174400] 
[   22.436157] Pid: 1, comm: swapper Not tainted 3.5.0-bisect-00018-gfa8bbb1-dirty #129
[   22.624071] Call Trace:
[   22.720558]  [<ffffffff832448c3>] ? CallcNew+0x56/0x56
[   22.815248]  [<ffffffff8222b623>] panic+0x110/0x329
[   22.914330]  [<ffffffff83244972>] ? isdnloop_init+0xaf/0xb1
[   23.014800]  [<ffffffff832448c3>] ? CallcNew+0x56/0x56
[   23.090763]  [<ffffffff8108e24b>] __stack_chk_fail+0x2b/0x30
[   23.185748]  [<ffffffff83244972>] isdnloop_init+0xaf/0xb1

Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
 drivers/isdn/isdnloop/isdnloop.c |   13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

--- linux.orig/drivers/isdn/isdnloop/isdnloop.c	2012-05-24 19:03:06.748430938 +0800
+++ linux/drivers/isdn/isdnloop/isdnloop.c	2012-08-02 11:57:16.806437909 +0800
@@ -16,7 +16,7 @@
 #include <linux/sched.h>
 #include "isdnloop.h"
 
-static char *revision = "$Revision: 1.11.6.7 $";
+static char *revision = "1.11.6.7";
 static char *isdnloop_id = "loop0";
 
 MODULE_DESCRIPTION("ISDN4Linux: Pseudo Driver that simulates an ISDN card");
@@ -1494,16 +1494,7 @@ isdnloop_addcard(char *id1)
 static int __init
 isdnloop_init(void)
 {
-	char *p;
-	char rev[10];
-
-	if ((p = strchr(revision, ':'))) {
-		strcpy(rev, p + 1);
-		p = strchr(rev, '$');
-		*p = 0;
-	} else
-		strcpy(rev, " ??? ");
-	printk(KERN_NOTICE "isdnloop-ISDN-driver Rev%s\n", rev);
+	printk(KERN_NOTICE "isdnloop-ISDN-driver Rev %s\n", revision);
 
 	if (isdnloop_id)
 		return (isdnloop_addcard(isdnloop_id));

^ permalink raw reply

* Re: [PATCH] isdnloop: fix and simplify isdnloop_init()
From: Fengguang Wu @ 2012-08-02 11:01 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Greg KH, devel@driverdev.osuosl.org, Joe Perches, Karsten Keil,
	LKML, open list:ISDN SUBSYSTEM
In-Reply-To: <20120802105736.GJ4352@mwanda>

On Thu, Aug 02, 2012 at 01:57:36PM +0300, Dan Carpenter wrote:
> On Thu, Aug 02, 2012 at 06:44:56PM +0800, Fengguang Wu wrote:
> > [Add more CC]
> > 
> 
> That's not helpful...  Resend the patch to netdev in a way that
> applies.

OK, sorry...

^ permalink raw reply

* Re: [PATCH] isdnloop: fix and simplify isdnloop_init()
From: Dan Carpenter @ 2012-08-02 10:57 UTC (permalink / raw)
  To: Fengguang Wu
  Cc: Greg KH, devel@driverdev.osuosl.org, Joe Perches, Karsten Keil,
	LKML, open list:ISDN SUBSYSTEM
In-Reply-To: <20120802104456.GA21537@localhost>

On Thu, Aug 02, 2012 at 06:44:56PM +0800, Fengguang Wu wrote:
> [Add more CC]
> 

That's not helpful...  Resend the patch to netdev in a way that
applies.

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH] isdnloop: fix and simplify isdnloop_init()
From: Greg KH @ 2012-08-02 10:56 UTC (permalink / raw)
  To: Fengguang Wu
  Cc: LKML, devel@driverdev.osuosl.org, Karsten Keil, Joe Perches,
	open list:ISDN SUBSYSTEM
In-Reply-To: <20120802104456.GA21537@localhost>

On Thu, Aug 02, 2012 at 06:44:56PM +0800, Fengguang Wu wrote:
> [Add more CC]

That's not going to do much, you just sent them a patch they can't apply
without editing by hand :(

^ permalink raw reply

* Re: [PATCH] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Olivier Sobrie @ 2012-08-02 10:53 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: Wolfgang Grandegger, linux-can, netdev
In-Reply-To: <5017DD5B.2030701@pengutronix.de>

Hello,

On Tue, Jul 31, 2012 at 03:27:55PM +0200, Marc Kleine-Budde wrote:
> We can continue the review process, this problem has to be sorted out
> before I can apply this patch to linux-can-next tree.

Ok.

> > 1) With the short circuit:
> > 
> > I perform the test you described. It showed that the Kvaser passes from
> > ERROR-WARNING to ERROR-PASSIVE and then BUS-OFF. But after going to the
> > state BUS-OFF it comes back to ERROR-WARNING.
> > 
> >   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
> >   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
> >   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
> >   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
> >   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
> >   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
> >   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
> >   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
> >   can1  20000088  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
> 
> Why don't we have any rx/tx numbers in the error frame?

Because the hardware seems to not update the tx/rx_errors_count
fields :-(

> From the hardware point of view the short circuit and open end tests
> look good. Please adjust the driver to turn off the CAN interface in
> case of a bus off if restart_ms is 0.

And in the case where restart_ms is not 0? Don't I've to put it off so
and drop the frame?
I actually implemeted it as you said and here is what I observed in
candump output with restart_ms set to 100 ms:

t0: Short circuit between CAN-H and CAN-L + cansend can1 123#1122
  can1  2000008C  [8] 00 04 90 00 00 00 00 00   ERRORFRAME
	controller-problem{rx-error-warning}
	protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
	bus-error
  can1  2000008C  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
	controller-problem{rx-error-passive}
	protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
	bus-error
  can1  200000C8  [8] 00 00 90 00 00 00 00 00   ERRORFRAME
	protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
	bus-off
	bus-error
...
  can1  2000008C  [8] 00 04 90 00 00 00 00 00   ERRORFRAME
	controller-problem{rx-error-warning}
	protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
	bus-error
  can1  2000008C  [8] 00 10 90 00 00 00 00 00   ERRORFRAME
	controller-problem{rx-error-passive}
	protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
	bus-error
  can1  200000C8  [8] 00 00 90 00 00 00 00 00   ERRORFRAME
	protocol-violation{{tx-recessive-bit-error,error-on-tx}{}}
	bus-off
	bus-error

t1: short circuit removed
  can1  123  [2] 11 22
  can1  20000100  [8] 00 00 00 00 00 00 00 00   ERRORFRAME
	restarted-after-bus-of

The echo coming before the restart looks weird? No?
Shouldn't we drop the frame once BUF-OFF is reached?

> >>>>> +		if ((priv->can.state == CAN_STATE_ERROR_WARNING) ||
> >>>>> +		    (priv->can.state == CAN_STATE_ERROR_PASSIVE)) {
> >>>>> +			cf->data[1] = (txerr > rxerr) ?
> >>>>> +				CAN_ERR_CRTL_TX_PASSIVE
> >>>>> +				: CAN_ERR_CRTL_RX_PASSIVE;
> 
> Please use CAN_ERR_CRTL_RX_WARNING, CAN_ERR_CRTL_TX_WARNING where
> appropriate.

Ok. As the hardware doesn't report good values for txerr and rxerr, I'll
also remove the tests on txerr and rxerr.
I observed the same behavior with the original driver.
I asked Kvaser for this problem. I've to wait before their developer is
back (same for the GPL issue).

> >>>>> +static int kvaser_usb_get_berr_counter(const struct net_device *netdev,
> >>>>> +				       struct can_berr_counter *bec)
> >>>>> +{
> >>>>> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> >>>>> +
> >>>>> +	bec->txerr = priv->bec.txerr;
> >>>>> +	bec->rxerr = priv->bec.rxerr;
> 
> I think you can copy the struct like this:
> 
> 	*bec = priv->bec;

Thanks. I'll remove the function kvaser_usb_get_berr_counter as the
hardware seems to never report txerr and rxerr.

I'll look deeper at this driver during the week-end if possible...

Thanks a lot for your help,

-- 
Olivier

^ permalink raw reply

* Re: [PATCH] isdnloop: fix and simplify isdnloop_init()
From: Fengguang Wu @ 2012-08-02 10:44 UTC (permalink / raw)
  To: Greg KH
  Cc: LKML, devel@driverdev.osuosl.org, Karsten Keil, Joe Perches,
	open list:ISDN SUBSYSTEM
In-Reply-To: <20120802040158.GA11860@localhost>

[Add more CC]

On Thu, Aug 02, 2012 at 12:01:58PM +0800, Fengguang Wu wrote:
> Remove the non-sense isdnloop revision transform code, so as to fix:
> 
> [   22.016214] isdnloop-ISDN-driver Rev 1.11.6.7 
> [   22.097508] isdnloop: (loop0) virtual card added
> [   22.174400] Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: ffffffff83244972
> [   22.174400] 
> [   22.436157] Pid: 1, comm: swapper Not tainted 3.5.0-bisect-00018-gfa8bbb1-dirty #129
> [   22.624071] Call Trace:
> [   22.720558]  [<ffffffff832448c3>] ? CallcNew+0x56/0x56
> [   22.815248]  [<ffffffff8222b623>] panic+0x110/0x329
> [   22.914330]  [<ffffffff83244972>] ? isdnloop_init+0xaf/0xb1
> [   23.014800]  [<ffffffff832448c3>] ? CallcNew+0x56/0x56
> [   23.090763]  [<ffffffff8108e24b>] __stack_chk_fail+0x2b/0x30
> [   23.185748]  [<ffffffff83244972>] isdnloop_init+0xaf/0xb1
> 
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> ---
>  drivers/isdn/isdnloop/isdnloop.c |   13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> --- linux.orig/drivers/isdn/isdnloop/isdnloop.c	2012-05-24 19:03:06.748430938 +0800
> +++ linux/drivers/isdn/isdnloop/isdnloop.c	2012-08-02 11:57:16.806437909 +0800
> @@ -16,7 +16,7 @@
>  #include <linux/sched.h>
>  #include "isdnloop.h"
>  
> -static char *revision = "$Revision: 1.11.6.7 $";
> +static char *revision = "1.11.6.7";
>  static char *isdnloop_id = "loop0";
>  
>  MODULE_DESCRIPTION("ISDN4Linux: Pseudo Driver that simulates an ISDN card");
> @@ -1494,16 +1494,7 @@ isdnloop_addcard(char *id1)
>  static int __init
>  isdnloop_init(void)
>  {
> -	char *p;
> -	char rev[10];
> -
> -	if ((p = strchr(revision, ':'))) {
> -		strcpy(rev, p + 1);
> -		p = strchr(rev, '$');
> -		*p = 0;
> -	} else
> -		strcpy(rev, " ??? ");
> -	printk(KERN_NOTICE "isdnloop-ISDN-driver Rev%s\n", rev);
> +	printk(KERN_NOTICE "isdnloop-ISDN-driver Rev %s\n", revision);
>  
>  	if (isdnloop_id)
>  		return (isdnloop_addcard(isdnloop_id));

^ permalink raw reply

* Re: [PATCH 1/2] net: Allow to create links with given ifindex
From: Eric Dumazet @ 2012-08-02 10:28 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Pavel Emelyanov, Linux Netdev List, David Miller
In-Reply-To: <878ve0dtw3.fsf@xmission.com>

On Tue, 2012-07-31 at 04:58 -0700, Eric W. Biederman wrote:

> Making lo the particularly interesting case.


BTW, I noticed in my benchmarks, that once I remove the contention on
dst refcnt (using a percpu cache of dsts), I have a strange performance
cost accessing net->loopback_dev->ifindex in ip_route_output_key.

Strange because I see no false sharing on this ifindex location for
loopback device.

So we probably can save some cycles adding a net->loopback_ifindex
to remove one dereference.

If ifindex are per network space, I guess we'll need to change
arp_hashfn() or else we'll use some slots more than others.

diff --git a/include/net/arp.h b/include/net/arp.h
index 7f7df93..37aac58 100644
--- a/include/net/arp.h
+++ b/include/net/arp.h
@@ -10,7 +10,7 @@ extern struct neigh_table arp_tbl;
 
 static inline u32 arp_hashfn(u32 key, const struct net_device *dev, u32 hash_rnd)
 {
-	u32 val = key ^ dev->ifindex;
+	u32 val = key ^ (u32)(unsigned long)dev;
 
 	return val * hash_rnd;
 }

^ permalink raw reply related

* Re: [PATCH] ipv4: route.c cleanup
From: David Miller @ 2012-08-02  9:53 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1343899420.9299.118.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 02 Aug 2012 11:23:40 +0200

> On Thu, 2012-08-02 at 01:45 -0700, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Thu, 02 Aug 2012 09:49:13 +0200
>> 
>> > From: Eric Dumazet <edumazet@google.com>
>> > 
>> > Remove some unused includes and sysctls after route cache removal.
>> > 
>> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>> 
>> I'm largely against removing the (now pointless) sysctls.
>> 
>> If the settings do nothing, so be it.
>> 
>> But it shouldn't generate warnings and errors during bootup,
>> which is what removing them is going to do.
> 
> Are you referring to a stale key in your /etc/sysctl.conf ?
> 
> I see no distro setting any of the route knob in their sysctl.conf file.

I'm talking about anyone, and people do set these knobs.  I've been
told by people at some large search engine that they, in fact, have
to :-)

> [PATCH v2] ipv4: route.c cleanup
> 
> Remove unused includes after IP cache removal
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

I'll apply this one, thanks.

^ permalink raw reply

* Re: [PATCH] ipv4: route.c cleanup
From: Eric Dumazet @ 2012-08-02  9:23 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20120802.014534.1221330015548828529.davem@davemloft.net>

On Thu, 2012-08-02 at 01:45 -0700, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 02 Aug 2012 09:49:13 +0200
> 
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > Remove some unused includes and sysctls after route cache removal.
> > 
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> 
> I'm largely against removing the (now pointless) sysctls.
> 
> If the settings do nothing, so be it.
> 
> But it shouldn't generate warnings and errors during bootup,
> which is what removing them is going to do.

Are you referring to a stale key in your /etc/sysctl.conf ?

I see no distro setting any of the route knob in their sysctl.conf file.



[PATCH v2] ipv4: route.c cleanup

Remove unused includes after IP cache removal

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/route.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index c035251..67790ba 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -70,7 +70,6 @@
 #include <linux/types.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
-#include <linux/bootmem.h>
 #include <linux/string.h>
 #include <linux/socket.h>
 #include <linux/sockios.h>
@@ -80,7 +79,6 @@
 #include <linux/netdevice.h>
 #include <linux/proc_fs.h>
 #include <linux/init.h>
-#include <linux/workqueue.h>
 #include <linux/skbuff.h>
 #include <linux/inetdevice.h>
 #include <linux/igmp.h>
@@ -88,11 +86,9 @@
 #include <linux/mroute.h>
 #include <linux/netfilter_ipv4.h>
 #include <linux/random.h>
-#include <linux/jhash.h>
 #include <linux/rcupdate.h>
 #include <linux/times.h>
 #include <linux/slab.h>
-#include <linux/prefetch.h>
 #include <net/dst.h>
 #include <net/net_namespace.h>
 #include <net/protocol.h>

^ permalink raw reply related

* Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date
From: Fan Du @ 2012-08-02  8:58 UTC (permalink / raw)
  To: David Miller; +Cc: herbert, netdev
In-Reply-To: <20120802.002308.367808111426941908.davem@davemloft.net>

Hi, all

*Apologize* for all the trouble I brought to everyone, who gave me
advices/suggestions using fdu@windriver.com.
I'm truly sorry that all the inconvenience caused by my mistake.

fdu@windriver.com is obsolete!!!
I will use fan.du@windriver.com from now on.

Thanks


On 2012年08月02日 15:23, David Miller wrote:
>
> You know what Fan Du, I'm extremely irritated about your email
> situation.
>
> Every time you post a patch, I reply, and I get this crap:
>
> Diagnostic-Code: smtp; 5.1.0 - Unknown address error 550-'5.2.1<fdu@windriver.com>... Mailbox disabled for this recipient' (delivery attempts: 0)
>
> I've seen this at least 7 times, and this absolutely has to stop.
>
> Otherwise I'm ignoring every patch you submit, it's as simple as
> that.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 

Love each day!
--fan

^ permalink raw reply

* [PATCH] Makefile: add missing blankspace
From: Li Wei @ 2012-08-02  8:45 UTC (permalink / raw)
  To: shemminger; +Cc: netdev, Li Wei

The missing blankspace caused us can't get tc.8 and tc-codel.8
installed.

Signed-off-by: Li Wei <lw@cn.fujitsu.com>
---
 man/man8/Makefile |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/man8/Makefile b/man/man8/Makefile
index 1b671a4..c344094 100644
--- a/man/man8/Makefile
+++ b/man/man8/Makefile
@@ -3,7 +3,7 @@ TARGETS = ip-address.8 ip-link.8 ip-route.8
 MAN8PAGES = $(TARGETS) ip.8 arpd.8 lnstat.8 routel.8 rtacct.8 rtmon.8 ss.8 \
 	tc-bfifo.8 tc-cbq-details.8 tc-cbq.8 tc-drr.8 tc-htb.8 \
 	tc-pfifo.8 tc-pfifo_fast.8 tc-prio.8 tc-red.8 tc-sfq.8 \
-	tc-tbf.8 tc.8tc-codel.8 tc-fq_codel.8 tc-sfb.8 tc-netem.8 tc-choke.8 \
+	tc-tbf.8 tc.8 tc-codel.8 tc-fq_codel.8 tc-sfb.8 tc-netem.8 tc-choke.8 \
 	bridge.8 rtstat.8 ctstat.8 nstat.8 routef.8 \
 	ip-tunnel.8 ip-rule.8 ip-ntable.8 \
 	ip-monitor.8 tc-stab.8 tc-hfsc.8 ip-xfrm.8 ip-netns.8 \
-- 
1.7.10.1

^ permalink raw reply related

* Re: [PATCH] ipv4: route.c cleanup
From: David Miller @ 2012-08-02  8:45 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1343893753.9299.13.camel@edumazet-glaptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 02 Aug 2012 09:49:13 +0200

> From: Eric Dumazet <edumazet@google.com>
> 
> Remove some unused includes and sysctls after route cache removal.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

I'm largely against removing the (now pointless) sysctls.

If the settings do nothing, so be it.

But it shouldn't generate warnings and errors during bootup,
which is what removing them is going to do.

^ permalink raw reply

* Re: [PATCH V2 08/12] net/eipoib: Add ethtool file support
From: Erez Shitrit @ 2012-08-02  8:35 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Or Gerlitz, davem, roland, netdev, ali, sean.hefty, Erez Shitrit
In-Reply-To: <1343866955.2602.66.camel@bwh-desktop.uk.solarflarecom.com>

On 8/2/2012 3:22 AM, Ben Hutchings wrote:
> On Wed, 2012-08-01 at 20:09 +0300, Or Gerlitz wrote:
>> From: Erez Shitrit <erezsh@mellanox.co.il>
>>
>> Via ethtool the driver describes its version, ABI version, on what PIF
>> interface it runs and various statistics.
> [...]
>> --- /dev/null
>> +++ b/drivers/net/eipoib/eth_ipoib_ethtool.c
> [...]
>> +static void parent_ethtool_get_drvinfo(struct net_device *parent_dev,
>> +				       struct ethtool_drvinfo *drvinfo)
>> +{
>> +	struct parent *parent = netdev_priv(parent_dev);
>> +
>> +	strncpy(drvinfo->driver, DRV_NAME, 32);
>> +
>> +	strncpy(drvinfo->version, DRV_VERSION, 32);
>> +
>> +	strncpy(drvinfo->bus_info, parent->ipoib_main_interface,
>> +		ETHTOOL_BUSINFO_LEN);
> These must be null-terminated; therefore use strlcpy().

ok, will fix.

>> +	/* indicates ABI version */
>> +	snprintf(drvinfo->fw_version, 32, "%d", EIPOIB_ABI_VER);
> [...]
>
> This is an abuse of fw_version.
>
> Ben.
we took the idea from the bonding driver,
(snprintf(drvinfo->fw_version, 32, "%d", BOND_ABI_VERSION);)

Do you have any idea where can we keep the abi version?

Thanks, Erez

^ permalink raw reply

* Re: [PATCH V2 05/12] net: Add ndo_set_vif_param operation to serve eIPoIB VIFs
From: Erez Shitrit @ 2012-08-02  8:25 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Or Gerlitz, davem, roland, netdev, ali, sean.hefty, Erez Shitrit
In-Reply-To: <1343866622.2602.62.camel@bwh-desktop.uk.solarflarecom.com>

On 8/2/2012 3:17 AM, Ben Hutchings wrote:
> On Wed, 2012-08-01 at 20:09 +0300, Or Gerlitz wrote:
>> From: Erez Shitrit <erezsh@mellanox.co.il>
>>
>> The Ethernet IPoIB driver enslaves IPoIB devices and uses them as
>> VIFs (Virtual Interface) which serve an Ethernet NIC e.g present in a
>> guest OS. For each such slave that acts as a VIF, eIPoIB needs to know
>> the mac and optionally the vlan uses by that NIC, the new ndo opertaion
>> is used to associate the mac/vlan for that slave.
>>
>> Signed-off-by: Erez Shitrit <erezsh@mellanox.co.il>
>> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
>> ---
>>   include/linux/netdevice.h |    5 ++++-
>>   1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index eb06e58..2709551 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1013,7 +1013,10 @@ struct net_device_ops {
>>   						    netdev_features_t features);
>>   	int			(*ndo_neigh_construct)(struct neighbour *n);
>>   	void			(*ndo_neigh_destroy)(struct neighbour *n);
>> -
>> +	int			(*ndo_set_vif_param) (struct net_device *dev,
>> +						      struct net_device *vif,
>> +						      u16 vlan,
>> +						      u8 *mac);
> The semantics of this operation should be documented in the comment
> above the structure definition.  One detail worth covering is whether
> 'vlan' is just a VID or can also include priority+CFI bits.
We will add more documentation for that.

The idea was just for the VID, (in our driver at least)

>
> If this is specific to eIPoIB, why not put that in the name of the
> operation?  If not, this *really* needs explaining because so far I have
> no whether it is something I should consider implementing on a real
> Ethernet device.
>
> Ben.
Will add more documentation here, perhaps other drivers can use it as 
well for visualization uses and more.
Thanks.

>>   	int			(*ndo_fdb_add)(struct ndmsg *ndm,
>>   					       struct net_device *dev,
>>   					       unsigned char *addr,

^ permalink raw reply

* [PATCH] ipv6: Fix premature timeout for MLDv1 compatibility mode
From: Dragos Ilie @ 2012-08-02  7:51 UTC (permalink / raw)
  To: David S. Miller
  Cc: Dragos Ilie, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, netdev, linux-kernel

The timeout for MLDv1 compatibility mode is now calculated according
to Section 9.2 in RFC 3810.

Signed-off-by: Dragos Ilie <dragos.ilie@gmail.com>
---
 net/ipv6/mcast.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index 92f8e48..ef824a0 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -109,6 +109,7 @@ static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *iml,
 
 #define IGMP6_UNSOLICITED_IVAL	(10*HZ)
 #define MLD_QRV_DEFAULT		2
+#define MLD_QIVAL_DEFAULT       (125*HZ)
 
 #define MLD_V1_SEEN(idev) (dev_net((idev)->dev)->ipv6.devconf_all->force_mld_version == 1 || \
 		(idev)->cnf.force_mld_version == 1 || \
@@ -1152,7 +1153,7 @@ int igmp6_event_query(struct sk_buff *skb)
 		/* Translate milliseconds to jiffies */
 		max_delay = (ntohs(mld->mld_maxdelay)*HZ)/1000;
 
-		switchback = (idev->mc_qrv + 1) * max_delay;
+		switchback = (idev->mc_qrv * MLD_QIVAL_DEFAULT) + max_delay;
 		idev->mc_v1_seen = jiffies + switchback;
 
 		/* cancel the interface change timer */
-- 
1.7.8.6

^ permalink raw reply related

* [PATCH] ipv4: route.c cleanup
From: Eric Dumazet @ 2012-08-02  7:49 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

Remove some unused includes and sysctls after route cache removal.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
This probably can wait net-next 

 net/ipv4/route.c |   58 ---------------------------------------------
 1 file changed, 1 insertion(+), 57 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index c035251..67790ba 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -70,7 +70,6 @@
 #include <linux/types.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
-#include <linux/bootmem.h>
 #include <linux/string.h>
 #include <linux/socket.h>
 #include <linux/sockios.h>
@@ -80,7 +79,6 @@
 #include <linux/netdevice.h>
 #include <linux/proc_fs.h>
 #include <linux/init.h>
-#include <linux/workqueue.h>
 #include <linux/skbuff.h>
 #include <linux/inetdevice.h>
 #include <linux/igmp.h>
@@ -88,11 +86,9 @@
 #include <linux/mroute.h>
 #include <linux/netfilter_ipv4.h>
 #include <linux/random.h>
-#include <linux/jhash.h>
 #include <linux/rcupdate.h>
 #include <linux/times.h>
 #include <linux/slab.h>
-#include <linux/prefetch.h>
 #include <net/dst.h>
 #include <net/net_namespace.h>
 #include <net/protocol.h>
@@ -118,18 +114,11 @@
 
 #define IP_MAX_MTU	0xFFF0
 
-#define RT_GC_TIMEOUT (300*HZ)
-
-static int ip_rt_max_size;
-static int ip_rt_gc_timeout __read_mostly	= RT_GC_TIMEOUT;
-static int ip_rt_gc_interval __read_mostly  = 60 * HZ;
-static int ip_rt_gc_min_interval __read_mostly	= HZ / 2;
 static int ip_rt_redirect_number __read_mostly	= 9;
 static int ip_rt_redirect_load __read_mostly	= HZ / 50;
 static int ip_rt_redirect_silence __read_mostly	= ((HZ / 50) << (9 + 1));
 static int ip_rt_error_cost __read_mostly	= HZ;
 static int ip_rt_error_burst __read_mostly	= 5 * HZ;
-static int ip_rt_gc_elasticity __read_mostly	= 8;
 static int ip_rt_mtu_expires __read_mostly	= 10 * 60 * HZ;
 static int ip_rt_min_pmtu __read_mostly		= 512 + 20 + 20;
 static int ip_rt_min_advmss __read_mostly	= 256;
@@ -2382,43 +2371,6 @@ static ctl_table ipv4_route_table[] = {
 		.proc_handler	= proc_dointvec,
 	},
 	{
-		.procname	= "max_size",
-		.data		= &ip_rt_max_size,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
-	},
-	{
-		/*  Deprecated. Use gc_min_interval_ms */
-
-		.procname	= "gc_min_interval",
-		.data		= &ip_rt_gc_min_interval,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_jiffies,
-	},
-	{
-		.procname	= "gc_min_interval_ms",
-		.data		= &ip_rt_gc_min_interval,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_ms_jiffies,
-	},
-	{
-		.procname	= "gc_timeout",
-		.data		= &ip_rt_gc_timeout,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_jiffies,
-	},
-	{
-		.procname	= "gc_interval",
-		.data		= &ip_rt_gc_interval,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec_jiffies,
-	},
-	{
 		.procname	= "redirect_load",
 		.data		= &ip_rt_redirect_load,
 		.maxlen		= sizeof(int),
@@ -2454,13 +2406,6 @@ static ctl_table ipv4_route_table[] = {
 		.proc_handler	= proc_dointvec,
 	},
 	{
-		.procname	= "gc_elasticity",
-		.data		= &ip_rt_gc_elasticity,
-		.maxlen		= sizeof(int),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
-	},
-	{
 		.procname	= "mtu_expires",
 		.data		= &ip_rt_mtu_expires,
 		.maxlen		= sizeof(int),
@@ -2599,7 +2544,6 @@ int __init ip_rt_init(void)
 		panic("IP: failed to allocate ipv4_dst_blackhole_ops counter\n");
 
 	ipv4_dst_ops.gc_thresh = ~0;
-	ip_rt_max_size = INT_MAX;
 
 	devinet_init();
 	ip_fib_init();
@@ -2608,7 +2552,7 @@ int __init ip_rt_init(void)
 		pr_err("Unable to create route proc files\n");
 #ifdef CONFIG_XFRM
 	xfrm_init();
-	xfrm4_init(ip_rt_max_size);
+	xfrm4_init(INT_MAX);
 #endif
 	rtnl_register(PF_INET, RTM_GETROUTE, inet_rtm_getroute, NULL, NULL);
 

^ permalink raw reply related

* Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date
From: David Miller @ 2012-08-02  7:23 UTC (permalink / raw)
  To: fdu; +Cc: herbert, netdev
In-Reply-To: <20120802.002308.367808111426941908.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Thu, 02 Aug 2012 00:23:08 -0700 (PDT)

> 
> You know what Fan Du, I'm extremely irritated about your email
> situation.
> 
> Every time you post a patch, I reply, and I get this crap:

And this email triggered it too, what gives?

^ permalink raw reply

* Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date
From: David Miller @ 2012-08-02  7:23 UTC (permalink / raw)
  To: fdu; +Cc: herbert, netdev
In-Reply-To: <20120802.002100.1997441066753440100.davem@davemloft.net>


You know what Fan Du, I'm extremely irritated about your email
situation.

Every time you post a patch, I reply, and I get this crap:

Diagnostic-Code: smtp; 5.1.0 - Unknown address error 550-'5.2.1 <fdu@windriver.com>... Mailbox disabled for this recipient' (delivery attempts: 0)

I've seen this at least 7 times, and this absolutely has to stop.

Otherwise I'm ignoring every patch you submit, it's as simple as
that.

^ permalink raw reply

* Re: [PATCH] bnx2x: fix mem leak when command is unknown
From: David Miller @ 2012-08-02  7:21 UTC (permalink / raw)
  To: jj; +Cc: eilong, netdev, linux-kernel
In-Reply-To: <alpine.LNX.2.00.1207312337590.7100@swampdragon.chaosbits.net>

From: Jesper Juhl <jj@chaosbits.net>
Date: Tue, 31 Jul 2012 23:39:37 +0200 (CEST)

> In bnx2x_mcast_enqueue_cmd() we'll leak the memory allocated to
> 'new_cmd' if we hit the deafault case of the 'switch (cmd)'.
> Add a 'kfree(new_cmd)' to that case to avoid the leak.
> 
> Signed-off-by: Jesper Juhl <jj@chaosbits.net>

Applied.

^ permalink raw reply

* Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date
From: David Miller @ 2012-08-02  7:21 UTC (permalink / raw)
  To: fdu; +Cc: herbert, netdev
In-Reply-To: <1343720634-1176-2-git-send-email-fdu@windriver.com>

From: Fan Du <fdu@windriver.com>
Date: Tue, 31 Jul 2012 15:43:54 +0800

> After SA is setup, one timer is armed to detect soft/hard expiration,
> however the timer handler uses xtime to do the math. This makes hard
> expiration occurs first before soft expiration after setting new date
> with big interval. As a result new child SA is deleted before rekeying
> the new one.
> 
> Signed-off-by: Fan Du <fdu@windriver.com>

Applied.

^ permalink raw reply

* Re: [PATCHv2 net 0/3] Prevent extreme TSO parameters from stalling TX queues
From: David Miller @ 2012-08-02  7:20 UTC (permalink / raw)
  To: bhutchings; +Cc: netdev, linux-net-drivers, greearb, eric.dumazet, shemminger
In-Reply-To: <1343699476.2667.69.camel@bwh-desktop.uk.solarflarecom.com>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 31 Jul 2012 02:51:16 +0100

> The following changes fix a potential DoS by peers or local users on
> network interfaces using the sfc driver (and possibly others) with TSO
> enabled (as it is by default).
> 
> Please apply patches 1 and 2 to the net tree and your stable update
> queue.  I'm not sure whether patch 3 is really important.
> 
> Ben.
> 
> Ben Hutchings (3):
>   net: Allow driver to limit number of GSO segments per skb
>   sfc: Fix maximum number of TSO segments and minimum TX queue size
>   tcp: Apply device TSO segment limit earlier

Series applied, thanks Ben.  I think patch #3 does matter, because
anything in TCP that cares about the "limit" now must make both
tests.

^ permalink raw reply

* Re: [PATCH] firmware: Remove obsolete Chelsio cxgb3 firmware
From: David Miller @ 2012-08-02  7:20 UTC (permalink / raw)
  To: tim.gardner
  Cc: linux-kernel, paul.gortmaker, ben, JBottomley, dan.j.williams,
	divy, netdev
In-Reply-To: <1343848670-31504-1-git-send-email-tim.gardner@canonical.com>

From: Tim Gardner <tim.gardner@canonical.com>
Date: Wed,  1 Aug 2012 13:17:50 -0600

> The current firmware version used by the device driver
> is 7.12.0
> 
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> Cc: Ben Hutchings <ben@decadent.org.uk>
> Cc: James Bottomley <JBottomley@Parallels.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Divy Le Ray <divy@chelsio.com>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>

"git am" refuses to apply this to current 'net':

Applying: firmware: Remove obsolete Chelsio cxgb3 firmware
error: removal patch leaves file contents
error: firmware/cxgb3/t3fw-7.10.0.bin.ihex: patch does not apply

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox