netdev - Re: [PATCH] net: add support for STMicroelectronics Ethernet controllers.

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: <4AD54E71.7070400@gmail.com>
Date:	Wed, 14 Oct 2009 06:07:13 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Giuseppe CAVALLARO <peppe.cavallaro@...com>
CC:	netdev@...r.kernel.org
Subject: Re: [PATCH] net: add support for STMicroelectronics Ethernet controllers.

Giuseppe CAVALLARO a écrit :
> Hi Eric,
> 
> Giuseppe CAVALLARO wrote:
>> I'm going to post a new patch for the stmmac as soon as I fix all these
>> points.
> 
> Hoping to have well followed all your advice, I'm attaching the
> patch again.
> 
> As you will see, I've reviewed the tx clean process by removing the
> wrong check that improperly impacted the poll logic.
> I've noticed an improvement in terms of performances as well (*).
> Taking as example the tg3 driver, I've reviewed and modified the
> locking mechanism (fixing the issue on tso you had raised).
> 
> Please, let me know if you have other suggestions and/or advice.
> 
> Regards,
> Peppe
> 
> 
> P.S. (*)
> I've just done some stress tests on our STB (mb618 - STi7111 SH4-300
> @450MHz) and I cannot see any failures or strange issues at this time.
> 

Hi Giuseppe

I reviewed your code and found no obvious issues

Could you please avoid defining this

#define STMMAC_IP_ALIGN NET_IP_ALIGN

(I see tg3.c uses a similar TG3_RAW_IP_ALIGN, this is probably why you felt it was necessary :
In case of tg3, TG3_RAW_IP_ALIGN is used in cases where we always want to align the
IP/network header on dword boundaries, even on platforms where NET_IP_ALIGN)

I am not sure this is what you want for STM.

In that case, please check new netdev_alloc_skb_ip_align() helper

+		skb = netdev_alloc_skb(dev, bfsize);
+		if (unlikely(skb == NULL)) {
+			pr_err("%s: Rx init fails; skb is NULL\n",
+			       __func__);
+			break;
+		}
+		skb_reserve(skb, STMMAC_IP_ALIGN);
+
+		priv->rx_skbuff[i] = skb;
+		priv->rx_skbuff_dma[i] = dma_map_single(priv->device,
+						skb->data,
+						bfsize - STMMAC_IP_ALIGN,
+						DMA_FROM_DEVICE);

becomes  (pktsize being real packet size, not packet size + NET_IP_ALIGN)

+		skb = netdev_alloc_skb_ip_align(dev, pktsize);
+		if (unlikely(skb == NULL)) {
+			pr_err("%s: Rx init fails; skb is NULL\n",
+			       __func__);
+			break;
+		}
+
+		priv->rx_skbuff[i] = skb;
+		priv->rx_skbuff_dma[i] = dma_map_single(priv->device,
+						skb->data,
+						pktsize,
+						DMA_FROM_DEVICE);


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html