[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <201007201028.12470.sven.eckelmann@gmx.de>
Date: Tue, 20 Jul 2010 03:28:10 -0500
From: "Sven Eckelmann" <sven.eckelmann@....de>
To: <b.a.t.m.a.n@...ts.open-mesh.org>,
"Simon Wunderlich" <siwu@....tu-chemnitz.de>
Cc: "David Miller" <davem@...emloft.net>, <netdev@...r.kernel.org>
Subject: Re: [PATCH] net: Add batman-adv meshing protocol
Thanks a lot for your review and for your hints.
David Miller wrote:
> From: Sven Eckelmann <sven.eckelmann@....de>
> Date: Fri, 16 Jul 2010 16:39:16 +0200
[...]
>
> The kernel has a hamming weight library function which takes advantage
> of population count instructions on cpus that suport it, and also has
> a sw version than is faster than what you're doing here, please use
> it.
>
> The interfaces are called "hweight{8,16,32,64}()" where the number in
> the name indicates the bit-size of the word the interface operates on.
Correct, the inner loop is a quite straight forward implementation without any
kind of optimization. I will change that.
> I also notice that this code uses it's own internal buffering scheme
> with kmalloc()'d buffers, then seperately allocates actual SKB's and
> copies the data there.
>
> Just use the SKB facilities how they were designed to be used, instead
> of needlessly inventing new things. Allocate your initial SKB and put
> the initial forwarding header in it, then when you want to send a copy
> off, skb_clone() it, and push the other bits you want at the head
> and/or the tail of the cloned SKB, then simply send it off.
Good catch. That comes from a time when batman-adv was a minimalistic
conversation of the userspace proof of concept implementation. This happens
for example in vis.c, icmp_socket.c and send.c (just grepping for
send_raw_packet is a good way to find those places). But is also happening
with batman_if->packet_buff in schedule_own_packet and similar places.
I would leave that to the original author of those functions.
thanks,
Sven
Download attachment "winmail.dat" of type "application/ms-tnef" (4977 bytes)
Powered by blists - more mailing lists