Jeff Kirsher wrote, On 08/30/2008 09:23 AM:
quoted text > From: Alexander Duyck <alexander.h.duyck@intel.com>
>
> This patch is intended to add a qdisc to support the new tx multiqueue
> architecture by providing a band for each hardware queue. By doing
> this it is possible to support a different qdisc per physical hardware
> queue.
>
> This qdisc uses the skb->queue_mapping to select which band to place
> the traffic onto. It then uses a round robin w/ a check to see if the
> subqueue is stopped to determine which band to dequeue the packet from.
Mostly looks OK to me, but a few (late) doubts below:
quoted text >
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>
> include/linux/pkt_sched.h | 6 +
> net/sched/Kconfig | 9 +
> net/sched/Makefile | 1
> net/sched/sch_multiq.c | 470 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 486 insertions(+), 0 deletions(-)
> create mode 100644 net/sched/sch_multiq.c
>
> diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
> index e5de421..7fbc952 100644
> --- a/include/linux/pkt_sched.h
> +++ b/include/linux/pkt_sched.h
> @@ -123,6 +123,12 @@ struct tc_prio_qopt
> __u8 priomap[TC_PRIO_MAX+1]; /* Map: logical priority -> PRIO band */
> };
>
> +/* MULTIQ section */
> +
> +struct tc_multiq_qopt {
> + int bands; /* Number of bands */
Probably __u16 or __u32 would look better here.
quoted text > +};
> +
> /* TBF section */
>
> struct tc_tbf_qopt
> diff --git a/net/sched/Kconfig b/net/sched/Kconfig
> index 9437b27..efaa7a7 100644
> --- a/net/sched/Kconfig
> +++ b/net/sched/Kconfig
> @@ -106,6 +106,15 @@ config NET_SCH_PRIO
> To compile this code as a module, choose M here: the
> module will be called sch_prio.
>
> +config NET_SCH_MULTIQ
> + tristate "Hardware Multiqueue-aware Multi Band Queuing (MULTIQ)"
> + ---help---
> + Say Y here if you want to use an n-band queue packet scheduler
> + to support devices that have multiple hardware transmit queues.
> +
> + To compile this code as a module, choose M here: the
> + module will be called sch_multiq.
> +
It would be nice to bring back a few lines about MULTIQ(RR) to
Documentation/networking/multiqueue.txt and mention this here.
quoted text > config NET_SCH_RED
> tristate "Random Early Detection (RED)"
> ---help---
> diff --git a/net/sched/Makefile b/net/sched/Makefile
> index 1d2b0f7..3d9b953 100644
> --- a/net/sched/Makefile
> +++ b/net/sched/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_NET_SCH_SFQ) += sch_sfq.o
> obj-$(CONFIG_NET_SCH_TBF) += sch_tbf.o
> obj-$(CONFIG_NET_SCH_TEQL) += sch_teql.o
> obj-$(CONFIG_NET_SCH_PRIO) += sch_prio.o
> +obj-$(CONFIG_NET_SCH_MULTIQ) += sch_multiq.o
> obj-$(CONFIG_NET_SCH_ATM) += sch_atm.o
> obj-$(CONFIG_NET_SCH_NETEM) += sch_netem.o
> obj-$(CONFIG_NET_CLS_U32) += cls_u32.o
> diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c
> new file mode 100644
> index 0000000..708dd5c
> --- /dev/null
> +++ b/net/sched/sch_multiq.c
> @@ -0,0 +1,470 @@
> +/*
> + * net/sched/sch_multiq.c
> + * This qdisc is based off of the rr qdisc and is meant to
> + * prevent head-of-line blocking on devices that have multiple
> + * hardware queues.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + * Authors: Alexander Duyck <alexander.h.duyck@intel.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/string.h>
> +#include <linux/errno.h>
> +#include <linux/skbuff.h>
> +#include <net/netlink.h>
> +#include <net/pkt_sched.h>
> +
> +
> +struct multiq_sched_data {
> + int bands;
> + int curband;
unsigned etc?
quoted text > + struct tcf_proto *filter_list;
> + struct Qdisc **queues;
> +
A spurious line.
quoted text > +};
...
quoted text > +static void
> +multiq_reset(struct Qdisc *sch)
> +{
> + int band;
> + struct multiq_sched_data *q = qdisc_priv(sch);
> +
> + for (band = 0; band < q->bands; band++)
> + qdisc_reset(q->queues[band]);
> + sch->q.qlen = 0;
+ q->curband = 0; ?
...
quoted text > +static int multiq_tune(struct Qdisc *sch, struct nlattr *opt)
> +{
> + struct multiq_sched_data *q = qdisc_priv(sch);
> + struct tc_multiq_qopt *qopt;
> + struct Qdisc **queues;
> + int i;
> +
> + if (sch->parent != TC_H_ROOT)
> + return -EINVAL;
Is it necessary?
quoted text > + if (!netif_is_multiqueue(qdisc_dev(sch)))
> + return -EINVAL;
> + if (nla_len(opt) < sizeof(*qopt))
> + return -EINVAL;
> +
> + qopt = nla_data(opt);
> +
> + qopt->bands = qdisc_dev(sch)->real_num_tx_queues;
> +
> + queues = kzalloc(sizeof(struct Qdisc *)*qopt->bands, GFP_KERNEL);
kcalloc()?
...
quoted text > +static int multiq_dump(struct Qdisc *sch, struct sk_buff *skb)
> +{
> + struct multiq_sched_data *q = qdisc_priv(sch);
> + unsigned char *b = skb_tail_pointer(skb);
> + struct nlattr *nest;
> + struct tc_multiq_qopt opt;
> +
> + opt.bands = q->bands;
> +
> + nest = nla_nest_compat_start(skb, TCA_OPTIONS, sizeof(opt), &opt);
http://marc.info/?l=linux-netdev&m=121993231608269&w=2
quoted text > + if (nest == NULL)
> + goto nla_put_failure;
> + nla_nest_compat_end(skb, nest);
...
Jarek P.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to
majordomo@vger.kernel.org
More majordomo info at
http://vger.kernel.org/majordomo-info.html
