Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] net/stmmac: Use clk_prepare_enable and clk_disable_unprepare
From: Viresh Kumar @ 2012-09-21 11:21 UTC (permalink / raw)
  To: Stefan Roese; +Cc: netdev, Giuseppe Cavallaro, spear-devel
In-Reply-To: <1348225589-8126-1-git-send-email-sr@denx.de>

On 21 September 2012 16:36, Stefan Roese <sr@denx.de> wrote:
> This patch fixes an issue introduced by commit ID 6a81c26f
> [net/stmmac: remove conditional compilation of clk code], which
> switched from the internal stmmac_clk_{en}{dis}able calls to
> clk_{en}{dis}able. By this, calling clk_prepare and clk_unprepare
> was removed.
>
> clk_{un}prepare is mandatory for platforms using common clock framework.
> Since these drivers are used by SPEAr platform, which supports common
> clock framework, add clk_{un}prepare() support for them. Otherwise
> the clocks are not correctly en-/disabled and ethernet support doesn't
> work.

I can't believe i have done this. :)

IIRC, when i wrote this code prepare/unprepare weren't there. And by the
time my code got merged, they were. And this mistake was missed there.

Thanks for fixing it.

Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>

^ permalink raw reply

* [PATCH] net/stmmac: Use clk_prepare_enable and clk_disable_unprepare
From: Stefan Roese @ 2012-09-21 11:06 UTC (permalink / raw)
  To: netdev; +Cc: Viresh Kumar, Giuseppe Cavallaro

This patch fixes an issue introduced by commit ID 6a81c26f
[net/stmmac: remove conditional compilation of clk code], which
switched from the internal stmmac_clk_{en}{dis}able calls to
clk_{en}{dis}able. By this, calling clk_prepare and clk_unprepare
was removed.

clk_{un}prepare is mandatory for platforms using common clock framework.
Since these drivers are used by SPEAr platform, which supports common
clock framework, add clk_{un}prepare() support for them. Otherwise
the clocks are not correctly en-/disabled and ethernet support doesn't
work.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c  | 10 +++++-----
 drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c136162..3be8833 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1066,7 +1066,7 @@ static int stmmac_open(struct net_device *dev)
 	} else
 		priv->tm->enable = 1;
 #endif
-	clk_enable(priv->stmmac_clk);
+	clk_prepare_enable(priv->stmmac_clk);
 
 	stmmac_check_ether_addr(priv);
 
@@ -1188,7 +1188,7 @@ open_error:
 	if (priv->phydev)
 		phy_disconnect(priv->phydev);
 
-	clk_disable(priv->stmmac_clk);
+	clk_disable_unprepare(priv->stmmac_clk);
 
 	return ret;
 }
@@ -1246,7 +1246,7 @@ static int stmmac_release(struct net_device *dev)
 #ifdef CONFIG_STMMAC_DEBUG_FS
 	stmmac_exit_fs();
 #endif
-	clk_disable(priv->stmmac_clk);
+	clk_disable_unprepare(priv->stmmac_clk);
 
 	return 0;
 }
@@ -2178,7 +2178,7 @@ int stmmac_suspend(struct net_device *ndev)
 	else {
 		stmmac_set_mac(priv->ioaddr, false);
 		/* Disable clock in case of PWM is off */
-		clk_disable(priv->stmmac_clk);
+		clk_disable_unprepare(priv->stmmac_clk);
 	}
 	spin_unlock_irqrestore(&priv->lock, flags);
 	return 0;
@@ -2203,7 +2203,7 @@ int stmmac_resume(struct net_device *ndev)
 		priv->hw->mac->pmt(priv->ioaddr, 0);
 	else
 		/* enable the clk prevously disabled */
-		clk_enable(priv->stmmac_clk);
+		clk_prepare_enable(priv->stmmac_clk);
 
 	netif_device_attach(ndev);
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c
index 2a0e1ab..63ea9987 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c
@@ -97,12 +97,12 @@ static struct clk *timer_clock;
 static void stmmac_tmu_start(unsigned int new_freq)
 {
 	clk_set_rate(timer_clock, new_freq);
-	clk_enable(timer_clock);
+	clk_prepare_enable(timer_clock);
 }
 
 static void stmmac_tmu_stop(void)
 {
-	clk_disable(timer_clock);
+	clk_disable_unprepare(timer_clock);
 }
 
 int stmmac_open_ext_timer(struct net_device *dev, struct stmmac_timer *tm)
@@ -126,7 +126,7 @@ int stmmac_open_ext_timer(struct net_device *dev, struct stmmac_timer *tm)
 
 int stmmac_close_ext_timer(void)
 {
-	clk_disable(timer_clock);
+	clk_disable_unprepare(timer_clock);
 	tmu2_unregister_user();
 	clk_put(timer_clock);
 	return 0;
-- 
1.7.12.1

^ permalink raw reply related

* [PATCH 2/2] can: ti_hecc: fix oops during rmmod
From: Marc Kleine-Budde @ 2012-09-21 11:05 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-can, Marc Kleine-Budde, stable, Anant Gole
In-Reply-To: <1348225535-24976-1-git-send-email-mkl@pengutronix.de>

This patch fixes an oops which occurs when unloading the driver, while the
network interface is still up. The problem is that first the io mapping is
teared own, then the CAN device is unregistered, resulting in accessing the
hardware's iomem:

[  172.744232] Unable to handle kernel paging request at virtual address c88b0040
[  172.752441] pgd = c7be4000
[  172.755645] [c88b0040] *pgd=87821811, *pte=00000000, *ppte=00000000
[  172.762207] Internal error: Oops: 807 [#1] PREEMPT ARM
[  172.767517] Modules linked in: ti_hecc(-) can_dev
[  172.772430] CPU: 0    Not tainted  (3.5.0alpha-00037-g3554cc0 #126)
[  172.778961] PC is at ti_hecc_close+0xb0/0x100 [ti_hecc]
[  172.784423] LR is at __dev_close_many+0x90/0xc0
[  172.789123] pc : [<bf00c768>]    lr : [<c033be58>]    psr: 60000013
[  172.789123] sp : c5c1de68  ip : 00040081  fp : 00000000
[  172.801025] r10: 00000001  r9 : c5c1c000  r8 : 00100100
[  172.806457] r7 : c5d0a48c  r6 : c5d0a400  r5 : 00000000  r4 : c5d0a000
[  172.813232] r3 : c88b0000  r2 : 00000001  r1 : c5d0a000  r0 : c5d0a000
[  172.820037] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
[  172.827423] Control: 10c5387d  Table: 87be4019  DAC: 00000015
[  172.833404] Process rmmod (pid: 600, stack limit = 0xc5c1c2f0)
[  172.839447] Stack: (0xc5c1de68 to 0xc5c1e000)
[  172.843994] de60:                   bf00c6b8 c5c1dec8 c5d0a000 c5d0a000 00200200 c033be58
[  172.852478] de80: c5c1de44 c5c1dec8 c5c1dec8 c033bf2c c5c1de90 c5c1de90 c5d0a084 c5c1de44
[  172.860992] dea0: c5c1dec8 c033c098 c061d3dc c5d0a000 00000000 c05edf28 c05edb34 c000d724
[  172.869476] dec0: 00000000 c033c2f8 c5d0a084 c5d0a084 00000000 c033c370 00000000 c5d0a000
[  172.877990] dee0: c05edb00 c033c3b8 c5d0a000 bf00d3ac c05edb00 bf00d7c8 bf00d7c8 c02842dc
[  172.886474] df00: c02842c8 c0282f90 c5c1c000 c05edb00 bf00d7c8 c0283668 bf00d7c8 00000000
[  172.894989] df20: c0611f98 befe2f80 c000d724 c0282d10 bf00d804 00000000 00000013 c0068a8c
[  172.903472] df40: c5c538e8 685f6974 00636365 c61571a8 c5cb9980 c61571a8 c6158a20 c00c9bc4
[  172.911987] df60: 00000000 00000000 c5cb9980 00000000 c5cb9980 00000000 c7823680 00000006
[  172.920471] df80: bf00d804 00000880 c5c1df8c 00000000 000d4267 befe2f80 00000001 b6d90068
[  172.928985] dfa0: 00000081 c000d5a0 befe2f80 00000001 befe2f80 00000880 b6d90008 00000008
[  172.937469] dfc0: befe2f80 00000001 b6d90068 00000081 00000001 00000000 befe2eac 00000000
[  172.945983] dfe0: 00000000 befe2b18 00023ba4 b6e6addc 60000010 befe2f80 a8e00190 86d2d344
[  172.954498] [<bf00c768>] (ti_hecc_close+0xb0/0x100 [ti_hecc]) from [<c033be58>] (__dev__registered_many+0xc0/0x2a0)
[  172.984161] [<c033c098>] (rollback_registered_many+0xc0/0x2a0) from [<c033c2f8>] (rollback_registered+0x20/0x30)
[  172.994750] [<c033c2f8>] (rollback_registered+0x20/0x30) from [<c033c370>] (unregister_netdevice_queue+0x68/0x98)
[  173.005401] [<c033c370>] (unregister_netdevice_queue+0x68/0x98) from [<c033c3b8>] (unregister_netdev+0x18/0x20)
[  173.015899] [<c033c3b8>] (unregister_netdev+0x18/0x20) from [<bf00d3ac>] (ti_hecc_remove+0x60/0x80 [ti_hecc])
[  173.026245] [<bf00d3ac>] (ti_hecc_remove+0x60/0x80 [ti_hecc]) from [<c02842dc>] (platform_drv_remove+0x14/0x18)
[  173.036712] [<c02842dc>] (platform_drv_remove+0x14/0x18) from [<c0282f90>] (__device_release_driver+0x7c/0xbc)

Cc: stable <stable@vger.kernel.org>
Cc: Anant Gole <anantgole@ti.com>
Tested-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/ti_hecc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index 527dbcf..9ded21e 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -984,12 +984,12 @@ static int __devexit ti_hecc_remove(struct platform_device *pdev)
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct ti_hecc_priv *priv = netdev_priv(ndev);
 
+	unregister_candev(ndev);
 	clk_disable(priv->clk);
 	clk_put(priv->clk);
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	iounmap(priv->base);
 	release_mem_region(res->start, resource_size(res));
-	unregister_candev(ndev);
 	free_candev(ndev);
 	platform_set_drvdata(pdev, NULL);
 
-- 
1.7.10

^ permalink raw reply related

* [PATCH 1/2] can: janz-ican3: fix support for older hardware revisions
From: Marc Kleine-Budde @ 2012-09-21 11:05 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-can, Ira W. Snyder, stable, Marc Kleine-Budde
In-Reply-To: <1348225535-24976-1-git-send-email-mkl@pengutronix.de>

From: "Ira W. Snyder" <iws@ovro.caltech.edu>

The Revision 1.0 Janz CMOD-IO Carrier Board does not have support for
the reset registers. To support older hardware, the code is changed to
use the hardware reset register on the Janz VMOD-ICAN3 hardware itself.

Cc: stable <stable@vger.kernel.org>
Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/janz-ican3.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
index 98ee438..7edadee 100644
--- a/drivers/net/can/janz-ican3.c
+++ b/drivers/net/can/janz-ican3.c
@@ -1391,7 +1391,6 @@ static irqreturn_t ican3_irq(int irq, void *dev_id)
  */
 static int ican3_reset_module(struct ican3_dev *mod)
 {
-	u8 val = 1 << mod->num;
 	unsigned long start;
 	u8 runold, runnew;
 
@@ -1405,8 +1404,7 @@ static int ican3_reset_module(struct ican3_dev *mod)
 	runold = ioread8(mod->dpm + TARGET_RUNNING);
 
 	/* reset the module */
-	iowrite8(val, &mod->ctrl->reset_assert);
-	iowrite8(val, &mod->ctrl->reset_deassert);
+	iowrite8(0x00, &mod->dpmctrl->hwreset);
 
 	/* wait until the module has finished resetting and is running */
 	start = jiffies;
-- 
1.7.10


^ permalink raw reply related

* pull-request: can 2012-09-21
From: Marc Kleine-Budde @ 2012-09-21 11:05 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-can

Hello David,

two patches for the v3.6 release cycle. Ira W. Snyder fixed support for the
older version of the Janz CMOD-IO Carrier Board. I found and fixed an oops in
the ti_hecc driver, which occurs when removing the module if the network
interface is still open.

If it's too late for these patches, I'll rebase them to net-next.
 
regards, Marc

--

The following changes since commit c0d680e577ff171e7b37dbdb1b1bf5451e851f04:

  net: do not disable sg for packets requiring no checksum (2012-09-20 22:23:40 -0400)

are available in the git repository at:

  git://gitorious.org/linux-can/linux-can.git fixes-for-3.6

for you to fetch changes up to ab04c8bd423edb03e2148350a091836c196107fc:

  can: ti_hecc: fix oops during rmmod (2012-09-21 12:54:53 +0200)

----------------------------------------------------------------
Ira W. Snyder (1):
      can: janz-ican3: fix support for older hardware revisions

Marc Kleine-Budde (1):
      can: ti_hecc: fix oops during rmmod

 drivers/net/can/janz-ican3.c |    4 +---
 drivers/net/can/ti_hecc.c    |    2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)



^ permalink raw reply

* [PATCH] mISDN: suppress compiler warning
From: Paul Bolle @ 2012-09-21 10:25 UTC (permalink / raw)
  To: Karsten Keil; +Cc: netdev, linux-kernel

Building the hfcpci driver triggers this GCC warning:
    drivers/isdn/hardware/mISDN/hfcpci.c:2298:2: warning: ignoring return value of 'driver_for_each_device', declared with attribute warn_unused_result [-Wunused-result]

That return value is apparently ignored because _hfcpci_softirq() will
always return 0. Suppress this warning in the way a few other drivers do
that too.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
0) I noticed this warning while building v3.6-rc6 on current Fedora 17,
using Fedora's default config.

1) Compile tested only.

 drivers/isdn/hardware/mISDN/hfcpci.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/isdn/hardware/mISDN/hfcpci.c b/drivers/isdn/hardware/mISDN/hfcpci.c
index 81363ff..a547c8c 100644
--- a/drivers/isdn/hardware/mISDN/hfcpci.c
+++ b/drivers/isdn/hardware/mISDN/hfcpci.c
@@ -2295,8 +2295,11 @@ _hfcpci_softirq(struct device *dev, void *arg)
 static void
 hfcpci_softirq(void *arg)
 {
-	(void) driver_for_each_device(&hfc_driver.driver, NULL, arg,
+	int ret;
+
+	ret = driver_for_each_device(&hfc_driver.driver, NULL, arg,
 				      _hfcpci_softirq);
+	(void)ret;	/* suppress compiler warning */
 
 	/* if next event would be in the past ... */
 	if ((s32)(hfc_jiffies + tics - jiffies) <= 0)
-- 
1.7.11.4

^ permalink raw reply related

* Re: Oops with latest (netfilter) nf-next tree, when unloading iptable_nat
From: Pablo Neira Ayuso @ 2012-09-21 10:17 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: David Miller, Patrick McHardy, Florian Westphal, netfilter-devel,
	netdev
In-Reply-To: <20120921100308.GA24155@1984>

On Fri, Sep 21, 2012 at 12:03:08PM +0200, Pablo Neira Ayuso wrote:
> On Fri, Sep 21, 2012 at 11:47:22AM +0200, Jesper Dangaard Brouer wrote:
> > On Fri, 2012-09-21 at 03:00 +0200, Pablo Neira Ayuso wrote:
> > > On Thu, Sep 20, 2012 at 07:06:52PM +0200, Patrick McHardy wrote:
> > > > On Thu, 20 Sep 2012, Patrick McHardy wrote:
> > [cut]
> > (discussion of fixes by Patrick and Florian)
> > (...settling on Patricks second patch)
> > 
> > > Makes sense. And we can revisit this to improve it later.
> > > 
> > > I'll take this patch. I'll send a batch with updates for the nf-nat
> > > thin asap.
> > 
> > What git tree is that?
> > 
> > I'm trying to work off Pablo's nf-next tree (for my IPVS changes):
> >   git://1984.lsi.us.es/nf-next
> > 
> > But I don't see the patch in that tree ...yet.
> 
> I didn't push it yet, will do asap.

Done.

You may require git pull --rebase to get your patches up on the git
head.

^ permalink raw reply

* Re: Oops with latest (netfilter) nf-next tree, when unloading iptable_nat
From: Pablo Neira Ayuso @ 2012-09-21 10:03 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: David Miller, Patrick McHardy, Florian Westphal, netfilter-devel,
	netdev
In-Reply-To: <1348220842.3103.17.camel@localhost>

On Fri, Sep 21, 2012 at 11:47:22AM +0200, Jesper Dangaard Brouer wrote:
> On Fri, 2012-09-21 at 03:00 +0200, Pablo Neira Ayuso wrote:
> > On Thu, Sep 20, 2012 at 07:06:52PM +0200, Patrick McHardy wrote:
> > > On Thu, 20 Sep 2012, Patrick McHardy wrote:
> [cut]
> (discussion of fixes by Patrick and Florian)
> (...settling on Patricks second patch)
> 
> > Makes sense. And we can revisit this to improve it later.
> > 
> > I'll take this patch. I'll send a batch with updates for the nf-nat
> > thin asap.
> 
> What git tree is that?
> 
> I'm trying to work off Pablo's nf-next tree (for my IPVS changes):
>   git://1984.lsi.us.es/nf-next
> 
> But I don't see the patch in that tree ...yet.

I didn't push it yet, will do asap.

> Notice, the bug is also present in DaveM's net-next tree.
> (I know I stated earlier that it didn't affect net-next, but I just
> forgot to select the new netfilter .config options for nat)

^ permalink raw reply

* [PATCH] ipw2x00: silence GCC warning for unused variable 'dev'
From: Paul Bolle @ 2012-09-21 10:02 UTC (permalink / raw)
  To: Stanislav Yakovlev; +Cc: John W. Linville, linux-wireless, netdev, linux-kernel

Building the libipw component without CONFIG_LIBIPW_DEBUG set triggers this GCC
warning:
    drivers/net/wireless/ipw2x00/libipw_wx.c:526:21: warning: unused variable 'dev' [-Wunused-variable]

The cause of this warning is that, without CONFIG_LIBIPW_DEBUG set,
LIBIPW_DEBUG_WX compiles away. Fix it by substituting ieee->dev for (its
equivalent) dev.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
0) I noticed this warning while building v3.6-rc6 on current Fedora 17,
using Fedora's default config.

1) Compile tested only (by just compiling libipw_wx.o).

 drivers/net/wireless/ipw2x00/libipw_wx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ipw2x00/libipw_wx.c b/drivers/net/wireless/ipw2x00/libipw_wx.c
index 1571505..54aba47 100644
--- a/drivers/net/wireless/ipw2x00/libipw_wx.c
+++ b/drivers/net/wireless/ipw2x00/libipw_wx.c
@@ -675,7 +675,7 @@ int libipw_wx_set_encodeext(struct libipw_device *ieee,
 	}
       done:
 	if (ieee->set_security)
-		ieee->set_security(ieee->dev, &sec);
+		ieee->set_security(dev, &sec);
 
 	return ret;
 }
