netdev - Re: [PATCH net-next 4/4] net_sched: sch_fq: add TCA_FQ

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Mon, 2 Oct 2023 14:24:22 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Toke Høiland-Jørgensen <toke@...hat.com>
Cc: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, Willem de Bruijn <willemb@...gle.com>, 
	Soheil Hassas Yeganeh <soheil@...gle.com>, Neal Cardwell <ncardwell@...gle.com>, 
	Jamal Hadi Salim <jhs@...atatu.com>, Cong Wang <xiyou.wangcong@...il.com>, 
	Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org, eric.dumazet@...il.com
Subject: Re: [PATCH net-next 4/4] net_sched: sch_fq: add TCA_FQ_WEIGHTS attribute

On Mon, Oct 2, 2023 at 1:47 PM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>
> Eric Dumazet <edumazet@...gle.com> writes:
>
> > This attribute can be used to tune the per band weight
> > and report them in "tc qdisc show" output:
> >
> > qdisc fq 802f: parent 1:9 limit 100000p flow_limit 500p buckets 1024 orphan_mask 1023
> >  quantum 8364b initial_quantum 41820b low_rate_threshold 550Kbit
> >  refill_delay 40ms timer_slack 10us horizon 10s horizon_drop
> >  bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 weights 589824 196608 65536
> >  Sent 236460814 bytes 792991 pkt (dropped 0, overlimits 0 requeues 0)
> >  rate 25816bit 10pps backlog 0b 0p requeues 0
> >   flows 4 (inactive 4 throttled 0)
> >   gc 0 throttled 19 latency 17.6us fastpath 773882
> >
> > Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> > ---
> >  include/uapi/linux/pkt_sched.h |  3 +++
> >  net/sched/sch_fq.c             | 32 ++++++++++++++++++++++++++++++++
> >  2 files changed, 35 insertions(+)
> >
> > diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
> > index ec5ab44d41a2493130670870dc9e68c71187740f..f762a10bfb78ed896d8a5b936045a956d97b3831 100644
> > --- a/include/uapi/linux/pkt_sched.h
> > +++ b/include/uapi/linux/pkt_sched.h
> > @@ -943,12 +943,15 @@ enum {
> >
> >       TCA_FQ_PRIOMAP,         /* prio2band */
> >
> > +     TCA_FQ_WEIGHTS,         /* Weights for each band */
> > +
> >       __TCA_FQ_MAX
> >  };
> >
> >  #define TCA_FQ_MAX   (__TCA_FQ_MAX - 1)
> >
> >  #define FQ_BANDS 3
> > +#define FQ_MIN_WEIGHT 16384
> >
> >  struct tc_fq_qd_stats {
> >       __u64   gc_flows;
> > diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
> > index 1bae145750a66f769bd30f1db09203f725801249..1a411fe36c79a86635f319c230a045d653571700 100644
> > --- a/net/sched/sch_fq.c
> > +++ b/net/sched/sch_fq.c
> > @@ -919,6 +919,10 @@ static const struct nla_policy fq_policy[TCA_FQ_MAX + 1] = {
> >                       .type = NLA_BINARY,
> >                       .len = sizeof(struct tc_prio_qopt),
> >               },
> > +     [TCA_FQ_WEIGHTS]                = {
> > +                     .type = NLA_BINARY,
> > +                     .len = FQ_BANDS * sizeof(s32),
> > +             },
> >  };
> >
> >  /* compress a u8 array with all elems <= 3 to an array of 2-bit fields */
> > @@ -941,6 +945,24 @@ static void fq_prio2band_decompress_crumb(const u8 *in, u8 *out)
> >               out[i] = fq_prio2band(in, i);
> >  }
> >
> > +static int fq_load_weights(struct fq_sched_data *q,
> > +                        const struct nlattr *attr,
> > +                        struct netlink_ext_ack *extack)
> > +{
> > +     s32 *weights = nla_data(attr);
> > +     int i;
> > +
> > +     for (i = 0; i < FQ_BANDS; i++) {
> > +             if (weights[i] < FQ_MIN_WEIGHT) {
> > +                     NL_SET_ERR_MSG_MOD(extack, "Incorrect weight");
>
> As in the previous patch, can we be a bit more specific here? "Weight %d
> less that minimum allowed %d"?

I guess I can do this. Again this is to prevent syzbot from doing bad things.