netdev - Re: [PATCH net-next] ravb: Remove rx buffer ALIGN

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: <CAH1o70L+Dy845CZ4rP7AbFL779o6YtrWu9jp0nVRfWVmkWb93w@mail.gmail.com>
Date:	Sun, 1 May 2016 23:29:29 +0900
From:	Yoshihiro Kaneko <ykaneko0929@...il.com>
To:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
Cc:	netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
	Simon Horman <horms@...ge.net.au>,
	Magnus Damm <magnus.damm@...il.com>,
	linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH net-next] ravb: Remove rx buffer ALIGN

Hi Sergei,

Sorry for the late reply.

2016-04-27 4:14 GMT+09:00 Sergei Shtylyov <sergei.shtylyov@...entembedded.com>:
> Hello.
>
> On 04/24/2016 07:16 PM, Yoshihiro Kaneko wrote:
>
>> From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@...esas.com>
>>
>> Aligning the reception data size is not required.
>
>
>    OK, the gen 2/3 manuals indeed don't require this. I assume the patch has
> been tested...
>
>> Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@...esas.com>
>> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@...il.com>
>
>
>    I have a few comments though...
>
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
>> b/drivers/net/ethernet/renesas/ravb_main.c
>> index 238b56f..66ed80c 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -246,10 +246,10 @@ static void ravb_ring_format(struct net_device
>> *ndev, int q)
>>         for (i = 0; i < priv->num_rx_ring[q]; i++) {
>>                 /* RX descriptor */
>>                 rx_desc = &priv->rx_ring[q][i];
>> -               /* The size of the buffer should be on 16-byte boundary.
>> */
>> -               rx_desc->ds_cc = cpu_to_le16(ALIGN(PKT_BUF_SZ, 16));
>> -               dma_addr = dma_map_single(ndev->dev.parent,
>> priv->rx_skb[q][i]->data,
>> -                                         ALIGN(PKT_BUF_SZ, 16),
>> +               rx_desc->ds_cc = cpu_to_le16(PKT_BUF_SZ);
>> +               dma_addr = dma_map_single(ndev->dev.parent,
>> +                                         priv->rx_skb[q][i]->data,
>
>
>    Please don't reformat the lines above.

Got it.

>
>> +                                         PKT_BUF_SZ,
>>                                           DMA_FROM_DEVICE);
>>                 /* We just set the data size to 0 for a failed mapping
>> which
>>                  * should prevent DMA from happening...
>> @@ -557,8 +557,9 @@ static bool ravb_rx(struct net_device *ndev, int
>> *quota, int q)
>>
>>                         skb = priv->rx_skb[q][entry];
>>                         priv->rx_skb[q][entry] = NULL;
>> -                       dma_unmap_single(ndev->dev.parent,
>> le32_to_cpu(desc->dptr),
>> -                                        ALIGN(PKT_BUF_SZ, 16),
>> +                       dma_unmap_single(ndev->dev.parent,
>> +                                        le32_to_cpu(desc->dptr),
>
>
>   Same here.

Likewise.

>
> [...]
>
> MBR, Sergei
>

Thanks,
kaneko