* Re: [PATCH net] net: Remove unused variables in rt_cache_stat
From: David Miller @ 2012-07-30 21:52 UTC (permalink / raw)
To: subramanian.vijay; +Cc: netdev
In-Reply-To: <1343684750-2987-1-git-send-email-subramanian.vijay@gmail.com>
From: Vijay Subramanian <subramanian.vijay@gmail.com>
Date: Mon, 30 Jul 2012 14:45:50 -0700
> With the removal of the routing cache, some variables in rt_cache_stat are no
> longer used. Remove them from rt_cache_stat and do not print them out in
> /proc/net/stat/rt_cache.
>
> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
> ---
> checkpatch complains that the seq_printf line is over 80 chars which was already
> the case. I left it as is to aid in grepping the sources.
You cannot make this change, these fields are exported via procfs and
therefore you will break any application that is parsing the existing
layout.
^ permalink raw reply
* Re: [PATCH] codel: refine one condition to avoid a nul rec_inv_sqrt
From: David Miller @ 2012-07-30 21:54 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, lp2s1h
In-Reply-To: <1343631141.2626.13293.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 30 Jul 2012 08:52:21 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> One condition before codel_Newton_step() was not good if
> we never left the dropping state for a flow. As a result
> rec_inv_sqrt was 0, instead of the ~0 initial value.
>
> codel control law was then set to a very aggressive mode, dropping
> many packets before reaching 'target' and recovering from this problem.
>
> To keep codel_vars_init() as efficient as possible, refine
> the condition to make sure rec_inv_sqrt initial value is correct
>
> Many thanks to Anton Mich for discovering the issue and suggesting
> a fix.
>
> Reported-by: Anton Mich <lp2s1h@gmail.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH] net: TCP early demux cleanup
From: David Miller @ 2012-07-30 21:54 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1343631973.2626.13317.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 30 Jul 2012 09:06:13 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> early_demux() handlers should be called in RCU context, and as we
> use skb_dst_set_noref(skb, dst), caller must not exit from RCU context
> before dst use (skb_dst(skb)) or release (skb_drop(dst))
>
> Therefore, rcu_read_lock()/rcu_read_unlock() pairs around
> ->early_demux() are confusing and not needed :
>
> Protocol handlers are already in an RCU read lock section.
> (__netif_receive_skb() does the rcu_read_lock() )
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
I wondered about this very issue when I wrote this code, thanks
for clearing things up.
Applied.
^ permalink raw reply
* Re: [PATCH] net: ipv4: fix RCU races on dst refcounts
From: David Miller @ 2012-07-30 21:56 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1343640037.21269.15.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 30 Jul 2012 11:20:37 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> commit c6cffba4ffa2 (ipv4: Fix input route performance regression.)
> added various fatal races with dst refcounts.
>
> crashes happen on tcp workloads if routes are added/deleted at the same
> time.
>
> The dst_free() calls from free_fib_info_rcu() are clearly racy.
>
> We need instead regular dst refcounting (dst_release()) and make
> sure dst_release() is aware of RCU grace periods :
>
> Add DST_RCU_FREE flag so that dst_release() respects an RCU grace period
> before dst destruction for cached dst
>
> Introduce a new inet_sk_rx_dst_set() helper, using atomic_inc_not_zero()
> to make sure we dont increase a zero refcount (On a dst currently
> waiting an rcu grace period before destruction)
>
> rt_cache_route() must take a reference on the new cached route, and
> release it if was not able to install it.
>
> With this patch, my machines survive various benchmarks.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
I'm applying this patch, however:
> +static inline void inet_sk_rx_dst_set(struct sock *sk, const struct sk_buff *skb)
> +{
> + struct dst_entry *dst = skb_dst(skb);
> +
> + if (atomic_inc_not_zero(&dst->__refcnt)) {
> + if (!(dst->flags & DST_RCU_FREE))
> + dst->flags |= DST_RCU_FREE;
> +
> + sk->sk_rx_dst = dst;
> + inet_sk(sk)->rx_dst_ifindex = skb->skb_iif;
> + }
> +}
This is not safe.
We cannot allow clients outside of the DST providers make non-atomic
changes to the dst attributes, as you are here with this dst->flags
modification.
Make this "needs RCU liberation" indication at the spot where we get
rid of sk->sk_rx_dst, make a dst_release_rcu() or somthing like that.
^ permalink raw reply
* Re: [PATCH] ipv4: remove rt_cache_rebuild_count
From: David Miller @ 2012-07-30 21:56 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1343668469.21269.26.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 30 Jul 2012 19:14:29 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> After IP route cache removal, rt_cache_rebuild_count is no longer
> used.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied.
^ permalink raw reply
* Re: [PATCH] bridge: make port attributes const
From: David Miller @ 2012-07-30 21:56 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20120730115549.0d53d8cb@nehalam.linuxnetplumber.net>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Mon, 30 Jul 2012 11:55:49 -0700
> Simple table that can be marked const.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Applied.
^ permalink raw reply
* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Hutchings @ 2012-07-30 22:20 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev, linux-net-drivers
In-Reply-To: <20120730.144632.478408817308488569.davem@davemloft.net>
On Mon, 2012-07-30 at 14:46 -0700, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Mon, 30 Jul 2012 20:35:52 +0100
>
> > On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote:
> >> Or you could introduce a new wk->sk_gso_max_segments, that your sfc
> >> driver sets to whatever limit ?
> >
> > Yes, that's another option.
>
> This is how I want this handled.
How should that be applied in the GRO-forwarding case?
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Stephen Hemminger @ 2012-07-30 22:50 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Miller, eric.dumazet, netdev, linux-net-drivers
In-Reply-To: <1343686857.2667.60.camel@bwh-desktop.uk.solarflarecom.com>
On Mon, 30 Jul 2012 23:20:57 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Mon, 2012-07-30 at 14:46 -0700, David Miller wrote:
> > From: Ben Hutchings <bhutchings@solarflare.com>
> > Date: Mon, 30 Jul 2012 20:35:52 +0100
> >
> > > On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote:
> > >> Or you could introduce a new wk->sk_gso_max_segments, that your sfc
> > >> driver sets to whatever limit ?
> > >
> > > Yes, that's another option.
> >
> > This is how I want this handled.
>
> How should that be applied in the GRO-forwarding case?
>
> Ben.
>
Why not make max_frags a property of the device?
Something like the following untested idea:
diff --git a/net/core/dev.c b/net/core/dev.c
index 0ebaea1..bfb005b 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2159,14 +2159,16 @@ EXPORT_SYMBOL(netif_skb_features);
* at least one of fragments is in highmem and device does not
* support DMA from it.
*/
-static inline int skb_needs_linearize(struct sk_buff *skb,
- int features)
+static inline bool skb_needs_linearize(struct sk_buff *skb,
+ int features, unsigned int maxfrags)
{
- return skb_is_nonlinear(skb) &&
- ((skb_has_frag_list(skb) &&
- !(features & NETIF_F_FRAGLIST)) ||
- (skb_shinfo(skb)->nr_frags &&
- !(features & NETIF_F_SG)));
+ if (!skb_is_nonlinear(skb))
+ return false;
+
+ if (skb_has_frag_list(skb))
+ return !(features & NETIF_F_FRAGLIST);
+ else
+ return skb_shinfo(skb)->nr_frags > maxfrags;
}
int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
@@ -2206,7 +2208,7 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
if (skb->next)
goto gso;
} else {
- if (skb_needs_linearize(skb, features) &&
+ if (skb_needs_linearize(skb, features, dev->max_frags) &&
__skb_linearize(skb))
goto out_kfree_skb;
@@ -5544,6 +5546,20 @@ int register_netdevice(struct net_device *dev)
dev->features |= NETIF_F_SOFT_FEATURES;
dev->wanted_features = dev->features & dev->hw_features;
+ if (dev->max_frags > 0) {
+ if (!(features & NETIF_F_SG)) {
+ netdev_dbg(dev,
+ "Resetting max fragments since no NETIF_F_SG\n");
+ dev->max_frags = 0;
+ }
+ } else {
+ /* If device has not set maximum number of fragments
+ * then assume it can take any number of them
+ */
+ if (features & NETIF_F_SG)
+ dev->max_frags = MAX_SKB_FRAGS;
+ }
+
/* Turn on no cache copy if HW is doing checksum */
if (!(dev->flags & IFF_LOOPBACK)) {
dev->hw_features |= NETIF_F_NOCACHE_COPY;
^ permalink raw reply related
* Re: [PATCH net 1/2] tcp: Limit number of segments generated by GSO per skb
From: Ben Hutchings @ 2012-07-30 23:07 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, eric.dumazet, netdev, linux-net-drivers
In-Reply-To: <20120730155026.7460a9a6@nehalam.linuxnetplumber.net>
On Mon, 2012-07-30 at 15:50 -0700, Stephen Hemminger wrote:
> On Mon, 30 Jul 2012 23:20:57 +0100
> Ben Hutchings <bhutchings@solarflare.com> wrote:
>
> > On Mon, 2012-07-30 at 14:46 -0700, David Miller wrote:
> > > From: Ben Hutchings <bhutchings@solarflare.com>
> > > Date: Mon, 30 Jul 2012 20:35:52 +0100
> > >
> > > > On Mon, 2012-07-30 at 19:31 +0200, Eric Dumazet wrote:
> > > >> Or you could introduce a new wk->sk_gso_max_segments, that your sfc
> > > >> driver sets to whatever limit ?
> > > >
> > > > Yes, that's another option.
> > >
> > > This is how I want this handled.
> >
> > How should that be applied in the GRO-forwarding case?
> >
> > Ben.
> >
> Why not make max_frags a property of the device?
[...]
This has nothing to do with the number of input fragments. But I think
you're on the right track - this can be checked in netif_skb_features()
or something like that.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] net: ipv4: fix RCU races on dst refcounts
From: David Miller @ 2012-07-31 0:40 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <20120730.145637.906924670032055493.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Mon, 30 Jul 2012 14:56:37 -0700 (PDT)
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 30 Jul 2012 11:20:37 +0200
>
>> From: Eric Dumazet <edumazet@google.com>
>>
>> We need instead regular dst refcounting (dst_release()) and make
>> sure dst_release() is aware of RCU grace periods :
Eric, we really cannot do this.
We absolutely must call dst_free() directly when cached entries
are flushed.
Your delayed scheme using dst_release() doesn't work, it exposes us to
the "stale netdevice references" issue even for cached entries. This
exact issue is why I moved away from a dst_release() based scheme to
a !DST_NOCACHE + dst_free() one.
dst_free() is special. It is special in that if there are existing
references, it adds the dst onto the generic GC list of busy dsts in
net/core/dst.c
This is important, because it means that even if references are still
held on the dst, we will still be able to purge spurious netdevice
references when those netdevices try to go down or unregister
themselves.
You can't defer the dst_free() to the final refcount drop like your
code does now. A socket, or other dst caching entity, can hold onto
the dst forever, do no socket operations at all, and therefore hold
onto a netdevice for an infinite amount of time.
Look at how net/core/dst.c:dst_dev_event() walks dst_busy_list.
That's why we must propagate cached dsts into dst_busy_list no later
than when we trim them from ->nh_rth_{input,output}
I noticed this because I'm trying to work on a fix the lingering dst
netdevice reference problem for non-cached dsts.
^ permalink raw reply
* [PATCH 0/2] ipv4: Fix dangling netdev refs
From: David Miller @ 2012-07-31 1:23 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
Eric, this should give you an idea of what I was working on.
It really assumes the universe in place before your change last
night. It is very likely we'll have to revert it and look for
another solution to the problem you were trying to solve. And
actually I don't understand the actual bug very well.
I can only assume that the core issue was that, unlike back when
we had the routing cache, the fib_info nexthops are not persistent
memory like the routing cache hash table was?
Otherwise I can see absolutely no change in reference counting and
dst destruction logic between the routing cache, and how I modified
fib_info nexthop cached routes to behave.
Anyways, the first patch caches routes in the nexthop exception
entries.
And the second patch has a global list for uncached routes so we
can purge netdevice references properly when such devices are
unregistered.
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* [PATCH 1/2] ipv4: Cache routes in nexthop exception entries.
From: David Miller @ 2012-07-31 1:23 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/net/ip_fib.h | 1 +
net/ipv4/fib_semantics.c | 4 +++
net/ipv4/route.c | 82 ++++++++++++++++++++++++++----------------------
3 files changed, 49 insertions(+), 38 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index e69c3a4..c4770fc 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -54,6 +54,7 @@ struct fib_nh_exception {
u32 fnhe_pmtu;
__be32 fnhe_gw;
unsigned long fnhe_expires;
+ struct rtable *fnhe_rth;
unsigned long fnhe_stamp;
};
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index e55171f..eaccdb5 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -153,6 +153,10 @@ static void free_nh_exceptions(struct fib_nh *nh)
struct fib_nh_exception *next;
next = rcu_dereference_protected(fnhe->fnhe_next, 1);
+
+ if (fnhe->fnhe_rth)
+ dst_release(&fnhe->fnhe_rth->dst);
+
kfree(fnhe);
fnhe = next;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index d6eabcf..e2abb0d 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -587,7 +587,7 @@ static void ip_rt_build_flow_key(struct flowi4 *fl4, const struct sock *sk,
build_sk_flow_key(fl4, sk);
}
-static DEFINE_SEQLOCK(fnhe_seqlock);
+static DEFINE_SPINLOCK(fnhe_lock);
static struct fib_nh_exception *fnhe_oldest(struct fnhe_hash_bucket *hash)
{
@@ -599,6 +599,10 @@ static struct fib_nh_exception *fnhe_oldest(struct fnhe_hash_bucket *hash)
if (time_before(fnhe->fnhe_stamp, oldest->fnhe_stamp))
oldest = fnhe;
}
+ if (oldest->fnhe_rth) {
+ dst_release(&oldest->fnhe_rth->dst);
+ oldest->fnhe_rth = NULL;
+ }
return oldest;
}
@@ -620,7 +624,7 @@ static void update_or_create_fnhe(struct fib_nh *nh, __be32 daddr, __be32 gw,
int depth;
u32 hval = fnhe_hashfun(daddr);
- write_seqlock_bh(&fnhe_seqlock);
+ spin_lock_bh(&fnhe_lock);
hash = nh->nh_exceptions;
if (!hash) {
@@ -667,7 +671,7 @@ static void update_or_create_fnhe(struct fib_nh *nh, __be32 daddr, __be32 gw,
fnhe->fnhe_stamp = jiffies;
out_unlock:
- write_sequnlock_bh(&fnhe_seqlock);
+ spin_unlock_bh(&fnhe_lock);
return;
}
@@ -1167,36 +1171,37 @@ static struct fib_nh_exception *find_exception(struct fib_nh *nh, __be32 daddr)
static void rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
__be32 daddr)
{
- __be32 fnhe_daddr, gw;
- unsigned long expires;
- unsigned int seq;
- u32 pmtu;
-
-restart:
- seq = read_seqbegin(&fnhe_seqlock);
- fnhe_daddr = fnhe->fnhe_daddr;
- gw = fnhe->fnhe_gw;
- pmtu = fnhe->fnhe_pmtu;
- expires = fnhe->fnhe_expires;
- if (read_seqretry(&fnhe_seqlock, seq))
- goto restart;
-
- if (daddr != fnhe_daddr)
- return;
+ spin_lock_bh(&fnhe_lock);
+
+ if (daddr == fnhe->fnhe_daddr) {
+ struct rtable *orig;
- if (pmtu) {
- unsigned long diff = expires - jiffies;
+ if (fnhe->fnhe_pmtu) {
+ unsigned long expires = fnhe->fnhe_expires;
+ unsigned long diff = expires - jiffies;
- if (time_before(jiffies, expires)) {
- rt->rt_pmtu = pmtu;
- dst_set_expires(&rt->dst, diff);
+ if (time_before(jiffies, expires)) {
+ rt->rt_pmtu = fnhe->fnhe_pmtu;
+ dst_set_expires(&rt->dst, diff);
+ }
}
+ if (fnhe->fnhe_gw) {
+ rt->rt_flags |= RTCF_REDIRECTED;
+ rt->rt_gateway = fnhe->fnhe_gw;
+ }
+
+ orig = fnhe->fnhe_rth;
+ if (orig)
+ dst_release(&orig->dst);
+
+ rt->dst.flags |= DST_RCU_FREE;
+ dst_hold(&rt->dst);
+ fnhe->fnhe_rth = rt;
+
+ fnhe->fnhe_stamp = jiffies;
}
- if (gw) {
- rt->rt_flags |= RTCF_REDIRECTED;
- rt->rt_gateway = gw;
- }
- fnhe->fnhe_stamp = jiffies;
+
+ spin_unlock_bh(&fnhe_lock);
}
static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
@@ -1236,13 +1241,13 @@ static void rt_set_nexthop(struct rtable *rt, __be32 daddr,
if (nh->nh_gw && nh->nh_scope == RT_SCOPE_LINK)
rt->rt_gateway = nh->nh_gw;
- if (unlikely(fnhe))
- rt_bind_exception(rt, fnhe, daddr);
dst_init_metrics(&rt->dst, fi->fib_metrics, true);
#ifdef CONFIG_IP_ROUTE_CLASSID
rt->dst.tclassid = nh->nh_tclassid;
#endif
- if (!(rt->dst.flags & DST_NOCACHE))
+ if (unlikely(fnhe))
+ rt_bind_exception(rt, fnhe, daddr);
+ else if (!(rt->dst.flags & DST_NOCACHE))
rt_cache_route(nh, rt);
}
@@ -1741,18 +1746,19 @@ static struct rtable *__mkroute_output(const struct fib_result *res,
fnhe = NULL;
if (fi) {
fnhe = find_exception(&FIB_RES_NH(*res), fl4->daddr);
- if (!fnhe) {
+ if (fnhe)
+ rth = fnhe->fnhe_rth;
+ else
rth = FIB_RES_NH(*res).nh_rth_output;
- if (rt_cache_valid(rth)) {
- dst_hold(&rth->dst);
- return rth;
- }
+ if (rt_cache_valid(rth)) {
+ dst_hold(&rth->dst);
+ return rth;
}
}
rth = rt_dst_alloc(dev_out,
IN_DEV_CONF_GET(in_dev, NOPOLICY),
IN_DEV_CONF_GET(in_dev, NOXFRM),
- fi && !fnhe);
+ fi);
if (!rth)
return ERR_PTR(-ENOBUFS);
--
1.7.11.2
^ permalink raw reply related
* [PATCH 2/2] ipv4: Properly purge netdev references on uncached routes.
From: David Miller @ 2012-07-31 1:23 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
When a device is unregistered, we have to purge all of the
references to it that may exist in the entire system.
If a route is uncached, we currently have no way of accomplishing
this.
So create a global list that is scanned when a network device goes
down. This mirrors the logic in net/core/dst.c's dst_ifdown().
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/net/route.h | 3 +++
net/ipv4/fib_frontend.c | 1 +
net/ipv4/route.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++---
net/ipv4/xfrm4_policy.c | 1 +
4 files changed, 69 insertions(+), 4 deletions(-)
diff --git a/include/net/route.h b/include/net/route.h
index 8c52bc6..776a27f 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -57,6 +57,8 @@ struct rtable {
/* Miscellaneous cached information */
u32 rt_pmtu;
+
+ struct list_head rt_uncached;
};
static inline bool rt_is_input_route(const struct rtable *rt)
@@ -107,6 +109,7 @@ extern struct ip_rt_acct __percpu *ip_rt_acct;
struct in_device;
extern int ip_rt_init(void);
extern void rt_cache_flush(struct net *net, int how);
+extern void rt_flush_dev(struct net_device *dev);
extern struct rtable *__ip_route_output_key(struct net *, struct flowi4 *flp);
extern struct rtable *ip_route_output_flow(struct net *, struct flowi4 *flp,
struct sock *sk);
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 8732cc7..c43ae3f 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -1046,6 +1046,7 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo
if (event == NETDEV_UNREGISTER) {
fib_disable_ip(dev, 2, -1);
+ rt_flush_dev(dev);
return NOTIFY_DONE;
}
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index e2abb0d..7111fce 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -147,6 +147,7 @@ static void ip_rt_update_pmtu(struct dst_entry *dst, struct sock *sk,
struct sk_buff *skb, u32 mtu);
static void ip_do_redirect(struct dst_entry *dst, struct sock *sk,
struct sk_buff *skb);
+static void ipv4_dst_destroy(struct dst_entry *dst);
static void ipv4_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
int how)
@@ -170,6 +171,7 @@ static struct dst_ops ipv4_dst_ops = {
.default_advmss = ipv4_default_advmss,
.mtu = ipv4_mtu,
.cow_metrics = ipv4_cow_metrics,
+ .destroy = ipv4_dst_destroy,
.ifdown = ipv4_dst_ifdown,
.negative_advice = ipv4_negative_advice,
.link_failure = ipv4_link_failure,
@@ -1168,9 +1170,11 @@ static struct fib_nh_exception *find_exception(struct fib_nh *nh, __be32 daddr)
return NULL;
}
-static void rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
+static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
__be32 daddr)
{
+ bool ret = false;
+
spin_lock_bh(&fnhe_lock);
if (daddr == fnhe->fnhe_daddr) {
@@ -1199,14 +1203,18 @@ static void rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
fnhe->fnhe_rth = rt;
fnhe->fnhe_stamp = jiffies;
+ ret = true;
}
spin_unlock_bh(&fnhe_lock);
+
+ return ret;
}
-static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
+static bool rt_cache_route(struct fib_nh *nh, struct rtable *rt)
{
struct rtable *orig, *prev, **p = &nh->nh_rth_output;
+ bool ret = true;
if (rt_is_input_route(rt))
p = &nh->nh_rth_input;
@@ -1221,6 +1229,48 @@ static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
dst_release(&orig->dst);
} else {
dst_release(&rt->dst);
+ ret = false;
+ }
+
+ return ret;
+}
+
+static DEFINE_SPINLOCK(rt_uncached_lock);
+static LIST_HEAD(rt_uncached_list);
+
+static void rt_add_uncached_list(struct rtable *rt)
+{
+ spin_lock_bh(&rt_uncached_lock);
+ list_add_tail(&rt->rt_uncached, &rt_uncached_list);
+ spin_unlock_bh(&rt_uncached_lock);
+}
+
+static void ipv4_dst_destroy(struct dst_entry *dst)
+{
+ struct rtable *rt = (struct rtable *) dst;
+
+ if (dst->flags & DST_NOCACHE) {
+ spin_lock_bh(&rt_uncached_lock);
+ list_del(&rt->rt_uncached);
+ spin_unlock_bh(&rt_uncached_lock);
+ }
+}
+
+void rt_flush_dev(struct net_device *dev)
+{
+ if (!list_empty(&rt_uncached_list)) {
+ struct net *net = dev_net(dev);
+ struct rtable *rt;
+
+ spin_lock_bh(&rt_uncached_lock);
+ list_for_each_entry(rt, &rt_uncached_list, rt_uncached) {
+ if (rt->dst.dev != dev)
+ continue;
+ rt->dst.dev = net->loopback_dev;
+ dev_hold(rt->dst.dev);
+ dev_put(dev);
+ }
+ spin_unlock_bh(&rt_uncached_lock);
}
}
@@ -1236,6 +1286,8 @@ static void rt_set_nexthop(struct rtable *rt, __be32 daddr,
struct fib_nh_exception *fnhe,
struct fib_info *fi, u16 type, u32 itag)
{
+ bool cached = false;
+
if (fi) {
struct fib_nh *nh = &FIB_RES_NH(*res);
@@ -1246,10 +1298,12 @@ static void rt_set_nexthop(struct rtable *rt, __be32 daddr,
rt->dst.tclassid = nh->nh_tclassid;
#endif
if (unlikely(fnhe))
- rt_bind_exception(rt, fnhe, daddr);
+ cached = rt_bind_exception(rt, fnhe, daddr);
else if (!(rt->dst.flags & DST_NOCACHE))
- rt_cache_route(nh, rt);
+ cached = rt_cache_route(nh, rt);
}
+ if (unlikely(!cached))
+ rt_add_uncached_list(rt);
#ifdef CONFIG_IP_ROUTE_CLASSID
#ifdef CONFIG_IP_MULTIPLE_TABLES
@@ -1316,6 +1370,7 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
rth->rt_iif = 0;
rth->rt_pmtu = 0;
rth->rt_gateway = 0;
+ INIT_LIST_HEAD(&rth->rt_uncached);
if (our) {
rth->dst.input= ip_local_deliver;
rth->rt_flags |= RTCF_LOCAL;
@@ -1441,6 +1496,7 @@ static int __mkroute_input(struct sk_buff *skb,
rth->rt_iif = 0;
rth->rt_pmtu = 0;
rth->rt_gateway = 0;
+ INIT_LIST_HEAD(&rth->rt_uncached);
rth->dst.input = ip_forward;
rth->dst.output = ip_output;
@@ -1607,6 +1663,7 @@ local_input:
rth->rt_iif = 0;
rth->rt_pmtu = 0;
rth->rt_gateway = 0;
+ INIT_LIST_HEAD(&rth->rt_uncached);
if (res.type == RTN_UNREACHABLE) {
rth->dst.input= ip_error;
rth->dst.error= -err;
@@ -1771,6 +1828,7 @@ static struct rtable *__mkroute_output(const struct fib_result *res,
rth->rt_iif = orig_oif ? : 0;
rth->rt_pmtu = 0;
rth->rt_gateway = 0;
+ INIT_LIST_HEAD(&rth->rt_uncached);
RT_CACHE_STAT_INC(out_slow_tot);
@@ -2050,6 +2108,8 @@ struct dst_entry *ipv4_blackhole_route(struct net *net, struct dst_entry *dst_or
rt->rt_type = ort->rt_type;
rt->rt_gateway = ort->rt_gateway;
+ INIT_LIST_HEAD(&rt->rt_uncached);
+
dst_free(new);
}
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index c628184..681ea2f 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -92,6 +92,7 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
xdst->u.rt.rt_type = rt->rt_type;
xdst->u.rt.rt_gateway = rt->rt_gateway;
xdst->u.rt.rt_pmtu = rt->rt_pmtu;
+ INIT_LIST_HEAD(&xdst->u.rt.rt_uncached);
return 0;
}
--
1.7.11.2
^ permalink raw reply related
* [PATCHv2 net 0/3] Prevent extreme TSO parameters from stalling TX queues
From: Ben Hutchings @ 2012-07-31 1:51 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-net-drivers, Ben Greear, Eric Dumazet,
Stephen Hemminger
The following changes fix a potential DoS by peers or local users on
network interfaces using the sfc driver (and possibly others) with TSO
enabled (as it is by default).
Please apply patches 1 and 2 to the net tree and your stable update
queue. I'm not sure whether patch 3 is really important.
Ben.
Ben Hutchings (3):
net: Allow driver to limit number of GSO segments per skb
sfc: Fix maximum number of TSO segments and minimum TX queue size
tcp: Apply device TSO segment limit earlier
drivers/net/ethernet/sfc/efx.c | 6 ++++++
drivers/net/ethernet/sfc/efx.h | 14 ++++++++++----
drivers/net/ethernet/sfc/ethtool.c | 16 +++++++++++-----
drivers/net/ethernet/sfc/tx.c | 19 +++++++++++++++++++
include/linux/netdevice.h | 2 ++
include/net/sock.h | 2 ++
net/core/dev.c | 4 ++++
net/core/sock.c | 1 +
net/ipv4/tcp.c | 4 +++-
net/ipv4/tcp_cong.c | 3 ++-
net/ipv4/tcp_output.c | 21 ++++++++++++---------
11 files changed, 72 insertions(+), 20 deletions(-)
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCHv2 net 1/3] net: Allow driver to limit number of GSO segments per skb
From: Ben Hutchings @ 2012-07-31 1:57 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-net-drivers, Ben Greear, Eric Dumazet,
Stephen Hemminger
In-Reply-To: <1343699476.2667.69.camel@bwh-desktop.uk.solarflarecom.com>
A peer (or local user) may cause TCP to use a nominal MSS of as little
as 88 (actual MSS of 76 with timestamps). Given that we have a
sufficiently prodigious local sender and the peer ACKs quickly enough,
it is nevertheless possible to grow the window for such a connection
to the point that we will try to send just under 64K at once. This
results in a single skb that expands to 861 segments.
In some drivers with TSO support, such an skb will require hundreds of
DMA descriptors; a substantial fraction of a TX ring or even more than
a full ring. The TX queue selected for the skb may stall and trigger
the TX watchdog repeatedly (since the problem skb will be retried
after the TX reset). This particularly affects sfc, for which the
issue is designated as CVE-2012-3412.
Therefore:
1. Add the field net_device::gso_max_segs holding the device-specific
limit.
2. In netif_skb_features(), if the number of segments is too high then
mask out GSO features to force fall back to software GSO.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
include/linux/netdevice.h | 2 ++
net/core/dev.c | 4 ++++
2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index eb06e58..a9db4f3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1300,6 +1300,8 @@ struct net_device {
/* for setting kernel sock attribute on TCP connection setup */
#define GSO_MAX_SIZE 65536
unsigned int gso_max_size;
+#define GSO_MAX_SEGS 65535
+ u16 gso_max_segs;
#ifdef CONFIG_DCB
/* Data Center Bridging netlink ops */
diff --git a/net/core/dev.c b/net/core/dev.c
index 0ebaea1..4020646 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2133,6 +2133,9 @@ netdev_features_t netif_skb_features(struct sk_buff *skb)
__be16 protocol = skb->protocol;
netdev_features_t features = skb->dev->features;
+ if (skb_shinfo(skb)->gso_segs > skb->dev->gso_max_segs)
+ features &= ~NETIF_F_GSO_MASK;
+
if (protocol == htons(ETH_P_8021Q)) {
struct vlan_ethhdr *veh = (struct vlan_ethhdr *)skb->data;
protocol = veh->h_vlan_encapsulated_proto;
@@ -5942,6 +5945,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
dev_net_set(dev, &init_net);
dev->gso_max_size = GSO_MAX_SIZE;
+ dev->gso_max_segs = GSO_MAX_SEGS;
INIT_LIST_HEAD(&dev->napi_list);
INIT_LIST_HEAD(&dev->unreg_list);
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCHv2 net 2/3] sfc: Fix maximum number of TSO segments and minimum TX queue size
From: Ben Hutchings @ 2012-07-31 1:57 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-net-drivers, Ben Greear, Eric Dumazet,
Stephen Hemminger
In-Reply-To: <1343699476.2667.69.camel@bwh-desktop.uk.solarflarecom.com>
Currently an skb requiring TSO may not fit within a minimum-size TX
queue. The TX queue selected for the skb may stall and trigger the TX
watchdog repeatedly (since the problem skb will be retried after the
TX reset). This issue is designated as CVE-2012-3412.
Set the maximum number of TSO segments for our devices to 100. This
should make no difference to behaviour unless the actual MSS is less
than about 700. Increase the minimum TX queue size accordingly to
allow for 2 worst-case skbs, so that there will definitely be space
to add an skb after we wake a queue.
To avoid invalidating existing configurations, change
efx_ethtool_set_ringparam() to fix up values that are too small rather
than returning -EINVAL.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
drivers/net/ethernet/sfc/efx.c | 6 ++++++
drivers/net/ethernet/sfc/efx.h | 14 ++++++++++----
drivers/net/ethernet/sfc/ethtool.c | 16 +++++++++++-----
drivers/net/ethernet/sfc/tx.c | 19 +++++++++++++++++++
4 files changed, 46 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 70554a1..65a8d49 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -1503,6 +1503,11 @@ static int efx_probe_all(struct efx_nic *efx)
goto fail2;
}
+ BUILD_BUG_ON(EFX_DEFAULT_DMAQ_SIZE < EFX_RXQ_MIN_ENT);
+ if (WARN_ON(EFX_DEFAULT_DMAQ_SIZE < EFX_TXQ_MIN_ENT(efx))) {
+ rc = -EINVAL;
+ goto fail3;
+ }
efx->rxq_entries = efx->txq_entries = EFX_DEFAULT_DMAQ_SIZE;
rc = efx_probe_filters(efx);
@@ -2070,6 +2075,7 @@ static int efx_register_netdev(struct efx_nic *efx)
net_dev->irq = efx->pci_dev->irq;
net_dev->netdev_ops = &efx_netdev_ops;
SET_ETHTOOL_OPS(net_dev, &efx_ethtool_ops);
+ net_dev->gso_max_segs = EFX_TSO_MAX_SEGS;
rtnl_lock();
diff --git a/drivers/net/ethernet/sfc/efx.h b/drivers/net/ethernet/sfc/efx.h
index be8f915..70755c9 100644
--- a/drivers/net/ethernet/sfc/efx.h
+++ b/drivers/net/ethernet/sfc/efx.h
@@ -30,6 +30,7 @@ extern netdev_tx_t
efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb);
extern void efx_xmit_done(struct efx_tx_queue *tx_queue, unsigned int index);
extern int efx_setup_tc(struct net_device *net_dev, u8 num_tc);
+extern unsigned int efx_tx_max_skb_descs(struct efx_nic *efx);
/* RX */
extern int efx_probe_rx_queue(struct efx_rx_queue *rx_queue);
@@ -52,10 +53,15 @@ extern void efx_schedule_slow_fill(struct efx_rx_queue *rx_queue);
#define EFX_MAX_EVQ_SIZE 16384UL
#define EFX_MIN_EVQ_SIZE 512UL
-/* The smallest [rt]xq_entries that the driver supports. Callers of
- * efx_wake_queue() assume that they can subsequently send at least one
- * skb. Falcon/A1 may require up to three descriptors per skb_frag. */
-#define EFX_MIN_RING_SIZE (roundup_pow_of_two(2 * 3 * MAX_SKB_FRAGS))
+/* Maximum number of TCP segments we support for soft-TSO */
+#define EFX_TSO_MAX_SEGS 100
+
+/* The smallest [rt]xq_entries that the driver supports. RX minimum
+ * is a bit arbitrary. For TX, we must have space for at least 2
+ * TSO skbs.
+ */
+#define EFX_RXQ_MIN_ENT 128U
+#define EFX_TXQ_MIN_ENT(efx) (2 * efx_tx_max_skb_descs(efx))
/* Filters */
extern int efx_probe_filters(struct efx_nic *efx);
diff --git a/drivers/net/ethernet/sfc/ethtool.c b/drivers/net/ethernet/sfc/ethtool.c
index 10536f9..8cba2df 100644
--- a/drivers/net/ethernet/sfc/ethtool.c
+++ b/drivers/net/ethernet/sfc/ethtool.c
@@ -680,21 +680,27 @@ static int efx_ethtool_set_ringparam(struct net_device *net_dev,
struct ethtool_ringparam *ring)
{
struct efx_nic *efx = netdev_priv(net_dev);
+ u32 txq_entries;
if (ring->rx_mini_pending || ring->rx_jumbo_pending ||
ring->rx_pending > EFX_MAX_DMAQ_SIZE ||
ring->tx_pending > EFX_MAX_DMAQ_SIZE)
return -EINVAL;
- if (ring->rx_pending < EFX_MIN_RING_SIZE ||
- ring->tx_pending < EFX_MIN_RING_SIZE) {
+ if (ring->rx_pending < EFX_RXQ_MIN_ENT) {
netif_err(efx, drv, efx->net_dev,
- "TX and RX queues cannot be smaller than %ld\n",
- EFX_MIN_RING_SIZE);
+ "RX queues cannot be smaller than %u\n",
+ EFX_RXQ_MIN_ENT);
return -EINVAL;
}
- return efx_realloc_channels(efx, ring->rx_pending, ring->tx_pending);
+ txq_entries = max(ring->tx_pending, EFX_TXQ_MIN_ENT(efx));
+ if (txq_entries != ring->tx_pending)
+ netif_warn(efx, drv, efx->net_dev,
+ "increasing TX queue size to minimum of %u\n",
+ txq_entries);
+
+ return efx_realloc_channels(efx, ring->rx_pending, txq_entries);
}
static int efx_ethtool_set_pauseparam(struct net_device *net_dev,
diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
index 9b225a7..1871343 100644
--- a/drivers/net/ethernet/sfc/tx.c
+++ b/drivers/net/ethernet/sfc/tx.c
@@ -119,6 +119,25 @@ efx_max_tx_len(struct efx_nic *efx, dma_addr_t dma_addr)
return len;
}
+unsigned int efx_tx_max_skb_descs(struct efx_nic *efx)
+{
+ /* Header and payload descriptor for each output segment, plus
+ * one for every input fragment boundary within a segment
+ */
+ unsigned int max_descs = EFX_TSO_MAX_SEGS * 2 + MAX_SKB_FRAGS;
+
+ /* Possibly one more per segment for the alignment workaround */
+ if (EFX_WORKAROUND_5391(efx))
+ max_descs += EFX_TSO_MAX_SEGS;
+
+ /* Possibly more for PCIe page boundaries within input fragments */
+ if (PAGE_SIZE > EFX_PAGE_SIZE)
+ max_descs += max_t(unsigned int, MAX_SKB_FRAGS,
+ DIV_ROUND_UP(GSO_MAX_SIZE, EFX_PAGE_SIZE));
+
+ return max_descs;
+}
+
/*
* Add a socket buffer to a TX queue
*
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* [PATCHv2 net 3/3] tcp: Apply device TSO segment limit earlier
From: Ben Hutchings @ 2012-07-31 2:11 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-net-drivers, Ben Greear, Eric Dumazet,
Stephen Hemminger
In-Reply-To: <1343699476.2667.69.camel@bwh-desktop.uk.solarflarecom.com>
Cache the device gso_max_segs in sock::sk_gso_max_segs and use it to
limit the size of TSO skbs. This avoids the need to fall back to
software GSO for local TCP senders.
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
This is similar to v1 patch 1, but using the device's gso_max_segs
instead of a constant. It also covers a couple of additional cases
where sk_gso_max_size is currently used. It improves performance in the
case that the limit is reached, but this may not be worth doing if
legitimate peers don't cause us to hit that limit.
Ben.
include/net/sock.h | 2 ++
net/core/sock.c | 1 +
net/ipv4/tcp.c | 4 +++-
net/ipv4/tcp_cong.c | 3 ++-
net/ipv4/tcp_output.c | 21 ++++++++++++---------
5 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index e067f8c..25a823a 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -218,6 +218,7 @@ struct cg_proto;
* @sk_route_nocaps: forbidden route capabilities (e.g NETIF_F_GSO_MASK)
* @sk_gso_type: GSO type (e.g. %SKB_GSO_TCPV4)
* @sk_gso_max_size: Maximum GSO segment size to build
+ * @sk_gso_max_segs: Maximum number of GSO segments
* @sk_lingertime: %SO_LINGER l_linger setting
* @sk_backlog: always used with the per-socket spinlock held
* @sk_callback_lock: used with the callbacks in the end of this struct
@@ -338,6 +339,7 @@ struct sock {
netdev_features_t sk_route_nocaps;
int sk_gso_type;
unsigned int sk_gso_max_size;
+ u16 sk_gso_max_segs;
int sk_rcvlowat;
unsigned long sk_lingertime;
struct sk_buff_head sk_error_queue;
diff --git a/net/core/sock.c b/net/core/sock.c
index 2676a88..34cc7bb 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1403,6 +1403,7 @@ void sk_setup_caps(struct sock *sk, struct dst_entry *dst)
} else {
sk->sk_route_caps |= NETIF_F_SG | NETIF_F_HW_CSUM;
sk->sk_gso_max_size = dst->dev->gso_max_size;
+ sk->sk_gso_max_segs = dst->dev->gso_max_segs;
}
}
}
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index e7e6eea..2109ff4 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -811,7 +811,9 @@ static unsigned int tcp_xmit_size_goal(struct sock *sk, u32 mss_now,
old_size_goal + mss_now > xmit_size_goal)) {
xmit_size_goal = old_size_goal;
} else {
- tp->xmit_size_goal_segs = xmit_size_goal / mss_now;
+ tp->xmit_size_goal_segs =
+ min_t(u16, xmit_size_goal / mss_now,
+ sk->sk_gso_max_segs);
xmit_size_goal = tp->xmit_size_goal_segs * mss_now;
}
}
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index 4d4db16..1432cdb 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -291,7 +291,8 @@ bool tcp_is_cwnd_limited(const struct sock *sk, u32 in_flight)
left = tp->snd_cwnd - in_flight;
if (sk_can_gso(sk) &&
left * sysctl_tcp_tso_win_divisor < tp->snd_cwnd &&
- left * tp->mss_cache < sk->sk_gso_max_size)
+ left * tp->mss_cache < sk->sk_gso_max_size &&
+ left < sk->sk_gso_max_segs)
return true;
return left <= tcp_max_tso_deferred_mss(tp);
}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 33cd065..0c9db8f 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1522,21 +1522,21 @@ static void tcp_cwnd_validate(struct sock *sk)
* when we would be allowed to send the split-due-to-Nagle skb fully.
*/
static unsigned int tcp_mss_split_point(const struct sock *sk, const struct sk_buff *skb,
- unsigned int mss_now, unsigned int cwnd)
+ unsigned int mss_now, unsigned int max_segs)
{
const struct tcp_sock *tp = tcp_sk(sk);
- u32 needed, window, cwnd_len;
+ u32 needed, window, max_len;
window = tcp_wnd_end(tp) - TCP_SKB_CB(skb)->seq;
- cwnd_len = mss_now * cwnd;
+ max_len = mss_now * max_segs;
- if (likely(cwnd_len <= window && skb != tcp_write_queue_tail(sk)))
- return cwnd_len;
+ if (likely(max_len <= window && skb != tcp_write_queue_tail(sk)))
+ return max_len;
needed = min(skb->len, window);
- if (cwnd_len <= needed)
- return cwnd_len;
+ if (max_len <= needed)
+ return max_len;
return needed - needed % mss_now;
}
@@ -1765,7 +1765,8 @@ static bool tcp_tso_should_defer(struct sock *sk, struct sk_buff *skb)
limit = min(send_win, cong_win);
/* If a full-sized TSO skb can be sent, do it. */
- if (limit >= sk->sk_gso_max_size)
+ if (limit >= min_t(unsigned int, sk->sk_gso_max_size,
+ sk->sk_gso_max_segs * tp->mss_cache))
goto send_now;
/* Middle in queue won't get any more data, full sendable already? */
@@ -1999,7 +2000,9 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
limit = mss_now;
if (tso_segs > 1 && !tcp_urg_mode(tp))
limit = tcp_mss_split_point(sk, skb, mss_now,
- cwnd_quota);
+ min_t(unsigned int,
+ cwnd_quota,
+ sk->sk_gso_max_segs));
if (skb->len > limit &&
unlikely(tso_fragment(sk, skb, limit, mss_now, gfp)))
--
1.7.7.6
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* Re: [RFC v2 1/2] PCI-Express Non-Transparent Bridge Support
From: Jianbin Kang @ 2012-07-31 3:35 UTC (permalink / raw)
To: Jon Mason; +Cc: Bjorn Helgaas, linux-kernel, netdev, linux-pci, Dave Jiang
In-Reply-To: <20120730181542.GA987@jonmason-lab>
> I've tried to make it all generic enough that non-Intel NTBs should plug in with
> minimal changes to ntb_hw.c. If their design is too divergent, then a slight
> redesign of ntb_hw.c might be necessary. But from what I've seen of other
> designs on the internet, they appear to be extremely similar. The transport and
> client drivers were written with the hardware abstracted away as much as
> possible to prevent the need to modify it for different hardware. If there is
> anything which is Intel hardware specific, I'd be happy to change it to make it
> more generic.
In ntb_process_tx(), ntb uses hard-coding 'memcpy_toio' to copy data
to remote.
Is it better to provide a function pointer like 'tx()' and call qp->tx().
memcpy_toio is a slow operation. Some hardware can setup a dma
transfer and wait.
IMHO, the best way is to handle tx in async mode. But it requires
lots of modification.
^ permalink raw reply
* [PATCH] ipv4: Restore old dst_free() behavior.
From: David Miller @ 2012-07-31 5:38 UTC (permalink / raw)
To: edumazet; +Cc: netdev
Eric, this is what I'd like to propose.
It seems the problem you were likely running into was simply
the fact that we were not inserting an RCU grace period for
the dst_free() that we do when purging a FIB nexthop.
So this reverts your change, and instead adds the necessary
call_rcu_bh() wrapper around the dst_free() done in fib_semantics.c
That makes it so that we don't need all of that inc_not_zero stuff for
sockets, and the special dst flag. If we set the pointer to NULL,
then do the dst_free() via RCU, we can test that refcount safely in
dst_free() since it can only decrease at that point.
What do you think? Does it pass your tests?
Thanks.
diff --git a/include/net/dst.h b/include/net/dst.h
index 31a9fd3..baf5978 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -61,7 +61,6 @@ struct dst_entry {
#define DST_NOPEER 0x0040
#define DST_FAKE_RTABLE 0x0080
#define DST_XFRM_TUNNEL 0x0100
-#define DST_RCU_FREE 0x0200
unsigned short pending_confirm;
@@ -383,6 +382,12 @@ static inline void dst_free(struct dst_entry *dst)
__dst_free(dst);
}
+static inline void dst_rcu_free(struct rcu_head *head)
+{
+ struct dst_entry *dst = container_of(head, struct dst_entry, rcu_head);
+ dst_free(dst);
+}
+
static inline void dst_confirm(struct dst_entry *dst)
{
dst->pending_confirm = 1;
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index e3fd34c..613cfa4 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -249,17 +249,4 @@ static inline __u8 inet_sk_flowi_flags(const struct sock *sk)
return flags;
}
-static inline void inet_sk_rx_dst_set(struct sock *sk, const struct sk_buff *skb)
-{
- struct dst_entry *dst = skb_dst(skb);
-
- if (atomic_inc_not_zero(&dst->__refcnt)) {
- if (!(dst->flags & DST_RCU_FREE))
- dst->flags |= DST_RCU_FREE;
-
- sk->sk_rx_dst = dst;
- inet_sk(sk)->rx_dst_ifindex = skb->skb_iif;
- }
-}
-
#endif /* _INET_SOCK_H */
diff --git a/net/core/dst.c b/net/core/dst.c
index d9e33eb..069d51d 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -258,15 +258,6 @@ again:
}
EXPORT_SYMBOL(dst_destroy);
-static void dst_rcu_destroy(struct rcu_head *head)
-{
- struct dst_entry *dst = container_of(head, struct dst_entry, rcu_head);
-
- dst = dst_destroy(dst);
- if (dst)
- __dst_free(dst);
-}
-
void dst_release(struct dst_entry *dst)
{
if (dst) {
@@ -274,14 +265,10 @@ void dst_release(struct dst_entry *dst)
newrefcnt = atomic_dec_return(&dst->__refcnt);
WARN_ON(newrefcnt < 0);
- if (unlikely(dst->flags & (DST_NOCACHE | DST_RCU_FREE)) && !newrefcnt) {
- if (dst->flags & DST_RCU_FREE) {
- call_rcu_bh(&dst->rcu_head, dst_rcu_destroy);
- } else {
- dst = dst_destroy(dst);
- if (dst)
- __dst_free(dst);
- }
+ if (unlikely(dst->flags & DST_NOCACHE) && !newrefcnt) {
+ dst = dst_destroy(dst);
+ if (dst)
+ __dst_free(dst);
}
}
}
@@ -333,14 +320,11 @@ EXPORT_SYMBOL(__dst_destroy_metrics_generic);
*/
void skb_dst_set_noref(struct sk_buff *skb, struct dst_entry *dst)
{
- bool hold;
-
WARN_ON(!rcu_read_lock_held() && !rcu_read_lock_bh_held());
/* If dst not in cache, we must take a reference, because
* dst_release() will destroy dst as soon as its refcount becomes zero
*/
- hold = (dst->flags & (DST_NOCACHE | DST_RCU_FREE)) == DST_NOCACHE;
- if (unlikely(hold)) {
+ if (unlikely(dst->flags & DST_NOCACHE)) {
dst_hold(dst);
skb_dst_set(skb, dst);
} else {
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 2671977..85a3604 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -184,12 +184,6 @@ static __inline__ unsigned int dn_hash(__le16 src, __le16 dst)
return dn_rt_hash_mask & (unsigned int)tmp;
}
-static inline void dst_rcu_free(struct rcu_head *head)
-{
- struct dst_entry *dst = container_of(head, struct dst_entry, rcu_head);
- dst_free(dst);
-}
-
static inline void dnrt_free(struct dn_route *rt)
{
call_rcu_bh(&rt->dst.rcu_head, dst_rcu_free);
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index e55171f..67bbaf5 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -161,6 +161,17 @@ static void free_nh_exceptions(struct fib_nh *nh)
kfree(hash);
}
+static void rt_nexthop_free(struct rtable **rtp)
+{
+ struct rtable *rt = *rtp;
+
+ if (!rt)
+ return;
+ *rtp = NULL;
+
+ call_rcu_bh(&rt->dst.rcu_head, dst_rcu_free);
+}
+
/* Release a nexthop info record */
static void free_fib_info_rcu(struct rcu_head *head)
{
@@ -171,10 +182,8 @@ static void free_fib_info_rcu(struct rcu_head *head)
dev_put(nexthop_nh->nh_dev);
if (nexthop_nh->nh_exceptions)
free_nh_exceptions(nexthop_nh);
- if (nexthop_nh->nh_rth_output)
- dst_release(&nexthop_nh->nh_rth_output->dst);
- if (nexthop_nh->nh_rth_input)
- dst_release(&nexthop_nh->nh_rth_input->dst);
+ rt_nexthop_free(&nexthop_nh->nh_rth_output);
+ rt_nexthop_free(&nexthop_nh->nh_rth_input);
} endfor_nexthops(fi);
release_net(fi->fib_net);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index d6eabcf..fc1a81c 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1199,6 +1199,11 @@ restart:
fnhe->fnhe_stamp = jiffies;
}
+static inline void rt_free(struct rtable *rt)
+{
+ call_rcu_bh(&rt->dst.rcu_head, dst_rcu_free);
+}
+
static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
{
struct rtable *orig, *prev, **p = &nh->nh_rth_output;
@@ -1208,14 +1213,17 @@ static void rt_cache_route(struct fib_nh *nh, struct rtable *rt)
orig = *p;
- rt->dst.flags |= DST_RCU_FREE;
- dst_hold(&rt->dst);
prev = cmpxchg(p, orig, rt);
if (prev == orig) {
if (orig)
- dst_release(&orig->dst);
+ rt_free(orig);
} else {
- dst_release(&rt->dst);
+ /* Routes we intend to cache in the FIB nexthop have
+ * the DST_NOCACHE bit clear. However, if we are
+ * unsuccessful at storing this route into the cache
+ * we really need to set it.
+ */
+ rt->dst.flags |= DST_NOCACHE;
}
}
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 9be30b0..a356e1f 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5604,7 +5604,8 @@ void tcp_finish_connect(struct sock *sk, struct sk_buff *skb)
tcp_set_state(sk, TCP_ESTABLISHED);
if (skb != NULL) {
- inet_sk_rx_dst_set(sk, skb);
+ sk->sk_rx_dst = dst_clone(skb_dst(skb));
+ inet_sk(sk)->rx_dst_ifindex = skb->skb_iif;
security_inet_conn_established(sk, skb);
}
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 7f91e5a..2fbd992 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1617,19 +1617,19 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
#endif
if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
- struct dst_entry *dst = sk->sk_rx_dst;
-
sock_rps_save_rxhash(sk, skb);
- if (dst) {
+ if (sk->sk_rx_dst) {
+ struct dst_entry *dst = sk->sk_rx_dst;
if (inet_sk(sk)->rx_dst_ifindex != skb->skb_iif ||
dst->ops->check(dst, 0) == NULL) {
dst_release(dst);
sk->sk_rx_dst = NULL;
}
}
- if (unlikely(sk->sk_rx_dst == NULL))
- inet_sk_rx_dst_set(sk, skb);
-
+ if (unlikely(sk->sk_rx_dst == NULL)) {
+ sk->sk_rx_dst = dst_clone(skb_dst(skb));
+ inet_sk(sk)->rx_dst_ifindex = skb->skb_iif;
+ }
if (tcp_rcv_established(sk, skb, tcp_hdr(skb), skb->len)) {
rsk = sk;
goto reset;
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 232a90c..3f1cc20 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -387,7 +387,8 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
struct tcp_sock *oldtp = tcp_sk(sk);
struct tcp_cookie_values *oldcvp = oldtp->cookie_values;
- inet_sk_rx_dst_set(newsk, skb);
+ newsk->sk_rx_dst = dst_clone(skb_dst(skb));
+ inet_sk(newsk)->rx_dst_ifindex = skb->skb_iif;
/* TCP Cookie Transactions require space for the cookie pair,
* as it differs for each connection. There is no need to
^ permalink raw reply related
* Re: [3.5 regression / mcs7830 / bisected] bridge constantly toggeling between disabled and forwarding
From: Michael Leun @ 2012-07-31 5:52 UTC (permalink / raw)
To: linux; +Cc: davem, netdev, linux-kernel, gregkh
In-Reply-To: <20120724013634.11bf1360@xenia.leun.net>
On Tue, 24 Jul 2012 01:36:34 +0200
Michael Leun <lkml20120218@newton.leun.net> wrote:
My report might sound like I'm complaining that link state detection
works because link state detection was implemented - sorry, but thats
not true.
I do NOT see this link state changes if there is no traffic on the
interface, but I start seeing them once I start pinging. I think this
is not the idea of link state detection?
I would very much appreciate if you could have a look into that,
because it is rather annoying.
> On Mon, 23 Jul 2012 09:15:04 +0200
> Michael Leun <lkml20120218@newton.leun.net> wrote:
>
> [see issue description below]
>
> Bisecting yielded
>
> b1ff4f96fd1c63890d78d8939c6e0f2b44ce3113 is the first bad commit
> commit b1ff4f96fd1c63890d78d8939c6e0f2b44ce3113
> Author: Ondrej Zary <linux@rainbow-software.org>
> Date: Fri Jun 1 10:29:08 2012 +0000
>
> mcs7830: Implement link state detection
>
> Add .status callback that detects link state changes.
> Tested with MCS7832CV-AA chip (9710:7830, identified as rev.C by the driver).
> Fixes https://bugzilla.kernel.org/show_bug.cgi?id=28532
>
> Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> :040000 040000 5480780cb5e75c57122a621fc3bab0108c16be27 d97efd9cc0a465dff76bcd3a3c547f718f2a5345 M drivers
>
>
> Reverting that from 3.5 makes the issue go away.
>
> > Hi,
> >
> > when I use my usb ethernet adapter
> >
> > # > lsusb
> > [...]
> > Bus 002 Device 009: ID 9710:7830 MosChip Semiconductor MCS7830 10/100 Mbps Ethernet adapter
> > [...]
> >
> > as port of an bridge
> >
> > > # brctl addbr br0
> > > # brctl addif br0 eth0
> > > # brctl addif br0 ue5
> > > # ifconfig ue5 up
> > > # ifconfig br0 up
> >
> > (Also does happen when eth0 is not part of the bridge, but the logs I
> > had available were from that situation...)
> >
> > I constantly get messages showing the interface toggeling between
> > disabled and forwarding state:
> >
> > Jul 23 07:40:50 elektra kernel: [ 1539.497337] br0: port 2(ue5) entered disabled state
> > Jul 23 07:40:50 elektra kernel: [ 1539.554992] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:40:50 elektra kernel: [ 1539.555005] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:40:51 elektra kernel: [ 1540.496242] br0: port 2(ue5) entered disabled state
> > Jul 23 07:40:51 elektra kernel: [ 1540.552534] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:40:51 elektra kernel: [ 1540.552548] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:40:52 elektra kernel: [ 1541.550413] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:40:53 elektra kernel: [ 1542.529672] br0: port 2(ue5) entered disabled state
> > Jul 23 07:40:53 elektra kernel: [ 1542.587162] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:40:53 elektra kernel: [ 1542.587175] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:40:54 elektra kernel: [ 1543.585309] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:41:00 elektra kernel: [ 1549.360600] br0: port 2(ue5) entered disabled state
> > Jul 23 07:41:00 elektra kernel: [ 1549.442998] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:41:00 elektra kernel: [ 1549.443011] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:41:01 elektra kernel: [ 1550.357686] br0: port 2(ue5) entered disabled state
> > Jul 23 07:41:01 elektra kernel: [ 1550.408208] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:41:01 elektra kernel: [ 1550.408222] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:41:02 elektra kernel: [ 1551.407656] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:41:03 elektra kernel: [ 1552.401578] br0: port 2(ue5) entered disabled state
> > Jul 23 07:41:03 elektra kernel: [ 1552.474773] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:41:03 elektra kernel: [ 1552.474786] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:41:04 elektra kernel: [ 1553.472487] br0: port 2(ue5) entered forwarding state
> > Jul 23 07:41:05 elektra kernel: [ 1554.356138] br0: port 2(ue5) entered disabled state
> > [...]
> >
> > This does (in the same situation, nothing else than the kernel changed)
> > not happen with 3.4.5.
> >
> > Does anybody have an idea what the issue might be or do I need to bisect?
>
>
> --
> MfG,
>
> Michael Leun
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
MfG,
Michael Leun
^ permalink raw reply
* Re: [PATCH] ipv4: Restore old dst_free() behavior.
From: Eric Dumazet @ 2012-07-31 5:54 UTC (permalink / raw)
To: David Miller; +Cc: edumazet, netdev
In-Reply-To: <20120730.223827.74792864437911339.davem@davemloft.net>
On Mon, 2012-07-30 at 22:38 -0700, David Miller wrote:
> Eric, this is what I'd like to propose.
>
> It seems the problem you were likely running into was simply
> the fact that we were not inserting an RCU grace period for
> the dst_free() that we do when purging a FIB nexthop.
>
> So this reverts your change, and instead adds the necessary
> call_rcu_bh() wrapper around the dst_free() done in fib_semantics.c
>
> That makes it so that we don't need all of that inc_not_zero stuff for
> sockets, and the special dst flag. If we set the pointer to NULL,
> then do the dst_free() via RCU, we can test that refcount safely in
> dst_free() since it can only decrease at that point.
>
> What do you think? Does it pass your tests?
>
> Thanks.
I'll test that ASAP, I was trying to understand why Linus tree gave me a
non workable machine ( a panic in igb driver ... NULL RIP) .
I dont understand how I did not have this bug with net tree.
Please give me a couple of hours, I need to break my fast ;)
^ permalink raw reply
* Re: [PATCH] ipv4: Restore old dst_free() behavior.
From: David Miller @ 2012-07-31 5:57 UTC (permalink / raw)
To: eric.dumazet; +Cc: edumazet, netdev
In-Reply-To: <1343714097.21269.38.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 31 Jul 2012 07:54:57 +0200
> Please give me a couple of hours, I need to break my fast ;)
No problem.
Meanwhile, I'm busy rebasing my uncached list patches :-)
^ permalink raw reply
* [BUG] igb: panic at boot time with latest Linus tree
From: Eric Dumazet @ 2012-07-31 6:14 UTC (permalink / raw)
To: David Miller, Jeff Kirsher; +Cc: netdev
For information, I get this each time I boot on latest Linus tree :
RTNL is left locked, so machine unusable.
No problem with David net tree, so thats a bit strange...
Not sure I'll have time to investigate today
(And tomorrow I am off for the day)
[ 11.153682] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 11.153806] IP: [< (null)>] (null)
[ 11.153872] PGD 310544067 PUD 311b2d067 PMD 0
[ 11.153945] Oops: 0010 [#1] SMP
[ 11.154012] Modules linked in: nfsd exportfs nfs lockd auth_rpcgss sunrpc asix usbnet rt61pci crc_itu_t rt2x00pci rt2x00lib eeprom_93cx6 tg3 ixgbe mdio igb
[ 11.154227] CPU 10
[ 11.154239] Pid: 2476, comm: NetworkManager Not tainted 3.5.0+ #123 Hewlett-Packard HP Z600 Workstation/0B54h
[ 11.154437] RIP: 0010:[<0000000000000000>] [< (null)>] (null)
[ 11.154574] RSP: 0018:ffff88030f667630 EFLAGS: 00010282
[ 11.154645] RAX: 0000000017cf7980 RBX: ffff88031080f100 RCX: 0000000000000200
[ 11.154720] RDX: 0000000000000040 RSI: ffffea0017cf7980 RDI: ffff880611bc1098
[ 11.154794] RBP: ffff88030f667678 R08: 0000000000000002 R09: 0000000000000000
[ 11.154868] R10: ffffea0017cf7980 R11: 0000000000000000 R12: ffffc90007d58000
[ 11.154942] R13: ffff880611bc1098 R14: ffff8803106ff000 R15: ffff8805f3de6040
[ 11.155016] FS: 00007fa2e4b94800(0000) GS:ffff88061fc80000(0000) knlGS:0000000000000000
[ 11.155148] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 11.155220] CR2: 0000000000000000 CR3: 000000031066a000 CR4: 00000000000007e0
[ 11.155283] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 11.155347] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 11.155411] Process NetworkManager (pid: 2476, threadinfo ffff88030f666000, task ffff880310dd2d00)
[ 11.155523] Stack:
[ 11.155579] ffffffffa00071fe 0000000000000000 00ffff00a000400a ffff88030f667678
[ 11.155712] ffff88030f610700 0000000000000001 ffff88030f610708 ffff88030f610740
[ 11.155840] 0000000000000001 ffff88030f6676b8 ffffffffa000834b ffff88030f610700
[ 11.155969] Call Trace:
[ 11.156036] [<ffffffffa00071fe>] ? igb_alloc_rx_buffers+0x13e/0x2d0 [igb]
[ 11.156104] [<ffffffffa000834b>] igb_configure+0x34b/0x4d0 [igb]
[ 11.156170] [<ffffffffa0008572>] __igb_open+0xa2/0x510 [igb]
[ 11.156237] [<ffffffff812c0731>] ? find_next_bit+0x21/0xd0
[ 11.156303] [<ffffffffa0008b50>] igb_open+0x10/0x20 [igb]
[ 11.156369] [<ffffffff8155288f>] __dev_open+0x8f/0xe0
[ 11.156432] [<ffffffff81552b31>] __dev_change_flags+0xa1/0x180
[ 11.156495] [<ffffffff81552cc8>] dev_change_flags+0x28/0x70
[ 11.156559] [<ffffffff8155f664>] do_setlink+0x284/0x9e0
[ 11.156624] [<ffffffff81560c8a>] ? rtnl_fill_ifinfo+0x92a/0xb30
[ 11.156690] [<ffffffff812cc220>] ? nla_parse+0x30/0xe0
[ 11.156755] [<ffffffff81561b35>] rtnl_newlink+0x345/0x580
[ 11.156820] [<ffffffff812655b9>] ? selinux_capable+0x39/0x50
[ 11.156885] [<ffffffff81262538>] ? security_capable+0x18/0x20
[ 11.156948] [<ffffffff81561384>] rtnetlink_rcv_msg+0x124/0x310
[ 11.157012] [<ffffffff8113794b>] ? kfree+0x3b/0x160
[ 11.157074] [<ffffffff81561260>] ? __rtnl_unlock+0x20/0x20
[ 11.157137] [<ffffffff8157a569>] netlink_rcv_skb+0xa9/0xd0
[ 11.157200] [<ffffffff8155ed55>] rtnetlink_rcv+0x25/0x40
[ 11.157262] [<ffffffff81579f4d>] netlink_unicast+0x1ad/0x230
[ 11.157326] [<ffffffff8157a286>] netlink_sendmsg+0x2b6/0x310
[ 11.157396] [<ffffffff815381ac>] sock_sendmsg+0xdc/0xf0
[ 11.157459] [<ffffffff8153aab8>] ? move_addr_to_kernel+0x68/0xb0
[ 11.157526] [<ffffffff81539b12>] __sys_sendmsg+0x392/0x3a0
[ 11.157590] [<ffffffff81118bbe>] ? handle_mm_fault+0x13e/0x210
[ 11.157656] [<ffffffff81155008>] ? __d_free+0x48/0x70
[ 11.157720] [<ffffffff8115e616>] ? mntput+0x26/0x40
[ 11.157783] [<ffffffff81140371>] ? __fput+0x191/0x250
[ 11.157846] [<ffffffff8153b809>] sys_sendmsg+0x49/0x90
[ 11.157911] [<ffffffff816ca652>] system_call_fastpath+0x16/0x1b
[ 11.157973] Code: Bad RIP value.
[ 11.158041] RIP [< (null)>] (null)
[ 11.158106] RSP <ffff88030f667630>
[ 11.158163] CR2: 0000000000000000
[ 11.158227] ---[ end trace bbfaed088efd61cb ]---
[ 11.158300] NetworkManager (2476) used greatest stack depth: 2936 bytes left
^ permalink raw reply
* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Xufeng Zhang @ 2012-07-31 6:17 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Neil Horman, xufeng zhang, sri, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <CA+=dFzh1wQx2rBs_2RAwrXsz79WS3njnO8=2ntQZUbB5So69gg@mail.gmail.com>
I'm wondering if the below solution is fine to you which is based on
your changes.
BTW, I have verified this patch and it works ok for all the situation,
but only one problem persists:
there is a potential that commands will exceeds SCTP_MAX_NUM_COMMANDS
which happens during sending lots of small error DATA chunks.
Thanks,
Xufeng Zhang
---
diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
index 712b3be..62c34f5 100644
--- a/include/net/sctp/command.h
+++ b/include/net/sctp/command.h
@@ -110,6 +110,7 @@ typedef enum {
SCTP_CMD_SEND_NEXT_ASCONF, /* Send the next ASCONF after ACK */
SCTP_CMD_PURGE_ASCONF_QUEUE, /* Purge all asconf queues.*/
SCTP_CMD_SET_ASOC, /* Restore association context */
+ SCTP_CMD_GEN_BAD_STREAM, /* Invalid Stream errors happened command */
SCTP_CMD_LAST
} sctp_verb_t;
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index fc5e600..3d218e0 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1183,6 +1183,9 @@ struct sctp_outq {
*/
struct list_head abandoned;
+ /* Put Invalid Stream error chunks on this list */
+ struct list_head bad_stream_err;
+
/* How many unackd bytes do we have in-flight? */
__u32 outstanding_bytes;
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index e7aa177..1e87b0b 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -211,6 +211,7 @@ void sctp_outq_init(struct sctp_association *asoc,
struct sctp_outq *q)
INIT_LIST_HEAD(&q->retransmit);
INIT_LIST_HEAD(&q->sacked);
INIT_LIST_HEAD(&q->abandoned);
+ INIT_LIST_HEAD(&q->bad_stream_err);
q->fast_rtx = 0;
q->outstanding_bytes = 0;
@@ -283,6 +284,12 @@ void sctp_outq_teardown(struct sctp_outq *q)
list_del_init(&chunk->list);
sctp_chunk_free(chunk);
}
+
+ /* Throw away any pending Invalid Stream error chunks */
+ list_for_each_entry_safe(chunk, tmp, &q->bad_stream_err, list) {
+ list_del_init(&chunk->list);
+ sctp_chunk_free(chunk);
+ }
}
/* Free the outqueue structure and any related pending chunks. */
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index fe99628..ab63fa1 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -1060,6 +1060,17 @@ static void sctp_cmd_send_asconf(struct
sctp_association *asoc)
}
}
+static void sctp_cmd_make_inv_stream_err(sctp_cmd_seq_t *commands,
+ struct sctp_association *asoc)
+{
+ struct sctp_chunk *err, *tmp;
+ struct sctp_outq *q = &asoc->outqueue;
+
+ list_for_each_entry_safe(err, tmp, &q->bad_stream_err, list) {
+ list_del_init(&err->list);
+ sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(err));
+ struct sctp_association *asoc)
+{
+ struct sctp_chunk *err, *tmp;
+ struct sctp_outq *q = &asoc->outqueue;
+
+ list_for_each_entry_safe(err, tmp, &q->bad_stream_err, list) {
+ list_del_init(&err->list);
+ sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(err));
+ }
+}
/* These three macros allow us to pull the debugging code out of the
* main flow of sctp_do_sm() to keep attention focused on the real
@@ -1724,6 +1735,10 @@ static int sctp_cmd_interpreter(sctp_event_t event_type,
asoc = cmd->obj.asoc;
break;
+ case SCTP_CMD_GEN_BAD_STREAM:
+ sctp_cmd_make_inv_stream_err(commands, asoc);
+ break;
+
default:
pr_warn("Impossible command: %u, %p\n",
cmd->verb, cmd->obj.ptr);
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 9fca103..cab539f 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -2967,8 +2967,14 @@ discard_force:
return SCTP_DISPOSITION_DISCARD;
discard_noforce:
- if (chunk->end_of_packet)
+ if (chunk->end_of_packet) {
+ struct sctp_outq *q = &asoc->outqueue;
+
sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, force);
+ /* Queue the INVALID STREAM error after the SACK if
one is needed. */
+ if (!list_empty(&q->bad_stream_err))
+ sctp_add_cmd_sf(commands,
SCTP_CMD_GEN_BAD_STREAM, SCTP_NULL());
+ }
return SCTP_DISPOSITION_DISCARD;
consume:
@@ -3037,11 +3043,15 @@ sctp_disposition_t
sctp_sf_eat_data_fast_4_4(const struct sctp_endpoint *ep,
* with a SACK, a SHUTDOWN chunk, and restart the T2-shutdown timer
*/
if (chunk->end_of_packet) {
+ struct sctp_outq *q = &asoc->outqueue;
/* We must delay the chunk creation since the cumulative
* TSN has not been updated yet.
*/
sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SHUTDOWN, SCTP_NULL());
sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, SCTP_FORCE());
+ /* Queue the INVALID STREAM error after the SACK if
one is needed. */
+ if (!list_empty(&q->bad_stream_err))
+ sctp_add_cmd_sf(commands,
SCTP_CMD_GEN_BAD_STREAM, SCTP_NULL());
sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_RESTART,
SCTP_TO(SCTP_EVENT_TIMEOUT_T2_SHUTDOWN));
}
@@ -6136,6 +6146,7 @@ static int sctp_eat_data(const struct
sctp_association *asoc,
*/
sid = ntohs(data_hdr->stream);
if (sid >= asoc->c.sinit_max_instreams) {
+ struct sctp_outq *q = &asoc->outqueue;
/* Mark tsn as received even though we drop it */
sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_TSN, SCTP_U32(tsn));
@@ -6144,8 +6155,7 @@ static int sctp_eat_data(const struct
sctp_association *asoc,
sizeof(data_hdr->stream),
sizeof(u16));
if (err)
- sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
- SCTP_CHUNK(err));
+ list_add_tail(&err->list, &q->bad_stream_err);
return SCTP_IERROR_BAD_STREAM;
}
^ permalink raw reply related
* Re: [BUG] igb: panic at boot time with latest Linus tree
From: Jeff Kirsher @ 2012-07-31 6:26 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1343715255.21269.47.camel@edumazet-glaptop>
[-- Attachment #1: Type: text/plain, Size: 7783 bytes --]
On Tue, 2012-07-31 at 08:14 +0200, Eric Dumazet wrote:
> For information, I get this each time I boot on latest Linus tree :
>
> RTNL is left locked, so machine unusable.
>
> No problem with David net tree, so thats a bit strange...
>
> Not sure I'll have time to investigate today
> (And tomorrow I am off for the day)
>
> [ 11.153682] BUG: unable to handle kernel NULL pointer dereference at (null)
> [ 11.153806] IP: [< (null)>] (null)
> [ 11.153872] PGD 310544067 PUD 311b2d067 PMD 0
> [ 11.153945] Oops: 0010 [#1] SMP
> [ 11.154012] Modules linked in: nfsd exportfs nfs lockd auth_rpcgss sunrpc asix usbnet rt61pci crc_itu_t rt2x00pci rt2x00lib eeprom_93cx6 tg3 ixgbe mdio igb
> [ 11.154227] CPU 10
> [ 11.154239] Pid: 2476, comm: NetworkManager Not tainted 3.5.0+ #123 Hewlett-Packard HP Z600 Workstation/0B54h
> [ 11.154437] RIP: 0010:[<0000000000000000>] [< (null)>] (null)
> [ 11.154574] RSP: 0018:ffff88030f667630 EFLAGS: 00010282
> [ 11.154645] RAX: 0000000017cf7980 RBX: ffff88031080f100 RCX: 0000000000000200
> [ 11.154720] RDX: 0000000000000040 RSI: ffffea0017cf7980 RDI: ffff880611bc1098
> [ 11.154794] RBP: ffff88030f667678 R08: 0000000000000002 R09: 0000000000000000
> [ 11.154868] R10: ffffea0017cf7980 R11: 0000000000000000 R12: ffffc90007d58000
> [ 11.154942] R13: ffff880611bc1098 R14: ffff8803106ff000 R15: ffff8805f3de6040
> [ 11.155016] FS: 00007fa2e4b94800(0000) GS:ffff88061fc80000(0000) knlGS:0000000000000000
> [ 11.155148] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 11.155220] CR2: 0000000000000000 CR3: 000000031066a000 CR4: 00000000000007e0
> [ 11.155283] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 11.155347] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 11.155411] Process NetworkManager (pid: 2476, threadinfo ffff88030f666000, task ffff880310dd2d00)
> [ 11.155523] Stack:
> [ 11.155579] ffffffffa00071fe 0000000000000000 00ffff00a000400a ffff88030f667678
> [ 11.155712] ffff88030f610700 0000000000000001 ffff88030f610708 ffff88030f610740
> [ 11.155840] 0000000000000001 ffff88030f6676b8 ffffffffa000834b ffff88030f610700
> [ 11.155969] Call Trace:
> [ 11.156036] [<ffffffffa00071fe>] ? igb_alloc_rx_buffers+0x13e/0x2d0 [igb]
> [ 11.156104] [<ffffffffa000834b>] igb_configure+0x34b/0x4d0 [igb]
> [ 11.156170] [<ffffffffa0008572>] __igb_open+0xa2/0x510 [igb]
> [ 11.156237] [<ffffffff812c0731>] ? find_next_bit+0x21/0xd0
> [ 11.156303] [<ffffffffa0008b50>] igb_open+0x10/0x20 [igb]
> [ 11.156369] [<ffffffff8155288f>] __dev_open+0x8f/0xe0
> [ 11.156432] [<ffffffff81552b31>] __dev_change_flags+0xa1/0x180
> [ 11.156495] [<ffffffff81552cc8>] dev_change_flags+0x28/0x70
> [ 11.156559] [<ffffffff8155f664>] do_setlink+0x284/0x9e0
> [ 11.156624] [<ffffffff81560c8a>] ? rtnl_fill_ifinfo+0x92a/0xb30
> [ 11.156690] [<ffffffff812cc220>] ? nla_parse+0x30/0xe0
> [ 11.156755] [<ffffffff81561b35>] rtnl_newlink+0x345/0x580
> [ 11.156820] [<ffffffff812655b9>] ? selinux_capable+0x39/0x50
> [ 11.156885] [<ffffffff81262538>] ? security_capable+0x18/0x20
> [ 11.156948] [<ffffffff81561384>] rtnetlink_rcv_msg+0x124/0x310
> [ 11.157012] [<ffffffff8113794b>] ? kfree+0x3b/0x160
> [ 11.157074] [<ffffffff81561260>] ? __rtnl_unlock+0x20/0x20
> [ 11.157137] [<ffffffff8157a569>] netlink_rcv_skb+0xa9/0xd0
> [ 11.157200] [<ffffffff8155ed55>] rtnetlink_rcv+0x25/0x40
> [ 11.157262] [<ffffffff81579f4d>] netlink_unicast+0x1ad/0x230
> [ 11.157326] [<ffffffff8157a286>] netlink_sendmsg+0x2b6/0x310
> [ 11.157396] [<ffffffff815381ac>] sock_sendmsg+0xdc/0xf0
> [ 11.157459] [<ffffffff8153aab8>] ? move_addr_to_kernel+0x68/0xb0
> [ 11.157526] [<ffffffff81539b12>] __sys_sendmsg+0x392/0x3a0
> [ 11.157590] [<ffffffff81118bbe>] ? handle_mm_fault+0x13e/0x210
> [ 11.157656] [<ffffffff81155008>] ? __d_free+0x48/0x70
> [ 11.157720] [<ffffffff8115e616>] ? mntput+0x26/0x40
> [ 11.157783] [<ffffffff81140371>] ? __fput+0x191/0x250
> [ 11.157846] [<ffffffff8153b809>] sys_sendmsg+0x49/0x90
> [ 11.157911] [<ffffffff816ca652>] system_call_fastpath+0x16/0x1b
> [ 11.157973] Code: Bad RIP value.
> [ 11.158041] RIP [< (null)>] (null)
> [ 11.158106] RSP <ffff88030f667630>
> [ 11.158163] CR2: 0000000000000000
> [ 11.158227] ---[ end trace bbfaed088efd61cb ]---
> [ 11.158300] NetworkManager (2476) used greatest stack depth: 2936 bytes left
>
>
I have an igb patch current in test to fix a panic in igb. Here is the
patch:
From: Emil Tantilov <emil.s.tantilov@intel.com>
Subject: igb: fix panic while dumping packets on Tx hang with IOMMU
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>
---
drivers/net/ethernet/intel/igb/igb_main.c | 19 +++++++++----------
1 files 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 447e131..8d7e5da 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);
}
}
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply related
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