Hi,
first thing: thanks for your help to get some review for batman-adv from davem and the netdev ml. I've added some new points to the TODO list. They are partly resolved or work is done to solve them, but will postpone them and get them hopefully ready and tested for 2.6.37.
Nevertheless there are some smaller changes which want to resolve with 2.6.36. One is to allow to build it together with dt3155 directly into the kernel and the other one should fix that we try to change stats of a different device. The latter one will clash with an unlucky try by the netdev guys to change the behaviour of dev_get_stats and the batman-adv source code.
This merge conflict is quite easy to resolve. Just delete the dev_get_stats line in hard-interface.c in batman_skb_recv and the new variable temp added by them.
I can also provide a patch on top of linux-next instead of staging-next-2.6 if you would prefer it in this situation. I will call it "[PATCH-next 1/3] Staging: batman-adv: Don't increment stats of foreign device"
I hope that it doesn't create more confusion as necessary. :)
thanks, Sven
Randy Dunlap (1): Staging: batman-adv: don't use default init_module/cleanup_module function names
Sven Eckelmann (2): Staging: batman-adv: Don't increment stats of foreign device Staging: batman-adv: Update TODO with new points from review
drivers/staging/batman-adv/TODO | 9 ++++++--- drivers/staging/batman-adv/hard-interface.c | 7 ------- drivers/staging/batman-adv/main.c | 7 +++++-- 3 files changed, 11 insertions(+), 12 deletions(-)
The receive hook for batman-adv ethernet frames tried to get the last device which processed the skb before us. It only used that information to update the rx_bytes and rx_packets stat of that foreign device which already has updated it using its own receive functions.
Signed-off-by: Sven Eckelmann sven.eckelmann@gmx.de --- drivers/staging/batman-adv/hard-interface.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/batman-adv/hard-interface.c b/drivers/staging/batman-adv/hard-interface.c index 5cc369c..f6345c4 100644 --- a/drivers/staging/batman-adv/hard-interface.c +++ b/drivers/staging/batman-adv/hard-interface.c @@ -442,7 +442,6 @@ int batman_skb_recv(struct sk_buff *skb, struct net_device *dev, struct bat_priv *bat_priv = netdev_priv(soft_device); struct batman_packet *batman_packet; struct batman_if *batman_if; - struct net_device_stats *stats; int ret;
skb = skb_share_check(skb, GFP_ATOMIC); @@ -478,12 +477,6 @@ int batman_skb_recv(struct sk_buff *skb, struct net_device *dev, if (batman_if->if_status != IF_ACTIVE) goto err_free;
- stats = (struct net_device_stats *)dev_get_stats(skb->dev); - if (stats) { - stats->rx_packets++; - stats->rx_bytes += skb->len; - } - batman_packet = (struct batman_packet *)skb->data;
if (batman_packet->version != COMPAT_VERSION) {
From: Randy Dunlap randy.dunlap@oracle.com
Fix two staging drivers to use module_init()/module_exit() instead of default init_module() and cleanup_module() function names so that there are no name conflicts when both are built-in.
drivers/staging/dt3155/built-in.o: In function `cleanup_module': (.text+0xc0): multiple definition of `cleanup_module' drivers/staging/batman-adv/built-in.o:(.text+0x330): first defined here drivers/staging/dt3155/built-in.o: In function `init_module': (.text+0xe60): multiple definition of `init_module' drivers/staging/batman-adv/built-in.o:(.text+0x400): first defined here
Signed-off-by: Randy Dunlap randy.dunlap@oracle.com [sven.eckelmann@gmx.de: Mark module init and exit functions as static] Signed-off-by: Sven Eckelmann sven.eckelmann@gmx.de --- drivers/staging/batman-adv/main.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/batman-adv/main.c b/drivers/staging/batman-adv/main.c index a4d238b..2686019 100644 --- a/drivers/staging/batman-adv/main.c +++ b/drivers/staging/batman-adv/main.c @@ -59,7 +59,7 @@ static struct packet_type batman_adv_packet_type __read_mostly = {
struct workqueue_struct *bat_event_workqueue;
-int init_module(void) +static int __init batman_init(void) { int retval;
@@ -131,7 +131,7 @@ end: return -ENOMEM; }
-void cleanup_module(void) +static void __exit batman_exit(void) { deactivate_module();
@@ -273,6 +273,9 @@ int is_mcast(uint8_t *addr) return *addr & 0x01; }
+module_init(batman_init); +module_exit(batman_exit); + MODULE_LICENSE("GPL");
MODULE_AUTHOR(DRIVER_AUTHOR);
David S. Miller provided some new ideas and found problems in his review 20100719.212625.255369607.davem@davemloft.net. These issues must be resolved before it can be merged into net.
Signed-off-by: Sven Eckelmann sven.eckelmann@gmx.de --- drivers/staging/batman-adv/TODO | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/batman-adv/TODO b/drivers/staging/batman-adv/TODO index 518db7f..9c5aea2 100644 --- a/drivers/staging/batman-adv/TODO +++ b/drivers/staging/batman-adv/TODO @@ -1,6 +1,9 @@ -Request a review. -Process the comments from the review. -Move into mainline proper. + * Use hweight* for hamming weight calculation + * Save/cache packets direktly as skb instead of using a normal memory region + and copying it in a skb using send_raw_packet and similar functions + * Request a new review + * Process the comments from the review + * Move into mainline proper
Please send all patches to: Marek Lindner lindner_marek@yahoo.de
On Wed, Jul 21, 2010 at 12:25:19AM +0200, Sven Eckelmann wrote:
Hi,
first thing: thanks for your help to get some review for batman-adv from davem and the netdev ml. I've added some new points to the TODO list. They are partly resolved or work is done to solve them, but will postpone them and get them hopefully ready and tested for 2.6.37.
Nevertheless there are some smaller changes which want to resolve with 2.6.36. One is to allow to build it together with dt3155 directly into the kernel and the other one should fix that we try to change stats of a different device. The latter one will clash with an unlucky try by the netdev guys to change the behaviour of dev_get_stats and the batman-adv source code.
This merge conflict is quite easy to resolve. Just delete the dev_get_stats line in hard-interface.c in batman_skb_recv and the new variable temp added by them.
I can also provide a patch on top of linux-next instead of staging-next-2.6 if you would prefer it in this situation. I will call it "[PATCH-next 1/3] Staging: batman-adv: Don't increment stats of foreign device"
I hope that it doesn't create more confusion as necessary. :)
I just applied the 3 patches, I shouldn't need to apply the other one on top of -next, right?
thanks,
greg k-h
Greg KH wrote: [...]
I can also provide a patch on top of linux-next instead of staging-next-2.6 if you would prefer it in this situation. I will call it "[PATCH-next 1/3] Staging: batman-adv: Don't increment stats of foreign device"
I hope that it doesn't create more confusion as necessary. :)
I just applied the 3 patches, I shouldn't need to apply the other one on top of -next, right?
No, it was just in case that some merges happened before you apply them (and you would get the same situation as it is currently in linux-next).
I hope that the person who must resolve the merge conflict between staging- next and net-next does the right thing... but I am sure that everything will work fine (it is relative trivial).
thanks, Sven
b.a.t.m.a.n@lists.open-mesh.org