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

[Eric Dumazet <eric.dumazet@gmail.com>]

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);