-- 
1.7.11.4

^ permalink raw reply related

* [PATCH net-next v4 1/1] ipv6: add support of ECMP
From: Nicolas Dichtel @ 2012-09-21  9:59 UTC (permalink / raw)
  To: davem; +Cc: bernat, netdev, yoshfuji, Nicolas Dichtel
In-Reply-To: <1348221545-14747-1-git-send-email-nicolas.dichtel@6wind.com>

This patch adds the support of equal cost multipath for IPv6.

The patch is based on a previous work from
Luc Saillard <luc.saillard@6wind.com>.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 Documentation/networking/ip-sysctl.txt |   8 ++
 include/net/ip6_fib.h                  |  13 ++
 include/net/netns/ipv6.h               |   3 +
 net/ipv6/Kconfig                       |  10 ++
 net/ipv6/ip6_fib.c                     |  73 +++++++++++
 net/ipv6/route.c                       | 222 ++++++++++++++++++++++++++++++++-
 6 files changed, 325 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index c7fc107..018bf8b 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1330,6 +1330,14 @@ ratelimit - INTEGER
 	otherwise the minimal space between responses in milliseconds.
 	Default: 1000
 
+route/*:
+multipath_algorithm - INTEGER
+	Define the method to select route between each possible path.
+	0 for hash/flow method (recommanded by RFC4311)
+	1 for round robin method
+	2 for random method
+	Default: 0
+
 
 IPv6 Update by:
 Pekka Savola <pekkas@netcore.fi>
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index cd64cf3..37e502a 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -47,6 +47,10 @@ struct fib6_config {
 	unsigned long	fc_expires;
 	struct nlattr	*fc_mx;
 	int		fc_mx_len;
+#ifdef CONFIG_IPV6_MULTIPATH
+	struct nlattr	*fc_mp;
+	int		fc_mp_len;
+#endif
 
 	struct nl_info	fc_nlinfo;
 };
@@ -98,6 +102,15 @@ struct rt6_info {
 	struct fib6_node		*rt6i_node;
 
 	struct in6_addr			rt6i_gateway;
+#ifdef CONFIG_IPV6_MULTIPATH
+	/*
+	 * siblings is a list of rt6_info that have the the same metric/weight,
+	 * destination, but not the same gateway. nsiblings is just a cache
+	 * to speed up lookup.
+	 */
+	unsigned int			rt6i_nsiblings;
+	struct list_head		rt6i_siblings;
+#endif
 
 	atomic_t			rt6i_ref;
 
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 214cb0a..820d4a6 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -26,6 +26,9 @@ struct netns_sysctl_ipv6 {
 	int ip6_rt_gc_elasticity;
 	int ip6_rt_mtu_expires;
 	int ip6_rt_min_advmss;
+#ifdef CONFIG_IPV6_MULTIPATH
+	int ip6_rt_multipath_algo;
+#endif
 	int icmpv6_time;
 };
 
diff --git a/net/ipv6/Kconfig b/net/ipv6/Kconfig
index 4f7fe72..c43fdf7 100644
--- a/net/ipv6/Kconfig
+++ b/net/ipv6/Kconfig
@@ -266,4 +266,14 @@ config IPV6_PIMSM_V2
 	  Support for IPv6 PIM multicast routing protocol PIM-SMv2.
 	  If unsure, say N.
 
+config IPV6_MULTIPATH
+	bool "IPv6: equal cost multipath for IPv6 routing"
+	depends on IPV6
+	default y
+	---help---
+	  Enable this option to support ECMP for IPv6.
+
+	  Three algorithms for route selection are available: hash of packet
+	  header (recommanded by RFC4311), round robin and random.
+
 endif # IPV6
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 13690d6..3541e44 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -672,6 +672,10 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 			    iter->rt6i_idev == rt->rt6i_idev &&
 			    ipv6_addr_equal(&iter->rt6i_gateway,
 					    &rt->rt6i_gateway)) {
+#ifdef CONFIG_IPV6_MULTIPATH
+				if (rt->rt6i_nsiblings)
+					rt->rt6i_nsiblings = 0;
+#endif
 				if (!(iter->rt6i_flags & RTF_EXPIRES))
 					return -EEXIST;
 				if (!(rt->rt6i_flags & RTF_EXPIRES))
@@ -680,6 +684,23 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 					rt6_set_expires(iter, rt->dst.expires);
 				return -EEXIST;
 			}
+#ifdef CONFIG_IPV6_MULTIPATH
+			/* If we have the same destination and the same metric,
+			 * but not the same gateway, then the route we try to
+			 * add is sibling to this route, increment our counter
+			 * of siblings, and later we will add our route to the
+			 * list.
+			 * Only static routes (which don't have flag
+			 * RTF_EXPIRES) are used for ECMPv6.
+			 *
+			 * To avoid long list, we only had siblings if the
+			 * route have a gateway.
+			 */
+			if (rt->rt6i_flags & RTF_GATEWAY &&
+			    !(rt->rt6i_flags & RTF_EXPIRES) &&
+			    !(iter->rt6i_flags & RTF_EXPIRES))
+				rt->rt6i_nsiblings++;
+#endif
 		}
 
 		if (iter->rt6i_metric > rt->rt6i_metric)
@@ -692,6 +713,43 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 	if (ins == &fn->leaf)
 		fn->rr_ptr = NULL;
 
+#ifdef CONFIG_IPV6_MULTIPATH
+	/* Link this route to others same route. */
+	if (rt->rt6i_nsiblings) {
+		unsigned int rt6i_nsiblings;
+		struct rt6_info *sibling, *temp_sibling;
+
+		/* Find the first route that have the same metric */
+		sibling = fn->leaf;
+		while (sibling) {
+			if (sibling->rt6i_metric == rt->rt6i_metric) {
+				list_add_tail(&rt->rt6i_siblings,
+					      &sibling->rt6i_siblings);
+				break;
+			}
+			sibling = sibling->dst.rt6_next;
+		}
+		/* For each sibling in the list, increment the counter of
+		 * siblings. We can check if all the counter are equal.
+		 */
+		rt6i_nsiblings = 0;
+		list_for_each_entry_safe(sibling, temp_sibling,
+					 &rt->rt6i_siblings,
+					 rt6i_siblings) {
+			sibling->rt6i_nsiblings++;
+			if (unlikely(sibling->rt6i_nsiblings !=
+				     rt->rt6i_nsiblings)) {
+				pr_err("Wrong number of siblings for route %p (%d)\n",
+				       sibling, sibling->rt6i_nsiblings);
+			}
+			rt6i_nsiblings++;
+		}
+		if (unlikely(rt6i_nsiblings != rt->rt6i_nsiblings)) {
+			pr_err("Wrong number of siblings for route %p. I have %d routes, but count %d siblings\n",
+			       rt, rt6i_nsiblings, rt->rt6i_nsiblings);
+		}
+	}
+#endif
 	/*
 	 *	insert node
 	 */
@@ -1197,6 +1255,21 @@ static void fib6_del_route(struct fib6_node *fn, struct rt6_info **rtp,
 	if (fn->rr_ptr == rt)
 		fn->rr_ptr = NULL;
 
+#ifdef CONFIG_IPV6_MULTIPATH
+	/* Remove this entry from other siblings */
+	if (rt->rt6i_nsiblings) {
+		struct rt6_info *sibling, *next_sibling;
+
+		/* For each siblings, decrement the counter of siblings */
+		list_for_each_entry_safe(sibling, next_sibling,
+					 &rt->rt6i_siblings, rt6i_siblings) {
+			sibling->rt6i_nsiblings--;
+		}
+		rt->rt6i_nsiblings = 0;
+		list_del_init(&rt->rt6i_siblings);
+	}
+#endif
+
 	/* Adjust walkers */
 	read_lock(&fib6_walker_lock);
 	FOR_WALKERS(w) {
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 0607ee3..bfad74f 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -57,6 +57,9 @@
 #include <net/xfrm.h>
 #include <net/netevent.h>
 #include <net/netlink.h>
+#ifdef CONFIG_IPV6_MULTIPATH
+#include <net/nexthop.h>
+#endif
 
 #include <asm/uaccess.h>
 
@@ -288,6 +291,10 @@ static inline struct rt6_info *ip6_dst_alloc(struct net *net,
 
 		memset(dst + 1, 0, sizeof(*rt) - sizeof(*dst));
 		rt6_init_peer(rt, table ? &table->tb6_peers : net->ipv6.peers);
+#ifdef CONFIG_IPV6_MULTIPATH
+		INIT_LIST_HEAD(&rt->rt6i_siblings);
+		rt->rt6i_nsiblings = 0;
+#endif
 	}
 	return rt;
 }
@@ -384,6 +391,121 @@ static bool rt6_need_strict(const struct in6_addr *daddr)
 		(IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL | IPV6_ADDR_LOOPBACK);
 }
 
+#ifdef CONFIG_IPV6_MULTIPATH
+/*
+ *	Multipath route selection.
+ */
+
+/*
+ * Pseudo random candidate function
+ */
+static int rt6_info_hash_randomfn(unsigned int candidate_count)
+{
+	return random32() % candidate_count;
+}
+
+/*
+ * Fake Round Robin candidate function
+ * If we want real RR, we need to add a counter in each route
+ */
+static int rt6_info_hash_falserr(unsigned int candidate_count)
+{
+	static unsigned int seed;
+	seed++;
+	return seed % candidate_count;
+}
+
+/*
+ * Pseudo random candidate using the src port, and other information
+ * Adapted from fib_info_hashfn()
+ */
+static int rt6_info_hash_nhsfn(unsigned int candidate_count,
+			       const struct flowi6 *fl6)
+{
+	unsigned int val = fl6->flowi6_proto;
+
+	val ^= fl6->daddr.s6_addr32[0];
+	val ^= fl6->daddr.s6_addr32[1];
+	val ^= fl6->daddr.s6_addr32[2];
+	val ^= fl6->daddr.s6_addr32[3];
+
+	val ^= fl6->saddr.s6_addr32[0];
+	val ^= fl6->saddr.s6_addr32[1];
+	val ^= fl6->saddr.s6_addr32[2];
+	val ^= fl6->saddr.s6_addr32[3];
+
+	/* Work only if this not encapsulated */
+	switch (fl6->flowi6_proto) {
+	case IPPROTO_UDP:
+	case IPPROTO_TCP:
+	case IPPROTO_SCTP:
+		val ^= fl6->fl6_sport;
+		val ^= fl6->fl6_dport;
+		break;
+
+	case IPPROTO_ICMPV6:
+		val ^= fl6->fl6_icmp_type;
+		val ^= fl6->fl6_icmp_code;
+		break;
+	}
+	/* RFC6438 recommands to use flowlabel */
+	val ^= fl6->flowlabel;
+
+	/* Perhaps, we need to tune, this function? */
+	val = val ^ (val >> 7) ^ (val >> 12);
+	return val % candidate_count;
+}
+
+/*
+ * This function return an index used to select (at random, round robin, ...)
+ * a route between any siblings.
+ *
+ * Note: fl6 can be NULL
+ */
+static unsigned int rt6_info_hashfn(struct net *net,
+				    const struct rt6_info *rt,
+				    const struct flowi6 *fl6)
+{
+	int candidate_count = rt->rt6i_nsiblings + 1;
+
+	switch (net->ipv6.sysctl.ip6_rt_multipath_algo) {
+	case 0:
+		if (fl6 == NULL)
+			return 0;
+		return rt6_info_hash_nhsfn(candidate_count, fl6);
+	case 1:
+		return rt6_info_hash_falserr(candidate_count);
+	case 2:
+		return rt6_info_hash_randomfn(candidate_count);
+	}
+
+	return 0;
+}
+
+static struct rt6_info *rt6_multipath_select(struct net *net,
+					     struct rt6_info *match,
+					     struct flowi6 *fl6)
+{
+	struct rt6_info *sibling, *next_sibling;
+	int route_choosen;
+
+	route_choosen = rt6_info_hashfn(net, match, fl6);
+	/* Don't change the route, if route_choosen == 0
+	 * (siblings does not include ourself)
+	 */
+	if (route_choosen)
+		list_for_each_entry_safe(sibling, next_sibling,
+				&match->rt6i_siblings, rt6i_siblings) {
+			route_choosen--;
+			if (route_choosen == 0) {
+				match = sibling;
+				break;
+			}
+		}
+	return match;
+}
+#endif /* CONFIG_IPV6_MULTIPATH */
+
 /*
  *	Route lookup. Any table->tb6_lock is implied.
  */
@@ -701,6 +823,10 @@ static struct rt6_info *ip6_pol_route_lookup(struct net *net,
 restart:
 	rt = fn->leaf;
 	rt = rt6_device_match(net, rt, &fl6->saddr, fl6->flowi6_oif, flags);
+#ifdef CONFIG_IPV6_MULTIPATH
+	if (rt->rt6i_nsiblings && fl6->flowi6_oif == 0)
+		rt = rt6_multipath_select(net, rt, fl6);
+#endif
 	BACKTRACK(net, &fl6->saddr);
 out:
 	dst_use(&rt->dst, jiffies);
@@ -862,7 +988,10 @@ restart_2:
 
 restart:
 	rt = rt6_select(fn, oif, strict | reachable);
-
+#ifdef CONFIG_IPV6_MULTIPATH
+	if (rt->rt6i_nsiblings && oif == 0)
+		rt = rt6_multipath_select(net, rt, fl6);
+#endif
 	BACKTRACK(net, &fl6->saddr);
 	if (rt == net->ipv6.ip6_null_entry ||
 	    rt->rt6i_flags & RTF_CACHE)
@@ -2243,6 +2372,9 @@ static const struct nla_policy rtm_ipv6_policy[RTA_MAX+1] = {
 	[RTA_IIF]		= { .type = NLA_U32 },
 	[RTA_PRIORITY]          = { .type = NLA_U32 },
 	[RTA_METRICS]           = { .type = NLA_NESTED },
+#ifdef CONFIG_IPV6_MULTIPATH
+	[RTA_MULTIPATH]		= { .len = sizeof(struct rtnexthop) },
+#endif
 };
 
 static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
@@ -2320,11 +2452,69 @@ static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (tb[RTA_TABLE])
 		cfg->fc_table = nla_get_u32(tb[RTA_TABLE]);
 
+#ifdef CONFIG_IPV6_MULTIPATH
+	if (tb[RTA_MULTIPATH]) {
+		cfg->fc_mp = nla_data(tb[RTA_MULTIPATH]);
+		cfg->fc_mp_len = nla_len(tb[RTA_MULTIPATH]);
+	}
+#endif
+
 	err = 0;
 errout:
 	return err;
 }
 
+#ifdef CONFIG_IPV6_MULTIPATH
+static int ip6_route_multipath(struct fib6_config *cfg, int add)
+{
+	struct fib6_config r_cfg;
+	struct rtnexthop *rtnh;
+	int remaining;
+	int attrlen;
+	int err = 0, last_err = 0;
+
+beginning:
+	rtnh = (struct rtnexthop *)cfg->fc_mp;
+	remaining = cfg->fc_mp_len;
+
+	/* Parse a Multipath Entry */
+	while (rtnh_ok(rtnh, remaining)) {
+		memcpy(&r_cfg, cfg, sizeof(*cfg));
+		if (rtnh->rtnh_ifindex)
+			r_cfg.fc_ifindex = rtnh->rtnh_ifindex;
+
+		attrlen = rtnh_attrlen(rtnh);
+		if (attrlen > 0) {
+			struct nlattr *nla, *attrs = rtnh_attrs(rtnh);
+
+			nla = nla_find(attrs, attrlen, RTA_GATEWAY);
+			if (nla) {
+				nla_memcpy(&r_cfg.fc_gateway, nla, 16);
+				r_cfg.fc_flags |= RTF_GATEWAY;
+			}
+		}
+		err = add ? ip6_route_add(&r_cfg) : ip6_route_del(&r_cfg);
+		if (err) {
+			last_err = err;
+			/* If we are trying to remove a route, do not stop the
+			 * loop when ip6_route_del() fails (because next hop is
+			 * already gone), we should try to remove all next hops.
+			 */
+			if (add) {
+				/* If add fails, we should try to delete all
+				 * next hops that have been already added.
+				 */
+				add = 0;
+				goto beginning;
+			}
+		}
+		rtnh = rtnh_next(rtnh, &remaining);
+	}
+
+	return last_err;
+}
+#endif /* CONFIG_IPV6_MULTIPATH */
+
 static int inet6_rtm_delroute(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
 {
 	struct fib6_config cfg;
@@ -2334,7 +2524,12 @@ static int inet6_rtm_delroute(struct sk_buff *skb, struct nlmsghdr* nlh, void *a
 	if (err < 0)
 		return err;
 
-	return ip6_route_del(&cfg);
+#ifdef CONFIG_IPV6_MULTIPATH
+	if (cfg.fc_mp)
+		return ip6_route_multipath(&cfg, 0);
+	else
+#endif
+		return ip6_route_del(&cfg);
 }
 
 static int inet6_rtm_newroute(struct sk_buff *skb, struct nlmsghdr* nlh, void *arg)
@@ -2346,7 +2541,12 @@ static int inet6_rtm_newroute(struct sk_buff *skb, struct nlmsghdr* nlh, void *a
 	if (err < 0)
 		return err;
 
-	return ip6_route_add(&cfg);
+#ifdef CONFIG_IPV6_MULTIPATH
+	if (cfg.fc_mp)
+		return ip6_route_multipath(&cfg, 1);
+	else
+#endif
+		return ip6_route_add(&cfg);
 }
 
 static inline size_t rt6_nlmsg_size(void)
@@ -2844,6 +3044,15 @@ ctl_table ipv6_route_table_template[] = {
 		.mode		=	0644,
 		.proc_handler	=	proc_dointvec_ms_jiffies,
 	},
+#ifdef CONFIG_IPV6_MULTIPATH
+	{
+		.procname	=	"multipath_algorithm",
+		.data		=	&init_net.ipv6.sysctl.ip6_rt_multipath_algo,
+		.maxlen		=	sizeof(int),
+		.mode		=	0644,
+		.proc_handler	=	proc_dointvec,
+	},
+#endif
 	{ }
 };
 
@@ -2867,6 +3076,9 @@ struct ctl_table * __net_init ipv6_route_sysctl_init(struct net *net)
 		table[7].data = &net->ipv6.sysctl.ip6_rt_mtu_expires;
 		table[8].data = &net->ipv6.sysctl.ip6_rt_min_advmss;
 		table[9].data = &net->ipv6.sysctl.ip6_rt_gc_min_interval;
+#ifdef CONFIG_IPV6_MULTIPATH
+		table[10].data = &net->ipv6.sysctl.ip6_rt_multipath_algo;
+#endif
 	}
 
 	return table;
