linux-kernel - RE: [PATCH net-next v5 10/10] net: axienet: Introduce dmaengine 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]

Message-ID: <MN0PR12MB5953AC3094F6BC7190266104B71CA@MN0PR12MB5953.namprd12.prod.outlook.com>
Date:   Wed, 23 Aug 2023 17:38:58 +0000
From:   "Pandey, Radhey Shyam" <radhey.shyam.pandey@....com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     "vkoul@...nel.org" <vkoul@...nel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "conor+dt@...nel.org" <conor+dt@...nel.org>,
        "Simek, Michal" <michal.simek@....com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "git (AMD-Xilinx)" <git@....com>
Subject: RE: [PATCH net-next v5 10/10] net: axienet: Introduce dmaengine
 support

> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Monday, August 14, 2023 9:00 PM
> To: Pandey, Radhey Shyam <radhey.shyam.pandey@....com>
> Cc: vkoul@...nel.org; robh+dt@...nel.org;
> krzysztof.kozlowski+dt@...aro.org; conor+dt@...nel.org; Simek, Michal
> <michal.simek@....com>; davem@...emloft.net; edumazet@...gle.com;
> pabeni@...hat.com; linux@...linux.org.uk; dmaengine@...r.kernel.org;
> devicetree@...r.kernel.org; linux-arm-kernel@...ts.infradead.org; linux-
> kernel@...r.kernel.org; netdev@...r.kernel.org; git (AMD-Xilinx)
> <git@....com>
> Subject: Re: [PATCH net-next v5 10/10] net: axienet: Introduce dmaengine
> support
> 
> On Sat, 12 Aug 2023 15:27:19 +0000 Pandey, Radhey Shyam wrote:
> > > Drop on error, you're not stopping the queue correctly, just drop, return
> OK
> > > and avoid bugs.
> >
> > As I understand NETDEV_TX_OK returns means driver took care of packet.
> > So inline with non-dmaengine xmit (axienet_start_xmit_legacy) should
> > we stop the queue and return TX_BUSY?
> 
> You should only return BUSY if there is no space. All other errors
> should lead to drops, and increment of tx_error. Otherwise problem
> with handling a single packet may stall the NIC forever.
> It is somewhat confusing that we return TX_OK in that case but it
> is what it is.
> 
> > > Why create a cache ?
> > > Isn't it cleaner to create a fake ring buffer of sgl? Most packets will not
> have
> > > MAX_SKB_FRAGS of memory. On a ring buffer you can use only as many
> sg
> > > entries as the packet requires. Also no need to alloc/free.
> >
> > The kmem_cache is used with intent to use slab cache interface and
> > make use of reusing objects in the kernel. slab cache maintains a
> > cache of objects. When we free an object, instead of
> > deallocating it, it give it back to the cache. Next time, if we
> > want to create a new object, slab cache gives us one object from the
> > slab cache.
> >
> > If we maintain custom circular buffer (struct circ_buf) ring buffer
> > we have to create two such ring buffers one for TX and other for RX.
> > For multichannel this will multiply to * no of queues. Also we have to
> > ensure proper occupancy checks and head/tail pointer updates.
> >
> > With kmem_cache pool we are offloading queue maintenance ops to
> > framework with a benefit of optimized alloc/dealloc. Let me know if it
> > looks functionally fine and can retain it for this baseline dmaengine
> > support version?
> 
> The kmemcache is not the worst possible option but note that the
> objects you're allocating (with zeroing) are 512+ bytes. That's
> pretty large, when most packets will not have full 16 fragments.
> Ring buffer would allow to better match the allocation size to
> the packets. Not to mention that it can be done fully locklessly.

I modified the implementation to use a circular ring buffer for TX
and RX. It seems to be working in initial testing and now running 
perf tests.

Just had one question on when to submit v6 ? Wait till dmaengine
patches([01/10-[07/10] is part of net-next? Or can I send it now also.

Thanks,
Radhey