linux-kernel - Re: [PATCH net] amd/lance.c: dev_alloc_skb to netdev_alloc

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: <CA+Dx6Ha4dnqnkR_Pc+dFZ+Ef_mxRNN_Jk44vJ_R0viuHd0JGHw@mail.gmail.com>
Date:	Thu, 26 Jan 2012 01:31:45 +0530
From:	"Pradeep A. Dalvi" <netdev@...deepdalvi.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] amd/lance.c: dev_alloc_skb to netdev_alloc_skb

On Thu, Jan 26, 2012 at 12:25 AM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> Le jeudi 26 janvier 2012 à 00:16 +0530, Pradeep A. Dalvi a écrit :
>> Replaced deprecating dev_alloc_skb with netdev_alloc_skb for amd/lance.c
>>   - Removed extra skb->dev = dev
>>
>> Signed-off-by: Pradeep A. Dalvi <netdev@...deepdalvi.com>
>> ---
>>  drivers/net/ethernet/amd/lance.c |    9 ++++-----
>>  1 files changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/amd/lance.c b/drivers/net/ethernet/amd/lance.c
>> index a6e2e84..a8a6b45 100644
>> --- a/drivers/net/ethernet/amd/lance.c
>> +++ b/drivers/net/ethernet/amd/lance.c
>> @@ -871,12 +871,11 @@ lance_init_ring(struct net_device *dev, gfp_t gfp)
>>               struct sk_buff *skb;
>>               void *rx_buff;
>>
>> -             skb = alloc_skb(PKT_BUF_SZ, GFP_DMA | gfp);
>> +             skb = __netdev_alloc_skb(dev, PKT_BUF_SZ, GFP_DMA | gfp);
>
> Nack
>
> As I already explained, alloc_skb() and netdev_alloc_skb() are
> different.
>
> Why do you repost this ?
>
> nedev_alloc_skb() adds NET_SKB_PAD bytes.
>
> This driver doesnt need this extra space.
>
> (In fact it could use kmalloc(), since the skb themselves are not used
> at all : At RX time, we perform a copybreak)

Thanks Eric! Even if we try to skip NET_SKB_PAD i.e. max of 32 or
L1_CACHE_BYTES, SKB_DATA_ALIGN is anyway going to add some additional
bytes for the same reason.

And towards the end, as from my current understanding regarding skb
allocations, for MTU including alignments < 2048, there is not much to
worry about. As long as, it doesn't cross the boundary.

(And in fact, this driver doesn't even get compiled with current
configs, even after forcefully adding CONFIG_ISA & CONFIG_LANCE, of
course along with CONFIG_ISA_DMA_API. Had to change the Makefile)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/