@@ -2926,7 +3138,9 @@ static int __net_init ip6_route_net_init(struct net *net)
 	net->ipv6.sysctl.ip6_rt_gc_elasticity = 9;
 	net->ipv6.sysctl.ip6_rt_mtu_expires = 10*60*HZ;
 	net->ipv6.sysctl.ip6_rt_min_advmss = IPV6_MIN_MTU - 20 - 40;
-
+#ifdef CONFIG_IPV6_MULTIPATH
+	net->ipv6.sysctl.ip6_rt_multipath_algo = 0;
+#endif
 	net->ipv6.ip6_rt_gc_expire = 30*HZ;
 
 	ret = 0;
-- 
1.7.12

^ permalink raw reply related

* [PATCH net-next v4 0/1] Add support of ECMPv6
From: Nicolas Dichtel @ 2012-09-21  9:59 UTC (permalink / raw)
  To: davem; +Cc: bernat, netdev, yoshfuji
In-Reply-To: <20120920.171525.2005584636029506440.davem@davemloft.net>

Here is a proposal to add the support of ECMPv6. The previous patch
from Vincent against iproute2 can be used, but a little other patch is needed
too, see http://patchwork.ozlabs.org/patch/183277/

If the kernel patch is approved, I can submit formally the patch for
iproute2.

Here is an example of a command to add an ECMP route:
$ ip -6 route add 3ffe:304:124:2306::/64 \
	nexthop via fe80::230:1bff:feb4:e05c dev eth0 \
	nexthop via fe80::230:1bff:feb4:dd4f dev eth0

But note that this command is a shortcut and previous patches are not
mandatory to set ECMP routes. The following commands can be used too:
$ ip -6 route add 3ffe:304:124:2306::/64 via fe80::230:1bff:feb4:dd4f dev
eth0
$ ip -6 route append 3ffe:304:124:2306::/64 via fe80::230:1bff:feb4:e05c dev
eth0

Here is an example of a dump:
$ ip -6 route | grep 3ffe:304:124:2306::/64
3ffe:304:124:2306::/64 via fe80::230:1bff:feb4:dd4f dev eth0  metric 1024
3ffe:304:124:2306::/64 via fe80::230:1bff:feb4:e05c dev eth0  metric 1024

v2: rename CONFIG_IPV6_MULTIPATH_ROUTE to CONFIG_IPV6_MULTIPATH_HASH
    use flowlabel in the hash function
    add reference to RFC
    fix a small identation issue
    remove "If unsure, say N." from the help of CONFIG_IPV6_MULTIPATH

v3: rebase after updating net-next

v4: remove compilation options to choose multipath algorithm for next hop
    selection. Now the choice can be done at run time via
    /proc/sys/net/ipv6/route/multipath_algorithm

Comments are welcome.

Regards,
Nicolas

^ permalink raw reply

* Re: [PATCH v4] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Marc Kleine-Budde @ 2012-09-21  9:54 UTC (permalink / raw)
  To: Olivier Sobrie
  Cc: Wolfgang Grandegger, linux-can-u79uwXL29TY76Z2rM5mHXA,
	Kvaser Support, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1348117587-2905-1-git-send-email-olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>

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

On 09/20/2012 07:06 AM, Olivier Sobrie wrote:
> This driver provides support for several Kvaser CAN/USB devices.
> Such kind of devices supports up to three CAN network interfaces.
> 
> It has been tested with a Kvaser USB Leaf Light (one network interface)
> connected to a pch_can interface.
> The firmware version of the Kvaser device was 2.5.205.

I don't remember, have the USB people already had a look on your driver
and gave some comments?

From the CAN and network point of view looks good, some comments inline.
Would be fine, if Wolfgang can give comments or Ack about the error
frame generation.

Marc

