linux-kernel - RE: [PATCH net-next v18 02/13] rtase: Implement the .ndo

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, 9 May 2024 09:09:00 +0000
From: Ratheesh Kannoth <rkannoth@...vell.com>
To: Justin Lai <justinlai0215@...ltek.com>
CC: "kuba@...nel.org" <kuba@...nel.org>,
        "davem@...emloft.net"
	<davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "jiri@...nulli.us" <jiri@...nulli.us>,
        "horms@...nel.org" <horms@...nel.org>,
        Ping-Ke Shih <pkshih@...ltek.com>, Larry Chiu <larry.chiu@...ltek.com>
Subject: RE: [PATCH net-next v18 02/13] rtase: Implement the .ndo_open
 function

> From: Justin Lai <justinlai0215@...ltek.com>
> Sent: Thursday, May 9, 2024 2:29 PM
> > > +
> > > +     /* rx and tx descriptors needs 256 bytes alignment.
> > > +      * dma_alloc_coherent provides more.
> > > +      */
> > > +     for (i = 0; i < tp->func_tx_queue_num; i++) {
> > > +             tp->tx_ring[i].desc =
> > > +                             dma_alloc_coherent(&pdev->dev,
> > > +
> > RTASE_TX_RING_DESC_SIZE,
> > > +
> > &tp->tx_ring[i].phy_addr,
> > > +                                                GFP_KERNEL);
> > > +             if (!tp->tx_ring[i].desc)
> > You have handled errors gracefully very where else. why not here ?
> 
> I would like to ask you, are you referring to other places where there are error
> description messages, but not here?
other functions, you are freeing allocated resources in case of failure, but here, you are returning error directly.


> > Did you mark the skb for recycle ? Hmm ... did i miss to find the code ?
> >
> We have done this part when using the skb and before finally releasing the skb
> resource. Do you think it would be better to do this part of the process when
> allocating the skb?
i think, you added skb_for_recycle() in the following patch. Sorry I missed it . ignore my comment. 

> 
> > > +
> > > +err_free_all_allocated_irq:
> > You are allocating from i = 1, but freeing from j = 0;
> 
> Hi Ratheesh,
> I have done request_irq() once before the for loop, so there should be no
> problem starting free from j=0 here.
Thanks for pointing out.