* HFSC dangerous behaviour (not a bug)
From: Denys @ 2007-10-29 0:02 UTC (permalink / raw)
To: netdev; +Cc: kaber
Hi All
During testing i found very strange thing.
After applying even example shaper:
http://linux-ip.net/tc/hfsc.en/
-------------
# Example from Figure 1.
tc qdisc add dev eth0 root handle 1: hfsc
tc class add dev eth0 parent 1: classid 1:1 hfsc sc rate
1000kbit ul rate 1000kbit
tc class add dev eth0 parent 1:1 classid 1:10 hfsc sc rate
500kbit ul rate 1000kbit
tc class add dev eth0 parent 1:1 classid 1:20 hfsc sc rate
500kbit ul rate 1000kbit
tc class add dev eth0 parent 1:10 classid 1:11 hfsc sc umax
1500b dmax 53ms rate 400kbit ul rate 1000kbit
tc class add dev eth0 parent 1:10 classid 1:12 hfsc sc umax
1500b dmax 30ms rate 100kbit ul rate 1000kbit
---------------
I had all traffic on eth0 stopped. Tried on br0 - same result. Even ARP
becoming non-functional.
Stats:
tc -s class show dev br0
class hfsc 1: root
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
period 0 level 3
class hfsc 1:11 parent 1:10 sc m1 0bit d 23.0ms m2 400000bit ul m1 0bit d 0us
m2 1000Kbit
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
period 0 level 0
class hfsc 1:1 parent 1: sc m1 0bit d 0us m2 1000Kbit ul m1 0bit d 0us m2
1000Kbit
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
period 0 level 2
class hfsc 1:10 parent 1:1 sc m1 0bit d 0us m2 500000bit ul m1 0bit d 0us m2
1000Kbit
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
period 0 level 1
class hfsc 1:20 parent 1:1 sc m1 0bit d 0us m2 500000bit ul m1 0bit d 0us m2
1000Kbit
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
period 0 level 0
class hfsc 1:12 parent 1:10 sc m1 400000bit d 30.0ms m2 100000bit ul m1 0bit
d 0us m2 1000Kbit
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
period 0 level 0
tc -s qdisc show dev br0
qdisc hfsc 1: root
Sent 0 bytes 0 pkt (dropped 3, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
After specifying correct default class everything worked fine.
In HTB if you dont specify default class, traffic just pass without
"shaping".
Is it possible to keep same behaviour on both disciplines?
Probably just dropping all traffic not good idea, cause if user working on
remote box by forgetting specifying default class or by mistake using
incorrect class number he will loose access to the box, if same interface is
used for tests on shaping and access.
In same time it is good, and can show accurate results on shaping, without
bypassing some "forgotten" traffic.
But at least it must be same, IMHO, on HTB and HFSC.
--
Denys Fedoryshchenko
Technical Manager
Virtual ISP S.A.L.
^ permalink raw reply
* HFSC dangerous behaviour (not a bug)
From: Denys @ 2007-10-29 0:55 UTC (permalink / raw)
To: netdev; +Cc: kaber
Additionally, it doesn't show rate in stats (so it is difficult to measure,
how much is really using each class).
qdisc hfsc 1: root default 200
Sent 1392761062 bytes 965768 pkt (dropped 52, overlimits 1620539 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
qdisc bfifo 200: parent 1:200 limit 5000Kb
Sent 3159118 bytes 17598 pkt (dropped 0, overlimits 0 requeues 177)
rate 0bit 0pps backlog 0b 0p requeues 177
qdisc bfifo 910: parent 1:910 limit 2120000b
Sent 514985563 bytes 360381 pkt (dropped 0, overlimits 0 requeues 314751)
rate 0bit 0pps backlog 0b 0p requeues 314751
qdisc bfifo 1101: parent 1:1101 limit 1000000b
Sent 139858294 bytes 94506 pkt (dropped 0, overlimits 0 requeues 85889)
rate 0bit 0pps backlog 0b 0p requeues 85889
qdisc bfifo 1102: parent 1:1102 limit 1000000b
Sent 96248464 bytes 65436 pkt (dropped 0, overlimits 0 requeues 58030)
rate 0bit 0pps backlog 0b 0p requeues 58030
qdisc bfifo 1103: parent 1:1103 limit 1000000b
Sent 111740612 bytes 75945 pkt (dropped 0, overlimits 0 requeues 66919)
rate 0bit 0pps backlog 0b 0p requeues 66919
qdisc bfifo 923: parent 1:923 limit 375000b
Sent 244333639 bytes 163399 pkt (dropped 0, overlimits 0 requeues 134977)
rate 0bit 0pps backlog 0b 0p requeues 134977
qdisc bfifo 924: parent 1:924 limit 200000b
Sent 101499905 bytes 68041 pkt (dropped 0, overlimits 0 requeues 43722)
rate 0bit 0pps backlog 0b 0p requeues 43722
qdisc bfifo 925: parent 1:925 limit 200000b
Sent 159908141 bytes 106077 pkt (dropped 51, overlimits 0 requeues 34702)
rate 0bit 0pps backlog 0b 0p requeues 34702
qdisc bfifo 926: parent 1:926 limit 200000b
Sent 21027140 bytes 14382 pkt (dropped 0, overlimits 0 requeues 10707)
rate 0bit 0pps backlog 0b 0p requeues 10707
qdisc bfifo 955: parent 1:955 limit 500000b
Sent 186 bytes 3 pkt (dropped 0, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
--
Denys Fedoryshchenko
Technical Manager
Virtual ISP S.A.L.
^ permalink raw reply
* Bonding in active-backup mode with arp monitoring on Xen
From: Tsutomu Fujii @ 2007-10-29 1:16 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 1725 bytes --]
Hi.
I was trying to use bonding from a guest OS on xen in active-backup
mode with arp monitoring, but the virtual server can't communicate to
the peer server via bonding interface.
The network structure is the following.
+-------------------------------+
| virtual I/F(eth0) | <- have IP address.
+-------------------------------+
| virtual bridge(xenbr0) |
+-------------------------------+
| bonding(bond0) | <- don't have IP address
+--------------+-+--------------+
|phy I/F(peth0)| |phy I/F(peth1)|
+--------------+ +--------------+
The attached file describes how I configured the network.
In this case, when I run 'cat /proc/net/bonding/bond0', 'MII Status'
is shown as down.
This behavior only occurs in active-backup mode with arp monitoring.
It does not happen in active-backup mode with mii monitoring nor other
mode that can use arp monitoring.
>From the output of tcpdump, it seems that arp packet isn't transmitted.
This problem is caused because bond_activebackup_arp_mon(), which is the
function that sends arp packet in active-backup mode, checks whether
bonding master interface has an IP address(bond_has_ip()), before arp
packets are transmitted using bond_arp_send_all().
In the above setting, bonding master interface don't have IP address.
How about removing bond_has_ip() from the condition for calling
bond_arp_send_all() to use bonding with xen?
When I removed bond_has_ip(), the resulting kernel worked fine and
active-backup mode works with arp monitoring.
I confirmed this behavior in RHEL5(2.6.18-44.el5xen). And I confirmed
that the upstream kernel(2.6.23) also has the same condition before
calling bond_arp_send_all().
Thanks.
---
Tsutomu Fujii
[-- Attachment #2: set_bonding.txt --]
[-- Type: text/plain, Size: 1444 bytes --]
Setting procedure of bonding.
1) Make or modify the following config file on dom0.
- /etc/sysconfig/network-scripts/ifcfg-bond0
TYPE=Bonding
DEVICE=bond0
BOOTPROTO=none
ONBOOT=no
- /etc/sysconfig/network-scripts/ifcfg-eth1
DEVICE=eth1
BOOTPROTO=none
ONBOOT=no
- /etc/sysconfig/network-scripts/ifcfg-eth2
DEVICE=eth2
BOOTPROTO=none
ONBOOT=no
- /etc/modprobe.conf (Add to following two lines.)
alias bond0 bonding
options bonding mode=1 arp_interval=100 arp_ip_target=192.168.0.254
2) Run the following command on dom0.
# brctl addbr xenbr0
# ip link set xenbr0 arp off
# ip link set xenbr0 multicast off
# ip link set eth0 name peth0
# ip link set eth1 name peth1
# ip link set veth0 name eth0
# ip link set veth1 name eth1
# ifup bond0
# ifenslave bond0 peth0 peth1
# ifdown bond0
# ip link set bond0 down
# ip link set bond0 arp off
# ip link set bond0 multicast off
# ip link set bond0 addr fe:ff:ff:ff:ff:ff
# ip link set vif0.0 down
# ip link set vif0.0 arp off
# ip link set vif0.0 multicast off
# ip link set vif0.0 addr fe:ff:ff:ff:ff:ff
# ip link set xenbr0 up
# brctl addif xenbr0 bond0
# ip link set bond0 up
# brctl addif xenbr0 vif0.0
# ip link set vif0.0 up
# ip link set eth0 addr 00:16:3e:70:00:03
# ip link set eth0 arp on
# ifup eth0
# ifconfig eth0 192.168.0.1
^ permalink raw reply
* Re: [PATCH] - trivial - Improve appletalk checksum calculation
From: Joe Perches @ 2007-10-29 2:40 UTC (permalink / raw)
To: Urs Thuermann; +Cc: Herbert Xu, davem, acme, netdev
In-Reply-To: <1193605297.26695.121.camel@localhost>
On Sun, 2007-10-28 at 14:01 -0700, Joe Perches wrote:
> probably unsigned long should be u16.
Or not.
x86 u16 performs much worse than u32 (~ 50% worse)
^ permalink raw reply
* [PATCH 2.6.24-rc] [IPV6] NDISC: Fix setting base_reachable_time_ms variable.
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2007-10-29 2:49 UTC (permalink / raw)
To: davem; +Cc: netdev, yoshfuji, usagi-core
This bug was introduced by the commit
d12af679bcf8995a237560bdf7a4d734f8df5dbb.
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 20cfc90..36f7dbf 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1670,7 +1670,7 @@ int ndisc_ifinfo_sysctl_change(struct ctl_table *ctl, int write, struct file * f
filp, buffer, lenp, ppos);
else if ((strcmp(ctl->procname, "retrans_time_ms") == 0) ||
- (strcmp(ctl->procname, "base_reacable_time_ms") == 0))
+ (strcmp(ctl->procname, "base_reachable_time_ms") == 0))
ret = proc_dointvec_ms_jiffies(ctl, write,
filp, buffer, lenp, ppos);
else
--
YOSHIFUJI Hideaki @ USAGI Project <yoshfuji@linux-ipv6.org>
GPG-FP : 9022 65EB 1ECF 3AD1 0BDF 80D8 4807 F894 E062 0EEA
^ permalink raw reply related
* RE: [PATCH] ucc_geth: add support for netpoll
From: Li Yang-r58472 @ 2007-10-29 6:12 UTC (permalink / raw)
To: cbou, Sergei Shtylyov; +Cc: Anton Vorontsov, netdev, linux-kernel, linuxppc-dev
In-Reply-To: <20071027143731.GA2616@zarina>
> -----Original Message-----
> From: Anton Vorontsov [mailto:cbou@mail.ru]
> Sent: Saturday, October 27, 2007 10:38 PM
> To: Sergei Shtylyov
> Cc: Anton Vorontsov; netdev@vger.kernel.org; Li Yang-r58472;
> linux-kernel@vger.kernel.org; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] ucc_geth: add support for netpoll
>
> On Sat, Oct 27, 2007 at 05:09:51PM +0400, Sergei Shtylyov wrote:
> > Hello.
> >
> > Anton Vorontsov wrote:
> >
> > > This patch adds netpoll support for the QE UCC Gigabit Ethernet
> > > driver. The approach is very similar to the gianfar driver.
> >
> > It's rather contrarywise -- this is standard approach
> and gianfar
> > with its
> > 3 TSEC IRQs has a quite non-standard poll_controller()
> implementation.
>
> Oh.. well, right -- gianfar a bit more comlex in that regard.
>
> >
> > > Tested using netconsole.
> >
> > KGDBoE is considered a better test (I hope you've also
> tested with it).
>
> At the time of posting it was tested using netconsole only, a
> few days later it's was tested using KGDBoE also. So, it works indeed.
>
> > > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> diff --git
> > > a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c index
> > > 18a6f48..06807ce 100644
> > > --- a/drivers/net/ucc_geth.c
> > > +++ b/drivers/net/ucc_geth.c
> > > @@ -3691,6 +3691,22 @@ static irqreturn_t
> ucc_geth_irq_handler(int irq, void *info)
> > > return IRQ_HANDLED;
> > > }
> > >
> > > +#ifdef CONFIG_NET_POLL_CONTROLLER
> > > +/*
> > > + * Polling 'interrupt' - used by things like netconsole to send
> > > +skbs
> > > + * without having to re-enable interrupts. It's not called while
> > > + * the interrupt routine is executing.
> > > + */
> > > +static void ucc_netpoll(struct net_device *dev) {
> > > + struct ucc_geth_private *ugeth = netdev_priv(dev);
> > > +
> > > + disable_irq(ugeth->ug_info->uf_info.irq);
> > > + ucc_geth_irq_handler(ugeth->ug_info->uf_info.irq, dev);
> > > + enable_irq(ugeth->ug_info->uf_info.irq);
> >
> > Why not make it less complex (for a reader and gcc too :-) ?
>
> Yup, I'm agree here but it's too late. Again. ;-)
>
> This patch already accepted into the -mm (a week or so after
> the silence), so.. now I'd rather not bother Andrew with such
> really cosmetic changes. But if Jeff would directly apply
> modfied patch, I'll send it. ;-)
Oops. The original patch happened to hit the Junk mail box. :( I think
the patch is good to merge after the cosmetic change. I can do it in
next pull request to Jeff.
Thanks
- Leo
^ permalink raw reply
* Re: [UDP6]: Restore sk_filter optimisation
From: Mitsuru Chinen @ 2007-10-29 6:33 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, netdev, YOSHIFUJI Hideaki
In-Reply-To: <20070306012010.GA25763@gondor.apana.org.au>
Hello Herbert,
Let me ask a question about this patch.
After this patch was applied, 2 of the protocol stack behaviors were
changed when it receives a UDP datagram with broken checksum:
1. udp6InDatagrams is incremented instead of udpInErrors
2. In userland, recvfrom() replies an error with EAGAIN.
recvfrom() wasn't aware of such a packet before.
Are these changes intentional?
Best Regards,
----
Mitsuru Chinen <mitch@linux.vnet.ibm.com>
On Tue, 6 Mar 2007 12:20:10 +1100
Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Hi Dave:
>
> [UDP6]: Restore sk_filter optimisation
>
> This reverts the changeset
>
> [IPV6]: UDPv6 checksum.
>
> We always need to check UDPv6 checksum because it is mandatory.
>
> The sk_filter optimisation has nothing to do whether we verify the
> checksum. It simply postpones it to the point when the user calls
> recv or poll.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
>
> Cheers,
> --
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> --
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 0ad4719..4474480 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -279,8 +279,10 @@ int udpv6_queue_rcv_skb(struct sock * sk, struct sk_buff *skb)
> }
> }
>
> - if (udp_lib_checksum_complete(skb))
> - goto drop;
> + if (sk->sk_filter) {
> + if (udp_lib_checksum_complete(skb))
> + goto drop;
> + }
>
> if ((rc = sock_queue_rcv_skb(sk,skb)) < 0) {
> /* Note that an ENOMEM error is charged twice */
^ permalink raw reply
* Re: [UDP6]: Restore sk_filter optimisation
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2007-10-29 6:41 UTC (permalink / raw)
To: mitch, herbert; +Cc: davem, netdev, yoshfuji
In-Reply-To: <20071029153320.d2c00f62.mitch@linux.vnet.ibm.com>
In article <20071029153320.d2c00f62.mitch@linux.vnet.ibm.com> (at Mon, 29 Oct 2007 15:33:20 +0900), Mitsuru Chinen <mitch@linux.vnet.ibm.com> says:
> Hello Herbert,
>
> Let me ask a question about this patch.
> After this patch was applied, 2 of the protocol stack behaviors were
> changed when it receives a UDP datagram with broken checksum:
>
> 1. udp6InDatagrams is incremented instead of udpInErrors
> 2. In userland, recvfrom() replies an error with EAGAIN.
> recvfrom() wasn't aware of such a packet before.
>
> Are these changes intentional?
And, we're not sure how much the "optimization"'s benefit is.
It is even worse when we are handling multicast packets.
--yoshfuji
^ permalink raw reply
* IPVS: use proper timeout instead of fixed value
From: Simon Horman @ 2007-10-29 7:05 UTC (permalink / raw)
To: netdev, lvs-devel
Cc: wensong, ja, Joseph Mack NA3T, Graeme Fowler, Andreas Lundqvist,
Andy Gospodarek
From: Andy Gospodarek <andy@greyhouse.net>
Instead of using the default timeout of 3 minutes, this uses the timeout
specific to the protocol used for the connection. The 3 minute timeout
seems somewhat arbitrary (though I know it is used other places in the
ipvs code) and when failing over it would be much nicer to use one of
the configured timeout values.
Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
Acked-by: Simon Horman <horms@verge.net.au>
---
Hi,
I'd like to revisit this patch which was originally posted
to netdev in May 2006.
Looking through the archives as far as I can see there was
some discussion as to whether it would be good to send timeout
information in the synchronisation packet, and some discussion
as to whether it would be good to make the timeout configurable
on the slave side, and then the ball was dropped.
It seems to me that regardless of whether or not the syncronisation
protocol should be expanded to include timeout information,
and wheather or not it should be configurable on the slave side,
this patch is a good idea as the default that it provides seems
to be much more sensible than the current arrangement.
Andreas Lundqvist provided me with an example where his
cluser has long often idle connections and that in this case
the short, 3 minute default timeout, really is quite useless.
ip_vs_sync.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/ipvs/ip_vs_sync.c b/net/ipv4/ipvs/ip_vs_sync.c
--- a/net/ipv4/ipvs/ip_vs_sync.c
+++ b/net/ipv4/ipvs/ip_vs_sync.c
@@ -67,7 +67,6 @@ struct ip_vs_sync_conn_options {
struct ip_vs_seq out_seq; /* outgoing seq. struct */
};
-#define IP_VS_SYNC_CONN_TIMEOUT (3*60*HZ)
#define SIMPLE_CONN_SIZE (sizeof(struct ip_vs_sync_conn))
#define FULL_CONN_SIZE \
(sizeof(struct ip_vs_sync_conn) + sizeof(struct ip_vs_sync_conn_options))
@@ -279,6 +278,7 @@ static void ip_vs_process_message(const
struct ip_vs_sync_conn *s;
struct ip_vs_sync_conn_options *opt;
struct ip_vs_conn *cp;
+ struct ip_vs_protocol *pp;
char *p;
int i;
@@ -337,7 +337,8 @@ static void ip_vs_process_message(const
p += SIMPLE_CONN_SIZE;
atomic_set(&cp->in_pkts, sysctl_ip_vs_sync_threshold[0]);
- cp->timeout = IP_VS_SYNC_CONN_TIMEOUT;
+ pp = ip_vs_proto_get(s->protocol);
+ cp->timeout = pp->timeout_table[cp->state];
ip_vs_conn_put(cp);
if (p > buffer+buflen) {
^ permalink raw reply
* SFQ: backport some features from ESFQ (try 5)
From: Corey Hickey @ 2007-10-29 7:22 UTC (permalink / raw)
To: netdev
Patchset try 2 addresses the review by Michael Buesch.
Patchset try 3 addresses the review by Patrick McHardy.
Patchset try 4 has a few cosmetic improvements.
Patchset try 5 addresses further review by Patrick McHardy.
This set of patches is substantially the same as my previous try, with
changes made according to Patrick's recommendations.
Iproute2 patches will follow shortly.
The following is the original patch text.
This set of patches adds some of ESFQ's modifications to the original
SFQ. Thus far, I have received support for this approach rather than for
trying to get ESFQ included as a separate qdisc.
http://mailman.ds9a.nl/pipermail/lartc/2007q2/021056.html
My patches here implement "tc qdisc change", user-configurable depth
(number of flows), and user-configurable divisor (for setting hash table
size). I've left out the remaining ESFQ features (usage of jhash and
different hashing methods) because Patrick McHardy intends to submit a
patch that will supersede that functionality; see the URL above.
Default values remain the same, and SFQ's default behavior remains the
same, so there should be no user disruption.
Thanks for your consideration,
Corey
include/linux/pkt_sched.h | 23 ++-
net/sched/sch_sfq.c | 434 +++++++++++++++++++++++++++++++--------------
2 files changed, 319 insertions(+), 138 deletions(-)
[PATCH 1/8] Preparatory refactoring part 1.
[PATCH 2/8] Preparatory refactoring part 2.
[PATCH 3/8] Make "depth" (number of queues) user-configurable
[PATCH 4/8] Add divisor.
[PATCH 5/8] Make qdisc changeable.
[PATCH 6/8] Remove comments about hardcoded values.
[PATCH 7/8] Rework perturb_period.
[PATCH 8/8] Use nested compat attributes to pass parameters.
^ permalink raw reply
* [PATCH 2/8] Preparatory refactoring part 2.
From: Corey Hickey @ 2007-10-29 7:23 UTC (permalink / raw)
To: netdev; +Cc: Corey Hickey
In-Reply-To: <1193642587-32657-1-git-send-email-bugfood-ml@fatooh.org>
Factor code out of sfq_init() so that the new function can be used
by sfq_change() later.
Actually, as the diff itself shows, most of the sfq_q_init() code
comes from the original sfq_change(), but sfq_change() is only
called by sfq_init() right now. Thus, it is safe to remove
sfq_change(); "tc qdisc change" doesn't yet work for sfq anyway.
Setting default parameters is moved into a separate function for
clarity.
Signed-off-by: Corey Hickey <bugfood-ml@fatooh.org>
---
net/sched/sch_sfq.c | 88 +++++++++++++++++++++++++++-----------------------
1 files changed, 47 insertions(+), 41 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 10e2f3d..8ea816a 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -413,43 +413,41 @@ static void sfq_perturbation(unsigned long arg)
mod_timer(&q->perturb_timer, jiffies + q->perturb_period);
}
-static int sfq_change(struct Qdisc *sch, struct rtattr *opt)
+static void
+sfq_default_parameters(struct Qdisc *sch)
{
struct sfq_sched_data *q = qdisc_priv(sch);
- struct tc_sfq_qopt *ctl = RTA_DATA(opt);
- unsigned int qlen;
-
- if (opt->rta_len < RTA_LENGTH(sizeof(*ctl)))
- return -EINVAL;
-
- sch_tree_lock(sch);
- q->quantum = ctl->quantum ? : psched_mtu(sch->dev);
- q->perturb_period = ctl->perturb_period*HZ;
- if (ctl->limit)
- q->limit = min_t(u32, ctl->limit, SFQ_DEPTH - 1);
- qlen = sch->q.qlen;
- while (sch->q.qlen > q->limit)
- sfq_drop(sch);
- qdisc_tree_decrease_qlen(sch, qlen - sch->q.qlen);
-
- del_timer(&q->perturb_timer);
- if (q->perturb_period) {
- mod_timer(&q->perturb_timer, jiffies + q->perturb_period);
- get_random_bytes(&q->perturbation, 4);
- }
- sch_tree_unlock(sch);
- return 0;
+ q->quantum = psched_mtu(sch->dev);
+ q->perturbation = 0;
+ q->perturb_period = 0;
+ q->limit = SFQ_DEPTH - 1;
}
-static int sfq_init(struct Qdisc *sch, struct rtattr *opt)
+static int
+sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt)
{
- struct sfq_sched_data *q = qdisc_priv(sch);
int i;
- init_timer(&q->perturb_timer);
- q->perturb_timer.data = (unsigned long)sch;
- q->perturb_timer.function = sfq_perturbation;
+ /* At this point, parameters are set to either defaults (sfq_init) or
+ * the previous values (sfq_change). So, overwrite the parameters as
+ * specified. */
+ if (opt) {
+ struct tc_sfq_qopt *ctl = RTA_DATA(opt);
+
+ if (opt->rta_len < RTA_LENGTH(sizeof(*ctl)))
+ return -EINVAL;
+
+ if (ctl->quantum)
+ q->quantum = ctl->quantum;
+ if (ctl->perturb_period)
+ q->perturb_period = ctl->perturb_period * HZ;
+ if (ctl->limit)
+ q->limit = ctl->limit;
+ }
+ q->limit = min_t(u32, q->limit, SFQ_DEPTH - 1);
+ q->tail = SFQ_DEPTH;
+ q->max_depth = 0;
for (i=0; i<SFQ_HASH_DIVISOR; i++)
q->ht[i] = SFQ_DEPTH;
@@ -458,23 +456,31 @@ static int sfq_init(struct Qdisc *sch, struct rtattr *opt)
q->dep[i+SFQ_DEPTH].next = i+SFQ_DEPTH;
q->dep[i+SFQ_DEPTH].prev = i+SFQ_DEPTH;
}
- q->limit = SFQ_DEPTH - 1;
- q->max_depth = 0;
- q->tail = SFQ_DEPTH;
- if (opt == NULL) {
- q->quantum = psched_mtu(sch->dev);
- q->perturb_period = 0;
- get_random_bytes(&q->perturbation, 4);
- } else {
- int err = sfq_change(sch, opt);
- if (err)
- return err;
- }
for (i=0; i<SFQ_DEPTH; i++)
sfq_link(q, i);
return 0;
}
+static int sfq_init(struct Qdisc *sch, struct rtattr *opt)
+{
+ struct sfq_sched_data *q = qdisc_priv(sch);
+ int err;
+
+ sfq_default_parameters(sch);
+ if ((err = sfq_q_init(q, opt)))
+ return err;
+
+ init_timer(&q->perturb_timer);
+ q->perturb_timer.data = (unsigned long)sch;
+ q->perturb_timer.function = sfq_perturbation;
+ if (q->perturb_period) {
+ q->perturb_timer.expires = jiffies + q->perturb_period;
+ add_timer(&q->perturb_timer);
+ }
+
+ return 0;
+}
+
static void sfq_destroy(struct Qdisc *sch)
{
struct sfq_sched_data *q = qdisc_priv(sch);
--
1.5.3.4
^ permalink raw reply related
* [PATCH 4/8] Add divisor.
From: Corey Hickey @ 2007-10-29 7:23 UTC (permalink / raw)
To: netdev; +Cc: Corey Hickey
In-Reply-To: <1193642587-32657-1-git-send-email-bugfood-ml@fatooh.org>
Make hash divisor user-configurable.
Signed-off-by: Corey Hickey <bugfood-ml@fatooh.org>
---
net/sched/sch_sfq.c | 27 +++++++++++++++++++++------
1 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 1c1bf08..c74d5ce 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -76,7 +76,7 @@
It is easy to increase these values, but not in flight. */
#define SFQ_DEPTH_DEFAULT 128
-#define SFQ_HASH_DIVISOR 1024
+#define SFQ_DIVISOR_DEFAULT 10
#define SFQ_HEAD 0
#define SFQ_TAIL 1
@@ -86,6 +86,10 @@
typedef unsigned int sfq_index;
#define SFQ_MAX_DEPTH (UINT_MAX / 2 - 1)
+/* In practice, the actual divisor size is limited by kcalloc, but we still
+ * don't want to left shift by more than 31. */
+#define SFQ_MAX_DIVISOR 31
+
struct sfq_head
{
sfq_index next;
@@ -99,6 +103,7 @@ struct sfq_sched_data
unsigned quantum; /* Allotment per round: MUST BE >= MTU */
int limit;
unsigned depth;
+ unsigned hash_divisor;
/* Variables */
struct timer_list perturb_timer;
@@ -106,7 +111,7 @@ struct sfq_sched_data
sfq_index tail; /* Index of current slot in round */
sfq_index max_depth; /* Maximal depth */
- sfq_index ht[SFQ_HASH_DIVISOR]; /* Hash table */
+ sfq_index *ht; /* Hash table */
sfq_index *next; /* Active slots link */
short *allot; /* Current allotment per slot */
unsigned short *hash; /* Hash value indexed by slots */
@@ -116,7 +121,9 @@ struct sfq_sched_data
static __inline__ unsigned sfq_fold_hash(struct sfq_sched_data *q, u32 h, u32 h1)
{
- return jhash_2words(h, h1, q->perturbation) & (SFQ_HASH_DIVISOR - 1);
+ unsigned mask = (1<<q->hash_divisor) - 1;
+
+ return jhash_2words(h, h1, q->perturbation) & mask;
}
static unsigned sfq_hash(struct sfq_sched_data *q, struct sk_buff *skb)
@@ -418,6 +425,7 @@ static void sfq_perturbation(unsigned long arg)
static void sfq_q_destroy(struct sfq_sched_data *q)
{
+ kfree(q->ht);
kfree(q->dep);
kfree(q->next);
kfree(q->allot);
@@ -441,6 +449,7 @@ sfq_default_parameters(struct Qdisc *sch)
q->quantum = psched_mtu(sch->dev);
q->perturbation = 0;
q->perturb_period = 0;
+ q->hash_divisor = SFQ_DIVISOR_DEFAULT;
q->depth = SFQ_DEPTH_DEFAULT;
q->limit = SFQ_DEPTH_DEFAULT - 1;
}
@@ -463,18 +472,24 @@ sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt)
q->quantum = ctl->quantum;
if (ctl->perturb_period)
q->perturb_period = ctl->perturb_period * HZ;
+ if (ctl->divisor)
+ q->hash_divisor = ctl->divisor;
if (ctl->flows)
q->depth = ctl->flows;
if (ctl->limit)
q->limit = ctl->limit;
- if (q->depth > SFQ_MAX_DEPTH)
+ if (q->depth > SFQ_MAX_DEPTH ||
+ q->hash_divisor > SFQ_MAX_DIVISOR)
return -EINVAL;
}
q->limit = min_t(u32, q->limit, q->depth - 1);
q->tail = q->depth;
q->max_depth = 0;
+ q->ht = kcalloc(1<<q->hash_divisor, sizeof(sfq_index), GFP_KERNEL);
+ if (!q->ht)
+ goto err_case;
q->dep = kcalloc(1 + q->depth*2, sizeof(struct sfq_head), GFP_KERNEL);
if (!q->dep)
goto err_case;
@@ -491,7 +506,7 @@ sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt)
if (!q->qs)
goto err_case;
- for (i=0; i<SFQ_HASH_DIVISOR; i++)
+ for (i=0; i < 1<<q->hash_divisor; i++)
q->ht[i] = q->depth;
for (i=0; i < q->depth; i++) {
skb_queue_head_init(&q->qs[i]);
@@ -537,7 +552,7 @@ static int sfq_dump(struct Qdisc *sch, struct sk_buff *skb)
opt.perturb_period = q->perturb_period/HZ;
opt.limit = q->limit;
- opt.divisor = SFQ_HASH_DIVISOR;
+ opt.divisor = q->hash_divisor;
opt.flows = q->depth;
RTA_PUT(skb, TCA_OPTIONS, sizeof(opt), &opt);
--
1.5.3.4
^ permalink raw reply related
* [PATCH 3/8] Make "depth" (number of queues) user-configurable
From: Corey Hickey @ 2007-10-29 7:23 UTC (permalink / raw)
To: netdev; +Cc: Corey Hickey
In-Reply-To: <1193642587-32657-1-git-send-email-bugfood-ml@fatooh.org>
* replace #define with a parameter
* use old hardcoded value as a default
* kcalloc() arrays in sfq_q_init()
* free() arrays in new function sfq_q_destroy()
* move sfq_destroy() to near sfq_q_destroy(), for clarity
Signed-off-by: Corey Hickey <bugfood-ml@fatooh.org>
---
net/sched/sch_sfq.c | 104 +++++++++++++++++++++++++++++++++++----------------
1 files changed, 72 insertions(+), 32 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 8ea816a..1c1bf08 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -75,14 +75,16 @@
It is easy to increase these values, but not in flight. */
-#define SFQ_DEPTH 128
+#define SFQ_DEPTH_DEFAULT 128
#define SFQ_HASH_DIVISOR 1024
#define SFQ_HEAD 0
#define SFQ_TAIL 1
-/* This type should contain at least SFQ_DEPTH*2 values */
-typedef unsigned char sfq_index;
+/* This type must contain greater than depth*2 values, so depth is constrained
+ * accordingly. */
+typedef unsigned int sfq_index;
+#define SFQ_MAX_DEPTH (UINT_MAX / 2 - 1)
struct sfq_head
{
@@ -96,6 +98,7 @@ struct sfq_sched_data
int perturb_period;
unsigned quantum; /* Allotment per round: MUST BE >= MTU */
int limit;
+ unsigned depth;
/* Variables */
struct timer_list perturb_timer;
@@ -104,11 +107,11 @@ struct sfq_sched_data
sfq_index max_depth; /* Maximal depth */
sfq_index ht[SFQ_HASH_DIVISOR]; /* Hash table */
- sfq_index next[SFQ_DEPTH]; /* Active slots link */
- short allot[SFQ_DEPTH]; /* Current allotment per slot */
- unsigned short hash[SFQ_DEPTH]; /* Hash value indexed by slots */
- struct sk_buff_head qs[SFQ_DEPTH]; /* Slot queue */
- struct sfq_head dep[SFQ_DEPTH*2]; /* Linked list of slots, indexed by depth */
+ sfq_index *next; /* Active slots link */
+ short *allot; /* Current allotment per slot */
+ unsigned short *hash; /* Hash value indexed by slots */
+ struct sk_buff_head *qs; /* Slot queue */
+ struct sfq_head *dep; /* Linked list of slots, indexed by depth */
};
static __inline__ unsigned sfq_fold_hash(struct sfq_sched_data *q, u32 h, u32 h1)
@@ -160,7 +163,7 @@ static unsigned sfq_hash(struct sfq_sched_data *q, struct sk_buff *skb)
static inline void sfq_link(struct sfq_sched_data *q, sfq_index x)
{
sfq_index p, n;
- int d = q->qs[x].qlen + SFQ_DEPTH;
+ int d = q->qs[x].qlen + q->depth;
p = d;
n = q->dep[d].next;
@@ -211,7 +214,7 @@ static unsigned int sfq_drop(struct Qdisc *sch)
drop a packet from it */
if (d > 1) {
- sfq_index x = q->dep[d+SFQ_DEPTH].next;
+ sfq_index x = q->dep[d + q->depth].next;
skb = q->qs[x].prev;
len = skb->len;
__skb_unlink(skb, &q->qs[x]);
@@ -234,7 +237,7 @@ static unsigned int sfq_drop(struct Qdisc *sch)
kfree_skb(skb);
sfq_dec(q, d);
sch->q.qlen--;
- q->ht[q->hash[d]] = SFQ_DEPTH;
+ q->ht[q->hash[d]] = q->depth;
sch->qstats.drops++;
sch->qstats.backlog -= len;
return len;
@@ -250,8 +253,8 @@ sfq_q_enqueue(struct sk_buff *skb, struct sfq_sched_data *q, int end)
sfq_index x;
x = q->ht[hash];
- if (x == SFQ_DEPTH) {
- q->ht[hash] = x = q->dep[SFQ_DEPTH].next;
+ if (x == q->depth) {
+ q->ht[hash] = x = q->dep[q->depth].next;
q->hash[x] = hash;
}
@@ -287,7 +290,7 @@ sfq_q_enqueue(struct sk_buff *skb, struct sfq_sched_data *q, int end)
sfq_inc(q, x);
if (q->qs[x].qlen == 1) { /* The flow is new */
- if (q->tail == SFQ_DEPTH) { /* It is the first flow */
+ if (q->tail == q->depth) { /* It is the first flow */
q->tail = x;
q->next[x] = x;
q->allot[x] = q->quantum;
@@ -351,7 +354,7 @@ sk_buff *sfq_q_dequeue(struct sfq_sched_data *q)
sfq_index a, old_a;
/* No active slots */
- if (q->tail == SFQ_DEPTH)
+ if (q->tail == q->depth)
return NULL;
a = old_a = q->next[q->tail];
@@ -362,10 +365,10 @@ sk_buff *sfq_q_dequeue(struct sfq_sched_data *q)
/* Is the slot empty? */
if (q->qs[a].qlen == 0) {
- q->ht[q->hash[a]] = SFQ_DEPTH;
+ q->ht[q->hash[a]] = q->depth;
a = q->next[a];
if (a == old_a) {
- q->tail = SFQ_DEPTH;
+ q->tail = q->depth;
return skb;
}
q->next[q->tail] = a;
@@ -413,6 +416,23 @@ static void sfq_perturbation(unsigned long arg)
mod_timer(&q->perturb_timer, jiffies + q->perturb_period);
}
+static void sfq_q_destroy(struct sfq_sched_data *q)
+{
+ kfree(q->dep);
+ kfree(q->next);
+ kfree(q->allot);
+ kfree(q->hash);
+ kfree(q->qs);
+}
+
+static void sfq_destroy(struct Qdisc *sch)
+{
+ struct sfq_sched_data *q = qdisc_priv(sch);
+
+ del_timer(&q->perturb_timer);
+ sfq_q_destroy(q);
+}
+
static void
sfq_default_parameters(struct Qdisc *sch)
{
@@ -421,7 +441,8 @@ sfq_default_parameters(struct Qdisc *sch)
q->quantum = psched_mtu(sch->dev);
q->perturbation = 0;
q->perturb_period = 0;
- q->limit = SFQ_DEPTH - 1;
+ q->depth = SFQ_DEPTH_DEFAULT;
+ q->limit = SFQ_DEPTH_DEFAULT - 1;
}
static int
@@ -442,23 +463,48 @@ sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt)
q->quantum = ctl->quantum;
if (ctl->perturb_period)
q->perturb_period = ctl->perturb_period * HZ;
+ if (ctl->flows)
+ q->depth = ctl->flows;
if (ctl->limit)
q->limit = ctl->limit;
+
+ if (q->depth > SFQ_MAX_DEPTH)
+ return -EINVAL;
}
- q->limit = min_t(u32, q->limit, SFQ_DEPTH - 1);
- q->tail = SFQ_DEPTH;
+ q->limit = min_t(u32, q->limit, q->depth - 1);
+ q->tail = q->depth;
q->max_depth = 0;
+ q->dep = kcalloc(1 + q->depth*2, sizeof(struct sfq_head), GFP_KERNEL);
+ if (!q->dep)
+ goto err_case;
+ q->next = kcalloc(q->depth, sizeof(sfq_index), GFP_KERNEL);
+ if (!q->next)
+ goto err_case;
+ q->allot = kcalloc(q->depth, sizeof(short), GFP_KERNEL);
+ if (!q->allot)
+ goto err_case;
+ q->hash = kcalloc(q->depth, sizeof(unsigned short), GFP_KERNEL);
+ if (!q->hash)
+ goto err_case;
+ q->qs = kcalloc(q->depth, sizeof(struct sk_buff_head), GFP_KERNEL);
+ if (!q->qs)
+ goto err_case;
+
for (i=0; i<SFQ_HASH_DIVISOR; i++)
- q->ht[i] = SFQ_DEPTH;
- for (i=0; i<SFQ_DEPTH; i++) {
+ q->ht[i] = q->depth;
+ for (i=0; i < q->depth; i++) {
skb_queue_head_init(&q->qs[i]);
- q->dep[i+SFQ_DEPTH].next = i+SFQ_DEPTH;
- q->dep[i+SFQ_DEPTH].prev = i+SFQ_DEPTH;
+ q->dep[i + q->depth].next = i + q->depth;
+ q->dep[i + q->depth].prev = i + q->depth;
}
- for (i=0; i<SFQ_DEPTH; i++)
+
+ for (i=0; i < q->depth; i++)
sfq_link(q, i);
return 0;
+err_case:
+ sfq_q_destroy(q);
+ return -ENOBUFS;
}
static int sfq_init(struct Qdisc *sch, struct rtattr *opt)
@@ -481,12 +527,6 @@ static int sfq_init(struct Qdisc *sch, struct rtattr *opt)
return 0;
}
-static void sfq_destroy(struct Qdisc *sch)
-{
- struct sfq_sched_data *q = qdisc_priv(sch);
- del_timer(&q->perturb_timer);
-}
-
static int sfq_dump(struct Qdisc *sch, struct sk_buff *skb)
{
struct sfq_sched_data *q = qdisc_priv(sch);
@@ -498,7 +538,7 @@ static int sfq_dump(struct Qdisc *sch, struct sk_buff *skb)
opt.limit = q->limit;
opt.divisor = SFQ_HASH_DIVISOR;
- opt.flows = q->limit;
+ opt.flows = q->depth;
RTA_PUT(skb, TCA_OPTIONS, sizeof(opt), &opt);
--
1.5.3.4
^ permalink raw reply related
* [PATCH 1/8] Preparatory refactoring part 1.
From: Corey Hickey @ 2007-10-29 7:23 UTC (permalink / raw)
To: netdev; +Cc: Corey Hickey
In-Reply-To: <1193642587-32657-1-git-send-email-bugfood-ml@fatooh.org>
Make a new function sfq_q_enqueue() that operates directly on the
queue data. This will be useful for implementing sfq_change() in
a later patch. A pleasant side-effect is reducing most of the
duplicate code in sfq_enqueue() and sfq_requeue().
Similarly, make a new function sfq_q_dequeue().
Signed-off-by: Corey Hickey <bugfood-ml@fatooh.org>
---
net/sched/sch_sfq.c | 119 ++++++++++++++++++++++++++++++---------------------
1 files changed, 70 insertions(+), 49 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index b542c87..10e2f3d 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -78,6 +78,9 @@
#define SFQ_DEPTH 128
#define SFQ_HASH_DIVISOR 1024
+#define SFQ_HEAD 0
+#define SFQ_TAIL 1
+
/* This type should contain at least SFQ_DEPTH*2 values */
typedef unsigned char sfq_index;
@@ -241,9 +244,8 @@ static unsigned int sfq_drop(struct Qdisc *sch)
}
static int
-sfq_enqueue(struct sk_buff *skb, struct Qdisc* sch)
+sfq_q_enqueue(struct sk_buff *skb, struct sfq_sched_data *q, int end)
{
- struct sfq_sched_data *q = qdisc_priv(sch);
unsigned hash = sfq_hash(q, skb);
sfq_index x;
@@ -252,15 +254,37 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc* sch)
q->ht[hash] = x = q->dep[SFQ_DEPTH].next;
q->hash[x] = hash;
}
- /* If selected queue has length q->limit, this means that
- * all another queues are empty and that we do simple tail drop,
- * i.e. drop _this_ packet.
- */
- if (q->qs[x].qlen >= q->limit)
- return qdisc_drop(skb, sch);
- sch->qstats.backlog += skb->len;
- __skb_queue_tail(&q->qs[x], skb);
+ if (end == SFQ_TAIL) {
+ /* If selected queue has length q->limit, this means that
+ * all other queues are empty and that we do simple tail drop,
+ * i.e. drop _this_ packet.
+ */
+ if (q->qs[x].qlen >= q->limit) {
+ unsigned int drop_len = skb->len;
+
+ kfree_skb(skb);
+ return drop_len;
+ }
+ __skb_queue_tail(&q->qs[x], skb);
+ } else { /* end == SFQ_HEAD */
+ __skb_queue_head(&q->qs[x], skb);
+ /* If selected queue has length q->limit+1, this means that
+ * all other queues are empty and we do simple tail drop.
+ * This packet is still requeued at head of queue, tail packet
+ * is dropped.
+ */
+ if (q->qs[x].qlen > q->limit) {
+ unsigned int drop_len;
+
+ skb = q->qs[x].prev;
+ drop_len = skb->len;
+ __skb_unlink(skb, &q->qs[x]);
+ kfree_skb(skb);
+ return drop_len;
+ }
+ }
+
sfq_inc(q, x);
if (q->qs[x].qlen == 1) { /* The flow is new */
if (q->tail == SFQ_DEPTH) { /* It is the first flow */
@@ -273,6 +297,21 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc* sch)
q->tail = x;
}
}
+
+ return 0;
+}
+
+static int
+sfq_enqueue(struct sk_buff *skb, struct Qdisc* sch)
+{
+ struct sfq_sched_data *q = qdisc_priv(sch);
+
+ if (sfq_q_enqueue(skb, q, SFQ_TAIL)) {
+ sch->qstats.drops++;
+ return NET_XMIT_DROP;
+ }
+
+ sch->qstats.backlog += skb->len;
if (++sch->q.qlen <= q->limit) {
sch->bstats.bytes += skb->len;
sch->bstats.packets++;
@@ -287,58 +326,27 @@ static int
sfq_requeue(struct sk_buff *skb, struct Qdisc* sch)
{
struct sfq_sched_data *q = qdisc_priv(sch);
- unsigned hash = sfq_hash(q, skb);
- sfq_index x;
+ unsigned int drop_len;
- x = q->ht[hash];
- if (x == SFQ_DEPTH) {
- q->ht[hash] = x = q->dep[SFQ_DEPTH].next;
- q->hash[x] = hash;
- }
sch->qstats.backlog += skb->len;
- __skb_queue_head(&q->qs[x], skb);
- /* If selected queue has length q->limit+1, this means that
- * all another queues are empty and we do simple tail drop.
- * This packet is still requeued at head of queue, tail packet
- * is dropped.
- */
- if (q->qs[x].qlen > q->limit) {
- skb = q->qs[x].prev;
- __skb_unlink(skb, &q->qs[x]);
+ if ((drop_len = sfq_q_enqueue(skb, q, SFQ_HEAD))) {
+ sch->qstats.backlog -= drop_len;
sch->qstats.drops++;
- sch->qstats.backlog -= skb->len;
- kfree_skb(skb);
return NET_XMIT_CN;
}
- sfq_inc(q, x);
- if (q->qs[x].qlen == 1) { /* The flow is new */
- if (q->tail == SFQ_DEPTH) { /* It is the first flow */
- q->tail = x;
- q->next[x] = x;
- q->allot[x] = q->quantum;
- } else {
- q->next[x] = q->next[q->tail];
- q->next[q->tail] = x;
- q->tail = x;
- }
- }
+
if (++sch->q.qlen <= q->limit) {
sch->qstats.requeues++;
return 0;
}
- sch->qstats.drops++;
sfq_drop(sch);
return NET_XMIT_CN;
}
-
-
-
-static struct sk_buff *
-sfq_dequeue(struct Qdisc* sch)
+static struct
+sk_buff *sfq_q_dequeue(struct sfq_sched_data *q)
{
- struct sfq_sched_data *q = qdisc_priv(sch);
struct sk_buff *skb;
sfq_index a, old_a;
@@ -351,8 +359,6 @@ sfq_dequeue(struct Qdisc* sch)
/* Grab packet */
skb = __skb_dequeue(&q->qs[a]);
sfq_dec(q, a);
- sch->q.qlen--;
- sch->qstats.backlog -= skb->len;
/* Is the slot empty? */
if (q->qs[a].qlen == 0) {
@@ -369,6 +375,21 @@ sfq_dequeue(struct Qdisc* sch)
a = q->next[a];
q->allot[a] += q->quantum;
}
+
+ return skb;
+}
+
+static struct sk_buff
+*sfq_dequeue(struct Qdisc* sch)
+{
+ struct sfq_sched_data *q = qdisc_priv(sch);
+ struct sk_buff *skb;
+
+ skb = sfq_q_dequeue(q);
+ if (skb == NULL)
+ return NULL;
+ sch->q.qlen--;
+ sch->qstats.backlog -= skb->len;
return skb;
}
--
1.5.3.4
^ permalink raw reply related
* [PATCH 6/8] Remove comments about hardcoded values.
From: Corey Hickey @ 2007-10-29 7:23 UTC (permalink / raw)
To: netdev; +Cc: Corey Hickey
In-Reply-To: <1193642587-32657-1-git-send-email-bugfood-ml@fatooh.org>
None of these are true anymore (hooray!).
Signed-off-by: Corey Hickey <bugfood-ml@fatooh.org>
---
include/linux/pkt_sched.h | 8 --------
net/sched/sch_sfq.c | 13 +------------
2 files changed, 1 insertions(+), 20 deletions(-)
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 919af93..d754a3d 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -148,14 +148,6 @@ struct tc_sfq_qopt
unsigned flows; /* Maximal number of flows */
};
-/*
- * NOTE: limit, divisor and flows are hardwired to code at the moment.
- *
- * limit=flows=128, divisor=1024;
- *
- * The only reason for this is efficiency, it is possible
- * to change these parameters in compile time.
- */
/* RED section */
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index ca8716f..7b11086 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -62,18 +62,7 @@
We still need true WFQ for top level CSZ, but using WFQ
for the best effort traffic is absolutely pointless:
- SFQ is superior for this purpose.
-
- IMPLEMENTATION:
- This implementation limits maximal queue length to 128;
- maximal mtu to 2^15-1; number of hash buckets to 1024.
- The only goal of this restrictions was that all data
- fit into one 4K page :-). Struct sfq_sched_data is
- organized in anti-cache manner: all the data for a bucket
- are scattered over different locations. This is not good,
- but it allowed me to put it into 4K.
-
- It is easy to increase these values, but not in flight. */
+ SFQ is superior for this purpose. */
#define SFQ_DEPTH_DEFAULT 128
#define SFQ_DIVISOR_DEFAULT 10
--
1.5.3.4
^ permalink raw reply related
* [PATCH 5/8] Make qdisc changeable.
From: Corey Hickey @ 2007-10-29 7:23 UTC (permalink / raw)
To: netdev; +Cc: Corey Hickey
In-Reply-To: <1193642587-32657-1-git-send-email-bugfood-ml@fatooh.org>
Re-implement sfq_change() and enable Qdisc_opts.change so "tc qdisc
change" will work.
Signed-off-by: Corey Hickey <bugfood-ml@fatooh.org>
---
net/sched/sch_sfq.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 66 insertions(+), 1 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index c74d5ce..ca8716f 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -454,6 +454,17 @@ sfq_default_parameters(struct Qdisc *sch)
q->limit = SFQ_DEPTH_DEFAULT - 1;
}
+static void
+sfq_copy_parameters(struct sfq_sched_data *dst, struct sfq_sched_data *src)
+{
+ dst->quantum = src->quantum;
+ dst->perturbation = src->perturbation;
+ dst->perturb_period = src->perturb_period;
+ dst->hash_divisor = src->hash_divisor;
+ dst->limit = src->limit;
+ dst->depth = src->depth;
+}
+
static int
sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt)
{
@@ -542,6 +553,60 @@ static int sfq_init(struct Qdisc *sch, struct rtattr *opt)
return 0;
}
+static int sfq_change(struct Qdisc *sch, struct rtattr *opt)
+{
+ struct sfq_sched_data *q = qdisc_priv(sch);
+ struct sfq_sched_data tmp;
+ struct sk_buff *skb;
+ unsigned int qlen;
+ int err;
+
+ /* set up tmp queue */
+ memset(&tmp, 0, sizeof(struct sfq_sched_data));
+ sfq_copy_parameters(&tmp, q);
+ if ((err = sfq_q_init(&tmp, opt)))
+ return err;
+
+ /* handle perturbation */
+ /* This code avoids resetting the perturb_timer unless perturb_period
+ * is changed. Note that the rest of this function leaves
+ * q->perturb_timer alone, whereas all other members of q get
+ * overwritten from tmp. */
+ if (!tmp.perturb_period) {
+ tmp.perturbation = 0;
+ del_timer(&q->perturb_timer);
+ } else if (tmp.perturb_period != q->perturb_period) {
+ mod_timer(&q->perturb_timer, jiffies + tmp.perturb_period);
+ }
+
+ /* move packets from the old queue to the tmp queue */
+ sch_tree_lock(sch);
+ qlen = sch->q.qlen;
+ while (sch->q.qlen >= tmp.limit - 1)
+ sfq_drop(sch);
+ qdisc_tree_decrease_qlen(sch, qlen - sch->q.qlen);
+ while ((skb = sfq_q_dequeue(q)) != NULL)
+ sfq_q_enqueue(skb, &tmp, SFQ_TAIL);
+
+ /* clean up the old queue */
+ sfq_q_destroy(q);
+
+ /* copy elements of the tmp queue into the old queue */
+ sfq_copy_parameters(q, &tmp);
+ q->tail = tmp.tail;
+ q->max_depth = tmp.max_depth;
+ q->ht = tmp.ht;
+ q->dep = tmp.dep;
+ q->next = tmp.next;
+ q->allot = tmp.allot;
+ q->hash = tmp.hash;
+ q->qs = tmp.qs;
+
+ /* finish up */
+ sch_tree_unlock(sch);
+ return 0;
+}
+
static int sfq_dump(struct Qdisc *sch, struct sk_buff *skb)
{
struct sfq_sched_data *q = qdisc_priv(sch);
@@ -576,7 +641,7 @@ static struct Qdisc_ops sfq_qdisc_ops = {
.init = sfq_init,
.reset = sfq_reset,
.destroy = sfq_destroy,
- .change = NULL,
+ .change = sfq_change,
.dump = sfq_dump,
.owner = THIS_MODULE,
};
--
1.5.3.4
^ permalink raw reply related
* [PATCH 7/8] Rework perturb_period.
From: Corey Hickey @ 2007-10-29 7:23 UTC (permalink / raw)
To: netdev; +Cc: Corey Hickey
In-Reply-To: <1193642587-32657-1-git-send-email-bugfood-ml@fatooh.org>
perturb_period is the only parameter that doesn't match 1:1 with the
value from userspace. Multiplying perturb_period by HZ when used rather
than when assigned makes it easy and clean to use a small function for
setting parameters (in a subsequent patch).
perturb_period is currently a signed integer, but I can't see any good
reason why this is so--a negative perturbation period will add a timer
that expires in the past, causing constant perturbation, which makes
hashing useless.
Strictly speaking, this will break binary compatibility with older
versions of tc, but that ought not to be a problem because (a) there's
no valid use for a negative perturb_period, and (b) negative values
will be seen as high values (> INT_MAX), which don't work anyway.
Signed-off-by: Corey Hickey <bugfood-ml@fatooh.org>
---
include/linux/pkt_sched.h | 2 +-
net/sched/sch_sfq.c | 14 ++++++++------
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index d754a3d..14a08ad 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -142,7 +142,7 @@ enum
struct tc_sfq_qopt
{
unsigned quantum; /* Bytes per round allocated to flow */
- int perturb_period; /* Period of hash perturbation */
+ unsigned perturb_period; /* Period of hash perturbation */
__u32 limit; /* Maximal packets in queue */
unsigned divisor; /* Hash divisor */
unsigned flows; /* Maximal number of flows */
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 7b11086..2764a54 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -70,6 +70,8 @@
#define SFQ_HEAD 0
#define SFQ_TAIL 1
+#define SFQ_PERTURB(period) (jiffies + (unsigned long)period * HZ)
+
/* This type must contain greater than depth*2 values, so depth is constrained
* accordingly. */
typedef unsigned int sfq_index;
@@ -88,7 +90,7 @@ struct sfq_head
struct sfq_sched_data
{
/* Parameters */
- int perturb_period;
+ unsigned perturb_period;
unsigned quantum; /* Allotment per round: MUST BE >= MTU */
int limit;
unsigned depth;
@@ -409,7 +411,7 @@ static void sfq_perturbation(unsigned long arg)
get_random_bytes(&q->perturbation, 4);
if (q->perturb_period)
- mod_timer(&q->perturb_timer, jiffies + q->perturb_period);
+ mod_timer(&q->perturb_timer, SFQ_PERTURB(q->perturb_period));
}
static void sfq_q_destroy(struct sfq_sched_data *q)
@@ -471,7 +473,7 @@ sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt)
if (ctl->quantum)
q->quantum = ctl->quantum;
if (ctl->perturb_period)
- q->perturb_period = ctl->perturb_period * HZ;
+ q->perturb_period = ctl->perturb_period;
if (ctl->divisor)
q->hash_divisor = ctl->divisor;
if (ctl->flows)
@@ -535,7 +537,7 @@ static int sfq_init(struct Qdisc *sch, struct rtattr *opt)
q->perturb_timer.data = (unsigned long)sch;
q->perturb_timer.function = sfq_perturbation;
if (q->perturb_period) {
- q->perturb_timer.expires = jiffies + q->perturb_period;
+ q->perturb_timer.expires = SFQ_PERTURB(q->perturb_period);
add_timer(&q->perturb_timer);
}
@@ -565,7 +567,7 @@ static int sfq_change(struct Qdisc *sch, struct rtattr *opt)
tmp.perturbation = 0;
del_timer(&q->perturb_timer);
} else if (tmp.perturb_period != q->perturb_period) {
- mod_timer(&q->perturb_timer, jiffies + tmp.perturb_period);
+ mod_timer(&q->perturb_timer, SFQ_PERTURB(tmp.perturb_period));
}
/* move packets from the old queue to the tmp queue */
@@ -603,7 +605,7 @@ static int sfq_dump(struct Qdisc *sch, struct sk_buff *skb)
struct tc_sfq_qopt opt;
opt.quantum = q->quantum;
- opt.perturb_period = q->perturb_period/HZ;
+ opt.perturb_period = q->perturb_period;
opt.limit = q->limit;
opt.divisor = q->hash_divisor;
--
1.5.3.4
^ permalink raw reply related
* [PATCH 8/8] Use nested compat attributes to pass parameters.
From: Corey Hickey @ 2007-10-29 7:23 UTC (permalink / raw)
To: netdev; +Cc: Corey Hickey
In-Reply-To: <1193642587-32657-1-git-send-email-bugfood-ml@fatooh.org>
This fixes the ambiguity between, for example:
tc qdisc change ... perturb 0
tc qdisc change ...
Without this patch, there is no way for SFQ to differentiate between
a parameter specified to be 0 and a parameter that was omitted.
Signed-off-by: Corey Hickey <bugfood-ml@fatooh.org>
---
include/linux/pkt_sched.h | 13 ++++++++
net/sched/sch_sfq.c | 69 ++++++++++++++++++++++++++++++++++----------
2 files changed, 66 insertions(+), 16 deletions(-)
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 14a08ad..b1a1a52 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -148,6 +148,19 @@ struct tc_sfq_qopt
unsigned flows; /* Maximal number of flows */
};
+enum
+{
+ TCA_SFQ_UNSPEC,
+ TCA_SFQ_COMPAT,
+ TCA_SFQ_QUANTUM,
+ TCA_SFQ_PERTURB,
+ TCA_SFQ_LIMIT,
+ TCA_SFQ_DIVISOR,
+ TCA_SFQ_FLOWS,
+ __TCA_SFQ_MAX,
+};
+
+#define TCA_SFQ_MAX (__TCA_SFQ_MAX - 1)
/* RED section */
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 2764a54..5c25b05 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -456,6 +456,29 @@ sfq_copy_parameters(struct sfq_sched_data *dst, struct sfq_sched_data *src)
dst->depth = src->depth;
}
+/* SFQ parameters exist as individual rtattr attributes, with a nested
+ * "struct tc_sfq_qopt" for compatibility with older userspace tools. If an
+ * individual attribute is set, we want to use it; otherwise, fall back to the
+ * nested struct.
+ * There is one caveat: if a member of the nested struct is 0, we cannot
+ * determine if that parameter is supposed to be 0 or if it is merely unset.
+ * So, only set a parameter if the corresponding struct member (u32 compat) is
+ * nonzero. When setting a parameter to 0, it is necessary to use the
+ * individual attribute. */
+static inline int
+sfq_get_parameter(u32 *dst, struct rtattr *tb[TCA_SFQ_MAX], int attr,
+ u32 compat)
+{
+ struct rtattr *rta = tb[(attr - 1)];
+ if (rta)
+ *dst = RTA_GET_U32(rta);
+ else if (compat)
+ *dst = compat;
+
+ rtattr_failure:
+ return -EINVAL;
+}
+
static int
sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt)
{
@@ -465,21 +488,24 @@ sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt)
* the previous values (sfq_change). So, overwrite the parameters as
* specified. */
if (opt) {
- struct tc_sfq_qopt *ctl = RTA_DATA(opt);
-
- if (opt->rta_len < RTA_LENGTH(sizeof(*ctl)))
- return -EINVAL;
-
- if (ctl->quantum)
- q->quantum = ctl->quantum;
- if (ctl->perturb_period)
- q->perturb_period = ctl->perturb_period;
- if (ctl->divisor)
- q->hash_divisor = ctl->divisor;
- if (ctl->flows)
- q->depth = ctl->flows;
- if (ctl->limit)
- q->limit = ctl->limit;
+ struct tc_sfq_qopt *ctl;
+ struct rtattr *tb[TCA_SFQ_MAX];
+
+ if (rtattr_parse_nested_compat(tb, TCA_SFQ_MAX, opt, ctl,
+ sizeof(*ctl)))
+ goto rtattr_failure;
+
+ if (sfq_get_parameter(&(q->quantum), tb, TCA_SFQ_QUANTUM,
+ ctl->quantum) ||
+ sfq_get_parameter(&(q->perturb_period), tb, TCA_SFQ_PERTURB,
+ ctl->perturb_period) ||
+ sfq_get_parameter(&(q->hash_divisor), tb, TCA_SFQ_DIVISOR,
+ ctl->divisor) ||
+ sfq_get_parameter(&(q->depth), tb, TCA_SFQ_FLOWS,
+ ctl->flows) ||
+ sfq_get_parameter(&(q->limit), tb, TCA_SFQ_LIMIT,
+ ctl->limit))
+ goto rtattr_failure;
if (q->depth > SFQ_MAX_DEPTH ||
q->hash_divisor > SFQ_MAX_DIVISOR)
@@ -519,6 +545,8 @@ sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt)
for (i=0; i < q->depth; i++)
sfq_link(q, i);
return 0;
+rtattr_failure:
+ return -EINVAL;
err_case:
sfq_q_destroy(q);
return -ENOBUFS;
@@ -602,17 +630,26 @@ 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 rtattr *nest;
struct tc_sfq_qopt opt;
opt.quantum = q->quantum;
opt.perturb_period = q->perturb_period;
-
opt.limit = q->limit;
opt.divisor = q->hash_divisor;
opt.flows = q->depth;
+ nest = RTA_NEST_COMPAT(skb, TCA_OPTIONS, sizeof(opt), &opt);
+
+ RTA_PUT_U32(skb, TCA_SFQ_QUANTUM, q->quantum);
+ RTA_PUT_U32(skb, TCA_SFQ_PERTURB, q->perturb_period);
+ RTA_PUT_U32(skb, TCA_SFQ_LIMIT, q->limit);
+ RTA_PUT_U32(skb, TCA_SFQ_DIVISOR, q->hash_divisor);
+ RTA_PUT_U32(skb, TCA_SFQ_FLOWS, q->depth);
RTA_PUT(skb, TCA_OPTIONS, sizeof(opt), &opt);
+ RTA_NEST_COMPAT_END(skb, nest);
+
return skb->len;
rtattr_failure:
--
1.5.3.4
^ permalink raw reply related
* [iproute2] SFQ: backport some features from ESFQ (try 5)
From: Corey Hickey @ 2007-10-29 7:24 UTC (permalink / raw)
To: netdev
These patches follow the ESFQ-->SFQ kernel patches. See the kernel
patch summary for general information.
Thanks,
Corey
include/linux/pkt_sched.h | 23 ++++++++++++++---------
tc/q_sfq.c | 42 +++++++++++++++++++++++++++++++++++++-----
2 files changed, 51 insertions(+), 14 deletions(-)
[PATCH 1/3] SFQ: Support changing depth and divisor.
[PATCH 2/3] Change perturb_period to unsigned.
[PATCH 3/3] Use nested compat attributes for passing parameters to the kernel.
^ permalink raw reply
* [PATCH 2/3] Change perturb_period to unsigned.
From: Corey Hickey @ 2007-10-29 7:24 UTC (permalink / raw)
To: netdev
In-Reply-To: <1193642687-1272-1-git-send-email-bugfood-ml@fatooh.org>
This corresponds to the kernel patch doing the same.
Here, too, this will technically break binary compatibility with older
kernels, but that shouldn't be a problem because negative perturb_period
values aren't usable anyway.
---
include/linux/pkt_sched.h | 2 +-
tc/q_sfq.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 9d41f63..fb04a89 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -142,7 +142,7 @@ enum
struct tc_sfq_qopt
{
unsigned quantum; /* Bytes per round allocated to flow */
- int perturb_period; /* Period of hash perturbation */
+ unsigned perturb_period; /* Period of hash perturbation */
__u32 limit; /* Maximal packets in queue */
unsigned divisor; /* Hash divisor */
unsigned flows; /* Maximal number of flows */
diff --git a/tc/q_sfq.c b/tc/q_sfq.c
index 19e76ba..83c8a54 100644
--- a/tc/q_sfq.c
+++ b/tc/q_sfq.c
@@ -47,7 +47,7 @@ static int sfq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
ok++;
} else if (strcmp(*argv, "perturb") == 0) {
NEXT_ARG();
- if (get_integer(&opt.perturb_period, *argv, 0)) {
+ if (get_u32(&opt.perturb_period, *argv, 0)) {
fprintf(stderr, "Illegal \"perturb\"\n");
return -1;
}
@@ -114,7 +114,7 @@ static int sfq_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
fprintf(f, "flows %u/%u ", qopt->flows, qopt->divisor);
}
if (qopt->perturb_period)
- fprintf(f, "perturb %dsec ", qopt->perturb_period);
+ fprintf(f, "perturb %usec ", qopt->perturb_period);
return 0;
}
--
1.5.3.4
^ permalink raw reply related
* [PATCH 1/3] SFQ: Support changing depth and divisor.
From: Corey Hickey @ 2007-10-29 7:24 UTC (permalink / raw)
To: netdev
In-Reply-To: <1193642687-1272-1-git-send-email-bugfood-ml@fatooh.org>
This can safely be applied either before or after the kernel
patches because the tc_sfq_qopt struct is unchanged:
- old kernels will ignore the new parameters from new iproute2
- new kernels will use the same defaults for the new parameters
---
include/linux/pkt_sched.h | 9 ---------
tc/q_sfq.c | 20 +++++++++++++++++++-
2 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 268c515..9d41f63 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -148,15 +148,6 @@ struct tc_sfq_qopt
unsigned flows; /* Maximal number of flows */
};
-/*
- * NOTE: limit, divisor and flows are hardwired to code at the moment.
- *
- * limit=flows=128, divisor=1024;
- *
- * The only reason for this is efficiency, it is possible
- * to change these parameters in compile time.
- */
-
/* RED section */
enum
diff --git a/tc/q_sfq.c b/tc/q_sfq.c
index 05385cf..19e76ba 100644
--- a/tc/q_sfq.c
+++ b/tc/q_sfq.c
@@ -25,7 +25,7 @@
static void explain(void)
{
- fprintf(stderr, "Usage: ... sfq [ limit NUMBER ] [ perturb SECS ] [ quantum BYTES ]\n");
+ fprintf(stderr, "Usage: ... sfq [ limit NUMBER ] [ depth FLOWS ] [ divisor HASHBITS ] [ perturb SECS ] [ quantum BYTES ]\n");
}
#define usage() return(-1)
@@ -63,6 +63,24 @@ static int sfq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
return -1;
}
ok++;
+ } else if (strcmp(*argv, "depth") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&opt.flows, *argv, 0)) {
+ fprintf(stderr, "Illegal \"depth\"\n");
+ return -1;
+ }
+ ok++;
+ } else if (strcmp(*argv, "divisor") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&opt.divisor, *argv, 0)) {
+ fprintf(stderr, "Illegal \"divisor\"\n");
+ return -1;
+ }
+ if (opt.divisor >= 15) {
+ fprintf(stderr, "Illegal \"divisor\", must be < 15\n");
+ return -1;
+ }
+ ok++;
} else if (strcmp(*argv, "help") == 0) {
explain();
return -1;
--
1.5.3.4
^ permalink raw reply related
* [PATCH 3/3] Use nested compat attributes for passing parameters to the kernel.
From: Corey Hickey @ 2007-10-29 7:24 UTC (permalink / raw)
To: netdev; +Cc: Corey Hickey
In-Reply-To: <1193642687-1272-1-git-send-email-bugfood-ml@fatooh.org>
Note that I have left sfq_print_opt() alone. At this point, there
can be no difference between the data in the nested rtattrs and the
data in the compat rtattr, and I didn't want to add clutter that
isn't useful. Let me know if I should do differently.
Signed-off-by: Corey Hickey <bugfood-ml@fatooh.org>
---
include/linux/pkt_sched.h | 14 ++++++++++++++
tc/q_sfq.c | 18 ++++++++++++++++--
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index fb04a89..aad04eb 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -148,6 +148,20 @@ struct tc_sfq_qopt
unsigned flows; /* Maximal number of flows */
};
+enum
+{
+ TCA_SFQ_UNSPEC,
+ TCA_SFQ_COMPAT,
+ TCA_SFQ_QUANTUM,
+ TCA_SFQ_PERTURB,
+ TCA_SFQ_LIMIT,
+ TCA_SFQ_DIVISOR,
+ TCA_SFQ_FLOWS,
+ __TCA_SFQ_MAX,
+};
+
+#define TCA_SFQ_MAX (__TCA_SFQ_MAX - 1)
+
/* RED section */
enum
diff --git a/tc/q_sfq.c b/tc/q_sfq.c
index 83c8a54..69f17c8 100644
--- a/tc/q_sfq.c
+++ b/tc/q_sfq.c
@@ -34,9 +34,13 @@ static int sfq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
{
int ok=0;
struct tc_sfq_qopt opt;
+ struct rtattr *nest;
memset(&opt, 0, sizeof(opt));
+ /* put blank data in rtattr so there is a "hole" to fill later */
+ nest = addattr_nest_compat(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
+
while (argc > 0) {
if (strcmp(*argv, "quantum") == 0) {
NEXT_ARG();
@@ -44,6 +48,7 @@ static int sfq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
fprintf(stderr, "Illegal \"limit\"\n");
return -1;
}
+ addattr32(n, 1024, TCA_SFQ_QUANTUM, opt.quantum);
ok++;
} else if (strcmp(*argv, "perturb") == 0) {
NEXT_ARG();
@@ -51,6 +56,7 @@ static int sfq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
fprintf(stderr, "Illegal \"perturb\"\n");
return -1;
}
+ addattr32(n, 1024, TCA_SFQ_PERTURB, opt.perturb_period);
ok++;
} else if (strcmp(*argv, "limit") == 0) {
NEXT_ARG();
@@ -62,6 +68,7 @@ static int sfq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
fprintf(stderr, "Illegal \"limit\", must be > 1\n");
return -1;
}
+ addattr32(n, 1024, TCA_SFQ_LIMIT, opt.limit);
ok++;
} else if (strcmp(*argv, "depth") == 0) {
NEXT_ARG();
@@ -69,6 +76,7 @@ static int sfq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
fprintf(stderr, "Illegal \"depth\"\n");
return -1;
}
+ addattr32(n, 1024, TCA_SFQ_FLOWS, opt.flows);
ok++;
} else if (strcmp(*argv, "divisor") == 0) {
NEXT_ARG();
@@ -80,6 +88,7 @@ static int sfq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
fprintf(stderr, "Illegal \"divisor\", must be < 15\n");
return -1;
}
+ addattr32(n, 1024, TCA_SFQ_DIVISOR, opt.divisor);
ok++;
} else if (strcmp(*argv, "help") == 0) {
explain();
@@ -92,8 +101,13 @@ static int sfq_parse_opt(struct qdisc_util *qu, int argc, char **argv, struct nl
argc--; argv++;
}
- if (ok)
- addattr_l(n, 1024, TCA_OPTIONS, &opt, sizeof(opt));
+ if (ok) {
+ /* fill the "hole" we left earlier with real compat data */
+ memcpy(RTA_DATA(nest), &opt, sizeof(opt));
+ addattr_nest_compat_end(n, nest);
+ }
+ else
+ nest->rta_len = 0;
return 0;
}
--
1.5.3.4
^ permalink raw reply related
* Re: [PATCH 8/8] Use nested compat attributes to pass parameters.
From: Corey Hickey @ 2007-10-29 7:27 UTC (permalink / raw)
To: netdev
In-Reply-To: <1193642587-32657-9-git-send-email-bugfood-ml@fatooh.org>
Corey Hickey wrote:
> +/* SFQ parameters exist as individual rtattr attributes, with a nested
> + * "struct tc_sfq_qopt" for compatibility with older userspace tools. If an
> + * individual attribute is set, we want to use it; otherwise, fall back to the
> + * nested struct.
> + * There is one caveat: if a member of the nested struct is 0, we cannot
> + * determine if that parameter is supposed to be 0 or if it is merely unset.
> + * So, only set a parameter if the corresponding struct member (u32 compat) is
> + * nonzero. When setting a parameter to 0, it is necessary to use the
> + * individual attribute. */
> +static inline int
> +sfq_get_parameter(u32 *dst, struct rtattr *tb[TCA_SFQ_MAX], int attr,
> + u32 compat)
> +{
> + struct rtattr *rta = tb[(attr - 1)];
> + if (rta)
> + *dst = RTA_GET_U32(rta);
> + else if (compat)
> + *dst = compat;
> +
> + rtattr_failure:
> + return -EINVAL;
> +}
> +
> static int
> sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt)
> {
> @@ -465,21 +488,24 @@ sfq_q_init(struct sfq_sched_data *q, struct rtattr *opt)
> * the previous values (sfq_change). So, overwrite the parameters as
> * specified. */
> if (opt) {
> - struct tc_sfq_qopt *ctl = RTA_DATA(opt);
> -
> - if (opt->rta_len < RTA_LENGTH(sizeof(*ctl)))
> - return -EINVAL;
> -
> - if (ctl->quantum)
> - q->quantum = ctl->quantum;
> - if (ctl->perturb_period)
> - q->perturb_period = ctl->perturb_period;
> - if (ctl->divisor)
> - q->hash_divisor = ctl->divisor;
> - if (ctl->flows)
> - q->depth = ctl->flows;
> - if (ctl->limit)
> - q->limit = ctl->limit;
> + struct tc_sfq_qopt *ctl;
> + struct rtattr *tb[TCA_SFQ_MAX];
> +
> + if (rtattr_parse_nested_compat(tb, TCA_SFQ_MAX, opt, ctl,
> + sizeof(*ctl)))
> + goto rtattr_failure;
> +
> + if (sfq_get_parameter(&(q->quantum), tb, TCA_SFQ_QUANTUM,
> + ctl->quantum) ||
> + sfq_get_parameter(&(q->perturb_period), tb, TCA_SFQ_PERTURB,
> + ctl->perturb_period) ||
> + sfq_get_parameter(&(q->hash_divisor), tb, TCA_SFQ_DIVISOR,
> + ctl->divisor) ||
> + sfq_get_parameter(&(q->depth), tb, TCA_SFQ_FLOWS,
> + ctl->flows) ||
> + sfq_get_parameter(&(q->limit), tb, TCA_SFQ_LIMIT,
> + ctl->limit))
> + goto rtattr_failure;
>
You may note that this part ended up being rather ugly, and I wouldn't
blame anyone for wanting it to be improved. I don't see any good
solutions; the alternatives I can provide are:
1. Use a macro, as I had originally written for this patch:
http://marc.info/?l=linux-netdev&m=119102007907626&w=2
(search for GET_PARAM)
The macro itself doesn't look pretty, but the usage is fairly clean.
2. Use neither macro nor function.
There would be five repetitions of very similar code, with five lines
per repetition: not very nice, but the formatting would still look better.
3. Only use a separate rtattr for perturb, since the other parameters
work fine already. From the start, I had wanted to keep parameter
parsing consistent, but it may not be worth it. This would definitely be
the cleanest approach for readability, and the most simple.
Sorry to bother you with such a superficial problem, but I really don't
know what would be preferable.
Thanks,
Corey
^ permalink raw reply
* Re: last git: BUG: unable to handle kernel paging request at virtual address 92184900
From: Andrew Morton @ 2007-10-29 8:11 UTC (permalink / raw)
To: Giacomo Catenazzi; +Cc: Linux Kernel Mailing List, netdev
In-Reply-To: <471E45B4.9050504@debian.org>
On Tue, 23 Oct 2007 21:04:20 +0200 Giacomo Catenazzi <cate@debian.org> wrote:
> Hello people,
>
> I've still some kernel bug
>
> ciao
> cate
>
> Oct 23 20:20:05 catee kernel: BUG: unable to handle kernel paging request at virtual address 92184900
> Oct 23 20:20:05 catee kernel: printing eip: c017b9c2 *pde = 00000000
> Oct 23 20:20:05 catee kernel: Oops: 0002 [#1] SMP
> Oct 23 20:20:05 catee kernel: Modules linked in: fuse tuner tea5767 tda8290 tuner_simple mt20xx floppy bttv ir_common videobuf_dma_sg btcx_risc tveeprom
> Oct 23 20:20:05 catee kernel:
> Oct 23 20:20:05 catee kernel: Pid: 2191, comm: dirmngr Not tainted (2.6.23-g0b776eb5 #16)
> Oct 23 20:20:05 catee kernel: EIP: 0060:[<c017b9c2>] EFLAGS: 00010203 CPU: 2
> Oct 23 20:20:05 catee kernel: EIP is at generic_drop_inode+0x5d/0x147
> Oct 23 20:20:05 catee kernel: EAX: c478b238 EBX: c478b228 ECX: c2491600 EDX: c4d170b8
> Oct 23 20:20:05 catee kernel: ESI: c2405f40 EDI: c478b228 EBP: c4933940 ESP: c3798e90
> Oct 23 20:20:05 catee kernel: DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> Oct 23 20:20:05 catee kernel: Process dirmngr (pid: 2191, ti=c3798000 task=c2c99ab0 task.ti=c3798000)
> Oct 23 20:20:05 catee kernel: Stack: c478b228 c017b00d c5046314 c01791bf c5046314 c2405f40 c017a0d7 00000008
> Oct 23 20:20:05 catee kernel: c016b622 00000010 00000000 c5046314 c478b228 c037f600 c478b200 c2832200
> Oct 23 20:20:05 catee kernel: 00000000 c02f9a4d 00000000 c20107b4 c2469ae0 00000000 00000001 ffffffff
> Oct 23 20:20:05 catee kernel: Call Trace:
> Oct 23 20:20:05 catee kernel: [<c017b00d>] iput+0x5c/0x62
> Oct 23 20:20:05 catee kernel: [<c01791bf>] d_kill+0x2b/0x44
> Oct 23 20:20:05 catee kernel: [<c017a0d7>] dput+0x6d/0xf3
> Oct 23 20:20:05 catee kernel: [<c016b622>] __fput+0x124/0x169
> Oct 23 20:20:05 catee kernel: [<c02f9a4d>] sys_accept+0x17b/0x1d8
> Oct 23 20:20:05 catee kernel: [<c013527f>] autoremove_wake_function+0x0/0x35
> Oct 23 20:20:05 catee kernel: [<c013d617>] tick_program_event+0x38/0x58
> Oct 23 20:20:05 catee kernel: [<c02f9cb2>] sys_socketcall+0x208/0x273
> Oct 23 20:20:05 catee kernel: [<c0103fde>] sysenter_past_esp+0x5f/0x85
> Oct 23 20:20:05 catee kernel: =======================
> Oct 23 20:20:05 catee kernel: Code: 8b 43 14 89 42 04 89 10 8d 43 10 89 43 10 89 43 14 83 8b 44 01 00 00 20 83 2d 40 e8 49 c0 01 08 05 e4 27 41 c0 01 8b 83 e0 00 00 <00> 85 c0 0f 85 cd 00 00 00 89 d8 e8 e3 fd ff ff 89 d8 e8 b0 f5
Is this still happening in the latest Linus tree?
If so, please send some more oops traces so we can see if it's always happening
in the same place.
Please only send the first oops trace after a bootup: the one with [#1] in
the "Oops:" line. All the others could be a consequence of the first.
Thanks.
^ permalink raw reply
* Re: [PATCH 2.6.24-rc] [IPV6] NDISC: Fix setting base_reachable_time_ms variable.
From: David Miller @ 2007-10-29 8:33 UTC (permalink / raw)
To: yoshfuji; +Cc: netdev, usagi-core
In-Reply-To: <20071029.114954.02253064.yoshfuji@linux-ipv6.org>
From: YOSHIFUJI Hideaki / 吉藤英明 <yoshfuji@linux-ipv6.org>
Date: Mon, 29 Oct 2007 11:49:54 +0900 (JST)
> This bug was introduced by the commit
> d12af679bcf8995a237560bdf7a4d734f8df5dbb.
>
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Patch applied, thank you!
^ 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