> 
> List of Kvaser devices supported by the driver:
>   - Kvaser Leaf prototype (P010v2 and v3)
>   - Kvaser Leaf Light (P010v3)
>   - Kvaser Leaf Professional HS
>   - Kvaser Leaf SemiPro HS
>   - Kvaser Leaf Professional LS
>   - Kvaser Leaf Professional SWC
>   - Kvaser Leaf Professional LIN
>   - Kvaser Leaf SemiPro LS
>   - Kvaser Leaf SemiPro SWC
>   - Kvaser Memorator II, Prototype
>   - Kvaser Memorator II HS/HS
>   - Kvaser USBcan Professional HS/HS
>   - Kvaser Leaf Light GI
>   - Kvaser Leaf Professional HS (OBD-II connector)
>   - Kvaser Memorator Professional HS/LS
>   - Kvaser Leaf Light "China"
>   - Kvaser BlackBird SemiPro
>   - Kvaser OEM Mercury
>   - Kvaser OEM Leaf
>   - Kvaser USBcan R
> 
> Signed-off-by: Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
> ---
> Changes since v3:
>  - add support for CMD_LOG_MESSAGE
> 
>  drivers/net/can/usb/Kconfig      |   33 +
>  drivers/net/can/usb/Makefile     |    1 +
>  drivers/net/can/usb/kvaser_usb.c | 1555 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1589 insertions(+)
>  create mode 100644 drivers/net/can/usb/kvaser_usb.c
> 
> diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
> index 0a68768..578955f 100644
> --- a/drivers/net/can/usb/Kconfig
> +++ b/drivers/net/can/usb/Kconfig
> @@ -13,6 +13,39 @@ config CAN_ESD_USB2
>            This driver supports the CAN-USB/2 interface
>            from esd electronic system design gmbh (http://www.esd.eu).
>  
> +config CAN_KVASER_USB
> +	tristate "Kvaser CAN/USB interface"
> +	---help---
> +	  This driver adds support for Kvaser CAN/USB devices like Kvaser
> +	  Leaf Light.
> +
> +	  The driver gives support for the following devices:
> +	    - Kvaser Leaf prototype (P010v2 and v3)
> +	    - Kvaser Leaf Light (P010v3)
> +	    - Kvaser Leaf Professional HS
> +	    - Kvaser Leaf SemiPro HS
> +	    - Kvaser Leaf Professional LS
> +	    - Kvaser Leaf Professional SWC
> +	    - Kvaser Leaf Professional LIN
> +	    - Kvaser Leaf SemiPro LS
> +	    - Kvaser Leaf SemiPro SWC
> +	    - Kvaser Memorator II, Prototype
> +	    - Kvaser Memorator II HS/HS
> +	    - Kvaser USBcan Professional HS/HS
> +	    - Kvaser Leaf Light GI
> +	    - Kvaser Leaf Professional HS (OBD-II connector)
> +	    - Kvaser Memorator Professional HS/LS
> +	    - Kvaser Leaf Light "China"
> +	    - Kvaser BlackBird SemiPro
> +	    - Kvaser OEM Mercury
> +	    - Kvaser OEM Leaf
> +	    - Kvaser USBcan R
> +
> +	  If unsure, say N.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called kvaser_usb.
> +
>  config CAN_PEAK_USB
>  	tristate "PEAK PCAN-USB/USB Pro interfaces"
>  	---help---
> diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
> index da6d1d3..80a2ee4 100644
> --- a/drivers/net/can/usb/Makefile
> +++ b/drivers/net/can/usb/Makefile
> @@ -4,6 +4,7 @@
>  
>  obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o
>  obj-$(CONFIG_CAN_ESD_USB2) += esd_usb2.o
> +obj-$(CONFIG_CAN_KVASER_USB) += kvaser_usb.o
>  obj-$(CONFIG_CAN_PEAK_USB) += peak_usb/
>  
>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> new file mode 100644
> index 0000000..3509ca5
> --- /dev/null
> +++ b/drivers/net/can/usb/kvaser_usb.c
> @@ -0,0 +1,1555 @@
> +/*
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * Parts of this driver are based on the following:
> + *  - Kvaser linux leaf driver (version 4.78)
> + *  - CAN driver for esd CAN-USB/2
> + *
> + * Copyright (C) 2002-2006 KVASER AB, Sweden. All rights reserved.
> + * Copyright (C) 2010 Matthias Fuchs <matthias.fuchs-iOnpLzIbIdM@public.gmane.org>, esd gmbh
> + * Copyright (C) 2012 Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>
> + */
> +
> +#include <linux/init.h>
> +#include <linux/completion.h>
> +#include <linux/module.h>
> +#include <linux/netdevice.h>
> +#include <linux/usb.h>
> +
> +#include <linux/can.h>
> +#include <linux/can/dev.h>
> +#include <linux/can/error.h>
> +
> +#define MAX_TX_URBS			16
> +#define MAX_RX_URBS			4
> +#define START_TIMEOUT			1000 /* msecs */
> +#define STOP_TIMEOUT			1000 /* msecs */
> +#define USB_SEND_TIMEOUT		1000 /* msecs */
> +#define USB_RECV_TIMEOUT		1000 /* msecs */
> +#define RX_BUFFER_SIZE			3072
> +#define CAN_USB_CLOCK			8000000
> +#define MAX_NET_DEVICES			3
> +
> +/* Kvaser USB devices */
> +#define KVASER_VENDOR_ID		0x0bfd
> +#define USB_LEAF_DEVEL_PRODUCT_ID	10
> +#define USB_LEAF_LITE_PRODUCT_ID	11
> +#define USB_LEAF_PRO_PRODUCT_ID		12
> +#define USB_LEAF_SPRO_PRODUCT_ID	14
> +#define USB_LEAF_PRO_LS_PRODUCT_ID	15
> +#define USB_LEAF_PRO_SWC_PRODUCT_ID	16
> +#define USB_LEAF_PRO_LIN_PRODUCT_ID	17
> +#define USB_LEAF_SPRO_LS_PRODUCT_ID	18
> +#define USB_LEAF_SPRO_SWC_PRODUCT_ID	19
> +#define USB_MEMO2_DEVEL_PRODUCT_ID	22
> +#define USB_MEMO2_HSHS_PRODUCT_ID	23
> +#define USB_UPRO_HSHS_PRODUCT_ID	24
> +#define USB_LEAF_LITE_GI_PRODUCT_ID	25
> +#define USB_LEAF_PRO_OBDII_PRODUCT_ID	26
> +#define USB_MEMO2_HSLS_PRODUCT_ID	27
> +#define USB_LEAF_LITE_CH_PRODUCT_ID	28
> +#define USB_BLACKBIRD_SPRO_PRODUCT_ID	29
> +#define USB_OEM_MERCURY_PRODUCT_ID	34
> +#define USB_OEM_LEAF_PRODUCT_ID		35
> +#define USB_CAN_R_PRODUCT_ID		39
> +
> +/* USB devices features */
> +#define KVASER_HAS_SILENT_MODE		BIT(0)
> +#define KVASER_HAS_TXRX_ERRORS		BIT(1)
> +
> +/* Message header size */
> +#define MSG_HEADER_LEN			2
> +
> +/* Can message flags */
> +#define MSG_FLAG_ERROR_FRAME		BIT(0)
> +#define MSG_FLAG_OVERRUN		BIT(1)
> +#define MSG_FLAG_NERR			BIT(2)
> +#define MSG_FLAG_WAKEUP			BIT(3)
> +#define MSG_FLAG_REMOTE_FRAME		BIT(4)
> +#define MSG_FLAG_RESERVED		BIT(5)
> +#define MSG_FLAG_TX_ACK			BIT(6)
> +#define MSG_FLAG_TX_REQUEST		BIT(7)
> +
> +/* Can states */
> +#define M16C_STATE_BUS_RESET		BIT(0)
> +#define M16C_STATE_BUS_ERROR		BIT(4)
> +#define M16C_STATE_BUS_PASSIVE		BIT(5)
> +#define M16C_STATE_BUS_OFF		BIT(6)
> +
> +/* Can msg ids */
> +#define CMD_RX_STD_MESSAGE		12
> +#define CMD_TX_STD_MESSAGE		13
> +#define CMD_RX_EXT_MESSAGE		14
> +#define CMD_TX_EXT_MESSAGE		15
> +#define CMD_SET_BUS_PARAMS		16
> +#define CMD_GET_BUS_PARAMS		17
> +#define CMD_GET_BUS_PARAMS_REPLY	18
> +#define CMD_GET_CHIP_STATE		19
> +#define CMD_CHIP_STATE_EVENT		20
> +#define CMD_SET_CTRL_MODE		21
> +#define CMD_GET_CTRL_MODE		22
> +#define CMD_GET_CTRL_MODE_REPLY		23
> +#define CMD_RESET_CHIP			24
> +#define CMD_RESET_CARD			25
> +#define CMD_START_CHIP			26
> +#define CMD_START_CHIP_REPLY		27
> +#define CMD_STOP_CHIP			28
> +#define CMD_STOP_CHIP_REPLY		29
> +#define CMD_GET_CARD_INFO2		32
> +#define CMD_GET_CARD_INFO		34
> +#define CMD_GET_CARD_INFO_REPLY		35
> +#define CMD_GET_SOFTWARE_INFO		38
> +#define CMD_GET_SOFTWARE_INFO_REPLY	39
> +#define CMD_ERROR_EVENT			45
> +#define CMD_FLUSH_QUEUE			48
> +#define CMD_RESET_ERROR_COUNTER		49
> +#define CMD_TX_ACKNOWLEDGE		50
> +#define CMD_CAN_ERROR_EVENT		51
> +#define CMD_USB_THROTTLE		77
> +#define CMD_LOG_MESSAGE			106
> +
> +/* error factors */
> +#define M16C_EF_ACKE			BIT(0)
> +#define M16C_EF_CRCE			BIT(1)
> +#define M16C_EF_FORME			BIT(2)
> +#define M16C_EF_STFE			BIT(3)
> +#define M16C_EF_BITE0			BIT(4)
> +#define M16C_EF_BITE1			BIT(5)
> +#define M16C_EF_RCVE			BIT(6)
> +#define M16C_EF_TRE			BIT(7)
> +
> +/* bittiming parameters */
> +#define KVASER_USB_TSEG1_MIN		1
> +#define KVASER_USB_TSEG1_MAX		16
> +#define KVASER_USB_TSEG2_MIN		1
> +#define KVASER_USB_TSEG2_MAX		8
> +#define KVASER_USB_SJW_MAX		4
> +#define KVASER_USB_BRP_MIN		1
> +#define KVASER_USB_BRP_MAX		64
> +#define KVASER_USB_BRP_INC		1
> +
> +/* ctrl modes */
> +#define KVASER_CTRL_MODE_NORMAL		1
> +#define KVASER_CTRL_MODE_SILENT		2
> +#define KVASER_CTRL_MODE_SELFRECEPTION	3
> +#define KVASER_CTRL_MODE_OFF		4
> +
> +struct kvaser_msg_simple {
> +	u8 tid;
> +	u8 channel;
> +} __packed;
> +
> +struct kvaser_msg_cardinfo {
> +	u8 tid;
> +	u8 nchannels;
> +	__le32 serial_number;
> +	__le32 padding;
> +	__le32 clock_resolution;
> +	__le32 mfgdate;
> +	u8 ean[8];
> +	u8 hw_revision;
> +	u8 usb_hs_mode;
> +	__le16 padding2;
> +} __packed;
> +
> +struct kvaser_msg_cardinfo2 {
> +	u8 tid;
> +	u8 channel;
> +	u8 pcb_id[24];
> +	__le32 oem_unlock_code;
> +} __packed;
> +
> +struct kvaser_msg_softinfo {
> +	u8 tid;
> +	u8 channel;
> +	__le32 sw_options;
> +	__le32 fw_version;
> +	__le16 max_outstanding_tx;
> +	__le16 padding[9];
> +} __packed;
> +
> +struct kvaser_msg_busparams {
> +	u8 tid;
> +	u8 channel;
> +	__le32 bitrate;
> +	u8 tseg1;
> +	u8 tseg2;
> +	u8 sjw;
> +	u8 no_samp;
> +} __packed;
> +
> +struct kvaser_msg_tx_can {
> +	u8 channel;
> +	u8 tid;
> +	u8 msg[14];
> +	u8 padding;
> +	u8 flags;
> +} __packed;
> +
> +struct kvaser_msg_rx_can {
> +	u8 channel;
> +	u8 flag;
> +	__le16 time[3];
> +	u8 msg[14];
> +} __packed;
> +
> +struct kvaser_msg_chip_state_event {
> +	u8 tid;
> +	u8 channel;
> +	__le16 time[3];
> +	u8 tx_errors_count;
> +	u8 rx_errors_count;
> +	u8 status;
> +	u8 padding[3];
> +} __packed;
> +
> +struct kvaser_msg_tx_acknowledge {
> +	u8 channel;
> +	u8 tid;
> +	__le16 time[3];
> +	u8 flags;
> +	u8 time_offset;
> +} __packed;
> +
> +struct kvaser_msg_error_event {
> +	u8 tid;
> +	u8 flags;
> +	__le16 time[3];
> +	u8 channel;
> +	u8 padding;
> +	u8 tx_errors_count;
> +	u8 rx_errors_count;
> +	u8 status;
> +	u8 error_factor;
> +} __packed;
> +
> +struct kvaser_msg_ctrl_mode {
> +	u8 tid;
> +	u8 channel;
> +	u8 ctrl_mode;
> +	u8 padding[3];
> +} __packed;
> +
> +struct kvaser_msg_flush_queue {
> +	u8 tid;
> +	u8 channel;
> +	u8 flags;
> +	u8 padding[3];
> +} __packed;
> +
> +struct kvaser_msg_log_message {
> +	u8 channel;
> +	u8 flags;
> +	__le16 time[3];
> +	u8 dlc;
> +	u8 time_offset;
> +	__le32 id;
> +	u8 data[8];
> +} __packed;
> +
> +struct kvaser_msg {
> +	u8 len;
> +	u8 id;
> +	union	{
> +		struct kvaser_msg_simple simple;
> +		struct kvaser_msg_cardinfo cardinfo;
> +		struct kvaser_msg_cardinfo2 cardinfo2;
> +		struct kvaser_msg_softinfo softinfo;
> +		struct kvaser_msg_busparams busparams;
> +		struct kvaser_msg_tx_can tx_can;
> +		struct kvaser_msg_rx_can rx_can;
> +		struct kvaser_msg_chip_state_event chip_state_event;
> +		struct kvaser_msg_tx_acknowledge tx_acknowledge;
> +		struct kvaser_msg_error_event error_event;
> +		struct kvaser_msg_ctrl_mode ctrl_mode;
> +		struct kvaser_msg_flush_queue flush_queue;
> +		struct kvaser_msg_log_message log_message;
> +	} u;
> +} __packed;
> +
> +struct kvaser_usb_tx_urb_context {
> +	struct kvaser_usb_net_priv *priv;
> +	u32 echo_index;
> +	int dlc;
> +};
> +
> +struct kvaser_usb {
> +	struct usb_device *udev;
> +	struct kvaser_usb_net_priv *nets[MAX_NET_DEVICES];
> +
> +	struct usb_endpoint_descriptor *bulk_in, *bulk_out;
> +	struct usb_anchor rx_submitted;
> +
> +	u32 fw_version;
> +	unsigned int nchannels;
> +
> +	bool rxinitdone;
> +	void *rxbuf[MAX_RX_URBS];
> +	dma_addr_t rxbuf_dma[MAX_RX_URBS];
> +};
> +
> +struct kvaser_usb_net_priv {
> +	struct can_priv can;
> +
> +	atomic_t active_tx_urbs;
> +	struct usb_anchor tx_submitted;
> +	struct kvaser_usb_tx_urb_context tx_contexts[MAX_TX_URBS];
> +
> +	struct completion start_comp, stop_comp;
> +
> +	struct kvaser_usb *dev;
> +	struct net_device *netdev;
> +	int channel;
> +
> +	struct can_berr_counter bec;
> +};
> +
> +static struct usb_device_id kvaser_usb_table[] = {
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_DEVEL_PRODUCT_ID) },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_PRODUCT_ID) },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> +			       KVASER_HAS_SILENT_MODE },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> +			       KVASER_HAS_SILENT_MODE },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LS_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> +			       KVASER_HAS_SILENT_MODE },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_SWC_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> +			       KVASER_HAS_SILENT_MODE },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LIN_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> +			       KVASER_HAS_SILENT_MODE },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_LS_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> +			       KVASER_HAS_SILENT_MODE },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_SWC_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> +			       KVASER_HAS_SILENT_MODE },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_DEVEL_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> +			       KVASER_HAS_SILENT_MODE },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSHS_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> +			       KVASER_HAS_SILENT_MODE },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_UPRO_HSHS_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_GI_PRODUCT_ID) },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_OBDII_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
> +			       KVASER_HAS_SILENT_MODE },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSLS_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_CH_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_BLACKBIRD_SPRO_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_MERCURY_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_LEAF_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_CAN_R_PRODUCT_ID),
> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
> +
> +static inline int kvaser_usb_send_msg(const struct kvaser_usb *dev,
> +				      struct kvaser_msg *msg)
> +{
> +	int actual_len;
> +
> +	return usb_bulk_msg(dev->udev,
> +			    usb_sndbulkpipe(dev->udev,
> +					dev->bulk_out->bEndpointAddress),
> +			    msg, msg->len, &actual_len,
> +			    USB_SEND_TIMEOUT);
> +}
> +
> +static int kvaser_usb_wait_msg(const struct kvaser_usb *dev, u8 id,
> +			       struct kvaser_msg *msg)
> +{
> +	struct kvaser_msg *tmp;
> +	void *buf;
> +	int actual_len;
> +	int err;
> +	int pos = 0;
> +
> +	buf = kzalloc(RX_BUFFER_SIZE, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	err = usb_bulk_msg(dev->udev,
> +			   usb_rcvbulkpipe(dev->udev,
> +					   dev->bulk_in->bEndpointAddress),
> +			   buf, RX_BUFFER_SIZE, &actual_len,
> +			   USB_RECV_TIMEOUT);
> +	if (err < 0)
> +		goto end;
> +
> +	while (pos <= actual_len - MSG_HEADER_LEN) {
> +		tmp = buf + pos;
> +
> +		if (!tmp->len)
> +			break;
> +
> +		if (pos + tmp->len > actual_len) {
> +			dev_err(dev->udev->dev.parent, "Format error\n");
> +			break;
> +		}
> +
> +		if (tmp->id == id) {
> +			memcpy(msg, tmp, tmp->len);
> +			goto end;
> +		}
> +
> +		pos += tmp->len;
> +	}
> +
> +	err = -EINVAL;
> +
> +end:
> +	kfree(buf);
> +
> +	return err;
> +}
> +
> +static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev,
> +				      u8 msg_id, int channel)
> +{
> +	struct kvaser_msg msg = {
> +		.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple),
> +		.id = msg_id,
> +		.u.simple.channel = channel,
> +		.u.simple.tid = 0xff,
> +	};
> +
> +	return kvaser_usb_send_msg(dev, &msg);
> +}
> +
> +static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
> +{
> +	struct kvaser_msg msg;
> +	int err;
> +
> +	err = kvaser_usb_send_simple_msg(dev, CMD_GET_SOFTWARE_INFO, 0);
> +	if (err)
> +		return err;
> +
> +	err = kvaser_usb_wait_msg(dev, CMD_GET_SOFTWARE_INFO_REPLY, &msg);
> +	if (err)
> +		return err;
> +
> +	dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
> +
> +	return 0;
> +}
> +
> +static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
> +{
> +	struct kvaser_msg msg;
> +	int err;
> +
> +	err = kvaser_usb_send_simple_msg(dev, CMD_GET_CARD_INFO, 0);
> +	if (err)
> +		return err;
> +
> +	err = kvaser_usb_wait_msg(dev, CMD_GET_CARD_INFO_REPLY, &msg);
> +	if (err)
> +		return err;
> +
> +	dev->nchannels = msg.u.cardinfo.nchannels;
> +
> +	return 0;
> +}
> +
> +static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
> +				      const struct kvaser_msg *msg)
> +{
> +	struct net_device_stats *stats;
> +	struct kvaser_usb_tx_urb_context *context;
> +	struct kvaser_usb_net_priv *priv;
> +	struct sk_buff *skb;
> +	struct can_frame *cf;
> +	u8 channel = msg->u.tx_acknowledge.channel;
> +	u8 tid = msg->u.tx_acknowledge.tid;
> +
> +	if (channel >= dev->nchannels) {
> +		dev_err(dev->udev->dev.parent,
> +			"Invalid channel number (%d)\n", channel);
> +		return;
> +	}
> +
> +	priv = dev->nets[channel];
> +
> +	if (!netif_device_present(priv->netdev))
> +		return;
> +
> +	stats = &priv->netdev->stats;
> +
> +	context = &priv->tx_contexts[tid % MAX_TX_URBS];
> +
> +	/* Sometimes the state change doesn't come after a bus-off event */
> +	if (priv->can.restart_ms &&
> +	    (priv->can.state >= CAN_STATE_BUS_OFF)) {
> +		skb = alloc_can_err_skb(priv->netdev, &cf);
> +		if (skb) {
> +			cf->can_id |= CAN_ERR_RESTARTED;
> +			netif_rx(skb);
> +
> +			stats->rx_packets++;
> +			stats->rx_bytes += cf->can_dlc;
> +		} else {
> +			netdev_err(priv->netdev,
> +				   "No memory left for err_skb\n");
> +		}
> +
> +		priv->can.can_stats.restarts++;
> +		netif_carrier_on(priv->netdev);
> +
> +		priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +	}
> +
> +	stats->tx_packets++;
> +	stats->tx_bytes += context->dlc;
> +	can_get_echo_skb(priv->netdev, context->echo_index);
> +
> +	context->echo_index = MAX_TX_URBS;
> +	atomic_dec(&priv->active_tx_urbs);
> +
> +	netif_wake_queue(priv->netdev);
> +}
> +
> +static void kvaser_usb_simple_msg_callback(struct urb *urb)
> +{
> +	struct net_device *netdev = urb->context;
> +
> +	kfree(urb->transfer_buffer);
> +
> +	if (urb->status)
> +		netdev_warn(netdev, "urb status received: %d\n",
> +			    urb->status);
> +}
> +
> +static int kvaser_usb_simple_msg_async(struct kvaser_usb_net_priv *priv,
> +				       u8 msg_id)
> +{
> +	struct kvaser_usb *dev = priv->dev;
> +	struct net_device *netdev = priv->netdev;
> +	struct kvaser_msg *msg;
> +	struct urb *urb;
> +	void *buf;
> +	int err;
> +
> +	urb = usb_alloc_urb(0, GFP_ATOMIC);
> +	if (!urb) {
> +		netdev_err(netdev, "No memory left for URBs\n");
> +		return -ENOMEM;
> +	}
> +
> +	buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC);
> +	if (!buf) {

Do you have to free the usb you just allocated?

> +		netdev_err(netdev, "No memory left for USB buffer\n");
> +		return -ENOMEM;
> +	}
> +
> +	msg = (struct kvaser_msg *)buf;
> +	msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple);
> +	msg->id = msg_id;
> +	msg->u.simple.channel = priv->channel;
> +
> +	usb_fill_bulk_urb(urb, dev->udev,
> +			  usb_sndbulkpipe(dev->udev,
> +					  dev->bulk_out->bEndpointAddress),
> +			  buf, msg->len,
> +			  kvaser_usb_simple_msg_callback, priv);
> +	usb_anchor_urb(urb, &priv->tx_submitted);
> +
> +	err = usb_submit_urb(urb, GFP_ATOMIC);
> +	if (err) {
> +		netdev_err(netdev, "Error transmitting URB\n");
> +		usb_unanchor_urb(urb);
> +		kfree(buf);
> +		return err;

and here?

> +	}
> +
> +	usb_free_urb(urb);
> +
> +	return 0;
> +}
> +
> +static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> +{
> +	int i;
> +
> +	usb_kill_anchored_urbs(&priv->tx_submitted);
> +	atomic_set(&priv->active_tx_urbs, 0);
> +
> +	for (i = 0; i < MAX_TX_URBS; i++)
> +		priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> +}
> +
> +static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> +				const struct kvaser_msg *msg)
> +{
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	struct net_device_stats *stats;
> +	struct kvaser_usb_net_priv *priv;
> +	unsigned int new_state;
> +	u8 channel, status, txerr, rxerr, error_factor;
> +
> +	switch (msg->id) {
> +	case CMD_CAN_ERROR_EVENT:
> +		channel = msg->u.error_event.channel;
> +		status =  msg->u.error_event.status;
> +		txerr = msg->u.error_event.tx_errors_count;
> +		rxerr = msg->u.error_event.rx_errors_count;
> +		error_factor = msg->u.error_event.error_factor;
> +		break;
> +	case CMD_LOG_MESSAGE:
> +		channel = msg->u.log_message.channel;
> +		status = msg->u.log_message.data[0];
> +		txerr = msg->u.log_message.data[2];
> +		rxerr = msg->u.log_message.data[3];
> +		error_factor = msg->u.log_message.data[1];
> +		break;
> +	case CMD_CHIP_STATE_EVENT:
> +		channel = msg->u.chip_state_event.channel;
> +		status =  msg->u.chip_state_event.status;
> +		txerr = msg->u.chip_state_event.tx_errors_count;
> +		rxerr = msg->u.chip_state_event.rx_errors_count;
> +		error_factor = 0;
> +		break;
> +	default:
> +		dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
> +			msg->id);
> +		return;
> +	}
> +
> +	if (channel >= dev->nchannels) {
> +		dev_err(dev->udev->dev.parent,
> +			"Invalid channel number (%d)\n", channel);
> +		return;
> +	}
> +
> +	priv = dev->nets[channel];
> +	stats = &priv->netdev->stats;
> +
> +	if (status & M16C_STATE_BUS_RESET) {
> +		kvaser_usb_unlink_tx_urbs(priv);
> +		return;
> +	}
> +
> +	skb = alloc_can_err_skb(priv->netdev, &cf);
> +	if (!skb) {
> +		stats->rx_dropped++;
> +		return;
> +	}
> +
> +	cf->can_id |= CAN_ERR_BUSERROR;
> +
> +	new_state = priv->can.state;
> +
> +	netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
> +
> +	if (status & M16C_STATE_BUS_OFF) {
> +		cf->can_id |= CAN_ERR_BUSOFF;
> +
> +		priv->can.can_stats.bus_off++;
> +		if (!priv->can.restart_ms)
> +			kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> +
> +		netif_carrier_off(priv->netdev);
> +
> +		new_state = CAN_STATE_BUS_OFF;
> +	}
> +
> +	if (status & M16C_STATE_BUS_PASSIVE) {

else if ()

as bus passive and bus off is mutually exclusive.

> +		if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
> +			cf->can_id |= CAN_ERR_CRTL;
> +
> +			if ((txerr > 0) || (rxerr > 0))
> +				cf->data[1] = (txerr > rxerr)
> +						? CAN_ERR_CRTL_TX_PASSIVE
> +						: CAN_ERR_CRTL_RX_PASSIVE;
> +			else
> +				cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
> +					      CAN_ERR_CRTL_RX_PASSIVE;
> +
> +			priv->can.can_stats.error_passive++;
> +		}
> +
> +		new_state = CAN_STATE_ERROR_PASSIVE;
> +	}
> +
> +	if (status == M16C_STATE_BUS_ERROR) {
> +		if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
> +		    ((txerr > 96) || (rxerr > 96))) {

Is is >= 96 ?

> +			cf->can_id |= CAN_ERR_CRTL;
> +			cf->data[1] = (txerr > rxerr)
> +					? CAN_ERR_CRTL_TX_WARNING
> +					: CAN_ERR_CRTL_RX_WARNING;
> +
> +			priv->can.can_stats.error_warning++;
> +			new_state = CAN_STATE_ERROR_WARNING;
> +		} else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) {
> +			cf->can_id |= CAN_ERR_PROT;
> +			cf->data[2] = CAN_ERR_PROT_ACTIVE;
> +
> +			new_state = CAN_STATE_ERROR_ACTIVE;
> +		}
> +	}
> +
> +	if (!status) {
> +		cf->can_id |= CAN_ERR_PROT;
> +		cf->data[2] = CAN_ERR_PROT_ACTIVE;
> +
> +		new_state = CAN_STATE_ERROR_ACTIVE;
> +	}
> +
> +	if (priv->can.restart_ms &&
> +	    (priv->can.state >= CAN_STATE_BUS_OFF) &&
> +	    (new_state < CAN_STATE_BUS_OFF)) {
> +		cf->can_id |= CAN_ERR_RESTARTED;
> +		netif_carrier_on(priv->netdev);
> +
> +		priv->can.can_stats.restarts++;
> +	}
> +
> +	if (error_factor) {
> +		priv->can.can_stats.bus_error++;
> +		stats->rx_errors++;
> +
> +		cf->can_id |= CAN_ERR_PROT;
> +
> +		if (error_factor & M16C_EF_ACKE)
> +			cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
> +		if (error_factor & M16C_EF_CRCE)
> +			cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> +					CAN_ERR_PROT_LOC_CRC_DEL);
> +		if (error_factor & M16C_EF_FORME)
> +			cf->data[2] |= CAN_ERR_PROT_FORM;
> +		if (error_factor & M16C_EF_STFE)
> +			cf->data[2] |= CAN_ERR_PROT_STUFF;
> +		if (error_factor & M16C_EF_BITE0)
> +			cf->data[2] |= CAN_ERR_PROT_BIT0;
> +		if (error_factor & M16C_EF_BITE1)
> +			cf->data[2] |= CAN_ERR_PROT_BIT1;
> +		if (error_factor & M16C_EF_TRE)
> +			cf->data[2] |= CAN_ERR_PROT_TX;
> +	}
> +
> +	cf->data[6] = txerr;
> +	cf->data[7] = rxerr;
> +
> +	priv->bec.txerr = txerr;
> +	priv->bec.rxerr = rxerr;
> +
> +	priv->can.state = new_state;
> +
> +	netif_rx(skb);
> +
> +	stats->rx_packets++;
> +	stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> +				  const struct kvaser_msg *msg)
> +{
> +	struct kvaser_usb_net_priv *priv;
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	struct net_device_stats *stats;
> +	u8 channel = msg->u.rx_can.channel;
> +
> +	if (channel >= dev->nchannels) {
> +		dev_err(dev->udev->dev.parent,
> +			"Invalid channel number (%d)\n", channel);
> +		return;
> +	}
> +
> +	priv = dev->nets[channel];
> +	stats = &priv->netdev->stats;
> +
> +	skb = alloc_can_skb(priv->netdev, &cf);
> +	if (!skb) {
> +		stats->tx_dropped++;
> +		return;
> +	}
> +
> +	cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
> +		     (msg->u.rx_can.msg[1] & 0x3f);
> +	cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
> +
> +	if (msg->id == CMD_RX_EXT_MESSAGE) {
> +		cf->can_id <<= 18;
> +		cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
> +			      ((msg->u.rx_can.msg[3] & 0xff) << 6) |
> +			      (msg->u.rx_can.msg[4] & 0x3f);
> +		cf->can_id |= CAN_EFF_FLAG;
> +	}
> +
> +	if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME) {
> +		cf->can_id |= CAN_RTR_FLAG;
> +	} else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
> +					 MSG_FLAG_NERR)) {
> +		cf->can_id |= CAN_ERR_FLAG;
> +		cf->can_dlc = CAN_ERR_DLC;

Please move the error skb creation handling into a subfunction, use
can_alloc_err_skb() in that function. Please move the:

    if (msg->u.rx_can.flag & ...)

up in this function, before the alloc_can_skb().

> +
> +		netdev_err(priv->netdev, "Unknow error (flags: 0x%02x)\n",
> +			   msg->u.rx_can.flag);
> +
> +		stats->rx_errors++;
> +	} else if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
> +		cf->can_id = CAN_ERR_FLAG | CAN_ERR_CRTL;
> +		cf->can_dlc = CAN_ERR_DLC;
> +		cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> +
> +		stats->rx_over_errors++;
> +		stats->rx_errors++;

