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: <CAF=yD-+exVoOBQpd35pvTnDEWG1oMn3AxRvTmYo_pWR4Ptz-GQ@mail.gmail.com>
Date:   Mon, 23 Jul 2018 21:23:09 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     caleb.raitto@...il.com, "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        David Miller <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>,
        caraitto@...gle.com
Subject: Re: [PATCH net-next] virtio_net: force_napi_tx module param.

On Mon, Jul 23, 2018 at 8:55 PM Stephen Hemminger
<stephen@...workplumber.org> wrote:
>
> On Mon, 23 Jul 2018 16:11:19 -0700
> Caleb Raitto <caleb.raitto@...il.com> wrote:
>
> > From: Caleb Raitto <caraitto@...gle.com>
> >
> > The driver disables tx napi if it's not certain that completions will
> > be processed affine with tx service.
> >
> > Its heuristic doesn't account for some scenarios where it is, such as
> > when the queue pair count matches the core but not hyperthread count.
> >
> > Allow userspace to override the heuristic. This is an alternative
> > solution to that in the linked patch. That added more logic in the
> > kernel for these cases, but the agreement was that this was better left
> > to user control.
> >
> > Do not expand the existing napi_tx variable to a ternary value,
> > because doing so can break user applications that expect
> > boolean ('Y'/'N') instead of integer output. Add a new param instead.
> >
> > Link: https://patchwork.ozlabs.org/patch/725249/
> > Acked-by: Willem de Bruijn <willemb@...gle.com>
> > Acked-by: Jon Olson <jonolson@...gle.com>
> > Signed-off-by: Caleb Raitto <caraitto@...gle.com>
> > ---
>
> Not a fan of this.
> Module parameters are frowned on by the distributions because they
> never get well tested and they force the user to do magic things
> to enable features. It looks like you are using it to paper
> over a bug in this case.

This has actually been a catch-22 that this patch tries to break.

In micro benchmarks napi-tx was an improvement for most cases. We need
wider validation to make it default. Or even to start enabling it for some
users. But we cannot get the data, because understandably no one is
going to make it default without more data.

Enabling the feature selectively to safely roll out is the intent of
(temporary) param napi_tx. But the requirement to have
has_affinity_set is proving a real obstruction.

Especially in cases where we enable the feature to do A:B comparisons,
and thus are monitoring performance metrics closely, we should be able
to override the kernel heuristic.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