Re: [B.A.T.M.A.N.] [PATCH 09/12] batman-adv: Adding redundant bonding mode transmission

On Wednesday 02 March 2011 18:18:38 Linus Lüssing wrote:

diff --git a/main.h b/main.h index a0059dd..c6d4848 100644 --- a/main.h +++ b/main.h @@ -97,6 +97,11 @@

• Vis

*/

+/* Bonding modes */ +#define THROUGHPUT_BONDING 1 +#define REDUNDANT_BONDING 2

Here you go into the right direction but ...

@@ -347,6 +346,7 @@ void frag_packet_list(struct bat_priv *bat_priv,

•   bonding_mode = atomic_read(&bat_priv->bonding) <<
  

•                   atomic_read(&bat_priv->red_bonding);
  

here you resort to some magic. I believe that can be simplified (see my sysfs reply).

+static int unicast_to_unicast_safe(struct sk_buff *skb,

•                              struct bat_priv *bat_priv)
  

[..]

•   unicast_packet_safe->header = unicast_packet.header;
  

•   memcpy(unicast_packet_safe->dest, unicast_packet.dest, ETH_ALEN);
  

•   unicast_packet_safe->header.packet_type = BAT_UNICAST_SAFE;
  

I don't think "unicast_packet_safe->header = unicast_packet.header;" does what you want it to do.

+static void red_bonding_copy(struct sk_buff *skb, struct list_head [..]

•   list_for_each_entry_rcu(neigh_node, bond_list, bonding_list) {
  

•           entry = kmalloc(sizeof(struct packet_list_entry),
  

GFP_ATOMIC);

•           if (!entry) {
  

•                   kfree_skb(skb);
  

•                   return;
  

•           }
  

•           if (!num_entries)
  

•                   entry->skb = skb;
  

•           else {
  

•                   entry->skb = skb_copy(skb, GFP_ATOMIC);
  

•                   if (!entry->skb) {
  

•                           kfree_skb(skb);
  

•                           kfree(entry);
  

•                           return;
  

•                   }
  

•           }
  

Yet another malloc() followed by an skb_copy() ? Seems to be unnecessary ballast.

Cheers, Marek