This should go into the error skb generation function, too.

> +	} else if (!msg->u.rx_can.flag) {
> +		memcpy(cf->data, &msg->u.rx_can.msg[6], cf->can_dlc);

Please don't copy the contents of RTR frames.

> +	} else {
> +		kfree_skb(skb);

After you have moved the error skb generation into a seperate function,
you should get rid of the kfree(skb), too. The function should look like
this (pseude code):

static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
				  const struct kvaser_msg *msg)
{
	if (channel_invalid())
		return;

	if (msg->u.rx_can.flag & ERROR) {
		kvaser_usb_rx_can_err_msg();
		return;
	} else if (msg->u.rx_can.flag & INVALID_FRAME) {
		return;
	}

	skb = alloc_can_skb();

	/* existing dlc, rtr and data handling code */
	...
}


> +		return;
> +	}
> +
> +	netif_rx(skb);
> +
> +	stats->rx_packets++;
> +	stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static void kvaser_usb_start_chip_reply(const struct kvaser_usb *dev,
> +					const struct kvaser_msg *msg)
> +{
> +	struct kvaser_usb_net_priv *priv;
> +	u8 channel = msg->u.simple.channel;
> +
> +	if (channel >= dev->nchannels) {
> +		dev_err(dev->udev->dev.parent,
> +			"Invalid channel number (%d)\n", channel);
> +		return;
> +	}
> +
> +	priv = dev->nets[channel];
> +
> +	if (completion_done(&priv->start_comp) &&
> +	    netif_queue_stopped(priv->netdev)) {
> +		netif_wake_queue(priv->netdev);
> +	} else {
> +		netif_start_queue(priv->netdev);
> +		complete(&priv->start_comp);
> +	}
> +}
> +
> +static void kvaser_usb_stop_chip_reply(const struct kvaser_usb *dev,
> +				       const struct kvaser_msg *msg)
> +{
> +	struct kvaser_usb_net_priv *priv;
> +	u8 channel = msg->u.simple.channel;
> +
> +	if (channel >= dev->nchannels) {
> +		dev_err(dev->udev->dev.parent,
> +			"Invalid channel number (%d)\n", channel);
> +		return;
> +	}
> +
> +	priv = dev->nets[channel];
> +
> +	complete(&priv->stop_comp);
> +}
> +
> +static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
> +				      const struct kvaser_msg *msg)
> +{
> +	switch (msg->id) {
> +	case CMD_START_CHIP_REPLY:
> +		kvaser_usb_start_chip_reply(dev, msg);
> +		break;
> +
> +	case CMD_STOP_CHIP_REPLY:
> +		kvaser_usb_stop_chip_reply(dev, msg);
> +		break;
> +
> +	case CMD_RX_STD_MESSAGE:
> +	case CMD_RX_EXT_MESSAGE:
> +		kvaser_usb_rx_can_msg(dev, msg);
> +		break;
> +
> +	case CMD_CHIP_STATE_EVENT:
> +	case CMD_CAN_ERROR_EVENT:
> +		kvaser_usb_rx_error(dev, msg);
> +		break;
> +
> +	case CMD_LOG_MESSAGE:
> +		if (msg->u.log_message.flags & MSG_FLAG_ERROR_FRAME)
> +			kvaser_usb_rx_error(dev, msg);
> +		break;
> +
> +	case CMD_TX_ACKNOWLEDGE:
> +		kvaser_usb_tx_acknowledge(dev, msg);
> +		break;
> +
> +	default:
> +		dev_warn(dev->udev->dev.parent,
> +			 "Unhandled message (%d)\n", msg->id);
> +		break;
> +	}
> +}
> +
> +static void kvaser_usb_read_bulk_callback(struct urb *urb)
> +{
> +	struct kvaser_usb *dev = urb->context;
> +	struct kvaser_msg *msg;
> +	int pos = 0;
> +	int err, i;
> +
> +	switch (urb->status) {
> +	case 0:
> +		break;
> +	case -ENOENT:
> +	case -ESHUTDOWN:
> +		return;
> +	default:
> +		dev_info(dev->udev->dev.parent, "Rx URB aborted (%d)\n",
> +			 urb->status);
> +		goto resubmit_urb;
> +	}
> +
> +	while (pos <= urb->actual_length - MSG_HEADER_LEN) {
> +		msg = urb->transfer_buffer + pos;
> +
> +		if (!msg->len)
> +			break;
> +
> +		if (pos + msg->len > urb->actual_length) {
> +			dev_err(dev->udev->dev.parent, "Format error\n");
> +			break;
> +		}
> +
> +		kvaser_usb_handle_message(dev, msg);
> +
> +		pos += msg->len;
> +	}
> +
> +resubmit_urb:
> +	usb_fill_bulk_urb(urb, dev->udev,
> +			  usb_rcvbulkpipe(dev->udev,
> +					  dev->bulk_in->bEndpointAddress),
> +			  urb->transfer_buffer, RX_BUFFER_SIZE,
> +			  kvaser_usb_read_bulk_callback, dev);
> +
> +	err = usb_submit_urb(urb, GFP_ATOMIC);
> +	if (err == -ENODEV) {
> +		for (i = 0; i < dev->nchannels; i++) {
> +			if (!dev->nets[i])
> +				continue;
> +
> +			netif_device_detach(dev->nets[i]->netdev);
> +		}
> +	} else if (err) {
> +		dev_err(dev->udev->dev.parent,
> +			"Failed resubmitting read bulk urb: %d\n", err);
> +	}
> +
> +	return;
> +}
> +
> +static int kvaser_usb_setup_rx_urbs(struct kvaser_usb *dev)
> +{
> +	int i, err = 0;
> +
> +	if (dev->rxinitdone)
> +		return 0;
> +
> +	for (i = 0; i < MAX_RX_URBS; i++) {
> +		struct urb *urb = NULL;
> +		u8 *buf = NULL;
> +		dma_addr_t buf_dma;
> +
> +		urb = usb_alloc_urb(0, GFP_KERNEL);
> +		if (!urb) {
> +			dev_warn(dev->udev->dev.parent,
> +				 "No memory left for URBs\n");
> +			err = -ENOMEM;
> +			break;
> +		}
> +
> +		buf = usb_alloc_coherent(dev->udev, RX_BUFFER_SIZE,
> +					 GFP_KERNEL, &buf_dma);
> +		if (!buf) {
> +			dev_warn(dev->udev->dev.parent,
> +				 "No memory left for USB buffer\n");
> +			usb_free_urb(urb);
> +			err = -ENOMEM;
> +			break;
> +		}
> +
> +		usb_fill_bulk_urb(urb, dev->udev,
> +				  usb_rcvbulkpipe(dev->udev,
> +					  dev->bulk_in->bEndpointAddress),
> +				  buf, RX_BUFFER_SIZE,
> +				  kvaser_usb_read_bulk_callback,
> +				  dev);
> +		urb->transfer_dma = buf_dma;
> +		urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> +		usb_anchor_urb(urb, &dev->rx_submitted);
> +
> +		err = usb_submit_urb(urb, GFP_KERNEL);
> +		if (err) {
> +			usb_unanchor_urb(urb);
> +			usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf,
> +					  buf_dma);

Do you have to call usb_free_usb() here, or does the usb frame work take
care of this?

> +			break;
> +		}
> +
> +		dev->rxbuf[i] = buf;
> +		dev->rxbuf_dma[i] = buf_dma;
> +
> +		usb_free_urb(urb);
> +	}
> +
> +	if (i == 0) {
> +		dev_warn(dev->udev->dev.parent,
> +			 "Cannot setup read URBs, error %d\n", err);
> +		return err;
> +	} else if (i < MAX_RX_URBS) {
> +		dev_warn(dev->udev->dev.parent,
> +			 "RX performances may be slow\n");
> +	}
> +
> +	dev->rxinitdone = true;
> +
> +	return 0;
> +}
> +
> +static int kvaser_usb_set_opt_mode(const struct kvaser_usb_net_priv *priv)
> +{
> +	struct kvaser_msg msg = {
> +		.id = CMD_SET_CTRL_MODE,
> +		.len = MSG_HEADER_LEN +
> +		       sizeof(struct kvaser_msg_ctrl_mode),
> +		.u.ctrl_mode.tid = 0xff,
> +		.u.ctrl_mode.channel = priv->channel,
> +	};
> +
> +	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> +		msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT;
> +	else
> +		msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL;
> +
> +	return kvaser_usb_send_msg(priv->dev, &msg);
> +}
> +
> +static int kvaser_usb_start_chip(struct kvaser_usb_net_priv *priv)
> +{
> +	int err;
> +
> +	init_completion(&priv->start_comp);
> +
> +	err = kvaser_usb_send_simple_msg(priv->dev, CMD_START_CHIP,
> +					 priv->channel);
> +	if (err)
> +		return err;
> +
> +	if (!wait_for_completion_timeout(&priv->start_comp,
> +					 msecs_to_jiffies(START_TIMEOUT)))
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +static int kvaser_usb_open(struct net_device *netdev)
> +{
> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> +	struct kvaser_usb *dev = priv->dev;
> +	int err;
> +
> +	err = open_candev(netdev);
> +	if (err)
> +		return err;
> +
> +	err = kvaser_usb_setup_rx_urbs(dev);
> +	if (err)
> +		goto error;
> +
> +	err = kvaser_usb_set_opt_mode(priv);
> +	if (err)
> +		goto error;
> +
> +	err = kvaser_usb_start_chip(priv);
> +	if (err) {
> +		netdev_warn(netdev, "Cannot start device, error %d\n", err);
> +		goto error;
> +	}
> +
> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> +	return 0;
> +
> +error:
> +	close_candev(netdev);
> +	return err;
> +}
> +
> +static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev)
> +{
> +	int i;
> +
> +	usb_kill_anchored_urbs(&dev->rx_submitted);
> +
> +	for (i = 0; i < MAX_RX_URBS; i++)
> +		usb_free_coherent(dev->udev, RX_BUFFER_SIZE,
> +				  dev->rxbuf[i],
> +				  dev->rxbuf_dma[i]);
> +
> +	for (i = 0; i < MAX_NET_DEVICES; i++) {
> +		struct kvaser_usb_net_priv *priv = dev->nets[i];
> +
> +		if (priv)
> +			kvaser_usb_unlink_tx_urbs(priv);
> +	}
> +}
> +
> +static int kvaser_usb_stop_chip(struct kvaser_usb_net_priv *priv)
> +{
> +	int err;
> +
> +	init_completion(&priv->stop_comp);
> +
> +	err = kvaser_usb_send_simple_msg(priv->dev, CMD_STOP_CHIP,
> +					 priv->channel);
> +	if (err)
> +		return err;
> +
> +	if (!wait_for_completion_timeout(&priv->stop_comp,
> +					 msecs_to_jiffies(STOP_TIMEOUT)))
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
> +static int kvaser_usb_flush_queue(struct kvaser_usb_net_priv *priv)
> +{
> +	struct kvaser_msg msg = {
> +		.id = CMD_FLUSH_QUEUE,
> +		.len = MSG_HEADER_LEN +
> +		       sizeof(struct kvaser_msg_flush_queue),
> +		.u.flush_queue.channel = priv->channel,
> +		.u.flush_queue.flags = 0x00,
> +	};
> +
> +	return kvaser_usb_send_msg(priv->dev, &msg);
> +}
> +
> +static int kvaser_usb_close(struct net_device *netdev)
> +{
> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> +	struct kvaser_usb *dev = priv->dev;
> +	int err;
> +
> +	netif_stop_queue(netdev);
> +
> +	err = kvaser_usb_flush_queue(priv);
> +	if (err)
> +		netdev_warn(netdev, "Cannot flush queue, error %d\n", err);
> +
> +	if (kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, priv->channel))
> +		netdev_warn(netdev, "Cannot reset card, error %d\n", err);
> +
> +	err = kvaser_usb_stop_chip(priv);
> +	if (err)
> +		netdev_warn(netdev, "Cannot stop device, error %d\n", err);
> +
> +	priv->can.state = CAN_STATE_STOPPED;
> +	close_candev(priv->netdev);
> +
> +	return 0;
> +}
> +
> +static void kvaser_usb_write_bulk_callback(struct urb *urb)
> +{
> +	struct kvaser_usb_tx_urb_context *context = urb->context;
> +	struct kvaser_usb_net_priv *priv;
> +	struct net_device *netdev;
> +
> +	if (WARN_ON(!context))
> +		return;
> +
> +	priv = context->priv;
> +	netdev = priv->netdev;
> +
> +	kfree(urb->transfer_buffer);
> +
> +	if (!netif_device_present(netdev))
> +		return;
> +
> +	if (urb->status)
> +		netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status);
> +}
> +
> +static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> +					 struct net_device *netdev)
> +{
> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> +	struct kvaser_usb *dev = priv->dev;
> +	struct net_device_stats *stats = &netdev->stats;
> +	struct can_frame *cf = (struct can_frame *)skb->data;
> +	struct kvaser_usb_tx_urb_context *context = NULL;
> +	struct urb *urb;
> +	void *buf;
> +	struct kvaser_msg *msg;
> +	int i, err;
> +	int ret = NETDEV_TX_OK;
> +
> +	if (can_dropped_invalid_skb(netdev, skb))
> +		return NETDEV_TX_OK;
> +
> +	urb = usb_alloc_urb(0, GFP_ATOMIC);
> +	if (!urb) {
> +		netdev_err(netdev, "No memory left for URBs\n");
> +		stats->tx_dropped++;
> +		dev_kfree_skb(skb);
> +		return NETDEV_TX_OK;
> +	}
> +
> +	buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC);
> +	if (!buf) {
> +		netdev_err(netdev, "No memory left for USB buffer\n");
> +		stats->tx_dropped++;
> +		dev_kfree_skb(skb);
What about the urb?
> +		goto nobufmem;
> +	}
> +
> +	msg = (struct kvaser_msg *)buf;

nitpick: cast is not needed, as buf is void *

> +	msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_tx_can);
> +	msg->u.tx_can.flags = 0;
> +	msg->u.tx_can.channel = priv->channel;
> +
> +	if (cf->can_id & CAN_EFF_FLAG) {
> +		msg->id = CMD_TX_EXT_MESSAGE;
> +		msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f;
> +		msg->u.tx_can.msg[1] = (cf->can_id >> 18) & 0x3f;
> +		msg->u.tx_can.msg[2] = (cf->can_id >> 14) & 0x0f;
> +		msg->u.tx_can.msg[3] = (cf->can_id >> 6) & 0xff;
> +		msg->u.tx_can.msg[4] = cf->can_id & 0x3f;
> +	} else {
> +		msg->id = CMD_TX_STD_MESSAGE;
> +		msg->u.tx_can.msg[0] = (cf->can_id >> 6) & 0x1f;
> +		msg->u.tx_can.msg[1] = cf->can_id & 0x3f;
> +	}
> +
> +	msg->u.tx_can.msg[5] = cf->can_dlc;
> +	memcpy(&msg->u.tx_can.msg[6], cf->data, cf->can_dlc);
> +
> +	if (cf->can_id & CAN_RTR_FLAG)
> +		msg->u.tx_can.flags |= MSG_FLAG_REMOTE_FRAME;
> +
> +	for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++) {
> +		if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
> +			context = &priv->tx_contexts[i];
> +			break;
> +		}
> +	}
> +
> +	if (!context) {
> +		netdev_warn(netdev, "cannot find free context\n");
> +		ret =  NETDEV_TX_BUSY;
> +		goto releasebuf;
> +	}
> +
> +	context->priv = priv;
> +	context->echo_index = i;
> +	context->dlc = cf->can_dlc;
> +
> +	msg->u.tx_can.tid = context->echo_index;
> +
> +	usb_fill_bulk_urb(urb, dev->udev,
> +			  usb_sndbulkpipe(dev->udev,
> +					  dev->bulk_out->bEndpointAddress),
> +			  buf, msg->len,
> +			  kvaser_usb_write_bulk_callback, context);
> +	usb_anchor_urb(urb, &priv->tx_submitted);
> +
> +	can_put_echo_skb(skb, netdev, context->echo_index);
> +
> +	atomic_inc(&priv->active_tx_urbs);
> +
> +	if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
> +		netif_stop_queue(netdev);
> +
> +	err = usb_submit_urb(urb, GFP_ATOMIC);
> +	if (unlikely(err)) {
> +		can_free_echo_skb(netdev, context->echo_index);
> +
> +		atomic_dec(&priv->active_tx_urbs);
> +		usb_unanchor_urb(urb);
> +
> +		stats->tx_dropped++;
> +
> +		if (err == -ENODEV)
> +			netif_device_detach(netdev);
> +		else
> +			netdev_warn(netdev, "Failed tx_urb %d\n", err);
> +
> +		goto releasebuf;
> +	}
> +
> +	netdev->trans_start = jiffies;

Is this still needed?

