Netdev List
 help / color / mirror / Atom feed
* Re: [RFC] IFLA_PORT_* iproute2 cmd line
From: Chris Wright @ 2010-05-26 15:56 UTC (permalink / raw)
  To: Stefan Berger
  Cc: Arnd Bergmann, Chris Wright, Dirk Herrendoerfer, netdev,
	Scott Feldman, Stephen Hemminger, Vivek Kashyap
In-Reply-To: <OFCF88A167.122DD206-ON8525772F.00470999-8525772F.0047F4A5@us.ibm.com>

* Stefan Berger (stefanb@us.ibm.com) wrote:
> When I have libvirt talk to my dummy server, I pass the getpid() of
> libvirt into the request message, just to fulfill the requirement of it
> being != 0 so the kernel driver doesn't process the msg. I also don't
> want to have to determine the pid of the target process. Now I believe
> the problem with that *may* be that multiple daemons may want to process
> the message,

What other daemon do you expect to be listening besides lldpad?

thanks,
-chris

^ permalink raw reply

* [PATCH 9/17] net/iucv: Add missing spin_unlock
From: Julia Lawall @ 2010-05-26 15:56 UTC (permalink / raw)
  To: Ursula Braun, linux390, David S. , Miller, linux-s390, netdev,
	linux-kernel

From: Julia Lawall <julia@diku.dk>

Add a spin_unlock missing on the error path.  There seems like no reason
why the lock should continue to be held if the kzalloc fail.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression E1;
@@

* spin_lock(E1,...);
  <+... when != E1
  if (...) {
    ... when != E1
*   return ...;
  }
  ...+>
* spin_unlock(E1,...);
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 net/iucv/af_iucv.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index c8b4599..9637e45 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -1619,7 +1619,7 @@ static void iucv_callback_rx(struct iucv_path *path, struct iucv_message *msg)
 save_message:
 	save_msg = kzalloc(sizeof(struct sock_msg_q), GFP_ATOMIC | GFP_DMA);
 	if (!save_msg)
-		return;
+		goto out_unlock;
 	save_msg->path = path;
 	save_msg->msg = *msg;
 

^ permalink raw reply related

* [PATCH 8/17] net/caif: Add missing spin_unlock
From: Julia Lawall @ 2010-05-26 15:56 UTC (permalink / raw)
  To: David S. Miller, netdev, linux-kernel, kernel-janitors

From: Julia Lawall <julia@diku.dk>

Add a spin_unlock missing on the error path.  The spin lock is used in a
balanced way elsewhere in the file.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression E1;
@@

* spin_lock(E1,...);
  <+... when != E1
  if (...) {
    ... when != E1
*   return ...;
  }
  ...+>
* spin_unlock(E1,...);
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 net/caif/cfmuxl.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/caif/cfmuxl.c b/net/caif/cfmuxl.c
index 7372f27..80c8d33 100644
--- a/net/caif/cfmuxl.c
+++ b/net/caif/cfmuxl.c
@@ -174,10 +174,11 @@ struct cflayer *cfmuxl_remove_uplayer(struct cflayer *layr, u8 id)
 	spin_lock(&muxl->receive_lock);
 	up = get_up(muxl, id);
 	if (up == NULL)
-		return NULL;
+		goto out;
 	memset(muxl->up_cache, 0, sizeof(muxl->up_cache));
 	list_del(&up->node);
 	cfsrvl_put(up);
+out:
 	spin_unlock(&muxl->receive_lock);
 	return up;
 }

^ permalink raw reply related

* [PATCH 4/17] drivers/isdn/hardware/mISDN: Add missing spin_unlock
From: Julia Lawall @ 2010-05-26 15:55 UTC (permalink / raw)
  To: Karsten Keil, netdev, linux-kernel, kernel-janitors

From: Julia Lawall <julia@diku.dk>

Add a spin_unlock missing on the error path.  The return value of write_reg
seems to be completely ignored, so it seems that the lock should be
released in every case.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression E1;
@@

* spin_lock(E1,...);
  <+... when != E1
  if (...) {
    ... when != E1
*   return ...;
  }
  ...+>
* spin_unlock(E1,...);
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

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

diff --git a/drivers/isdn/hardware/mISDN/hfcsusb.c b/drivers/isdn/hardware/mISDN/hfcsusb.c
index b3b7e28..8700474 100644
--- a/drivers/isdn/hardware/mISDN/hfcsusb.c
+++ b/drivers/isdn/hardware/mISDN/hfcsusb.c
@@ -97,8 +97,10 @@ static int write_reg(struct hfcsusb *hw, __u8 reg, __u8 val)
 			hw->name, __func__, reg, val);
 
 	spin_lock(&hw->ctrl_lock);
-	if (hw->ctrl_cnt >= HFC_CTRL_BUFSIZE)
+	if (hw->ctrl_cnt >= HFC_CTRL_BUFSIZE) {
+		spin_unlock(&hw->ctrl_lock);
 		return 1;
+	}
 	buf = &hw->ctrl_buff[hw->ctrl_in_idx];
 	buf->hfcs_reg = reg;
 	buf->reg_val = val;

^ permalink raw reply related

* [PATCH 1/17] net/rds: Add missing mutex_unlock
From: Julia Lawall @ 2010-05-26 15:54 UTC (permalink / raw)
  To: Andy Grover, David S. Miller, rds-devel, netdev, linux-kernel,
	kernel-janitors

From: Julia Lawall <julia@diku.dk>

Add a mutex_unlock missing on the error path.  In each case, whenever the
label out is reached from elsewhere in the function, mutex is not locked.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@@
expression E1;
@@

* mutex_lock(E1);
  <+... when != E1
  if (...) {
    ... when != E1
*   return ...;
  }
  ...+>
* mutex_unlock(E1);
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---
 net/rds/ib_cm.c |    1 +
 net/rds/iw_cm.c |    1 +
 2 files changed, 2 insertions(+)

