Netdev List
 help / color / mirror / Atom feed
* Re: [ofa-general] InfiniBand/RDMA merge plans for 2.6.24
From: Jack Morgenstein @ 2007-09-18  7:09 UTC (permalink / raw)
  To: general; +Cc: Roland Dreier, linux-kernel, netdev, tziporet
In-Reply-To: <adahclymos8.fsf@cisco.com>

On Thursday 13 September 2007 20:57, Roland Dreier wrote:
> HW specific:
> 
>  - I already merged patches to enable MSI-X by default for mthca and
>    mlx4.  I hope there aren't too many systems that get hosed if a
>    MSI-X interrupt is generated.
> 
>  - Jack and Michael's mlx4 FMR support.  Will merge I guess, although
>    I do hope to have time to address the DMA API abuse that is being
>    copied from mthca, so that mlx4 and mthca work in Xen domU.
> 
>  - ehca patch queue.  Will merge, pending fixes for the few minor
>    issues I commented on.
> 
>  - Steve's mthca router mode support.  Would be nice to see a review
>    from someone at Mellanox.
> 
>  - Arthur's mthca doorbell alignment fixes.  I will experiment with a
>    few different approaches and post what I like (and fix mlx4 as
>    well).  I hope Arthur can review.
> 
>  - Michael's mlx4 WQE shrinking patch.  Not sure yet; I'll reply to
>    the latest patch directly.
> 
Missing from this list (IMPORTANT patch!):
[ofa-general] [PATCH 2 of 2] IB/mlx4: Handle new FW requirement for send request prefetching, for WQE sg lists
(Posted by me to list on Sept 4)
{patch header: 
This is an addendum to Roland's commit 0e6e74162164d908edf7889ac66dca09e7505745
(June 18). This addendum adds prefetch headroom marking processing for s/g segments.

We write s/g segments in reverse order into the WQE, in order to guarantee
that the first dword of all cachelines containing s/g segments is written last
(overwriting the headroom invalidation pattern). The entire cacheline will thus
contain valid data when the invalidation pattern is overwritten.
}
This patch series (1 of 2 is for libmlx4, the same issue).
============================================================

Also, I'm now posting (in a separate post) the following patch to mlx4, which is important:
  display the following device information via sysfs:
  board_id, fw_ver, hw_rev, hca_type.

  The info is displayed under directory /sys/class/infiniband/mlx4_x, where x is
  the pci bus sequence number (starting from zero).

  This patch makes information available to ibstat and ibv_devinfo under the
  same directory as is used for tavor/arbel/sinai -- thus requiring no userspace
  modifications.

- Jack



^ permalink raw reply

* Re: [PATCH 7/7] CAN: Add documentation
From: Urs Thuermann @ 2007-09-18  7:20 UTC (permalink / raw)
  To: Bill Fink
  Cc: Thomas Gleixner, Randy Dunlap, netdev, David Miller,
	Patrick McHardy, Oliver Hartkopp, Oliver Hartkopp, Urs Thuermann
In-Reply-To: <20070918025144.71dcb85b.billfink@mindspring.com>

Bill Fink <billfink@mindspring.com> writes:

> One more typo.

> "decive" -> "device" above.

Thank you.  It's fixed now.

urs

^ permalink raw reply

* Re: [RFC][PATCH] trivial typo correction in net/ipv4/xfrm4_policy.c
From: Ian Brown @ 2007-09-18  7:46 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20070917.095919.48528018.davem@davemloft.net>

Hello,

Now it seems to me ok and I hope it can be applied cleanly. The only
change here is  "Copy neighbour" instead "Copy neighbout"  in the
comment in net/ipv4/xfrm4_policy.c

Regards,
Ian Brown

Signed-off-by: ianbrn@gmail.com


--- a/net/ipv4/xfrm4_policy.c	2007-09-01 09:08:24.000000000 +0300
+++ b/net/ipv4/xfrm4_policy.c	2007-09-17 09:36:04.000000000 +0200
@@ -166,7 +166,7 @@
 		dst_prev->trailer_len	= trailer_len;
 		memcpy(&dst_prev->metrics, &x->route->metrics, sizeof(dst_prev->metrics));

-		/* Copy neighbout for reachability confirmation */
+		/* Copy neighbour for reachability confirmation */
 		dst_prev->neighbour	= neigh_clone(rt->u.dst.neighbour);
 		dst_prev->input		= rt->u.dst.input;
 		/* XXX: When IPv6 module can be unloaded, we should manage reference



On 9/17/07, David Miller <davem@davemloft.net> wrote:
> From: "Ian Brown" <ianbrn@gmail.com>
> Date: Mon, 17 Sep 2007 09:50:15 +0200
>
> > Hello,
> >   This is a  trivial typo correction in net/ipv4/xfrm4_policy.c which
> > hunted my eye...
> >
> > Regards,
> > Ian Brown
> >
> > Signed-off-by: ianbrn@gmail.com
>
> Your mail client word-wrapped the patch so it doesn't apply
> cleanly, please report with full changelog et al. after
> you've corrected this.
>
> Thanks.
>

^ permalink raw reply

* Re: e1000 driver and samba
From: Urs Thuermann @ 2007-09-18  7:45 UTC (permalink / raw)
  To: Bill Fink; +Cc: Brandeburg, Jesse, L F, Kok, Auke-jan H, James Chapman, netdev
In-Reply-To: <20070918020358.d50653d5.billfink@mindspring.com>

Bill Fink <billfink@mindspring.com> writes:

> It may also be a useful test to disable hardware TSO support
> via "ethtool -K ethX tso off".

All suggestions here on the list, i.e. checking for flow control,
duplex, cable problems, etc. don't explain (at least to me) why LF
sees file corruption.  How can a corrupted frame pass the TCP checksum
check?  Does TCP use the hardware checksum of the NIC if available?
AFAICS, this would be the only way for a corrupt frame to make it into
the file.  But Bill already suggested this and LF reported that it
didn't make a difference.

A few months ago I had hadware problems with an embedded device, where
transmission from the NIC via the PCI bus to the CPU had some bits
flipped.  But tcpdump clearly showed the TCP checksum errors and also
TCP recognized the errors and the connection was stalled.  And, BTW,
we also observed an increasing percentage of corrupted frames with
increasing traffic on that interface, i.e. increasing load on the PCI
bus.

So I would run tcpdump -s0 and watch for "incorrect checksum" messages.


urs

^ permalink raw reply

* wrt Age Entry For IPv4 & IPv6 Route Table
From: Varun Chandramohan @ 2007-09-18  8:00 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Varun Chandramohan

Hi Dave,

                  Sorry for bothering you. I see that you are currently
very busy. I just wanted to remind you that iam waiting for your
feedback on the patch set. Just incase you want me to resend , please
let me know. If this work is already in your queue, ignore this mail.

Regards,
Varun

^ permalink raw reply

* [PATCH][NETNS] Cleanup list walking in setup_net and cleanup_net
From: Pavel Emelyanov @ 2007-09-18  8:06 UTC (permalink / raw)
  To: David Miller; +Cc: Eric W. Biederman, Linux Netdev List, devel

I proposed introducing a list_for_each_entry_continue_reverse macro
to be used in setup_net() when unrolling the failed ->init callback.

Here is the macro and some more cleanup in the setup_net() itself
to remove one variable from the stack :) The same thing is for the
cleanup_net() - the existing list_for_each_entry_reverse() is used.

Minor, but the code looks nicer.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

---

diff --git a/include/linux/list.h b/include/linux/list.h
index f29fc9c..ad9dcb9 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -525,6 +525,20 @@ static inline void list_splice_init_rcu(
 	     pos = list_entry(pos->member.next, typeof(*pos), member))
 
 /**
+ * list_for_each_entry_continue_reverse - iterate backwards from the given point
+ * @pos:	the type * to use as a loop cursor.
+ * @head:	the head for your list.
+ * @member:	the name of the list_struct within the struct.
+ *
+ * Start to iterate over list of given type backwards, continuing after
+ * the current position.
+ */
+#define list_for_each_entry_continue_reverse(pos, head, member)		\
+	for (pos = list_entry(pos->member.prev, typeof(*pos), member);	\
+	     prefetch(pos->member.prev), &pos->member != (head);	\
+	     pos = list_entry(pos->member.prev, typeof(*pos), member))
+
+/**
  * list_for_each_entry_from - iterate over list of given type from the current point
  * @pos:	the type * to use as a loop cursor.
  * @head:	the head for your list.
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 1fc513c..0e6cb02 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -56,7 +56,6 @@ static void net_free(struct net *net)
 static void cleanup_net(struct work_struct *work)
 {
 	struct pernet_operations *ops;
-	struct list_head *ptr;
 	struct net *net;
 
 	net = container_of(work, struct net, work);
@@ -69,8 +68,7 @@ static void cleanup_net(struct work_stru
 	net_unlock();
 
 	/* Run all of the network namespace exit methods */