> +
> +	usb_free_urb(urb);
> +
> +	return NETDEV_TX_OK;
> +
> +releasebuf:
> +	kfree(buf);
> +nobufmem:
> +	usb_free_urb(urb);
> +	return ret;
> +}
> +
> +static const struct net_device_ops kvaser_usb_netdev_ops = {
> +	.ndo_open = kvaser_usb_open,
> +	.ndo_stop = kvaser_usb_close,
> +	.ndo_start_xmit = kvaser_usb_start_xmit,
> +};
> +
> +static struct can_bittiming_const kvaser_usb_bittiming_const = {
> +	.name = "kvaser_usb",
> +	.tseg1_min = KVASER_USB_TSEG1_MIN,
> +	.tseg1_max = KVASER_USB_TSEG1_MAX,
> +	.tseg2_min = KVASER_USB_TSEG2_MIN,
> +	.tseg2_max = KVASER_USB_TSEG2_MAX,
> +	.sjw_max = KVASER_USB_SJW_MAX,
> +	.brp_min = KVASER_USB_BRP_MIN,
> +	.brp_max = KVASER_USB_BRP_MAX,
> +	.brp_inc = KVASER_USB_BRP_INC,
> +};
> +
> +static int kvaser_usb_set_bittiming(struct net_device *netdev)
> +{
> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> +	struct can_bittiming *bt = &priv->can.bittiming;
> +	struct kvaser_usb *dev = priv->dev;
> +	struct kvaser_msg msg = {
> +		.id = CMD_SET_BUS_PARAMS,
> +		.len = MSG_HEADER_LEN +
> +		       sizeof(struct kvaser_msg_busparams),
> +		.u.busparams.channel = priv->channel,
> +		.u.busparams.tid = 0xff,
> +		.u.busparams.bitrate = cpu_to_le32(bt->bitrate),
> +		.u.busparams.sjw = bt->sjw,
> +		.u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1,
> +		.u.busparams.tseg2 = bt->phase_seg2,
> +	};
> +
> +	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
> +		msg.u.busparams.no_samp = 3;
> +	else
> +		msg.u.busparams.no_samp = 1;
> +
> +	return kvaser_usb_send_msg(dev, &msg);
> +}
> +
> +static int kvaser_usb_set_mode(struct net_device *netdev,
> +			       enum can_mode mode)
> +{
> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
> +	int err;
> +
> +	switch (mode) {
> +	case CAN_MODE_START:
> +		err = kvaser_usb_simple_msg_async(priv, CMD_START_CHIP);
> +		if (err)
> +			return err;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +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 = priv->bec;
> +
> +	return 0;
> +}
> +
> +static int kvaser_usb_init_one(struct usb_interface *intf,
> +			       const struct usb_device_id *id, int channel)
> +{
> +	struct kvaser_usb *dev = usb_get_intfdata(intf);
> +	struct net_device *netdev;
> +	struct kvaser_usb_net_priv *priv;
> +	int i, err;
> +
> +	netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS);
> +	if (!netdev) {
> +		dev_err(&intf->dev, "Cannot alloc candev\n");
> +		return -ENOMEM;
> +	}
> +
> +	priv = netdev_priv(netdev);
> +
> +	init_completion(&priv->start_comp);
> +	init_completion(&priv->stop_comp);
> +
> +	init_usb_anchor(&priv->tx_submitted);
> +	atomic_set(&priv->active_tx_urbs, 0);
> +
> +	for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++)
> +		priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> +
> +	priv->dev = dev;
> +	priv->netdev = netdev;
> +	priv->channel = channel;
> +
> +	priv->can.state = CAN_STATE_STOPPED;
> +	priv->can.clock.freq = CAN_USB_CLOCK;
> +	priv->can.bittiming_const = &kvaser_usb_bittiming_const;
> +	priv->can.do_set_bittiming = kvaser_usb_set_bittiming;
> +	priv->can.do_set_mode = kvaser_usb_set_mode;
> +	if (id->driver_info & KVASER_HAS_TXRX_ERRORS)
> +		priv->can.do_get_berr_counter = kvaser_usb_get_berr_counter;
> +	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
> +	if (id->driver_info & KVASER_HAS_SILENT_MODE)
> +		priv->can.ctrlmode_supported |= CAN_CTRLMODE_LISTENONLY;
> +
> +	netdev->flags |= IFF_ECHO;
> +
> +	netdev->netdev_ops = &kvaser_usb_netdev_ops;
> +
> +	SET_NETDEV_DEV(netdev, &intf->dev);
> +
> +	dev->nets[channel] = priv;
> +
> +	err = register_candev(netdev);
> +	if (err) {
> +		dev_err(&intf->dev, "Failed to register can device\n");
> +		free_candev(netdev);
> +		return err;
> +	}
> +
> +	netdev_dbg(netdev, "device registered\n");
> +
> +	return 0;
> +}
> +
> +static void kvaser_usb_get_endpoints(const struct usb_interface *intf,
> +				     struct usb_endpoint_descriptor **in,
> +				     struct usb_endpoint_descriptor **out)
> +{
> +	const struct usb_host_interface *iface_desc;
> +	struct usb_endpoint_descriptor *endpoint;
> +	int i;
> +
> +	iface_desc = &intf->altsetting[0];
> +
> +	for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
> +		endpoint = &iface_desc->endpoint[i].desc;
> +
> +		if (usb_endpoint_is_bulk_in(endpoint))
> +			*in = endpoint;
> +
> +		if (usb_endpoint_is_bulk_out(endpoint))
> +			*out = endpoint;
> +	}
> +}
> +
> +static int kvaser_usb_probe(struct usb_interface *intf,
> +			    const struct usb_device_id *id)
> +{
> +	struct kvaser_usb *dev;
> +	int err = -ENOMEM;
> +	int i;
> +
> +	dev = devm_kzalloc(&intf->dev, sizeof(*dev), GFP_KERNEL);
> +	if (!dev)
> +		return -ENOMEM;
> +
> +	kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
> +	if (!dev->bulk_in || !dev->bulk_out) {
> +		dev_err(&intf->dev, "Cannot get usb endpoint(s)");
> +		return err;
> +	}
> +
> +	dev->udev = interface_to_usbdev(intf);
> +
> +	init_usb_anchor(&dev->rx_submitted);
> +
> +	usb_set_intfdata(intf, dev);
> +
> +	for (i = 0; i < MAX_NET_DEVICES; i++)
> +		kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, i);
> +
> +	err = kvaser_usb_get_software_info(dev);
> +	if (err) {
> +		dev_err(&intf->dev,
> +			"Cannot get software infos, error %d\n", err);
> +		return err;
> +	}
> +
> +	err = kvaser_usb_get_card_info(dev);
> +	if (err) {
> +		dev_err(&intf->dev,
> +			"Cannot get card infos, error %d\n", err);
> +		return err;
> +	}
> +
> +	dev_dbg(&intf->dev, "Firmware version: %d.%d.%d\n",
> +		((dev->fw_version >> 24) & 0xff),
> +		((dev->fw_version >> 16) & 0xff),
> +		(dev->fw_version & 0xffff));
> +
> +	for (i = 0; i < dev->nchannels; i++)
> +		kvaser_usb_init_one(intf, id, i);
> +
> +	return 0;
> +}
> +
> +static void kvaser_usb_disconnect(struct usb_interface *intf)
> +{
> +	struct kvaser_usb *dev = usb_get_intfdata(intf);
> +	int i;
> +
> +	usb_set_intfdata(intf, NULL);
> +
> +	if (!dev)
> +		return;
> +
> +	for (i = 0; i < dev->nchannels; i++) {
> +		if (!dev->nets[i])
> +			continue;
> +
> +		unregister_netdev(dev->nets[i]->netdev);
> +	}
> +
> +	kvaser_usb_unlink_all_urbs(dev);
> +
> +	for (i = 0; i < dev->nchannels; i++)
> +		free_candev(dev->nets[i]->netdev);
> +}
> +
> +static struct usb_driver kvaser_usb_driver = {
> +	.name = "kvaser_usb",
> +	.probe = kvaser_usb_probe,
> +	.disconnect = kvaser_usb_disconnect,
> +	.id_table = kvaser_usb_table
> +};
> +
> +module_usb_driver(kvaser_usb_driver);
> +
> +MODULE_AUTHOR("Olivier Sobrie <olivier-Ui3EtX6WB9GzQB+pC5nmwQ@public.gmane.org>");
> +MODULE_DESCRIPTION("CAN driver for Kvaser CAN/USB devices");
> +MODULE_LICENSE("GPL v2");
> 

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: 259 bytes --]

^ permalink raw reply

* Re: Oops with latest (netfilter) nf-next tree, when unloading iptable_nat
From: Jesper Dangaard Brouer @ 2012-09-21  9:47 UTC (permalink / raw)
  To: Pablo Neira Ayuso, David Miller
  Cc: Patrick McHardy, Florian Westphal, netfilter-devel, netdev
In-Reply-To: <20120921010025.GA20479@1984>

On Fri, 2012-09-21 at 03:00 +0200, Pablo Neira Ayuso wrote:
> On Thu, Sep 20, 2012 at 07:06:52PM +0200, Patrick McHardy wrote:
> > On Thu, 20 Sep 2012, Patrick McHardy wrote:
[cut]
(discussion of fixes by Patrick and Florian)
(...settling on Patricks second patch)

> Makes sense. And we can revisit this to improve it later.
> 
> I'll take this patch. I'll send a batch with updates for the nf-nat
> thin asap.

What git tree is that?

I'm trying to work off Pablo's nf-next tree (for my IPVS changes):
  git://1984.lsi.us.es/nf-next

But I don't see the patch in that tree ...yet.


Notice, the bug is also present in DaveM's net-next tree.
(I know I stated earlier that it didn't affect net-next, but I just
forgot to select the new netfilter .config options for nat)

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer


Oops output, for the sake of completeness:
------------------------------------------
[  866.878092] general protection fault: 0000 [#1] SMP 
[  866.878986] Modules linked in: netconsole ip_vs_lblc ip_vs_lc ip_vs_rr ip_vs libcrc32c ipt_MASQUERADE nf_nat_ipv4(-) nf_nat iptable_mangle xt_mark ip6table_mangle xt_LOG ip6table_filter ip6_tables virtio_net virtio_balloon [last unloaded: iptable_nat]
[  866.879045] CPU 0 
[  866.879045] Pid: 4053, comm: modprobe Not tainted 3.6.0-rc5-net-next-sysctl-tcp+ #13 Red Hat KVM
[  866.879045] RIP: 0010:[<ffffffffa002c2dd>]  [<ffffffffa002c2dd>] nf_nat_proto_clean+0x6d/0xc0 [nf_nat]
[  866.879045] RSP: 0018:ffff880078a41e18  EFLAGS: 00010246
[  866.879045] RAX: 0000000000000000 RBX: ffff880079142500 RCX: dead000000200200
[  866.879045] RDX: dead000000200200 RSI: ffff880078a41e88 RDI: ffffffffa002f268
[  866.879045] RBP: ffff880078a41e28 R08: ffff880078a40000 R09: 0000000000000002
[  866.879045] R10: 0000000000000000 R11: 0000000000000246 R12: ffffffff81c6db40
[  866.879045] R13: ffff880037d8f008 R14: ffff880037d8f000 R15: ffff880078a41e88
[  866.879045] FS:  00007fc30c11a700(0000) GS:ffff88007cc00000(0000) knlGS:0000000000000000
[  866.879045] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  866.879045] CR2: 00007fc30c127000 CR3: 00000000780ef000 CR4: 00000000000006f0
[  866.879045] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  866.879045] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[  866.879045] Process modprobe (pid: 4053, threadinfo ffff880078a40000, task ffff88007a379650)
[  866.879045] Stack:
[  866.879045]  ffff880078a41e68 ffffffffa002c270 ffff880078a41e78 ffffffff81541413
[  866.879045]  ffffffff00000000 0000147578a40303 ffff880078a41e68 ffffffff81c6db40
[  866.879045]  ffff880078a41e88 ffffffffa00358a0 0000000000000000 000000000040f5b0
[  866.879045] Call Trace:
[  866.879045]  [<ffffffffa002c270>] ? nf_nat_net_exit+0x50/0x50 [nf_nat]
[  866.879045]  [<ffffffff81541413>] nf_ct_iterate_cleanup+0xc3/0x170
[  866.879045]  [<ffffffffa002c50a>] nf_nat_l3proto_unregister+0x8a/0x100 [nf_nat]
[  866.879045]  [<ffffffff81290303>] ? proc_keys_next+0x23/0x60
[  866.879045]  [<ffffffffa0035848>] nf_nat_l3proto_ipv4_exit+0x10/0x23 [nf_nat_ipv4]
[  866.879045]  [<ffffffff810988b5>] sys_delete_module+0x235/0x2b0
[  866.879045]  [<ffffffff810b38c3>] ? __audit_syscall_entry+0x1b3/0x1f0
[  866.879045]  [<ffffffff810b36e6>] ? __audit_syscall_exit+0x3e6/0x410
[  866.879045]  [<ffffffff81640122>] system_call_fastpath+0x16/0x1b
[  866.879045] Code: 75 6c 0f b6 46 01 84 c0 74 05 3a 42 3e 75 5f 80 7e 02 00 74 41 48 c7 c7 68 f2 02 a0 e8 1d c5 60 e1 48 8b 03 48 8b 53 08 48 85 c0 <48> 89 02 74 04 48 89 50 08 48 be 00 02 20 00 00 00 ad de 48 c7 
[  866.879045] RIP  [<ffffffffa002c2dd>] nf_nat_proto_clean+0x6d/0xc0 [nf_nat]
[  866.879045]  RSP <ffff880078a41e18>



^ permalink raw reply

* [PATCH v3] ucc_geth: Lockless xmit
From: Joakim Tjernlund @ 2012-09-21  9:11 UTC (permalink / raw)
  To: Francois Romieu, netdev; +Cc: Joakim Tjernlund
In-Reply-To: <1348129021-28333-1-git-send-email-Joakim.Tjernlund@transmode.se>

Currently ucc_geth_start_xmit wraps IRQ off for the
whole body just to be safe. By rearranging the code a bit
one can avoid the lock completely.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---

 v2: Move assignment of ugeth->tx_skbuff[txQ][ugeth->skb_curtx[txQ]]
     inside IRQ off section to prevent racing against
     ucc_geth_tx(). Spotted by Francois Romieu <romieu@fr.zoreil.com>

 v3: Lockless xmit
     Here is my attemept to do lockless xmit. Thanks to
     Francois Romieu <romieu@fr.zoreil.com> for the idea.

 drivers/net/ethernet/freescale/ucc_geth.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 9ac14f8..040aa70 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3177,19 +3177,20 @@ static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	u8 __iomem *bd;			/* BD pointer */
 	u32 bd_status;
 	u8 txQ = 0;
-	unsigned long flags;
 
 	ugeth_vdbg("%s: IN", __func__);
 
-	spin_lock_irqsave(&ugeth->lock, flags);
-
 	dev->stats.tx_bytes += skb->len;
 
+	/* We are running in BH disabled context with netif_tx_lock
+	 * and TX reclaim runs via tp->napi.poll inside of a software
+	 * interrupt.  Furthermore, IRQ processing runs lockless so we have
+	 * no IRQ context deadlocks to worry about either.  Rejoice!
+	 */
+
 	/* Start from the next BD that should be filled */
 	bd = ugeth->txBd[txQ];
 	bd_status = in_be32((u32 __iomem *)bd);
-	/* Save the skb pointer so we can free it later */
-	ugeth->tx_skbuff[txQ][ugeth->skb_curtx[txQ]] = skb;
 
 	/* Update the current skb pointer (wrapping if this was the last) */
 	ugeth->skb_curtx[txQ] =
@@ -3207,6 +3208,8 @@ static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	/* set bd status and length */
 	out_be32((u32 __iomem *)bd, bd_status);
+	/* Save the skb pointer so we can free it later */
+	ugeth->tx_skbuff[txQ][ugeth->skb_curtx[txQ]] = skb;
 
 	/* Move to next BD in the ring */
 	if (!(bd_status & T_W))
@@ -3238,8 +3241,6 @@ static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	uccf = ugeth->uccf;
 	out_be16(uccf->p_utodr, UCC_FAST_TOD);
 #endif
-	spin_unlock_irqrestore(&ugeth->lock, flags);
-
 	return NETDEV_TX_OK;
 }
 
@@ -3387,10 +3388,8 @@ static int ucc_geth_poll(struct napi_struct *napi, int budget)
 	ug_info = ugeth->ug_info;
 
 	/* Tx event processing */
-	spin_lock(&ugeth->lock);
 	for (i = 0; i < ug_info->numQueuesTx; i++)
 		ucc_geth_tx(ugeth->ndev, i);
-	spin_unlock(&ugeth->lock);
 
 	howmany = 0;
 	for (i = 0; i < ug_info->numQueuesRx; i++)
-- 
1.7.8.6

^ permalink raw reply related

* Re: [PATCH v2] ucc_geth: Reduce IRQ off in xmit path
From: Joakim Tjernlund @ 2012-09-21  9:00 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, romieu
In-Reply-To: <20120920.170852.391382615902951147.davem@davemloft.net>

David Miller <davem@davemloft.net> wrote on 2012/09/20 23:08:52:
>
> From: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> Date: Thu, 20 Sep 2012 10:17:01 +0200
>
> > Currently ucc_geth_start_xmit wraps IRQ off for the
> > whole body just to be safe.
> > Reduce the IRQ off period to a minimum.
> >
> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> > ---
> >
> >  v2: Move assignment of ugeth->tx_skbuff[txQ][ugeth->skb_curtx[txQ]]
> >      inside IRQ off section to prevent racing against
> >      ucc_geth_tx(). Spotted by Francois Romieu <romieu@fr.zoreil.com>
>
> I agree with Francois's initial analysis, and disagree with you're
> response to him, wrt. the suggest to remove all locking entirely.
>
> Unlike what you claim, there isn't much of a gain at all from merely
> make the window of lock holding smaller, especially on the scale
> in which you are doing it here.
>
> Whereas removing the lock and the atomic completely, as tg3 does,
> will give very significant performance gains.
>
> The locking cost of grabbing the spinlock, and the memory transactions
> associated with it, dominate.
>
> Furthermore, even if the gains of your change are non-trivial, you
> haven't documented it.  So unless you should some noticable gains from
> this, it's just code masterbation as far as I'm concerned and I'm
> therefore inclined to not apply patches like this.
>
> TG3's core interrupt locking is not that difficult to understand and
> replicate in other drivers, so I dismiss your attempts to avoid that
> approach on difficulty grounds as well.

OK, I will give it a go. Got something working that I will send in shortly.
I hope the other patches were OK?

 Jocke

^ permalink raw reply

* [PATCH] tcp: sysctl for initial receive window
From: Jesper Dangaard Brouer @ 2012-09-21  8:55 UTC (permalink / raw)
  To: netdev; +Cc: Jesper Dangaard Brouer, Nandita Dukkipati, Eric Dumazet

