[PATCH] xgmac: use bufsz not dma_buf_sz in rx_refill

[PATCH] xgmac: use bufsz not dma_buf_sz in rx_refill

[Kyle McMartin]

Noticed while debugging another issue that DMA debug turned up, looks
like commit ef468d23 missed a case when switching to bufsz instead of
priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly
suspect since we're not tracking the size, but trusting the hardware
between map and unmap...)

Signed-off-by: Kyle McMartin <kyle@redhat.com>

--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -686,7 +686,7 @@ static void xgmac_rx_refill(struct xgmac_priv *priv)
 			priv->rx_skbuff[entry] = skb;
 			paddr = dma_map_single(priv->device, skb->data,
 					       bufsz, DMA_FROM_DEVICE);
-			desc_set_buf_addr(p, paddr, priv->dma_buf_sz);
+			desc_set_buf_addr(p, paddr, bufsz);
 		}
 
 		netdev_dbg(priv->dev, "rx ring: head %d, tail %d\n",

Re: [PATCH] xgmac: use bufsz not dma_buf_sz in rx_refill

[Rob Herring]

On 08/29/2013 03:11 PM, Kyle McMartin wrote:
> Noticed while debugging another issue that DMA debug turned up, looks
> like commit ef468d23 missed a case when switching to bufsz instead of
> priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly
> suspect since we're not tracking the size, but trusting the hardware
> between map and unmap...)

Please see my patch series of fixes:

http://www.spinics.net/lists/netdev/msg248076.html

> Signed-off-by: Kyle McMartin <kyle@redhat.com>
> 
> --- a/drivers/net/ethernet/calxeda/xgmac.c
> +++ b/drivers/net/ethernet/calxeda/xgmac.c
> @@ -686,7 +686,7 @@ static void xgmac_rx_refill(struct xgmac_priv *priv)
>  			priv->rx_skbuff[entry] = skb;
>  			paddr = dma_map_single(priv->device, skb->data,
>  					       bufsz, DMA_FROM_DEVICE);
> -			desc_set_buf_addr(p, paddr, priv->dma_buf_sz);
> +			desc_set_buf_addr(p, paddr, bufsz);

This is not right because the h/w wants the size to include the 2 bytes
skipped at the beginning for NET_IP_ALIGN. The h/w may also corrupt 1-7
bytes of data at the head or tail if the frame is not 8 byte aligned and
padded. We are relying that skb allocations are aligned.

Rob

Re: [PATCH] xgmac: use bufsz not dma_buf_sz in rx_refill

[Kyle McMartin]

On Thu, Aug 29, 2013 at 03:24:30PM -0500, Rob Herring wrote:
> On 08/29/2013 03:11 PM, Kyle McMartin wrote:
> > Noticed while debugging another issue that DMA debug turned up, looks
> > like commit ef468d23 missed a case when switching to bufsz instead of
> > priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly
> > suspect since we're not tracking the size, but trusting the hardware
> > between map and unmap...)
> 
> Please see my patch series of fixes:
> 
> http://www.spinics.net/lists/netdev/msg248076.html
> 

Hah, thanks. Yeah, I wondered wtf was up with get_buf_len, but figured
the hardware must have been doing something subtle. ;-)

I'll apply these to our Fedora tree and pass them along for testing.

Thanks Rob.

--Kyle

Re: [PATCH] xgmac: use bufsz not dma_buf_sz in rx_refill

[Sergei Shtylyov]

Hello.

On 08/30/2013 12:11 AM, Kyle McMartin wrote:

> Noticed while debugging another issue that DMA debug turned up, looks
> like commit ef468d23 missed a case when switching to bufsz instead of

    Please also specify that commit's summary line in parens.

> priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly
> suspect since we're not tracking the size, but trusting the hardware
> between map and unmap...)

> Signed-off-by: Kyle McMartin <kyle@redhat.com>

WBR, Sergei

Re: [PATCH] xgmac: use bufsz not dma_buf_sz in rx_refill

[David Miller]

From: Kyle McMartin <kyle@redhat.com>
Date: Thu, 29 Aug 2013 16:11:51 -0400

