* Re: [PATCH] net: add new QCA alx ethernet driver
From: David Miller @ 2012-08-09 6:18 UTC (permalink / raw)
To: cjren
Cc: joe, netdev, linux-kernel, qca-linux-team, nic-devel, xiong,
hao-ran.liu, rodrigue
In-Reply-To: <6349D7A510622448B1BA0967850A843801369364@NASANEXD02D.na.qualcomm.com>
From: "Ren, Cloud" <cjren@qca.qualcomm.com>
Date: Thu, 9 Aug 2012 04:44:08 +0000
> I only want to do some additional explanations to david. It makes nothing with original patch.
Stop this TOP POSTING, it's bad etiquette on a technical mailing list.
Look at how other people write their email responses on this mailing
list. They first provide a quoted portion of what they are replying
to:
> Quoted
> something like this.
And then they provide their response UNDERNEATH.
When you provide your response at the top of the email, it is unexpected,
and makes more work for people reading what you write.
You guys are doing everything possibly wrong.
You have unnecessarily quoted a HUGE patch just to provide some ancillary
in formation.
You are TOP POSTING.
You added a signoff of someone who did not give you explicit permission
to do so.
And you seem to not comprehend nor care about the implications or effects
of your actions that are hurting other people and creating a huge brudon.
I very much intend to simply ignore your work completely until you can
get your act in gear, write proper emails, and submit patches
properly.
It is the only deterrant I have for your incredibly rude and
inconsiderate behavior.
^ permalink raw reply
* Re: [PATCH net-next 0/7] sctp: network namespace support Part 2: per net tunables
From: David Miller @ 2012-08-09 6:20 UTC (permalink / raw)
To: ebiederm
Cc: vyasevich, linux-sctp, netdev, linux-kernel, jan.ariyasu,
jan.ariyasu, nhorman, tgraf, xi.wang
In-Reply-To: <87ipcud3ld.fsf_-_@xmission.com>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Tue, 07 Aug 2012 10:17:02 -0700
> Since I am motivated to get things done, and since there has been much
> grumbling about my patches not implementing tunables, I have added
> tunable support on top of my last patchset.
>
> I have performed basic testing on the these patches and nothing
> appears amis.
>
> The sm statemachine is a major tease as it has all of these association
> and endpoint pointers in the common set of function parameters that turn
> out to be NULL at the most inconvinient times. So I added to the common
> parameter list a struct net pointer, that is never NULL.
I like Eric's patch set and I'd like to apply it to net-next.
Vlad?
^ permalink raw reply
* [PATCH][XFRM][v2] Replace rwlock on xfrm_policy_afinfo with rcu
From: Priyanka Jain @ 2012-08-09 6:21 UTC (permalink / raw)
To: netdev, davem; +Cc: Priyanka Jain
xfrm_policy_afinfo is read mosly data structure.
Write on xfrm_policy_afinfo is done only at the
time of configuration.
So rwlocks can be safely replaced with RCU.
RCUs usage optimizes the performance.
Signed-off-by: Priyanka Jain <Priyanka.Jain@freescale.com>
---
Changes for v2:
Re-spined to netdev-next & corrected indentation as suggested by David
For IPSEC fwd test
-On p4080ds (8-core, SMP system)
Around 110% throughput increase in case of PREEMPT_RT enabled
Around 5-6% throughput increase in case of PREEMPT_RT disabled
-On p2020 (2-core, SMP system)
Around 4-5% throughput increase in case of PREEMPT_RT disabled
net/xfrm/xfrm_policy.c | 39 ++++++++++++++++++++++-----------------
1 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index c5a5165..39c4943 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -42,13 +42,14 @@ static DEFINE_SPINLOCK(xfrm_policy_sk_bundle_lock);
static struct dst_entry *xfrm_policy_sk_bundles;
static DEFINE_RWLOCK(xfrm_policy_lock);
-static DEFINE_RWLOCK(xfrm_policy_afinfo_lock);
-static struct xfrm_policy_afinfo *xfrm_policy_afinfo[NPROTO];
+static DEFINE_SPINLOCK(xfrm_policy_afinfo_lock);
+static struct xfrm_policy_afinfo __rcu *xfrm_policy_afinfo[NPROTO]
+ __read_mostly;
static struct kmem_cache *xfrm_dst_cache __read_mostly;
static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family);
-static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo);
+static inline void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo);
static void xfrm_init_pmtu(struct dst_entry *dst);
static int stale_bundle(struct dst_entry *dst);
static int xfrm_bundle_ok(struct xfrm_dst *xdst);
@@ -2418,7 +2419,7 @@ int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo)
return -EINVAL;
if (unlikely(afinfo->family >= NPROTO))
return -EAFNOSUPPORT;
- write_lock_bh(&xfrm_policy_afinfo_lock);
+ spin_lock_bh(&xfrm_policy_afinfo_lock);
if (unlikely(xfrm_policy_afinfo[afinfo->family] != NULL))
err = -ENOBUFS;
else {
@@ -2439,9 +2440,9 @@ int xfrm_policy_register_afinfo(struct xfrm_policy_afinfo *afinfo)
dst_ops->neigh_lookup = xfrm_neigh_lookup;
if (likely(afinfo->garbage_collect == NULL))
afinfo->garbage_collect = xfrm_garbage_collect_deferred;
- xfrm_policy_afinfo[afinfo->family] = afinfo;
+ rcu_assign_pointer(xfrm_policy_afinfo[afinfo->family], afinfo);
}
- write_unlock_bh(&xfrm_policy_afinfo_lock);
+ spin_unlock_bh(&xfrm_policy_afinfo_lock);
rtnl_lock();
for_each_net(net) {
@@ -2474,13 +2475,14 @@ int xfrm_policy_unregister_afinfo(struct xfrm_policy_afinfo *afinfo)
return -EINVAL;
if (unlikely(afinfo->family >= NPROTO))
return -EAFNOSUPPORT;
- write_lock_bh(&xfrm_policy_afinfo_lock);
+ spin_lock_bh(&xfrm_policy_afinfo_lock);
if (likely(xfrm_policy_afinfo[afinfo->family] != NULL)) {
if (unlikely(xfrm_policy_afinfo[afinfo->family] != afinfo))
err = -EINVAL;
else {
struct dst_ops *dst_ops = afinfo->dst_ops;
- xfrm_policy_afinfo[afinfo->family] = NULL;
+ rcu_assign_pointer(xfrm_policy_afinfo[afinfo->family],
+ NULL);
dst_ops->kmem_cachep = NULL;
dst_ops->check = NULL;
dst_ops->negative_advice = NULL;
@@ -2488,7 +2490,8 @@ int xfrm_policy_unregister_afinfo(struct xfrm_policy_afinfo *afinfo)
afinfo->garbage_collect = NULL;
}
}
- write_unlock_bh(&xfrm_policy_afinfo_lock);
+ spin_unlock_bh(&xfrm_policy_afinfo_lock);
+ synchronize_rcu();
return err;
}
EXPORT_SYMBOL(xfrm_policy_unregister_afinfo);
@@ -2497,8 +2500,9 @@ static void __net_init xfrm_dst_ops_init(struct net *net)
{
struct xfrm_policy_afinfo *afinfo;
- read_lock_bh(&xfrm_policy_afinfo_lock);
- afinfo = xfrm_policy_afinfo[AF_INET];
+ local_bh_disable();
+ rcu_read_lock();
+ afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET]);
if (afinfo)
net->xfrm.xfrm4_dst_ops = *afinfo->dst_ops;
#if IS_ENABLED(CONFIG_IPV6)
@@ -2506,7 +2510,8 @@ static void __net_init xfrm_dst_ops_init(struct net *net)
if (afinfo)
net->xfrm.xfrm6_dst_ops = *afinfo->dst_ops;
#endif
- read_unlock_bh(&xfrm_policy_afinfo_lock);
+ rcu_read_unlock();
+ local_bh_enable();
}
static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family)
@@ -2514,16 +2519,16 @@ static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family)
struct xfrm_policy_afinfo *afinfo;
if (unlikely(family >= NPROTO))
return NULL;
- read_lock(&xfrm_policy_afinfo_lock);
- afinfo = xfrm_policy_afinfo[family];
+ rcu_read_lock();
+ afinfo = rcu_dereference(xfrm_policy_afinfo[family]);
if (unlikely(!afinfo))
- read_unlock(&xfrm_policy_afinfo_lock);
+ rcu_read_unlock();
return afinfo;
}
-static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo)
+static inline void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo)
{
- read_unlock(&xfrm_policy_afinfo_lock);
+ rcu_read_unlock();
}
static int xfrm_dev_event(struct notifier_block *this, unsigned long event, void *ptr)
--
1.7.4.1
^ permalink raw reply related
* RE: [PATCH] net: add new QCA alx ethernet driver
From: Ren, Cloud @ 2012-08-09 6:49 UTC (permalink / raw)
To: David Miller
Cc: joe@perches.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, qca-linux-team, nic-devel,
Huang, Xiong, hao-ran.liu@canonical.com, Rodriguez, Luis
In-Reply-To: <20120808.231804.12694947300243733.davem@davemloft.net>
>-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net]
>Sent: 2012年8月9日 14:18
>To: Ren, Cloud
>Cc: joe@perches.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
>qca-linux-team; nic-devel; Huang, Xiong; hao-ran.liu@canonical.com; Rodriguez,
>Luis
>Subject: Re: [PATCH] net: add new QCA alx ethernet driver
>
>From: "Ren, Cloud" <cjren@qca.qualcomm.com>
>Date: Thu, 9 Aug 2012 04:44:08 +0000
>
>> I only want to do some additional explanations to david. It makes nothing with
>original patch.
>
>Stop this TOP POSTING, it's bad etiquette on a technical mailing list.
>
>Look at how other people write their email responses on this mailing list. They
>first provide a quoted portion of what they are replying
>to:
>
>> Quoted
>> something like this.
>
>And then they provide their response UNDERNEATH.
>
>When you provide your response at the top of the email, it is unexpected, and
>makes more work for people reading what you write.
>
>You guys are doing everything possibly wrong.
>
>You have unnecessarily quoted a HUGE patch just to provide some ancillary in
>formation.
>
>You are TOP POSTING.
>
>You added a signoff of someone who did not give you explicit permission to do
>so.
>
>And you seem to not comprehend nor care about the implications or effects of
>your actions that are hurting other people and creating a huge brudon.
>
>I very much intend to simply ignore your work completely until you can get your
>act in gear, write proper emails, and submit patches properly.
>
>It is the only deterrant I have for your incredibly rude and inconsiderate
>behavior.
Hi David,
Please calm down. I will follow rule of sending email from now on. Sorry for causing
your trouble. Luis has ever asked joe for adding as a Signed-off-by. At that time joe agreed
with it. If joe prefers his name not be listed as a Signed-off-by, I will respect his decision.
alx driver is innocent. Please help review. Thanks
cloud
^ permalink raw reply
* [PATCH net-next] time: jiffies_delta_to_clock_t() helper to the rescue
From: Eric Dumazet @ 2012-08-09 7:13 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-kernel, Thomas Gleixner, Maciej Żenczykowski,
Paul Gortmaker, Andrew Morton, hank
From: Eric Dumazet <edumazet@google.com>
Various /proc/net files sometimes report crazy timer values, expressed
in clock_t units.
This happens when an expired timer delta (expires - jiffies) is passed
to jiffies_to_clock_t().
This function has an overflow in :
return div_u64((u64)x * TICK_NSEC, NSEC_PER_SEC / USER_HZ);
commit cbbc719fccdb8cb (time: Change jiffies_to_clock_t() argument type
to unsigned long) only got around the problem.
As we cant output negative values in /proc/net/tcp without breaking
various tools, I suggest adding a jiffies_delta_to_clock_t() wrapper
that caps the negative delta to a 0 value.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Maciej Żenczykowski <maze@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: hank <pyu@redhat.com>
---
include/linux/jiffies.h | 6 ++++++
net/bridge/br_fdb.c | 2 +-
net/bridge/br_stp_timer.c | 2 +-
net/core/rtnetlink.c | 2 +-
net/ipv4/igmp.c | 7 +++++--
net/ipv4/tcp_ipv4.c | 13 +++++--------
net/ipv6/tcp_ipv6.c | 9 +++------
7 files changed, 22 insertions(+), 19 deletions(-)
diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
index 265e2c3..aded9b1 100644
--- a/include/linux/jiffies.h
+++ b/include/linux/jiffies.h
@@ -303,7 +303,13 @@ extern void jiffies_to_timespec(const unsigned long jiffies,
extern unsigned long timeval_to_jiffies(const struct timeval *value);
extern void jiffies_to_timeval(const unsigned long jiffies,
struct timeval *value);
+
extern clock_t jiffies_to_clock_t(unsigned long x);
+static inline clock_t jiffies_delta_to_clock_t(long delta)
+{
+ return jiffies_to_clock_t(max(0L, delta));
+}
+
extern unsigned long clock_t_to_jiffies(unsigned long x);
extern u64 jiffies_64_to_clock_t(u64 x);
extern u64 nsec_to_clock_t(u64 x);
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index d21f323..9ce430b 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -312,7 +312,7 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
fe->is_local = f->is_local;
if (!f->is_static)
- fe->ageing_timer_value = jiffies_to_clock_t(jiffies - f->updated);
+ fe->ageing_timer_value = jiffies_delta_to_clock_t(jiffies - f->updated);
++fe;
++num;
}
diff --git a/net/bridge/br_stp_timer.c b/net/bridge/br_stp_timer.c
index a6747e6..c3530a8 100644
--- a/net/bridge/br_stp_timer.c
+++ b/net/bridge/br_stp_timer.c
@@ -170,5 +170,5 @@ void br_stp_port_timer_init(struct net_bridge_port *p)
unsigned long br_timer_value(const struct timer_list *timer)
{
return timer_pending(timer)
- ? jiffies_to_clock_t(timer->expires - jiffies) : 0;
+ ? jiffies_delta_to_clock_t(timer->expires - jiffies) : 0;
}
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 2c5a0a0..db037c9 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -618,7 +618,7 @@ int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst, u32 id,
long expires, u32 error)
{
struct rta_cacheinfo ci = {
- .rta_lastuse = jiffies_to_clock_t(jiffies - dst->lastuse),
+ .rta_lastuse = jiffies_delta_to_clock_t(jiffies - dst->lastuse),
.rta_used = dst->__use,
.rta_clntref = atomic_read(&(dst->__refcnt)),
.rta_error = error,
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 6699f23..0b5580c 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -2435,6 +2435,8 @@ static int igmp_mc_seq_show(struct seq_file *seq, void *v)
struct ip_mc_list *im = (struct ip_mc_list *)v;
struct igmp_mc_iter_state *state = igmp_mc_seq_private(seq);
char *querier;
+ long delta;
+
#ifdef CONFIG_IP_MULTICAST
querier = IGMP_V1_SEEN(state->in_dev) ? "V1" :
IGMP_V2_SEEN(state->in_dev) ? "V2" :
@@ -2448,11 +2450,12 @@ static int igmp_mc_seq_show(struct seq_file *seq, void *v)
state->dev->ifindex, state->dev->name, state->in_dev->mc_count, querier);
}
+ delta = im->timer.expires - jiffies;
seq_printf(seq,
"\t\t\t\t%08X %5d %d:%08lX\t\t%d\n",
im->multiaddr, im->users,
- im->tm_running, im->tm_running ?
- jiffies_to_clock_t(im->timer.expires-jiffies) : 0,
+ im->tm_running,
+ im->tm_running ? jiffies_delta_to_clock_t(delta) : 0,
im->reporter);
}
return 0;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 42b2a6a..c660d2c 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2385,7 +2385,7 @@ static void get_openreq4(const struct sock *sk, const struct request_sock *req,
struct seq_file *f, int i, int uid, int *len)
{
const struct inet_request_sock *ireq = inet_rsk(req);
- int ttd = req->expires - jiffies;
+ long delta = req->expires - jiffies;
seq_printf(f, "%4d: %08X:%04X %08X:%04X"
" %02X %08X:%08X %02X:%08lX %08X %5d %8d %u %d %pK%n",
@@ -2397,7 +2397,7 @@ static void get_openreq4(const struct sock *sk, const struct request_sock *req,
TCP_SYN_RECV,
0, 0, /* could print option size, but that is af dependent. */
1, /* timers active (only the expire timer) */
- jiffies_to_clock_t(ttd),
+ jiffies_delta_to_clock_t(delta),
req->retrans,
uid,
0, /* non standard timer */
@@ -2448,7 +2448,7 @@ static void get_tcp4_sock(struct sock *sk, struct seq_file *f, int i, int *len)
tp->write_seq - tp->snd_una,
rx_queue,
timer_active,
- jiffies_to_clock_t(timer_expires - jiffies),
+ jiffies_delta_to_clock_t(timer_expires - jiffies),
icsk->icsk_retransmits,
sock_i_uid(sk),
icsk->icsk_probes_out,
@@ -2467,10 +2467,7 @@ static void get_timewait4_sock(const struct inet_timewait_sock *tw,
{
__be32 dest, src;
__u16 destp, srcp;
- int ttd = tw->tw_ttd - jiffies;
-
- if (ttd < 0)
- ttd = 0;
+ long delta = tw->tw_ttd - jiffies;
dest = tw->tw_daddr;
src = tw->tw_rcv_saddr;
@@ -2480,7 +2477,7 @@ static void get_timewait4_sock(const struct inet_timewait_sock *tw,
seq_printf(f, "%4d: %08X:%04X %08X:%04X"
" %02X %08X:%08X %02X:%08lX %08X %5d %8d %d %d %pK%n",
i, src, srcp, dest, destp, tw->tw_substate, 0, 0,
- 3, jiffies_to_clock_t(ttd), 0, 0, 0, 0,
+ 3, jiffies_delta_to_clock_t(delta), 0, 0, 0, 0,
atomic_read(&tw->tw_refcnt), tw, len);
}
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index c66b90f..aa41b0e 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1875,7 +1875,7 @@ static void get_tcp6_sock(struct seq_file *seq, struct sock *sp, int i)
tp->write_seq-tp->snd_una,
(sp->sk_state == TCP_LISTEN) ? sp->sk_ack_backlog : (tp->rcv_nxt - tp->copied_seq),
timer_active,
- jiffies_to_clock_t(timer_expires - jiffies),
+ jiffies_delta_to_clock_t(timer_expires - jiffies),
icsk->icsk_retransmits,
sock_i_uid(sp),
icsk->icsk_probes_out,
@@ -1895,10 +1895,7 @@ static void get_timewait6_sock(struct seq_file *seq,
const struct in6_addr *dest, *src;
__u16 destp, srcp;
const struct inet6_timewait_sock *tw6 = inet6_twsk((struct sock *)tw);
- int ttd = tw->tw_ttd - jiffies;
-
- if (ttd < 0)
- ttd = 0;
+ long delta = tw->tw_ttd - jiffies;
dest = &tw6->tw_v6_daddr;
src = &tw6->tw_v6_rcv_saddr;
@@ -1914,7 +1911,7 @@ static void get_timewait6_sock(struct seq_file *seq,
dest->s6_addr32[0], dest->s6_addr32[1],
dest->s6_addr32[2], dest->s6_addr32[3], destp,
tw->tw_substate, 0, 0,
- 3, jiffies_to_clock_t(ttd), 0, 0, 0, 0,
+ 3, jiffies_delta_to_clock_t(delta), 0, 0, 0, 0,
atomic_read(&tw->tw_refcnt), tw);
}
^ permalink raw reply related
* [PATCH 1/5] net: Dont use ifindices in hash fns
From: Pavel Emelyanov @ 2012-08-09 7:52 UTC (permalink / raw)
To: David Miller, Eric Dumazet, Eric W. Biederman, Linux Netdev List,
Ben Hutchings
In-Reply-To: <50236C20.4050108@parallels.com>
Eric noticed, that when there will be devices with equal indices, some
hash functions that use them will become less effective as they could.
Fix this in advance by mixing the net_device address into the hash value
instead of the device index.
This is true for arp and ndisc hash fns. The netlabel, can and llc ones
are also ifindex-based, but that three are init_net-only, thus will not
be affected.
Many thanks to David and Eric for the hash32_ptr implementation!
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/hash.h | 10 ++++++++++
include/net/arp.h | 3 ++-
include/net/ndisc.h | 3 ++-
3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/include/linux/hash.h b/include/linux/hash.h
index b80506b..24df9e7 100644
--- a/include/linux/hash.h
+++ b/include/linux/hash.h
@@ -67,4 +67,14 @@ static inline unsigned long hash_ptr(const void *ptr, unsigned int bits)
{
return hash_long((unsigned long)ptr, bits);
}
+
+static inline u32 hash32_ptr(const void *ptr)
+{
+ unsigned long val = (unsigned long)ptr;
+
+#if BITS_PER_LONG == 64
+ val ^= (val >> 32);
+#endif
+ return (u32)val;
+}
#endif /* _LINUX_HASH_H */
diff --git a/include/net/arp.h b/include/net/arp.h
index 7f7df93..b630dae 100644
--- a/include/net/arp.h
+++ b/include/net/arp.h
@@ -3,6 +3,7 @@
#define _ARP_H
#include <linux/if_arp.h>
+#include <linux/hash.h>
#include <net/neighbour.h>
@@ -10,7 +11,7 @@ extern struct neigh_table arp_tbl;
static inline u32 arp_hashfn(u32 key, const struct net_device *dev, u32 hash_rnd)
{
- u32 val = key ^ dev->ifindex;
+ u32 val = key ^ hash32_ptr(dev);
return val * hash_rnd;
}
diff --git a/include/net/ndisc.h b/include/net/ndisc.h
index 96a3b5c..980d263 100644
--- a/include/net/ndisc.h
+++ b/include/net/ndisc.h
@@ -49,6 +49,7 @@ enum {
#include <linux/types.h>
#include <linux/if_arp.h>
#include <linux/netdevice.h>
+#include <linux/hash.h>
#include <net/neighbour.h>
@@ -134,7 +135,7 @@ static inline u32 ndisc_hashfn(const void *pkey, const struct net_device *dev, _
{
const u32 *p32 = pkey;
- return (((p32[0] ^ dev->ifindex) * hash_rnd[0]) +
+ return (((p32[0] ^ hash32_ptr(dev)) * hash_rnd[0]) +
(p32[1] * hash_rnd[1]) +
(p32[2] * hash_rnd[2]) +
(p32[3] * hash_rnd[3]));
--
1.7.6.5
^ permalink raw reply related
* Re: [PATCH v6 7/7] ARM: davinci: add support for the am1808 based enbw_cmc board
From: Heiko Schocher @ 2012-08-09 7:52 UTC (permalink / raw)
To: Sekhar Nori
Cc: davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
David Woodhouse, Ben Dooks, Wolfram Sang, Kevin Hilman,
Wolfgang Denk, Scott Wood, Sylwester Nawrocki
In-Reply-To: <5022A367.1070903-l0cyMroinI0@public.gmane.org>
Hello Nori,
On 08.08.2012 19:35, Sekhar Nori wrote:
> Hi Heiko,
>
> On 7/5/2012 6:20 PM, Heiko Schocher wrote:
>> - AM1808 based board
>> - 64 MiB DDR ram
>> - 2 MiB Nor flash
>> - 128 MiB NAND flash
>> - use internal RTC
>> - I2C support
>> - hwmon lm75 support
>> - UBI/UBIFS support
>> - MMC support
>> - USB OTG support
>>
>> Signed-off-by: Heiko Schocher<hs-ynQEQJNshbs@public.gmane.org>
>> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>> Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> Cc: davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org
>> Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: David Woodhouse<dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
>> Cc: Ben Dooks<ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org>
>> Cc: Wolfram Sang<w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> Cc: Sekhar Nori<nsekhar-l0cyMroinI0@public.gmane.org>
>> Cc: Kevin Hilman<khilman-l0cyMroinI0@public.gmane.org>
>> Cc: Wolfgang Denk<wd-ynQEQJNshbs@public.gmane.org>
>> Cc: Scott Wood<scottwood-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> Cc: Sylwester Nawrocki<s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>>
>> ---
>> - post this board support with USB support, even though
>> USB is only working with the 10 ms "workaround", posted here:
>> http://comments.gmane.org/gmane.linux.usb.general/54505
>> I see this issue also on the AM1808 TMDXEXP1808L evalboard.
>> - MMC and USB are not using OF support yet, ideas how to port
>> this are welcome. I need for USB and MMC boards board
>> specific callbacks, how to solve this with OF support?
>>
>> - changes for v2:
>> - changes in the nand node due to comments from Scott Wood:
>> - add "ti,davinci-" prefix
>> - Dashes are preferred to underscores
>> - rename "nandflash" to "nand"
>> - introduce new "ti,davinci" specific properties for setting
>> up ecc_mode, ecc_bits, options and bbt options, instead
>> using linux defines
>> - changes for i2c due to comments from Sylwester Nawrocki:
>> - use "cell-index" instead "id"
>> - OF_DEV_AUXDATA in the machine code, instead pre-define
>> platform device name
>> - add comment from Grant Likely for i2c:
>> - removed "id" resp. "cell-index" completely
>> - fixed documentation
>> - use of_match_ptr()
>> - use devm_kzalloc() for allocating plattform data mem
>> - fixed a whitespace issue
>> - add net comments from Grant Likely:
>> - add prefix "ti,davinci-" to davinci specific property names
>> - remove version property
>> - use compatible name "ti,davinci-dm6460-emac"
>> - add comment from Grant Likely:
>> - rename compatible node
>> - do not use cell-index
>> - CONFIG_OF required for this board
>> TODO:
>> - create a generic board support file, as I got no
>> answer to my ping to grant, maybe this could be done
>> in a second step?
>> - changes for v3:
>> - add comments from Sergei Shtylyov:
>> - rename compatible" prop to "ti,cp_intc"
>> - cp_intc_init now used for Interrupt controller init
>> - changes for v4:
>> add comment from Nori Sekhar:
>> - rename davinci emac compatible property to "ti,davinci-dm6467-emac"
>> - remove "pinmux-handle" property as discussed here:
>> http://www.spinics.net/lists/arm-kernel/msg175701.html
>> with Nori Sekhar
>>
>> - changes for v5:
>> add comments from Grant Likely:
>> - rename compatible" prop to "ti,cp-intc"
>>
>> - changes for v6:
>> rework this patch, as patch
>> ARM: davinci: cp_intc: Add OF support for TI interrupt controller
>> was changed from Nori Sekhar on Jul 03, 2012; 9:16pm
>> Changes therefore in this patch:
>>
>> Call of_irq_init() in the generic DT board file and not in the
>> interrupt controller code. See arch/arm/mach-at91/board-dt.c or
>> arch/arm/mach-omap2/board-generic.c for examples.
>>
>> At this point the question raises, if we should rename this
>> board port from arch/arm/mach-davinci/enbw_cmc.c to
>> arch/arm/mach-davinci/board-dt.c ? Also the defconfig to
>> davinci_of_defconfig ... ? The USB and MMC callbacks are
>> currently board specific, but if other boards come in,
>> that could be easily adapted for their needs ...
>>
>> arch/arm/boot/dts/enbw_cmc.dts | 183 +++++++++++
>> arch/arm/configs/enbw_cmc_defconfig | 126 ++++++++
>> arch/arm/mach-davinci/Kconfig | 9 +
>> arch/arm/mach-davinci/Makefile | 1 +
>> arch/arm/mach-davinci/board-enbw-cmc.c | 385 +++++++++++++++++++++++
>> arch/arm/mach-davinci/include/mach/uncompress.h | 1 +
>> 6 files changed, 705 insertions(+), 0 deletions(-)
>> create mode 100644 arch/arm/boot/dts/enbw_cmc.dts
>> create mode 100644 arch/arm/configs/enbw_cmc_defconfig
>> create mode 100644 arch/arm/mach-davinci/board-enbw-cmc.c
>
> With the move to DT, we are not going to add more board specific files
> and defconfigs. I am working on adding a generic dt board file for DA850
> (similar to what has been done for imx). That file is not going to do
> any platform device registrations. If you are OK with it, I will add the
> dts file for enbw_cmc board in the patch series I am planning to post.
I am fine with that. If you have a working version, you can sent it
(if you want) to me, so I can test it on the enbw_cmc board.
> Things like USB and MMC/SD will need to be converted to DT before they
> can be supported on new boards.
Ok, understood.
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply
* [PATCH net-next 0/5] Per-net and on-demand link indices (and related) v3
From: Pavel Emelyanov @ 2012-08-09 7:52 UTC (permalink / raw)
To: David Miller, Eric Dumazet, Eric W. Biederman, Linux Netdev List,
Ben Hutchings
Hi!
This set tries to summarize the recent discussion of making ifindices friendly
to checkpoint-restore and consists of:
1. Prepare hash function to non-unique ifindices
2. Allow for specifying the desired ifindex on net link creation
3. Make ifindex generation per-net
4. Simplify loopback device ifindex access
Changes since v2:
* The pointer hash mix routine is rewritten
* Put ifindex on struct net in a better place
* Fixed compatibility on veth peer device ifindex configuration
Changes since v1:
* Fixed a stupid mistake with a pointer bits shift
* Turned the netdev_hash_mix routine into the generic ptr_hash_mix one
* Added a comment describing why loopback index is always 1 after the patch
Thanks,
Pavel
^ permalink raw reply
* [PATCH 2/5] net: Allow to create links with given ifindex
From: Pavel Emelyanov @ 2012-08-09 7:52 UTC (permalink / raw)
To: David Miller, Eric Dumazet, Eric W. Biederman, Linux Netdev List,
Ben Hutchings
In-Reply-To: <50236C20.4050108@parallels.com>
Currently the RTM_NEWLINK results in -EOPNOTSUPP if the ifinfomsg->ifi_index
is not zero. I propose to allow requesting ifindices on link creation. This
is required by the checkpoint-restore to correctly restore a net namespace
(i.e. -- a container).
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Acked-by: Eric Dumazet <edumazet@google.com>
---
net/core/dev.c | 7 ++++++-
net/core/rtnetlink.c | 12 +++++++-----
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index f91abf8..3ca300d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5579,7 +5579,12 @@ int register_netdevice(struct net_device *dev)
}
}
- dev->ifindex = dev_new_index(net);
+ ret = -EBUSY;
+ if (!dev->ifindex)
+ dev->ifindex = dev_new_index(net);
+ else if (__dev_get_by_index(net, dev->ifindex))
+ goto err_uninit;
+
if (dev->iflink == -1)
dev->iflink = dev->ifindex;
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 2c5a0a0..1aa1456 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1812,8 +1812,6 @@ replay:
return -ENODEV;
}
- if (ifm->ifi_index)
- return -EOPNOTSUPP;
if (tb[IFLA_MAP] || tb[IFLA_MASTER] || tb[IFLA_PROTINFO])
return -EOPNOTSUPP;
@@ -1839,10 +1837,14 @@ replay:
return PTR_ERR(dest_net);
dev = rtnl_create_link(net, dest_net, ifname, ops, tb);
-
- if (IS_ERR(dev))
+ if (IS_ERR(dev)) {
err = PTR_ERR(dev);
- else if (ops->newlink)
+ goto out;
+ }
+
+ dev->ifindex = ifm->ifi_index;
+
+ if (ops->newlink)
err = ops->newlink(net, dev, tb, data);
else
err = register_netdevice(dev);
--
1.7.6.5
^ permalink raw reply related
* [PATCH 3/5] veth: Allow to create peer link with given ifindex
From: Pavel Emelyanov @ 2012-08-09 7:53 UTC (permalink / raw)
To: David Miller, Eric Dumazet, Eric W. Biederman, Linux Netdev List,
Ben Hutchings
In-Reply-To: <50236C20.4050108@parallels.com>
The ifinfomsg is in there (thanks kaber@ for foreseeing this long time ago),
so take the given ifidex and register netdev with it.
Ben noticed, that this code path previously ignored ifmp->ifi_index and
userland could be passing in garbage. Thus it may now fail occasionally
because the value clashes with an existing interface.
To address this it's assumed that if the caller specifies the ifindex for
the veth master device, then it's aware of this possibility and should
explicitly specify (or set to 0 for auto-assignment) the peer's ifindex as
well. With this the compatibility with old tools not setting ifindex is
preserved.
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
---
drivers/net/veth.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 5852361..e522ff7 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -348,6 +348,9 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
if (tbp[IFLA_ADDRESS] == NULL)
eth_hw_addr_random(peer);
+ if (ifmp && (dev->ifindex != 0))
+ peer->ifindex = ifmp->ifi_index;
+
err = register_netdevice(peer);
put_net(net);
net = NULL;
--
1.7.6.5
^ permalink raw reply related
* [PATCH 4/5] net: Make ifindex generation per-net namespace
From: Pavel Emelyanov @ 2012-08-09 7:53 UTC (permalink / raw)
To: David Miller, Eric Dumazet, Eric W. Biederman, Linux Netdev List,
Ben Hutchings
In-Reply-To: <50236C20.4050108@parallels.com>
Strictly speaking this is only _really_ required for checkpoint-restore to
make loopback device always have the same index.
This change appears to be safe wrt "ifindex should be unique per-system"
concept, as all the ifindex usage is either already made per net namespace
of is explicitly limited with init_net only.
There are two cool side effects of this. The first one -- ifindices of
devices in container are always small, regardless of how many containers
we've started (and re-started) so far. The second one is -- we can speed
up the loopback ifidex access as shown in the next patch.
v2: Place ifindex right after dev_base_seq : avoid two holes and use the
same cache line, dirtied in list_netdevice()/unlist_netdevice()
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Acked-by: Eric Dumazet <edumazet@google.com>
---
include/net/net_namespace.h | 1 +
net/core/dev.c | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index ae1cd6c..6dc3db3 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -66,6 +66,7 @@ struct net {
struct hlist_head *dev_name_head;
struct hlist_head *dev_index_head;
unsigned int dev_base_seq; /* protected by rtnl_mutex */
+ int ifindex;
/* core fib_rules */
struct list_head rules_ops;
diff --git a/net/core/dev.c b/net/core/dev.c
index 3ca300d..1f06df8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5221,12 +5221,12 @@ int dev_ioctl(struct net *net, unsigned int cmd, void __user *arg)
*/
static int dev_new_index(struct net *net)
{
- static int ifindex;
+ int ifindex = net->ifindex;
for (;;) {
if (++ifindex <= 0)
ifindex = 1;
if (!__dev_get_by_index(net, ifindex))
- return ifindex;
+ return net->ifindex = ifindex;
}
}
--
1.7.6.5
^ permalink raw reply related
* [PATCH 5/5] net: Loopback ifindex is constant now
From: Pavel Emelyanov @ 2012-08-09 7:53 UTC (permalink / raw)
To: David Miller, Eric Dumazet, Eric W. Biederman, Linux Netdev List,
Ben Hutchings
In-Reply-To: <50236C20.4050108@parallels.com>
As pointed out, there are places, that access net->loopback_dev->ifindex
and after ifindex generation is made per-net this value becomes constant
equals 1. So go ahead and introduce the LOOPBACK_IFINDEX constant and use
it where appropriate.
Signed-off-by: Pavel Emelyanov <xemul@parallels.com>
Acked-by: Eric Dumazet <edumazet@google.com>
---
drivers/net/loopback.c | 1 +
include/net/net_namespace.h | 7 +++++++
net/decnet/dn_route.c | 6 +++---
net/ipv4/fib_frontend.c | 2 +-
net/ipv4/ipmr.c | 2 +-
net/ipv4/netfilter/ipt_rpfilter.c | 2 +-
net/ipv4/route.c | 6 +++---
net/ipv6/route.c | 2 +-
8 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index e2a06fd..4a075ba 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -197,6 +197,7 @@ static __net_init int loopback_net_init(struct net *net)
if (err)
goto out_free_netdev;
+ BUG_ON(dev->ifindex != LOOPBACK_IFINDEX);
net->loopback_dev = dev;
return 0;
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 6dc3db3..97e4419 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -105,6 +105,13 @@ struct net {
struct sock *diag_nlsk;
};
+/*
+ * ifindex generation is per-net namespace, and loopback is
+ * always the 1st device in ns (see net_dev_init), thus any
+ * loopback device should get ifindex 1
+ */
+
+#define LOOPBACK_IFINDEX 1
#include <linux/seq_file_net.h>
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 85a3604..c855e8d 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -961,7 +961,7 @@ static int dn_route_output_slow(struct dst_entry **pprt, const struct flowidn *o
.saddr = oldflp->saddr,
.flowidn_scope = RT_SCOPE_UNIVERSE,
.flowidn_mark = oldflp->flowidn_mark,
- .flowidn_iif = init_net.loopback_dev->ifindex,
+ .flowidn_iif = LOOPBACK_IFINDEX,
.flowidn_oif = oldflp->flowidn_oif,
};
struct dn_route *rt = NULL;
@@ -979,7 +979,7 @@ static int dn_route_output_slow(struct dst_entry **pprt, const struct flowidn *o
"dn_route_output_slow: dst=%04x src=%04x mark=%d"
" iif=%d oif=%d\n", le16_to_cpu(oldflp->daddr),
le16_to_cpu(oldflp->saddr),
- oldflp->flowidn_mark, init_net.loopback_dev->ifindex,
+ oldflp->flowidn_mark, LOOPBACK_IFINDEX,
oldflp->flowidn_oif);
/* If we have an output interface, verify its a DECnet device */
@@ -1042,7 +1042,7 @@ source_ok:
if (!fld.daddr)
goto out;
}
- fld.flowidn_oif = init_net.loopback_dev->ifindex;
+ fld.flowidn_oif = LOOPBACK_IFINDEX;
res.type = RTN_LOCAL;
goto make_route;
}
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index c43ae3f..7f073a3 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -218,7 +218,7 @@ __be32 fib_compute_spec_dst(struct sk_buff *skb)
scope = RT_SCOPE_UNIVERSE;
if (!ipv4_is_zeronet(ip_hdr(skb)->saddr)) {
fl4.flowi4_oif = 0;
- fl4.flowi4_iif = net->loopback_dev->ifindex;
+ fl4.flowi4_iif = LOOPBACK_IFINDEX;
fl4.daddr = ip_hdr(skb)->saddr;
fl4.saddr = 0;
fl4.flowi4_tos = RT_TOS(ip_hdr(skb)->tos);
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 8eec8f4..3a57570 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1798,7 +1798,7 @@ static struct mr_table *ipmr_rt_fib_lookup(struct net *net, struct sk_buff *skb)
.flowi4_oif = (rt_is_output_route(rt) ?
skb->dev->ifindex : 0),
.flowi4_iif = (rt_is_output_route(rt) ?
- net->loopback_dev->ifindex :
+ LOOPBACK_IFINDEX :
skb->dev->ifindex),
.flowi4_mark = skb->mark,
};
diff --git a/net/ipv4/netfilter/ipt_rpfilter.c b/net/ipv4/netfilter/ipt_rpfilter.c
index 31371be..c301300 100644
--- a/net/ipv4/netfilter/ipt_rpfilter.c
+++ b/net/ipv4/netfilter/ipt_rpfilter.c
@@ -85,7 +85,7 @@ static bool rpfilter_mt(const struct sk_buff *skb, struct xt_action_param *par)
return ipv4_is_local_multicast(iph->daddr) ^ invert;
flow.flowi4_iif = 0;
} else {
- flow.flowi4_iif = dev_net(par->in)->loopback_dev->ifindex;
+ flow.flowi4_iif = LOOPBACK_IFINDEX;
}
flow.daddr = iph->saddr;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 21ad369..c581373 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1619,7 +1619,7 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
if (res.type == RTN_LOCAL) {
err = fib_validate_source(skb, saddr, daddr, tos,
- net->loopback_dev->ifindex,
+ LOOPBACK_IFINDEX,
dev, in_dev, &itag);
if (err < 0)
goto martian_source_keep_err;
@@ -1895,7 +1895,7 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
orig_oif = fl4->flowi4_oif;
- fl4->flowi4_iif = net->loopback_dev->ifindex;
+ fl4->flowi4_iif = LOOPBACK_IFINDEX;
fl4->flowi4_tos = tos & IPTOS_RT_MASK;
fl4->flowi4_scope = ((tos & RTO_ONLINK) ?
RT_SCOPE_LINK : RT_SCOPE_UNIVERSE);
@@ -1984,7 +1984,7 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
if (!fl4->daddr)
fl4->daddr = fl4->saddr = htonl(INADDR_LOOPBACK);
dev_out = net->loopback_dev;
- fl4->flowi4_oif = net->loopback_dev->ifindex;
+ fl4->flowi4_oif = LOOPBACK_IFINDEX;
res.type = RTN_LOCAL;
flags |= RTCF_LOCAL;
goto make_route;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 8e80fd2..0ddf2d1 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -965,7 +965,7 @@ struct dst_entry * ip6_route_output(struct net *net, const struct sock *sk,
{
int flags = 0;
- fl6->flowi6_iif = net->loopback_dev->ifindex;
+ fl6->flowi6_iif = LOOPBACK_IFINDEX;
if ((sk && sk->sk_bound_dev_if) || rt6_need_strict(&fl6->daddr))
flags |= RT6_LOOKUP_F_IFACE;
--
1.7.6.5
^ permalink raw reply related
* RE: [PATCH] net: add new QCA alx ethernet driver
From: Joe Perches @ 2012-08-09 7:58 UTC (permalink / raw)
To: Ren, Cloud
Cc: David Miller, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, qca-linux-team, nic-devel,
Huang, Xiong, hao-ran.liu@canonical.com, Rodriguez, Luis
In-Reply-To: <6349D7A510622448B1BA0967850A8438013693DD@NASANEXD02D.na.qualcomm.com>
On Thu, 2012-08-09 at 06:49 +0000, Ren, Cloud wrote:
> Luis has ever asked joe for adding as a Signed-off-by. At that time joe agreed
> with it.
What I replied to Luis and some qca list was:
On Fri, 2012-01-20 at 19:49 -0800, Joe Perches wrote:
> On Fri, 2012-01-20 at 18:25 -0800, Luis R. Rodriguez wrote:
> > Hey Joe, do you mind if we add your SOB tag on the next RFC for
> > submission of this driver? I see all your patches were merged by Ren.
> > I want to make sure you get proper credit for your contributions.
> Whatever's appropriate.
At this time I don't think what I did amounts to a
sign-off of the whole driver.
I appreciate the thoughtfulness of trying to credit
me as a person by improving the driver a little. I
just wouldn't say I signed off on the whole thing.
cheers, Joe
^ permalink raw reply
* RE: [PATCH] net: add new QCA alx ethernet driver
From: Ren, Cloud @ 2012-08-09 8:25 UTC (permalink / raw)
To: Joe Perches
Cc: David Miller, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, qca-linux-team, nic-devel,
Huang, Xiong, hao-ran.liu@canonical.com, Rodriguez, Luis
In-Reply-To: <1344499088.2093.5.camel@joe2Laptop>
>-----Original Message-----
>From: Joe Perches [mailto:joe@perches.com]
>Sent: Thursday, August 09, 2012 3:58 PM
>
>On Thu, 2012-08-09 at 06:49 +0000, Ren, Cloud wrote:
>> Luis has ever asked joe for adding as a Signed-off-by. At that time
>> joe agreed with it.
>
>What I replied to Luis and some qca list was:
>
>On Fri, 2012-01-20 at 19:49 -0800, Joe Perches wrote:
>> On Fri, 2012-01-20 at 18:25 -0800, Luis R. Rodriguez wrote:
>> > Hey Joe, do you mind if we add your SOB tag on the next RFC for
>> > submission of this driver? I see all your patches were merged by Ren.
>> > I want to make sure you get proper credit for your contributions.
>> Whatever's appropriate.
>
>At this time I don't think what I did amounts to a sign-off of the whole driver.
>
>I appreciate the thoughtfulness of trying to credit me as a person by improving
>the driver a little. I just wouldn't say I signed off on the whole thing.
>
>cheers, Joe
>
Thanks for your help with alx driver. I will respect your any decisions. I will remove your
Signed-off-by. Thanks.
cloud
^ permalink raw reply
* GET THIS LOAN NOW!
From: Alis Finance Loan @ 2012-08-09 8:48 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2: LOAN APPLICATION FORM ONLINE One.doc --]
[-- Type: application/msword, Size: 37376 bytes --]
^ permalink raw reply
* [PATCH] tcp: must free metrics at net dismantle
From: Eric Dumazet @ 2012-08-09 9:19 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
We currently leak all tcp metrics at struct net dismantle time.
tcp_net_metrics_exit() frees the hash table, we must first
iterate it to free all metrics.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_metrics.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index 2288a63..0abe67b 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -731,6 +731,18 @@ static int __net_init tcp_net_metrics_init(struct net *net)
static void __net_exit tcp_net_metrics_exit(struct net *net)
{
+ unsigned int i;
+
+ for (i = 0; i < (1U << net->ipv4.tcp_metrics_hash_log) ; i++) {
+ struct tcp_metrics_block *tm, *next;
+
+ tm = rcu_dereference_protected(net->ipv4.tcp_metrics_hash[i].chain, 1);
+ while (tm) {
+ next = rcu_dereference_protected(tm->tcpm_next, 1);
+ kfree(tm);
+ tm = next;
+ }
+ }
kfree(net->ipv4.tcp_metrics_hash);
}
^ permalink raw reply related
* Re: [PATCH] tcp: must free metrics at net dismantle
From: David Miller @ 2012-08-09 9:31 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1344503953.28967.575.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 09 Aug 2012 11:19:13 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> We currently leak all tcp metrics at struct net dismantle time.
>
> tcp_net_metrics_exit() frees the hash table, we must first
> iterate it to free all metrics.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks Eric.
^ permalink raw reply
* [net 0/3][pull request] Intel Wired LAN Driver Updates
From: Peter P Waskiewicz Jr @ 2012-08-09 9:39 UTC (permalink / raw)
To: davem; +Cc: Peter P Waskiewicz Jr, netdev, gospo, sassmann
This series contains bugfixes to the igb and e1000e drivers.
The following are changes since commit 36471012e2ae28ca3178f84d4687a2d88a36593e:
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/ppwaskie/net master
Alexander Duyck (1):
igb: Fix register defines for all non-82575 hardware
Emil Tantilov (2):
igb: fix panic while dumping packets on Tx hang with IOMMU
e1000e: fix panic while dumping packets on Tx hang with IOMMU
drivers/net/ethernet/intel/e1000e/netdev.c | 36 +++++++++++++++++++++--------
drivers/net/ethernet/intel/igb/e1000_regs.h | 8 +++++--
drivers/net/ethernet/intel/igb/igb_main.c | 19 ++++++++-------
3 files changed, 41 insertions(+), 22 deletions(-)
--
1.7.11.2
^ permalink raw reply
* [net 1/3] igb: fix panic while dumping packets on Tx hang with IOMMU
From: Peter P Waskiewicz Jr @ 2012-08-09 9:39 UTC (permalink / raw)
To: davem; +Cc: Emil Tantilov, netdev, gospo, sassmann, Peter P Waskiewicz Jr
In-Reply-To: <1344505152-28167-1-git-send-email-peter.p.waskiewicz.jr@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
This patch resolves a "BUG: unable to handle kernel paging request at ..."
oops while dumping packet data. The issue occurs with IOMMU enabled due to
the address provided by phys_to_virt().
This patch avoids phys_to_virt() by making using skb->data and the address
of the pages allocated for Rx.
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index b7c2d50..48cc4fb 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -462,10 +462,10 @@ static void igb_dump(struct igb_adapter *adapter)
(u64)buffer_info->time_stamp,
buffer_info->skb, next_desc);
- if (netif_msg_pktdata(adapter) && buffer_info->dma != 0)
+ if (netif_msg_pktdata(adapter) && buffer_info->skb)
print_hex_dump(KERN_INFO, "",
DUMP_PREFIX_ADDRESS,
- 16, 1, phys_to_virt(buffer_info->dma),
+ 16, 1, buffer_info->skb->data,
buffer_info->length, true);
}
}
@@ -547,18 +547,17 @@ rx_ring_summary:
(u64)buffer_info->dma,
buffer_info->skb, next_desc);
- if (netif_msg_pktdata(adapter)) {
+ if (netif_msg_pktdata(adapter) &&
+ buffer_info->dma && buffer_info->skb) {
print_hex_dump(KERN_INFO, "",
- DUMP_PREFIX_ADDRESS,
- 16, 1,
- phys_to_virt(buffer_info->dma),
- IGB_RX_HDR_LEN, true);
+ DUMP_PREFIX_ADDRESS,
+ 16, 1, buffer_info->skb->data,
+ IGB_RX_HDR_LEN, true);
print_hex_dump(KERN_INFO, "",
DUMP_PREFIX_ADDRESS,
16, 1,
- phys_to_virt(
- buffer_info->page_dma +
- buffer_info->page_offset),
+ page_address(buffer_info->page) +
+ buffer_info->page_offset,
PAGE_SIZE/2, true);
}
}
--
1.7.11.2
^ permalink raw reply related
* [net 3/3] igb: Fix register defines for all non-82575 hardware
From: Peter P Waskiewicz Jr @ 2012-08-09 9:39 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Peter P Waskiewicz Jr
In-Reply-To: <1344505152-28167-1-git-send-email-peter.p.waskiewicz.jr@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
It looks like the register defines for DCA were never updated after going from
82575 to 82576. This change addresses that by updating the defines.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
---
drivers/net/ethernet/intel/igb/e1000_regs.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/e1000_regs.h b/drivers/net/ethernet/intel/igb/e1000_regs.h
index 10efcd8..28394be 100644
--- a/drivers/net/ethernet/intel/igb/e1000_regs.h
+++ b/drivers/net/ethernet/intel/igb/e1000_regs.h
@@ -156,8 +156,12 @@
: (0x0E018 + ((_n) * 0x40)))
#define E1000_TXDCTL(_n) ((_n) < 4 ? (0x03828 + ((_n) * 0x100)) \
: (0x0E028 + ((_n) * 0x40)))
-#define E1000_DCA_TXCTRL(_n) (0x03814 + (_n << 8))
-#define E1000_DCA_RXCTRL(_n) (0x02814 + (_n << 8))
+#define E1000_RXCTL(_n) ((_n) < 4 ? (0x02814 + ((_n) * 0x100)) : \
+ (0x0C014 + ((_n) * 0x40)))
+#define E1000_DCA_RXCTRL(_n) E1000_RXCTL(_n)
+#define E1000_TXCTL(_n) ((_n) < 4 ? (0x03814 + ((_n) * 0x100)) : \
+ (0x0E014 + ((_n) * 0x40)))
+#define E1000_DCA_TXCTRL(_n) E1000_TXCTL(_n)
#define E1000_TDWBAL(_n) ((_n) < 4 ? (0x03838 + ((_n) * 0x100)) \
: (0x0E038 + ((_n) * 0x40)))
#define E1000_TDWBAH(_n) ((_n) < 4 ? (0x0383C + ((_n) * 0x100)) \
--
1.7.11.2
^ permalink raw reply related
* [net 2/3] e1000e: fix panic while dumping packets on Tx hang with IOMMU
From: Peter P Waskiewicz Jr @ 2012-08-09 9:39 UTC (permalink / raw)
To: davem; +Cc: Emil Tantilov, netdev, gospo, sassmann, Peter P Waskiewicz Jr
In-Reply-To: <1344505152-28167-1-git-send-email-peter.p.waskiewicz.jr@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
This patch resolves a "BUG: unable to handle kernel paging request at ..."
oops while dumping packet data. The issue occurs with IOMMU enabled due to
the address provided by phys_to_virt().
This patch avoids phys_to_virt() by using skb->data and the address of the
pages allocated for Rx.
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 36 +++++++++++++++++++++---------
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 95b2453..46c3b1f 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -178,6 +178,24 @@ static void e1000_regdump(struct e1000_hw *hw, struct e1000_reg_info *reginfo)
pr_info("%-15s %08x %08x\n", rname, regs[0], regs[1]);
}
+static void e1000e_dump_ps_pages(struct e1000_adapter *adapter,
+ struct e1000_buffer *bi)
+{
+ int i;
+ struct e1000_ps_page *ps_page;
+
+ for (i = 0; i < adapter->rx_ps_pages; i++) {
+ ps_page = &bi->ps_pages[i];
+
+ if (ps_page->page) {
+ pr_info("packet dump for ps_page %d:\n", i);
+ print_hex_dump(KERN_INFO, "", DUMP_PREFIX_ADDRESS,
+ 16, 1, page_address(ps_page->page),
+ PAGE_SIZE, true);
+ }
+ }
+}
+
/*
* e1000e_dump - Print registers, Tx-ring and Rx-ring
*/
@@ -299,10 +317,10 @@ static void e1000e_dump(struct e1000_adapter *adapter)
(unsigned long long)buffer_info->time_stamp,
buffer_info->skb, next_desc);
- if (netif_msg_pktdata(adapter) && buffer_info->dma != 0)
+ if (netif_msg_pktdata(adapter) && buffer_info->skb)
print_hex_dump(KERN_INFO, "", DUMP_PREFIX_ADDRESS,
- 16, 1, phys_to_virt(buffer_info->dma),
- buffer_info->length, true);
+ 16, 1, buffer_info->skb->data,
+ buffer_info->skb->len, true);
}
/* Print Rx Ring Summary */
@@ -381,10 +399,8 @@ rx_ring_summary:
buffer_info->skb, next_desc);
if (netif_msg_pktdata(adapter))
- print_hex_dump(KERN_INFO, "",
- DUMP_PREFIX_ADDRESS, 16, 1,
- phys_to_virt(buffer_info->dma),
- adapter->rx_ps_bsize0, true);
+ e1000e_dump_ps_pages(adapter,
+ buffer_info);
}
}
break;
@@ -444,12 +460,12 @@ rx_ring_summary:
(unsigned long long)buffer_info->dma,
buffer_info->skb, next_desc);
- if (netif_msg_pktdata(adapter))
+ if (netif_msg_pktdata(adapter) &&
+ buffer_info->skb)
print_hex_dump(KERN_INFO, "",
DUMP_PREFIX_ADDRESS, 16,
1,
- phys_to_virt
- (buffer_info->dma),
+ buffer_info->skb->data,
adapter->rx_buffer_len,
true);
}
--
1.7.11.2
^ permalink raw reply related
* Re: Bug with IPv6-UDP address binding
From: Jesper Dangaard Brouer @ 2012-08-09 9:40 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Thomas Graf
In-Reply-To: <1344459591.28967.271.camel@edumazet-glaptop>
On Wed, 2012-08-08 at 22:59 +0200, Eric Dumazet wrote:
> On Wed, 2012-08-08 at 22:37 +0200, Jesper Dangaard Brouer wrote:
> > Hi NetDev
> >
> > I think I have found a problem/bug with IPv6-UDP address binding.
> >
> > I found this problem while playing with IPVS and IPv6-UDP, but its also
> > present in more basic/normal situations.
> >
> > If you have two IPv6 addresses, within the same IPv6 subnet, then one
> > of the IPv6 addrs takes precedence over the other (for UDP only).
> >
> > Meaning that, if connecting to the "secondary" IPv6 via UDP, will
> > result in userspace see/bind the connection as being created to the
> > "primary" IP, even-though tcpdump shows that the IPv6-UDP packets are
> > dest the "secondary".
> >
> > The result is; that only the first IPv6-UDP packet is delivered to
> > userspace, and the next packets are denied by the kernel as the UDP
> > socket is "established" with the "primary" IPv6 addr.
> >
> > I would appreciate some hints to where in the IPv6 code I should look
> > for this bug. If any one else wants to fix it, I'm also fine with
> > that ;-)
> >
> >
> > Its quite easy to reproduce, using netcat (nc).
> >
> > Add two addresses to the "server" e.g.:
> > ip addr add fee0:cafe::102/64 dev eth0
> > ip addr add fee0:cafe::bad/64 dev eth0
> >
> > Run a netcat listener on "server":
> > nc -6 -u -l 2000
> > (Notice restart the listener between runs, due to limitation in nc)
> >
> > On the client add an IPv6 addr e.g.:
> > ip addr add fee0:cafe::101/64 dev eth0
> >
> > Run a netcat UDP-IPv6 producer on "client":
> > nc -6 -u fee0:cafe::bad 2000
> >
> > Notice that first packet, will get through, but second packets will
> > not (nc: Write error: Connection refused). Running a tcpdump shows
> > that the kernel is sending back ICMP6, destination unreachable,
> > unreachable port.
> >
> > Its also possible to see the problem, simply running "netstat -uan" on
> > "server", which will show that the "established" UDP connection, is
> > bound to the wrong "Local Address".
> >
> > (Tested on both latest net-next kernel at commit 79cda75a1, and also
> > on RHEL6 approx 2.6.32)
> >
>
> Hi Jesper
>
> Thats because the "nc -6 -u -l 2000" on server does :
>
> bind(3, {sa_family=AF_INET6, sin6_port=htons(2000), inet_pton(AF_INET6,
> "::", &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, 28) = 0
>
> recvfrom(3, "\n", 1024, MSG_PEEK, {sa_family=AF_INET6,
> sin6_port=htons(53696), inet_pton(AF_INET6, "fee0:cafe::101",
> &sin6_addr), sin6_flowinfo=0, sin6_scope_id=0}, [28]) = 1
>
> connect(3, {sa_family=AF_INET6, sin6_port=htons(53696),
> inet_pton(AF_INET6, "fee0:cafe::101", &sin6_addr), sin6_flowinfo=0,
> sin6_scope_id=0}, 28) = 0
>
> And the kernel automatically chooses a SOURCE address (fee0:cafe::102)
> that is not what you expected (fee0:cafe::bad)
Okay I see. And this is also the case for IPv4.
Guess I should have read Stephens[1] first, as this problem with
multihomed hosts is described (on page 219). He also states, that this
is a problem/feature related to Berkely-derived implementations. E.g.
Solaris handle this, the way I expected. That is, the source IP address
for the server's reply is the dest IP of the client's request.
> So its a bug in the application.
Yes, I guess its an application bug, because Berkely-derived
implementations don't handle multihomeing well for UDP.
Why are we keeping this, counter-intuitive behavior?
What about changing the implementation to act like Solaris, which IMHO
makes much more sense?
(BTW, iperf also have this "bug")
> UDP connect() is tricky : In this case, nc should learn on what IP
> address the client sent the frame. (using recvmsg() and appropriate
> ancillary message)
Reading through howto use recvmsg() and parsing of the ancillary
messages. See [1] "Advanced UDP sockets" page 531-538. Its quite an
extensive task to extract destination IP address. No wonder, netcat
missed this part.
> Then nc should bind a new socket on this address, then do the connect()
Yes, after the difficult extraction of the dest IP of the UDP packet.
Now I better understand, why the DNS server named/bind is so annoying,
that is requires a restart after adding IPs. I guess they didn't
implement this recvmsg(), and instead chooses to bind to all avail IPs
on init/start.
Hints for readers:
For IPv4 is easy to see which is the "secondary" IP via the command "ip
addr" (look for the word "secondary")
For IPv6 I cannot tell which one is the secondary/primary from the "ip
addr" output. But you can instead do a route lookup via the command
e.g: "ip route get fee0:cafe::102" and look for the "src" field.
[1] UNIX network programming Vol.1 (Networking APIs) by W. Richard
Stevens
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* RE: [PATCHv2 1/4] modem_shm: Add Modem Access Framework
From: Arun MURTHY @ 2012-08-09 9:53 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, Sjur BRENDELAND
In-Reply-To: <20120808133748.GA14459@kroah.com>
> On Wed, Aug 08, 2012 at 05:36:05AM +0200, Arun MURTHY wrote:
> > > On Tue, Aug 07, 2012 at 12:24:28PM +0530, Arun Murthy wrote:
> > > > Adds Modem Access Framework, which allows for registering platform
> > > specific
> > > > modem access mechanisms. The framework also exposes APIs for
> > > > client
> > > drivers
> > > > for getting and releasing access to modem, regardless of the
> > > underlying
> > > > platform specific access mechanism.
> > >
> > > The term "modems" here has a lot of legacy connotations. First of
> > > which is, userspace handles this today as tty devices, why aren't
> > > you doing the same here? Why does this have to be something
> > > "special"?
> > >
> >
> > The main focus over there the modem IPC.
>
> Over where?
>
> What "modem IPC"?
>
I mean the dir which is added in this patch, "modem_shm" is added to
have a separate list of such modem access drivers.
> You need to really explain what you are doing here, as I have no idea.
> And I have no idea why you still can't just use a tty device, why can't you?
>
Its not that tty can be used or cant be used, but just to separate out the
modem related stuff in a separate folder.
> > In doing so, there are some functionality like waking the modem, or
> > releasing the modem etc. These will be used by the modem IPC drivers
> > and also few others like sim driver and security drivers.
>
> What's a "sim driver", and what type of "security drivers" are you referring
> to?
SIM(Subscriber Identity Module) used in mobiles.
TEE(security) driver
>
> > Since this is a shared call and hence has to be synchronized. Hence so
> > a small framework like is being done to monitor the modem access
> > related only operations.
>
> Again, why can't the tty layer do this for you?
>
Maybe tty can do this, but want to have all modem related separately.
There are many such hardware and many drivers coming up in near future.
Thanks and Regards,
Arun R Murthy
-----------------
^ permalink raw reply
* RE: [PATCHv2 3/4] modem_shm: u8500-shm: U8500 Shared Memory Driver
From: Arun MURTHY @ 2012-08-09 10:11 UTC (permalink / raw)
To: Alan Cox
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, gregkh@linuxfoundation.org,
Sjur BRENDELAND
In-Reply-To: <20120808114606.2a1a3bd3@pyramind.ukuu.org.uk>
> > Basically it doesn't suit our protocol of having base addr, read/write
> > pointer, locking etc as the same set of structures and protocol will
> > be used on the modem side implementation.
>
> Ok. What happens about endianness or is the modem always the same
> endianness as the host ?
Yes its always the same endianess, because the same phonet protocol is
used on APE and modem side.
The flow on APE is like user space->kernel network->phonet->shrm
And on modem side its shrm->phonet->modem stack
>
>
> > > > + if (len <= 0)
> > > > + return -EFAULT;
> > >
> > > How can this occur ?
> >
> > Check for error condition
>
> So how can it occur ? The kernel char layer will never pass a negative length
> to a driver ?
>
Ok, will remove this.
> > > What happens with two parallel reads - I don't see what prevents
> > > corruption if that occurs or one racing read freeing the message
> > > before another has finished processing it.
> >
> > Two parallel reads for different L2 headers can happen, but within the
> > same L2 header is out of the scope. Since the client using this in
> > user space will not know about the message. i.e which msg is for which
> > client. Hence so that scenario is not considered.
>
> What stops a hostile application (or programmer error) from doing so
> deliberately ?
>
Will add checks over here so as to not to do so.
> > > > + if (len <= 0 || buf == NULL)
> > > > + return -EFAULT;
> > >
> > > len < 0 cannot occur, buf == NULL is not an error
> >
> > Error handling is for what which is not expected.
>
> Well buf = NULL is not an error (its weird but its not an error)
>
> Also length < 0 is never passed from the char layer to a driver.
>
Ok will remove.
> > > > + dev_err(shrm->dev, "Device not opened yet\n");
> > > > + mutex_unlock(&isa_lock);
> > > > + return -ENODEV;
> > > > + }
> > > > + atomic_set(&isa_context->is_open[idx], 1);
> > >
> > > How do you know it will always be one. Also given it's within the
> > > mutex in all uses I can see why is it an atomic ?
> > >
> >
> > As per our assumptions/protocol only one client per L2 header.
>
> So why use atomic. Also you can't make that assumption. If you need your
> device to have one user per channel and one write call at a time you must
> enforce it. There is nothing wrong with enforcing it but it needs to be done.
>
> That means your open path probably wants to do something (locked) like
>
> if (foo->users)
> return -EBUSY;
>
Done, will add check to restrict open for only one process at a time.
> you still then need to use a mutex or similar in read and write because a
> single open can pass to multiple processes (or multiple writes/reads occur at
> once in a multi-threaded app).
>
> User/Kernel is the security boundary so the kernel code must be robust
> against a hostile user rather than assuming a correctly functioning library.
>
> I suspect you simply need to wrap the read/write logic (except for a wait for
> new message) with a mutex and all will be well
>
That should be fine as far as the process is able to differentiate the messages.
> > > > + if (get_boot_state() != BOOT_DONE) {
> > > > + dev_err(shrm->dev, "Boot is not done\n");
> > > > + return -EBUSY;
> > > > + }
> > >
> > > Is it guaranteed that this is a one way path - ie a device never
> > > goes back into BOOT state ?
> >
> > No, on modem reset, everything happens from first.
>
> So what occurs if this modem reset happens between that test and the next
> line. You have no locking on it so you've got no guarantee that it won't reset
> during the test. So it covers the initial set up case but not a reset.
>
> It may not matter providing a reset wakes up things and it is handled later. It
> just looks suspicious.
>
That always remain open, there is no end for that suspection. What we need to
is to secure is the access to registers. Before access of any register we need to
make sure that modem is awake and not reset.
> > > > + isadev = &isa_context->isadev[idx];
> > > > + if (filp != NULL)
> > > > + filp->private_data = isadev;
> > >
> > > How can filp be NULL ?
> >
> > :-) just a error condition check
>
> These tests are not useful, if anything they hide bugs. If you have a real
> reason to check (eg its a complicated internal path) then use
>
> WARN_ON(condition)
>
> or
>
> BUG_ON(condition)
>
> so it gets noticed. For core kernel things however there is no point checking.
> If the kernel ever passes you null as a file pointer the game is already over.
>
Done.
> > > > + for (no_dev = 0; no_dev < ISA_DEVICES; no_dev++) {
> > > > + atomic_set(&isa_context->is_open[no_dev], 1);
> > > > + device_create(isa_context->shm_class, NULL,
> > > > + MKDEV(MAJOR(dev_id),
> > > > + map_dev[no_dev].l2_header), NULL,
> > > > + map_dev[no_dev].name);
> > > > + }
> > >
> > > What happens if I open the device right here... ?
> >
> > It can be opened, but nothing thereafter, since modem is not booted.
>
> You've not yet set up the isa_context but yes.. looks like it is covered by the
> boot check.
>
> (A good rule of thumb is btw to initialise everything, then register
> stuff)
>
Sure will redo this.
Thanks and Regards,
Arun R Murthy
------------------
^ permalink raw reply
* RE: [PATCHv2 1/4] modem_shm: Add Modem Access Framework
From: Arun MURTHY @ 2012-08-09 10:14 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, Sjur BRENDELAND
In-Reply-To: <20120808133748.GA14459@kroah.com>
> > On Wed, Aug 08, 2012 at 05:36:05AM +0200, Arun MURTHY wrote:
> > > > On Tue, Aug 07, 2012 at 12:24:28PM +0530, Arun Murthy wrote:
> > > > > Adds Modem Access Framework, which allows for registering
> > > > > platform
> > > > specific
> > > > > modem access mechanisms. The framework also exposes APIs for
> > > > > client
> > > > drivers
> > > > > for getting and releasing access to modem, regardless of the
> > > > underlying
> > > > > platform specific access mechanism.
> > > >
> > > > The term "modems" here has a lot of legacy connotations. First of
> > > > which is, userspace handles this today as tty devices, why aren't
> > > > you doing the same here? Why does this have to be something
> > > > "special"?
> > > >
> > >
> > > The main focus over there the modem IPC.
> >
> > Over where?
> >
> > What "modem IPC"?
> >
>
> I mean the dir which is added in this patch, "modem_shm" is added to have a
> separate list of such modem access drivers.
The modem that I am referring to is an on chip modem in embedded platforms.
Applications of these being mobile, tablets etc.
So the APE and modem resides on a single chip.
Thanks and Regards,
Arun R Murthy
------------------
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox