netdev - RE: [PATCH net-next 5/6] drivers: net: xgene-v2: Add transmit and receive

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: <063D6719AE5E284EB5DD2968C1650D6DB0277BB0@AcuExch.aculab.com>
Date:   Wed, 1 Feb 2017 11:09:44 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Florian Fainelli' <f.fainelli@...il.com>,
        Iyappan Subramanian <isubramanian@....com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "patches@....com" <patches@....com>,
        "kchudgar@....com" <kchudgar@....com>
Subject: RE: [PATCH net-next 5/6] drivers: net: xgene-v2: Add transmit and
 receive

From Florian Fainelli
> Sent: 31 January 2017 20:33
> On 01/31/2017 11:03 AM, Iyappan Subramanian wrote:
> > This patch adds,
> >     - Transmit
> >     - Transmit completion poll
> >     - Receive poll
> >     - NAPI handler
> >
> > and enables the driver.
> >
> > Signed-off-by: Iyappan Subramanian <isubramanian@....com>
> > Signed-off-by: Keyur Chudgar <kchudgar@....com>
> > ---
> 
> > +
> > +	tx_ring = pdata->tx_ring;
> > +	tail = tx_ring->tail;
> > +	len = skb_headlen(skb);
> > +	raw_desc = &tx_ring->raw_desc[tail];
> > +
> > +	/* Tx descriptor not available */
> > +	if (!GET_BITS(E, le64_to_cpu(raw_desc->m0)) ||
> > +	    GET_BITS(PKT_SIZE, le64_to_cpu(raw_desc->m0)))
> > +		return NETDEV_TX_BUSY;

Aren't you supposed to detect 'ring full' and stop the code
giving you packets to transmit.

> > +
> > +	/* Packet buffers should be 64B aligned */

Is that really a requirement of the hardware?
Almost all ethernet frames are 4n+2 aligned.

> > +	pkt_buf = dma_alloc_coherent(dev, XGENE_ENET_STD_MTU, &dma_addr,
> > +				     GFP_ATOMIC);
> > +	if (unlikely(!pkt_buf))
> > +		goto out;
> 
> Can't you obtain a DMA-API mapping for skb->data and pass it down to the
> hardware? This copy here is inefficient.
> 
> > +
> > +	memcpy(pkt_buf, skb->data, len);

You really need to verify that the len <= XGENE_ENET_STD_MTU.

Isn't this code only transmitting the 'head' of the packet?
What about the fragments??
...
	David