Make it possible to adjust the TCP default initial advertised receive
window, via sysctl /proc/sys/net/ipv4/tcp_init_recv_window.

The window size is this value multiplied by the MSS of the connection.
The default value is (still) 10, as descibed in commit 356f039822b
(TCP: increase default initial receive window.)

Allow minimum value of 1, but recommend against setting value below 2
in the documentation.

Its possible to control/override this value per route table entry via
the iproute2 option initrwnd.  Having the global default exported via
sysctl, helps determine the default setting, and make is easier to
adjust.

Cc: Nandita Dukkipati <nanditad@google.com>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---

 Documentation/networking/ip-sysctl.txt |   12 ++++++++++++
 include/net/tcp.h                      |    1 +
 net/ipv4/sysctl_net_ipv4.c             |    9 +++++++++
 net/ipv4/tcp_input.c                   |    6 +++---
 net/ipv4/tcp_output.c                  |    8 +++++---
 5 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index c7fc107..684131c 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -257,6 +257,18 @@ tcp_frto_response - INTEGER
 		  to the values prior timeout
 	Default: 0 (rate halving based)
 
+tcp_init_recv_window - INTEGER
+	Default initial advertised receive window.  Actual window size
+	is this value multiplied by the MSS of the connection.  Its
+	possible to control/override this value per route table entry
+	via the iproute2 option initrwnd.
+	Minimum value is 1, but 2 is the recommended minimum.
+	The effective max value, is limited by the sockets receive
+	buffer size (default tcp_rmem[1], and possibly scaled by
+	tcp_adv_win_scale), and can further be limited by window
+	clamp.
+	Default: 10
+
 tcp_keepalive_time - INTEGER
 	How often TCP sends out keepalive messages when keepalive is enabled.
 	Default: 2hours.
diff --git a/include/net/tcp.h b/include/net/tcp.h
index a8cb00c..3334852 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -292,6 +292,7 @@ extern int sysctl_tcp_thin_dupack;
 extern int sysctl_tcp_early_retrans;
 extern int sysctl_tcp_limit_output_bytes;
 extern int sysctl_tcp_challenge_ack_limit;
+extern u32 sysctl_tcp_init_recv_window;
 
 extern atomic_long_t tcp_memory_allocated;
 extern struct percpu_counter tcp_sockets_allocated;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 9205e49..9bb6608 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -27,6 +27,7 @@
 #include <net/tcp_memcontrol.h>
 
 static int zero;
+static int one = 1;
 static int two = 2;
 static int tcp_retr1_max = 255;
 static int ip_local_port_range_min[] = { 1, 1 };
@@ -794,6 +795,14 @@ static struct ctl_table ipv4_table[] = {
 		.proc_handler	= proc_dointvec_minmax,
 		.extra1		= &zero
 	},
