netdev - Re: [PATCH v5 2/2] virtio-net: use mtu size as buffer length for big packets

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: <20220922052233-mutt-send-email-mst@kernel.org>
Date:   Thu, 22 Sep 2022 05:26:51 -0400
From:   "Michael S. Tsirkin" <mst@...hat.com>
To:     Parav Pandit <parav@...dia.com>
Cc:     Gavin Li <gavinl@...dia.com>,
        "stephen@...workplumber.org" <stephen@...workplumber.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "jesse.brandeburg@...el.com" <jesse.brandeburg@...el.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "sridhar.samudrala@...el.com" <sridhar.samudrala@...el.com>,
        "jasowang@...hat.com" <jasowang@...hat.com>,
        "loseweigh@...il.com" <loseweigh@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "virtio-dev@...ts.oasis-open.org" <virtio-dev@...ts.oasis-open.org>,
        Gavi Teitz <gavi@...dia.com>,
        Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
        Si-Wei Liu <si-wei.liu@...cle.com>
Subject: Re: [PATCH v5 2/2] virtio-net: use mtu size as buffer length for big
 packets

On Wed, Sep 07, 2022 at 07:51:38PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@...hat.com>
> > Sent: Wednesday, September 7, 2022 3:36 PM
> > 
> > On Wed, Sep 07, 2022 at 07:27:16PM +0000, Parav Pandit wrote:
> > >
> > > > From: Michael S. Tsirkin <mst@...hat.com>
> > > > Sent: Wednesday, September 7, 2022 3:24 PM
> > > >
> > > > On Wed, Sep 07, 2022 at 07:18:06PM +0000, Parav Pandit wrote:
> > > > >
> > > > > > From: Michael S. Tsirkin <mst@...hat.com>
> > > > > > Sent: Wednesday, September 7, 2022 3:12 PM
> > > > >
> > > > > > > Because of shallow queue of 16 entries deep.
> > > > > >
> > > > > > but why is the queue just 16 entries?
> > > > > I explained the calculation in [1] about 16 entries.
> > > > >
> > > > > [1]
> > > > >
> > > >
> > https://lore.kernel.org/netdev/PH0PR12MB54812EC7F4711C1EA4CAA119DC
> > > > 419@
> > > > > PH0PR12MB5481.namprd12.prod.outlook.com/
> > > > >
> > > > > > does the device not support indirect?
> > > > > >
> > > > > Yes, indirect feature bit is disabled on the device.
> > > >
> > > > OK that explains it.
> > >
> > > So can we proceed with v6 to contain
> > > (a) updated commit message and
> > > (b) function name change you suggested to drop _fields suffix?
> > 
> > (c) replace mtu = 0 with sensibly not calling the function when mtu is
> > unknown.
> 
> > 
> > 
> > And I'd like commit log to include results of perf testing
> > - with indirect feature on
> Which device do you suggest using for this test?

AFAIK most devices support INDIRECT, e.g. don't nvidia cards do this?


> > - with mtu feature off
> Why is this needed when it is not touching the area of mtu being not offered?

I don't really like it that instead of checking the MTU feature bit
everywhere the patch sets mtu variable to 0. Because of this
it wasn't all that obvious that the patch did not affect !MTU
performance (the code does change).

Rereading it afresh I think it's ok. But explicit check for !MTU
would be better imho making it obvious we do not need to test !MTU.

-- 
MST