-	list_for_each_prev(ptr, &pernet_list) {
-		ops = list_entry(ptr, struct pernet_operations, list);
+	list_for_each_entry_reverse(ops, &pernet_list, list) {
 		if (ops->exit)
 			ops->exit(net);
 	}
@@ -102,7 +100,6 @@ static int setup_net(struct net *net)
 {
 	/* Must be called with net_mutex held */
 	struct pernet_operations *ops;
-	struct list_head *ptr;
 	int error;
 
 	memset(net, 0, sizeof(struct net));
@@ -110,8 +107,7 @@ static int setup_net(struct net *net)
 	atomic_set(&net->use_count, 0);
 
 	error = 0;
-	list_for_each(ptr, &pernet_list) {
-		ops = list_entry(ptr, struct pernet_operations, list);
+	list_for_each_entry(ops, &pernet_list, list) {
 		if (ops->init) {
 			error = ops->init(net);
 			if (error < 0)
@@ -120,12 +116,12 @@ static int setup_net(struct net *net)
 	}
 out:
 	return error;
+
 out_undo:
 	/* Walk through the list backwards calling the exit functions
 	 * for the pernet modules whose init functions did not fail.
 	 */
-	for (ptr = ptr->prev; ptr != &pernet_list; ptr = ptr->prev) {
-		ops = list_entry(ptr, struct pernet_operations, list);
+	list_for_each_entry_continue_reverse(ops, &pernet_list, list) {
 		if (ops->exit)
 			ops->exit(net);
 	}

^ permalink raw reply related

* Re: e1000 driver and samba
From: Bill Fink @ 2007-09-18  8:47 UTC (permalink / raw)
  To: Urs Thuermann
  Cc: Brandeburg, Jesse, L F, Kok, Auke-jan H, James Chapman, netdev
In-Reply-To: <ygfps0g8lj0.fsf@janus.isnogud.escape.de>

On 18 Sep 2007, Urs Thuermann wrote:

> Bill Fink <billfink@mindspring.com> writes:
> 
> > It may also be a useful test to disable hardware TSO support
> > via "ethtool -K ethX tso off".
> 
> All suggestions here on the list, i.e. checking for flow control,
> duplex, cable problems, etc. don't explain (at least to me) why LF
> sees file corruption.  How can a corrupted frame pass the TCP checksum
> check?  Does TCP use the hardware checksum of the NIC if available?
> AFAICS, this would be the only way for a corrupt frame to make it into
> the file.  But Bill already suggested this and LF reported that it
> didn't make a difference.
> 
> A few months ago I had hadware problems with an embedded device, where
> transmission from the NIC via the PCI bus to the CPU had some bits
> flipped.  But tcpdump clearly showed the TCP checksum errors and also
> TCP recognized the errors and the connection was stalled.  And, BTW,
> we also observed an increasing percentage of corrupted frames with
> increasing traffic on that interface, i.e. increasing load on the PCI
> bus.
> 
> So I would run tcpdump -s0 and watch for "incorrect checksum" messages.

I agree TSO is an unlikely candidate since it should only affect
transmits and the problem as I understand it is with receives.
But still one of the first things I try doing when dealing with
weird problems is disabling all hardware assists.

But I also agree with you that network errors should normally be
detected by the TCP checksum (unless hardware checksumming was
messed up), and from what I recall there were no receive checksum
errors being seen.  That and the fact that the problem was seen
with two different NICs would lead me to believe that the problem
is elsewhere in the system.

That leaves many possibilities.  It could be a memory problem,
although it was indicated that memory testing was successfully
performed (but we don't know how extensive the memory checking
is enabled via the BIOS).  It could be the PCI bus writes back
to the disk, or a problem with the disk/controller/fs writes
themselves (some kind of disk stress test might be useful).

						-Bill

^ permalink raw reply

* RE: [PATCH] ucc_geth: fix compilation
From: Li Yang-r58472 @ 2007-09-18  8:56 UTC (permalink / raw)
  To: Kumar Gala, Jeff Garzik; +Cc: linuxppc-dev, netdev
In-Reply-To: <C844988E-F6B9-44B9-AB78-C44E19A2E605@kernel.crashing.org>

> -----Original Message-----
> From: linuxppc-dev-bounces+leoli=freescale.com@ozlabs.org 
> [mailto:linuxppc-dev-bounces+leoli=freescale.com@ozlabs.org] 
> On Behalf Of Kumar Gala
> Sent: Friday, September 14, 2007 10:08 PM
> To: Jeff Garzik
> Cc: linuxppc-dev@ozlabs.org list; netdev
> Subject: Re: [PATCH] ucc_geth: fix compilation
> 
> 
> On Sep 13, 2007, at 10:23 AM, Anton Vorontsov wrote:
> 
> > Currently qe_bd_t is used in the macro call -- 
> dma_unmap_single, which 
> > is a no-op on PPC32, thus error is hidden today. Starting 
> with 2.6.24, 
> > macro will be replaced by the empty static function, and 
> erroneous use 
> > of qe_bd_t will trigger compilation error.
> >
> > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> > ---
> 
> Jeff, I'm going to pick this up via the powerpc.git tree 
> since its currently only broken in our for-2.6.24 branch 
> (because of other changes in there).  Any issues?

Kumar,

Kim Phillips has posted the same patch to netdev list before Anton.
Therefore, I do prefer to use his.  Thanks

- Leo

^ permalink raw reply

* Re: [ofa-general] InfiniBand/RDMA merge plans for 2.6.24
From: Tziporet Koren @ 2007-09-18  9:48 UTC (permalink / raw)
  To: Hal Rosenstock
  Cc: Sean Hefty, netdev, 'Roland Dreier', linux-kernel,
	general
In-Reply-To: <1190034015.6272.83.camel@hrosenstock-ws.xsigo.com>

Hal Rosenstock wrote:
>
> Has anyone tested these with QoS actually be used ? I suppose this
> requires Connect-X.
>   

You can test it with a switch without ConnectX.
If you want that the HCA will react to the QoS setting too then you 
should have ConnectX



Tziporet

^ permalink raw reply

* Re: [PATCH] tehuti: driver for Tehuti 10GbE network adapters
From: Christoph Hellwig @ 2007-09-18  9:47 UTC (permalink / raw)
  To: Andy Gospodarek; +Cc: jeff, davem, akpm, netdev, baum
In-Reply-To: <20070911164033.GA3754@gospo.rdu.redhat.com>


Some comment from looking at the driver in the -mm tree:

 - please kill the CPU_CHIP_SWAP macros and use the normal linux
   cpu_to_le*/le*_to_cpu and verify them using sparse.
   See Documentation/sparse.txt on how to do that
 - please include the linux header in the .c file, not the .h
 - please don't redefine the dma mask constants
 - please use the firmware loader instead of mebedding a firmware
   image
 - please don't invent your own debugging macros but use
   dev_dbg and friends
 - please kill the ENTER/RET macros
 - please kill BDX_ASSERT
 - the unregister_netdev directly followed by free_netdev in
   bdx_remove look buggy, but I'm not entirely sure how to handle
   multi-port devices properly here.
 - please declare bdx_ethtool_ops on file-scope and kill
   bdx_ethtool_ops
 - please don't put assignments into conditionals ala

	if ((err = pci_request_regions(pdev, BDX_DRV_NAME)))
		goto err_dma;

   but rather write

	err = pci_request_regions(pdev, BDX_DRV_NAME);
	if (err)
		goto err_dma;

^ permalink raw reply

* ipsec: icmp fragmentation-needed from ipsec-gateway is not encrypted
From: Wolfgang Walter @ 2007-09-18 10:57 UTC (permalink / raw)
  To: netdev; +Cc: herbert, davem

Hello,

I have the following problem:

router A has two interfaces eth0 and eth1.

router B has two interfaces eth0 and eth1.

The networks on A:eth1 and B:eth1 are connected over an ipsec-tunnel.

the mtu on A:eth1 is 1400 (all others are 1500).

both run 2.6.22.6

If I now ping a host HA on A:eth1 from host HB on B:eth1 with packet size 
greater 1400 the ping fails.

tcpdump on A:eth0 shows

an esp-tunnel-packet from B comes in
icmp echo-request packet from HB to HA comes in
        (the decrypted esp-packet)
an unecrypted icmp fragmentation-needed packet to HB from A (ip of eth1) sent 
out

It seems to me that this fragementation-needed packet generated by B is not 
handled by ipsec, is sent out unencrypted instead and this is the reason it 
does not reach HB.

I should not see the unecrypted packet going out at all? Because if I ping 
A:eth1 from HB then I don't see the unencrypted echo-reply packet (which has 
the same source-address as the fragmentation needed) but only the outgoing 
esp-packet (and the echo-reply reaches HB, by the way).

Regards,
-- 
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts

^ permalink raw reply

* Re: why does tcp_v[46]_conn_request not inc MIB stats
From: Andi Kleen @ 2007-09-18 11:07 UTC (permalink / raw)
  To: David Miller; +Cc: sri, rick.jones2, netdev
In-Reply-To: <20070914.201152.35026256.davem@davemloft.net>

David Miller <davem@davemloft.net> writes:

> This is limiting embryonic mini-socket creation.  The listen overflow
> should only increment when the 3-way handshake completion is aborted
> because the listening socket limit is exceeded, which is entirely
> different from the embryonic limit.

That's true, but I think Rick has a point in that there should 
be some sort of (different) counter counting this.  In general
I believe every point in the stack who drops a packet should
have a statistics counter so that it can be later diagnosed.

Rick, the best way to get such a counter in is to just send a patch,
don't ask for it.

-Andi

^ permalink raw reply

* [ofa-general] [PATCH 1/2] IPoIB: Fix unregister_netdev hang
From: Krishna Kumar @ 2007-09-18 11:18 UTC (permalink / raw)
  To: rdreier; +Cc: netdev, davem, general

While using IPoIB over EHCA (rc6 bits), unregister_netdev hangs with
the message: "waiting for ib2 to become free.  Usage count = -515276",
etc.

The problem is that the poll handler does netif_rx_complete (which
does a dev_put) followed by netif_rx_reschedule() to schedule for
more receives (which again does a dev_put). This reduces refcount to
< 0 (depending on how many times netif_rx_complete followed by
netif_rx_reschedule was called).

The following patch fixes the bug, but I don't know if there is some
specific IB issue that prevents this approach.

Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
 ipoib_ib.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

diff -ruNp org/drivers/infiniband/ulp/ipoib/ipoib_ib.c new1/drivers/infiniband/ulp/ipoib/ipoib_ib.c
--- org/drivers/infiniband/ulp/ipoib/ipoib_ib.c	2007-09-18 15:50:09.000000000 +0530
+++ new1/drivers/infiniband/ulp/ipoib/ipoib_ib.c	2007-09-18 16:14:20.000000000 +0530
@@ -291,7 +291,6 @@ int ipoib_poll(struct napi_struct *napi,
 
 	done  = 0;
 
-poll_more:
 	while (done < budget) {
 		int max = (budget - done);
 
@@ -316,12 +315,10 @@ poll_more:
 	}
 
 	if (done < budget) {
-		netif_rx_complete(dev, napi);
-		if (unlikely(ib_req_notify_cq(priv->cq,
-					      IB_CQ_NEXT_COMP |
-					      IB_CQ_REPORT_MISSED_EVENTS)) &&
-		    netif_rx_reschedule(napi))
-			goto poll_more;
+		if (likely(!ib_req_notify_cq(priv->cq,
+					     IB_CQ_NEXT_COMP |
+					     IB_CQ_REPORT_MISSED_EVENTS)))
+			netif_rx_complete(dev, napi);
 	}
 
 	return done;

^ permalink raw reply

* [ofa-general] [PATCH 2/2] IPoIB: Code cleanup
From: Krishna Kumar @ 2007-09-18 11:18 UTC (permalink / raw)
  To: rdreier; +Cc: netdev, davem, general
In-Reply-To: <20070918111803.1769.60619.sendpatchset@localhost.localdomain>

Follow-up cleanup and "while loop" optimization in the poll handler.
net_rx_action guarantees that 'budget' is atleast 1.

Note: This could also be done for poll handlers of other drivers.

Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
 ipoib_ib.c |   22 ++++++++--------------
 1 files changed, 8 insertions(+), 14 deletions(-)

diff -ruNp new1/drivers/infiniband/ulp/ipoib/ipoib_ib.c new2/drivers/infiniband/ulp/ipoib/ipoib_ib.c
--- new1/drivers/infiniband/ulp/ipoib/ipoib_ib.c	2007-09-18 16:14:20.000000000 +0530
+++ new2/drivers/infiniband/ulp/ipoib/ipoib_ib.c	2007-09-18 16:31:42.000000000 +0530
@@ -285,19 +285,16 @@ int ipoib_poll(struct napi_struct *napi,
 {
 	struct ipoib_dev_priv *priv = container_of(napi, struct ipoib_dev_priv, napi);
 	struct net_device *dev = priv->dev;
-	int done;
-	int t;
-	int n, i;
+	int num_wc, max_wc;
+	int done = 0;
 
-	done  = 0;
-
-	while (done < budget) {
-		int max = (budget - done);
+	do {
+		int i;
 
-		t = min(IPOIB_NUM_WC, max);
-		n = ib_poll_cq(priv->cq, t, priv->ibwc);
+		max_wc = min(IPOIB_NUM_WC, budget - done);
+		num_wc = ib_poll_cq(priv->cq, max_wc, priv->ibwc);
 
-		for (i = 0; i < n; i++) {
+		for (i = 0; i < num_wc; i++) {
 			struct ib_wc *wc = priv->ibwc + i;
 
 			if (wc->wr_id & IPOIB_CM_OP_SRQ) {
@@ -309,10 +306,7 @@ int ipoib_poll(struct napi_struct *napi,
 			} else
 				ipoib_ib_handle_tx_wc(dev, wc);
 		}
-
-		if (n != t)
-			break;
-	}
+	} while (num_wc == max_wc && done < budget);
 
 	if (done < budget) {
 		if (likely(!ib_req_notify_cq(priv->cq,

^ permalink raw reply

* [ofa-general] [PATCH] IPoIB: Optimizations in poll handler.
From: Krishna Kumar @ 2007-09-18 11:39 UTC (permalink / raw)
  To: rdreier; +Cc: netdev, davem, general

Final follow-up optimizations: If the poll loop executes more than
once (and it happens on my system with two flood pings):
	- no need to calculate "budget - done" on every iteration (but
		will require to do this once, when returning from fn)
	- check for one variable being non-zero instead of comparing two
		vars for every iteration.

Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
 ipoib_ib.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff -ruNp new2/drivers/infiniband/ulp/ipoib/ipoib_ib.c new3/drivers/infiniband/ulp/ipoib/ipoib_ib.c
--- new2/drivers/infiniband/ulp/ipoib/ipoib_ib.c	2007-09-18 16:31:42.000000000 +0530
+++ new3/drivers/infiniband/ulp/ipoib/ipoib_ib.c	2007-09-18 17:01:44.000000000 +0530
@@ -286,36 +286,37 @@ int ipoib_poll(struct napi_struct *napi,
 	struct ipoib_dev_priv *priv = container_of(napi, struct ipoib_dev_priv, napi);
 	struct net_device *dev = priv->dev;
 	int num_wc, max_wc;
-	int done = 0;
+	int remaining = budget;
 
 	do {
 		int i;
 
-		max_wc = min(IPOIB_NUM_WC, budget - done);
+		max_wc = min(IPOIB_NUM_WC, remaining);
 		num_wc = ib_poll_cq(priv->cq, max_wc, priv->ibwc);
 
 		for (i = 0; i < num_wc; i++) {
 			struct ib_wc *wc = priv->ibwc + i;
 
 			if (wc->wr_id & IPOIB_CM_OP_SRQ) {
-				++done;
+				--remaining;
 				ipoib_cm_handle_rx_wc(dev, wc);
 			} else if (wc->wr_id & IPOIB_OP_RECV) {
-				++done;
+				--remaining;
 				ipoib_ib_handle_rx_wc(dev, wc);
 			} else
 				ipoib_ib_handle_tx_wc(dev, wc);
 		}
-	} while (num_wc == max_wc && done < budget);
+	} while (num_wc == max_wc && remaining);
 
-	if (done < budget) {
+	if (remaining) {
 		if (likely(!ib_req_notify_cq(priv->cq,
 					     IB_CQ_NEXT_COMP |
 					     IB_CQ_REPORT_MISSED_EVENTS)))
 			netif_rx_complete(dev, napi);
 	}
 
-	return done;
+	/* return number of receives processed */
+	return budget - remaining;
 }
 
 void ipoib_ib_completion(struct ib_cq *cq, void *dev_ptr)

^ permalink raw reply

* Problems with 3c59x driver (extremely low throughput)
From: Mikael Leiviskä @ 2007-09-18 11:32 UTC (permalink / raw)
  To: netdev

OK first of all I'm not really sure if this is the place but was told by
 Dave Dillow to send to the netdev kernel mailing list. So that's what
I'm doing :)

I tried searching the mailing lists and didn't find anything that seemed
relevant, it is possible I missed something the archives are huge...

Following is the mail I sent originally:

I've run into something of a strange problem lately, I have a network
card that runs on the Vortex/Boomerang driver. I ran the 2.6.16 kernel
without any issues, and later switched to 2.6.22 because I couldn't get
my IPSEC tunnel working with 2.6.16 for some reason (didn't find the
right switches in menuconfig). And everything was seemingly working fine
but soon after my ISP got some problems and they said they had to
replace some hardware somewhere along my route and now when I access the
internet (the machine is setup as router/web-file-mail-server with this
card as the external interface) I get dreadfully slow speeds, I'll be
lucky to get 10KB/s on a 10MB/s link. But the funny thing is if I switch
to the 2.6.16 kernel, WHAM I get 700KB/s no sweat... The kernel
configurations are otherwise almost exactly the same bar tickless and
some switches in network options to get ipsec... The only thing I do is
change the kernel and things work...

So my questions are,
Has there been any recent changes to the drivers for the card lately?
(between .16 and .22)
Or any changes in the network infrastructure that could be the cause of it?
Or is there some other switch in the config that could be the cause of this?
Or is it my ISP's hardware that's not playing nice?
Or any other ideas as to what might be the cause.

I've posted the respective kernel configs, and lspci output and my
cpuinfo on http://www.zero-kelvin.org/pub/kern/

If there is any other info that you need I'll gladly provide it assuming
I can get my hands on it   :)

Oh btw, the affected card is:
00:10.0 Ethernet controller: 3Com Corporation 3c905C-TX/TX-M [Tornado]
(rev 74)

Kind regards, Mikael Leiviskä


^ permalink raw reply

* [0/3] [PPP]: Fix pppol2tp skb bugs
From: Herbert Xu @ 2007-09-18 12:04 UTC (permalink / raw)
  To: James Chapman, David S. Miller, Michal Ostrowski, Paul Mackerras
  Cc: Toralf Förster, netdev
In-Reply-To: <20070831090625.GA28175@gondor.apana.org.au>

On Fri, Aug 31, 2007 at 05:06:25PM +0800, Herbert Xu wrote:
> 
> I've audited ppp_generic.c and pppoe.c.  I'll do pppol2tp
> tomorrow.

Tomrrow took a while to come :)

Here are the fixes for pppol2tp.

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

* [PATCH 1/3] [PPP] L2TP: Disallow non-UDP datagram sockets
From: Herbert Xu @ 2007-09-18 12:08 UTC (permalink / raw)
  To: James Chapman, David S. Miller, Michal Ostrowski, Paul Mackerras,
	Herbert Xu
In-Reply-To: <20070918120438.GA9121@gondor.apana.org.au>

[PPP] L2TP: Disallow non-UDP datagram sockets

With the addition of UDP-Lite we need to refine the socket check so that
only genuine UDP sockets are allowed through.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 drivers/net/pppol2tp.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c
index 266e8b3..ed8ead4 100644
--- a/drivers/net/pppol2tp.c
+++ b/drivers/net/pppol2tp.c
@@ -1326,12 +1326,14 @@ static struct sock *pppol2tp_prepare_tunnel_socket(int fd, u16 tunnel_id,
 		goto err;
 	}
 
+	sk = sock->sk;
+
 	/* Quick sanity checks */
-	err = -ESOCKTNOSUPPORT;
-	if (sock->type != SOCK_DGRAM) {
+	err = -EPROTONOSUPPORT;
+	if (sk->sk_protocol != IPPROTO_UDP) {
 		PRINTK(-1, PPPOL2TP_MSG_CONTROL, KERN_ERR,
-		       "tunl %hu: fd %d wrong type, got %d, expected %d\n",
-		       tunnel_id, fd, sock->type, SOCK_DGRAM);
+		       "tunl %hu: fd %d wrong protocol, got %d, expected %d\n",
+		       tunnel_id, fd, sk->sk_protocol, IPPROTO_UDP);
 		goto err;
 	}
 	err = -EAFNOSUPPORT;
@@ -1343,7 +1345,6 @@ static struct sock *pppol2tp_prepare_tunnel_socket(int fd, u16 tunnel_id,
 	}
 
 	err = -ENOTCONN;
-	sk = sock->sk;
 
 	/* Check if this socket has already been prepped */
 	tunnel = (struct pppol2tp_tunnel *)sk->sk_user_data;

^ permalink raw reply related

* [PATCH 2/3] [PPP] L2TP: Fix skb handling in pppol2tp_recv_core
From: Herbert Xu @ 2007-09-18 12:08 UTC (permalink / raw)
  To: James Chapman, David S. Miller, Michal Ostrowski, Paul Mackerras,
	Herbert Xu
In-Reply-To: <20070918120438.GA9121@gondor.apana.org.au>

[PPP] L2TP: Fix skb handling in pppol2tp_recv_core

The function pppol2tp_recv_core doesn't handle non-linear packets properly.
It also fails to check the remote offset field.

This patch fixes these problems.  It also removes an unnecessary check on
the UDP header which has already been performed by the UDP layer.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 drivers/net/pppol2tp.c |   44 ++++++++++++++++++++++++--------------------
 1 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c
index ed8ead4..440e190 100644
--- a/drivers/net/pppol2tp.c
+++ b/drivers/net/pppol2tp.c
@@ -491,44 +491,46 @@ static int pppol2tp_recv_core(struct sock *sock, struct sk_buff *skb)
 	u16 hdrflags;
 	u16 tunnel_id, session_id;
 	int length;
-	struct udphdr *uh;
+	int offset;
 
 	tunnel = pppol2tp_sock_to_tunnel(sock);
 	if (tunnel == NULL)
 		goto error;
 
+	/* UDP always verifies the packet length. */
+	__skb_pull(skb, sizeof(struct udphdr));
+
 	/* Short packet? */
-	if (skb->len < sizeof(struct udphdr)) {
+	if (!pskb_may_pull(skb, 12)) {
 		PRINTK(tunnel->debug, PPPOL2TP_MSG_DATA, KERN_INFO,
 		       "%s: recv short packet (len=%d)\n", tunnel->name, skb->len);
 		goto error;
 	}
 
 	/* Point to L2TP header */
-	ptr = skb->data + sizeof(struct udphdr);
+	ptr = skb->data;
 
 	/* Get L2TP header flags */
 	hdrflags = ntohs(*(__be16*)ptr);
 
 	/* Trace packet contents, if enabled */
 	if (tunnel->debug & PPPOL2TP_MSG_DATA) {
+		length = min(16u, skb->len);
+		if (!pskb_may_pull(skb, length))
+			goto error;
+
 		printk(KERN_DEBUG "%s: recv: ", tunnel->name);
 
-		for (length = 0; length < 16; length++)
-			printk(" %02X", ptr[length]);
+		offset = 0;
+		do {
+			printk(" %02X", ptr[offset]);
+		} while (++offset < length);
+
 		printk("\n");
 	}
 
 	/* Get length of L2TP packet */
-	uh = (struct udphdr *) skb_transport_header(skb);
-	length = ntohs(uh->len) - sizeof(struct udphdr);
-
-	/* Too short? */
-	if (length < 12) {
-		PRINTK(tunnel->debug, PPPOL2TP_MSG_DATA, KERN_INFO,
-		       "%s: recv short L2TP packet (len=%d)\n", tunnel->name, length);
-		goto error;
-	}
+	length = skb->len;
 
 	/* If type is control packet, it is handled by userspace. */
 	if (hdrflags & L2TP_HDRFLAG_T) {
@@ -606,7 +608,6 @@ static int pppol2tp_recv_core(struct sock *sock, struct sk_buff *skb)
 			       "%s: recv data has no seq numbers when required. "
 			       "Discarding\n", session->name);
 			session->stats.rx_seq_discards++;
-			session->stats.rx_errors++;
 			goto discard;
 		}
 
@@ -625,7 +626,6 @@ static int pppol2tp_recv_core(struct sock *sock, struct sk_buff *skb)
 			       "%s: recv data has no seq numbers when required. "
 			       "Discarding\n", session->name);
 			session->stats.rx_seq_discards++;
-			session->stats.rx_errors++;
 			goto discard;
 		}
 
@@ -634,10 +634,14 @@ static int pppol2tp_recv_core(struct sock *sock, struct sk_buff *skb)
 	}
 
 	/* If offset bit set, skip it. */
-	if (hdrflags & L2TP_HDRFLAG_O)
-		ptr += 2 + ntohs(*(__be16 *) ptr);
+	if (hdrflags & L2TP_HDRFLAG_O) {
+		offset = ntohs(*(__be16 *)ptr);
+		skb->transport_header += 2 + offset;
+		if (!pskb_may_pull(skb, skb_transport_offset(skb) + 2))
+			goto discard;
+	}
 
-	skb_pull(skb, ptr - skb->data);
+	__skb_pull(skb, skb_transport_offset(skb));
 
 	/* Skip PPP header, if present.	 In testing, Microsoft L2TP clients
 	 * don't send the PPP header (PPP header compression enabled), but
@@ -673,7 +677,6 @@ static int pppol2tp_recv_core(struct sock *sock, struct sk_buff *skb)
 			 */
 			if (PPPOL2TP_SKB_CB(skb)->ns != session->nr) {
 				session->stats.rx_seq_discards++;
-				session->stats.rx_errors++;
 				PRINTK(session->debug, PPPOL2TP_MSG_SEQ, KERN_DEBUG,
 				       "%s: oos pkt %hu len %d discarded, "
 				       "waiting for %hu, reorder_q_len=%d\n",
@@ -698,6 +701,7 @@ static int pppol2tp_recv_core(struct sock *sock, struct sk_buff *skb)
 	return 0;
 
 discard:
+	session->stats.rx_errors++;
 	kfree_skb(skb);
 	sock_put(session->sock);
 

^ permalink raw reply related

* [PATCH 3/3] [PPP] L2TP: Fix skb handling in pppol2tp_xmit
From: Herbert Xu @ 2007-09-18 12:08 UTC (permalink / raw)
  To: James Chapman, David S. Miller, Michal Ostrowski, Paul Mackerras,
	Herbert Xu
In-Reply-To: <20070918120438.GA9121@gondor.apana.org.au>

[PPP] L2TP: Fix skb handling in pppol2tp_xmit

This patch makes pppol2tp_xmit call skb_cow_head so that we don't modify
cloned skb data.  It also gets rid of skb2 we only need to preserve the
original skb for congestion notification, which is only applicable for
ppp_async and ppp_sync.

The other semantic change made here is the removal of socket accounting
for data tranmitted out of pppol2tp_xmit.  The original code leaked any
existing socket skb accounting.  We could fix this by dropping the
original skb owner.  However, this is undesirable as the packet has not
physically left the host yet.

In fact, all other tunnels in the kernel do not account skb's passing
through to their own socket.  In partciular, ESP over UDP does not do
so and it is the closest tunnel type to PPPoL2TP.  So this patch simply
removes the socket accounting in pppol2tp_xmit.  The accounting still
applies to control packets of course.

I've also added a reminder that the outgoing checksum here doesn't work.
I suppose existing deployments don't actually enable checksums.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
---

 drivers/net/pppol2tp.c |   60 +++++++++++++++++--------------------------------
 1 files changed, 21 insertions(+), 39 deletions(-)

diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c
index 440e190..e259f45 100644
--- a/drivers/net/pppol2tp.c
+++ b/drivers/net/pppol2tp.c
@@ -962,7 +962,6 @@ static int pppol2tp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
 	int data_len = skb->len;
 	struct inet_sock *inet;
 	__wsum csum = 0;
-	struct sk_buff *skb2 = NULL;
 	struct udphdr *uh;
 	unsigned int len;
 
@@ -993,41 +992,30 @@ static int pppol2tp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
 	 */
 	headroom = NET_SKB_PAD + sizeof(struct iphdr) +
 		sizeof(struct udphdr) + hdr_len + sizeof(ppph);
-	if (skb_headroom(skb) < headroom) {
-		skb2 = skb_realloc_headroom(skb, headroom);
-		if (skb2 == NULL)
-			goto abort;
-	} else
-		skb2 = skb;
-
-	/* Check that the socket has room */
-	if (atomic_read(&sk_tun->sk_wmem_alloc) < sk_tun->sk_sndbuf)
-		skb_set_owner_w(skb2, sk_tun);
-	else
-		goto discard;
+	if (skb_cow_head(skb, headroom))
+		goto abort;
 
 	/* Setup PPP header */
-	skb_push(skb2, sizeof(ppph));
-	skb2->data[0] = ppph[0];
-	skb2->data[1] = ppph[1];
+	__skb_push(skb, sizeof(ppph));
+	skb->data[0] = ppph[0];
+	skb->data[1] = ppph[1];
 
 	/* Setup L2TP header */
-	skb_push(skb2, hdr_len);
-	pppol2tp_build_l2tp_header(session, skb2->data);
+	pppol2tp_build_l2tp_header(session, __skb_push(skb, hdr_len));
 
 	/* Setup UDP header */
 	inet = inet_sk(sk_tun);
-	skb_push(skb2, sizeof(struct udphdr));
-	skb_reset_transport_header(skb2);
-	uh = (struct udphdr *) skb2->data;
+	__skb_push(skb, sizeof(*uh));
+	skb_reset_transport_header(skb);
+	uh = udp_hdr(skb);
 	uh->source = inet->sport;
 	uh->dest = inet->dport;
 	uh->len = htons(sizeof(struct udphdr) + hdr_len + sizeof(ppph) + data_len);
 	uh->check = 0;
 
-	/* Calculate UDP checksum if configured to do so */
+	/* *BROKEN* Calculate UDP checksum if configured to do so */
 	if (sk_tun->sk_no_check != UDP_CSUM_NOXMIT)
-		csum = udp_csum_outgoing(sk_tun, skb2);
+		csum = udp_csum_outgoing(sk_tun, skb);
 
 	/* Debug */
 	if (session->send_seq)
@@ -1040,7 +1028,7 @@ static int pppol2tp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
 
 	if (session->debug & PPPOL2TP_MSG_DATA) {
 		int i;
-		unsigned char *datap = skb2->data;
+		unsigned char *datap = skb->data;
 
 		printk(KERN_DEBUG "%s: xmit:", session->name);
 		for (i = 0; i < data_len; i++) {
@@ -1053,18 +1041,18 @@ static int pppol2tp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
 		printk("\n");
 	}
 
-	memset(&(IPCB(skb2)->opt), 0, sizeof(IPCB(skb2)->opt));
-	IPCB(skb2)->flags &= ~(IPSKB_XFRM_TUNNEL_SIZE | IPSKB_XFRM_TRANSFORMED |
-			       IPSKB_REROUTED);
-	nf_reset(skb2);
+	memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
+	IPCB(skb)->flags &= ~(IPSKB_XFRM_TUNNEL_SIZE | IPSKB_XFRM_TRANSFORMED |
+			      IPSKB_REROUTED);
+	nf_reset(skb);
 
 	/* Get routing info from the tunnel socket */
-	dst_release(skb2->dst);
-	skb2->dst = sk_dst_get(sk_tun);
+	dst_release(skb->dst);
+	skb->dst = sk_dst_get(sk_tun);
 
 	/* Queue the packet to IP for output */
-	len = skb2->len;
-	rc = ip_queue_xmit(skb2, 1);
+	len = skb->len;
+	rc = ip_queue_xmit(skb, 1);
 
 	/* Update stats */
 	if (rc >= 0) {
@@ -1078,16 +1066,10 @@ static int pppol2tp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
 	}
 
 	/* Free the original skb */
+abort:
 	kfree_skb(skb);
 
 	return 1;
-
-discard:
-	/* Free the new skb. Caller will free original skb. */
-	if (skb2 != skb)
-		kfree_skb(skb2);
-abort:
-	return 0;
 }
 
 /*****************************************************************************

^ permalink raw reply related

* Re: net-2.6.24 plans
From: John W. Linville @ 2007-09-18  3:23 UTC (permalink / raw)
  To: Andrew Morton; +Cc: David Miller, netdev, jgarzik
In-Reply-To: <20070917164929.b53ac118.akpm@linux-foundation.org>

On Mon, Sep 17, 2007 at 04:49:29PM -0700, Andrew Morton wrote:
> On Mon, 17 Sep 2007 19:18:30 -0400
> "John W. Linville" <linville@tuxdriver.com> wrote:
> 
> > P.S.  Andrew, I'll send you a link to a new git-wireless.patch --
> > I'm sure you don't want a complicated git invocation...  Until then,
> > I don't think you should try pulling wireless-dev...
> 
> OK, thanks.
> 
> The stuff I have now does actually compile, although the chances of it
> actually working are epsilon.
> 
> So I'd rather not breathe on it today: I'l shove it out the door as-is
> with a big "this is probably broken" label on it.   I'll at least test
> ipw2200 before doing so.

Very reasonable -- I suspect even and "easy" -mm looks a lot like
the last few days have looked in my trees...

Anyway, I do have prepared an omnibus patch for you:

	http://www.kernel.org/pub/linux/kernel/people/linville/git-wireless-2007-09-17.patch

And FWIW, there is an 'mm-master' branch matching the usage to which
you are accustomed.  The git merge if you pull from that branch onto
net-2.6.24 is reasonably straight-forward and not too painful.  Still,
I'd recommend the patch at the URL above... :-)

Thanks,

John
-- 
John W. Linville
linville@tuxdriver.com

^ permalink raw reply

* Re: net-2.6.24 plans
From: John W. Linville @ 2007-09-18  3:20 UTC (permalink / raw)
  To: David Miller; +Cc: akpm, netdev, jgarzik, andy, romieu
In-Reply-To: <20070917.185157.26533038.davem@davemloft.net>

On Mon, Sep 17, 2007 at 06:51:57PM -0700, David Miller wrote:
> From: Andrew Morton <akpm@linux-foundation.org>
> Date: Mon, 17 Sep 2007 16:53:18 -0700
> 
> > The Tehuti driver you should probably pull from the above git tree.
> 
> Ok I added in the Tehuti driver to net-2.6.24 and made sure the
> napi_struct conversion was good too.
> 
> That only leaves the wireless bits :-)

OK, OK...I read you! :-)

I'll send some stuff your way tomorrow...

John
-- 
John W. Linville
linville@tuxdriver.com

^ permalink raw reply

* Re: Problems with 3c59x driver (extremely low throughput)
From: Steffen Klassert @ 2007-09-18 13:28 UTC (permalink / raw)
  To: Mikael Leivisk?; +Cc: netdev
In-Reply-To: <46EFB730.2070201@zero-kelvin.org>

On Tue, Sep 18, 2007 at 01:32:00PM +0200, Mikael Leivisk? wrote:
> OK first of all I'm not really sure if this is the place but was told by
>  Dave Dillow to send to the netdev kernel mailing list. So that's what
> I'm doing :)
> 
> I tried searching the mailing lists and didn't find anything that seemed
> relevant, it is possible I missed something the archives are huge...
> 
> Following is the mail I sent originally:
> 
> I've run into something of a strange problem lately, I have a network
> card that runs on the Vortex/Boomerang driver. I ran the 2.6.16 kernel
> without any issues, and later switched to 2.6.22 because I couldn't get
> my IPSEC tunnel working with 2.6.16 for some reason (didn't find the
> right switches in menuconfig). And everything was seemingly working fine
> but soon after my ISP got some problems and they said they had to
> replace some hardware somewhere along my route and now when I access the
> internet (the machine is setup as router/web-file-mail-server with this
> card as the external interface) I get dreadfully slow speeds, I'll be
> lucky to get 10KB/s on a 10MB/s link. 

Does this mean that the driver worked well with 2.6.22 befor the hardware
changes of your ISP? What did they change?

> But the funny thing is if I switch
> to the 2.6.16 kernel, WHAM I get 700KB/s no sweat... The kernel
> configurations are otherwise almost exactly the same bar tickless and
> some switches in network options to get ipsec... The only thing I do is
> change the kernel and things work...
> 
> So my questions are,
> Has there been any recent changes to the drivers for the card lately?
> (between .16 and .22)

There were some changes, your problem could be related to one of them. 

> Or any changes in the network infrastructure that could be the cause of it?
> Or is there some other switch in the config that could be the cause of this?
> Or is it my ISP's hardware that's not playing nice?
> Or any other ideas as to what might be the cause.
> 
> I've posted the respective kernel configs, and lspci output and my
> cpuinfo on http://www.zero-kelvin.org/pub/kern/
> 
> If there is any other info that you need I'll gladly provide it assuming
> I can get my hands on it   :)

There were some duplex related fixes on 3c59x recently,
could you please try whether your problem persist in 2.6.23-rc6?

Steffen

^ permalink raw reply

* Re: [PATCH 1/7] CAN: Allocate protocol numbers for PF_CAN
From: Patrick McHardy @ 2007-09-18 13:31 UTC (permalink / raw)
  To: Urs Thuermann
  Cc: netdev, David Miller, Thomas Gleixner, Oliver Hartkopp,
	Oliver Hartkopp, Urs Thuermann
In-Reply-To: <20070917100437.18347.1@janus.isnogud.escape.de>

Urs Thuermann wrote:
> This patch adds a protocol/address family number, ARP hardware type,
> ethernet packet type, and a line discipline number for the SocketCAN
> implementation.
> 
> Signed-off-by: Oliver Hartkopp <oliver.hartkopp@volkswagen.de>
> Signed-off-by: Urs Thuermann <urs.thuermann@volkswagen.de>

Acked-by: Patrick McHardy <kaber@trash.net>


^ permalink raw reply

* Re: [PATCH 2/7] CAN: Add PF_CAN core module
From: Patrick McHardy @ 2007-09-18 13:31 UTC (permalink / raw)
  To: Urs Thuermann
  Cc: netdev, David Miller, Thomas Gleixner, Oliver Hartkopp,
	Oliver Hartkopp, Urs Thuermann
In-Reply-To: <20070917100438.18347.2@janus.isnogud.escape.de>

Urs Thuermann wrote:
> This patch adds the CAN core functionality but no protocols or drivers.
> No protocol implementations are included here.  They come as separate
> patches.  Protocol numbers are already in include/linux/can.h.
> 
> Signed-off-by: Oliver Hartkopp <oliver.hartkopp@volkswagen.de>
> Signed-off-by: Urs Thuermann <urs.thuermann@volkswagen.de>
> 


Looks pretty good, please see below for a few comments (mostly minor
nitpicking, a few things that look like real bugs). Nothing that
couldn't be fixed after merging though.

> +++ net-2.6.24/include/linux/can.h	2007-09-17 10:27:09.000000000 +0200
> @@ -0,0 +1,113 @@
> +/*
> + * linux/can.h
> + *
> + * Definitions for CAN networklayer (socket addr / CAN frame / CAN filter)
> + *
> + * Authors: Oliver Hartkopp <oliver.hartkopp@volkswagen.de>
> + *          Urs Thuermann   <urs.thuermann@volkswagen.de>
> + * Copyright (c) 2002-2007 Volkswagen Group Electronic Research
> + * All rights reserved.
> + *
> + * Send feedback to <socketcan-users@lists.berlios.de>
> + *
> + */
> +
> +#ifndef CAN_H
> +#define CAN_H
> +
> +#include <linux/types.h>
> +#include <linux/socket.h>
> +
> +/* controller area network (CAN) kernel definitions */


Is this file used only from within the kernel? If so you could use
the nicer-to-look-at u8/u16/u32 types instead of the double underscored
ones.

> +++ net-2.6.24/include/linux/can/core.h	2007-09-17 11:08:39.000000000 +0200
> @@ -0,0 +1,78 @@
> +
> +extern int can_proto_register(struct can_proto *cp);
> +extern int can_proto_unregister(struct can_proto *cp);


The callers of the unregister function don't check the return code,
and they can't handle errors anyways since they use it in the
module unload path, so making it void seems more appropriate
(and maybe a WARN_ON for the "not-found" case).

> +extern int can_rx_register(struct net_device *dev, canid_t can_id,
> +			   canid_t mask,
> +			   void (*func)(struct sk_buff *, void *),
> +			   void *data, char *ident);
> +
> +extern int can_rx_unregister(struct net_device *dev, canid_t can_id,
> +			     canid_t mask,
> +			     void (*func)(struct sk_buff *, void *),
> +			     void *data);


Same here, none of the callers check the return value and since
they're all declared as void they can't propagate any errors back.

> +++ net-2.6.24/net/can/af_can.c	2007-09-17 11:06:52.000000000 +0200
> @@ -0,0 +1,1002 @@
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kmod.h>
> +#include <linux/slab.h>
> +#include <linux/list.h>
> +#include <linux/spinlock.h>
> +#include <linux/rcupdate.h>
> +#include <linux/uaccess.h>
> +#include <linux/net.h>
> +#include <linux/netdevice.h>
> +#include <linux/socket.h>
> +#include <linux/if_ether.h>
> +#include <linux/if_arp.h>
> +#include <linux/skbuff.h>
> +#include <linux/can.h>
> +#include <linux/can/core.h>
> +#include <net/net_namespace.h>
> +#include <net/sock.h>
> +
> +#include "af_can.h"


It seems most of the things declared in that file are only used within
af_can.c. Might be easier to read the code if you'd just move it over.

> +
> +#define IDENT "core"
> +static __initdata const char banner[] = KERN_INFO
> +	"can: controller area network core (" CAN_VERSION_STRING ")\n";
> +
> +MODULE_DESCRIPTION("Controller Area Network PF_CAN core");
> +MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_AUTHOR("Urs Thuermann <urs.thuermann@volkswagen.de>, "
> +	      "Oliver Hartkopp <oliver.hartkopp@volkswagen.de>");
> +
> +MODULE_ALIAS_NETPROTO(PF_CAN);
> +
> +int stats_timer = 1; /* default: on */


This seems to be only used in af_can.c, so it could be static.
__read_mostly also seems to be approriate. There are a few
more that look like they could be __read_mostly below, but
I'll skip these since you probably know better than me.

> +module_param(stats_timer, int, S_IRUGO);
> +MODULE_PARM_DESC(stats_timer, "enable timer for statistics (default:on)");
> +
> +#ifdef CONFIG_CAN_DEBUG_CORE
> +static int debug;
> +module_param(debug, int, S_IRUGO);
> +MODULE_PARM_DESC(debug, "debug print mask: 1:debug, 2:frames, 4:skbs");
> +#endif
> +
> +HLIST_HEAD(rx_dev_list);


Same here (static).

> +static struct dev_rcv_lists rx_alldev_list;
> +static DEFINE_SPINLOCK(rcv_lists_lock);
> +
> +static struct kmem_cache *rcv_cache __read_mostly;
> +
> +/* table of registered CAN protocols */
> +static struct can_proto *proto_tab[CAN_NPROTO];
> +
> +struct timer_list stattimer; /* timer for statistics update */
> +struct s_stats  stats;       /* packet statistics */
> +struct s_pstats pstats;      /* receive list statistics */
> +
> +/*
> + * af_can socket functions
> + */
> +
> +static int can_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
> +{
> +	struct sock *sk = sock->sk;
> +
> +	switch (cmd) {
> +
> +	case SIOCGSTAMP:
> +		return sock_get_timestamp(sk, (struct timeval __user *)arg);
> +
> +	default:
> +		return -ENOIOCTLCMD;
> +	}
> +}
> +
> +static void can_sock_destruct(struct sock *sk)
> +{
> +	DBG("called for sock %p\n", sk);
> +
> +	skb_queue_purge(&sk->sk_receive_queue);
> +	if (sk->sk_protinfo)
> +		kfree(sk->sk_protinfo);
> +}
> +
> +static int can_create(struct net *net, struct socket *sock, int protocol)
> +{
> +	struct sock *sk;
> +	struct can_proto *cp;
> +	char module_name[sizeof("can-proto-000")];
> +	int ret = 0;
> +
> +	DBG("socket %p, type %d, proto %d\n", sock, sock->type, protocol);
> +
> +	sock->state = SS_UNCONNECTED;
> +
> +	if (protocol < 0 || protocol >= CAN_NPROTO)
> +		return -EINVAL;
> +
> +	DBG("looking up proto %d in proto_tab[]\n", protocol);
> +
> +	/* try to load protocol module, when CONFIG_KMOD is defined */
> +	if (!proto_tab[protocol]) {
> +		sprintf(module_name, "can-proto-%d", protocol);
> +		ret = request_module(module_name);
> +
> +		/*
> +		 * In case of error we only print a message but don't
> +		 * return the error code immediately.  Below we will
> +		 * return -EPROTONOSUPPORT
> +		 */
> +		if (ret == -ENOSYS)
> +			printk(KERN_INFO "can: request_module(%s) not"
> +			       " implemented.\n", module_name);
> +		else if (ret)
> +			printk(KERN_ERR "can: request_module(%s) failed\n",
> +			       module_name);


Both of these printks seem to be user-triggerable, so they should
be rate-limited (or maybe get removed completely/changed to DBG).

> +	}
> +
> +	/* check for success and correct type */
> +	cp = proto_tab[protocol];


What prevents the module from getting unloaded again (and using
a stale pointer)?

> +	if (!cp || cp->type != sock->type)
> +		return -EPROTONOSUPPORT;
> +
> +	if (net != &init_net)
> +		return -EAFNOSUPPORT;


Shouldn't this be done before attempting the module load?

> +
> +	if (cp->capability >= 0 && !capable(cp->capability))
> +		return -EPERM;
> +
> +	sock->ops = cp->ops;
> +
> +	sk = sk_alloc(net, PF_CAN, GFP_KERNEL, cp->prot, 1);
> +	if (!sk)
> +		return -ENOMEM;
> +
> +	sock_init_data(sock, sk);
> +	sk->sk_destruct = can_sock_destruct;
> +
> +	DBG("created sock: %p\n", sk);
> +
> +	if (sk->sk_prot->init)
> +		ret = sk->sk_prot->init(sk);
> +
> +	if (ret) {
> +		/* release sk on errors */
> +		sock_orphan(sk);
> +		sock_put(sk);
> +	}
> +
> +	return ret;
> +}
> +
> +/*
> + * af_can tx path
> + */
> +
> +/**
> + * can_send - transmit a CAN frame (optional with local loopback)
> + * @skb: pointer to socket buffer with CAN frame in data section
> + * @loop: loopback for listeners on local CAN sockets (recommended default!)
> + *
> + * Return:
> + *  0 on success
> + *  -ENETDOWN when the selected interface is down
> + *  -ENOBUFS on full driver queue (see net_xmit_errno())
> + *  -ENOMEM when local loopback failed at calling skb_clone()
> + */
> +int can_send(struct sk_buff *skb, int loop)
> +{
> +	int err;
> +
> +	if (skb->dev->type != ARPHRD_CAN) {
> +		kfree_skb(skb);
> +		return -EPERM;


EPERM doesn't seem like the best fit, but I don't have a better
suggestion myself at the moment.

> +	}
> +
> +	if (!(skb->dev->flags & IFF_UP)) {
> +		kfree_skb(skb);
> +		return -ENETDOWN;
> +	}
> +
> +	skb->protocol = htons(ETH_P_CAN);
> +
> +	if (loop) {
> +		/* local loopback of sent CAN frames */
> +
> +		/* indication for the CAN driver: do loopback */
> +		skb->pkt_type = PACKET_LOOPBACK;
> +
> +		/*
> +		 * The reference to the originating sock may be required
> +		 * by the receiving socket to check whether the frame is
> +		 * its own. Example: can_raw sockopt CAN_RAW_RECV_OWN_MSGS
> +		 * Therefore we have to ensure that skb->sk remains the
> +		 * reference to the originating sock by restoring skb->sk
> +		 * after each skb_clone() or skb_orphan() usage.
> +		 */
> +
> +		if (!(skb->dev->flags & IFF_LOOPBACK)) {
> +			/*
> +			 * If the interface is not capable to do loopback
> +			 * itself, we do it here.
> +			 */
> +			struct sk_buff *newskb = skb_clone(skb, GFP_ATOMIC);
> +
> +			if (!newskb) {
> +				kfree_skb(skb);
> +				return -ENOMEM;
> +			}
> +
> +			newskb->sk = skb->sk;
> +			newskb->ip_summed = CHECKSUM_UNNECESSARY;
> +			newskb->pkt_type = PACKET_BROADCAST;
> +			netif_rx(newskb);


So the intention here is to send the packet to the non-loopback device
and manually loop it, which means sending it twice?

> +		}
> +	} else {
> +		/* indication for the CAN driver: no loopback required */
> +		skb->pkt_type = PACKET_HOST;
> +	}
> +
> +	/* send to netdevice */
> +	err = dev_queue_xmit(skb);
> +	if (err > 0)
> +		err = net_xmit_errno(err);
> +
> +	/* update statistics */
> +	stats.tx_frames++;
> +	stats.tx_frames_delta++;
> +
> +	return err;
> +}
> +EXPORT_SYMBOL(can_send);
> +
> +/*
> + * af_can rx path
> + */
> +
> +static struct dev_rcv_lists *find_dev_rcv_lists(struct net_device *dev)
> +{
> +	struct dev_rcv_lists *d;
> +	struct hlist_node *n;
> +
> +	/*
> +	 * find receive list for this device
> +	 *
> +	 * The hlist_for_each_entry*() macros curse through the list
> +	 * using the pointer variable n and set d to the containing
> +	 * struct in each list iteration.  Therefore, after list
> +	 * iteration, d is unmodified when the list is empty, and it
> +	 * points to last list element, when the list is non-empty
> +	 * but no match in the loop body is found.  I.e. d is *not*
> +	 * NULL when no match is found.  We can, however, use the
> +	 * cursor variable n to decide if a match was found.
> +	 */
> +
> +	hlist_for_each_entry(d, n, &rx_dev_list, list) {


On the receive path you use RCU, so this should be
hlist_for_each_entry_rcu(), no? The bottem half disabling during
addition/removal also seems unnecessary, but I might be missing
something.

> +		if (d->dev == dev)
> +			break;
> +	}
> +
> +	return n ? d : NULL;
> +}
> +
> +static struct hlist_head *find_rcv_list(canid_t *can_id, canid_t *mask,
> +					struct dev_rcv_lists *d)
> +{
> +	canid_t inv = *can_id & CAN_INV_FILTER; /* save flag before masking */
> +
> +	/* filter error frames */
> +	if (*mask & CAN_ERR_FLAG) {
> +		/* clear CAN_ERR_FLAG in list entry */
> +		*mask &= CAN_ERR_MASK;
> +		return &d->rx[RX_ERR];
> +	}
> +
> +	/* ensure valid values in can_mask */
> +	if (*mask & CAN_EFF_FLAG)
> +		*mask &= (CAN_EFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG);
> +	else
> +		*mask &= (CAN_SFF_MASK | CAN_RTR_FLAG);
> +
> +	/* reduce condition testing at receive time */
> +	*can_id &= *mask;
> +
> +	/* inverse can_id/can_mask filter */
> +	if (inv)
> +		return &d->rx[RX_INV];
> +
> +	/* mask == 0 => no condition testing at receive time */
> +	if (!(*mask))
> +		return &d->rx[RX_ALL];
> +
> +	/* use extra filterset for the subscription of exactly *ONE* can_id */
> +	if (*can_id & CAN_EFF_FLAG) {
> +		if (*mask == (CAN_EFF_MASK | CAN_EFF_FLAG)) {
> +			/* RFC: a use-case for hash-tables in the future? */
> +			return &d->rx[RX_EFF];
> +		}
> +	} else {
> +		if (*mask == CAN_SFF_MASK)
> +			return &d->rx_sff[*can_id];
> +	}
> +
> +	/* default: filter via can_id/can_mask */
> +	return &d->rx[RX_FIL];
> +}
> +
> +/**
> + * can_rx_register - subscribe CAN frames from a specific interface
> + * @dev: pointer to netdevice (NULL => subcribe from 'all' CAN devices list)
> + * @can_id: CAN identifier (see description)
> + * @mask: CAN mask (see description)
> + * @func: callback function on filter match
> + * @data: returned parameter for callback function
> + * @ident: string for calling module indentification
> + *
> + * Description:
> + *  Invokes the callback function with the received sk_buff and the given
> + *  parameter 'data' on a matching receive filter. A filter matches, when
> + *
> + *          <received_can_id> & mask == can_id & mask
> + *
> + *  The filter can be inverted (CAN_INV_FILTER bit set in can_id) or it can
> + *  filter for error frames (CAN_ERR_FLAG bit set in mask).
> + *
> + * Return:
> + *  0 on success
> + *  -ENOMEM on missing cache mem to create subscription entry
> + *  -ENODEV unknown device
> + */
> +int can_rx_register(struct net_device *dev, canid_t can_id, canid_t mask,
> +		    void (*func)(struct sk_buff *, void *), void *data,
> +		    char *ident)
> +{
> +	struct receiver *r;
> +	struct hlist_head *rl;
> +	struct dev_rcv_lists *d;
> +	int ret = 0;
> +
> +	/* insert new receiver  (dev,canid,mask) -> (func,data) */
> +
> +	DBG("dev %p (%s), id %03X, mask %03X, callback %p, data %p, "
> +	    "ident %s\n", dev, DNAME(dev), can_id, mask, func, data, ident);
> +
> +	r = kmem_cache_alloc(rcv_cache, GFP_KERNEL);
> +	if (!r)
> +		return -ENOMEM;
> +
> +	spin_lock_bh(&rcv_lists_lock);
> +
> +	d = find_dev_rcv_lists(dev);
> +	if (d) {
> +		rl = find_rcv_list(&can_id, &mask, d);
> +
> +		r->can_id  = can_id;
> +		r->mask    = mask;
> +		r->matches = 0;
> +		r->func    = func;
> +		r->data    = data;
> +		r->ident   = ident;
> +
> +		hlist_add_head_rcu(&r->list, rl);
> +		d->entries++;
> +
> +		pstats.rcv_entries++;
> +		if (pstats.rcv_entries_max < pstats.rcv_entries)
> +			pstats.rcv_entries_max = pstats.rcv_entries;
> +	} else {
> +		DBG("receive list not found for dev %s, id %03X, mask %03X\n",
> +		    DNAME(dev), can_id, mask);
> +		kmem_cache_free(rcv_cache, r);
> +		ret = -ENODEV;
> +	}
> +
> +	spin_unlock_bh(&rcv_lists_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(can_rx_register);
> +
> +/*
> + * can_rx_delete_device - rcu callback for dev_rcv_lists structure removal
> + */
> +static void can_rx_delete_device(struct rcu_head *rp)
> +{
> +	struct dev_rcv_lists *d = container_of(rp, struct dev_rcv_lists, rcu);
> +
> +	DBG("removing dev_rcv_list at %p\n", d);
> +	kfree(d);
> +}
> +
> +/*
> + * can_rx_delete_receiver - rcu callback for single receiver entry removal
> + */
> +static void can_rx_delete_receiver(struct rcu_head *rp)
> +{
> +	struct receiver *r = container_of(rp, struct receiver, rcu);
> +
> +	DBG("removing receiver at %p\n", r);
> +	kmem_cache_free(rcv_cache, r);
> +}
> +
> +/**
> + * can_rx_unregister - unsubscribe CAN frames from a specific interface
> + * @dev: pointer to netdevice (NULL => unsubcribe from 'all' CAN devices list)
> + * @can_id: CAN identifier
> + * @mask: CAN mask
> + * @func: callback function on filter match
> + * @data: returned parameter for callback function
> + *
> + * Description:
> + *  Removes subscription entry depending on given (subscription) values.
> + *
> + * Return:
> + *  0 on success
> + *  -EINVAL on missing subscription entry
> + *  -ENODEV unknown device
> + */
> +int can_rx_unregister(struct net_device *dev, canid_t can_id, canid_t mask,
> +		      void (*func)(struct sk_buff *, void *), void *data)
> +{
> +	struct receiver *r = NULL;
> +	struct hlist_head *rl;
> +	struct hlist_node *next;
> +	struct dev_rcv_lists *d;
> +	int ret = 0;
> +
> +	DBG("dev %p (%s), id %03X, mask %03X, callback %p, data %p\n",
> +	    dev, DNAME(dev), can_id, mask, func, data);
> +
> +	spin_lock_bh(&rcv_lists_lock);
> +
> +	d = find_dev_rcv_lists(dev);
> +	if (!d) {
> +		DBG("receive list not found for dev %s, id %03X, mask %03X\n",
> +		    DNAME(dev), can_id, mask);
> +		ret = -ENODEV;
> +		goto out;
> +	}
> +
> +	rl = find_rcv_list(&can_id, &mask, d);
> +
> +	/*
> +	 * Search the receiver list for the item to delete.  This should
> +	 * exist, since no receiver may be unregistered that hasn't
> +	 * been registered before.
> +	 */
> +
> +	hlist_for_each_entry(r, next, rl, list) {
> +		if (r->can_id == can_id && r->mask == mask
> +		    && r->func == func && r->data == data)
> +			break;
> +	}
> +
> +	/*
> +	 * Check for bug in CAN protocol implementations:
> +	 * If no matching list item was found, the list cursor variable next
> +	 * will be NULL, while r will point to the last item of the list.
> +	 */
> +
> +	if (!next) {
> +		DBG("receive list entry not found for "
> +		    "dev %s, id %03X, mask %03X\n", DNAME(dev), can_id, mask);
> +		ret = -EINVAL;
> +		r = NULL;
> +		d = NULL;
> +		goto out;
> +	}
> +
> +	hlist_del_rcu(&r->list);
> +	d->entries--;
> +
> +	if (pstats.rcv_entries > 0)
> +		pstats.rcv_entries--;
> +
> +	/* remove device structure requested by NETDEV_UNREGISTER */
> +	if (d->remove_on_zero_entries && !d->entries) {
> +		DBG("removing dev_rcv_list for %s on zero entries\n",
> +		    dev->name);
> +		hlist_del_rcu(&d->list);
> +	} else
> +		d = NULL;
> +
> + out:
> +	spin_unlock_bh(&rcv_lists_lock);
> +
> +	/* schedule the receiver item for deletion */
> +	if (r)
> +		call_rcu(&r->rcu, can_rx_delete_receiver);
> +
> +	/* schedule the device structure for deletion */
> +	if (d)
> +		call_rcu(&d->rcu, can_rx_delete_device);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(can_rx_unregister);
> +
> +static inline void deliver(struct sk_buff *skb, struct receiver *r)
> +{
> +	struct sk_buff *clone = skb_clone(skb, GFP_ATOMIC);
> +
> +	DBG("skbuff %p cloned to %p\n", skb, clone);
> +	if (clone) {
> +		clone->sk = skb->sk;
> +		r->func(clone, r->data);
> +		r->matches++;
> +	}
> +}
> +
> +static int can_rcv_filter(struct dev_rcv_lists *d, struct sk_buff *skb)
> +{
> +	struct receiver *r;
> +	struct hlist_node *n;
> +	int matches = 0;
> +	struct can_frame *cf = (struct can_frame *)skb->data;
> +	canid_t can_id = cf->can_id;
> +
> +	if (d->entries == 0)
> +		return 0;
> +
> +	if (can_id & CAN_ERR_FLAG) {
> +		/* check for error frame entries only */
> +		hlist_for_each_entry_rcu(r, n, &d->rx[RX_ERR], list) {
> +			if (can_id & r->mask) {
> +				DBG("match on rx_err skbuff %p\n", skb);
> +				deliver(skb, r);
> +				matches++;
> +			}
> +		}
> +		return matches;
> +	}
> +
> +	/* check for unfiltered entries */
> +	hlist_for_each_entry_rcu(r, n, &d->rx[RX_ALL], list) {
> +		DBG("match on rx_all skbuff %p\n", skb);
> +		deliver(skb, r);
> +		matches++;
> +	}
> +
> +	/* check for can_id/mask entries */
> +	hlist_for_each_entry_rcu(r, n, &d->rx[RX_FIL], list) {
> +		if ((can_id & r->mask) == r->can_id) {
> +			DBG("match on rx_fil skbuff %p\n", skb);
> +			deliver(skb, r);
> +			matches++;
> +		}
> +	}
> +
> +	/* check for inverted can_id/mask entries */
> +	hlist_for_each_entry_rcu(r, n, &d->rx[RX_INV], list) {
> +		if ((can_id & r->mask) != r->can_id) {
> +			DBG("match on rx_inv skbuff %p\n", skb);
> +			deliver(skb, r);
> +			matches++;
> +		}
> +	}
> +
> +	/* check CAN_ID specific entries */
> +	if (can_id & CAN_EFF_FLAG) {
> +		hlist_for_each_entry_rcu(r, n, &d->rx[RX_EFF], list) {
> +			if (r->can_id == can_id) {
> +				DBG("match on rx_eff skbuff %p\n", skb);
> +				deliver(skb, r);
> +				matches++;
> +			}
> +		}
> +	} else {
> +		can_id &= CAN_SFF_MASK;
> +		hlist_for_each_entry_rcu(r, n, &d->rx_sff[can_id], list) {
> +			DBG("match on rx_sff skbuff %p\n", skb);
> +			deliver(skb, r);
> +			matches++;
> +		}
> +	}
> +
> +	return matches;
> +}
> +
> +static int can_rcv(struct sk_buff *skb, struct net_device *dev,
> +		   struct packet_type *pt, struct net_device *orig_dev)
> +{
> +	struct dev_rcv_lists *d;
> +	int matches;
> +
> +	DBG("received skbuff on device %s, ptype %04x\n",
> +	    dev->name, ntohs(pt->type));
> +	DBG_SKB(skb);
> +	DBG_FRAME("af_can: can_rcv: received CAN frame",
> +		  (struct can_frame *)skb->data);
> +
> +	if (dev->type != ARPHRD_CAN || dev->nd_net != &init_net) {
> +		kfree_skb(skb);
> +		return 0;
> +	}
> +
> +	/* update statistics */
> +	stats.rx_frames++;
> +	stats.rx_frames_delta++;
> +
> +	rcu_read_lock();
> +
> +	/* deliver the packet to sockets listening on all devices */
> +	matches = can_rcv_filter(&rx_alldev_list, skb);
> +
> +	/* find receive list for this device */
> +	d = find_dev_rcv_lists(dev);
> +	if (d)
> +		matches += can_rcv_filter(d, skb);
> +
> +	rcu_read_unlock();
> +
> +	/* free the skbuff allocated by the netdevice driver */
> +	DBG("freeing skbuff %p\n", skb);
> +	kfree_skb(skb);
> +
> +	if (matches > 0) {
> +		stats.matches++;
> +		stats.matches_delta++;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * af_can protocol functions
> + */
> +
> +/**
> + * can_proto_register - register CAN transport protocol
> + * @cp: pointer to CAN protocol structure
> + *
> + * Return:
> + *  0 on success
> + *  -EINVAL invalid (out of range) protocol number
> + *  -EBUSY  protocol already in use
> + *  -ENOBUF if proto_register() fails
> + */
> +int can_proto_register(struct can_proto *cp)
> +{
> +	int proto = cp->protocol;
> +	int err = 0;
> +
> +	if (proto < 0 || proto >= CAN_NPROTO) {
> +		printk(KERN_ERR "can: protocol number %d out of range\n",
> +		       proto);
> +		return -EINVAL;
> +	}
> +	if (proto_tab[proto]) {
> +		printk(KERN_ERR "can: protocol %d already registered\n",
> +		       proto);
> +		return -EBUSY;
> +	}
> +
> +	err = proto_register(cp->prot, 0);
> +	if (err < 0)
> +		return err;
> +
> +	proto_tab[proto] = cp;
> +
> +	/* use generic ioctl function if the module doesn't bring its own */
> +	if (!cp->ops->ioctl)
> +		cp->ops->ioctl = can_ioctl;
> +
> +	return err;
> +}
> +EXPORT_SYMBOL(can_proto_register);
> +
> +/**
> + * can_proto_unregister - unregister CAN transport protocol
> + * @cp: pointer to CAN protocol structure
> + *
> + * Return:
> + *  0 on success
> + *  -ESRCH protocol number was not registered
> + */
> +int can_proto_unregister(struct can_proto *cp)
> +{
> +	int proto = cp->protocol;
> +
> +	if (!proto_tab[proto]) {
> +		printk(KERN_ERR "can: protocol %d is not registered\n", proto);
> +		return -ESRCH;
> +	}
> +	proto_unregister(cp->prot);
> +	proto_tab[proto] = NULL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(can_proto_unregister);
> +
> +/*
> + * af_can notifier to create/remove CAN netdevice specific structs
> + */
> +static int can_notifier(struct notifier_block *nb, unsigned long msg,
> +			void *data)
> +{
> +	struct net_device *dev = (struct net_device *)data;
> +	struct dev_rcv_lists *d;
> +
> +	DBG("msg %ld for dev %p (%s idx %d)\n",
> +	    msg, dev, dev->name, dev->ifindex);
> +
> +	if (dev->nd_net != &init_net)
> +		return NOTIFY_DONE;
> +
> +	if (dev->type != ARPHRD_CAN)
> +		return NOTIFY_DONE;
> +
> +	switch (msg) {
> +
> +	case NETDEV_REGISTER:
> +
> +		/*
> +		 * create new dev_rcv_lists for this device
> +		 *
> +		 * N.B. zeroing the struct is the correct initialization
> +		 * for the embedded hlist_head structs.
> +		 * Another list type, e.g. list_head, would require
> +		 * explicit initialization.
> +		 */
> +
> +		DBG("creating new dev_rcv_lists for %s\n", dev->name);
> +
> +		d = kzalloc(sizeof(*d),
> +			    in_interrupt() ? GFP_ATOMIC : GFP_KERNEL);


netdevice registration should never happen from interrupt handlers.

> +		if (!d) {
> +			printk(KERN_ERR
> +			       "can: allocation of receive list failed\n");
> +			return NOTIFY_DONE;
> +		}
> +		d->dev = dev;
> +
> +		spin_lock_bh(&rcv_lists_lock);
> +		hlist_add_head_rcu(&d->list, &rx_dev_list);
> +		spin_unlock_bh(&rcv_lists_lock);
> +
> +		break;
> +
> +	case NETDEV_UNREGISTER:
> +		spin_lock_bh(&rcv_lists_lock);
> +
> +		d = find_dev_rcv_lists(dev);
> +		if (d) {
> +			DBG("remove dev_rcv_list for %s (%d entries)\n",
> +			    dev->name, d->entries);
> +
> +			if (d->entries) {
> +				d->remove_on_zero_entries = 1;
> +				d = NULL;
> +			} else
> +				hlist_del_rcu(&d->list);
> +		} else
> +			printk(KERN_ERR "can: notifier: receive list not "
> +			       "found for dev %s\n", dev->name);
> +
> +		spin_unlock_bh(&rcv_lists_lock);
> +
> +		if (d)
> +			call_rcu(&d->rcu, can_rx_delete_device);
> +
> +		break;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +/*
> + * af_can debugging stuff
> + */
> +
> +#ifdef CONFIG_CAN_DEBUG_CORE
> +
> +#define DBG_BSIZE 1024
> +
> +/**
> + * can_debug_cframe - print CAN frame
> + * @msg: pointer to message printed before the given CAN frame
> + * @cf: pointer to CAN frame
> + */
> +void can_debug_cframe(const char *msg, struct can_frame *cf, ...)
> +{
> +	va_list ap;
> +	int len;
> +	int dlc, i;
> +	char *buf;
> +
> +	buf = kmalloc(DBG_BSIZE, GFP_ATOMIC);
> +	if (!buf)
> +		return;
> +
> +	len = sprintf(buf, KERN_DEBUG);
> +	va_start(ap, cf);
> +	len += snprintf(buf + len, DBG_BSIZE - 64, msg, ap);
> +	buf[len++] = ':';
> +	buf[len++] = ' ';
> +	va_end(ap);
> +
> +	dlc = cf->can_dlc;
> +	if (dlc > 8)
> +		dlc = 8;
> +
> +	if (cf->can_id & CAN_EFF_FLAG)
> +		len += sprintf(buf + len, "<%08X> [%X] ",
> +			       cf->can_id & CAN_EFF_MASK, dlc);
> +	else
> +		len += sprintf(buf + len, "<%03X> [%X] ",
> +			       cf->can_id & CAN_SFF_MASK, dlc);
> +
> +	for (i = 0; i < dlc; i++)
> +		len += sprintf(buf + len, "%02X ", cf->data[i]);
> +
> +	if (cf->can_id & CAN_RTR_FLAG)
> +		len += sprintf(buf + len, "(RTR)");
> +
> +	buf[len++] = '\n';
> +	buf[len]   = '\0';
> +	printk(buf);
> +	kfree(buf);
> +}
> +EXPORT_SYMBOL(can_debug_cframe);
> +
> +/**
> + * can_debug_skb - print socket buffer content to kernel log
> + * @skb: pointer to socket buffer
> + */
> +void can_debug_skb(struct sk_buff *skb)
> +{
> +	int len, nbytes, i;
> +	char *buf;
> +
> +	buf = kmalloc(DBG_BSIZE, GFP_ATOMIC);
> +	if (!buf)
> +		return;
> +
> +	len = sprintf(buf,
> +		      KERN_DEBUG "  skbuff at %p, dev: %d, proto: %04x\n"
> +		      KERN_DEBUG "  users: %d, dataref: %d, nr_frags: %d, "
> +		      "h,d,t,e,l: %p %+d %+d %+d, %d",
> +		      skb, skb->dev ? skb->dev->ifindex : -1,
> +		      ntohs(skb->protocol),
> +		      atomic_read(&skb->users),
> +		      atomic_read(&(skb_shinfo(skb)->dataref)),
> +		      skb_shinfo(skb)->nr_frags,
> +		      skb->head, skb->data - skb->head,
> +		      skb->tail - skb->head, skb->end - skb->head, skb->len);
> +	nbytes = skb->end - skb->head;
> +	for (i = 0; i < nbytes; i++) {
> +		if (i % 16 == 0)
> +			len += sprintf(buf + len, "\n" KERN_DEBUG "  ");
> +		if (len < DBG_BSIZE - 16) {
> +			len += sprintf(buf + len, " %02x", skb->head[i]);
> +		} else {
> +			len += sprintf(buf + len, "...");
> +			break;
> +		}
> +	}
> +	buf[len++] = '\n';
> +	buf[len]   = '\0';
> +	printk(buf);
> +	kfree(buf);
> +}
> +EXPORT_SYMBOL(can_debug_skb);
> +
> +#endif
> +
> +/*
> + * af_can module init/exit functions
> + */
> +
> +static struct packet_type can_packet = {
> +	.type = __constant_htons(ETH_P_CAN),
> +	.dev  = NULL,
> +	.func = can_rcv,
> +};

__read_mostly (for those below as well)?

> +
> +static struct net_proto_family can_family_ops = {
> +	.family = PF_CAN,
> +	.create = can_create,
> +	.owner  = THIS_MODULE,
> +};
> +
> +/* notifier block for netdevice event */
> +static struct notifier_block can_netdev_notifier = {
> +	.notifier_call = can_notifier,
> +};
> +
> +static __init int can_init(void)
> +{
> +	printk(banner);
> +
> +	rcv_cache = kmem_cache_create("can_receiver", sizeof(struct receiver),
> +				      0, 0, NULL);
> +	if (!rcv_cache)
> +		return -ENOMEM;
> +
> +	/*
> +	 * Insert rx_alldev_list for reception on all devices.
> +	 * This struct is zero initialized which is correct for the
> +	 * embedded hlist heads, the dev pointer, and the entries counter.
> +	 */
> +
> +	spin_lock_bh(&rcv_lists_lock);
> +	hlist_add_head_rcu(&rx_alldev_list.list, &rx_dev_list);
> +	spin_unlock_bh(&rcv_lists_lock);
> +
> +	if (stats_timer) {
> +		/* the statistics are updated every second (timer triggered) */
> +		init_timer(&stattimer);
> +		stattimer.function = can_stat_update;
> +		stattimer.data = 0;
> +		/* update every second */
> +		stattimer.expires = jiffies + HZ;

round_jiffies?

> +		/* start statistics timer */
> +		add_timer(&stattimer);
> +	} else
> +		stattimer.function = NULL;
> +
> +	/* procfs init */
> +	can_init_proc();
> +
> +	/* protocol register */
> +	sock_register(&can_family_ops);
> +	register_netdevice_notifier(&can_netdev_notifier);
> +	dev_add_pack(&can_packet);
> +
> +	return 0;
> +}
> +
> +static __exit void can_exit(void)
> +{
> +	struct dev_rcv_lists *d;
> +	struct hlist_node *n, *next;
> +
> +	if (stats_timer)
> +		del_timer(&stattimer);
> +
> +	/* procfs remove */
> +	can_remove_proc();
> +
> +	/* protocol unregister */
> +	dev_remove_pack(&can_packet);
> +	unregister_netdevice_notifier(&can_netdev_notifier);
> +	sock_unregister(PF_CAN);
> +
> +	/* remove rx_dev_list */
> +	spin_lock_bh(&rcv_lists_lock);
> +	hlist_del(&rx_alldev_list.list);
> +	hlist_for_each_entry_safe(d, n, next, &rx_dev_list, list) {
> +		hlist_del(&d->list);
> +		kfree(d);
> +	}
> +	spin_unlock_bh(&rcv_lists_lock);
> +
> +	kmem_cache_destroy(rcv_cache);
> +}
> +
> +module_init(can_init);
> +module_exit(can_exit);

> +++ net-2.6.24/net/can/proc.c	2007-09-17 11:07:19.000000000 +0200
> @@ -0,0 +1,531 @@
> +#include <linux/module.h>
> +#include <linux/proc_fs.h>
> +#include <linux/list.h>
> +#include <linux/rcupdate.h>
> +#include <linux/can/core.h>
> +
> +#include "af_can.h"
> +
> +/*
> + * proc filenames for the PF_CAN core
> + */
> +
> +#define CAN_PROC_VERSION     "version"
> +#define CAN_PROC_STATS       "stats"
> +#define CAN_PROC_RESET_STATS "reset_stats"
> +#define CAN_PROC_RCVLIST_ALL "rcvlist_all"
> +#define CAN_PROC_RCVLIST_FIL "rcvlist_fil"
> +#define CAN_PROC_RCVLIST_INV "rcvlist_inv"
> +#define CAN_PROC_RCVLIST_SFF "rcvlist_sff"
> +#define CAN_PROC_RCVLIST_EFF "rcvlist_eff"
> +#define CAN_PROC_RCVLIST_ERR "rcvlist_err"
> +
> +static struct proc_dir_entry *can_dir;
> +static struct proc_dir_entry *pde_version;
> +static struct proc_dir_entry *pde_stats;
> +static struct proc_dir_entry *pde_reset_stats;
> +static struct proc_dir_entry *pde_rcvlist_all;
> +static struct proc_dir_entry *pde_rcvlist_fil;
> +static struct proc_dir_entry *pde_rcvlist_inv;
> +static struct proc_dir_entry *pde_rcvlist_sff;
> +static struct proc_dir_entry *pde_rcvlist_eff;
> +static struct proc_dir_entry *pde_rcvlist_err;
> +
> +static int user_reset;
> +
> +static const char *rx_list_name[] = {
> +	[RX_ERR] = "rx_err",
> +	[RX_ALL] = "rx_all",
> +	[RX_FIL] = "rx_fil",
> +	[RX_INV] = "rx_inv",
> +	[RX_EFF] = "rx_eff",
> +};
> +
> +/*
> + * af_can statistics stuff
> + */
> +
> +static void can_init_stats(void)
> +{
> +	/*
> +	 * This memset function is called from a timer context (when
> +	 * stattimer is active which is the default) OR in a process
> +	 * context (reading the proc_fs when stattimer is disabled).
> +	 */
> +	memset(&stats, 0, sizeof(stats));
> +	stats.jiffies_init = jiffies;
> +
> +	pstats.stats_reset++;
> +
> +	if (user_reset) {
> +		user_reset = 0;
> +		pstats.user_reset++;
> +	}
> +}
> +
> +static unsigned long calc_rate(unsigned long oldjif, unsigned long newjif,
> +			       unsigned long count)
> +{
> +	unsigned long ret = 0;
> +
> +	if (oldjif == newjif)
> +		return 0;
> +
> +	/* see can_rcv() - this should NEVER happen! */


If I'm not mistaken this comment is outdated and should refer to
can_stat_update().

> +	if (count > (ULONG_MAX / HZ)) {
> +		printk(KERN_ERR "can: calc_rate: count exceeded! %ld\n",
> +		       count);
> +		return 99999999;
> +	}
> +
> +	ret = (count * HZ) / (newjif - oldjif);
> +
> +	return ret;
> +}
> +
> +void can_stat_update(unsigned long data)
> +{
> +	unsigned long j = jiffies; /* snapshot */
> +
> +	/* restart counting in timer context on user request */
> +	if (user_reset)
> +		can_init_stats();
> +
> +	/* restart counting on jiffies overflow */
> +	if (j < stats.jiffies_init)
> +		can_init_stats();
> +
> +	/* stats.rx_frames is the definitively max. statistic value */
> +
> +	/* prevent overflow in calc_rate() */
> +	if (stats.rx_frames > (ULONG_MAX / HZ))
> +		can_init_stats();
> +
> +	/* matches overflow - very improbable */
> +	if (stats.matches > (ULONG_MAX / 100))
> +		can_init_stats();
> +
> +	/* calc total values */
> +	if (stats.rx_frames)
> +		stats.total_rx_match_ratio = (stats.matches * 100) /
> +						stats.rx_frames;
> +
> +	stats.total_tx_rate = calc_rate(stats.jiffies_init, j,
> +					stats.tx_frames);
> +	stats.total_rx_rate = calc_rate(stats.jiffies_init, j,
> +					stats.rx_frames);
> +
> +	/* calc current values */
> +	if (stats.rx_frames_delta)
> +		stats.current_rx_match_ratio =
> +			(stats.matches_delta * 100) / stats.rx_frames_delta;
> +
> +	stats.current_tx_rate = calc_rate(0, HZ, stats.tx_frames_delta);
> +	stats.current_rx_rate = calc_rate(0, HZ, stats.rx_frames_delta);
> +
> +	/* check / update maximum values */
> +	if (stats.max_tx_rate < stats.current_tx_rate)
> +		stats.max_tx_rate = stats.current_tx_rate;
> +
> +	if (stats.max_rx_rate < stats.current_rx_rate)
> +		stats.max_rx_rate = stats.current_rx_rate;
> +
> +	if (stats.max_rx_match_ratio < stats.current_rx_match_ratio)
> +		stats.max_rx_match_ratio = stats.current_rx_match_ratio;
> +
> +	/* clear values for 'current rate' calculation */
> +	stats.tx_frames_delta = 0;
> +	stats.rx_frames_delta = 0;
> +	stats.matches_delta   = 0;
> +
> +	/* restart timer (one second) */
> +	stattimer.expires = jiffies + HZ;


round_jiffies?

> +	add_timer(&stattimer);
> +}
> +
> +/*
> + * proc read functions
> + *
> + * From known use-cases we expect about 10 entries in a receive list to be
> + * printed in the proc_fs. So PAGE_SIZE is definitely enough space here.


Would be nicer to use seq_file (for all the proc stuff).

> + *
> + */
> +
> +static int can_print_rcvlist(char *page, int len, struct hlist_head *rx_list,
> +			     struct net_device *dev)
> +{
> +	struct receiver *r;
> +	struct hlist_node *n;
> +
> +	rcu_read_lock();
> +	hlist_for_each_entry_rcu(r, n, rx_list, list) {
> +		char *fmt = (r->can_id & CAN_EFF_FLAG)?
> +			"   %-5s  %08X  %08x  %08x  %08x  %8ld  %s\n" :
> +			"   %-5s     %03X    %08x  %08lx  %08lx  %8ld  %s\n";
> +
> +		len += snprintf(page + len, PAGE_SIZE - len, fmt,
> +				DNAME(dev), r->can_id, r->mask,
> +				(unsigned long)r->func, (unsigned long)r->data,
> +				r->matches, r->ident);
> +
> +		/* does a typical line fit into the current buffer? */
> +
> +		/* 100 Bytes before end of buffer */
> +		if (len > PAGE_SIZE - 100) {
> +			/* mark output cut off */
> +			len += snprintf(page + len, PAGE_SIZE - len,
> +					"   (..)\n");
> +			break;
> +		}
> +	}
> +	rcu_read_unlock();
> +
> +	return len;
> +}
> +
> +static int can_print_recv_banner(char *page, int len)
> +{
> +	/*
> +	 *                  can1.  00000000  00000000  00000000
> +	 *                 .......          0  tp20
> +	 */
> +	len += snprintf(page + len, PAGE_SIZE - len,
> +			"  device   can_id   can_mask  function"
> +			"  userdata   matches  ident\n");
> +
> +	return len;
> +}
> +
> +static int can_proc_read_stats(char *page, char **start, off_t off,
> +			       int count, int *eof, void *data)
> +{
> +	int len = 0;
> +
> +	len += snprintf(page + len, PAGE_SIZE - len, "\n");
> +	len += snprintf(page + len, PAGE_SIZE - len,
> +			" %8ld transmitted frames (TXF)\n", stats.tx_frames);
> +	len += snprintf(page + len, PAGE_SIZE - len,
> +			" %8ld received frames (RXF)\n", stats.rx_frames);
> +	len += snprintf(page + len, PAGE_SIZE - len,
> +			" %8ld matched frames (RXMF)\n", stats.matches);
> +
> +	len += snprintf(page + len, PAGE_SIZE - len, "\n");
> +
> +	if (stattimer.function == can_stat_update) {
> +		len += snprintf(page + len, PAGE_SIZE - len,
> +				" %8ld %% total match ratio (RXMR)\n",
> +				stats.total_rx_match_ratio);
> +
> +		len += snprintf(page + len, PAGE_SIZE - len,
> +				" %8ld frames/s total tx rate (TXR)\n",
> +				stats.total_tx_rate);
> +		len += snprintf(page + len, PAGE_SIZE - len,
> +				" %8ld frames/s total rx rate (RXR)\n",
> +				stats.total_rx_rate);
> +
> +		len += snprintf(page + len, PAGE_SIZE - len, "\n");
> +
> +		len += snprintf(page + len, PAGE_SIZE - len,
> +				" %8ld %% current match ratio (CRXMR)\n",
> +				stats.current_rx_match_ratio);
> +
> +		len += snprintf(page + len, PAGE_SIZE - len,
> +				" %8ld frames/s current tx rate (CTXR)\n",
> +				stats.current_tx_rate);
> +		len += snprintf(page + len, PAGE_SIZE - len,
> +				" %8ld frames/s current rx rate (CRXR)\n",
> +				stats.current_rx_rate);
> +
> +		len += snprintf(page + len, PAGE_SIZE - len, "\n");
> +
> +		len += snprintf(page + len, PAGE_SIZE - len,
> +				" %8ld %% max match ratio (MRXMR)\n",
> +				stats.max_rx_match_ratio);
> +
> +		len += snprintf(page + len, PAGE_SIZE - len,
> +				" %8ld frames/s max tx rate (MTXR)\n",
> +				stats.max_tx_rate);
> +		len += snprintf(page + len, PAGE_SIZE - len,
> +				" %8ld frames/s max rx rate (MRXR)\n",
> +				stats.max_rx_rate);
> +
> +		len += snprintf(page + len, PAGE_SIZE - len, "\n");
> +	}
> +
> +	len += snprintf(page + len, PAGE_SIZE - len,
> +			" %8ld current receive list entries (CRCV)\n",
> +			pstats.rcv_entries);
> +	len += snprintf(page + len, PAGE_SIZE - len,
> +			" %8ld maximum receive list entries (MRCV)\n",
> +			pstats.rcv_entries_max);
> +
> +	if (pstats.stats_reset)
> +		len += snprintf(page + len, PAGE_SIZE - len,
> +				"\n %8ld statistic resets (STR)\n",
> +				pstats.stats_reset);
> +
> +	if (pstats.user_reset)
> +		len += snprintf(page + len, PAGE_SIZE - len,
> +				" %8ld user statistic resets (USTR)\n",
> +				pstats.user_reset);
> +
> +	len += snprintf(page + len, PAGE_SIZE - len, "\n");
> +
> +	*eof = 1;
> +	return len;
> +}
> +
> +static int can_proc_read_reset_stats(char *page, char **start, off_t off,
> +				     int count, int *eof, void *data)
> +{
> +	int len = 0;
> +
> +	user_reset = 1;
> +
> +	if (stattimer.function == can_stat_update) {
> +		len += snprintf(page + len, PAGE_SIZE - len,
> +				"Scheduled statistic reset #%ld.\n",
> +				pstats.stats_reset + 1);
> +
> +	} else {
> +		if (stats.jiffies_init != jiffies)
> +			can_init_stats();
> +
> +		len += snprintf(page + len, PAGE_SIZE - len,
> +				"Performed statistic reset #%ld.\n",
> +				pstats.stats_reset);
> +	}
> +
> +	*eof = 1;
> +	return len;
> +}
> +
> +static int can_proc_read_version(char *page, char **start, off_t off,
> +				 int count, int *eof, void *data)
> +{
> +	int len = 0;
> +
> +	len += snprintf(page + len, PAGE_SIZE - len, "%s\n",
> +			CAN_VERSION_STRING);
> +	*eof = 1;
> +	return len;
> +}
> +
> +static int can_proc_read_rcvlist(char *page, char **start, off_t off,
> +				 int count, int *eof, void *data)
> +{
> +	/* double cast to prevent GCC warning */
> +	int idx = (int)(long)data;
> +	int len = 0;
> +	struct dev_rcv_lists *d;
> +	struct hlist_node *n;
> +
> +	len += snprintf(page + len, PAGE_SIZE - len,
> +			"\nreceive list '%s':\n", rx_list_name[idx]);
> +
> +	rcu_read_lock();
> +	hlist_for_each_entry_rcu(d, n, &rx_dev_list, list) {
> +
> +		if (!hlist_empty(&d->rx[idx])) {
> +			len = can_print_recv_banner(page, len);
> +			len = can_print_rcvlist(page, len, &d->rx[idx], d->dev);
> +		} else
> +			len += snprintf(page + len, PAGE_SIZE - len,
> +					"  (%s: no entry)\n", DNAME(d->dev));
> +
> +		/* exit on end of buffer? */
> +		if (len > PAGE_SIZE - 100)
> +			break;
> +	}
> +	rcu_read_unlock();
> +
> +	len += snprintf(page + len, PAGE_SIZE - len, "\n");
> +
> +	*eof = 1;
> +	return len;
> +}
> +
> +static int can_proc_read_rcvlist_sff(char *page, char **start, off_t off,
> +				     int count, int *eof, void *data)
> +{
> +	int len = 0;
> +	struct dev_rcv_lists *d;
> +	struct hlist_node *n;
> +
> +	/* RX_SFF */
> +	len += snprintf(page + len, PAGE_SIZE - len,
> +			"\nreceive list 'rx_sff':\n");
> +
> +	rcu_read_lock();
> +	hlist_for_each_entry_rcu(d, n, &rx_dev_list, list) {
> +		int i, all_empty = 1;
> +		/* check wether at least one list is non-empty */
> +		for (i = 0; i < 0x800; i++)
> +			if (!hlist_empty(&d->rx_sff[i])) {
> +				all_empty = 0;
> +				break;
> +			}
> +
> +		if (!all_empty) {
> +			len = can_print_recv_banner(page, len);
> +			for (i = 0; i < 0x800; i++) {
> +				if (!hlist_empty(&d->rx_sff[i]) &&
> +				    len < PAGE_SIZE - 100)
> +					len = can_print_rcvlist(page, len,
> +								&d->rx_sff[i],
> +								d->dev);
> +			}
> +		} else
> +			len += snprintf(page + len, PAGE_SIZE - len,
> +					"  (%s: no entry)\n", DNAME(d->dev));
> +
> +		/* exit on end of buffer? */
> +		if (len > PAGE_SIZE - 100)
> +			break;
> +	}
> +	rcu_read_unlock();
> +
> +	len += snprintf(page + len, PAGE_SIZE - len, "\n");
> +
> +	*eof = 1;
> +	return len;
> +}
> +
> +/*
> + * proc utility functions
> + */
> +
> +static struct proc_dir_entry *can_create_proc_readentry(const char *name,
> +							mode_t mode,
> +							read_proc_t *read_proc,
> +							void *data)
> +{
> +	if (can_dir)
> +		return create_proc_read_entry(name, mode, can_dir, read_proc,
> +					      data);
> +	else
> +		return NULL;
> +}
> +
> +static void can_remove_proc_readentry(const char *name)
> +{
> +	if (can_dir)
> +		remove_proc_entry(name, can_dir);
> +}
> +
> +/*
> + * can_init_proc - create main CAN proc directory and procfs entries
> + */
> +void can_init_proc(void)
> +{
> +	/* create /proc/net/can directory */
> +	can_dir = proc_mkdir("can", init_net.proc_net);
> +
> +	if (!can_dir) {
> +		printk(KERN_INFO "can: failed to create /proc/net/can . "
> +		       "CONFIG_PROC_FS missing?\n");
> +		return;
> +	}
> +
> +	can_dir->owner = THIS_MODULE;
> +
> +	/* own procfs entries from the AF_CAN core */
> +	pde_version     = can_create_proc_readentry(CAN_PROC_VERSION, 0644,
> +					can_proc_read_version, NULL);
> +	pde_stats       = can_create_proc_readentry(CAN_PROC_STATS, 0644,
> +					can_proc_read_stats, NULL);
> +	pde_reset_stats = can_create_proc_readentry(CAN_PROC_RESET_STATS, 0644,
> +					can_proc_read_reset_stats, NULL);
> +	pde_rcvlist_err = can_create_proc_readentry(CAN_PROC_RCVLIST_ERR, 0644,
> +					can_proc_read_rcvlist, (void *)RX_ERR);
> +	pde_rcvlist_all = can_create_proc_readentry(CAN_PROC_RCVLIST_ALL, 0644,
> +					can_proc_read_rcvlist, (void *)RX_ALL);
> +	pde_rcvlist_fil = can_create_proc_readentry(CAN_PROC_RCVLIST_FIL, 0644,
> +					can_proc_read_rcvlist, (void *)RX_FIL);
> +	pde_rcvlist_inv = can_create_proc_readentry(CAN_PROC_RCVLIST_INV, 0644,
> +					can_proc_read_rcvlist, (void *)RX_INV);
> +	pde_rcvlist_eff = can_create_proc_readentry(CAN_PROC_RCVLIST_EFF, 0644,
> +					can_proc_read_rcvlist, (void *)RX_EFF);
> +	pde_rcvlist_sff = can_create_proc_readentry(CAN_PROC_RCVLIST_SFF, 0644,
> +					can_proc_read_rcvlist_sff, NULL);
> +}
> +
> +/*
> + * can_remove_proc - remove procfs entries and main CAN proc directory
> + */
> +void can_remove_proc(void)
> +{
> +	if (pde_version)
> +		can_remove_proc_readentry(CAN_PROC_VERSION);
> +
> +	if (pde_stats)
> +		can_remove_proc_readentry(CAN_PROC_STATS);
> +
> +	if (pde_reset_stats)
> +		can_remove_proc_readentry(CAN_PROC_RESET_STATS);
> +
> +	if (pde_rcvlist_err)
> +		can_remove_proc_readentry(CAN_PROC_RCVLIST_ERR);
> +
> +	if (pde_rcvlist_all)
> +		can_remove_proc_readentry(CAN_PROC_RCVLIST_ALL);
> +
> +	if (pde_rcvlist_fil)
> +		can_remove_proc_readentry(CAN_PROC_RCVLIST_FIL);
> +
> +	if (pde_rcvlist_inv)
> +		can_remove_proc_readentry(CAN_PROC_RCVLIST_INV);
> +
> +	if (pde_rcvlist_eff)
> +		can_remove_proc_readentry(CAN_PROC_RCVLIST_EFF);
> +
> +	if (pde_rcvlist_sff)
> +		can_remove_proc_readentry(CAN_PROC_RCVLIST_SFF);
> +
> +	if (can_dir)
> +		proc_net_remove(&init_net, "can");
> +}
> Index: net-2.6.24/include/linux/can/error.h
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ net-2.6.24/include/linux/can/error.h	2007-09-17 10:27:09.000000000 +0200
> @@ -0,0 +1,93 @@
> +/*
> + * linux/can/error.h
> + *
> + * Definitions of the CAN error frame to be filtered and passed to the user.
> + *
> + * Author: Oliver Hartkopp <oliver.hartkopp@volkswagen.de>
> + * Copyright (c) 2002-2007 Volkswagen Group Electronic Research
> + * All rights reserved.
> + *
> + * Send feedback to <socketcan-users@lists.berlios.de>
> + *
> + */
> +
> +#ifndef CAN_ERROR_H
> +#define CAN_ERROR_H
> +
> +#define CAN_ERR_DLC 8 /* dlc for error frames */
> +
> +/* error class (mask) in can_id */
> +#define CAN_ERR_TX_TIMEOUT   0x00000001U /* TX timeout (by netdevice driver) */
> +#define CAN_ERR_LOSTARB      0x00000002U /* lost arbitration    / data[0]    */
> +#define CAN_ERR_CRTL         0x00000004U /* controller problems / data[1]    */
> +#define CAN_ERR_PROT         0x00000008U /* protocol violations / data[2..3] */
> +#define CAN_ERR_TRX          0x00000010U /* transceiver status  / data[4]    */
> +#define CAN_ERR_ACK          0x00000020U /* received no ACK on transmission */
> +#define CAN_ERR_BUSOFF       0x00000040U /* bus off */
> +#define CAN_ERR_BUSERROR     0x00000080U /* bus error (may flood!) */
> +#define CAN_ERR_RESTARTED    0x00000100U /* controller restarted */
> +
> +/* arbitration lost in bit ... / data[0] */
> +#define CAN_ERR_LOSTARB_UNSPEC   0x00 /* unspecified */
> +				      /* else bit number in bitstream */
> +
> +/* error status of CAN-controller / data[1] */
> +#define CAN_ERR_CRTL_UNSPEC      0x00 /* unspecified */
> +#define CAN_ERR_CRTL_RX_OVERFLOW 0x01 /* RX buffer overflow */
> +#define CAN_ERR_CRTL_TX_OVERFLOW 0x02 /* TX buffer overflow */
> +#define CAN_ERR_CRTL_RX_WARNING  0x04 /* reached warning level for RX errors */
> +#define CAN_ERR_CRTL_TX_WARNING  0x08 /* reached warning level for TX errors */
> +#define CAN_ERR_CRTL_RX_PASSIVE  0x10 /* reached error passive status RX */
> +#define CAN_ERR_CRTL_TX_PASSIVE  0x20 /* reached error passive status TX */
> +				      /* (at least one error counter exceeds */
> +				      /* the protocol-defined level of 127)  */
> +
> +/* error in CAN protocol (type) / data[2] */
> +#define CAN_ERR_PROT_UNSPEC      0x00 /* unspecified */
> +#define CAN_ERR_PROT_BIT         0x01 /* single bit error */
> +#define CAN_ERR_PROT_FORM        0x02 /* frame format error */
> +#define CAN_ERR_PROT_STUFF       0x04 /* bit stuffing error */
> +#define CAN_ERR_PROT_BIT0        0x08 /* unable to send dominant bit */
> +#define CAN_ERR_PROT_BIT1        0x10 /* unable to send recessive bit */
> +#define CAN_ERR_PROT_OVERLOAD    0x20 /* bus overload */
> +#define CAN_ERR_PROT_ACTIVE      0x40 /* active error announcement */
> +#define CAN_ERR_PROT_TX          0x80 /* error occured on transmission */
> +
> +/* error in CAN protocol (location) / data[3] */
> +#define CAN_ERR_PROT_LOC_UNSPEC  0x00 /* unspecified */
> +#define CAN_ERR_PROT_LOC_SOF     0x03 /* start of frame */
> +#define CAN_ERR_PROT_LOC_ID28_21 0x02 /* ID bits 28 - 21 (SFF: 10 - 3) */
> +#define CAN_ERR_PROT_LOC_ID20_18 0x06 /* ID bits 20 - 18 (SFF: 2 - 0 )*/
> +#define CAN_ERR_PROT_LOC_SRTR    0x04 /* substitute RTR (SFF: RTR) */
> +#define CAN_ERR_PROT_LOC_IDE     0x05 /* identifier extension */
> +#define CAN_ERR_PROT_LOC_ID17_13 0x07 /* ID bits 17-13 */
> +#define CAN_ERR_PROT_LOC_ID12_05 0x0F /* ID bits 12-5 */
> +#define CAN_ERR_PROT_LOC_ID04_00 0x0E /* ID bits 4-0 */
> +#define CAN_ERR_PROT_LOC_RTR     0x0C /* RTR */
> +#define CAN_ERR_PROT_LOC_RES1    0x0D /* reserved bit 1 */
> +#define CAN_ERR_PROT_LOC_RES0    0x09 /* reserved bit 0 */
> +#define CAN_ERR_PROT_LOC_DLC     0x0B /* data length code */
> +#define CAN_ERR_PROT_LOC_DATA    0x0A /* data section */
> +#define CAN_ERR_PROT_LOC_CRC_SEQ 0x08 /* CRC sequence */
> +#define CAN_ERR_PROT_LOC_CRC_DEL 0x18 /* CRC delimiter */
> +#define CAN_ERR_PROT_LOC_ACK     0x19 /* ACK slot */
> +#define CAN_ERR_PROT_LOC_ACK_DEL 0x1B /* ACK delimiter */
> +#define CAN_ERR_PROT_LOC_EOF     0x1A /* end of frame */
> +#define CAN_ERR_PROT_LOC_INTERM  0x12 /* intermission */
> +
> +/* error status of CAN-transceiver / data[4] */
> +/*                                             CANH CANL */
> +#define CAN_ERR_TRX_UNSPEC             0x00 /* 0000 0000 */
> +#define CAN_ERR_TRX_CANH_NO_WIRE       0x04 /* 0000 0100 */
> +#define CAN_ERR_TRX_CANH_SHORT_TO_BAT  0x05 /* 0000 0101 */
> +#define CAN_ERR_TRX_CANH_SHORT_TO_VCC  0x06 /* 0000 0110 */
> +#define CAN_ERR_TRX_CANH_SHORT_TO_GND  0x07 /* 0000 0111 */
> +#define CAN_ERR_TRX_CANL_NO_WIRE       0x40 /* 0100 0000 */
> +#define CAN_ERR_TRX_CANL_SHORT_TO_BAT  0x50 /* 0101 0000 */
> +#define CAN_ERR_TRX_CANL_SHORT_TO_VCC  0x60 /* 0110 0000 */
> +#define CAN_ERR_TRX_CANL_SHORT_TO_GND  0x70 /* 0111 0000 */
> +#define CAN_ERR_TRX_CANL_SHORT_TO_CANH 0x80 /* 1000 0000 */
> +
> +/* controller specific additional information / data[5..7] */
> +
> +#endif /* CAN_ERROR_H */
> 
> --
> 



^ permalink raw reply


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