> Noticed while debugging another issue that DMA debug turned up, looks
> like commit ef468d23 missed a case when switching to bufsz instead of
> priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly
> suspect since we're not tracking the size, but trusting the hardware
> between map and unmap...)
> 
> Signed-off-by: Kyle McMartin <kyle@redhat.com>

Applied and queued up for -stable, thanks.

Re: [PATCH] xgmac: use bufsz not dma_buf_sz in rx_refill

[Kyle McMartin]

On Thu, Aug 29, 2013 at 04:34:37PM -0400, David Miller wrote:
> From: Kyle McMartin <kyle@redhat.com>
> Date: Thu, 29 Aug 2013 16:11:51 -0400
> 
> > Noticed while debugging another issue that DMA debug turned up, looks
> > like commit ef468d23 missed a case when switching to bufsz instead of
> > priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly
> > suspect since we're not tracking the size, but trusting the hardware
> > between map and unmap...)
> > 
> > Signed-off-by: Kyle McMartin <kyle@redhat.com>
> 
> Applied and queued up for -stable, thanks.

Heh, too fast David, Rob nak'd it. :)

Sorry about that.

--Kyle

Re: [PATCH] xgmac: use bufsz not dma_buf_sz in rx_refill

[David Miller]

From: Rob Herring <rob.herring@calxeda.com>
Date: Thu, 29 Aug 2013 15:24:30 -0500

> On 08/29/2013 03:11 PM, Kyle McMartin wrote:
>> Noticed while debugging another issue that DMA debug turned up, looks
>> like commit ef468d23 missed a case when switching to bufsz instead of
>> priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly
>> suspect since we're not tracking the size, but trusting the hardware
>> between map and unmap...)
> 
> Please see my patch series of fixes:
> 
> http://www.spinics.net/lists/netdev/msg248076.html

Which need to be resubmitted with feedback intergrated.

I knew your series existed, but Kyle submitted a fix in the correct
format meanwhile so I applied it.

Re: [PATCH] xgmac: use bufsz not dma_buf_sz in rx_refill

[David Miller]

From: Kyle McMartin <kmcmarti@redhat.com>
Date: Thu, 29 Aug 2013 16:35:23 -0400

> On Thu, Aug 29, 2013 at 04:34:37PM -0400, David Miller wrote:
>> From: Kyle McMartin <kyle@redhat.com>
>> Date: Thu, 29 Aug 2013 16:11:51 -0400
>> 
>> > Noticed while debugging another issue that DMA debug turned up, looks
>> > like commit ef468d23 missed a case when switching to bufsz instead of
>> > priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly
>> > suspect since we're not tracking the size, but trusting the hardware
>> > between map and unmap...)
>> > 
>> > Signed-off-by: Kyle McMartin <kyle@redhat.com>
>> 
>> Applied and queued up for -stable, thanks.
> 
> Heh, too fast David, Rob nak'd it. :)
> 
> Sorry about that.

If Rob had resubmitted his series in a timely manner this wouldn't
have happened.

I reverted.

Re: [PATCH] xgmac: use bufsz not dma_buf_sz in rx_refill

[Rob Herring]

On 08/29/2013 03:38 PM, David Miller wrote:
> From: Kyle McMartin <kmcmarti@redhat.com>
> Date: Thu, 29 Aug 2013 16:35:23 -0400
> 
>> On Thu, Aug 29, 2013 at 04:34:37PM -0400, David Miller wrote:
>>> From: Kyle McMartin <kyle@redhat.com>
>>> Date: Thu, 29 Aug 2013 16:11:51 -0400
>>>
>>>> Noticed while debugging another issue that DMA debug turned up, looks
>>>> like commit ef468d23 missed a case when switching to bufsz instead of
>>>> priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly
>>>> suspect since we're not tracking the size, but trusting the hardware
>>>> between map and unmap...)
>>>>
>>>> Signed-off-by: Kyle McMartin <kyle@redhat.com>
>>>
>>> Applied and queued up for -stable, thanks.
>>
>> Heh, too fast David, Rob nak'd it. :)
>>
>> Sorry about that.
> 
> If Rob had resubmitted his series in a timely manner this wouldn't
> have happened.

I was giving them more time hoping to have some feedback from Lennert.
Anyways, I've just resubmitted v2 which addresses Ben's comments.

Rob