diff --git a/net/rds/ib_cm.c b/net/rds/ib_cm.c
index 10ed0d5..f688327 100644
--- a/net/rds/ib_cm.c
+++ b/net/rds/ib_cm.c
@@ -475,6 +475,7 @@ int rds_ib_cm_handle_connect(struct rdma_cm_id *cm_id,
 	err = rds_ib_setup_qp(conn);
 	if (err) {
 		rds_ib_conn_error(conn, "rds_ib_setup_qp failed (%d)\n", err);
+		mutex_unlock(&conn->c_cm_lock);
 		goto out;
 	}
 
diff --git a/net/rds/iw_cm.c b/net/rds/iw_cm.c
index a9d951b..b5dd6ac 100644
--- a/net/rds/iw_cm.c
+++ b/net/rds/iw_cm.c
@@ -452,6 +452,7 @@ int rds_iw_cm_handle_connect(struct rdma_cm_id *cm_id,
 	err = rds_iw_setup_qp(conn);
 	if (err) {
 		rds_iw_conn_error(conn, "rds_iw_setup_qp failed (%d)\n", err);
+		mutex_unlock(&conn->c_cm_lock);
 		goto out;
 	}
 

^ permalink raw reply related

* Re: [PATCH] net/core: use net_device dev_id to indicate port number
From: Stephen Hemminger @ 2010-05-26 15:33 UTC (permalink / raw)
  To: Eli Cohen; +Cc: Eli Cohen, davem, netdev, linux-rdma, rdreier, yevgenyp
In-Reply-To: <20100526152730.GA10890@mtldesk030.lab.mtl.com>

On Wed, 26 May 2010 18:27:30 +0300
Eli Cohen <eli@dev.mellanox.co.il> wrote:

> On Wed, May 26, 2010 at 08:23:06AM -0700, Stephen Hemminger wrote:
> > On Wed, 26 May 2010 12:52:00 +0300
> > Eli Cohen <eli@mellanox.co.il> wrote:
> > 
> > > Today, there are no means to know which port of a hardware device a netdev
> > > interface uses. struct net_device conatins a field, dev_id, that can be used
> > > for that. This patch adds a new macro, SET_NETDEV_DEV_ID(), to provide a
> > > standard way to set the value of this field.
> > > Also also make use of this feature in the mlx4_en driver to set the port
> > > number; port numbers are zero based.
> > 
> > Why is a macro wrapper needed?
> > 
> 
> I guess for the same reason we use SET_NETDEV_DEV - to provide a
> consistent interface for setting this value...

SET_NETDEV_DEV macro exists because at the time 2.5 kernel was being developed
it was important to be able to maintain source compatibility between 2.4 and
2.6 (nee 2.5) drivers. Since 2.4 did not have sysfs, the macro was a mechanism
to allow the same code to run on both kernel versions.

Your situation is different, just use dev_id and update documentation if
you need to.

-- 

^ permalink raw reply

* Re: [PATCH] net/core: use net_device dev_id to indicate port number
From: Eli Cohen @ 2010-05-26 15:27 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Eli Cohen, davem, netdev, linux-rdma, rdreier, yevgenyp
In-Reply-To: <20100526082306.25959fb6@nehalam>

On Wed, May 26, 2010 at 08:23:06AM -0700, Stephen Hemminger wrote:
> On Wed, 26 May 2010 12:52:00 +0300
> Eli Cohen <eli@mellanox.co.il> wrote:
> 
> > Today, there are no means to know which port of a hardware device a netdev
> > interface uses. struct net_device conatins a field, dev_id, that can be used
> > for that. This patch adds a new macro, SET_NETDEV_DEV_ID(), to provide a
> > standard way to set the value of this field.
> > Also also make use of this feature in the mlx4_en driver to set the port
> > number; port numbers are zero based.
> 
> Why is a macro wrapper needed?
> 

I guess for the same reason we use SET_NETDEV_DEV - to provide a
consistent interface for setting this value...

^ permalink raw reply

* Re: [PATCH] net/core: use net_device dev_id to indicate port number
From: Stephen Hemminger @ 2010-05-26 15:23 UTC (permalink / raw)
  To: Eli Cohen
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, rdreier-FYB4Gu1CFyUAvxtiuMwx3w,
	yevgenyp-VPRAkNaXOzVS1MOuV/RT9w
In-Reply-To: <20100526095200.GA7370-8YAHvHwT2UEvbXDkjdHOrw/a8Rv0c6iv@public.gmane.org>

On Wed, 26 May 2010 12:52:00 +0300
Eli Cohen <eli-VPRAkNaXOzVS1MOuV/RT9w@public.gmane.org> wrote:

> Today, there are no means to know which port of a hardware device a netdev
> interface uses. struct net_device conatins a field, dev_id, that can be used
> for that. This patch adds a new macro, SET_NETDEV_DEV_ID(), to provide a
> standard way to set the value of this field.
> Also also make use of this feature in the mlx4_en driver to set the port
> number; port numbers are zero based.

Why is a macro wrapper needed?

-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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

* Re: ipv6: Add GSO support on forwarding path
From: Ralf Baechle @ 2010-05-26 15:20 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David S. Miller, netdev
In-Reply-To: <20100526102758.GA25914@gondor.apana.org.au>

On Wed, May 26, 2010 at 08:27:58PM +1000, Herbert Xu wrote:

> ipv6: Add GSO support on forwarding path
> 
> Currently we disallow GSO packets on the IPv6 forward path.
> This patch fixes this.
> 
> Note that I discovered that our existing GSO MTU checks (e.g.,
> IPv4 forwarding) are buggy in that they skip the check altogether, 
> hen they really should be checking gso_size instead.
> 
> I have also been lazy here in that I haven't bothered to segment
> the GSO packet by hand before generating an ICMP message.  Someone
> should add that to be 100% correct.
> 
> Reported-by: Ralf Baechle <ralf@linux-mips.org>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

I tested this on top of a 2.6.34 release kernel and asked the user
experiencing the problem to re-test and the problem still persists.

Unlike what I told you earlier on IRC my tester can trigger the issue
by going to any page on linux-ax25.org from any XP, Vista or Windows 7
client.

I've got three tcpdumps of the hang occuring on ftp.linux-ax25.org

  /pub/dl1bff.log
  /pub/dl1bff-2.log
  /pub/dl1bff-3.log

The first two were taken with a 2.6.32 Fedora 12 kernel; the 3rd with
a stock 2.6.34 kernel and your patch applied on top.  The issue exists
for quite a while; it has first been noticed with Fedora 11.

  Ralf

^ permalink raw reply

* [patch] caif: unlock on error path in cfserl_receive()
From: Dan Carpenter @ 2010-05-26 15:16 UTC (permalink / raw)
  To: Sjur Braendeland; +Cc: David S. Miller, netdev, kernel-janitors

There was an spin_unlock missing on the error path.  The spin_lock was
tucked in with the declarations so it was hard to spot.  I added a new
line.

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/net/caif/cfserl.c b/net/caif/cfserl.c
index cb4325a..965c5ba 100644
--- a/net/caif/cfserl.c
+++ b/net/caif/cfserl.c
@@ -59,16 +59,18 @@ static int cfserl_receive(struct cflayer *l, struct cfpkt *newpkt)
 	u8 stx = CFSERL_STX;
 	int ret;
 	u16 expectlen = 0;
+
 	caif_assert(newpkt != NULL);
 	spin_lock(&layr->sync);
 
 	if (layr->incomplete_frm != NULL) {
-
 		layr->incomplete_frm =
 		    cfpkt_append(layr->incomplete_frm, newpkt, expectlen);
 		pkt = layr->incomplete_frm;
-		if (pkt == NULL)
+		if (pkt == NULL) {
+			spin_unlock(&layr->sync);
 			return -ENOMEM;
+		}
 	} else {
 		pkt = newpkt;
 	}

^ permalink raw reply related

* [patch 1/2] be2net: add unlock on error path
From: Dan Carpenter @ 2010-05-26 14:46 UTC (permalink / raw)
  To: Sathya Perla
  Cc: Subbu Seetharaman, Sarveshwar Bandi, Ajit Khaparde,
	David S. Miller, netdev, kernel-janitors

The unlock accidentally got removed from the error path in dd131e76e5:
"be2net: Bug fix to avoid disabling bottom half during firmware upgrade."

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
index c911bfb..18d5789 100644
--- a/drivers/net/benet/be_cmds.c
+++ b/drivers/net/benet/be_cmds.c
@@ -1429,7 +1429,7 @@ int be_cmd_write_flashrom(struct be_adapter *adapter, struct be_dma_mem *cmd,
 	wrb = wrb_from_mccq(adapter);
 	if (!wrb) {
 		status = -EBUSY;
-		goto err;
+		goto err_unlock;
 	}
 	req = cmd->va;
 	sge = nonembedded_sgl(wrb);
@@ -1457,7 +1457,10 @@ int be_cmd_write_flashrom(struct be_adapter *adapter, struct be_dma_mem *cmd,
 	else
 		status = adapter->flash_status;
 
-err:
+	return status;
+
+err_unlock:
+	spin_unlock_bh(&adapter->mcc_lock);
 	return status;
 }
 

^ permalink raw reply related

* [patch 2/2] be2net: remove superfluous externs
From: Dan Carpenter @ 2010-05-26 14:47 UTC (permalink / raw)
  To: Sathya Perla
  Cc: Subbu Seetharaman, Sarveshwar Bandi, Ajit Khaparde,
	David S. Miller, netdev, kernel-janitors

This fixes some sparse warnings:
drivers/net/benet/be_cmds.c:1503:12: warning: function
	'be_cmd_enable_magic_wol' with external linkage has definition
drivers/net/benet/be_cmds.c:1668:12: warning: function
	'be_cmd_get_seeprom_data' with external linkage has definition

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/drivers/net/benet/be_cmds.c b/drivers/net/benet/be_cmds.c
index c911bfb..f9c0d4d 100644
--- a/drivers/net/benet/be_cmds.c
+++ b/drivers/net/benet/be_cmds.c
@@ -1497,7 +1500,7 @@ err:
 	return status;
 }
 
-extern int be_cmd_enable_magic_wol(struct be_adapter *adapter, u8 *mac,
+int be_cmd_enable_magic_wol(struct be_adapter *adapter, u8 *mac,
 				struct be_dma_mem *nonemb_cmd)
 {
 	struct be_mcc_wrb *wrb;
@@ -1662,7 +1665,7 @@ err:
 	return status;
 }
 
-extern int be_cmd_get_seeprom_data(struct be_adapter *adapter,
+int be_cmd_get_seeprom_data(struct be_adapter *adapter,
 				struct be_dma_mem *nonemb_cmd)
 {
 	struct be_mcc_wrb *wrb;

^ permalink raw reply related

* Re: [RFC] IFLA_PORT_* iproute2 cmd line
From: Scott Feldman @ 2010-05-26 14:49 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: netdev, Chris Wright, Stephen Hemminger, Stefan Berger,
	Dirk Herrendoerfer, Vivek Kashyap
In-Reply-To: <201005261438.07004.arnd@arndb.de>

On 5/26/10 5:38 AM, "Arnd Bergmann" <arnd@arndb.de> wrote:

> On Wednesday 26 May 2010, Scott Feldman wrote:
>> I need to provide an iproute2 patch for IFLA_PORT_* and I wanted to hash out
>> the cmd line before I submit it.  Here's what I think would work based on
>> previous input from Arnd:
>> 
>> Usage:  ip port associate DEVICE [ vf NUM ] {PROFILE|VSI}
>>         ip port pre-associate DEVICE [ vf NUM ] VSI
>>         ip port pre-associate-rr DEVICE [ vf NUM ] VSI
>>         ip port dis-associate DEVICE [ vf NUM ]
>>         ip port show [ DEVICE [ vf NUM ] ]
>> 
>>         PROFILE := port-profile PORT-PROFILE
>>                    [ instance-uuid INSTANCE-UUID ]
>>                    [ host-uuid HOST-UUID ]
>> 
>>         VSI := vsi managerid MGR typeid VTID typeidversion VER
>>                [ instance-uuid INSTANCE-UUID ]
>> 
>> Comments?
> 
> The syntax of the PROFILE and VSI arguments seems ok, but I'm
> not sure where exactly to put them.
> 
> When talking to the kernel, I think this should be part of
> link command, because that is the underlying protocol:
> 
> ip link set DEVICE [vf NUM] {associate {PROFILE|VSI}    |
>     pre-associate-rr VSI       |
>     pre-associate VSI          |
>     disassociate }
> ip link show [ DEVICE [ vf NUM ] ]
> 
> This will also let you combine the association with additional
> "vf mac" and "vf vlan" commands as needed.

How does this strike you?

  Usage: ip link add link DEV [ name ] NAME
                     [ txqueuelen PACKETS ]
                     [ address LLADDR ]
                     [ broadcast LLADDR ]
                     [ mtu MTU ]
                     type TYPE [ ARGS ]
         ip link delete DEV type TYPE [ ARGS ]

         ip link set DEVICE [ { up | down } ]
                            [ arp { on | off } ]
                            [ dynamic { on | off } ]
                            [ multicast { on | off } ]
                            [ allmulticast { on | off } ]
                            [ promisc { on | off } ]
                            [ trailers { on | off } ]
                            [ txqueuelen PACKETS ]
                            [ name NEWNAME ]
                            [ address LLADDR ]
                            [ broadcast LLADDR ]
                            [ mtu MTU ]
                            [ netns PID ]
                            [ alias NAME ]
+                           [ virtualport MODE {PROFILE|VSI} ]
                            [ vf NUM [ mac LLADDR ]
                                     [ vlan VLANID [ qos VLAN-QOS ] ]
-                                    [ rate TXRATE ] ]
+                                    [ rate TXRATE ]
+                                    [ virtualport MODE {PROFILE|VSI} ] ]
         ip link show [ DEVICE ]

  TYPE := { vlan | veth | vcan | dummy | ifb | macvlan | can }
+
+ MODE := { associate | preassociate | preassociaterr | disassociate }
+
+ PROFILE := port-profile PORT-PROFILE
+            [ instance-uuid INSTANCE-UUID ]
+            [ host-uuid HOST-UUID ]
+ 
+ VSI := vsi managerid MGR typeid VTID typeidversion VER
+        [ instance-uuid INSTANCE-UUID ]


> The more interesting question is how to do this when we
> talk to lldpad. One idea was to use the same protocol
> but to direct the message to a specific pid (that of lldpad).
> That would require adding an option like '-p PID' to ip
> that lets us change who we talk to.

Let me get the user-to-kernel part working to establish the cmd line and you
can follow up with alternative addressing schemes.
 
> Alternatively, we could also create a top-level command like
> the one you described, but just use it for the case when
> we're talking to lldpad, finding out the PID from the
> /var/run/lldpdad.pid internally. Right now, I'm leaning
> towards the more flexible option of being able to direct
> the command anywhere.


^ permalink raw reply

* [Patch] r8169: use u32 instead of unsigned long
From: Junchang Wang @ 2010-05-26 14:01 UTC (permalink / raw)
  To: romieu; +Cc: netdev

RTL_R32 should return value with 32-bit width. But "unsigned long"
implies u64 on some 64-bit platforms.

Signed-off-by: Junchang Wang <junchangwang@gmail.com>
---
 drivers/net/r8169.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 217e709..4234d6a 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -88,7 +88,7 @@ static const int multicast_filter_limit = 32;
 #define RTL_W32(reg, val32)	writel ((val32), ioaddr + (reg))
 #define RTL_R8(reg)		readb (ioaddr + (reg))
 #define RTL_R16(reg)		readw (ioaddr + (reg))
-#define RTL_R32(reg)		((unsigned long) readl (ioaddr + (reg)))
+#define RTL_R32(reg)		((u32) readl(ioaddr + (reg)))

 enum mac_version {
 	RTL_GIGA_MAC_NONE   = 0x00,
--


--Junchang

^ permalink raw reply related

* Re: [PATCH v3] can: Add driver for esd CAN-USB/2 device
From: Oliver Neukum @ 2010-05-26 13:47 UTC (permalink / raw)
  To: Matthias Fuchs; +Cc: netdev, Socketcan-core, linux-usb
In-Reply-To: <201005261114.03214.matthias.fuchs@esd.eu>

Am Mittwoch, 26. Mai 2010 11:14:03 schrieb Matthias Fuchs:
> +       netdev->trans_start = jiffies;
> +
> +       /* Slow down tx path */
> +       if (atomic_read(&priv->active_tx_jobs) >= MAX_TX_URBS)
> +               netif_stop_queue(netdev);

Where is the queue started again?

	Regards
		Oliver

^ permalink raw reply

* Re: [PATCH v3] can: Add driver for esd CAN-USB/2 device
From: Matthias Fuchs @ 2010-05-26 12:38 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	Socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4BFCF4AD.6030000-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

Hi Wolfgang,

oops. I did my tests againt latest net-next-2.6. But
I forgot to commit these missing changes. I attached 
a fixed version of patch v3. v4 will propably come when 
I finally checked Viral's comments.

Matthias

On Wednesday 26 May 2010 12:15, Wolfgang Grandegger wrote:
> Hi Matthias,
> 
> On 05/26/2010 11:14 AM, Matthias Fuchs wrote:
> > This patch adds a driver for esd's USB high speed
> > CAN interface. The driver supports devices with
> > multiple CAN interfaces.
> > 
> > Signed-off-by: Matthias Fuchs <matthias.fuchs-iOnpLzIbIdM@public.gmane.org>
> > ---
> > version 3: 
> >  - remove bus-error reporting feature because 
> >    it cannot be controlled by user on demand
> >    with current device's firmware
> >  - rebased against current net-next-2.6 tree
> > 
> > version 2: 
> >  - use bus-error reporting and counters
> >  - minor cleanup
> >  - rebased against current net-next-2.6 tree
> >  - initial post to linux-usb list for review
> 
> I get the following compiler warnings when compiling the kernel:
> 
>  CC      drivers/net/can/usb/esd_usb2.o
> drivers/net/can/usb/esd_usb2.c: In function 'esd_usb2_write_bulk_callback':
> drivers/net/can/usb/esd_usb2.c:466: error: implicit declaration of function 'usb_buffer_free'
> drivers/net/can/usb/esd_usb2.c: In function 'esd_usb2_setup_rx_urbs':
> drivers/net/can/usb/esd_usb2.c:562: error: implicit declaration of function 'usb_buffer_alloc'
> drivers/net/can/usb/esd_usb2.c:563: warning: assignment makes pointer from integer without a cast
> drivers/net/can/usb/esd_usb2.c: In function 'esd_usb2_start_xmit':
> drivers/net/can/usb/esd_usb2.c:732: warning: assignment makes pointer from integer without a cast
> make[4]: *** [drivers/net/can/usb/esd_usb2.o] Error 1
> 
> This is due to commit e26bcf37234c67624f62d9fc95f922b8dbda1363.
> You need a similar fix like for ems_usb.c. Are you using a recent
> version of the net-next-2.6 tree?
> 
> Wolfgang.
> 
> 

---
 drivers/net/can/usb/Kconfig    |    6 +
 drivers/net/can/usb/Makefile   |    1 +
 drivers/net/can/usb/esd_usb2.c | 1126 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1133 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/can/usb/esd_usb2.c

diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
index 97ff6fe..0452549 100644
--- a/drivers/net/can/usb/Kconfig
+++ b/drivers/net/can/usb/Kconfig
@@ -7,4 +7,10 @@ config CAN_EMS_USB
 	  This driver is for the one channel CPC-USB/ARM7 CAN/USB interface
 	  from EMS Dr. Thomas Wuensche (http://www.ems-wuensche.de).
 
+config CAN_ESD_USB2
+        tristate "ESD USB/2 CAN/USB interface"
+        ---help---
+          This driver supports the CAN-USB/2 interface
+          from esd electronic system design gmbh (http://www.esd.eu).
+
 endmenu
diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
index 0afd51d..fce3cf1 100644
--- a/drivers/net/can/usb/Makefile
+++ b/drivers/net/can/usb/Makefile
@@ -3,5 +3,6 @@
 #
 
 obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o
+obj-$(CONFIG_CAN_ESD_USB2) += esd_usb2.o
 
 ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
new file mode 100644
index 0000000..8f32638
--- /dev/null
+++ b/drivers/net/can/usb/esd_usb2.c
@@ -0,0 +1,1126 @@
+/*
+ * CAN driver for esd CAN-USB/2
+ *
+ * Copyright (C) 2010 Matthias Fuchs <matthias.fuchs-iOnpLzIbIdM@public.gmane.org>, esd gmbh
+ *
+ * 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 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+#include <linux/init.h>
+#include <linux/signal.h>
+#include <linux/slab.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>
+
+MODULE_AUTHOR("Matthias Fuchs <matthias.fuchs-iOnpLzIbIdM@public.gmane.org>");
+MODULE_DESCRIPTION("CAN driver for esd CAN-USB/2 interfaces");
+MODULE_LICENSE("GPL v2");
+
+/* Define these values to match your devices */
+#define USB_ESDGMBH_VENDOR_ID	0x0ab4
+#define USB_CANUSB2_PRODUCT_ID	0x0010
+
+#define ESD_USB2_CAN_CLOCK	60000000
+#define ESD_USB2_MAX_NETS	2
+
+/* USB2 commands */
+#define CMD_VERSION		1 /* also used for VERSION_REPLY */
+#define CMD_CAN_RX		2 /* device to host only */
+#define CMD_CAN_TX		3 /* also used for TX_DONE */
+#define CMD_SETBAUD		4 /* also used for SETBAUD_REPLY */
+#define CMD_TS			5 /* also used for TS_REPLY */
+#define CMD_IDADD		6 /* also used for IDADD_REPLY */
+
+/* esd CAN message flags - dlc field */
+#define ESD_RTR			0x10
+
+/* esd CAN message flags - id field */
+#define ESD_EXTID		0x20000000
+#define ESD_EVENT		0x40000000
+#define ESD_IDMASK		0x1fffffff
+
+/* esd CAN event ids used by this driver */
+#define ESD_EV_CAN_ERROR_EXT	2
+
+/* baudrate message flags */
+#define ESD_USB2_UBR		0x80000000
+#define ESD_USB2_LOM		0x40000000
+#define ESD_USB2_NO_BAUDRATE	0x7fffffff
+#define ESD_USB2_TSEG1_MIN	1
+#define ESD_USB2_TSEG1_MAX	16
+#define ESD_USB2_TSEG1_SHIFT	16
+#define ESD_USB2_TSEG2_MIN	1
+#define ESD_USB2_TSEG2_MAX	8
+#define ESD_USB2_TSEG2_SHIFT	20
+#define ESD_USB2_SJW_MAX	4
+#define ESD_USB2_SJW_SHIFT	14
+#define ESD_USB2_BRP_MIN	1
+#define ESD_USB2_BRP_MAX	1024
+#define ESD_USB2_BRP_INC	1
+#define ESD_USB2_3_SAMPLES	0x00800000
+
+/* esd IDADD message */
+#define ESD_ID_ENABLE		0x80
+#define ESD_MAX_ID_SEGMENT	64
+
+/* SJA1000 ECC register (emulated by usb2 firmware) */
+#define SJA1000_ECC_SEG		0x1F
+#define SJA1000_ECC_DIR		0x20
+#define SJA1000_ECC_ERR		0x06
+#define SJA1000_ECC_BIT		0x00
+#define SJA1000_ECC_FORM	0x40
+#define SJA1000_ECC_STUFF	0x80
+#define SJA1000_ECC_MASK	0xc0
+
+/* esd bus state event codes */
+#define ESD_BUSSTATE_MASK	0xc0
+#define ESD_BUSSTATE_WARN	0x40
+#define ESD_BUSSTATE_ERRPASSIVE	0x80
+#define ESD_BUSSTATE_BUSOFF	0xc0
+
+#define RX_BUFFER_SIZE		1024
+#define MAX_RX_URBS		4
+#define MAX_TX_URBS		16 /* must be power of 2 */
+
+struct header_msg {
+	u8 len; /* len is always the total message length in 32bit words */
+	u8 cmd;
+	u8 rsvd[2];
+};
+
+struct version_msg {
+	u8 len;
+	u8 cmd;
+	u8 rsvd;
+	u8 flags;
+	__le32 drv_version;
+};
+
+struct version_reply_msg {
+	u8 len;
+	u8 cmd;
+	u8 nets;
+	u8 features;
+	__le32 version;
+	u8 name[16];
+	__le32 rsvd;
+	__le32 ts;
+};
+
+struct rx_msg {
+	u8 len;
+	u8 cmd;
+	u8 net;
+	u8 dlc;
+	__le32 ts;
+	__le32 id; /* upper 3 bits contain flags */
+	u8 data[8];
+};
+
+struct tx_msg {
+	u8 len;
+	u8 cmd;
+	u8 net;
+	u8 dlc;
+	__le32 hnd;
+	__le32 id; /* upper 3 bits contain flags */
+	u8 data[8];
+};
+
+struct tx_done_msg {
+	u8 len;
+	u8 cmd;
+	u8 net;
+	u8 status;
+	__le32 hnd;
+	__le32 ts;
+};
+
+struct id_filter_msg {
+	u8 len;
+	u8 cmd;
+	u8 net;
+	u8 option;
+	__le32 mask[ESD_MAX_ID_SEGMENT + 1];
+};
+
+struct set_baudrate_msg {
+	u8 len;
+	u8 cmd;
+	u8 net;
+	u8 rsvd;
+	__le32 baud;
+};
+
+/* Main message type used between library and application */
+struct __attribute__ ((packed)) esd_usb2_msg {
+	union {
+		struct header_msg hdr;
+		struct version_msg version;
+		struct version_reply_msg version_reply;
+		struct rx_msg rx;
+		struct tx_msg tx;
+		struct tx_done_msg txdone;
+		struct set_baudrate_msg setbaud;
+		struct id_filter_msg filter;
+	} msg;
+};
+
+static struct usb_device_id esd_usb2_table[] = {
+	{USB_DEVICE(USB_ESDGMBH_VENDOR_ID, USB_CANUSB2_PRODUCT_ID)},
+	{}
+};
+MODULE_DEVICE_TABLE(usb, esd_usb2_table);
+
+struct esd_usb2_net_priv;
+
+struct esd_tx_urb_context {
+	struct esd_usb2_net_priv *priv;
+	u32 echo_index;
+	int dlc;
+};
+
+struct esd_usb2 {
+	struct usb_device *udev;
+	struct esd_usb2_net_priv *nets[ESD_USB2_MAX_NETS];
+
+	struct usb_anchor rx_submitted;
+
+	int net_count;
+	u32 version;
+	int rxinitdone;
+};
+
+struct esd_usb2_net_priv {
+	struct can_priv can; /* must be the first member */
+
+	atomic_t active_tx_jobs;
+	struct usb_anchor tx_submitted;
+	struct esd_tx_urb_context tx_contexts[MAX_TX_URBS];
+
+	int open_time;
+	struct esd_usb2 *usb2;
+	struct net_device *netdev;
+	int index;
+	u8 old_state;
+	struct can_berr_counter bec;
+};
+
+static void esd_usb2_rx_event(struct esd_usb2_net_priv *priv,
+			      struct esd_usb2_msg *msg)
+{
+	struct net_device_stats *stats = &priv->netdev->stats;
+	struct can_frame *cf;
+	struct sk_buff *skb;
+	u32 id = le32_to_cpu(msg->msg.rx.id) & ESD_IDMASK;
+
+	if (id == ESD_EV_CAN_ERROR_EXT) {
+		u8 state = msg->msg.rx.data[0];
+		u8 ecc = msg->msg.rx.data[1];
+		u8 txerr = msg->msg.rx.data[2];
+		u8 rxerr = msg->msg.rx.data[3];
+
+		skb = alloc_can_err_skb(priv->netdev, &cf);
+		if (skb == NULL) {
+			stats->rx_dropped++;
+			return;
+		}
+
+		if (state != priv->old_state) {
+			priv->old_state = state;
+
+			switch (state & ESD_BUSSTATE_MASK) {
+			case ESD_BUSSTATE_BUSOFF:
+				priv->can.state = CAN_STATE_BUS_OFF;
+				cf->can_id |= CAN_ERR_BUSOFF;
+				can_bus_off(priv->netdev);
+				break;
+			case ESD_BUSSTATE_WARN:
+				priv->can.state = CAN_STATE_ERROR_WARNING;
+				priv->can.can_stats.error_warning++;
+				break;
+			case ESD_BUSSTATE_ERRPASSIVE:
+				priv->can.state = CAN_STATE_ERROR_PASSIVE;
+				priv->can.can_stats.error_passive++;
+				break;
+			default:
+				priv->can.state = CAN_STATE_ERROR_ACTIVE;
+				break;
+			}
+		} else {
+			priv->can.can_stats.bus_error++;
+			stats->rx_errors++;
+
+			cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+
+			switch (ecc & SJA1000_ECC_MASK) {
+			case SJA1000_ECC_BIT:
+				cf->data[2] |= CAN_ERR_PROT_BIT;
+				break;
+			case SJA1000_ECC_FORM:
+				cf->data[2] |= CAN_ERR_PROT_FORM;
+				break;
+			case SJA1000_ECC_STUFF:
+				cf->data[2] |= CAN_ERR_PROT_STUFF;
+				break;
+			default:
+				cf->data[2] |= CAN_ERR_PROT_UNSPEC;
+				cf->data[3] = ecc & SJA1000_ECC_SEG;
+				break;
+			}
+
+			/* Error occured during transmission? */
+			if (!(ecc & SJA1000_ECC_DIR))
+				cf->data[2] |= CAN_ERR_PROT_TX;
+
+			if (priv->can.state == CAN_STATE_ERROR_WARNING ||
+			    priv->can.state == CAN_STATE_ERROR_PASSIVE) {
+				cf->data[1] = (txerr > rxerr) ?
+					CAN_ERR_CRTL_TX_PASSIVE :
+					CAN_ERR_CRTL_RX_PASSIVE;
+			}
+			cf->data[6] = txerr;
+			cf->data[7] = rxerr;
+		}
+
+		netif_rx(skb);
+
+		priv->bec.txerr = txerr;
+		priv->bec.rxerr = rxerr;
+
+		stats->rx_packets++;
+		stats->rx_bytes += cf->can_dlc;
+	}
+}
+
+static void esd_usb2_rx_can_msg(struct esd_usb2_net_priv *priv,
+				struct esd_usb2_msg *msg)
+{
+	struct net_device_stats *stats = &priv->netdev->stats;
+	struct can_frame *cf;
+	struct sk_buff *skb;
+	int i;
+	u32 id;
+
+	if (!netif_device_present(priv->netdev))
+		return;
+
+	id = le32_to_cpu(msg->msg.rx.id);
+
+	if (id & ESD_EVENT) {
+		esd_usb2_rx_event(priv, msg);
+	} else {
+		skb = alloc_can_skb(priv->netdev, &cf);
+		if (skb == NULL) {
+			stats->rx_dropped++;
+			return;
+		}
+
+		cf->can_id = id & ESD_IDMASK;
+		cf->can_dlc = get_can_dlc(msg->msg.rx.dlc);
+
+		if (id & ESD_EXTID)
+			cf->can_id |= CAN_EFF_FLAG;
+
+		if (msg->msg.rx.dlc & ESD_RTR) {
+			cf->can_id |= CAN_RTR_FLAG;
+		} else {
+			for (i = 0; i < cf->can_dlc; i++)
+				cf->data[i] = msg->msg.rx.data[i];
+		}
+
+		netif_rx(skb);
+
+		stats->rx_packets++;
+		stats->rx_bytes += cf->can_dlc;
+	}
+
+	return;
+}
+
+static void esd_usb2_tx_done_msg(struct esd_usb2_net_priv *priv,
+				 struct esd_usb2_msg *msg)
+{
+	struct net_device_stats *stats = &priv->netdev->stats;
+	struct net_device *netdev = priv->netdev;
+	struct esd_tx_urb_context *context;
+
+	if (!netif_device_present(netdev))
+		return;
+
+	context = &priv->tx_contexts[msg->msg.txdone.hnd & (MAX_TX_URBS - 1)];
+
+	if (!msg->msg.txdone.status) {
+		stats->tx_packets++;
+		stats->tx_bytes += context->dlc;
+		can_get_echo_skb(netdev, context->echo_index);
+	} else {
+		stats->tx_errors++;
+		can_free_echo_skb(netdev, context->echo_index);
+	}
+
+	/* Release context */
+	context->echo_index = MAX_TX_URBS;
+	atomic_dec(&priv->active_tx_jobs);
+
+	netif_wake_queue(netdev);
+}
+
+static void esd_usb2_read_bulk_callback(struct urb *urb)
+{
+	struct esd_usb2 *dev = urb->context;
+	int retval;
+	int pos = 0;
+	int i;
+
+	switch (urb->status) {
+	case 0: /* success */
+		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) {
+		struct esd_usb2_msg *msg;
+
+		msg = (struct esd_usb2_msg *)(urb->transfer_buffer + pos);
+
+		switch (msg->msg.hdr.cmd) {
+		case CMD_CAN_RX:
+			esd_usb2_rx_can_msg(dev->nets[msg->msg.rx.net], msg);
+			break;
+
+		case CMD_CAN_TX:
+			esd_usb2_tx_done_msg(dev->nets[msg->msg.txdone.net],
+					     msg);
+			break;
+		}
+
+		pos += msg->msg.hdr.len << 2;
+
+		if (pos > urb->actual_length) {
+			dev_err(dev->udev->dev.parent, "format error\n");
+			break;
+		}
+	}
+
+resubmit_urb:
+	usb_fill_bulk_urb(urb, dev->udev, usb_rcvbulkpipe(dev->udev, 1),
+			  urb->transfer_buffer, RX_BUFFER_SIZE,
+			  esd_usb2_read_bulk_callback, dev);
+
+	retval = usb_submit_urb(urb, GFP_ATOMIC);
+	if (retval == -ENODEV) {
+		for (i = 0; i < dev->net_count; i++) {
+			if (dev->nets[i])
+				netif_device_detach(dev->nets[i]->netdev);
+		}
+	} else if (retval) {
+		dev_err(dev->udev->dev.parent,
+			"failed resubmitting read bulk urb: %d\n", retval);
+	}
+
+	return;
+}
+
+/*
+ * callback for bulk IN urb
+ */
+static void esd_usb2_write_bulk_callback(struct urb *urb)
+{
+	struct esd_tx_urb_context *context = urb->context;
+	struct esd_usb2_net_priv *priv;
+	struct esd_usb2 *dev;
+	struct net_device *netdev;
+	size_t size = sizeof(struct esd_usb2_msg);
+
+	BUG_ON(!context);
+
+	priv = context->priv;
+	netdev = priv->netdev;
+	dev = priv->usb2;
+
+	/* free up our allocated buffer */
+	usb_free_coherent(urb->dev, size,
+			  urb->transfer_buffer, urb->transfer_dma);
+
+	if (!netif_device_present(netdev))
+		return;
+
+	if (urb->status)
+		dev_info(netdev->dev.parent, "Tx URB aborted (%d)\n",
+			 urb->status);
+
+	netdev->trans_start = jiffies;
+}
+
+#ifdef CONFIG_SYSFS
+static ssize_t show_firmware(struct device *d,
+			     struct device_attribute *attr, char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(d);
+	struct esd_usb2 *dev = usb_get_intfdata(intf);
+
+	return sprintf(buf, "%d.%d.%d\n",
+		       (dev->version >> 12) & 0xf,
+		       (dev->version >> 8) & 0xf,
+		       dev->version & 0xff);
+}
+static DEVICE_ATTR(firmware, S_IRUGO, show_firmware, NULL);
+
+static ssize_t show_hardware(struct device *d,
+			     struct device_attribute *attr, char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(d);
+	struct esd_usb2 *dev = usb_get_intfdata(intf);
+
+	return sprintf(buf, "%d.%d.%d\n",
+		       (dev->version >> 28) & 0xf,
+		       (dev->version >> 24) & 0xf,
+		       (dev->version >> 16) & 0xff);
+}
+static DEVICE_ATTR(hardware, S_IRUGO, show_hardware, NULL);
+
+static ssize_t show_nets(struct device *d,
+			 struct device_attribute *attr, char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(d);
+	struct esd_usb2 *dev = usb_get_intfdata(intf);
+
+	return sprintf(buf, "%d", dev->net_count);
+}
+static DEVICE_ATTR(nets, S_IRUGO, show_nets, NULL);
+#endif
+
+static int esd_usb2_send_msg(struct esd_usb2 *dev, struct esd_usb2_msg *msg)
+{
+	int actual_length;
+
+	return usb_bulk_msg(dev->udev,
+			    usb_sndbulkpipe(dev->udev, 2),
+			    msg,
+			    msg->msg.hdr.len << 2,
+			    &actual_length,
+			    1000);
+}
+
+static int esd_usb2_wait_msg(struct esd_usb2 *dev,
+			     struct esd_usb2_msg *msg)
+{
+	int actual_length;
+
+	return usb_bulk_msg(dev->udev,
+			    usb_rcvbulkpipe(dev->udev, 1),
+			    msg,
+			    sizeof(*msg),
+			    &actual_length,
+			    1000);
+}
+
+static int esd_usb2_setup_rx_urbs(struct esd_usb2 *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;
+
+		/* create a URB, and a buffer for it */
+		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,
+					 &urb->transfer_dma);
+		if (!buf) {
+			dev_warn(dev->udev->dev.parent,
+				 "No memory left for USB buffer\n");
+			err = -ENOMEM;
+			goto freeurb;
+		}
+
+		usb_fill_bulk_urb(urb, dev->udev,
+				  usb_rcvbulkpipe(dev->udev, 1),
+				  buf, RX_BUFFER_SIZE,
+				  esd_usb2_read_bulk_callback, dev);
+		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,
+					  urb->transfer_dma);
+		}
+
+freeurb:
+		/* Drop reference, USB core will take care of freeing it */
+		usb_free_urb(urb);
+		if (err)
+			break;
+	}
+
+	/* Did we submit any URBs */
+	if (i == 0) {
+		dev_err(dev->udev->dev.parent, "couldn't setup read URBs\n");
+		return err;
+	}
+
+	/* Warn if we've couldn't transmit all the URBs */
+	if (i < MAX_RX_URBS) {
+		dev_warn(dev->udev->dev.parent,
+			 "rx performance may be slow\n");
+	}
+
+	dev->rxinitdone = 1;
+	return 0;
+}
+
+/*
+ * Start interface
+ */
+static int esd_usb2_start(struct esd_usb2_net_priv *priv)
+{
+	struct esd_usb2 *dev = priv->usb2;
+	struct net_device *netdev = priv->netdev;
+	struct esd_usb2_msg msg;
+	int err, i;
+
+	/*
+	 * Enable all IDs
+	 * The IDADD message takes up to 64 32 bit bitmasks (2048 bits).
+	 * Each bit represents one 11 bit CAN identifier. A set bit
+	 * enables reception of the corresponding CAN identifier. A cleared
+	 * bit disabled this identifier. An additional bitmask value
+	 * following the CAN 2.0A bits is used to enable reception of
+	 * extended CAN frames. Only the LSB of this final mask is checked
+	 * for the complete 29 bit ID range. The IDADD message also allows
+	 * filter configuration for an ID subset. In this case you can add
+	 * the number of the starting bitmask (0..64) to the filter.option
+	 * field followed by only some bitmasks.
+	 */
+	msg.msg.hdr.cmd = CMD_IDADD;
+	msg.msg.hdr.len = 2 + ESD_MAX_ID_SEGMENT;
+	msg.msg.filter.net = priv->index;
+	msg.msg.filter.option = ESD_ID_ENABLE; /* start with segment 0 */
+	for (i = 0; i < ESD_MAX_ID_SEGMENT; i++)
+		msg.msg.filter.mask[i] = cpu_to_le32(0xffffffff);
+	/* enable 29bit extended IDs */
+	msg.msg.filter.mask[ESD_MAX_ID_SEGMENT] = cpu_to_le32(0x00000001);
+
+	err = esd_usb2_send_msg(dev, &msg);
+	if (err)
+		goto failed;
+
+	err = esd_usb2_setup_rx_urbs(dev);
+	if (err)
+		goto failed;
+
+	priv->can.state = CAN_STATE_ERROR_ACTIVE;
+
+	return 0;
+
+failed:
+	if (err == -ENODEV)
+		netif_device_detach(netdev);
+
+	dev_err(netdev->dev.parent, "couldn't start device: %d\n", err);
+
+	return err;
+}
+
+static void unlink_all_urbs(struct esd_usb2 *dev)
+{
+	struct esd_usb2_net_priv *priv;
+	int i;
+
+	usb_kill_anchored_urbs(&dev->rx_submitted);
+	for (i = 0; i < dev->net_count; i++) {
+		priv = dev->nets[i];
+		if (priv) {
+			usb_kill_anchored_urbs(&priv->tx_submitted);
+			atomic_set(&priv->active_tx_jobs, 0);
+
+			for (i = 0; i < MAX_TX_URBS; i++)
+				priv->tx_contexts[i].echo_index = MAX_TX_URBS;
+		}
+	}
+}
+
+static int esd_usb2_open(struct net_device *netdev)
+{
+	struct esd_usb2_net_priv *priv = netdev_priv(netdev);
+	int err;
+
+	/* common open */
+	err = open_candev(netdev);
+	if (err)
+		return err;
+
+	/* finally start device */
+	err = esd_usb2_start(priv);
+	if (err) {
+		dev_warn(netdev->dev.parent,
+			 "couldn't start device: %d\n", err);
+		close_candev(netdev);
+		return err;
+	}
+
+	priv->open_time = jiffies;
+
+	netif_start_queue(netdev);
+
+	return 0;
+}
+
+static netdev_tx_t esd_usb2_start_xmit(struct sk_buff *skb,
+				      struct net_device *netdev)
+{
+	struct esd_usb2_net_priv *priv = netdev_priv(netdev);
+	struct esd_usb2 *dev = priv->usb2;
+	struct esd_tx_urb_context *context = NULL;
+	struct net_device_stats *stats = &netdev->stats;
+	struct can_frame *cf = (struct can_frame *)skb->data;
+	struct esd_usb2_msg *msg;
+	struct urb *urb;
+	u8 *buf;
+	int i, err;
+	int ret = NETDEV_TX_OK;
+	size_t size = sizeof(struct esd_usb2_msg);
+
+	if (can_dropped_invalid_skb(netdev, skb))
+		return NETDEV_TX_OK;
+
+	/* create a URB, and a buffer for it, and copy the data to the URB */
+	urb = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!urb) {
+		dev_err(netdev->dev.parent, "No memory left for URBs\n");
+		stats->tx_dropped++;
+		dev_kfree_skb(skb);
+		goto nourbmem;
+	}
+
+	buf = usb_alloc_coherent(dev->udev, size, GFP_ATOMIC,
+				 &urb->transfer_dma);
+	if (!buf) {
+		dev_err(netdev->dev.parent, "No memory left for USB buffer\n");
+		stats->tx_dropped++;
+		dev_kfree_skb(skb);
+		goto nobufmem;
+	}
+
+	msg = (struct esd_usb2_msg *)buf;
+
+	msg->msg.hdr.len = 3; /* minimal length */
+	msg->msg.hdr.cmd = CMD_CAN_TX;
+	msg->msg.tx.net = priv->index;
+	msg->msg.tx.dlc = cf->can_dlc;
+	msg->msg.tx.id = cpu_to_le32(cf->can_id & CAN_ERR_MASK);
+
+	if (cf->can_id & CAN_RTR_FLAG)
+		msg->msg.tx.dlc |= ESD_RTR;
+
+	if (cf->can_id & CAN_EFF_FLAG)
+		msg->msg.tx.id |= cpu_to_le32(ESD_EXTID);
+
+	for (i = 0; i < cf->can_dlc; i++)
+		msg->msg.tx.data[i] = cf->data[i];
+
+	msg->msg.hdr.len += (cf->can_dlc + 3) >> 2;
+
+	for (i = 0; i < MAX_TX_URBS; i++) {
+		if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
+			context = &priv->tx_contexts[i];
+			break;
+		}
+	}
+
+	/*
+	 * This may never happen.
+	 */
+	if (!context) {
+		dev_warn(netdev->dev.parent, "couldn't find free context\n");
+		ret = NETDEV_TX_BUSY;
+		goto releasebuf;
+	}
+
+	context->priv = priv;
+	context->echo_index = i;
+	context->dlc = cf->can_dlc;
+
+	/* hnd must not be 0 - MSB is stripped in txdone handling */
+	msg->msg.tx.hnd = 0x80000000 | i; /* returned in TX done message */
+
+	usb_fill_bulk_urb(urb, dev->udev, usb_sndbulkpipe(dev->udev, 2), buf,
+			  msg->msg.hdr.len << 2,
+			  esd_usb2_write_bulk_callback, context);
+
+	urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+
+	usb_anchor_urb(urb, &priv->tx_submitted);
+
+	can_put_echo_skb(skb, netdev, context->echo_index);
+
+	atomic_inc(&priv->active_tx_jobs);
+
+	err = usb_submit_urb(urb, GFP_ATOMIC);
+	if (err) {
+		can_free_echo_skb(netdev, context->echo_index);
+
+		atomic_dec(&priv->active_tx_jobs);
+		usb_unanchor_urb(urb);
+
+		stats->tx_dropped++;
+
+		if (err == -ENODEV)
+			netif_device_detach(netdev);
+		else
+			dev_warn(netdev->dev.parent, "failed tx_urb %d\n", err);
+
+		goto releasebuf;
+	}
+
+	netdev->trans_start = jiffies;
+
+	/* Slow down tx path */
+	if (atomic_read(&priv->active_tx_jobs) >= MAX_TX_URBS)
+		netif_stop_queue(netdev);
+
+	/*
+	 * Release our reference to this URB, the USB core will eventually free
+	 * it entirely.
+	 */
+	usb_free_urb(urb);
+
+	return NETDEV_TX_OK;
+
+releasebuf:
+	usb_free_coherent(dev->udev, size, buf, urb->transfer_dma);
+
+nobufmem:
+	usb_free_urb(urb);
+
+nourbmem:
+	return ret;
+}
+
+static int esd_usb2_close(struct net_device *netdev)
+{
+	struct esd_usb2_net_priv *priv = netdev_priv(netdev);
+	struct esd_usb2_msg msg;
+	int i;
+
+	/* Disable all IDs (see esd_usb2_start()) */
+	msg.msg.hdr.cmd = CMD_IDADD;
+	msg.msg.hdr.len = 2 + ESD_MAX_ID_SEGMENT;
+	msg.msg.filter.net = priv->index;
+	msg.msg.filter.option = ESD_ID_ENABLE; /* start with segment 0 */
+	for (i = 0; i <= ESD_MAX_ID_SEGMENT; i++)
+		msg.msg.filter.mask[i] = 0;
+	esd_usb2_send_msg(priv->usb2, &msg);
+
+	/* set CAN controller to reset mode */
+	msg.msg.hdr.len = 2;
+	msg.msg.hdr.cmd = CMD_SETBAUD;
+	msg.msg.setbaud.net = priv->index;
+	msg.msg.setbaud.rsvd = 0;
+	msg.msg.setbaud.baud = cpu_to_le32(ESD_USB2_NO_BAUDRATE);
+	esd_usb2_send_msg(priv->usb2, &msg);
+
+	priv->can.state = CAN_STATE_STOPPED;
+
+	netif_stop_queue(netdev);
+
+	close_candev(netdev);
+
+	priv->open_time = 0;
+
+	return 0;
+}
+
+static const struct net_device_ops esd_usb2_netdev_ops = {
+	.ndo_open = esd_usb2_open,
+	.ndo_stop = esd_usb2_close,
+	.ndo_start_xmit = esd_usb2_start_xmit,
+};
+
+static struct can_bittiming_const esd_usb2_bittiming_const = {
+	.name = "esd_usb2",
+	.tseg1_min = ESD_USB2_TSEG1_MIN,
+	.tseg1_max = ESD_USB2_TSEG1_MAX,
+	.tseg2_min = ESD_USB2_TSEG2_MIN,
+	.tseg2_max = ESD_USB2_TSEG2_MAX,
+	.sjw_max = ESD_USB2_SJW_MAX,
+	.brp_min = ESD_USB2_BRP_MIN,
+	.brp_max = ESD_USB2_BRP_MAX,
+	.brp_inc = ESD_USB2_BRP_INC,
+};
+
+static int esd_usb2_set_bittiming(struct net_device *netdev)
+{
+	struct esd_usb2_net_priv *priv = netdev_priv(netdev);
+	struct can_bittiming *bt = &priv->can.bittiming;
+	struct esd_usb2_msg msg;
+	u32 canbtr;
+
+	canbtr = ESD_USB2_UBR;
+	canbtr |= (bt->brp - 1) & (ESD_USB2_BRP_MAX - 1);
+	canbtr |= ((bt->sjw - 1) & (ESD_USB2_SJW_MAX - 1))
+		<< ESD_USB2_SJW_SHIFT;
+	canbtr |= ((bt->prop_seg + bt->phase_seg1 - 1)
+		   & (ESD_USB2_TSEG1_MAX - 1))
+		<< ESD_USB2_TSEG1_SHIFT;
+	canbtr |= ((bt->phase_seg2 - 1) & (ESD_USB2_TSEG2_MAX - 1))
+		<< ESD_USB2_TSEG2_SHIFT;
+	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
+		canbtr |= ESD_USB2_3_SAMPLES;
+
+	msg.msg.hdr.len = 2;
+	msg.msg.hdr.cmd = CMD_SETBAUD;
+	msg.msg.setbaud.net = priv->index;
+	msg.msg.setbaud.rsvd = 0;
+	msg.msg.setbaud.baud = cpu_to_le32(canbtr);
+
+	dev_info(netdev->dev.parent, "setting BTR=%#x\n", canbtr);
+
+	return esd_usb2_send_msg(priv->usb2, &msg);
+}
+
+static int esd_usb2_get_berr_counter(const struct net_device *netdev,
+				     struct can_berr_counter *bec)
+{
+	struct esd_usb2_net_priv *priv = netdev_priv(netdev);
+
+	bec->txerr = priv->bec.txerr;
+	bec->rxerr = priv->bec.rxerr;
+
+	return 0;
+}
+
+static int esd_usb2_set_mode(struct net_device *netdev, enum can_mode mode)
+{
+	struct esd_usb2_net_priv *priv = netdev_priv(netdev);
+
+	if (!priv->open_time)
+		return -EINVAL;
+
+	switch (mode) {
+	case CAN_MODE_START:
+		netif_wake_queue(netdev);
+		break;
+
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static int esd_usb2_probe_one_net(struct usb_interface *intf, int index)
+{
+	struct esd_usb2 *dev = usb_get_intfdata(intf);
+	struct net_device *netdev;
+	struct esd_usb2_net_priv *priv;
+	int err;
+	int i;
+
+	netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS);
+	if (!netdev) {
+		dev_err(&intf->dev, "couldn't alloc candev\n");
+		return -ENOMEM;
+	}
+
+	priv = netdev_priv(netdev);
+
+	init_usb_anchor(&priv->tx_submitted);
+	atomic_set(&priv->active_tx_jobs, 0);
+
+	for (i = 0; i < MAX_TX_URBS; i++)
+		priv->tx_contexts[i].echo_index = MAX_TX_URBS;
+
+	priv->usb2 = dev;
+	priv->netdev = netdev;
+	priv->index = index;
+
+	priv->can.state = CAN_STATE_STOPPED;
+	priv->can.clock.freq = ESD_USB2_CAN_CLOCK;
+	priv->can.bittiming_const = &esd_usb2_bittiming_const;
+	priv->can.do_set_bittiming = esd_usb2_set_bittiming;
+	priv->can.do_set_mode = esd_usb2_set_mode;
+	priv->can.do_get_berr_counter = esd_usb2_get_berr_counter;
+	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
+
+	netdev->flags |= IFF_ECHO; /* we support local echo */
+
+	netdev->netdev_ops = &esd_usb2_netdev_ops;
+
+	SET_NETDEV_DEV(netdev, &intf->dev);
+
+	err = register_candev(netdev);
+	if (err) {
+		dev_err(&intf->dev,
+			"couldn't register CAN device: %d\n", err);
+		free_candev(netdev);
+		return -ENOMEM;
+	}
+
+	dev->nets[index] = priv;
+	dev_info(netdev->dev.parent, "device %s registered\n", netdev->name);
+	return 0;
+}
+
+/*
+ * probe function for new USB2 devices
+ *
+ * check version information and number of available
+ * CAN interfaces
+ */
+static int esd_usb2_probe(struct usb_interface *intf,
+			 const struct usb_device_id *id)
+{
+	struct esd_usb2 *dev;
+	struct esd_usb2_msg msg;
+	int i, err = -ENOMEM;
+
+	dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return -ENOMEM;
+
+	dev->udev = interface_to_usbdev(intf);
+
+	init_usb_anchor(&dev->rx_submitted);
+
+	usb_set_intfdata(intf, dev);
+
+	/* query number of CAN interfaces (nets) */
+	msg.msg.hdr.cmd = CMD_VERSION;
+	msg.msg.hdr.len = 2;
+	msg.msg.version.rsvd = 0;
+	msg.msg.version.flags = 0;
+	msg.msg.version.drv_version = 0;
+
+	if (esd_usb2_send_msg(dev, &msg) < 0) {
+		dev_err(&intf->dev, "sending version message failed\n");
+		goto free_dev;
+	}
+
+	if (esd_usb2_wait_msg(dev, &msg) < 0) {
+		dev_err(&intf->dev, "no version message answer\n");
+		goto free_dev;
+	}
+
+	dev->net_count = (int)msg.msg.version_reply.nets;
+	dev->version = le32_to_cpu(msg.msg.version_reply.version);
+
+#ifdef CONFIG_SYSFS
+	if (device_create_file(&intf->dev, &dev_attr_firmware))
+		dev_err(&intf->dev,
+			"Couldn't create device file for firmware\n");
+
+	if (device_create_file(&intf->dev, &dev_attr_hardware))
+		dev_err(&intf->dev,
+			"Couldn't create device file for hardware\n");
+
+	if (device_create_file(&intf->dev, &dev_attr_nets))
+		dev_err(&intf->dev,
+			"Couldn't create device file for nets\n");
+#endif
+
+	/* do per device probing */
+	for (i = 0; i < dev->net_count; i++)
+		esd_usb2_probe_one_net(intf, i);
+
+	return 0;
+
+free_dev:
+	kfree(dev);
+	return err;
+}
+
+/*
+ * called by the usb core when the device is removed from the system
+ */
+static void esd_usb2_disconnect(struct usb_interface *intf)
+{
+	struct esd_usb2 *dev = usb_get_intfdata(intf);
+	struct net_device *netdev;
+	int i;
+
+#ifdef CONFIG_SYSFS
+	device_remove_file(&intf->dev, &dev_attr_firmware);
+	device_remove_file(&intf->dev, &dev_attr_hardware);
+	device_remove_file(&intf->dev, &dev_attr_nets);
+#endif
+	usb_set_intfdata(intf, NULL);
+
+	if (dev) {
+		for (i = 0; i < dev->net_count; i++) {
+			if (dev->nets[i]) {
+				netdev = dev->nets[i]->netdev;
+				unregister_netdev(netdev);
+				free_candev(netdev);
+			}
+		}
+		unlink_all_urbs(dev);
+	}
+}
+
+/* usb specific object needed to register this driver with the usb subsystem */
+static struct usb_driver esd_usb2_driver = {
+	.name = "esd_usb2",
+	.probe = esd_usb2_probe,
+	.disconnect = esd_usb2_disconnect,
+	.id_table = esd_usb2_table,
+};
+
+static int __init esd_usb2_init(void)
+{
+	int err;
+
+	/* register this driver with the USB subsystem */
+	err = usb_register(&esd_usb2_driver);
+
+	if (err) {
+		err("usb_register failed. Error number %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+module_init(esd_usb2_init);
+
+static void __exit esd_usb2_exit(void)
+{
+	/* deregister this driver with the USB subsystem */
+	usb_deregister(&esd_usb2_driver);
+}
+module_exit(esd_usb2_exit);
-- 
1.5.6.3

--
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: [RFC] IFLA_PORT_* iproute2 cmd line
From: Arnd Bergmann @ 2010-05-26 12:38 UTC (permalink / raw)
  To: Scott Feldman
  Cc: netdev, Chris Wright, Stephen Hemminger, Stefan Berger,
	Dirk Herrendoerfer, Vivek Kashyap
In-Reply-To: <C821E15E.35236%scofeldm@cisco.com>

On Wednesday 26 May 2010, Scott Feldman wrote:
> I need to provide an iproute2 patch for IFLA_PORT_* and I wanted to hash out
> the cmd line before I submit it.  Here's what I think would work based on
> previous input from Arnd:
> 
> Usage:  ip port associate DEVICE [ vf NUM ] {PROFILE|VSI}
>         ip port pre-associate DEVICE [ vf NUM ] VSI
>         ip port pre-associate-rr DEVICE [ vf NUM ] VSI
>         ip port dis-associate DEVICE [ vf NUM ]
>         ip port show [ DEVICE [ vf NUM ] ]
> 
>         PROFILE := port-profile PORT-PROFILE
>                    [ instance-uuid INSTANCE-UUID ]
>                    [ host-uuid HOST-UUID ]
> 
>         VSI := vsi managerid MGR typeid VTID typeidversion VER
>                [ instance-uuid INSTANCE-UUID ]
> 
> Comments?

The syntax of the PROFILE and VSI arguments seems ok, but I'm
not sure where exactly to put them.

When talking to the kernel, I think this should be part of 
link command, because that is the underlying protocol:

ip link set DEVICE [vf NUM] {associate {PROFILE|VSI}    |
			     pre-associate-rr VSI       |
			     pre-associate VSI          |
			     disassociate }
ip link show [ DEVICE [ vf NUM ] ] 

This will also let you combine the association with additional
"vf mac" and "vf vlan" commands as needed.

The more interesting question is how to do this when we
talk to lldpad. One idea was to use the same protocol
but to direct the message to a specific pid (that of lldpad).
That would require adding an option like '-p PID' to ip
that lets us change who we talk to.

Alternatively, we could also create a top-level command like
the one you described, but just use it for the case when
we're talking to lldpad, finding out the PID from the
/var/run/lldpdad.pid internally. Right now, I'm leaning
towards the more flexible option of being able to direct
the command anywhere.

	Arnd

^ permalink raw reply

* RE: why the number of queue of NIC impact the ping results?
From: Jon Zhou @ 2010-05-26 11:47 UTC (permalink / raw)
  To: Rahul Gundecha, netdev@vger.kernel.org
In-Reply-To: <AANLkTil1vLvEcA6MbiamYVRZbGpPh-I8sza0WHOX69hr@mail.gmail.com>

No, I am using 1.52.12
seems 1.45.21 doesn't support too much queues.
I will try it with latest kernel.




-----Original Message-----
From: Rahul Gundecha [mailto:rahul.gundecha@gmail.com] 
Sent: Wednesday, May 26, 2010 7:17 PM
To: netdev@vger.kernel.org
Cc: Jon Zhou
Subject: Fwd: why the number of queue of NIC impact the ping results?

What is the driver version you are using?  Is it 1.45.21, the one
present in 2.6.27-19 ?
Upgrading the whole kernel to latest version would be helpful for multi-queue.

On Wed, May 26, 2010 at 2:31 PM, Jon Zhou <Jon.Zhou@jdsu.com> wrote:
>
> broadcom bnx2x netxtreme2-5.2.50,kernel 2.6.27-19
>
> @receiver:
>
> insmod ./bnx2x.ko  multi_mode=1 num_queues=12/14/16
>
> @sender:
> ping receiver ->response OK
>
>
> but no response when
>
> insmod ./bnx2x.ko  multi_mode=1 num_queues=2/4/8/10
>
> have any idea?
>
> thanks
> jon
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: Question about an assignment in handle_ing()
From: jamal @ 2010-05-26 11:38 UTC (permalink / raw)
  To: Herbert Xu; +Cc: Jiri Pirko, netdev, davem, kaber
In-Reply-To: <20100525231307.GA19475@gondor.apana.org.au>

On Wed, 2010-05-26 at 09:13 +1000, Herbert Xu wrote:

> If it did happen like you said then it would be a serious bug
> in our stack as everything else (including the TCP stack) relies
> on this.

It could have been a bug. Note this was not a simple test, so there
may be other factors involved. If you or Jiri are willing to run the
test i will construct a scenario which will test this out. It will need
a compile of the kernel and a small check in pedit to see if we see
cloned skbs when we run the two tcpdumps (and to make sure the tcpdumps
see the correct bytes). Otherwise i will get to it by weekend.
BTW: Jiri, out of curiosity - what was the issue seen that caused the
original question?

> But how can the caller make that decision when you return exactly
> the same value in the error case as the normal case?

Ok - i see your point Herbert ;-> 
it makes sense to have pedit have an error action code like some of the
others actions which defaults to a drop.
I will do a proper patch sometime this weekend.

cheers,
jamal


^ permalink raw reply

* Re: [PATCH] mlx4_en: show device's port used
From: Tziporet Koren @ 2010-05-26 11:35 UTC (permalink / raw)
  To: Roland Dreier
  Cc: Eli Cohen, Eli Cohen,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux RDMA list,
	Yevgeny Petrilin
In-Reply-To: <adask5fy8et.fsf-BjVyx320WGW9gfZ95n9DRSW4+XlvGpQz@public.gmane.org>

On 5/26/2010 12:32 AM, Roland Dreier wrote:
>   >  I don't think there are many devices out there which have more than
>   >  one port.
>
> ??
>
> http://developer.intel.com/network/connectivity/solutions/gigabit.htm
> http://www.broadcom.com/products/Ethernet-Controllers/Enterprise-Server/BCM5704C
>
> etc
>
>    
But they have different PCI function for each port and we have 2 ports 
on same PCI device

Tziporet

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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

* RE: [PATCH v3] can: Add driver for esd CAN-USB/2 device
From: Viral Mehta @ 2010-05-26 11:31 UTC (permalink / raw)
  To: Matthias Fuchs, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <201005261114.03214.matthias.fuchs-iOnpLzIbIdM@public.gmane.org>

Hi,
>________________________________________
>From: linux-usb-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [linux-usb-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Matthias Fuchs [matthias.fuchs-iOnpLzIbIdM@public.gmane.org]
>Sent: Wednesday, May 26, 2010 2:44 PM
>To: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>Cc: Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org; linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>Subject: [PATCH v3] can: Add driver for esd CAN-USB/2 device
>
>This patch adds a driver for esd's USB high speed
>CAN interface. The driver supports devices with
>multiple CAN interfaces.
>
>Signed-off-by: Matthias Fuchs <matthias.fuchs-iOnpLzIbIdM@public.gmane.org>
>---

>+static void esd_usb2_write_bulk_callback(struct urb *urb)
>+{
>+       struct esd_tx_urb_context *context = urb->context;
>+       struct esd_usb2_net_priv *priv;
>+       struct esd_usb2 *dev;
>+       struct net_device *netdev;
>+       size_t size = sizeof(struct esd_usb2_msg);
>+
>+       BUG_ON(!context);

It is preferred to used WARN_ON and avoid using BUG_ON and thus dont kill the whole system....
[...]
>+
>+       priv = context->priv;
>+       netdev = priv->netdev;
>+       dev = priv->usb2;
>+       err = usb_submit_urb(urb, GFP_ATOMIC);
>+       if (err) {
>+               can_free_echo_skb(netdev, context->echo_index);
>+
>+               atomic_dec(&priv->active_tx_jobs);
>+               usb_unanchor_urb(urb);
>+
>+               stats->tx_dropped++;
>+
>+               if (err == -ENODEV)
>+                       netif_device_detach(netdev);
>+               else
>+                       dev_warn(netdev->dev.parent, "failed tx_urb %d\n", err);
>+
>+               goto releasebuf;

You probably want to set "ret" here or do you really want to return NETDEV_TX_OK
>+       }

[...]
>+static int esd_usb2_close(struct net_device *netdev)
>+{
>+       struct esd_usb2_net_priv *priv = netdev_priv(netdev);
>+       struct esd_usb2_msg msg;
>+       int i;
>+
>+       /* Disable all IDs (see esd_usb2_start()) */
>+       msg.msg.hdr.cmd = CMD_IDADD;
>+       msg.msg.hdr.len = 2 + ESD_MAX_ID_SEGMENT;
>+       msg.msg.filter.net = priv->index;
>+       msg.msg.filter.option = ESD_ID_ENABLE; /* start with segment 0 */
>+       for (i = 0; i <= ESD_MAX_ID_SEGMENT; i++)
>+               msg.msg.filter.mask[i] = 0;
>+       esd_usb2_send_msg(priv->usb2, &msg);

Probably, you may not care but it can return error value....
>+
>+       /* set CAN controller to reset mode */
>+       msg.msg.hdr.len = 2;
>+       msg.msg.hdr.cmd = CMD_SETBAUD;
>+       msg.msg.setbaud.net = priv->index;
>+       msg.msg.setbaud.rsvd = 0;
>+       msg.msg.setbaud.baud = cpu_to_le32(ESD_USB2_NO_BAUDRATE);
>+       esd_usb2_send_msg(priv->usb2, &msg);

Probably, you may not care but it can return error value....

>+
>+       priv->can.state = CAN_STATE_STOPPED;
>+
>+       netif_stop_queue(netdev);
>+
>+       close_candev(netdev);
>+

[...]
>+static int esd_usb2_probe(struct usb_interface *intf,
>+                        const struct usb_device_id *id)
>+{
>+       struct esd_usb2 *dev;
>+       struct esd_usb2_msg msg;
>+       int i, err = -ENOMEM;
>+
>+       dev = kzalloc(sizeof(*dev), GFP_KERNEL);
>+       if (!dev)
>+               return -ENOMEM;

You probably would like to rewrite "return" statements.... for example, above you define err=-ENOMEM
and then here you are saying "return -ENOMEM" later on at the end you are saying "return err";

Also, people prefer and practice to have only one return point. Read further.

>+
>+       dev->udev = interface_to_usbdev(intf);
>+
>+       init_usb_anchor(&dev->rx_submitted);
>+
>+       usb_set_intfdata(intf, dev);
>+
>+       /* query number of CAN interfaces (nets) */
>+       msg.msg.hdr.cmd = CMD_VERSION;
>+       msg.msg.hdr.len = 2;
>+       msg.msg.version.rsvd = 0;
>+       msg.msg.version.flags = 0;
>+       msg.msg.version.drv_version = 0;
>+
>+       if (esd_usb2_send_msg(dev, &msg) < 0) {
>+               dev_err(&intf->dev, "sending version message failed\n");
>+               goto free_dev;

You might like to set "err" here, since you are not propogating error values properly.....
Irrespective of what esd_usb2_send_msg and thus in turn usb_bulk_msg returns,
you are ignoring those values and always returning -ENOMEM at the end......
>+       }
>+
>+       if (esd_usb2_wait_msg(dev, &msg) < 0) {
>+               dev_err(&intf->dev, "no version message answer\n");
>+               goto free_dev;

Same here.

>+free_dev:
>+       kfree(dev);
>+       return err;
>+}

HTH,
Viral Mehta

This Email may contain confidential or privileged information for the intended recipient (s) If you are not the intended recipient, please do not use or disseminate the information, notify the sender and delete it from your system.

______________________________________________________________________
--
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

* Fwd: why the number of queue of NIC impact the ping results?
From: Rahul Gundecha @ 2010-05-26 11:16 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: Jon Zhou
In-Reply-To: <AANLkTil27c_dBP0hgcEMSCUNakyU-4kc4_0ATw2gt2eT@mail.gmail.com>

What is the driver version you are using?  Is it 1.45.21, the one
present in 2.6.27-19 ?
Upgrading the whole kernel to latest version would be helpful for multi-queue.

On Wed, May 26, 2010 at 2:31 PM, Jon Zhou <Jon.Zhou@jdsu.com> wrote:
>
> broadcom bnx2x netxtreme2-5.2.50,kernel 2.6.27-19
>
> @receiver:
>
> insmod ./bnx2x.ko  multi_mode=1 num_queues=12/14/16
>
> @sender:
> ping receiver ->response OK
>
>
> but no response when
>
> insmod ./bnx2x.ko  multi_mode=1 num_queues=2/4/8/10
>
> have any idea?
>
> thanks
> jon
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RX/close vcc race with solos/atmtcp/usbatm/he
From: David Woodhouse @ 2010-05-26 11:16 UTC (permalink / raw)
  To: linux-atm-general, netdev; +Cc: Nathan Williams

I've had this crash reported to me...

[18842.727906] EIP: [<e082f490>] br2684_push+0x19/0x234 [br2684] SS:ESP 0068:dfb89d14 

[18845.090712] [<c13ecff3>] ? do_page_fault+0x0/0x2e1 
[18845.120042] [<e082f490>] ? br2684_push+0x19/0x234 [br2684] 
[18845.153530] [<e084fa13>] solos_bh+0x28b/0x7c8 [solos_pci] 
[18845.186488] [<e084f711>] ? solos_irq+0x2d/0x51 [solos_pci] 
[18845.219960] [<c100387b>] ? handle_irq+0x3b/0x48 
[18845.247732] [<c10265cb>] ? irq_exit+0x34/0x57 
[18845.274437] [<c1025720>] tasklet_action+0x42/0x69 
[18845.303247] [<c102643f>] __do_softirq+0x8e/0x129 
[18845.331540] [<c10264ff>] do_softirq+0x25/0x2a 
[18845.358274] [<c102664c>] _local_bh_enable_ip+0x5e/0x6a 
[18845.389677] [<c102666d>] local_bh_enable+0xb/0xe 
[18845.417944] [<e08490a8>] ppp_unregister_channel+0x32/0xbb [ppp_generic] 
[18845.458193] [<e08731ad>] pppox_unbind_sock+0x18/0x1f [pppox] 
[18845.492712] [<e087f5f7>] pppoe_device_event+0xa7/0x159 [pppoe] 
[18845.528269] [<c13ed2ff>] notifier_call_chain+0x2b/0x4a 
[18845.559674] [<c1038a62>] raw_notifier_call_chain+0xc/0xe 
[18845.592110] [<c1300867>] dev_close+0x51/0x8b 
[18845.618266] [<c1300927>] rollback_registered_many+0x86/0x15e 
[18845.652813] [<c1300ab3>] unregister_netdevice_queue+0x67/0x91 
[18845.687849] [<c1300b79>] unregister_netdev+0x14/0x1c 
[18845.718221] [<e082f4d1>] br2684_push+0x5a/0x234 [br2684] 
[18845.750676] [<e083dc21>] vcc_release+0x64/0x100 [atm] 

The problem is that the 'find_vcc' functions in these drivers are
returning a vcc with the ATM_VF_READY bit cleared, because it's already
in the process of being destroyed. If we fix that simple oversight,
there's still a race condition because the socket can still be closed
(and completely freed, afaict) between our call to find_vcc() and our
call to vcc->push() in the RX tasklet.

Here's a patch for solos-pci which should fix it. We prevent the race by
making the dev->ops->close() function wait for the RX tasklet to
complete, so it can't still be using the vcc in question.

I think this same approach should work OK for usbatm and he. Less sure
about atmtcp -- we may need some extra locking there to protect
atmtcp_c_send(). And I'm ignoring eni_proc_read() for now.

Can anyone see a better approach -- short of rewriting the whole ATM
layer to make the locking saner?

diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index c5f5186..a73f102 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -774,7 +774,8 @@ static struct atm_vcc *find_vcc(struct atm_dev *dev, short vpi, int vci)
 	sk_for_each(s, node, head) {
 		vcc = atm_sk(s);
 		if (vcc->dev == dev && vcc->vci == vci &&
-		    vcc->vpi == vpi && vcc->qos.rxtp.traffic_class != ATM_NONE)
+		    vcc->vpi == vpi && vcc->qos.rxtp.traffic_class != ATM_NONE &&
+		    test_bit(ATM_VF_READY, &vcc->flags))
 			goto out;
 	}
 	vcc = NULL;
@@ -900,6 +901,10 @@ static void pclose(struct atm_vcc *vcc)
 	clear_bit(ATM_VF_ADDR, &vcc->flags);
 	clear_bit(ATM_VF_READY, &vcc->flags);
 
+	/* Hold up vcc_destroy_socket() (our caller) until solos_bh() in the
+	   tasklet has finished processing any incoming packets (and, more to
+	   the point, using the vcc pointer). */
+	tasklet_unlock_wait(&card->tlet);
 	return;
 }
 

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation


^ permalink raw reply related

* [PATCH net 1/1] Phonet: listening socket lock protects the connected socket list
From: Rémi Denis-Courmont @ 2010-05-26 10:44 UTC (permalink / raw)
  To: netdev

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

The accept()'d socket need to be unhashed while the (listen()'ing)
socket lock is held. This fixes a race condition that could lead to an
OOPS.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 net/phonet/pep.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index 7b048a3..94d72e8 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -1045,12 +1045,12 @@ static void pep_sock_unhash(struct sock *sk)
 	lock_sock(sk);
 	if ((1 << sk->sk_state) & ~(TCPF_CLOSE|TCPF_LISTEN)) {
 		skparent = pn->listener;
-		sk_del_node_init(sk);
 		release_sock(sk);
 
-		sk = skparent;
 		pn = pep_sk(skparent);
-		lock_sock(sk);
+		lock_sock(skparent);
+		sk_del_node_init(sk);
+		sk = skparent;
 	}
 	/* Unhash a listening sock only when it is closed
 	 * and all of its active connected pipes are closed. */
-- 
1.7.0.4


^ permalink raw reply related

* ipv6: Add GSO support on forwarding path
From: Herbert Xu @ 2010-05-26 10:27 UTC (permalink / raw)
  To: Ralf Baechle, David S. Miller, netdev

Hi:

ipv6: Add GSO support on forwarding path

Currently we disallow GSO packets on the IPv6 forward path.
This patch fixes this.

Note that I discovered that our existing GSO MTU checks (e.g.,
IPv4 forwarding) are buggy in that they skip the check altogether, 
hen they really should be checking gso_size instead.

I have also been lazy here in that I haven't bothered to segment
the GSO packet by hand before generating an ICMP message.  Someone
should add that to be 100% correct.

Reported-by: Ralf Baechle <ralf@linux-mips.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 7cdfb4d..64f9c5a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2117,6 +2117,13 @@ static inline int skb_is_gso(const struct sk_buff *skb)
 	return skb_shinfo(skb)->gso_size;
 }
 
+static inline int skb_gso_len(const struct sk_buff *skb)
+{
+	return skb_is_gso(skb) ?
+	       skb_shinfo(skb)->gso_size + skb_transport_offset(skb) :
+	       skb->len;
+}
+
 static inline int skb_is_gso_v6(const struct sk_buff *skb)
 {
 	return skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6;
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index cd963f6..8904767 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -507,7 +507,7 @@ int ip6_forward(struct sk_buff *skb)
 	if (mtu < IPV6_MIN_MTU)
 		mtu = IPV6_MIN_MTU;
 
-	if (skb->len > mtu) {
+	if (skb_gso_len(skb) > mtu) {
 		/* Again, force OUTPUT device used as source address */
 		skb->dev = dst->dev;
 		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply related


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