* [RFC] SFQ planned changes
From: Eric Dumazet @ 2012-01-03 10:40 UTC (permalink / raw)
To: Dave Taht; +Cc: Michal Kubeček, netdev, John A. Sullivan III
In-Reply-To: <CAA93jw5NYChp+hKWjOWiw7SnGhNe+REofr2KM-85GnKNVpkw6w@mail.gmail.com>
Le mardi 03 janvier 2012 à 10:36 +0100, Dave Taht a écrit :
> I note that (as of yesterday) sfq is performing as well as qfq did
> under most workloads, and is considerably simpler than qfq, but
> what I have in mind for shaping in a asymmetric scenario
> *may* involve 'weighting' - rather than strictly prioritizing -
> small acks... and it may not - I'd like to be able to benchmark
> the various AQM approaches against a variety of workloads
> before declaring victory.
A QFQ setup with more than 1024 classes/qdisc is way too slow at init
time, and consume ~384 bytes per class : ~12582912 bytes for 32768
classes.
We also are limited to 65536 qdisc per device, so QFQ setup using hash
is limited to a 32768 divisor.
Now SFQ as implemented in Linux is very limited, with at most 127 flows
and limit of 127 packets. [ So if 127 flows are active, we have one
packet per flow ]
I plan to add to SFQ following features :
- Ability to specify a per flow limit
Its what is called the 'depth',
currently hardcoded to min(127, limit)
- Ability to have up to 65535 flows (instead of 127)
- Ability to have a head drop (to drop old packets from a flow)
example of use : No more than 20 packets per flow, max 8000 flows, max
20000 packets in SFQ qdisc, hash table of 65536 slots.
tc qdisc add ... sfq \
flows 8000 \
depth 20 \
headdrop \
limit 20000 divisor 65536
Ram usage : 32 bytes per flow, instead of 384 for QFQ, so much better
cache hit ratio. 2 bytes per hash table slots, instead of 8 for QFQ.
(perturb timer for a huge SFQ setup would be not recommended)
^ permalink raw reply
* RE: [PATCH 06/19] netfilter: nf_conntrack: use atomic64 for accounting counters
From: David Laight @ 2012-01-03 12:01 UTC (permalink / raw)
To: pablo, netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1324778255-2830-7-git-send-email-pablo@netfilter.org>
> if (acct) {
> - spin_lock_bh(&ct->lock);
> - acct[CTINFO2DIR(ctinfo)].packets++;
> - acct[CTINFO2DIR(ctinfo)].bytes += skb->len;
> - spin_unlock_bh(&ct->lock);
> + atomic64_inc(&acct[CTINFO2DIR(ctinfo)].packets);
> + atomic64_add(skb->len,
&acct[CTINFO2DIR(ctinfo)].bytes);
> }
On a 32bit arch the two atomic64 operations require a locked
bus cycle each. The spin_unlock_bh() may not need one - so
the code may now be slower (modulo lock contention etc).
Probably worth caching &acct[CTINFO2DIR(ctinfo)] in a local,
the compiler probably can't do it itself.
David
^ permalink raw reply
* Re: [RFC] SFQ planned changes
From: Dave Taht @ 2012-01-03 12:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Michal Kubeček, netdev, John A. Sullivan III
In-Reply-To: <1325587235.2320.37.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
It will take me a while to fully comment on this... there are
all sorts of subtlties to deal with (one biggie - ledbat vs multi-queue
behavior)... but I am encouraged by the events of the past
months and my testing today....
On Tue, Jan 3, 2012 at 11:40 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 03 janvier 2012 à 10:36 +0100, Dave Taht a écrit :
>
>> I note that (as of yesterday) sfq is performing as well as qfq did
>> under most workloads, and is considerably simpler than qfq, but
>> what I have in mind for shaping in a asymmetric scenario
>> *may* involve 'weighting' - rather than strictly prioritizing -
>> small acks... and it may not - I'd like to be able to benchmark
I need to be clear that the above is a subtle problem that I'd
have to talk to in a separate mail - AND both SFQ and QFQ
do such a better job than wshaper did in the first place that
weighting small acks only wins in a limited number of
scenarios.
We have a larger problem in dealing with TSO/GSO
size superpackets that's hard to solve.
I'd prefer to think, design tests, and benchmark, and think again
for a while...
>> the various AQM approaches against a variety of workloads
>> before declaring victory.
>
>
> A QFQ setup with more than 1024 classes/qdisc is way too slow at init
> time, and consume ~384 bytes per class : ~12582912 bytes for 32768
> classes.
QFQ could be improved with some of the same techniques you
describe below.
> We also are limited to 65536 qdisc per device, so QFQ setup using hash
> is limited to a 32768 divisor.
>
>
> Now SFQ as implemented in Linux is very limited, with at most 127 flows
> and limit of 127 packets. [ So if 127 flows are active, we have one
> packet per flow ]
I agree SFQ can be improved upwards in scale, greatly.
My own personal goal is to evolve towards something that
can replace pfifo_fast as the default in linux.
I don't know if that goal is shared by all as yet. :)
> I plan to add to SFQ following features :
From a 'doing science' perspective, I'd like it if it remained possible
to continue using and benchmarking SFQ as it was, and create
this set of ideas as a new qdisc ('efq'?)
As these changes seem to require changes to userspace tc, anyway,
and (selfishly) my patching burden is great enough...
Perhaps some additional benefit could be had by losing
full backward API compatability with sfq, as well?
> - Ability to specify a per flow limit
> Its what is called the 'depth',
> currently hardcoded to min(127, limit)
Introducing per-flow buffering (as QFQ does) *re-introduces*
the overall AQM problem of managing the size of the
individual flows.
this CDF graph shows how badly wireless is currently behaving
(courtesy Albert Rafetseder of the university of vienna)
http://www.teklibre.com/~d/bloat/qfq_vs_pfifo_fast_wireless_iwl_card_vs_cerowrt.pdf
(I have to convince gnuplot to give me these!!)
If I were to add a larger sub-qdisc depth on QFQ than what's in there
(presently 24)
the same graph would also show the median latency increase proportionately.
The Time in Queue idea for managing that queue depth is quite
strong, there may be others.
(in fact, I'm carrying your preliminary TiQ patch in my
bql trees, not that I've done anything with it yet)
> - Ability to have up to 65535 flows (instead of 127)
>
> - Ability to have a head drop (to drop old packets from a flow)
The head drop idea is strong, when combined with time in queue.
However: it would be useful to be able to pull forward the next packet
in that sub-queue and deliver it, so as to provide proper signalling
upstream. Packets nowadays arrive in bursts, which means that
once one time stamp has expired, many will. What I just suggested
would (worst case) deliver every other packet in a backlog and
obviously needs refinement.....
>
> example of use : No more than 20 packets per flow, max 8000 flows, max
> 20000 packets in SFQ qdisc, hash table of 65536 slots.
>
> tc qdisc add ... sfq \
> flows 8000 \
> depth 20 \
> headdrop \
> limit 20000 divisor 65536
>
> Ram usage : 32 bytes per flow, instead of 384 for QFQ, so much better
> cache hit ratio. 2 bytes per hash table slots, instead of 8 for QFQ.
I do like it!
I retain liking for QFQ because other qdiscs (red, for example)
can be attached to it, but having a simple, yet good default with SFQ
scaled up to modern requirements would also be awesome!
> (perturb timer for a huge SFQ setup would be not recommended)
no kidding!
>
>
>
--
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
FR Tel: 0638645374
http://www.bufferbloat.net
^ permalink raw reply
* Re: tc filter mask for ACK packets off?
From: John A. Sullivan III @ 2012-01-03 12:18 UTC (permalink / raw)
To: Dave Taht; +Cc: Michal Kubeček, netdev
In-Reply-To: <CAA93jw5NYChp+hKWjOWiw7SnGhNe+REofr2KM-85GnKNVpkw6w@mail.gmail.com>
On Tue, 2012-01-03 at 10:36 +0100, Dave Taht wrote:
<snip>
> I'd go into more detail, but after what I hope are the final two
> fixes to sfq and qfq land in the net-next kernel (after some more
> testing), I like to think I have a more valid approach than this
> in the works, but that too will require some more development
> and testing.
>
> http://www.teklibre.com/~d/bloat/pfifo_fast_vs_sfq_qfq_linear.png
>
<snip>
Hmmm . . . certainly shattered my concerns about replacing pfifo_fast
with SFQ! Thanks - John
^ permalink raw reply
* Re: tc filter mask for ACK packets off?
From: Eric Dumazet @ 2012-01-03 12:32 UTC (permalink / raw)
To: John A. Sullivan III; +Cc: Dave Taht, Michal Kubeček, netdev
In-Reply-To: <1325593115.7219.36.camel@denise.theartistscloset.com>
Le mardi 03 janvier 2012 à 07:18 -0500, John A. Sullivan III a écrit :
> On Tue, 2012-01-03 at 10:36 +0100, Dave Taht wrote:
> <snip>
> > I'd go into more detail, but after what I hope are the final two
> > fixes to sfq and qfq land in the net-next kernel (after some more
> > testing), I like to think I have a more valid approach than this
> > in the works, but that too will require some more development
> > and testing.
> >
> > http://www.teklibre.com/~d/bloat/pfifo_fast_vs_sfq_qfq_linear.png
> >
> <snip>
> Hmmm . . . certainly shattered my concerns about replacing pfifo_fast
> with SFQ! Thanks - John
Before you do, take the time to read the warning in sfq source :
ADVANTAGE:
- It is very cheap. Both CPU and memory requirements are minimal.
DRAWBACKS:
- "Stochastic" -> It is not 100% fair.
When hash collisions occur, several flows are considered as one.
- "Round-robin" -> It introduces larger delays than virtual clock
based schemes, and should not be used for isolating interactive
traffic from non-interactive. It means, that this scheduler
should be used as leaf of CBQ or P3, which put interactive traffic
to higher priority band.
SFQ (as a direct replacement of dev root qdisc) is fine if most of your trafic
is of same kind/priority.
^ permalink raw reply
* Re: tc filter mask for ACK packets off?
From: John A. Sullivan III @ 2012-01-03 12:45 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Dave Taht, Michal Kubeček, netdev
In-Reply-To: <1325593951.2320.40.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
On Tue, 2012-01-03 at 13:32 +0100, Eric Dumazet wrote:
> Le mardi 03 janvier 2012 à 07:18 -0500, John A. Sullivan III a écrit :
> > On Tue, 2012-01-03 at 10:36 +0100, Dave Taht wrote:
> > <snip>
> > > I'd go into more detail, but after what I hope are the final two
> > > fixes to sfq and qfq land in the net-next kernel (after some more
> > > testing), I like to think I have a more valid approach than this
> > > in the works, but that too will require some more development
> > > and testing.
> > >
> > > http://www.teklibre.com/~d/bloat/pfifo_fast_vs_sfq_qfq_linear.png
> > >
> > <snip>
> > Hmmm . . . certainly shattered my concerns about replacing pfifo_fast
> > with SFQ! Thanks - John
>
> Before you do, take the time to read the warning in sfq source :
>
>
> ADVANTAGE:
>
> - It is very cheap. Both CPU and memory requirements are minimal.
>
> DRAWBACKS:
>
> - "Stochastic" -> It is not 100% fair.
> When hash collisions occur, several flows are considered as one.
>
> - "Round-robin" -> It introduces larger delays than virtual clock
> based schemes, and should not be used for isolating interactive
> traffic from non-interactive. It means, that this scheduler
> should be used as leaf of CBQ or P3, which put interactive traffic
> to higher priority band.
>
>
> SFQ (as a direct replacement of dev root qdisc) is fine if most of your trafic
> is of same kind/priority.
>
>
>
Yes, I suppose I should have been more specific, replacing pfifo_fast
when I am using something else to prioritize and shape my traffic like
HFSC. Hmm . . . although I still wonder about iSCSI SANs . . . Thanks
- John
^ permalink raw reply
* Re: [RFC] SFQ planned changes
From: Eric Dumazet @ 2012-01-03 12:50 UTC (permalink / raw)
To: Dave Taht; +Cc: Michal Kubeček, netdev, John A. Sullivan III
In-Reply-To: <CAA93jw5T10PHD+YQQjy_r=LHKVbYeiWerwM0ou7HANGoenFRKw@mail.gmail.com>
Le mardi 03 janvier 2012 à 13:07 +0100, Dave Taht a écrit :
> From a 'doing science' perspective, I'd like it if it remained possible
> to continue using and benchmarking SFQ as it was, and create
> this set of ideas as a new qdisc ('efq'?)
>
> As these changes seem to require changes to userspace tc, anyway,
> and (selfishly) my patching burden is great enough...
>
> Perhaps some additional benefit could be had by losing
> full backward API compatability with sfq, as well?
No, it's completely compatable with prior version.
An old tc command, or lack of new arguments will setup the SFQ qdisc
exactly as before.
I coded the thing and am doing stress tests before submission.
^ permalink raw reply
* Re: tc filter mask for ACK packets off?
From: Dave Taht @ 2012-01-03 13:00 UTC (permalink / raw)
To: John A. Sullivan III; +Cc: Eric Dumazet, Michal Kubeček, netdev
In-Reply-To: <1325594716.7219.39.camel@denise.theartistscloset.com>
On Tue, Jan 3, 2012 at 1:45 PM, John A. Sullivan III
<jsullivan@opensourcedevel.com> wrote:
> On Tue, 2012-01-03 at 13:32 +0100, Eric Dumazet wrote:
>> Le mardi 03 janvier 2012 à 07:18 -0500, John A. Sullivan III a écrit :
>> > On Tue, 2012-01-03 at 10:36 +0100, Dave Taht wrote:
>> > <snip>
>> > > I'd go into more detail, but after what I hope are the final two
>> > > fixes to sfq and qfq land in the net-next kernel (after some more
>> > > testing), I like to think I have a more valid approach than this
>> > > in the works, but that too will require some more development
>> > > and testing.
>> > >
>> > > http://www.teklibre.com/~d/bloat/pfifo_fast_vs_sfq_qfq_linear.png
>> > >
>> > <snip>
>> > Hmmm . . . certainly shattered my concerns about replacing pfifo_fast
>> > with SFQ! Thanks - John
SFQ as presently implemented (and by presently, I mean, as of yesterday,
by tomorrow it could be different at the rate eric is going!) is VERY
suitable for
sub 100Mbit desktops, wireless stations/laptops other devices,
home gateways with sub 100Mbit uplinks, and the like. That's a few
hundred million devices that aren't using it today and defaulting to
pfifo_fast and suffering for it.
QFQ is it's big brother and I have hopes it can scale up to 10GigE,
once suitable techniques are found for managing the sub-queue depth.
The enhancements to SFQ eric proposed in the other thread might get it
to where it outperforms (by a lot) pfifo_fast in it's default configuration
(eg txqueuelen 1000) with few side effects. Scaling further up than that...
... I don't have a good picture of gigE performance at the moment with
any of these advanced qdiscs and have no recomendation.
I do recomend highly that you fiddle with this stuff! I do have to
note that the graph above had GSO/TSO turned off.
>> Before you do, take the time to read the warning in sfq source :
>>
>>
>> ADVANTAGE:
>>
>> - It is very cheap. Both CPU and memory requirements are minimal.
>>
>> DRAWBACKS:
>>
>> - "Stochastic" -> It is not 100% fair.
>> When hash collisions occur, several flows are considered as one.
This is in part the benefit of SFQ vs QFQ in that the maximum queue
depth is well managed.
>> - "Round-robin" -> It introduces larger delays than virtual clock
>> based schemes, and should not be used for isolating interactive
>> traffic from non-interactive. It means, that this scheduler
>> should be used as leaf of CBQ or P3, which put interactive traffic
>> to higher priority band.
These delays are NOTHING compared to what pfifo_fast can induce.
Very little traffic nowadays is marked as interactive to any statistically
significant extent, so any FQ method effectively makes more traffic
interactive than prioritization can.
>> SFQ (as a direct replacement of dev root qdisc) is fine if most of your trafic
>> is of same kind/priority.
Which is the case for most desktops, laptops, gws, wireless, etc.
> Yes, I suppose I should have been more specific, replacing pfifo_fast
> when I am using something else to prioritize and shape my traffic like
> HFSC.
I enjoyed getting your HFSC experience secondhand. It would be
very interesting getting your feedback on trying this stuff.
More data is needed to beat the bloat.
> Hmm . . . although I still wonder about iSCSI SANs . . . Thanks
I wonder too. Most of the people running iSCSI seem to have an
aversion to packet loss, yet are running over TCP. I *think*
FQ methods will improve latency dramatically for iSCSI
when iSCSI has multiple initiators....
> - John
>
--
Dave Täht
SKYPE: davetaht
US Tel: 1-239-829-5608
FR Tel: 0638645374
http://www.bufferbloat.net
^ permalink raw reply
* [PATCH] netfilter: Fix br_nf_pre_routing() in conjunction with bridge-nf-call-ip(6)tables=0
From: Richard Weinberger @ 2012-01-03 13:26 UTC (permalink / raw)
To: shemminger
Cc: davem, bridge, netdev, linux-kernel, netfilter-devel,
Richard Weinberger
In-Reply-To: <1325597164-13459-1-git-send-email-richard@nod.at>
If net.bridge.bridge-nf-call-iptables or net.bridge.bridge-nf-call-ip6tables
are set to zero xt_physdev has no effect because skb->nf_bridge has not been set up.
Signed-off-by: Richard Weinberger <richard@nod.at>
---
net/bridge/br_netfilter.c | 31 +++++++++++++++++++++++--------
1 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index fa8b8f7..f38a8e4 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -576,10 +576,12 @@ static unsigned int br_nf_pre_routing_ipv6(unsigned int hook,
struct sk_buff *skb,
const struct net_device *in,
const struct net_device *out,
- int (*okfn)(struct sk_buff *))
+ int (*okfn)(struct sk_buff *),
+ struct net_bridge *br)
{
const struct ipv6hdr *hdr;
u32 pkt_len;
+ struct nf_bridge_info *nf_bridge;
if (skb->len < sizeof(struct ipv6hdr))
return NF_DROP;
@@ -606,6 +608,15 @@ static unsigned int br_nf_pre_routing_ipv6(unsigned int hook,
nf_bridge_put(skb->nf_bridge);
if (!nf_bridge_alloc(skb))
return NF_DROP;
+
+ if (!brnf_call_ip6tables && !br->nf_call_ip6tables) {
+ nf_bridge = skb->nf_bridge;
+ nf_bridge->mask |= BRNF_NF_BRIDGE_PREROUTING;
+ nf_bridge->physindev = skb->dev;
+
+ return NF_ACCEPT;
+ }
+
if (!setup_pre_routing(skb))
return NF_DROP;
@@ -629,6 +640,7 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff *skb,
{
struct net_bridge_port *p;
struct net_bridge *br;
+ struct nf_bridge_info *nf_bridge;
__u32 len = nf_bridge_encap_header_len(skb);
if (unlikely(!pskb_may_pull(skb, len)))
@@ -641,16 +653,10 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff *skb,
if (skb->protocol == htons(ETH_P_IPV6) || IS_VLAN_IPV6(skb) ||
IS_PPPOE_IPV6(skb)) {
- if (!brnf_call_ip6tables && !br->nf_call_ip6tables)
- return NF_ACCEPT;
-
nf_bridge_pull_encap_header_rcsum(skb);
- return br_nf_pre_routing_ipv6(hook, skb, in, out, okfn);
+ return br_nf_pre_routing_ipv6(hook, skb, in, out, okfn, br);
}
- if (!brnf_call_iptables && !br->nf_call_iptables)
- return NF_ACCEPT;
-
if (skb->protocol != htons(ETH_P_IP) && !IS_VLAN_IP(skb) &&
!IS_PPPOE_IP(skb))
return NF_ACCEPT;
@@ -663,6 +669,15 @@ static unsigned int br_nf_pre_routing(unsigned int hook, struct sk_buff *skb,
nf_bridge_put(skb->nf_bridge);
if (!nf_bridge_alloc(skb))
return NF_DROP;
+
+ if (!brnf_call_iptables && !br->nf_call_iptables) {
+ nf_bridge = skb->nf_bridge;
+ nf_bridge->mask |= BRNF_NF_BRIDGE_PREROUTING;
+ nf_bridge->physindev = skb->dev;
+
+ return NF_ACCEPT;
+ }
+
if (!setup_pre_routing(skb))
return NF_DROP;
store_orig_dstaddr(skb);
--
1.7.7.3
^ permalink raw reply related
* Re: xt_physdev has no effect if net.bridge.bridge-nf-call-iptables=0
From: Richard Weinberger @ 2012-01-03 13:26 UTC (permalink / raw)
To: shemminger; +Cc: davem, bridge, netdev, linux-kernel, netfilter-devel
In-Reply-To: <4F025A07.2000304@nod.at>
Hi!
Here is a fix for the problem I've reported yesterday.
http://marc.info/?l=netfilter-devel&m=132555432331663&w=2
Please review the patch carefully, I'm not a br_netfilter ninja. 8-)
Thanks,
//richard
^ permalink raw reply
* RE: [PATCH 06/19] netfilter: nf_conntrack: use atomic64 for accounting counters
From: Eric Dumazet @ 2012-01-03 13:31 UTC (permalink / raw)
To: David Laight; +Cc: pablo, netfilter-devel, davem, netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6D8AF28@saturn3.aculab.com>
Le mardi 03 janvier 2012 à 12:01 +0000, David Laight a écrit :
> > if (acct) {
> > - spin_lock_bh(&ct->lock);
> > - acct[CTINFO2DIR(ctinfo)].packets++;
> > - acct[CTINFO2DIR(ctinfo)].bytes += skb->len;
> > - spin_unlock_bh(&ct->lock);
> > + atomic64_inc(&acct[CTINFO2DIR(ctinfo)].packets);
> > + atomic64_add(skb->len,
> &acct[CTINFO2DIR(ctinfo)].bytes);
> > }
>
> On a 32bit arch the two atomic64 operations require a locked
> bus cycle each. The spin_unlock_bh() may not need one - so
> the code may now be slower (modulo lock contention etc).
>
> Probably worth caching &acct[CTINFO2DIR(ctinfo)] in a local,
> the compiler probably can't do it itself.
You're mistaken.
Compile a UP kernel and check yourself before doing such claims.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH 06/19] netfilter: nf_conntrack: use atomic64 for accounting counters
From: Eric Dumazet @ 2012-01-03 13:37 UTC (permalink / raw)
To: David Laight; +Cc: pablo, netfilter-devel, davem, netdev
In-Reply-To: <1325597490.2320.45.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Le mardi 03 janvier 2012 à 14:31 +0100, Eric Dumazet a écrit :
> Le mardi 03 janvier 2012 à 12:01 +0000, David Laight a écrit :
> > > if (acct) {
> > > - spin_lock_bh(&ct->lock);
> > > - acct[CTINFO2DIR(ctinfo)].packets++;
> > > - acct[CTINFO2DIR(ctinfo)].bytes += skb->len;
> > > - spin_unlock_bh(&ct->lock);
> > > + atomic64_inc(&acct[CTINFO2DIR(ctinfo)].packets);
> > > + atomic64_add(skb->len,
> > &acct[CTINFO2DIR(ctinfo)].bytes);
> > > }
> >
> > On a 32bit arch the two atomic64 operations require a locked
> > bus cycle each. The spin_unlock_bh() may not need one - so
> > the code may now be slower (modulo lock contention etc).
> >
> > Probably worth caching &acct[CTINFO2DIR(ctinfo)] in a local,
> > the compiler probably can't do it itself.
>
> You're mistaken.
>
> Compile a UP kernel and check yourself before doing such claims.
>
>
Oops sorry, I misread your mail, I thought you were speaking of UP
kernel.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] net: fsl: fec: fix build for mx23-only kernel
From: Wolfram Sang @ 2012-01-03 13:46 UTC (permalink / raw)
To: netdev
Cc: linux-arm-kernel, Wolfram Sang, Fabio Estevam,
Uwe Kleine-König, Shawn Guo, David S. Miller
If one only selects mx23-based boards, compile fails:
drivers/net/ethernet/freescale/fec.c:410:2: error: 'FEC_HASH_TABLE_HIGH' undeclared (first use in this function)
drivers/net/ethernet/freescale/fec.c:411:2: error: 'FEC_HASH_TABLE_LOW' undeclared (first use in this function)
This is because fec.h uses CONFIG_SOC_IMX28 to determine the register
layout of the core which makes sense since the MX23 does not have a fec.
However, Kconfig uses the broader ARCH_MXS symbol and this way even
makes the fec-driver default for MX23. Adapt Kconfig to use the more
precise SOC_IMX28 as well.
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/freescale/Kconfig | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index 5272f9d..9de3764 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -23,8 +23,8 @@ if NET_VENDOR_FREESCALE
config FEC
bool "FEC ethernet controller (of ColdFire and some i.MX CPUs)"
depends on (M523x || M527x || M5272 || M528x || M520x || M532x || \
- ARCH_MXC || ARCH_MXS)
- default ARCH_MXC || ARCH_MXS if ARM
+ ARCH_MXC || SOC_IMX28)
+ default ARCH_MXC || SOC_IMX28 if ARM
select PHYLIB
---help---
Say Y here if you want to use the built-in 10/100 Fast ethernet
--
1.7.7.3
^ permalink raw reply related
* Re: [PATCH] x86: fix and improve cmpxchg_double{,_local}()
From: Eric Dumazet @ 2012-01-03 15:00 UTC (permalink / raw)
To: Jan Beulich; +Cc: mingo, tglx, hpa, Christoph Lameter, linux-kernel, netdev
In-Reply-To: <4F01F12A020000780006A19B@nat28.tlf.novell.com>
Le lundi 02 janvier 2012 à 17:02 +0000, Jan Beulich a écrit :
> Just like the per-CPU ones they had several problems/shortcomings:
>
> Only the first memory operand was mentioned in the asm() operands, and
> the 2x64-bit version didn't have a memory clobber while the 2x32-bit
> one did. The former allowed the compiler to not recognize the need to
> re-load the data in case it had it cached in some register, while the
> latter was overly destructive.
>
> The types of the local copies of the old and new values were incorrect
> (the types of the pointed-to variables should be used here, to make
> sure the respective old/new variable types are compatible).
>
> The __dummy/__junk variables were pointless, given that local copies
> of the inputs already existed (and can hence be used for discarded
> outputs).
>
> The 32-bit variant of cmpxchg_double_local() referenced
> cmpxchg16b_local().
>
> At once also
> - change the return value type to what it really is: 'bool'
> - unify 32- and 64-bit variants
> - abstract out the common part of the 'normal' and 'local' variants
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
While looking at your patch, I discovered that atomic64_add() /
atomic64_inc() on 32bit are completely buggy. Oh well...
Generated code :
c03bc00c <atomic64_add_return_cx8>:
c03bc00c: 55 push %ebp
c03bc00d: 53 push %ebx
c03bc00e: 56 push %esi
c03bc00f: 57 push %edi
c03bc010: 89 c6 mov %eax,%esi
c03bc012: 89 d7 mov %edx,%edi
c03bc014: 89 cd mov %ecx,%ebp
c03bc016: 89 d8 mov %ebx,%eax
c03bc018: 89 ca mov %ecx,%edx
c03bc01a: f0 0f c7 4d 00 lock cmpxchg8b 0x0(%ebp)
c03bc01f: 89 c3 mov %eax,%ebx
c03bc021: 89 d1 mov %edx,%ecx
c03bc023: 01 f3 add %esi,%ebx
c03bc025: 11 f9 adc %edi,%ecx
c03bc027: f0 0f c7 4d 00 lock cmpxchg8b 0x0(%ebp)
c03bc02c: 75 f9 jne c03bc027 <atomic64_add_return_cx8+0x1b>
c03bc02e: 89 d8 mov %ebx,%eax
c03bc030: 89 ca mov %ecx,%edx
c03bc032: 5f pop %edi
c03bc033: 5e pop %esi
c03bc034: 5b pop %ebx
c03bc035: 5d pop %ebp
c03bc036: c3 ret
The ' jne c03bc027' should really be 'jne c03bc01f'
No idea how old is this bug.
^ permalink raw reply
* Re: [PATCH] x86: fix and improve cmpxchg_double{,_local}()
From: Eric Dumazet @ 2012-01-03 15:15 UTC (permalink / raw)
To: Jan Beulich; +Cc: mingo, tglx, hpa, Christoph Lameter, linux-kernel, netdev
In-Reply-To: <1325602830.2320.69.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Le mardi 03 janvier 2012 à 16:00 +0100, Eric Dumazet a écrit :
> While looking at your patch, I discovered that atomic64_add() /
> atomic64_inc() on 32bit are completely buggy. Oh well...
>
> Generated code :
>
> c03bc00c <atomic64_add_return_cx8>:
> c03bc00c: 55 push %ebp
> c03bc00d: 53 push %ebx
> c03bc00e: 56 push %esi
> c03bc00f: 57 push %edi
> c03bc010: 89 c6 mov %eax,%esi
> c03bc012: 89 d7 mov %edx,%edi
> c03bc014: 89 cd mov %ecx,%ebp
> c03bc016: 89 d8 mov %ebx,%eax
> c03bc018: 89 ca mov %ecx,%edx
> c03bc01a: f0 0f c7 4d 00 lock cmpxchg8b 0x0(%ebp)
> c03bc01f: 89 c3 mov %eax,%ebx
> c03bc021: 89 d1 mov %edx,%ecx
> c03bc023: 01 f3 add %esi,%ebx
> c03bc025: 11 f9 adc %edi,%ecx
> c03bc027: f0 0f c7 4d 00 lock cmpxchg8b 0x0(%ebp)
> c03bc02c: 75 f9 jne c03bc027 <atomic64_add_return_cx8+0x1b>
> c03bc02e: 89 d8 mov %ebx,%eax
> c03bc030: 89 ca mov %ecx,%edx
> c03bc032: 5f pop %edi
> c03bc033: 5e pop %esi
> c03bc034: 5b pop %ebx
> c03bc035: 5d pop %ebp
> c03bc036: c3 ret
>
> The ' jne c03bc027' should really be 'jne c03bc01f'
>
> No idea how old is this bug.
>
Very old it seems...
arch/x86/lib/atomic64_cx8_32.S
all "jxx 1b" are wrong if a LOCK_PREFIX is included after the 1: label
1:
inst1
LOCK_PREFIX
cmpxchg8b (%ebp)
jne 1b / jumps to beginning of LOCK_PREFIX, inst1 is not replayed
^ permalink raw reply
* [PATCH] net/davinci: do not use all descriptors for tx packets
From: Sascha Hauer @ 2012-01-03 15:27 UTC (permalink / raw)
To: netdev; +Cc: Anant Gole, davem, srk, Sascha Hauer
The driver uses a shared pool for both rx and tx descriptors.
During open it queues fixed number of 128 descriptors for receive
packets. For each received packet it tries to queue another
descriptor. If this fails the descriptor is lost for rx.
The driver has no limitation on tx descriptors to use, so it
can happen during a nmap / ping -f attack that the driver
allocates all descriptors for tx and looses all rx descriptors.
The driver stops working then.
To fix this limit the number of tx descriptors used to half of
the descriptors available, the rx path uses the other half.
Tested on a custom board using nmap / ping -f to the board from
two different hosts.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/net/ethernet/ti/davinci_emac.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index 815c797..794ac30 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -115,6 +115,7 @@ static const char emac_version_string[] = "TI DaVinci EMAC Linux v6.1";
#define EMAC_DEF_TX_CH (0) /* Default 0th channel */
#define EMAC_DEF_RX_CH (0) /* Default 0th channel */
#define EMAC_DEF_RX_NUM_DESC (128)
+#define EMAC_DEF_TX_NUM_DESC (128)
#define EMAC_DEF_MAX_TX_CH (1) /* Max TX channels configured */
#define EMAC_DEF_MAX_RX_CH (1) /* Max RX channels configured */
#define EMAC_POLL_WEIGHT (64) /* Default NAPI poll weight */
@@ -336,6 +337,7 @@ struct emac_priv {
u32 mac_hash2;
u32 multicast_hash_cnt[EMAC_NUM_MULTICAST_BITS];
u32 rx_addr_type;
+ atomic_t cur_tx;
const char *phy_id;
struct phy_device *phydev;
spinlock_t lock;
@@ -1044,6 +1046,9 @@ static void emac_tx_handler(void *token, int len, int status)
{
struct sk_buff *skb = token;
struct net_device *ndev = skb->dev;
+ struct emac_priv *priv = netdev_priv(ndev);
+
+ atomic_dec(&priv->cur_tx);
if (unlikely(netif_queue_stopped(ndev)))
netif_start_queue(ndev);
@@ -1092,6 +1097,9 @@ static int emac_dev_xmit(struct sk_buff *skb, struct net_device *ndev)
goto fail_tx;
}
+ if (atomic_inc_return(&priv->cur_tx) >= EMAC_DEF_TX_NUM_DESC)
+ netif_stop_queue(ndev);
+
return NETDEV_TX_OK;
fail_tx:
--
1.7.7.3
^ permalink raw reply related
* Re: [PATCH] x86: fix and improve cmpxchg_double{,_local}()
From: Jan Beulich @ 2012-01-03 15:35 UTC (permalink / raw)
To: Eric Dumazet; +Cc: mingo, tglx, Christoph Lameter, linux-kernel, netdev, hpa
In-Reply-To: <1325602830.2320.69.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
>>> On 03.01.12 at 16:00, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le lundi 02 janvier 2012 à 17:02 +0000, Jan Beulich a écrit :
>> Just like the per-CPU ones they had several problems/shortcomings:
>>
>> Only the first memory operand was mentioned in the asm() operands, and
>> the 2x64-bit version didn't have a memory clobber while the 2x32-bit
>> one did. The former allowed the compiler to not recognize the need to
>> re-load the data in case it had it cached in some register, while the
>> latter was overly destructive.
>>
>> The types of the local copies of the old and new values were incorrect
>> (the types of the pointed-to variables should be used here, to make
>> sure the respective old/new variable types are compatible).
>>
>> The __dummy/__junk variables were pointless, given that local copies
>> of the inputs already existed (and can hence be used for discarded
>> outputs).
>>
>> The 32-bit variant of cmpxchg_double_local() referenced
>> cmpxchg16b_local().
>>
>> At once also
>> - change the return value type to what it really is: 'bool'
>> - unify 32- and 64-bit variants
>> - abstract out the common part of the 'normal' and 'local' variants
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> While looking at your patch, I discovered that atomic64_add() /
> atomic64_inc() on 32bit are completely buggy. Oh well...
>
> Generated code :
>
> c03bc00c <atomic64_add_return_cx8>:
> c03bc00c: 55 push %ebp
> c03bc00d: 53 push %ebx
> c03bc00e: 56 push %esi
> c03bc00f: 57 push %edi
> c03bc010: 89 c6 mov %eax,%esi
> c03bc012: 89 d7 mov %edx,%edi
> c03bc014: 89 cd mov %ecx,%ebp
> c03bc016: 89 d8 mov %ebx,%eax
> c03bc018: 89 ca mov %ecx,%edx
> c03bc01a: f0 0f c7 4d 00 lock cmpxchg8b 0x0(%ebp)
> c03bc01f: 89 c3 mov %eax,%ebx
> c03bc021: 89 d1 mov %edx,%ecx
> c03bc023: 01 f3 add %esi,%ebx
> c03bc025: 11 f9 adc %edi,%ecx
> c03bc027: f0 0f c7 4d 00 lock cmpxchg8b 0x0(%ebp)
> c03bc02c: 75 f9 jne c03bc027
> <atomic64_add_return_cx8+0x1b>
> c03bc02e: 89 d8 mov %ebx,%eax
> c03bc030: 89 ca mov %ecx,%edx
> c03bc032: 5f pop %edi
> c03bc033: 5e pop %esi
> c03bc034: 5b pop %ebx
> c03bc035: 5d pop %ebp
> c03bc036: c3 ret
>
> The ' jne c03bc027' should really be 'jne c03bc01f'
Indeed, and that's the same for all other routines in this file that
incorrectly use 1: together with LOCK_PREFIX between the label and
an intended jump to that label.
> No idea how old is this bug.
The file (and with it the bug) was introduced in 2.6.35.
While looking at this I also noticed this comment in read64: "we need
LOCK_PREFIX since otherwise cmpxchg8b always does the write",
which is saying quite the opposite of the Intel manual: "This instruction
can be used with a LOCK prefix to allow the instruction to be executed
atomically. To simplify the interface to the processor’s bus, the
destination operand receives a write cycle without regard to the result
of the comparison. The destination operand is written back if the
comparison fails; otherwise, the source operand is written into the
destination. (The processor never produces a locked read without
also producing a locked write.)" - I would conclude the LOCK prefix
actually hurts there.
And in atomic64_set_cx8 it's the other way around: The comment
explains why supposedly no LOCK prefix is needed, but that's again
in conflict with above quoted paragraph from the manual.
Jan
^ permalink raw reply
* Re: [PATCH] net: fsl: fec: fix build for mx23-only kernel
From: Fabio Estevam @ 2012-01-03 15:37 UTC (permalink / raw)
To: Wolfram Sang
Cc: netdev, Fabio Estevam, Uwe Kleine-König, Shawn Guo,
David S. Miller, linux-arm-kernel
In-Reply-To: <1325598407-14353-1-git-send-email-w.sang@pengutronix.de>
On Tue, Jan 3, 2012 at 11:46 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> If one only selects mx23-based boards, compile fails:
>
> drivers/net/ethernet/freescale/fec.c:410:2: error: 'FEC_HASH_TABLE_HIGH' undeclared (first use in this function)
> drivers/net/ethernet/freescale/fec.c:411:2: error: 'FEC_HASH_TABLE_LOW' undeclared (first use in this function)
>
> This is because fec.h uses CONFIG_SOC_IMX28 to determine the register
> layout of the core which makes sense since the MX23 does not have a fec.
> However, Kconfig uses the broader ARCH_MXS symbol and this way even
> makes the fec-driver default for MX23. Adapt Kconfig to use the more
> precise SOC_IMX28 as well.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: David S. Miller <davem@davemloft.net>
Acked-by: Fabio Estevam <fabio.estevam@freescale.com>
^ permalink raw reply
* Re: [PATCH] x86: fix and improve cmpxchg_double{,_local}()
From: Eric Dumazet @ 2012-01-03 15:41 UTC (permalink / raw)
To: Jan Beulich; +Cc: mingo, tglx, hpa, Christoph Lameter, linux-kernel, netdev
In-Reply-To: <1325603714.2320.73.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Le mardi 03 janvier 2012 à 16:15 +0100, Eric Dumazet a écrit :
> Very old it seems...
>
> arch/x86/lib/atomic64_cx8_32.S
>
> all "jxx 1b" are wrong if a LOCK_PREFIX is included after the 1: label
>
> 1:
> inst1
> LOCK_PREFIX
> cmpxchg8b (%ebp)
> jne 1b / jumps to beginning of LOCK_PREFIX, inst1 is not replayed
>
>
>
>
A possible fix would be to not use "1" label in LOCK_PREFIX macro,
but 672 magic value.
Not sure if we can use a local label in a macro ?
arch/x86/include/asm/alternative-asm.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/alternative-asm.h b/arch/x86/include/asm/alternative-asm.h
index 091508b..952bd01 100644
--- a/arch/x86/include/asm/alternative-asm.h
+++ b/arch/x86/include/asm/alternative-asm.h
@@ -4,10 +4,10 @@
#ifdef CONFIG_SMP
.macro LOCK_PREFIX
-1: lock
+672: lock
.section .smp_locks,"a"
.balign 4
- .long 1b - .
+ .long 672b - .
.previous
.endm
#else
^ permalink raw reply related
* Re: [PATCH v3] sctp: fix incorrect overflow check on autoclose
From: Vladislav Yasevich @ 2012-01-03 15:52 UTC (permalink / raw)
To: Xi Wang
Cc: netdev, linux-sctp, Andrew Morton, Andrei Pelinescu-Onciul,
David S. Miller
In-Reply-To: <4EEBC9BF.7020506@gmail.com>
On 12/16/2011 05:44 PM, Xi Wang wrote:
> Commit 8ffd3208 voids the previous patches f6778aab and 810c0719 for
> limiting the autoclose value. If userspace passes in -1 on 32-bit
> platform, the overflow check didn't work and autoclose would be set
> to 0xffffffff.
>
> This patch defines a max_autoclose (in seconds) for limiting the value
> and exposes it through sysctl, with the following intentions.
>
> 1) Avoid overflowing autoclose * HZ.
>
> 2) Keep the default autoclose bound consistent across 32- and 64-bit
> platforms (INT_MAX / HZ in this patch).
>
> 3) Keep the autoclose value consistent between setsockopt() and
> getsockopt() calls.
>
> Suggested-by: Vlad Yasevich <vladislav.yasevich@hp.com>
> Signed-off-by: Xi Wang <xi.wang@gmail.com>
Looks good to me.
Acked-by: Vlad Yasevich <vladislav.yasevich@hp.com>
-vlad
P.S. Sorry, took so long. Just got back from a long vacation.
> ---
> include/net/sctp/structs.h | 4 ++++
> net/sctp/associola.c | 2 +-
> net/sctp/protocol.c | 3 +++
> net/sctp/socket.c | 2 --
> net/sctp/sysctl.c | 13 +++++++++++++
> 5 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index e90e7a9..a15432da 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -241,6 +241,9 @@ extern struct sctp_globals {
> * bits is an indicator of when to send and window update SACK.
> */
> int rwnd_update_shift;
> +
> + /* Threshold for autoclose timeout, in seconds. */
> + unsigned long max_autoclose;
> } sctp_globals;
>
> #define sctp_rto_initial (sctp_globals.rto_initial)
> @@ -281,6 +284,7 @@ extern struct sctp_globals {
> #define sctp_auth_enable (sctp_globals.auth_enable)
> #define sctp_checksum_disable (sctp_globals.checksum_disable)
> #define sctp_rwnd_upd_shift (sctp_globals.rwnd_update_shift)
> +#define sctp_max_autoclose (sctp_globals.max_autoclose)
>
> /* SCTP Socket type: UDP or TCP style. */
> typedef enum {
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 152b5b3..acd2edb 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -173,7 +173,7 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
> asoc->timeouts[SCTP_EVENT_TIMEOUT_HEARTBEAT] = 0;
> asoc->timeouts[SCTP_EVENT_TIMEOUT_SACK] = asoc->sackdelay;
> asoc->timeouts[SCTP_EVENT_TIMEOUT_AUTOCLOSE] =
> - (unsigned long)sp->autoclose * HZ;
> + min_t(unsigned long, sp->autoclose, sctp_max_autoclose) * HZ;
>
> /* Initializes the timers */
> for (i = SCTP_EVENT_TIMEOUT_NONE; i < SCTP_NUM_TIMEOUT_TYPES; ++i)
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 61b9fca..6f6ad86 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -1285,6 +1285,9 @@ SCTP_STATIC __init int sctp_init(void)
> sctp_max_instreams = SCTP_DEFAULT_INSTREAMS;
> sctp_max_outstreams = SCTP_DEFAULT_OUTSTREAMS;
>
> + /* Initialize maximum autoclose timeout. */
> + sctp_max_autoclose = INT_MAX / HZ;
> +
> /* Initialize handle used for association ids. */
> idr_init(&sctp_assocs_id);
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index 13bf5fc..54a7cd2 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -2200,8 +2200,6 @@ static int sctp_setsockopt_autoclose(struct sock *sk, char __user *optval,
> return -EINVAL;
> if (copy_from_user(&sp->autoclose, optval, optlen))
> return -EFAULT;
> - /* make sure it won't exceed MAX_SCHEDULE_TIMEOUT */
> - sp->autoclose = min_t(long, sp->autoclose, MAX_SCHEDULE_TIMEOUT / HZ);
>
> return 0;
> }
> diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c
> index 6b39529..60ffbd0 100644
> --- a/net/sctp/sysctl.c
> +++ b/net/sctp/sysctl.c
> @@ -53,6 +53,10 @@ static int sack_timer_min = 1;
> static int sack_timer_max = 500;
> static int addr_scope_max = 3; /* check sctp_scope_policy_t in include/net/sctp/constants.h for max entries */
> static int rwnd_scale_max = 16;
> +static unsigned long max_autoclose_min = 0;
> +static unsigned long max_autoclose_max =
> + (MAX_SCHEDULE_TIMEOUT / HZ > UINT_MAX)
> + ? UINT_MAX : MAX_SCHEDULE_TIMEOUT / HZ;
>
> extern long sysctl_sctp_mem[3];
> extern int sysctl_sctp_rmem[3];
> @@ -258,6 +262,15 @@ static ctl_table sctp_table[] = {
> .extra1 = &one,
> .extra2 = &rwnd_scale_max,
> },
> + {
> + .procname = "max_autoclose",
> + .data = &sctp_max_autoclose,
> + .maxlen = sizeof(unsigned long),
> + .mode = 0644,
> + .proc_handler = &proc_doulongvec_minmax,
> + .extra1 = &max_autoclose_min,
> + .extra2 = &max_autoclose_max,
> + },
>
> { /* sentinel */ }
> };
^ permalink raw reply
* Re: [RFC] SFQ planned changes
From: Eric Dumazet @ 2012-01-03 16:08 UTC (permalink / raw)
To: Dave Taht; +Cc: Michal Kubeček, netdev, John A. Sullivan III
In-Reply-To: <1325595036.2320.43.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Here is the code I ran on my test server with 200 netperf TCP_STREAM
flows with pretty good results (each flow gets 0.5 % of bandwidth)
$TC qdisc add dev $DEV root handle 1: est 1sec 8sec htb default 1
$TC class add dev $DEV parent 1: classid 1:1 est 1sec 8sec htb \
rate 200Mbit mtu 40000 quantum 80000
$TC qdisc add dev $DEV parent 1:1 handle 10: est 1sec 8sec sfq \
limit 2000 depth 10 headdrop flows 1000 divisor 16384
# tcnew -s -d qdisc show dev eth3
qdisc htb 1: root refcnt 18 r2q 10 default 1 direct_packets_stat 0 ver 3.17
Sent 4512949730 bytes 3030391 pkt (dropped 44409, overlimits 6105100 requeues 1)
rate 198288Kbit 16629pps backlog 0b 1732p requeues 1
qdisc sfq 10: parent 1:1 limit 2000p quantum 1514b depth 10 headdrop flows 1000/16384 divisor 16384
Sent 4512949730 bytes 3030391 pkt (dropped 44409, overlimits 0 requeues 0)
rate 198288Kbit 16629pps backlog 2622248b 1732p requeues 0
patch on top of current net-next
include/linux/pkt_sched.h | 7 +
net/sched/sch_sfq.c | 144 ++++++++++++++++++++++++------------
2 files changed, 104 insertions(+), 47 deletions(-)
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 8daced3..c2c6cfd 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -162,6 +162,13 @@ struct tc_sfq_qopt {
unsigned flows; /* Maximal number of flows */
};
+struct tc_sfq_ext_qopt {
+ struct tc_sfq_qopt qopt;
+ unsigned int depth; /* max number of packets per flow */
+ unsigned int headdrop;
+};
+
+
struct tc_sfq_xstats {
__s32 allot;
};
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index d329a8a..66682fd 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -67,15 +67,16 @@
IMPLEMENTATION:
This implementation limits maximal queue length to 128;
- max mtu to 2^18-1; max 128 flows, number of hash buckets to 1024.
- The only goal of this restrictions was that all data
- fit into one 4K page on 32bit arches.
+ max mtu to 2^18-1;
+ max 65280 flows,
+ number of hash buckets to 65536.
It is easy to increase these values, but not in flight. */
#define SFQ_DEPTH 128 /* max number of packets per flow */
-#define SFQ_SLOTS 128 /* max number of flows */
-#define SFQ_EMPTY_SLOT 255
+#define SFQ_DEFAULT_FLOWS 128
+#define SFQ_MAX_FLOWS (0x10000 - 256) /* max number of flows */
+#define SFQ_EMPTY_SLOT 0xffff
#define SFQ_DEFAULT_HASH_DIVISOR 1024
/* We use 16 bits to store allot, and want to handle packets up to 64K
@@ -84,13 +85,13 @@
#define SFQ_ALLOT_SHIFT 3
#define SFQ_ALLOT_SIZE(X) DIV_ROUND_UP(X, 1 << SFQ_ALLOT_SHIFT)
-/* This type should contain at least SFQ_DEPTH + SFQ_SLOTS values */
-typedef unsigned char sfq_index;
+/* This type should contain at least SFQ_DEPTH + SFQ_MAX_FLOWS values */
+typedef u16 sfq_index;
/*
* We dont use pointers to save space.
- * Small indexes [0 ... SFQ_SLOTS - 1] are 'pointers' to slots[] array
- * while following values [SFQ_SLOTS ... SFQ_SLOTS + SFQ_DEPTH - 1]
+ * Small indexes [0 ... SFQ_MAX_FLOWS - 1] are 'pointers' to slots[] array
+ * while following values [SFQ_MAX_FLOWS ... SFQ_MAX_FLOWS + SFQ_DEPTH - 1]
* are 'pointers' to dep[] array
*/
struct sfq_head {
@@ -112,8 +113,11 @@ struct sfq_sched_data {
/* Parameters */
int perturb_period;
unsigned int quantum; /* Allotment per round: MUST BE >= MTU */
- int limit;
+ int limit; /* limit of total number of packets in this qdisc */
unsigned int divisor; /* number of slots in hash table */
+ unsigned int maxflows; /* number of flows in flows array */
+ int headdrop;
+ int depth; /* limit depth of each flow */
/* Variables */
struct tcf_proto *filter_list;
struct timer_list perturb_timer;
@@ -122,7 +126,7 @@ struct sfq_sched_data {
unsigned short scaled_quantum; /* SFQ_ALLOT_SIZE(quantum) */
struct sfq_slot *tail; /* current slot in round */
sfq_index *ht; /* Hash table (divisor slots) */
- struct sfq_slot slots[SFQ_SLOTS];
+ struct sfq_slot *slots;
struct sfq_head dep[SFQ_DEPTH]; /* Linked list of slots, indexed by depth */
};
@@ -131,9 +135,9 @@ struct sfq_sched_data {
*/
static inline struct sfq_head *sfq_dep_head(struct sfq_sched_data *q, sfq_index val)
{
- if (val < SFQ_SLOTS)
+ if (val < SFQ_MAX_FLOWS)
return &q->slots[val].dep;
- return &q->dep[val - SFQ_SLOTS];
+ return &q->dep[val - SFQ_MAX_FLOWS];
}
/*
@@ -199,18 +203,19 @@ static unsigned int sfq_classify(struct sk_buff *skb, struct Qdisc *sch,
}
/*
- * x : slot number [0 .. SFQ_SLOTS - 1]
+ * x : slot number [0 .. SFQ_MAX_FLOWS - 1]
*/
static inline void sfq_link(struct sfq_sched_data *q, sfq_index x)
{
sfq_index p, n;
- int qlen = q->slots[x].qlen;
+ struct sfq_slot *slot = &q->slots[x];
+ int qlen = slot->qlen;
- p = qlen + SFQ_SLOTS;
+ p = qlen + SFQ_MAX_FLOWS;
n = q->dep[qlen].next;
- q->slots[x].dep.next = n;
- q->slots[x].dep.prev = p;
+ slot->dep.next = n;
+ slot->dep.prev = p;
q->dep[qlen].next = x; /* sfq_dep_head(q, p)->next = x */
sfq_dep_head(q, n)->prev = x;
@@ -305,7 +310,7 @@ static unsigned int sfq_drop(struct Qdisc *sch)
x = q->dep[d].next;
slot = &q->slots[x];
drop:
- skb = slot_dequeue_tail(slot);
+ skb = q->headdrop ? slot_dequeue_head(slot) : slot_dequeue_tail(slot);
len = qdisc_pkt_len(skb);
sfq_dec(q, x);
kfree_skb(skb);
@@ -349,16 +354,26 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
slot = &q->slots[x];
if (x == SFQ_EMPTY_SLOT) {
x = q->dep[0].next; /* get a free slot */
+ if (x >= SFQ_MAX_FLOWS)
+ return qdisc_drop(skb, sch);
q->ht[hash] = x;
slot = &q->slots[x];
slot->hash = hash;
}
- /* If selected queue has length q->limit, do simple tail drop,
- * i.e. drop _this_ packet.
- */
- if (slot->qlen >= q->limit)
- return qdisc_drop(skb, sch);
+ if (slot->qlen >= q->depth) {
+ struct sk_buff *head;
+
+ if (!q->headdrop)
+ return qdisc_drop(skb, sch);
+ head = slot_dequeue_head(slot);
+ sch->qstats.backlog -= qdisc_pkt_len(head);
+ kfree_skb(head);
+ sch->qstats.drops++;
+ sch->qstats.backlog += qdisc_pkt_len(skb);
+ slot_queue_add(slot, skb);
+ return NET_XMIT_CN;
+ }
sch->qstats.backlog += qdisc_pkt_len(skb);
slot_queue_add(slot, skb);
@@ -366,11 +381,11 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
if (slot->qlen == 1) { /* The flow is new */
if (q->tail == NULL) { /* It is the first flow */
slot->next = x;
+ q->tail = slot;
} else {
slot->next = q->tail->next;
q->tail->next = x;
}
- q->tail = slot;
slot->allot = q->scaled_quantum;
}
if (++sch->q.qlen <= q->limit)
@@ -445,16 +460,17 @@ sfq_reset(struct Qdisc *sch)
* We dont use sfq_dequeue()/sfq_enqueue() because we dont want to change
* counters.
*/
-static void sfq_rehash(struct sfq_sched_data *q)
+static int sfq_rehash(struct sfq_sched_data *q)
{
struct sk_buff *skb;
int i;
struct sfq_slot *slot;
struct sk_buff_head list;
+ int dropped = 0;
__skb_queue_head_init(&list);
- for (i = 0; i < SFQ_SLOTS; i++) {
+ for (i = 0; i < q->maxflows; i++) {
slot = &q->slots[i];
if (!slot->qlen)
continue;
@@ -474,6 +490,11 @@ static void sfq_rehash(struct sfq_sched_data *q)
slot = &q->slots[x];
if (x == SFQ_EMPTY_SLOT) {
x = q->dep[0].next; /* get a free slot */
+ if (x >= SFQ_MAX_FLOWS) {
+ kfree_skb(skb);
+ dropped++;
+ continue;
+ }
q->ht[hash] = x;
slot = &q->slots[x];
slot->hash = hash;
@@ -491,6 +512,7 @@ static void sfq_rehash(struct sfq_sched_data *q)
slot->allot = q->scaled_quantum;
}
}
+ return dropped;
}
static void sfq_perturbation(unsigned long arg)
@@ -502,7 +524,7 @@ static void sfq_perturbation(unsigned long arg)
spin_lock(root_lock);
q->perturbation = net_random();
if (!q->filter_list && q->tail)
- sfq_rehash(q);
+ qdisc_tree_decrease_qlen(sch, sfq_rehash(q));
spin_unlock(root_lock);
if (q->perturb_period)
@@ -513,11 +535,13 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt)
{
struct sfq_sched_data *q = qdisc_priv(sch);
struct tc_sfq_qopt *ctl = nla_data(opt);
+ struct tc_sfq_ext_qopt *ctl_ext = NULL;
unsigned int qlen;
if (opt->nla_len < nla_attr_size(sizeof(*ctl)))
return -EINVAL;
-
+ if (opt->nla_len >= nla_attr_size(sizeof(*ctl_ext)))
+ ctl_ext = nla_data(opt);
if (ctl->divisor &&
(!is_power_of_2(ctl->divisor) || ctl->divisor > 65536))
return -EINVAL;
@@ -526,10 +550,18 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt)
q->quantum = ctl->quantum ? : psched_mtu(qdisc_dev(sch));
q->scaled_quantum = SFQ_ALLOT_SIZE(q->quantum);
q->perturb_period = ctl->perturb_period * HZ;
- if (ctl->limit)
- q->limit = min_t(u32, ctl->limit, SFQ_DEPTH - 1);
+ if (ctl->flows)
+ q->maxflows = min_t(u32, ctl->flows, SFQ_MAX_FLOWS);
if (ctl->divisor)
q->divisor = ctl->divisor;
+ if (ctl_ext) {
+ if (ctl_ext->depth)
+ q->depth = min_t(u32, ctl_ext->depth, SFQ_DEPTH - 1);
+ q->headdrop = ctl_ext->headdrop;
+ }
+ if (ctl->limit)
+ q->limit = min_t(u32, ctl->limit, q->depth * q->maxflows);
+
qlen = sch->q.qlen;
while (sch->q.qlen > q->limit)
sfq_drop(sch);
@@ -544,6 +576,16 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt)
return 0;
}
+static void sfq_free(void *addr)
+{
+ if (addr) {
+ if (is_vmalloc_addr(addr))
+ vfree(addr);
+ else
+ kfree(addr);
+ }
+}
+
static int sfq_init(struct Qdisc *sch, struct nlattr *opt)
{
struct sfq_sched_data *q = qdisc_priv(sch);
@@ -555,14 +597,16 @@ static int sfq_init(struct Qdisc *sch, struct nlattr *opt)
init_timer_deferrable(&q->perturb_timer);
for (i = 0; i < SFQ_DEPTH; i++) {
- q->dep[i].next = i + SFQ_SLOTS;
- q->dep[i].prev = i + SFQ_SLOTS;
+ q->dep[i].next = i + SFQ_MAX_FLOWS;
+ q->dep[i].prev = i + SFQ_MAX_FLOWS;
}
q->limit = SFQ_DEPTH - 1;
+ q->depth = SFQ_DEPTH - 1;
q->cur_depth = 0;
q->tail = NULL;
q->divisor = SFQ_DEFAULT_HASH_DIVISOR;
+ q->maxflows = SFQ_DEFAULT_FLOWS;
if (opt == NULL) {
q->quantum = psched_mtu(qdisc_dev(sch));
q->scaled_quantum = SFQ_ALLOT_SIZE(q->quantum);
@@ -575,15 +619,22 @@ static int sfq_init(struct Qdisc *sch, struct nlattr *opt)
}
sz = sizeof(q->ht[0]) * q->divisor;
- q->ht = kmalloc(sz, GFP_KERNEL);
+ q->ht = kmalloc(sz, GFP_KERNEL | __GFP_NOWARN);
if (!q->ht && sz > PAGE_SIZE)
q->ht = vmalloc(sz);
if (!q->ht)
return -ENOMEM;
+
+ q->slots = kzalloc(sizeof(q->slots[0]) * q->maxflows, GFP_KERNEL | __GFP_NOWARN);
+ if (!q->slots)
+ q->slots = vzalloc(sizeof(q->slots[0]) * q->maxflows);
+ if (!q->slots) {
+ sfq_free(q->ht);
+ return -ENOMEM;
+ }
for (i = 0; i < q->divisor; i++)
q->ht[i] = SFQ_EMPTY_SLOT;
-
- for (i = 0; i < SFQ_SLOTS; i++) {
+ for (i = 0; i < q->maxflows; i++) {
slot_queue_init(&q->slots[i]);
sfq_link(q, i);
}
@@ -601,25 +652,24 @@ static void sfq_destroy(struct Qdisc *sch)
tcf_destroy_chain(&q->filter_list);
q->perturb_period = 0;
del_timer_sync(&q->perturb_timer);
- if (is_vmalloc_addr(q->ht))
- vfree(q->ht);
- else
- kfree(q->ht);
+ sfq_free(q->ht);
+ sfq_free(q->slots);
}
static int sfq_dump(struct Qdisc *sch, struct sk_buff *skb)
{
struct sfq_sched_data *q = qdisc_priv(sch);
unsigned char *b = skb_tail_pointer(skb);
- struct tc_sfq_qopt opt;
-
- opt.quantum = q->quantum;
- opt.perturb_period = q->perturb_period / HZ;
+ struct tc_sfq_ext_qopt opt;
- opt.limit = q->limit;
- opt.divisor = q->divisor;
- opt.flows = q->limit;
+ opt.qopt.quantum = q->quantum;
+ opt.qopt.perturb_period = q->perturb_period / HZ;
+ opt.qopt.limit = q->limit;
+ opt.qopt.divisor = q->divisor;
+ opt.qopt.flows = q->maxflows;
+ opt.depth = q->depth;
+ opt.headdrop = q->headdrop;
NLA_PUT(skb, TCA_OPTIONS, sizeof(opt), &opt);
return skb->len;
^ permalink raw reply related
* Re: [PATCH] x86: fix and improve cmpxchg_double{,_local}()
From: Jan Beulich @ 2012-01-03 16:08 UTC (permalink / raw)
To: Eric Dumazet; +Cc: mingo, tglx, Christoph Lameter, linux-kernel, netdev, hpa
In-Reply-To: <1325605290.2320.76.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
>>> On 03.01.12 at 16:41, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 03 janvier 2012 à 16:15 +0100, Eric Dumazet a écrit :
>
>> Very old it seems...
>>
>> arch/x86/lib/atomic64_cx8_32.S
>>
>> all "jxx 1b" are wrong if a LOCK_PREFIX is included after the 1: label
>>
>> 1:
>> inst1
>> LOCK_PREFIX
>> cmpxchg8b (%ebp)
>> jne 1b / jumps to beginning of LOCK_PREFIX, inst1 is not replayed
>>
>>
>>
>>
>
> A possible fix would be to not use "1" label in LOCK_PREFIX macro,
> but 672 magic value.
>
> Not sure if we can use a local label in a macro ?
"1" and "672" are both local labels, so both are okay. As long as there's
no other (colliding) use of 672 anywhere, that would seem to be the
preferred fix (feel free to put my ack on the patch when you formally
submit it).
Jan
> arch/x86/include/asm/alternative-asm.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/alternative-asm.h
> b/arch/x86/include/asm/alternative-asm.h
> index 091508b..952bd01 100644
> --- a/arch/x86/include/asm/alternative-asm.h
> +++ b/arch/x86/include/asm/alternative-asm.h
> @@ -4,10 +4,10 @@
>
> #ifdef CONFIG_SMP
> .macro LOCK_PREFIX
> -1: lock
> +672: lock
> .section .smp_locks,"a"
> .balign 4
> - .long 1b - .
> + .long 672b - .
> .previous
> .endm
> #else
^ permalink raw reply
* Re: [PATCH] x86: fix and improve cmpxchg_double{,_local}()
From: Eric Dumazet @ 2012-01-03 16:13 UTC (permalink / raw)
To: Jan Beulich; +Cc: mingo, tglx, Christoph Lameter, linux-kernel, netdev, hpa
In-Reply-To: <4F03361C020000780006A3A5@nat28.tlf.novell.com>
Le mardi 03 janvier 2012 à 16:08 +0000, Jan Beulich a écrit :
> "1" and "672" are both local labels, so both are okay. As long as there's
> no other (colliding) use of 672 anywhere, that would seem to be the
> preferred fix (feel free to put my ack on the patch when you formally
> submit it).
I was referring the use of a label local to the macro itself, with
restricted scope.
following psudi code would trigger an asm error :
.macro FOO
.local_label 1
1: lock
.section .smp_locks,"a"
.balign 4
.long 1b - .
.previous
.endm
FOO
jne 1b
^ permalink raw reply
* Re: [PATCH] netfilter: Fix br_nf_pre_routing() in conjunction with bridge-nf-call-ip(6)tables=0
From: Stephen Hemminger @ 2012-01-03 16:15 UTC (permalink / raw)
To: Richard Weinberger; +Cc: davem, bridge, netdev, linux-kernel, netfilter-devel
In-Reply-To: <1325597164-13459-2-git-send-email-richard@nod.at>
On Tue, 3 Jan 2012 14:26:04 +0100
Richard Weinberger <richard@nod.at> wrote:
> If net.bridge.bridge-nf-call-iptables or net.bridge.bridge-nf-call-ip6tables
> are set to zero xt_physdev has no effect because skb->nf_bridge has not been set up.
>
> Signed-off-by: Richard Weinberger <richard@nod.at>
I am not sure if this is a valid configuration. The setting of sysctl is saying
"don't do iptables on bridge (since I won't be using it)" and then you are later
doing iptables and expecting the settings as if the iptables setup was being
done.
Instead, you should just enable the net.bridge.bridge-nf-call-iptables sysctl.
If a distro chooses to disable it then you may have to do it explicitly.
^ permalink raw reply
* Re: [PATCH] sch_qfq: fix overflow in qfq_update_start()
From: Stephen Hemminger @ 2012-01-03 16:25 UTC (permalink / raw)
To: Eric Dumazet, Luigi Rizzo; +Cc: David Miller, netdev, Dave Taht
In-Reply-To: <1325519277.2375.24.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
On Mon, 02 Jan 2012 16:47:57 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> grp->slot_shift is between 22 and 41, so using 32bit wide variables is
> probably a typo.
>
> This could explain QFQ hangs Dave reported to me, after 2^23 packets ?
>
> (23 = 64 - 41)
>
> Reported-by: Dave Taht <dave.taht@gmail.com>
> CC: Stephen Hemminger <shemminger@vyatta.com>
> CC: Dave Taht <dave.taht@gmail.com>
> ---
> net/sched/sch_qfq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
> index 1033434..7b03254 100644
> --- a/net/sched/sch_qfq.c
> +++ b/net/sched/sch_qfq.c
> @@ -817,11 +817,11 @@ skip_unblock:
> static void qfq_update_start(struct qfq_sched *q, struct qfq_class *cl)
> {
> unsigned long mask;
> - uint32_t limit, roundedF;
> + u64 limit, roundedF;
> int slot_shift = cl->grp->slot_shift;
>
> roundedF = qfq_round_down(cl->F, slot_shift);
> - limit = qfq_round_down(q->V, slot_shift) + (1UL << slot_shift);
> + limit = qfq_round_down(q->V, slot_shift) + (1ULL << slot_shift);
>
> if (!qfq_gt(cl->F, q->V) || qfq_gt(roundedF, limit)) {
> /* timestamp was stale */
>
>
You need to copy the BSD developers on these patches since
most of the code came from them and FreeBSD probably still has same bug!
^ 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