netdev - Re: [PATCH v2] net: virtio_net: notifications coalescing support

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]

Date:   Thu, 14 Jul 2022 15:02:13 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Alvaro Karsz <alvaro.karsz@...id-run.com>
Cc:     Jakub Kicinski <kuba@...nel.org>, netdev <netdev@...r.kernel.org>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH v2] net: virtio_net: notifications coalescing support

On Thu, Jul 14, 2022 at 2:54 PM Alvaro Karsz <alvaro.karsz@...id-run.com> wrote:
>
> Thanks Jakub and Jason.
>
> > I think we need to return -EBUSY here regardless whether or not
> > interrupt coalescing is negotiated.
>
>
> The part you are referring to is relevant only if we are going to update NAPI.
> Jakub suggested splitting the function into 2 cases.
>
> If interrupt coalescing is negotiated:
>  Send control commands to the device.
> Otherwise:
>  Update NAPI.
>
> So this is not relevant if interrupt coalescing is negotiated.
> You don't think that we should separate the function into 2 different cases?

So we use sq->napi.weight as a hint to use tx interrupt or not.

We need a safe switching from tx interrupt and skb_orphan(). The
current code guarantees this by only allowing the switching when the
interface is down.

So what I meant for the above "Update NAPI" is, consider that users
want to switch from tx_max_coalesced_frames from 0 to 100. This needs
to be down when the interface is down, since the driver need to enable
tx interrupt mode, otherwise the coalescing is meaningless.

This would be much easier if we only have tx interrupt mode, but this
requires more work.

>
>
> Or maybe I misunderstood you, and you are not referring to the following part:
> > +                             if (!notf_coal)
> > +                                     return -EBUSY;
> > +
> > +                             goto exit;
>
> But you are referring to the whole virtnet_set_coalesce function in general.

See above.

Thanks

>
>
> Alvaro.
>