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] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANn89iK4H5nBurGPtkaXdMX5jA-H29X6OVC7V6AAEDTW8Q3=rA@mail.gmail.com>
Date: Mon, 2 Oct 2023 14:23:38 +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 3/4] net_sched: sch_fq: add 3 bands and WRR scheduling

On Mon, Oct 2, 2023 at 1:46 PM Toke Høiland-Jørgensen <toke@...hat.com> wrote:
>
> Eric Dumazet <edumazet@...gle.com> writes:
>
> > Before Google adopted FQ for its production servers,
> > we had to ensure AF4 packets would get a higher share
> > than BE1 ones.
> >
> > As discussed this week in Netconf 2023 in Paris, it is time
> > to upstream this for public use.
>
> IIRC, when you mentioned this at Netconf you said the new behaviour
> would probably need to be behind a flag, but I don't see that in this
> series. What was the reason you decided to drop that?


Not a flag, this would add runtime costs.
"struct fq_sched_data" is very big and I try not adding fields unless
really necessary.

I mentioned at Netconf that we had been using this WRR mode for ~5 years
and without a flag.


>
> [..]
> > +static int fq_load_priomap(struct fq_sched_data *q,
> > +                        const struct nlattr *attr,
> > +                        struct netlink_ext_ack *extack)
> > +{
> > +     const struct tc_prio_qopt *map = nla_data(attr);
> > +     int i;
> > +
> > +     if (map->bands != FQ_BANDS) {
> > +             NL_SET_ERR_MSG_MOD(extack, "FQ only supports 3 bands");
> > +             return -EINVAL;
> > +     }
> > +     for (i = 0; i < TC_PRIO_MAX + 1; i++) {
> > +             if (map->priomap[i] >= FQ_BANDS) {
> > +                     NL_SET_ERR_MSG_MOD(extack, "Incorrect field in FQ priomap");
>
> Can we be a bit more specific than just "incorrect" here? Something like
> "FQ priomap field %d maps to a too high band %d"?

Maybe, but note sch_prio does not even set extack for this case.

This is mostly something that only fuzzers like syzbot could possibly
hit, iproute2 will not feed the kernel with such invalid values.

Powered by blists - more mailing lists