Skip to content

Commit

Permalink
Staging: batman-adv: receive packets directly using skbs
Browse files Browse the repository at this point in the history
This patch removes the (ugly and racy) packet receiving thread and the
kernel socket usage. Instead, packets are received directly by registering
the ethernet type and handling skbs instead of self-allocated buffers.

Some consequences and comments:

 * we don't copy the payload data when forwarding/sending/receiving data
   anymore. This should boost performance.
 * packets from/to different interfaces can be (theoretically) processed
   simultaneously. Only the big originator hash lock might be in the way.
 * no more polling or sleeping/wakeup/scheduling issues when receiving
   packets
 * this might introduce new race conditions.
 * aggregation and vis code still use packet buffers and are not (yet)
   converted.
 * all spinlocks were converted to irqsave/restore versions to solve
   some lifelock issues when preempted. This might be overkill, some
   of these locks might be reverted later.
 * skb copies are only done if neccesary to avoid overhead

performance differences:

 * we made some "benchmarks" with intel laptops.
 * bandwidth on Gigabit Ethernet increased from ~500 MBit/s to ~920 MBit/s
 * ping latency decresed from ~2ms to ~0.2 ms

I did some tests on my 9 node qemu environment and could confirm that
usual sending/receiving, forwarding, vis, batctl ping etc works.

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
Acked-by: Sven Eckelmann <sven.eckelmann@gmx.de>
Acked-by: Marek Lindner <lindner_marek@yahoo.de>
Acked-by: Linus Lüssing <linus.luessing@web.de>
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  • Loading branch information
Simon Wunderlich authored and Greg Kroah-Hartman committed Mar 4, 2010
1 parent c4bf05d commit e701719
Show file tree
Hide file tree
Showing 15 changed files with 548 additions and 509 deletions.
13 changes: 8 additions & 5 deletions drivers/staging/batman-adv/aggregation.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ static void new_aggregated_packet(unsigned char *packet_buff,
int own_packet)
{
struct forw_packet *forw_packet_aggr;
unsigned long flags;

forw_packet_aggr = kmalloc(sizeof(struct forw_packet), GFP_ATOMIC);
if (!forw_packet_aggr)
Expand All @@ -115,6 +116,7 @@ static void new_aggregated_packet(unsigned char *packet_buff,
packet_buff,
forw_packet_aggr->packet_len);

forw_packet_aggr->skb = NULL;
forw_packet_aggr->own = own_packet;
forw_packet_aggr->if_incoming = if_incoming;
forw_packet_aggr->num_packets = 0;
Expand All @@ -126,9 +128,9 @@ static void new_aggregated_packet(unsigned char *packet_buff,
forw_packet_aggr->direct_link_flags |= 1;

/* add new packet to packet list */
spin_lock(&forw_bat_list_lock);
spin_lock_irqsave(&forw_bat_list_lock, flags);
hlist_add_head(&forw_packet_aggr->list, &forw_bat_list);
spin_unlock(&forw_bat_list_lock);
spin_unlock_irqrestore(&forw_bat_list_lock, flags);

/* start timer for this packet */
INIT_DELAYED_WORK(&forw_packet_aggr->delayed_work,
Expand Down Expand Up @@ -168,9 +170,10 @@ void add_bat_packet_to_list(unsigned char *packet_buff, int packet_len,
struct batman_packet *batman_packet =
(struct batman_packet *)packet_buff;
bool direct_link = batman_packet->flags & DIRECTLINK ? 1 : 0;
unsigned long flags;

/* find position for the packet in the forward queue */
spin_lock(&forw_bat_list_lock);
spin_lock_irqsave(&forw_bat_list_lock, flags);
/* own packets are not to be aggregated */
if ((atomic_read(&aggregation_enabled)) && (!own_packet)) {
hlist_for_each_entry(forw_packet_pos, tmp_node, &forw_bat_list,
Expand All @@ -191,15 +194,15 @@ void add_bat_packet_to_list(unsigned char *packet_buff, int packet_len,
* suitable aggregation packet found */
if (forw_packet_aggr == NULL) {
/* the following section can run without the lock */
spin_unlock(&forw_bat_list_lock);
spin_unlock_irqrestore(&forw_bat_list_lock, flags);
new_aggregated_packet(packet_buff, packet_len,
send_time, direct_link,
if_incoming, own_packet);
} else {
aggregate(forw_packet_aggr,
packet_buff, packet_len,
direct_link);
spin_unlock(&forw_bat_list_lock);
spin_unlock_irqrestore(&forw_bat_list_lock, flags);
}
}

Expand Down
24 changes: 14 additions & 10 deletions drivers/staging/batman-adv/device.c
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,9 @@ int bat_device_release(struct inode *inode, struct file *file)
(struct device_client *)file->private_data;
struct device_packet *device_packet;
struct list_head *list_pos, *list_pos_tmp;
unsigned long flags;

spin_lock(&device_client->lock);
spin_lock_irqsave(&device_client->lock, flags);

/* for all packets in the queue ... */
list_for_each_safe(list_pos, list_pos_tmp, &device_client->queue_list) {
Expand All @@ -146,7 +147,7 @@ int bat_device_release(struct inode *inode, struct file *file)
}

device_client_hash[device_client->index] = NULL;
spin_unlock(&device_client->lock);
spin_unlock_irqrestore(&device_client->lock, flags);

kfree(device_client);
dec_module_count();
Expand All @@ -161,6 +162,7 @@ ssize_t bat_device_read(struct file *file, char __user *buf, size_t count,
(struct device_client *)file->private_data;
struct device_packet *device_packet;
int error;
unsigned long flags;

if ((file->f_flags & O_NONBLOCK) && (device_client->queue_len == 0))
return -EAGAIN;
Expand All @@ -177,14 +179,14 @@ ssize_t bat_device_read(struct file *file, char __user *buf, size_t count,
if (error)
return error;

spin_lock(&device_client->lock);
spin_lock_irqsave(&device_client->lock, flags);

device_packet = list_first_entry(&device_client->queue_list,
struct device_packet, list);
list_del(&device_packet->list);
device_client->queue_len--;

spin_unlock(&device_client->lock);
spin_unlock_irqrestore(&device_client->lock, flags);

error = __copy_to_user(buf, &device_packet->icmp_packet,
sizeof(struct icmp_packet));
Expand All @@ -205,6 +207,7 @@ ssize_t bat_device_write(struct file *file, const char __user *buff,
struct icmp_packet icmp_packet;
struct orig_node *orig_node;
struct batman_if *batman_if;
unsigned long flags;

if (len < sizeof(struct icmp_packet)) {
bat_dbg(DBG_BATMAN, "batman-adv:Error - can't send packet from char device: invalid packet size\n");
Expand Down Expand Up @@ -239,7 +242,7 @@ ssize_t bat_device_write(struct file *file, const char __user *buff,
if (atomic_read(&module_state) != MODULE_ACTIVE)
goto dst_unreach;

spin_lock(&orig_hash_lock);
spin_lock_irqsave(&orig_hash_lock, flags);
orig_node = ((struct orig_node *)hash_find(orig_hash, icmp_packet.dst));

if (!orig_node)
Expand All @@ -261,11 +264,11 @@ ssize_t bat_device_write(struct file *file, const char __user *buff,
sizeof(struct icmp_packet),
batman_if, orig_node->router->addr);

spin_unlock(&orig_hash_lock);
spin_unlock_irqrestore(&orig_hash_lock, flags);
goto out;

unlock:
spin_unlock(&orig_hash_lock);
spin_unlock_irqrestore(&orig_hash_lock, flags);
dst_unreach:
icmp_packet.msg_type = DESTINATION_UNREACHABLE;
bat_device_add_packet(device_client, &icmp_packet);
Expand All @@ -290,6 +293,7 @@ void bat_device_add_packet(struct device_client *device_client,
struct icmp_packet *icmp_packet)
{
struct device_packet *device_packet;
unsigned long flags;

device_packet = kmalloc(sizeof(struct device_packet), GFP_KERNEL);

Expand All @@ -300,12 +304,12 @@ void bat_device_add_packet(struct device_client *device_client,
memcpy(&device_packet->icmp_packet, icmp_packet,
sizeof(struct icmp_packet));

spin_lock(&device_client->lock);
spin_lock_irqsave(&device_client->lock, flags);

/* while waiting for the lock the device_client could have been
* deleted */
if (!device_client_hash[icmp_packet->uid]) {
spin_unlock(&device_client->lock);
spin_unlock_irqrestore(&device_client->lock, flags);
kfree(device_packet);
return;
}
Expand All @@ -322,7 +326,7 @@ void bat_device_add_packet(struct device_client *device_client,
device_client->queue_len--;
}

spin_unlock(&device_client->lock);
spin_unlock_irqrestore(&device_client->lock, flags);

wake_up(&device_client->queue_wait);
}
Expand Down
154 changes: 109 additions & 45 deletions drivers/staging/batman-adv/hard-interface.c
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,6 @@ void hardif_deactivate_interface(struct batman_if *batman_if)
if (batman_if->if_active != IF_ACTIVE)
return;

if (batman_if->raw_sock)
sock_release(batman_if->raw_sock);

/**
* batman_if->net_dev has been acquired by dev_get_by_name() in
* proc_interfaces_write() and has to be unreferenced.
Expand All @@ -164,9 +161,6 @@ void hardif_deactivate_interface(struct batman_if *batman_if)
if (batman_if->net_dev)
dev_put(batman_if->net_dev);

batman_if->raw_sock = NULL;
batman_if->net_dev = NULL;

batman_if->if_active = IF_INACTIVE;
active_ifs--;

Expand All @@ -177,9 +171,6 @@ void hardif_deactivate_interface(struct batman_if *batman_if)
/* (re)activate given interface. */
static void hardif_activate_interface(struct batman_if *batman_if)
{
struct sockaddr_ll bind_addr;
int retval;

if (batman_if->if_active != IF_INACTIVE)
return;

Expand All @@ -191,35 +182,8 @@ static void hardif_activate_interface(struct batman_if *batman_if)
if (!batman_if->net_dev)
goto dev_err;

retval = sock_create_kern(PF_PACKET, SOCK_RAW,
__constant_htons(ETH_P_BATMAN),
&batman_if->raw_sock);

if (retval < 0) {
printk(KERN_ERR "batman-adv:Can't create raw socket: %i\n",
retval);
goto sock_err;
}

bind_addr.sll_family = AF_PACKET;
bind_addr.sll_ifindex = batman_if->net_dev->ifindex;
bind_addr.sll_protocol = 0; /* is set by the kernel */

retval = kernel_bind(batman_if->raw_sock,
(struct sockaddr *)&bind_addr, sizeof(bind_addr));

if (retval < 0) {
printk(KERN_ERR "batman-adv:Can't create bind raw socket: %i\n",
retval);
goto bind_err;
}

check_known_mac_addr(batman_if->net_dev->dev_addr);

batman_if->raw_sock->sk->sk_user_data =
batman_if->raw_sock->sk->sk_data_ready;
batman_if->raw_sock->sk->sk_data_ready = batman_data_ready;

addr_to_string(batman_if->addr_str, batman_if->net_dev->dev_addr);

memcpy(((struct batman_packet *)(batman_if->packet_buff))->orig,
Expand All @@ -239,12 +203,7 @@ static void hardif_activate_interface(struct batman_if *batman_if)

return;

bind_err:
sock_release(batman_if->raw_sock);
sock_err:
dev_put(batman_if->net_dev);
dev_err:
batman_if->raw_sock = NULL;
batman_if->net_dev = NULL;
}

Expand Down Expand Up @@ -318,6 +277,7 @@ int hardif_add_interface(char *dev, int if_num)
struct batman_if *batman_if;
struct batman_packet *batman_packet;
struct orig_node *orig_node;
unsigned long flags;
HASHIT(hashit);

batman_if = kmalloc(sizeof(struct batman_if), GFP_KERNEL);
Expand All @@ -327,7 +287,6 @@ int hardif_add_interface(char *dev, int if_num)
return -1;
}

batman_if->raw_sock = NULL;
batman_if->net_dev = NULL;

if ((if_num == 0) && (num_hna > 0))
Expand Down Expand Up @@ -375,17 +334,17 @@ int hardif_add_interface(char *dev, int if_num)

/* resize all orig nodes because orig_node->bcast_own(_sum) depend on
* if_num */
spin_lock(&orig_hash_lock);
spin_lock_irqsave(&orig_hash_lock, flags);

while (hash_iterate(orig_hash, &hashit)) {
orig_node = hashit.bucket->data;
if (resize_orig(orig_node, if_num) == -1) {
spin_unlock(&orig_hash_lock);
spin_unlock_irqrestore(&orig_hash_lock, flags);
goto out;
}
}

spin_unlock(&orig_hash_lock);
spin_unlock_irqrestore(&orig_hash_lock, flags);

if (!hardif_is_interface_up(batman_if->dev))
printk(KERN_ERR "batman-adv:Not using interface %s (retrying later): interface not active\n", batman_if->dev);
Expand Down Expand Up @@ -443,6 +402,111 @@ static int hard_if_event(struct notifier_block *this,
return NOTIFY_DONE;
}

/* find batman interface by netdev. assumes rcu_read_lock on */
static struct batman_if *find_batman_if(struct net_device *dev)
{
struct batman_if *batman_if;

rcu_read_lock();
list_for_each_entry_rcu(batman_if, &if_list, list) {
if (batman_if->net_dev == dev) {
rcu_read_unlock();
return batman_if;
}
}
rcu_read_unlock();
return NULL;
}


/* receive a packet with the batman ethertype coming on a hard
* interface */
int batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
struct packet_type *ptype, struct net_device *orig_dev)
{
struct batman_packet *batman_packet;
struct batman_if *batman_if;
struct net_device_stats *stats;
int ret;

skb = skb_share_check(skb, GFP_ATOMIC);

if (skb == NULL)
goto err_free;

/* packet should hold at least type and version */
if (unlikely(skb_headlen(skb) < 2))
goto err_free;

/* expect a valid ethernet header here. */
if (unlikely(skb->mac_len != sizeof(struct ethhdr)
|| !skb_mac_header(skb)))
goto err_free;

batman_if = find_batman_if(skb->dev);
if (!batman_if)
goto err_free;

stats = &skb->dev->stats;
stats->rx_packets++;
stats->rx_bytes += skb->len;

batman_packet = (struct batman_packet *)skb->data;

if (batman_packet->version != COMPAT_VERSION) {
bat_dbg(DBG_BATMAN,
"Drop packet: incompatible batman version (%i)\n",
batman_packet->version);
goto err_free;
}

/* all receive handlers return whether they received or reused
* the supplied skb. if not, we have to free the skb. */

switch (batman_packet->packet_type) {
/* batman originator packet */
case BAT_PACKET:
ret = recv_bat_packet(skb, batman_if);
break;

/* batman icmp packet */
case BAT_ICMP:
ret = recv_icmp_packet(skb);
break;

/* unicast packet */
case BAT_UNICAST:
ret = recv_unicast_packet(skb);
break;

/* broadcast packet */
case BAT_BCAST:
ret = recv_bcast_packet(skb);
break;

/* vis packet */
case BAT_VIS:
ret = recv_vis_packet(skb);
break;
default:
ret = NET_RX_DROP;
}
if (ret == NET_RX_DROP)
kfree_skb(skb);

/* return NET_RX_SUCCESS in any case as we
* most probably dropped the packet for
* routing-logical reasons. */

return NET_RX_SUCCESS;

err_free:
kfree_skb(skb);
return NET_RX_DROP;

}


struct notifier_block hard_if_notifier = {
.notifier_call = hard_if_event,
};
Loading

0 comments on commit e701719

Please sign in to comment.