[B.A.T.M.A.N.] [PATCH] batman-adv: receive packets directly using skbs
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.
* this might introduce new race conditions.
* aggregation and vis code still use packet buffers and are not (yet)
converted.
This is the first version of this patch to be released, i would consider
it experimental and would hereby like to as for reviews before committing
it. Some things you might want to test:
* performace differences (are there any?)
* do all components still work? (vis, batctl ping, ...)
* do high load situations or multiple interfaces cause problems
* any memory leaks i might have overlooked?
I did some tests on my 9 node qemu environment and could confirm that
usual sending/receiving, forwarding, vis, batctl ping etc works. However
i can not talk about the performance from this setup.
Signed-off-by: Simon Wunderlich s...@hrz.tu-chemnitz.de
---
Index: a/batman-adv-kernelland/types.h
===
--- a/batman-adv-kernelland/types.h (revision 1507)
+++ b/batman-adv-kernelland/types.h (working copy)
@@ -39,7 +39,6 @@
char if_active;
char addr_str[ETH_STR_LEN];
struct net_device *net_dev;
- struct socket *raw_sock;
atomic_t seqno;
unsigned char *packet_buff;
int packet_len;
@@ -113,6 +112,7 @@
struct hlist_node list;
unsigned long send_time;
uint8_t own;
+ struct sk_buff *skb;
unsigned char *packet_buff;
uint16_t packet_len;
uint32_t direct_link_flags;
Index: a/batman-adv-kernelland/send.c
===
--- a/batman-adv-kernelland/send.c (revision 1507)
+++ b/batman-adv-kernelland/send.c (working copy)
@@ -58,25 +58,53 @@
return send_time;
}
-/* sends a raw packet. */
-void send_raw_packet(unsigned char *pack_buff, int pack_buff_len,
-struct batman_if *batman_if, uint8_t *dst_addr)
+/* send out an already prepared packet to the given address via the
+ * specified batman interface */
+int send_skb_packet(struct sk_buff *skb,
+ struct batman_if *batman_if,
+ uint8_t *dst_addr)
{
struct ethhdr *ethhdr;
- struct sk_buff *skb;
- int retval;
- char *data;
if (batman_if-if_active != IF_ACTIVE)
- return;
+ goto send_skb_err;
+ if (unlikely(!batman_if-net_dev))
+ goto send_skb_err;
+
if (!(batman_if-net_dev-flags IFF_UP)) {
printk(KERN_WARNING
batman-adv:Interface %s is not up - can't send packet
via that interface!\n,
batman_if-dev);
- return;
+ goto send_skb_err;
}
+
+ ethhdr = (struct ethhdr *) skb_mac_header(skb);
+ memcpy(ethhdr-h_source, batman_if-net_dev-dev_addr, ETH_ALEN);
+ memcpy(ethhdr-h_dest, dst_addr, ETH_ALEN);
+ ethhdr-h_proto = __constant_htons(ETH_P_BATMAN);
+
+ skb-dev = batman_if-net_dev;
+
+ /* dev_queue_xmit() returns a negative result on error. However on
+* congestion and traffic shaping, it drops and returns NET_XMIT_DROP
+* (which is 0). This will not be treated as an error. */
+
+ return dev_queue_xmit(skb);
+send_skb_err:
+ kfree_skb(skb);
+ return NET_XMIT_DROP;
+}
+
+/* sends a raw packet. */
+void send_raw_packet(unsigned char *pack_buff, int pack_buff_len,
+struct batman_if *batman_if, uint8_t *dst_addr)
+{
+ struct sk_buff *skb;
+ char *data;
+
+
skb = dev_alloc_skb(pack_buff_len + sizeof(struct ethhdr));
if (!skb)
return;
@@ -84,25 +112,12 @@
memcpy(data + sizeof(struct ethhdr), pack_buff, pack_buff_len);
- ethhdr = (struct ethhdr *) data;
- memcpy(ethhdr-h_source, batman_if-net_dev-dev_addr, ETH_ALEN);
- memcpy(ethhdr-h_dest, dst_addr, ETH_ALEN);
- ethhdr-h_proto = __constant_htons(ETH_P_BATMAN);
-
skb_reset_mac_header(skb);
skb_set_network_header(skb, ETH_HLEN);
skb-priority = TC_PRIO_CONTROL;
skb-protocol = __constant_htons(ETH_P_BATMAN);
- skb-dev = batman_if-net_dev;
- /* dev_queue_xmit() returns a negative result on error. However on
-* congestion and traffic shaping, it drops and returns NET_XMIT_DROP
-* (which is 0). This will not be treated as an error. */
- retval =
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: receive packets directly using skbs
Simon Wunderlich wrote:
+/* 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-len 2))
+ goto err_free;
Len field includes the length of the data in the main socket buffer and
possible fragments attached to the main socket buffer. This length is changed
when you move from layer to layer. So it is ok to test for 2 here to check if
it is enough data for version and type.
+
+ 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;
+
+
+ /* push back, we want to look at the ethernet header as well. */
+ skb_push(skb, sizeof(struct ethhdr));
+ skb_reset_mac_header(skb);
+
+ batman_packet = (struct batman_packet *)(skb-data
+ + sizeof(struct ethhdr));
+
+ if (batman_packet-version != COMPAT_VERSION) {
+ bat_dbg(DBG_BATMAN,
+ Drop packet: incompatible batman version (%i)\n,
+ batman_packet-version);
+ goto err_free;
+ }
Here you try to to access data which may be fragmented over different buffers.
This could for example happen if we send our data over a layer which fragments
it.
In the normal scenario of sending it directly over ethernet this shouldn't
happen, but that doesn't mean that nobody wants to do something special.
There are different other positions like recv_bat_packet where this is
ignored.
Maybe someone knows why this could never happen, but everything I have read
was more like you have to deal with it.
-static void recv_icmp_packet(struct ethhdr *ethhdr,
- unsigned char *packet_buff,
- int result,
- struct batman_if *batman_if)
+int recv_icmp_packet(struct sk_buff *skb)
{
struct icmp_packet *icmp_packet;
+ struct ethhdr *ethhdr;
struct orig_node *orig_node;
+ int hdr_size = sizeof(struct icmp_packet) + sizeof(struct ethhdr);
+ int ret;
+ /* drop packet if it has not necessary minimum size */
+ if (skb-len hdr_size)
+ return NET_RX_DROP;
+
+ ethhdr = (struct ethhdr *)skb_mac_header(skb);
+
/* packet with unicast indication but broadcast recipient */
if (is_bcast(ethhdr-h_dest))
- return;
+ return NET_RX_DROP;
/* packet with broadcast sender address */
if (is_bcast(ethhdr-h_source))
- return;
+ return NET_RX_DROP;
/* not for me */
if (!is_my_mac(ethhdr-h_dest))
- return;
+ return NET_RX_DROP;
- /* drop packet if it has not necessary minimum size */
- if (result sizeof(struct ethhdr) + sizeof(struct icmp_packet))
- return;
-
icmp_packet = (struct icmp_packet *)
- (packet_buff + sizeof(struct ethhdr));
+ (skb-data + sizeof(struct ethhdr));
/* packet for me */
if (is_my_mac(icmp_packet-dst))
- recv_my_icmp_packet(ethhdr, icmp_packet, packet_buff, result);
+ return recv_my_icmp_packet(skb);
/* TTL exceeded */
if (icmp_packet-ttl 2) {
- recv_icmp_ttl_exceeded(icmp_packet, ethhdr, packet_buff, result,
-batman_if);
- return;
+ return recv_icmp_ttl_exceeded(skb);
}
+ ret = NET_RX_DROP;
+
/* get routing information */
spin_lock(orig_hash_lock);
orig_node = ((struct orig_node *)
@@ -725,49 +721,51 @@
icmp_packet-ttl--;
You are modify the data inside the socket buffer (not the control structure
but the real data), but I cannot see where you created a private copy of it.
Data of cloned copies is not allowed to be changed. You must use (p)skb_copy
(it clones the the data too and not only the sk_buff structure).
This is explained in Understanding Linux Network Internals, Christian
Benvenuti, December 2005, O'Reilly, 2.1.5.5. Cloning and copying buffers
Best regards,
Sven
signature.asc
Description: This is a digitally signed message part.
[B.A.T.M.A.N.] can't start batmand
I tried to start batmand with batmand wlan0 got back and error stating which iptables /dev/null returned an error Does anyone know what is the cause and/or solution to this problem? Thanks Eric
Re: [B.A.T.M.A.N.] can't start batmand
Conner, Eric wrote: Does anyone know what is the cause and/or solution to this problem? Either iptables is not installed, iptables not installed in a path which is is part of $PATH, batmand is not started as root or which is not installed/not a builtin function of the shell. Best regards, Sven signature.asc Description: This is a digitally signed message part.
Re: [B.A.T.M.A.N.] [PATCH] batman-adv: receive packets directly using skbs
On Wed, Dec 23, 2009 at 09:34:17AM -0800, Gus Wirth wrote: On 12/23/2009 02:20 AM, Sven Eckelmann wrote: [snip] This is explained in Understanding Linux Network Internals, Christian Benvenuti, December 2005, O'Reilly, 2.1.5.5. Cloning and copying buffers Where did the 2.1.5.5 come from? Second chapter, first section, fifth subsection, fifth subsubsection. Simple counting :) But yes, some eBooks from oreilly seem to have that numbers. Cannot tell for sure if they are the same books you can download from them, but my library provides them that way. Publisher for that ebook is Safari Books. http://proquest.safaribooksonline.com/0596002556 Just noticed that the safari books image view (aka buggy flash viewer) also doesn't display the subsection numbers - but the HTML view does. Best regards, Sven signature.asc Description: Digital signature