+	{
+		.procname	= "tcp_init_recv_window",
+		.data		= &sysctl_tcp_init_recv_window,
+		.maxlen		= sizeof(sysctl_tcp_init_recv_window),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= &one
+	},
 	{ }
 };
 
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index e2bec81..bbf7a33 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -356,14 +356,14 @@ static void tcp_grow_window(struct sock *sk, const struct sk_buff *skb)
 static void tcp_fixup_rcvbuf(struct sock *sk)
 {
 	u32 mss = tcp_sk(sk)->advmss;
-	u32 icwnd = TCP_DEFAULT_INIT_RCVWND;
+	u32 icwnd = sysctl_tcp_init_recv_window;
 	int rcvmem;
 
-	/* Limit to 10 segments if mss <= 1460,
+	/* Limit to default 10 segments if mss <= 1460,
 	 * or 14600/mss segments, with a minimum of two segments.
 	 */
 	if (mss > 1460)
-		icwnd = max_t(u32, (1460 * TCP_DEFAULT_INIT_RCVWND) / mss, 2);
+		icwnd = max_t(u32, (1460 * icwnd) / mss, 2);
 
 	rcvmem = SKB_TRUESIZE(mss + MAX_TCP_HEADER);
 	while (tcp_win_from_space(rcvmem) < mss)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index cfe6ffe..5f3b26d 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -59,6 +59,8 @@ int sysctl_tcp_limit_output_bytes __read_mostly = 131072;
  */
 int sysctl_tcp_tso_win_divisor __read_mostly = 3;
 
+u32 sysctl_tcp_init_recv_window __read_mostly = TCP_DEFAULT_INIT_RCVWND;
+
 int sysctl_tcp_mtu_probing __read_mostly = 0;
 int sysctl_tcp_base_mss __read_mostly = TCP_BASE_MSS;
 
@@ -235,14 +237,14 @@ void tcp_select_initial_window(int __space, __u32 mss,
 	}
 
 	/* Set initial window to a value enough for senders starting with
-	 * initial congestion window of TCP_DEFAULT_INIT_RCVWND. Place
+	 * initial congestion window of sysctl_tcp_init_recv_window. Place
 	 * a limit on the initial window when mss is larger than 1460.
 	 */
 	if (mss > (1 << *rcv_wscale)) {
-		int init_cwnd = TCP_DEFAULT_INIT_RCVWND;
+		int init_cwnd = sysctl_tcp_init_recv_window;
 		if (mss > 1460)
 			init_cwnd =
-			max_t(u32, (1460 * TCP_DEFAULT_INIT_RCVWND) / mss, 2);
+			max_t(u32, (1460 * init_cwnd) / mss, 2);
 		/* when initializing use the value from init_rcv_wnd
 		 * rather than the default from above
 		 */

^ permalink raw reply related

* [PATCH] net: change return values from -EACCES to -EPERM
From: Zhao Hongjiang @ 2012-09-21  8:37 UTC (permalink / raw)
  To: davem; +Cc: netdev, serge.hallyn, Eric W. Biederman

From: Zhao Hongjiang <zhaohongjiang@huawei.com>

Change return value from -EACCES to -EPERM when the permission check fails.

Signed-off-by: Zhao Hongjiang <zhaohongjiang@huawei.com>
---
 net/bluetooth/bnep/sock.c |    4 ++--
 net/bluetooth/cmtp/sock.c |    4 ++--
 net/bluetooth/hci_sock.c  |   16 ++++++++--------
 net/bluetooth/hidp/sock.c |    4 ++--
 net/ipv4/devinet.c        |    4 ++--
 net/netrom/af_netrom.c    |    2 +-
 6 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/net/bluetooth/bnep/sock.c b/net/bluetooth/bnep/sock.c
index 5b6cc0b..e7154a5 100644
--- a/net/bluetooth/bnep/sock.c
+++ b/net/bluetooth/bnep/sock.c
@@ -64,7 +64,7 @@ static int bnep_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long
 	switch (cmd) {
 	case BNEPCONNADD:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;

 		if (copy_from_user(&ca, argp, sizeof(ca)))
 			return -EFAULT;
@@ -90,7 +90,7 @@ static int bnep_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long

 	case BNEPCONNDEL:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;

 		if (copy_from_user(&cd, argp, sizeof(cd)))
 			return -EFAULT;
diff --git a/net/bluetooth/cmtp/sock.c b/net/bluetooth/cmtp/sock.c
index d5cacef..aacb802 100644
--- a/net/bluetooth/cmtp/sock.c
+++ b/net/bluetooth/cmtp/sock.c
@@ -78,7 +78,7 @@ static int cmtp_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long
 	switch (cmd) {
 	case CMTPCONNADD:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;

 		if (copy_from_user(&ca, argp, sizeof(ca)))
 			return -EFAULT;
@@ -103,7 +103,7 @@ static int cmtp_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long

 	case CMTPCONNDEL:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;

 		if (copy_from_user(&cd, argp, sizeof(cd)))
 			return -EFAULT;
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index bb64331..07f0739 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -490,7 +490,7 @@ static int hci_sock_bound_ioctl(struct sock *sk, unsigned int cmd,
 	switch (cmd) {
 	case HCISETRAW:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;

 		if (test_bit(HCI_QUIRK_RAW_DEVICE, &hdev->quirks))
 			return -EPERM;
@@ -510,12 +510,12 @@ static int hci_sock_bound_ioctl(struct sock *sk, unsigned int cmd,

 	case HCIBLOCKADDR:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;
 		return hci_sock_blacklist_add(hdev, (void __user *) arg);

 	case HCIUNBLOCKADDR:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;
 		return hci_sock_blacklist_del(hdev, (void __user *) arg);

 	default:
@@ -546,22 +546,22 @@ static int hci_sock_ioctl(struct socket *sock, unsigned int cmd,

 	case HCIDEVUP:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;
 		return hci_dev_open(arg);

 	case HCIDEVDOWN:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;
 		return hci_dev_close(arg);

 	case HCIDEVRESET:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;
 		return hci_dev_reset(arg);

 	case HCIDEVRESTAT:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;
 		return hci_dev_reset_stat(arg);

 	case HCISETSCAN:
@@ -573,7 +573,7 @@ static int hci_sock_ioctl(struct socket *sock, unsigned int cmd,
 	case HCISETACLMTU:
 	case HCISETSCOMTU:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;
 		return hci_dev_cmd(cmd, argp);

 	case HCIINQUIRY:
diff --git a/net/bluetooth/hidp/sock.c b/net/bluetooth/hidp/sock.c
index eca3889..82a829d 100644
--- a/net/bluetooth/hidp/sock.c
+++ b/net/bluetooth/hidp/sock.c
@@ -62,7 +62,7 @@ static int hidp_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long
 	switch (cmd) {
 	case HIDPCONNADD:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;

 		if (copy_from_user(&ca, argp, sizeof(ca)))
 			return -EFAULT;
@@ -97,7 +97,7 @@ static int hidp_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long

 	case HIDPCONNDEL:
 		if (!capable(CAP_NET_ADMIN))
-			return -EACCES;
+			return -EPERM;

 		if (copy_from_user(&cd, argp, sizeof(cd)))
 			return -EFAULT;
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 7b00556..2a6abc1 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -722,7 +722,7 @@ int devinet_ioctl(struct net *net, unsigned int cmd, void __user *arg)
 		break;

 	case SIOCSIFFLAGS:
-		ret = -EACCES;
+		ret = -EPERM;
 		if (!capable(CAP_NET_ADMIN))
 			goto out;
 		break;
@@ -730,7 +730,7 @@ int devinet_ioctl(struct net *net, unsigned int cmd, void __user *arg)
 	case SIOCSIFBRDADDR:	/* Set the broadcast address */
 	case SIOCSIFDSTADDR:	/* Set the destination address */
 	case SIOCSIFNETMASK: 	/* Set the netmask for the interface */
-		ret = -EACCES;
+		ret = -EPERM;
 		if (!capable(CAP_NET_ADMIN))
 			goto out;
 		ret = -EINVAL;
diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
index 1b9024e..7261eb8 100644
--- a/net/netrom/af_netrom.c
+++ b/net/netrom/af_netrom.c
@@ -601,7 +601,7 @@ static int nr_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 		if (!capable(CAP_NET_BIND_SERVICE)) {
 			dev_put(dev);
 			release_sock(sk);
-			return -EACCES;
+			return -EPERM;
 		}
 		nr->user_addr   = addr->fsa_digipeater[0];
 		nr->source_addr = addr->fsa_ax25.sax25_call;
-- 1.7.1

^ permalink raw reply related

* removing the timer from cdc-ncm
From: Oliver Neukum @ 2012-09-21  8:35 UTC (permalink / raw)
  To: Alexey ORISHKO, bjorn-yOkvZcmFvRU, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA

Hi,

here is the patch that does everything I consider theoretically necessary
to have bundling of frames in usbnet and adapting cdc-ncm to it.

I'd appreciate any review in case I am doing something stupid.

	Regards
		Oliver

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 4cd582a..56ef743 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -135,9 +135,6 @@ struct cdc_ncm_ctx {
 	u16 connected;
 };
 
-static void cdc_ncm_txpath_bh(unsigned long param);
-static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
-static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
 static struct usb_driver cdc_ncm_driver;
 
 static void
@@ -464,10 +461,6 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
 	if (ctx == NULL)
 		return -ENODEV;
 
-	hrtimer_init(&ctx->tx_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
-	ctx->tx_timer.function = &cdc_ncm_tx_timer_cb;
-	ctx->bh.data = (unsigned long)ctx;
-	ctx->bh.func = cdc_ncm_txpath_bh;
 	atomic_set(&ctx->stop, 0);
 	spin_lock_init(&ctx->mtx);
 	ctx->netdev = dev->net;
@@ -650,7 +643,7 @@ static void cdc_ncm_zero_fill(u8 *ptr, u32 first, u32 end, u32 max)
 	memset(ptr + first, 0, end - first);
 }
 
-static struct sk_buff *
+static int
 cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 {
 	struct sk_buff *skb_out;
@@ -659,12 +652,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 	u32 last_offset;
 	u16 n = 0, index;
 	u8 ready2send = 0;
-
-	/* if there is a remaining skb, it gets priority */
-	if (skb != NULL)
-		swap(skb, ctx->tx_rem_skb);
-	else
-		ready2send = 1;
+	u8 error = 0;
 
 	/*
 	 * +----------------+
@@ -690,7 +678,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 				dev_kfree_skb_any(skb);
 				ctx->netdev->stats.tx_dropped++;
 			}
-			goto exit_no_skb;
+			return -EBUSY;
 		}
 
 		/* make room for NTH and NDP */
@@ -719,28 +707,15 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 		/* compute maximum buffer size */
 		rem = ctx->tx_max - offset;
 
-		if (skb == NULL) {
-			skb = ctx->tx_rem_skb;
-			ctx->tx_rem_skb = NULL;
-
-			/* check for end of skb */
-			if (skb == NULL)
-				break;
-		}
-
 		if (skb->len > rem) {
 			if (n == 0) {
 				/* won't fit, MTU problem? */
 				dev_kfree_skb_any(skb);
 				skb = NULL;
 				ctx->netdev->stats.tx_dropped++;
+				error = 1;
 			} else {
-				/* no room for skb - store for later */
-				if (ctx->tx_rem_skb != NULL) {
-					dev_kfree_skb_any(ctx->tx_rem_skb);
-					ctx->netdev->stats.tx_dropped++;
-				}
-				ctx->tx_rem_skb = skb;
+
 				skb = NULL;
 				ready2send = 1;
 			}
@@ -768,13 +743,6 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 		skb = NULL;
 	}
 
-	/* free up any dangling skb */
-	if (skb != NULL) {
-		dev_kfree_skb_any(skb);
-		skb = NULL;
-		ctx->netdev->stats.tx_dropped++;
-	}
-
 	ctx->tx_curr_frame_num = n;
 
 	if (n == 0) {
@@ -791,9 +759,7 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 		ctx->tx_curr_skb = skb_out;
 		ctx->tx_curr_offset = offset;
 		ctx->tx_curr_last_offset = last_offset;
-		/* set the pending count */
-		if (n < CDC_NCM_RESTART_TIMER_DATAGRAM_CNT)
-			ctx->tx_timer_pending = CDC_NCM_TIMER_PENDING_CNT;
+
 		goto exit_no_skb;
 
 	} else {
@@ -874,71 +840,37 @@ cdc_ncm_fill_tx_frame(struct cdc_ncm_ctx *ctx, struct sk_buff *skb)
 	/* return skb */
 	ctx->tx_curr_skb = NULL;
 	ctx->netdev->stats.tx_packets += ctx->tx_curr_frame_num;
-	return skb_out;
 
-exit_no_skb:
-	/* Start timer, if there is a remaining skb */
-	if (ctx->tx_curr_skb != NULL)
-		cdc_ncm_tx_timeout_start(ctx);
-	return NULL;
-}
-
-static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx)
-{
-	/* start timer, if not already started */
-	if (!(hrtimer_active(&ctx->tx_timer) || atomic_read(&ctx->stop)))
-		hrtimer_start(&ctx->tx_timer,
-				ktime_set(0, CDC_NCM_TIMER_INTERVAL),
-				HRTIMER_MODE_REL);
-}
+	if (error)
+		return -EBUSY;
+	if (ready2send)
+		return -EBUSY;
+	else
+		return 0;
 
-static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *timer)
-{
-	struct cdc_ncm_ctx *ctx =
-			container_of(timer, struct cdc_ncm_ctx, tx_timer);
+exit_no_skb:
 
-	if (!atomic_read(&ctx->stop))
-		tasklet_schedule(&ctx->bh);
-	return HRTIMER_NORESTART;
+	return -EAGAIN;
 }
 
-static void cdc_ncm_txpath_bh(unsigned long param)
+static int cdc_ncm_tx_bundle(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
 {
-	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)param;
-
-	spin_lock_bh(&ctx->mtx);
-	if (ctx->tx_timer_pending != 0) {
-		ctx->tx_timer_pending--;
-		cdc_ncm_tx_timeout_start(ctx);
-		spin_unlock_bh(&ctx->mtx);
-	} else if (ctx->netdev != NULL) {
-		spin_unlock_bh(&ctx->mtx);
-		netif_tx_lock_bh(ctx->netdev);
-		usbnet_start_xmit(NULL, ctx->netdev);
-		netif_tx_unlock_bh(ctx->netdev);
-	}
+	int err;
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	
+	err = cdc_ncm_fill_tx_frame(ctx, skb);
+	return err;
 }
 
 static struct sk_buff *
 cdc_ncm_tx_fixup(struct usbnet *dev, struct sk_buff *skb, gfp_t flags)
 {
-	struct sk_buff *skb_out;
 	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
 
-	/*
-	 * The Ethernet API we are using does not support transmitting
-	 * multiple Ethernet frames in a single call. This driver will
-	 * accumulate multiple Ethernet frames and send out a larger
-	 * USB frame when the USB buffer is full or when a single jiffies
-	 * timeout happens.
-	 */
 	if (ctx == NULL)
 		goto error;
 
-	spin_lock_bh(&ctx->mtx);
-	skb_out = cdc_ncm_fill_tx_frame(ctx, skb);
-	spin_unlock_bh(&ctx->mtx);
-	return skb_out;
+	return ctx->tx_curr_skb;
 
 error:
 	if (skb != NULL)
@@ -1197,6 +1129,7 @@ static const struct driver_info cdc_ncm_info = {
 	.manage_power = cdc_ncm_manage_power,
 	.status = cdc_ncm_status,
 	.rx_fixup = cdc_ncm_rx_fixup,
+	.tx_bundle = cdc_ncm_tx_bundle,
 	.tx_fixup = cdc_ncm_tx_fixup,
 };
 
@@ -1211,6 +1144,7 @@ static const struct driver_info wwan_info = {
 	.manage_power = cdc_ncm_manage_power,
 	.status = cdc_ncm_status,
 	.rx_fixup = cdc_ncm_rx_fixup,
+	.tx_bundle = cdc_ncm_tx_bundle,
 	.tx_fixup = cdc_ncm_tx_fixup,
 };
 
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 8531c1c..67b6ea0 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -86,6 +86,9 @@ static u8	node_id [ETH_ALEN];
 
 static const char driver_name [] = "usbnet";
 
+static void tx_complete(struct urb *urb);
+static int submit_next_bundle(struct usbnet *dev, struct sk_buff *skb);
+
 /* use ethtool to change the level for any given device */
 static int msg_level = -1;
 module_param (msg_level, int, 0);
@@ -923,7 +926,9 @@ kevent (struct work_struct *work)
 {
 	struct usbnet		*dev =
 		container_of(work, struct usbnet, kevent);
+	struct driver_info	*info = dev->driver_info;
 	int			status;
+	struct sk_buff		*skb;
 
 	/* usb_clear_halt() needs a thread context */
 	if (test_bit (EVENT_TX_HALT, &dev->flags)) {
@@ -942,8 +947,13 @@ fail_pipe:
 					   status);
 		} else {
 			clear_bit (EVENT_TX_HALT, &dev->flags);
-			if (status != -ESHUTDOWN)
+			if (status != -ESHUTDOWN) {
+				/* queue halted, no race */
+				skb = info->tx_fixup(dev, NULL, GFP_KERNEL);
+				if (skb)
+					submit_next_bundle(dev, skb);
 				netif_wake_queue (dev->net);
+			}
 		}
 	}
 	if (test_bit (EVENT_RX_HALT, &dev->flags)) {
@@ -1008,6 +1018,11 @@ skip_reset:
 				    dev->udev->devpath,
 				    info->description);
 		} else {
+			spin_lock_irq(&dev->txlock);
+			skb = info->tx_fixup(dev, NULL, GFP_ATOMIC);
+			if (skb)
+				submit_next_bundle(dev, skb);
+			spin_unlock_irq(&dev->txlock);
 			usb_autopm_put_interface(dev->intf);
 		}
 	}
@@ -1016,17 +1031,78 @@ skip_reset:
 		netdev_dbg(dev->net, "kevent done, flags = 0x%lx\n", dev->flags);
 }
 
+static int submit_next_bundle(struct usbnet *dev, struct sk_buff *skb)
+{
+	struct skb_data *entry;
+	int length = skb->len;
+	int retval;
+	struct driver_info *info = dev->driver_info;
+	struct urb *urb;
+
+	urb = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!urb)
+		return -ENOMEM;
+
+	entry = (struct skb_data *) skb->cb;
+	entry->urb = urb;
+	entry->dev = dev;
+	entry->length = length;
+
+	usb_fill_bulk_urb (urb, dev->udev, dev->out,
+			skb->data, skb->len, tx_complete, skb);
+
+	/* don't assume the hardware handles USB_ZERO_PACKET
+	 * NOTE:  strictly conforming cdc-ether devices should expect
+	 * the ZLP here, but ignore the one-byte packet.
+	 * NOTE2: CDC NCM specification is different from CDC ECM when
+	 * handling ZLP/short packets, so cdc_ncm driver will make short
+	 * packet itself if needed.
+	 */
+	if (length % dev->maxpacket == 0) {
+		if (!(info->flags & FLAG_SEND_ZLP)) {
+			if (!(info->flags & FLAG_MULTI_PACKET)) {
+				urb->transfer_buffer_length++;
+				if (skb_tailroom(skb)) {
+					skb->data[skb->len] = 0;
+					__skb_put(skb, 1);
+				}
+			}
+		} else
+			urb->transfer_flags |= URB_ZERO_PACKET;
+	}
+
+	atomic_inc(&dev->tx_in_flight);
+	retval = usb_submit_urb(urb, GFP_ATOMIC);
+	if (retval < 0)
+		atomic_dec(&dev->tx_in_flight);
+	//FIXME: full error handling
+
+	return retval;
+}
 /*-------------------------------------------------------------------------*/
 
 static void tx_complete (struct urb *urb)
 {
 	struct sk_buff		*skb = (struct sk_buff *) urb->context;
+	struct sk_buff		*skb2;
 	struct skb_data		*entry = (struct skb_data *) skb->cb;
 	struct usbnet		*dev = entry->dev;
+	struct driver_info	*info = dev->driver_info;
+	int			in_flight;
 
+	in_flight = atomic_dec_return(&dev->tx_in_flight);
 	if (urb->status == 0) {
-		if (!(dev->driver_info->flags & FLAG_MULTI_PACKET))
+		if (!(dev->driver_info->flags & FLAG_MULTI_PACKET)) {
 			dev->net->stats.tx_packets++;
+		} else {
+			if (in_flight < 2) {
+				spin_lock(&dev->txlock);
+				skb2 = info->tx_fixup(dev, NULL, GFP_ATOMIC);
+				if (skb2)
+					submit_next_bundle(dev, skb2);
+				spin_unlock(&dev->txlock);
+			}
+		}
 		dev->net->stats.tx_bytes += entry->length;
 	} else {
 		dev->net->stats.tx_errors++;
@@ -1089,23 +1165,51 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
 	struct urb		*urb = NULL;
 	struct skb_data		*entry;
 	struct driver_info	*info = dev->driver_info;
+	struct sk_buff		*skb_old = NULL;
 	unsigned long		flags;
 	int retval;
+	int transmit_now = 1;
+	int bundle_again = 0;
 
 	if (skb)
 		skb_tx_timestamp(skb);
 
+	/*
+	 * first we allow drivers to bundle packets together
+	 * maintainance of the buffer is the responsibility
+	 * of the lower layer
+	 */
+	spin_lock(&dev->txlock);
+rebundle:
+	if (info->tx_bundle) {
+		bundle_again = 0;
+		retval = info->tx_bundle(dev, skb, GFP_ATOMIC);
+
+		switch (retval) {
+		case 0: /* the package has been bundled */
+			if (atomic_read(&dev->tx_in_flight) < 2)
+				transmit_now = 1;
+			else
+				transmit_now = 0;
+			break;
+		case -EAGAIN:
+			transmit_now = 1;
+			bundle_again = 1;
+			skb_old = skb;
+			break;
+		case -EBUSY:
+			transmit_now = 1;
+			break;
+		}
+	}
 	// some devices want funky USB-level framing, for
 	// win32 driver (usually) and/or hardware quirks
-	if (info->tx_fixup) {
+	if (transmit_now && info->tx_fixup) {
 		skb = info->tx_fixup (dev, skb, GFP_ATOMIC);
 		if (!skb) {
 			if (netif_msg_tx_err(dev)) {
 				netif_dbg(dev, tx_err, dev->net, "can't tx_fixup skb\n");
 				goto drop;
-			} else {
-				/* cdc_ncm collected packet; waits for more */
-				goto not_drop;
 			}
 		}
 	}
@@ -1164,14 +1268,17 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
 	}
 #endif
 
+	atomic_inc(&dev->tx_in_flight);
 	switch ((retval = usb_submit_urb (urb, GFP_ATOMIC))) {
 	case -EPIPE:
 		netif_stop_queue (net);
 		usbnet_defer_kevent (dev, EVENT_TX_HALT);
+		atomic_dec(&dev->tx_in_flight);
 		usb_autopm_put_interface_async(dev->intf);
 		break;
 	default:
 		usb_autopm_put_interface_async(dev->intf);
+		atomic_dec(&dev->tx_in_flight);
 		netif_dbg(dev, tx_err, dev->net,
 			  "tx: submit urb err %d\n", retval);
 		break;
@@ -1187,7 +1294,6 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
 		netif_dbg(dev, tx_err, dev->net, "drop, code %d\n", retval);
 drop:
 		dev->net->stats.tx_dropped++;
-not_drop:
 		if (skb)
 			dev_kfree_skb_any (skb);
 		usb_free_urb (urb);
@@ -1197,6 +1303,11 @@ not_drop:
 #ifdef CONFIG_PM
 deferred:
 #endif
+	if (bundle_again) {
+		skb = skb_old;
+		goto rebundle;
+	}
+	spin_unlock(&dev->txlock);
 	return NETDEV_TX_OK;
 }
 EXPORT_SYMBOL_GPL(usbnet_start_xmit);
@@ -1393,6 +1504,8 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
 	dev->delay.data = (unsigned long) dev;
 	init_timer (&dev->delay);
 	mutex_init (&dev->phy_mutex);
+	spin_lock_init(&dev->txlock);
+	atomic_set(&dev->tx_in_flight, 0); 
 
 	dev->net = net;
 	strcpy (net->name, "usb%d");
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index f87cf62..544ddd2 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -33,6 +33,8 @@ struct usbnet {
 	wait_queue_head_t	*wait;
 	struct mutex		phy_mutex;
 	unsigned char		suspend_count;
+	atomic_t		tx_in_flight;
+	spinlock_t		txlock;
 
 	/* i/o info: pipes etc */
 	unsigned		in, out;
@@ -133,6 +135,12 @@ struct driver_info {
 	/* fixup rx packet (strip framing) */
 	int	(*rx_fixup)(struct usbnet *dev, struct sk_buff *skb);
 
+	/* bundle individual package for transmission as
+	 * larger package. This cannot sleep
+	 */
+	int	(*tx_bundle)(struct usbnet *dev,
+				struct sk_buff *skb, gfp_t flags);
+
 	/* fixup tx packet (add framing) */
 	struct sk_buff	*(*tx_fixup)(struct usbnet *dev,
 				struct sk_buff *skb, gfp_t flags);

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH] stmmac: fix return value check in stmmac_open_ext_timer()
From: Giuseppe CAVALLARO @ 2012-09-21  8:21 UTC (permalink / raw)
  To: Wei Yongjun; +Cc: yongjun_wei, netdev
In-Reply-To: <CAPgLHd87SqVa40-SGrgEfRAPWj1dr9aGPDD7b3aALCqka2ssuA@mail.gmail.com>

On 9/21/2012 9:06 AM, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> In case of error, the function clk_get() returns ERR_PTR()
> and never returns NULL pointer. The NULL test in the error
> handling should be replaced with IS_ERR().
>
> dpatch engine is used to auto generated this patch.
> (https://github.com/weiyj/dpatch)

FYI I have recently sent some patches (still under reviewing) to 
completely remove this dead code from the driver.

http://marc.info/?l=linux-netdev&m=134734656020277&w=2

Peppe

> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
>   drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c
> index 2a0e1ab..197fb8c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c
> @@ -109,7 +109,7 @@ int stmmac_open_ext_timer(struct net_device *dev, struct stmmac_timer *tm)
>   {
>   	timer_clock = clk_get(NULL, TMU_CHANNEL);
>
> -	if (timer_clock == NULL)
> +	if (IS_ERR(timer_clock))
>   		return -1;
>
>   	if (tmu2_register_user(stmmac_timer_handler, (void *)dev) < 0) {
>
>
>

^ permalink raw reply

* Re: [PATCH] can: mscan-mpc5xxx: fix return value check in mpc512x_can_get_clock()
From: Marc Kleine-Budde @ 2012-09-21  8:09 UTC (permalink / raw)
  To: Wei Yongjun
  Cc: wg, grant.likely, rob.herring, yongjun_wei, linux-can, netdev,
	devicetree-discuss
In-Reply-To: <CAPgLHd9fRAvQ77rD_PCp45XW9-FsBisUATOT00Je39nbv5o28A@mail.gmail.com>

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

On 09/21/2012 09:09 AM, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> In case of error, the function clk_get() returns ERR_PTR()
> and never returns NULL pointer. The NULL test in the error
> handling should be replaced with IS_ERR().
> 
> dpatch engine is used to auto generated this patch.
> (https://github.com/weiyj/dpatch)
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Applied to can-next

Tnx,
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: 259 bytes --]

^ permalink raw reply

* Re: HTB vs CoDel performance
From: Lin Ming @ 2012-09-21  7:23 UTC (permalink / raw)
  To: Rick Jones; +Cc: Eric Dumazet, networking
In-Reply-To: <5059FA30.8070209@hp.com>

On Thu, Sep 20, 2012 at 1:00 AM, Rick Jones <rick.jones2@hp.com> wrote:
> On 09/18/2012 06:26 PM, Lin Ming wrote:
>>
>> On Tue, Sep 18, 2012 at 6:15 PM, Eric Dumazet <eric.dumazet@gmail.com>
>> wrote:
>>>
>>> Are you really cpu limited ? You might hit some clocks artifacts.
>>
>>
>> Did you mean the cpu speed? It's an ARMv5 processor.
>> BogoMIPS        : 1196.03
>
>
> At the risk of typing into Eric's keyboard, he was asking if you were
> saturating the CPU - was it getting to 100% utilization, that sort of thing.

Yes, ~98% utilization.

>
>
>>> rate limiting to 1Gbps probably need high resolution timers.
>>
>>
>> High resolution timer is enabled.
>
>
> When you are running your tests, what sort of CPU utilization do you see on
> the CPU of your router with HTB on vs off.  Some "quick and dirty" netperf

HTB off: ~85%
HTB on: ~98%

> tests on an Centrino-based laptop suggested that as an end system at least,
> HTB at 1 GbE (using your tc commands) increases service demand (what netperf
> calculates as CPU consumed per unit of work performed) ~15% for bulk
> transfer (netperf TCP_STREAM) and ~18% for small packet request/response
> (TCP_RR).
>
> rick jones

^ permalink raw reply

* Re: [PATCH] can: mscan-mpc5xxx: fix return value check in mpc512x_can_get_clock()
From: Wolfgang Grandegger @ 2012-09-21  7:14 UTC (permalink / raw)
  To: Wei Yongjun
  Cc: mkl, grant.likely, rob.herring, yongjun_wei, linux-can, netdev,
	devicetree-discuss
In-Reply-To: <CAPgLHd9fRAvQ77rD_PCp45XW9-FsBisUATOT00Je39nbv5o28A@mail.gmail.com>

On 09/21/2012 09:09 AM, Wei Yongjun wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> In case of error, the function clk_get() returns ERR_PTR()
> and never returns NULL pointer. The NULL test in the error
> handling should be replaced with IS_ERR().
> 
> dpatch engine is used to auto generated this patch.
> (https://github.com/weiyj/dpatch)
> 
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Acked-by: Wolfgang Grandegger <wg@grandegger.com>

Thanks,

Wolfgang.



^ permalink raw reply

* [PATCH] net/irda: sh_sir: fix return value check in sh_sir_set_baudrate()
From: Wei Yongjun @ 2012-09-21  7:13 UTC (permalink / raw)
  To: samuel; +Cc: yongjun_wei, netdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

In case of error, the function clk_get() returns ERR_PTR()
and never returns NULL pointer. The NULL test in the error
handling should be replaced with IS_ERR().

dpatch engine is used to auto generated this patch.
(https://github.com/weiyj/dpatch)

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/net/irda/sh_sir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/irda/sh_sir.c b/drivers/net/irda/sh_sir.c
index 256eddf..7951094 100644
--- a/drivers/net/irda/sh_sir.c
+++ b/drivers/net/irda/sh_sir.c
@@ -280,7 +280,7 @@ static int sh_sir_set_baudrate(struct sh_sir_self *self, u32 baudrate)
 	}
 
 	clk = clk_get(NULL, "irda_clk");
-	if (!clk) {
+	if (IS_ERR(clk)) {
 		dev_err(dev, "can not get irda_clk\n");
 		return -EIO;
 	}

^ permalink raw reply related

* [PATCH] can: mscan-mpc5xxx: fix return value check in mpc512x_can_get_clock()
From: Wei Yongjun @ 2012-09-21  7:09 UTC (permalink / raw)
  To: wg, mkl, grant.likely, rob.herring
  Cc: yongjun_wei, linux-can, netdev, devicetree-discuss

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

In case of error, the function clk_get() returns ERR_PTR()
and never returns NULL pointer. The NULL test in the error
handling should be replaced with IS_ERR().

dpatch engine is used to auto generated this patch.
(https://github.com/weiyj/dpatch)

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/net/can/mscan/mpc5xxx_can.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
index 06adf88..524ef96 100644
--- a/drivers/net/can/mscan/mpc5xxx_can.c
+++ b/drivers/net/can/mscan/mpc5xxx_can.c
@@ -181,7 +181,7 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 
 		if (!clock_name || !strcmp(clock_name, "sys")) {
 			sys_clk = clk_get(&ofdev->dev, "sys_clk");
-			if (!sys_clk) {
+			if (IS_ERR(sys_clk)) {
 				dev_err(&ofdev->dev, "couldn't get sys_clk\n");
 				goto exit_unmap;
 			}
@@ -204,7 +204,7 @@ static u32 __devinit mpc512x_can_get_clock(struct platform_device *ofdev,
 
 		if (clocksrc < 0) {
 			ref_clk = clk_get(&ofdev->dev, "ref_clk");
-			if (!ref_clk) {
+			if (IS_ERR(ref_clk)) {
 				dev_err(&ofdev->dev, "couldn't get ref_clk\n");
 				goto exit_unmap;
 			}



^ permalink raw reply related

* [PATCH] stmmac: fix return value check in stmmac_open_ext_timer()
From: Wei Yongjun @ 2012-09-21  7:06 UTC (permalink / raw)
  To: peppe.cavallaro; +Cc: yongjun_wei, netdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

In case of error, the function clk_get() returns ERR_PTR()
and never returns NULL pointer. The NULL test in the error
handling should be replaced with IS_ERR().

dpatch engine is used to auto generated this patch.
(https://github.com/weiyj/dpatch)

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c
index 2a0e1ab..197fb8c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_timer.c
@@ -109,7 +109,7 @@ int stmmac_open_ext_timer(struct net_device *dev, struct stmmac_timer *tm)
 {
 	timer_clock = clk_get(NULL, TMU_CHANNEL);
 
-	if (timer_clock == NULL)
+	if (IS_ERR(timer_clock))
 		return -1;
 
 	if (tmu2_register_user(stmmac_timer_handler, (void *)dev) < 0) {

^ permalink raw reply related


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