Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Tomoya MORINAGA @ 2010-11-16  5:30 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Masayuki Ohtake,
	Samuel Ortiz, margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA, LKML,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CE0FF5A.3000100@pengutronix.de>

Hi Marc,

On Monday, November 15, 2010 6:37 PM, Marc Kleine-Budde wrote :

> Either use ioread8 or ioread16 in both the rx and tx path.
I will modify to ioread16

>> +  if (ioread32(&priv->regs->treq2) & 0xfc00) {
> what does this if check?
This is for whether all tx requests are complete or not.

---
Thanks,

Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

^ permalink raw reply

* [PATCH net-next-2.6] udp: use atomic_inc_not_zero_hint
From: Eric Dumazet @ 2010-11-16  5:58 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

UDP sockets refcount is usually 2, unless an incoming frame is going to
be queued in receive or backlog queue.

Using atomic_inc_not_zero_hint() permits to reduce latency, because
processor issues less memory transactions.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/sock.h |    2 +-
 net/ipv4/udp.c     |    4 ++--
 net/ipv6/udp.c     |    4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index a6338d0..eb0c1f5 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -57,7 +57,7 @@
 #include <linux/rculist_nulls.h>
 #include <linux/poll.h>
 
-#include <asm/atomic.h>
+#include <linux/atomic.h>
 #include <net/dst.h>
 #include <net/checksum.h>
 
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 5e0a3a5..491ecd3 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -430,7 +430,7 @@ begin:
 
 	if (result) {
 exact_match:
-		if (unlikely(!atomic_inc_not_zero(&result->sk_refcnt)))
+		if (unlikely(!atomic_inc_not_zero_hint(&result->sk_refcnt, 2)))
 			result = NULL;
 		else if (unlikely(compute_score2(result, net, saddr, sport,
 				  daddr, hnum, dif) < badness)) {
@@ -500,7 +500,7 @@ begin:
 		goto begin;
 
 	if (result) {
-		if (unlikely(!atomic_inc_not_zero(&result->sk_refcnt)))
+		if (unlikely(!atomic_inc_not_zero_hint(&result->sk_refcnt, 2)))
 			result = NULL;
 		else if (unlikely(compute_score(result, net, saddr, hnum, sport,
 				  daddr, dport, dif) < badness)) {
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 91def93..b541a4e 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -227,7 +227,7 @@ begin:
 
 	if (result) {
 exact_match:
-		if (unlikely(!atomic_inc_not_zero(&result->sk_refcnt)))
+		if (unlikely(!atomic_inc_not_zero_hint(&result->sk_refcnt, 2)))
 			result = NULL;
 		else if (unlikely(compute_score2(result, net, saddr, sport,
 				  daddr, hnum, dif) < badness)) {
@@ -294,7 +294,7 @@ begin:
 		goto begin;
 
 	if (result) {
-		if (unlikely(!atomic_inc_not_zero(&result->sk_refcnt)))
+		if (unlikely(!atomic_inc_not_zero_hint(&result->sk_refcnt, 2)))
 			result = NULL;
 		else if (unlikely(compute_score(result, net, hnum, saddr, sport,
 					daddr, dport, dif) < badness)) {



^ permalink raw reply related

* Re: the future of ethtool
From: Thomas Graf @ 2010-11-16  6:17 UTC (permalink / raw)
  To: David Miller; +Cc: jeff, bhutchings, shemminger, netdev
In-Reply-To: <20101115.180225.189678628.davem@davemloft.net>

On Mon, Nov 15, 2010 at 06:02:25PM -0800, David Miller wrote:
> It isn't sufficient.  You can still get into unwindable failures.
> 
> Earlier operations can consume fixed resources like TCAM filter
> slots or rx/tx queues, making a subsequent operation in the
> sequence fail.
> 
> A validate/commit scheme cannot detect this effectively.

I agree, there are many more scenarios where it would not work
reliably. It would have ensured that all provided values are
within range boundries and that all requested operations are in
fact supported. Since I have disregarded the idea, I does not
matter much anymore.

^ permalink raw reply

* Re: Fwd: a Great Idea - include Kademlia networking protocol in kernel -- REVISITED
From: Marcos @ 2010-11-16  6:21 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1289727544.2743.72.camel@edumazet-laptop>

[Eric Dumazet wrote:]
> But we dont want a "super operating system". We want a good one.

Yes, well you have that, I think, speaking at the kernel level.  But
the thing is, people are building tools that mimic such anyway, and
the next wave of new value will be found there.

> Memory stores done in userland are as fast as memory stores done in
> kernel.

Really? And how about the abstraction-level?  because that will either
make it lucrative or not for developers to build applications for
it.....

> Once you need to access files, perform complex searches, timers,
> logging, and all the stuff, you really want to do it from userland, in
> high level language that many programmers master, or get something that
> is too complex/buggy.

Yes, of course, all that will have to be considered.   But I'm
suggesting that such a move is an investment in the future, that the
the number of machines that will want or request peer-2-peer
connectivity will (or should) only increase.  Done right, such a move
should *simplify* things.  We're biased to think in centralized ways
because of the centuries-old history of *who* has the resources.  But
as networking, computation, and storage become commodified further,
whole new topologies for the *right* architecture become available.
The idea of "the OS" itself morphs.   And the *only* way maximize the
value of the network is to make it easy to connect and communicate
between peers -- what happens after that is so radical it can hardly
be speculated because it gets into the realm of emergent complexity.
Again, I refer you to Reed's law on the value of such networks.

marcos

^ permalink raw reply

* Re: the future of ethtool
From: Thomas Graf @ 2010-11-16  6:25 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Jeff Garzik, Stephen Hemminger, NetDev, David Miller
In-Reply-To: <1289866230.2586.65.camel@bwh-desktop>

On Tue, Nov 16, 2010 at 12:10:30AM +0000, Ben Hutchings wrote:
> I would expect to treat each operation in a multiple-set as conditional
> on the success of all previous operations.  ethtool or other utilities
> should then take care to put operations in a sensible order (e.g. enable
> TX checksum before TSO, if those remain separate operations).  Error
> reporting in the core is then as simple as reporting how many operations
> were successful plus the error code for the one that failed.

This is already not that trivial with current netlink limitations.
We are pretty much limited to a single int when returning error
states. One more reason to rethink current netlink error semantics.

^ permalink raw reply

* [net-2.6 PATCH] ipv6: fix missing in6_ifa_put in addrconf
From: John Fastabend @ 2010-11-16  6:29 UTC (permalink / raw)
  To: davem; +Cc: john.r.fastabend, eric.dumazet, netdev, shemminger

Fix ref count bug introduced by

commit 2de795707294972f6c34bae9de713e502c431296
Author: Lorenzo Colitti <lorenzo@google.com>
Date:   Wed Oct 27 18:16:49 2010 +0000

ipv6: addrconf: don't remove address state on ifdown if the address
is being kept


Fix logic so that addrconf_ifdown() decrements the inet6_ifaddr
refcnt correctly with in6_ifa_put().

Reported-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 net/ipv6/addrconf.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index b41ce0f..aaa3ca4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2754,13 +2754,13 @@ static int addrconf_ifdown(struct net_device *dev, int how)
 			ifa->state = INET6_IFADDR_STATE_DEAD;
 			spin_unlock_bh(&ifa->state_lock);
 
-			if (state == INET6_IFADDR_STATE_DEAD) {
-				in6_ifa_put(ifa);
-			} else {
+			if (state != INET6_IFADDR_STATE_DEAD) {
 				__ipv6_ifa_notify(RTM_DELADDR, ifa);
 				atomic_notifier_call_chain(&inet6addr_chain,
 							   NETDEV_DOWN, ifa);
 			}
+
+			in6_ifa_put(ifa);
 			write_lock_bh(&idev->lock);
 		}
 	}


^ permalink raw reply related

* ipvs: ipvs update for nf-next-2.6
From: Simon Horman @ 2010-11-16  6:34 UTC (permalink / raw)
  To: Eric Dumazet, Hans Schillstrom, Julian Anastasov, Patrick McHardy,
	lvs-devel, n

Hi Patrick,

the following patches contain updates to IPVS since 2.6.37-rc1
and are targeted at nf-next-2.6.

- Static and shadow variable clean-ups from Eric

- Preparation for persistence engine syncrhonisation

- Correctness fix for persistence engine match.
  I do not consider this a bug as it can only manifest in a problem
  if there is more than one persistence engine loaded and
  currently only one exists.

- Code cleanps for ip_vs_sync_conn(), ip_vs_process_message()
  and ip_vs_process_message()

- Handle GRO aggregated skbs
  The previous behaviour was to send a needs frag ICMP.
  This is probably stable material.

Please pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/horms/lvs-test-2.6.git for-patrick

I expect more activity before 2.6.37-rc1. In particular Hans is working
heavily with Julian and myself on the new synchronisation code.

^ permalink raw reply

* [PATCH 1/6] IPVS: Only match pe_data created by the same pe
From: Simon Horman @ 2010-11-16  6:34 UTC (permalink / raw)
  To: Eric Dumazet, Hans Schillstrom, Julian Anastasov, Patrick McHardy,
	lvs-devel, n
  Cc: Simon Horman
In-Reply-To: <1289889298-17287-1-git-send-email-horms@verge.net.au>

Only match persistence engine data if it was
created by the same persistence engine.

Reported-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_conn.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 64a9ca3..261db1a 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -354,7 +354,7 @@ struct ip_vs_conn *ip_vs_ct_in_get(const struct ip_vs_conn_param *p)
 
 	list_for_each_entry(cp, &ip_vs_conn_tab[hash], c_list) {
 		if (p->pe_data && p->pe->ct_match) {
-			if (p->pe->ct_match(p, cp))
+			if (p->pe == cp->pe && p->pe->ct_match(p, cp))
 				goto out;
 			continue;
 		}
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 2/6] IPVS: Make the cp argument to ip_vs_sync_conn() static
From: Simon Horman @ 2010-11-16  6:34 UTC (permalink / raw)
  To: Eric Dumazet, Hans Schillstrom, Julian Anastasov, Patrick McHardy,
	lvs-devel, n
  Cc: Simon Horman
In-Reply-To: <1289889298-17287-1-git-send-email-horms@verge.net.au>

Acked-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 include/net/ip_vs.h             |    2 +-
 net/netfilter/ipvs/ip_vs_sync.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index be2b569..d5a32e4 100644
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -916,7 +916,7 @@ extern char ip_vs_master_mcast_ifn[IP_VS_IFNAME_MAXLEN];
 extern char ip_vs_backup_mcast_ifn[IP_VS_IFNAME_MAXLEN];
 extern int start_sync_thread(int state, char *mcast_ifn, __u8 syncid);
 extern int stop_sync_thread(int state);
-extern void ip_vs_sync_conn(struct ip_vs_conn *cp);
+extern void ip_vs_sync_conn(const struct ip_vs_conn *cp);
 
 
 /*
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index ab85aed..a4dccbc 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -236,7 +236,7 @@ get_curr_sync_buff(unsigned long time)
  *      Add an ip_vs_conn information into the current sync_buff.
  *      Called by ip_vs_in.
  */
-void ip_vs_sync_conn(struct ip_vs_conn *cp)
+void ip_vs_sync_conn(const struct ip_vs_conn *cp)
 {
 	struct ip_vs_sync_mesg *m;
 	struct ip_vs_sync_conn *s;
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 3/6] IPVS: Remove useless { } block from ip_vs_process_message()
From: Simon Horman @ 2010-11-16  6:34 UTC (permalink / raw)
  To: Eric Dumazet, Hans Schillstrom, Julian Anastasov, Patrick McHardy,
	lvs-devel, n
  Cc: Simon Horman
In-Reply-To: <1289889298-17287-1-git-send-email-horms@verge.net.au>

Acked-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_sync.c |   24 +++++++++++-------------
 1 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index a4dccbc..72b3d88 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -381,20 +381,18 @@ static void ip_vs_process_message(const char *buffer, const size_t buflen)
 			}
 		}
 
-		{
-			if (ip_vs_conn_fill_param_sync(AF_INET, s->protocol,
-					      (union nf_inet_addr *)&s->caddr,
-					      s->cport,
-					      (union nf_inet_addr *)&s->vaddr,
-					      s->vport, &param)) {
-				pr_err("ip_vs_conn_fill_param_sync failed");
-				return;
-			}
-			if (!(flags & IP_VS_CONN_F_TEMPLATE))
-				cp = ip_vs_conn_in_get(&param);
-			else
-				cp = ip_vs_ct_in_get(&param);
+		if (ip_vs_conn_fill_param_sync(AF_INET, s->protocol,
+					       (union nf_inet_addr *)&s->caddr,
+					       s->cport,
+					       (union nf_inet_addr *)&s->vaddr,
+					       s->vport, &param)) {
+			pr_err("ip_vs_conn_fill_param_sync failed");
+			return;
 		}
+		if (!(flags & IP_VS_CONN_F_TEMPLATE))
+			cp = ip_vs_conn_in_get(&param);
+		else
+			cp = ip_vs_ct_in_get(&param);
 		if (!cp) {
 			/*
 			 * Find the appropriate destination for the connection.
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 4/6] IPVS: buffer argument to ip_vs_process_message() should not be const
From: Simon Horman @ 2010-11-16  6:34 UTC (permalink / raw)
  To: Eric Dumazet, Hans Schillstrom, Julian Anastasov, Patrick McHardy,
	lvs-devel, n
  Cc: Simon Horman
In-Reply-To: <1289889298-17287-1-git-send-email-horms@verge.net.au>

It is assigned to a non-const variable and its contents are modified.

Acked-by: Hans Schillstrom <hans.schillstrom@ericsson.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_sync.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index 72b3d88..3897d6b 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -303,7 +303,7 @@ ip_vs_conn_fill_param_sync(int af, int protocol,
  *      Process received multicast message and create the corresponding
  *      ip_vs_conn entries.
  */
-static void ip_vs_process_message(const char *buffer, const size_t buflen)
+static void ip_vs_process_message(char *buffer, const size_t buflen)
 {
 	struct ip_vs_sync_mesg *m = (struct ip_vs_sync_mesg *)buffer;
 	struct ip_vs_sync_conn *s;
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 5/6] ipvs: add static and read_mostly attributes
From: Simon Horman @ 2010-11-16  6:34 UTC (permalink / raw)
  To: Eric Dumazet, Hans Schillstrom, Julian Anastasov, Patrick McHardy,
	lvs-devel, n
  Cc: Eric Dumazet, Simon Horman
In-Reply-To: <1289889298-17287-1-git-send-email-horms@verge.net.au>

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

ip_vs_conn_tab_bits & ip_vs_conn_tab_mask are static to
ipvs/ip_vs_conn.c

ip_vs_conn_tab_size, ip_vs_conn_tab_mask, ip_vs_conn_tab [the pointer],
ip_vs_conn_rnd are mostly read.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_conn.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
index 261db1a..7615f9e 100644
--- a/net/netfilter/ipvs/ip_vs_conn.c
+++ b/net/netfilter/ipvs/ip_vs_conn.c
@@ -48,18 +48,18 @@
 /*
  * Connection hash size. Default is what was selected at compile time.
 */
-int ip_vs_conn_tab_bits = CONFIG_IP_VS_TAB_BITS;
+static int ip_vs_conn_tab_bits = CONFIG_IP_VS_TAB_BITS;
 module_param_named(conn_tab_bits, ip_vs_conn_tab_bits, int, 0444);
 MODULE_PARM_DESC(conn_tab_bits, "Set connections' hash size");
 
 /* size and mask values */
-int ip_vs_conn_tab_size;
-int ip_vs_conn_tab_mask;
+int ip_vs_conn_tab_size __read_mostly;
+static int ip_vs_conn_tab_mask __read_mostly;
 
 /*
  *  Connection hash table: for input and output packets lookups of IPVS
  */
-static struct list_head *ip_vs_conn_tab;
+static struct list_head *ip_vs_conn_tab __read_mostly;
 
 /*  SLAB cache for IPVS connections */
 static struct kmem_cache *ip_vs_conn_cachep __read_mostly;
@@ -71,7 +71,7 @@ static atomic_t ip_vs_conn_count = ATOMIC_INIT(0);
 static atomic_t ip_vs_conn_no_cport_cnt = ATOMIC_INIT(0);
 
 /* random value for IPVS connection hash */
-static unsigned int ip_vs_conn_rnd;
+static unsigned int ip_vs_conn_rnd __read_mostly;
 
 /*
  *  Fine locking granularity for big connection hash table
-- 
1.7.2.3


^ permalink raw reply related

* [PATCH 6/6] ipvs: remove shadow rt variable
From: Simon Horman @ 2010-11-16  6:34 UTC (permalink / raw)
  To: Eric Dumazet, Hans Schillstrom, Julian Anastasov, Patrick McHardy,
	lvs-devel, n
  Cc: Eric Dumazet, Simon Horman
In-Reply-To: <1289889298-17287-1-git-send-email-horms@verge.net.au>

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

Remove a sparse warning about rt variable.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
 net/netfilter/ipvs/ip_vs_xmit.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index 10bd39c..50b131c 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -188,7 +188,6 @@ __ip_vs_reroute_locally(struct sk_buff *skb)
 			},
 			.mark = skb->mark,
 		};
-		struct rtable *rt;
 
 		if (ip_route_output_key(net, &rt, &fl))
 			return 0;
-- 
1.7.2.3


^ permalink raw reply related

* Re: [net-2.6 PATCH] ipv6: fix missing in6_ifa_put in addrconf
From: Eric Dumazet @ 2010-11-16  6:37 UTC (permalink / raw)
  To: John Fastabend; +Cc: davem, netdev, shemminger
In-Reply-To: <20101116062921.31164.70903.stgit@jf-dev1-dcblab>

Le lundi 15 novembre 2010 à 22:29 -0800, John Fastabend a écrit :
> Fix ref count bug introduced by
> 
> commit 2de795707294972f6c34bae9de713e502c431296
> Author: Lorenzo Colitti <lorenzo@google.com>
> Date:   Wed Oct 27 18:16:49 2010 +0000
> 
> ipv6: addrconf: don't remove address state on ifdown if the address
> is being kept
> 
> 
> Fix logic so that addrconf_ifdown() decrements the inet6_ifaddr
> refcnt correctly with in6_ifa_put().
> 
> Reported-by: Stephen Hemminger <shemminger@vyatta.com>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
> 
>  net/ipv6/addrconf.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index b41ce0f..aaa3ca4 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2754,13 +2754,13 @@ static int addrconf_ifdown(struct net_device *dev, int how)
>  			ifa->state = INET6_IFADDR_STATE_DEAD;
>  			spin_unlock_bh(&ifa->state_lock);
>  
> -			if (state == INET6_IFADDR_STATE_DEAD) {
> -				in6_ifa_put(ifa);
> -			} else {
> +			if (state != INET6_IFADDR_STATE_DEAD) {
>  				__ipv6_ifa_notify(RTM_DELADDR, ifa);
>  				atomic_notifier_call_chain(&inet6addr_chain,
>  							   NETDEV_DOWN, ifa);
>  			}
> +
> +			in6_ifa_put(ifa);
>  			write_lock_bh(&idev->lock);
>  		}
>  	}
> 

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Thanks again John



^ permalink raw reply

* [net-next-2.6 PATCH] net: zero kobject in rx_queue_release
From: John Fastabend @ 2010-11-16  6:59 UTC (permalink / raw)
  To: davem; +Cc: john.r.fastabend, netdev, eric.dumazet, therbert

netif_set_real_num_rx_queues() can decrement and increment
the number of rx queues. For example ixgbe does this as
features and offloads are toggled. Presumably this could
also happen across down/up on most devices if the available
resources changed (cpu offlined).

The kobject needs to be zero'd in this case so that the
state is not preserved across kobject_put()/kobject_init_and_add().

This resolves the following error report.

ixgbe 0000:03:00.0: eth2: NIC Link is Up 10 Gbps, Flow Control: RX/TX
kobject (ffff880324b83210): tried to init an initialized object, something is seriously wrong.
Pid: 1972, comm: lldpad Not tainted 2.6.37-rc18021qaz+ #169
Call Trace:
 [<ffffffff8121c940>] kobject_init+0x3a/0x83
 [<ffffffff8121cf77>] kobject_init_and_add+0x23/0x57
 [<ffffffff8107b800>] ? mark_lock+0x21/0x267
 [<ffffffff813c6d11>] net_rx_queue_update_kobjects+0x63/0xc6
 [<ffffffff813b5e0e>] netif_set_real_num_rx_queues+0x5f/0x78
 [<ffffffffa0261d49>] ixgbe_set_num_queues+0x1c6/0x1ca [ixgbe]
 [<ffffffffa0262509>] ixgbe_init_interrupt_scheme+0x1e/0x79c [ixgbe]
 [<ffffffffa0274596>] ixgbe_dcbnl_set_state+0x167/0x189 [ixgbe]

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 net/core/net-sysfs.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 3ba526b..960c075 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -711,13 +711,18 @@ static void rx_queue_release(struct kobject *kobj)
 
 
 	map = rcu_dereference_raw(queue->rps_map);
-	if (map)
+	if (map) {
+		rcu_assign_pointer(queue->rps_map, NULL);
 		call_rcu(&map->rcu, rps_map_release);
+	}
 
 	flow_table = rcu_dereference_raw(queue->rps_flow_table);
-	if (flow_table)
+	if (flow_table) {
+		rcu_assign_pointer(queue->rps_flow_table, NULL);
 		call_rcu(&flow_table->rcu, rps_dev_flow_table_release);
+	}
 
+	memset(kobj, 0, sizeof(*kobj));
 	dev_put(queue->dev);
 }
 


^ permalink raw reply related

* Re: [PATCH 21/44] drivers/s390/net: Remove unnecessary semicolons
From: Frank Blaschka @ 2010-11-16  7:03 UTC (permalink / raw)
  To: David Miller; +Cc: joe, netdev, linux-s390
In-Reply-To: <20101115.110810.241442235.davem@davemloft.net>

On Mon, Nov 15, 2010 at 11:08:10AM -0800, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Sun, 14 Nov 2010 19:04:40 -0800
> 
> > Signed-off-by: Joe Perches <joe@perches.com>
> 
> I'll let the s390 folks take this one.
Ok, added this one to my patch queue. Thx!
> --
> To unsubscribe from this list: send the line "unsubscribe linux-s390" 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

* [PATCH net-next-2.6] net: reorder struct sock fields
From: Eric Dumazet @ 2010-11-16  7:12 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

Right now, fields in struct sock are not optimally ordered, because each
path (RX softirq, TX completion, RX user,  TX user) has to touch fields
that are contained in many different cache lines.

The really critical thing is to shrink number of cache lines that are
used at RX softirq time : CPU handling softirqs for a device can receive
many frames per second for many sockets. If load is too big, we can drop
frames at NIC level. RPS or multiqueue cards can help, but better reduce
latency if possible.

This patch starts with UDP protocol, then additional patches will try to
reduce latencies of other ones as well.

At RX softirq time, fields of interest for UDP protocol are :
(not counting ones in inet struct for the lookup)

Read/Written:
sk_refcnt   (atomic increment/decrement)
sk_rmem_alloc & sk_backlog.len (to check if there is room in queues)
sk_receive_queue
sk_backlog (if socket locked by user program)
sk_rxhash
sk_forward_alloc

Read only:
sk_rcvbuf (sk_rcvqueues_full())
sk_filter
sk_wq

Additional notes :

- sk_backlog has one hole on 64bit arches. We can fill it to save 8
bytes.
- sk_backlog is used only if RX sofirq handler finds the socket while
locked by user.
- sk_rxhash is written only once per flow.


Final layout :

1) One section grouping all read/write fields, but placing rxhash and
sk_backlog at the end of this section.

2) One section grouping all read fields in RX handler 
   (sk_filter, sk_rcv_buf, sk_wq)

3) Section used by other paths 

I'll post a patch on its own to put sk_refcnt in the same cache line
than section [1] (or at least, at the end of struct sock_common)

New offsets on 64bit arch :

sizeof(struct sock)=0x268
offsetof(struct sock, sk_refcnt)  =0x10
offsetof(struct sock, sk_lock)    =0x48
offsetof(struct sock, sk_receive_queue)=0x68
offsetof(struct sock, sk_backlog)=0x80
offsetof(struct sock, sk_rmem_alloc)=0x80
offsetof(struct sock, sk_forward_alloc)=0x98
offsetof(struct sock, sk_rxhash)=0x9c
offsetof(struct sock, sk_rcvbuf)=0xac
offsetof(struct sock, sk_filter)=0xa0
offsetof(struct sock, sk_wq)=0xb0

Instead of :

sizeof(struct sock)=0x270
offsetof(struct sock, sk_refcnt)  =0x10
offsetof(struct sock, sk_lock)    =0x50
offsetof(struct sock, sk_receive_queue)=0x178
offsetof(struct sock, sk_backlog)=0xf8
offsetof(struct sock, sk_rmem_alloc)=0x168
offsetof(struct sock, sk_forward_alloc)=0x26c
offsetof(struct sock, sk_rxhash)=0x288
offsetof(struct sock, sk_rcvbuf)=0x4c
offsetof(struct sock, sk_filter)=0x368
offsetof(struct sock, sk_wq)=0x110

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/sock.h |   49 ++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 21 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index a6338d0..7742572 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -241,51 +241,59 @@ struct sock {
 #define sk_bind_node		__sk_common.skc_bind_node
 #define sk_prot			__sk_common.skc_prot
 #define sk_net			__sk_common.skc_net
-	kmemcheck_bitfield_begin(flags);
-	unsigned int		sk_shutdown  : 2,
-				sk_no_check  : 2,
-				sk_userlocks : 4,
-				sk_protocol  : 8,
-				sk_type      : 16;
-	kmemcheck_bitfield_end(flags);
-	int			sk_rcvbuf;
 	socket_lock_t		sk_lock;
+	struct sk_buff_head	sk_receive_queue;
 	/*
 	 * The backlog queue is special, it is always used with
 	 * the per-socket spinlock held and requires low latency
 	 * access. Therefore we special case it's implementation.
+	 * Note : rmem_alloc is in this structure to fill a hole
+	 * on 64bit arches, not because its logically part of
+	 * backlog.
 	 */
 	struct {
-		struct sk_buff *head;
-		struct sk_buff *tail;
-		int len;
+		atomic_t	rmem_alloc;
+		int		len;
+		struct sk_buff	*head;
+		struct sk_buff	*tail;
 	} sk_backlog;
+#define sk_rmem_alloc sk_backlog.rmem_alloc
+	int			sk_forward_alloc;
+#ifdef CONFIG_RPS
+	__u32			sk_rxhash;
+#endif
+	struct sk_filter __rcu	*sk_filter;
+	kmemcheck_bitfield_begin(flags);
+	unsigned int		sk_shutdown  : 2,
+				sk_no_check  : 2,
+				sk_userlocks : 4,
+				sk_protocol  : 8,
+				sk_type      : 16;
+	kmemcheck_bitfield_end(flags);
+	int			sk_rcvbuf;
+
 	struct socket_wq	*sk_wq;
+
+#ifdef CONFIG_NET_DMA
+	struct sk_buff_head	sk_async_wait_queue;
+#endif
+
 	struct dst_entry	*sk_dst_cache;
 #ifdef CONFIG_XFRM
 	struct xfrm_policy	*sk_policy[2];
 #endif
 	spinlock_t		sk_dst_lock;
-	atomic_t		sk_rmem_alloc;
 	atomic_t		sk_wmem_alloc;
 	atomic_t		sk_omem_alloc;
 	int			sk_sndbuf;
-	struct sk_buff_head	sk_receive_queue;
 	struct sk_buff_head	sk_write_queue;
-#ifdef CONFIG_NET_DMA
-	struct sk_buff_head	sk_async_wait_queue;
-#endif
 	int			sk_wmem_queued;
-	int			sk_forward_alloc;
 	gfp_t			sk_allocation;
 	int			sk_route_caps;
 	int			sk_route_nocaps;
 	int			sk_gso_type;
 	unsigned int		sk_gso_max_size;
 	int			sk_rcvlowat;
-#ifdef CONFIG_RPS
-	__u32			sk_rxhash;
-#endif
 	unsigned long 		sk_flags;
 	unsigned long	        sk_lingertime;
 	struct sk_buff_head	sk_error_queue;
@@ -301,7 +309,6 @@ struct sock {
 	const struct cred	*sk_peer_cred;
 	long			sk_rcvtimeo;
 	long			sk_sndtimeo;
-	struct sk_filter __rcu	*sk_filter;
 	void			*sk_protinfo;
 	struct timer_list	sk_timer;
 	ktime_t			sk_stamp;



^ permalink raw reply related

* Re: [net-2.6 PATCH] nete zero kobject in rx_queue_release
From: John Fastabend @ 2010-11-16  7:13 UTC (permalink / raw)
  To: David Miller
  Cc: therbert@google.com, netdev@vger.kernel.org,
	eric.dumazet@gmail.com
In-Reply-To: <4CE1E73C.1000105@intel.com>

On 11/15/2010 6:06 PM, John Fastabend wrote:
> On 11/14/2010 3:15 PM, David Miller wrote:
>> From: Tom Herbert <therbert@google.com>
>> Date: Sun, 14 Nov 2010 14:40:00 -0800
>>
>>>> So we'll need something like:
>>>>
>>>>        if (atomic_dec_and_test(&first->count))
>>>>                kfree(first);
>>>>        else
>>>>                /* clear everything except queue->first */
>>>>
>>>
>>> The patches to get rid of the separate refcnt should obviate this
>>> complexity.  Could just clear the queue in kobject release.
>>
>> True but we'll still need a patch for older kernels.
> 
> OK Thanks. I'll have a stable patch and a net-2.6 patch soon.
> 
> -- John

To address Tom's comment, queue->dev would need to be reset if the queue was cleared. In the latest patch I didn't bother and just clear the kobject.

^ permalink raw reply

* MQ performance on other cards (cxgb3)
From: Krishna Kumar2 @ 2010-11-16  7:25 UTC (permalink / raw)
  To: davem, Divy Le Ray
  Cc: anthony, arnd, avi, eric.dumazet, kvm, Michael S. Tsirkin, netdev,
	rusty
In-Reply-To: <OF24E08752.2087FFA4-ON652577D6.00532DF1-652577D6.0054B291@LocalDomain>

I had sent this mail to Michael last week - he agrees that I should
share this information on the list:

On latest net-next-2.6, virtio-net (guest->host) results are:
______________________________________________________________
                         SQ vs MQ (#txqs=8)
#      BW1      BW2 (%)          CPU1     CPU2 (%)   RCPU1   RCPU2 (%)
_______________________________________________________________
1      105774  112256 (6.1)   257      255 (-.7)     532     549 (3.1)
2      20842   30674 (47.1)   107      150 (40.1)    208     279 (34.1)
4      22500   31953 (42.0)   241      409 (69.7)    467     619 (32.5)
8      22416   44507 (98.5)   477      1039 (117.8)  960     1459 (51.9)
16     22605   45372 (100.7)  905      2060 (127.6)  1895    2962 (56.3)
24     23192   44201 (90.5)   1360     3028 (122.6)  2833    4437 (56.6)
32     23158   43394 (87.3)   1811     3957 (118.4)  3770    5936 (57.4)
40     23322   42550 (82.4)   2276     4986 (119.0)  4711    7417 (57.4)
48     23564   41931 (77.9)   2757     5966 (116.3)  5653    8896 (57.3)
64     23949   41092 (71.5)   3788     7898 (108.5)  7609    11826 (55.4)
80     23256   41343 (77.7)   4597     9887 (115.0)  9503    14801 (55.7)
96     23310   40645 (74.3)   5588     11758 (110.4) 11381   17761 (56.0)
128    24095   41082 (70.5)   7587     15574 (105.2) 15029   23716 (57.8)
______________________________________________________________
Avg:      BW: (58.3)      CPU: (110.8)      RCPU: (55.9)

It's true that average CPU% on guest is almost double that of the BW
improvement. But I don't think this is due to the patch (driver does no
synchronization, etc). To compare MQ vs SQ on a 10G card, I ran the
same test from host to remote host across cxgb3. The results are
somewhat similar:

(I changed cxgb_open on the client system to:
	netif_set_real_num_tx_queues(dev, 1);
	err = netif_set_real_num_rx_queues(dev, 1);
to simulate single queue (SQ))
_____________________________________
            cxgb3 SQ vs cxgb3 MQ
#     BW1      BW2 (%)      CPU1   CPU2 (%)
_____________________________________
1     8301    8315 (.1)        5         4.66 (-6.6)
2     9395    9380 (-.1)      16        16 (0)
4     9411    9414 (0)        33        26 (-21.2)
8     9411    9398 (-.1)      60        62 (3.3)
16   9412    9413 (0)        116      117 (.8)
24   9442    9963 (5.5)     179      198 (10.6)
32   10031  10025 (0)       230     249 (8.2)
40   9953    10024 (.7)      300     312 (4.0)
48   10002  10015 (.1)      351     376 (7.1)
64   10022  10024 (0)       494     515 (4.2)
80   8894    10011 (12.5)   537    630 (17.3)
96   8465    9907 (17.0)     612    749 (22.3)
128  7541   9617 (27.5)     760    989 (30.1)
_____________________________________
Avg:     BW: (3.8)     CPU: (14.8)

(Each case runs runs once for 60 secs)

The BW increased modestly but CPU increased much more. I assume
the change I made above to convert the driver from MQ to SQ is not
incorrect.

Thanks,

- KK


^ permalink raw reply

* Re: [net-next-2.6 PATCH] net: zero kobject in rx_queue_release
From: Eric Dumazet @ 2010-11-16  7:38 UTC (permalink / raw)
  To: John Fastabend; +Cc: davem, netdev, therbert
In-Reply-To: <20101116065906.31611.36938.stgit@jf-dev1-dcblab>

Le lundi 15 novembre 2010 à 22:59 -0800, John Fastabend a écrit :
> netif_set_real_num_rx_queues() can decrement and increment
> the number of rx queues. For example ixgbe does this as
> features and offloads are toggled. Presumably this could
> also happen across down/up on most devices if the available
> resources changed (cpu offlined).
> 
> The kobject needs to be zero'd in this case so that the
> state is not preserved across kobject_put()/kobject_init_and_add().
> 
> This resolves the following error report.
> 
> ixgbe 0000:03:00.0: eth2: NIC Link is Up 10 Gbps, Flow Control: RX/TX
> kobject (ffff880324b83210): tried to init an initialized object, something is seriously wrong.
> Pid: 1972, comm: lldpad Not tainted 2.6.37-rc18021qaz+ #169
> Call Trace:
>  [<ffffffff8121c940>] kobject_init+0x3a/0x83
>  [<ffffffff8121cf77>] kobject_init_and_add+0x23/0x57
>  [<ffffffff8107b800>] ? mark_lock+0x21/0x267
>  [<ffffffff813c6d11>] net_rx_queue_update_kobjects+0x63/0xc6
>  [<ffffffff813b5e0e>] netif_set_real_num_rx_queues+0x5f/0x78
>  [<ffffffffa0261d49>] ixgbe_set_num_queues+0x1c6/0x1ca [ixgbe]
>  [<ffffffffa0262509>] ixgbe_init_interrupt_scheme+0x1e/0x79c [ixgbe]
>  [<ffffffffa0274596>] ixgbe_dcbnl_set_state+0x167/0x189 [ixgbe]
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
> 
>  net/core/net-sysfs.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
> index 3ba526b..960c075 100644
> --- a/net/core/net-sysfs.c
> +++ b/net/core/net-sysfs.c
> @@ -711,13 +711,18 @@ static void rx_queue_release(struct kobject *kobj)
>  
> 
>  	map = rcu_dereference_raw(queue->rps_map);
> -	if (map)
> +	if (map) {
> +		rcu_assign_pointer(queue->rps_map, NULL);

Hmm, yes this works, but I am not sure queue->rps_map can be read by
other cpus at this point.

rcu_assign_pointer() is a documented interface with implied semantic : I
put a NULL pointer on a RCU protected variable, and avoid a memory
barrier because NULL is special.

If this patch is for current kernel, I advise using RCU_INIT_POINTER()
instead to make clear we only want to set the pointer to NULL, and avoid
sparse warnings :)




>  		call_rcu(&map->rcu, rps_map_release);
> +	}
>  
>  	flow_table = rcu_dereference_raw(queue->rps_flow_table);
> -	if (flow_table)
> +	if (flow_table) {
> +		rcu_assign_pointer(queue->rps_flow_table, NULL);


same here ?

>  		call_rcu(&flow_table->rcu, rps_dev_flow_table_release);
> +	}
>  
> +	memset(kobj, 0, sizeof(*kobj));

Is it the regular way to perform this, no kobject_{clear|del|deinit}() ?

>  	dev_put(queue->dev);
>  }
>  
> 

Thanks 



^ permalink raw reply

* Vlan communication
From: Liew Kian Yang @ 2010-11-17  7:15 UTC (permalink / raw)
  To: netdev

Dear All,

Can I know is there any ways to make the vlan 1 interface able to send
and receive data from the other vlan without using the vconfig to add
the vlan in linux?

Thank you


Regards,
Kian Yang, Liew




^ permalink raw reply

* Re: Vlan communication
From: Eric Dumazet @ 2010-11-16  7:52 UTC (permalink / raw)
  To: kianyang; +Cc: netdev
In-Reply-To: <1289978124.3250.2.camel@localhost.localdomain>

Le mercredi 17 novembre 2010 à 15:15 +0800, Liew Kian Yang a écrit :
> Dear All,
> 
> Can I know is there any ways to make the vlan 1 interface able to send
> and receive data from the other vlan without using the vconfig to add
> the vlan in linux?

I dont understand your question.

If your problem is vconfig being not available, the answer is

ip link add link eth0 eth0.1 type vlan id 1




^ permalink raw reply

* Re: [net-next-2.6 PATCH] net: zero kobject in rx_queue_release
From: John Fastabend @ 2010-11-16  8:12 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: davem@davemloft.net, netdev@vger.kernel.org, therbert@google.com
In-Reply-To: <1289893125.3364.234.camel@edumazet-laptop>

On 11/15/2010 11:38 PM, Eric Dumazet wrote:
> Le lundi 15 novembre 2010 à 22:59 -0800, John Fastabend a écrit :
>> netif_set_real_num_rx_queues() can decrement and increment
>> the number of rx queues. For example ixgbe does this as
>> features and offloads are toggled. Presumably this could
>> also happen across down/up on most devices if the available
>> resources changed (cpu offlined).
>>
>> The kobject needs to be zero'd in this case so that the
>> state is not preserved across kobject_put()/kobject_init_and_add().
>>
>> This resolves the following error report.
>>
>> ixgbe 0000:03:00.0: eth2: NIC Link is Up 10 Gbps, Flow Control: RX/TX
>> kobject (ffff880324b83210): tried to init an initialized object, something is seriously wrong.
>> Pid: 1972, comm: lldpad Not tainted 2.6.37-rc18021qaz+ #169
>> Call Trace:
>>  [<ffffffff8121c940>] kobject_init+0x3a/0x83
>>  [<ffffffff8121cf77>] kobject_init_and_add+0x23/0x57
>>  [<ffffffff8107b800>] ? mark_lock+0x21/0x267
>>  [<ffffffff813c6d11>] net_rx_queue_update_kobjects+0x63/0xc6
>>  [<ffffffff813b5e0e>] netif_set_real_num_rx_queues+0x5f/0x78
>>  [<ffffffffa0261d49>] ixgbe_set_num_queues+0x1c6/0x1ca [ixgbe]
>>  [<ffffffffa0262509>] ixgbe_init_interrupt_scheme+0x1e/0x79c [ixgbe]
>>  [<ffffffffa0274596>] ixgbe_dcbnl_set_state+0x167/0x189 [ixgbe]
>>
>> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
>> ---
>>
>>  net/core/net-sysfs.c |    9 +++++++--
>>  1 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
>> index 3ba526b..960c075 100644
>> --- a/net/core/net-sysfs.c
>> +++ b/net/core/net-sysfs.c
>> @@ -711,13 +711,18 @@ static void rx_queue_release(struct kobject *kobj)
>>  
>>
>>  	map = rcu_dereference_raw(queue->rps_map);
>> -	if (map)
>> +	if (map) {
>> +		rcu_assign_pointer(queue->rps_map, NULL);
> 
> Hmm, yes this works, but I am not sure queue->rps_map can be read by
> other cpus at this point.

Why not? Sorry not sure I follow.

> 
> rcu_assign_pointer() is a documented interface with implied semantic : I
> put a NULL pointer on a RCU protected variable, and avoid a memory
> barrier because NULL is special.
> 
> If this patch is for current kernel, I advise using RCU_INIT_POINTER()
> instead to make clear we only want to set the pointer to NULL, and avoid
> sparse warnings :)
> 

OK, avoiding sparse warnings is good changed to RCU_INIT_POINTER().

> 
> 
> 
>>  		call_rcu(&map->rcu, rps_map_release);
>> +	}
>>  
>>  	flow_table = rcu_dereference_raw(queue->rps_flow_table);
>> -	if (flow_table)
>> +	if (flow_table) {
>> +		rcu_assign_pointer(queue->rps_flow_table, NULL);
> 
> 
> same here ?

changed here as well.

> 
>>  		call_rcu(&flow_table->rcu, rps_dev_flow_table_release);
>> +	}
>>  
>> +	memset(kobj, 0, sizeof(*kobj));
> 
> Is it the regular way to perform this, no kobject_{clear|del|deinit}() ?

None that I can see. kobject_del() unlinks the koject but does not do any clear.

> 
>>  	dev_put(queue->dev);
>>  }
>>  
>>
> 
> Thanks 
> 
> 


^ permalink raw reply

* Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Marc Kleine-Budde @ 2010-11-16  8:14 UTC (permalink / raw)
  To: Tomoya MORINAGA
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Masayuki Ohtake,
	Samuel Ortiz, margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA, LKML,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w, Wolfgang Grandegger,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <001401cb854d$b46e13c0$66f8800a-a06+6cuVnkTSQfdrb5gaxUEOCMrvLtNR@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 781 bytes --]

On 11/16/2010 06:18 AM, Tomoya MORINAGA wrote:
> On Monday, November 15, 2010 6:11 PM, Marc Kleine-Budde wrote:
>> Please make it one patch per topic. A review of your patch will follow.
> 
> It's too late now.
> This is difficult to separete patch per topic from current our CAN driver.
> 
> Next time posting another patch, I will do like so.

Sorry, but there is little chance to get a "this patch changes about 10
things" into the kernel, please try to split up the patch.

regards Marc

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


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

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [PATCH net-next-2.6 v2] can: Topcliff: PCH_CAN driver: Add Flow control,
From: Tomoya MORINAGA @ 2010-11-16  8:25 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Masayuki Ohtake,
	Samuel Ortiz, margie.foster-ral2JQCrhuEAvxtiuMwx3w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	yong.y.wang-ral2JQCrhuEAvxtiuMwx3w,
	kok.howg.ewe-ral2JQCrhuEAvxtiuMwx3w,
	joel.clark-ral2JQCrhuEAvxtiuMwx3w, David S. Miller,
	Christian Pellegrin, qi.wang-ral2JQCrhuEAvxtiuMwx3w
In-Reply-To: <4CE141EA.5070702@grandegger.com>

On Monday, November 15, 2010 11:21 PM, Wolfgang Grandegger wrote:

> 
> More comments to the lec handling below.
> 
> > + cf->data[6] = ioread32(&priv->regs->errc) & PCH_TEC;
> > + cf->data[7] = (ioread32(&priv->regs->errc) & PCH_REC) >> 8;
> 
> Could be handle with just *one* register access.

I will modify.

>   if (reg_stat & PCH_BUS_OFF ||
>     (reg_stat & PCH_LEC_ALL) != PCH_LEC_ALL) {
> 
> Your lec handling is still not correc, 

I will modify like above.

> I believe. The driver needs to
> write PCH_LEC_ALL to the "stat" register once in the initialization code
This is NOT true.
I heard even if CAN driver detects PCH_LEC_ALL, the driver doesn't have to
write PCH_LEC_ALL to LEC.
"PCH_LEC_ALL" means there is no error event.
In case error is occurred, lec value is updated automatically.


> and then after each error observed (lec != PCH_LEC_ALL). I still do not
> find such code. Could you show us the output of
> 
>   "# candump any,0:0,#FFFFFFFF"
> 
> when yo send CAN messages *without* a cable connected?.


[root@localhost can-utils]# ./candump any,0:0,#FFFFFFFF
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME
  can0  20000024  [8] 00 28 00 00 00 00 88 00   ERRORFRAME

......It seems the same line continues forever.

---
Thanks,

Tomoya MORINAGA
OKI SEMICONDUCTOR CO., LTD.

^ 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