From: Markus Elfring <elfring(a)users.sourceforge.net>
Date: Tue, 3 Nov 2015 21:34:29 +0100
Further update suggestions were taken into account after a patch
was applied from static source code analysis.
Markus Elfring (3):
Delete an unnecessary check before the function call "batadv_softif_vlan_free_ref"
Split a condition check
Less function calls in batadv_is_ap_isolated() after error detection
net/batman-adv/translation-table.c | 29 ++++++++++++++---------------
1 file changed, 14 insertions(+), 15 deletions(-)
--
2.6.2
Hi, it's Germano from Ninux.org community.
For personal purposes, I am commenting vis.c/vis.h alfred code, at the
beginning of each function, explaining also most important local
variables (keeping in mind documentation guidelines [1])
I would like to know if you are interested in such documentation
process. If yes, I could extend this kind of work to the whole alfred
code.
Have a nice day
[1]: https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt
So far on purging broadcast and ogm queues we temporarily give up the
spin lock of these queues to be able to cancel any scheduled forwarding
work. However this is unsafe and can lead to a general protection error
in batadv_purge_outstanding_packets().
With this patch we split the queue purging into two steps: First
removing forward packets from those queues and signaling the
cancelation. Secondly, we are actively canceling any scheduled
forwarding, wait for any running forwarding to finish and only free a
forw_packet afterwards.
Signed-off-by: Linus Lüssing <linus.luessing(a)web.de>
---
Fixes issue #168
send.c | 117 ++++++++++++++++++++++++++++++++++++++-------------------------
types.h | 1 +
2 files changed, 71 insertions(+), 47 deletions(-)
diff --git a/send.c b/send.c
index 0a0bb45..f93476b 100644
--- a/send.c
+++ b/send.c
@@ -245,6 +245,10 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct *work)
bat_priv = netdev_priv(soft_iface);
spin_lock_bh(&bat_priv->forw_bcast_list_lock);
+ if (hlist_unhashed(&forw_packet->list)) {
+ spin_unlock_bh(&bat_priv->forw_bcast_list_lock);
+ return;
+ }
hlist_del(&forw_packet->list);
spin_unlock_bh(&bat_priv->forw_bcast_list_lock);
@@ -293,6 +297,10 @@ void batadv_send_outstanding_bat_ogm_packet(struct work_struct *work)
delayed_work);
bat_priv = netdev_priv(forw_packet->if_incoming->soft_iface);
spin_lock_bh(&bat_priv->forw_bat_list_lock);
+ if (hlist_unhashed(&forw_packet->list)) {
+ spin_unlock_bh(&bat_priv->forw_bat_list_lock);
+ return;
+ }
hlist_del(&forw_packet->list);
spin_unlock_bh(&bat_priv->forw_bat_list_lock);
@@ -316,13 +324,68 @@ out:
batadv_forw_packet_free(forw_packet);
}
+/**
+ * batadv_cancel_packets - Cancels a list of forward packets
+ * @forw_list: The to be canceled forward packets
+ * @canceled_list: The backup list.
+ *
+ * This canceles any scheduled forwarding packet tasks in the provided
+ * forw_list. The packets are being moved from the forw_list to the
+ * canceled_list afterwards to unhash the forward packet list pointer,
+ * allowing any already running task to notice the cancelation.
+ */
+static void batadv_cancel_packets(struct hlist_head *forw_list,
+ struct hlist_head *canceled_list,
+ const struct batadv_hard_iface *hard_iface)
+{
+ struct batadv_forw_packet *forw_packet;
+ struct hlist_node *tmp_node, *safe_tmp_node;
+
+ hlist_for_each_entry_safe(forw_packet, tmp_node, safe_tmp_node,
+ forw_list, list) {
+ /* if purge_outstanding_packets() was called with an argument
+ * we delete only packets belonging to the given interface
+ */
+ if ((hard_iface) &&
+ (forw_packet->if_incoming != hard_iface))
+ continue;
+
+ hlist_del_init(&forw_packet->list);
+ hlist_add_head(&forw_packet->canceled_list, canceled_list);
+ }
+}
+
+/**
+ * batadv_canceled_packets_free - Frees canceled forward packets
+ * @head: A list of to be freed forw_packets
+ *
+ * This function canceles the scheduling of any packet in the provided list,
+ * waits for any possibly running packet forwarding thread to finish and
+ * finally, safely frees this forward packet.
+ *
+ * This function might sleep.
+ */
+static void batadv_canceled_packets_free(struct hlist_head *head)
+{
+ struct batadv_forw_packet *forw_packet;
+ struct hlist_node *tmp_node, *safe_tmp_node;
+
+ hlist_for_each_entry_safe(forw_packet, tmp_node, safe_tmp_node, head,
+ canceled_list) {
+ cancel_delayed_work_sync(&forw_packet->delayed_work);
+
+ hlist_del(&forw_packet->canceled_list);
+ batadv_forw_packet_free(forw_packet);
+ }
+}
+
void
batadv_purge_outstanding_packets(struct batadv_priv *bat_priv,
const struct batadv_hard_iface *hard_iface)
{
- struct batadv_forw_packet *forw_packet;
- struct hlist_node *tmp_node, *safe_tmp_node;
- bool pending;
+ struct hlist_head head;
+
+ INIT_HLIST_HEAD(&head);
if (hard_iface)
batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
@@ -334,53 +397,13 @@ batadv_purge_outstanding_packets(struct batadv_priv *bat_priv,
/* free bcast list */
spin_lock_bh(&bat_priv->forw_bcast_list_lock);
- hlist_for_each_entry_safe(forw_packet, tmp_node, safe_tmp_node,
- &bat_priv->forw_bcast_list, list) {
- /* if purge_outstanding_packets() was called with an argument
- * we delete only packets belonging to the given interface
- */
- if ((hard_iface) &&
- (forw_packet->if_incoming != hard_iface))
- continue;
-
- spin_unlock_bh(&bat_priv->forw_bcast_list_lock);
-
- /* batadv_send_outstanding_bcast_packet() will lock the list to
- * delete the item from the list
- */
- pending = cancel_delayed_work_sync(&forw_packet->delayed_work);
- spin_lock_bh(&bat_priv->forw_bcast_list_lock);
-
- if (pending) {
- hlist_del(&forw_packet->list);
- batadv_forw_packet_free(forw_packet);
- }
- }
+ batadv_cancel_packets(&bat_priv->forw_bcast_list, &head, hard_iface);
spin_unlock_bh(&bat_priv->forw_bcast_list_lock);
/* free batman packet list */
spin_lock_bh(&bat_priv->forw_bat_list_lock);
- hlist_for_each_entry_safe(forw_packet, tmp_node, safe_tmp_node,
- &bat_priv->forw_bat_list, list) {
- /* if purge_outstanding_packets() was called with an argument
- * we delete only packets belonging to the given interface
- */
- if ((hard_iface) &&
- (forw_packet->if_incoming != hard_iface))
- continue;
-
- spin_unlock_bh(&bat_priv->forw_bat_list_lock);
-
- /* send_outstanding_bat_packet() will lock the list to
- * delete the item from the list
- */
- pending = cancel_delayed_work_sync(&forw_packet->delayed_work);
- spin_lock_bh(&bat_priv->forw_bat_list_lock);
-
- if (pending) {
- hlist_del(&forw_packet->list);
- batadv_forw_packet_free(forw_packet);
- }
- }
+ batadv_cancel_packets(&bat_priv->forw_bat_list, &head, hard_iface);
spin_unlock_bh(&bat_priv->forw_bat_list_lock);
+
+ batadv_canceled_packets_free(&head);
}
diff --git a/types.h b/types.h
index aba8364..f62a35f 100644
--- a/types.h
+++ b/types.h
@@ -853,6 +853,7 @@ struct batadv_skb_cb {
*/
struct batadv_forw_packet {
struct hlist_node list;
+ struct hlist_node canceled_list;
unsigned long send_time;
uint8_t own;
struct sk_buff *skb;
--
1.7.10.4
This patchset introduces optimizations for batman-adv in setups having several
gateways into a common (switched) Ethernet backbone network especially if dat
is additionally enabled.
Using the current implementation with bla and dat enabled, several problems
can be observed in a real setup:
1. Multiplication of ARP replies from dat enabled gateways and dat enabled
mesh nodes leading to an "ARP reply storm" in the common backbone network.
2. In rare corner cases bla does not fully prevent looping of unicast frames
in the direction Backbone --> mesh --> backbone and looping of multicast
frames in the direction mesh --> backbone --> mesh.
The latter can lead to temporary confusion in the switched backbone resulting
in packet loss and communication timeouts.
The observed problems are solved by introduction of additional rules for the
dat handling, bla packet forwarding and bla claiming/unclaiming of clients.
v3:
- rebased patchset
- moved snooping of ip addresses for dat speed up into separate function
- removed "patch of a patch"
- removed automatic claiming during check of a claim
- fixed issues of the patchset not being compiled due to chosen batman
options
Kind regards,
Andreas
..................................................................
PHOENIX CONTACT ELECTRONICS GmbH
Sitz der Gesellschaft / registered office of the company: 31812 Bad Pyrmont
USt-Id-Nr.: DE811742156
Amtsgericht Hannover HRB 100528 / district court Hannover HRB 100528
Geschäftsführer / Executive Board: Roland Bent, Dr. Martin Heubeck
___________________________________________________________________
Diese E-Mail enthält vertrauliche und/oder rechtlich geschützte Informationen. Wenn Sie nicht der richtige Adressat sind oder diese E-Mail irrtümlich erhalten haben, informieren Sie bitte sofort den Absender und vernichten Sie diese Mail. Das unerlaubte Kopieren, jegliche anderweitige Verwendung sowie die unbefugte Weitergabe dieser Mail ist nicht gestattet.
----------------------------------------------------------------------------------------------------
This e-mail may contain confidential and/or privileged information. If you are not the intended recipient (or have received this e-mail in error) please notify the sender immediately and destroy this e-mail. Any unauthorized copying, disclosure, distribution or other use of the material or parts thereof is strictly forbidden.
___________________________________________________________________
I've followed the quick start guide, but apparently I'm missing
something when it comes to non-batman clients.
I've set all this up using vagrant and VirtualBox. The mesh seems to
work perfectly, however when I put in non-batman clients they cannot
communicate one with another. I would expect that they would be able to
ping one another.
I've given IP addresses to the bridge adapters on the 192.168.1.0/24
subnet. The clients I've given IP addresses on that same subnet
(192.168.1.121 and 192.168.1.142). When I tried pinging, I don't get
anything back. Sniffing packets in tcpdump shows that it is never
getting anything back, though I did get an ARP back once with the
correct MAC address, but still couldn't ping. Never got an ARP back
again...
I can ping from each client to the IP address of the bridge they are
connecting to, but cannot ping anything else on the network.
Any thoughts? Hopefully someone has seen this before and can point me
in the right direction!
Also, is there a separate list for alfred, or should I use this list for
alfred questions?
Thanks!
Jon
Here are the commands I used to setup:
batctl if add eth0 (eth0 already up)
ip link add name br0 type bridge
ip link set dev eth1 master br0
ip link set dev bat0 master br0
ip link set up dev eth1
ip link set up dev bat0
ip addr replace dev br0 192.168.1.102/24 (and 104 on the other node)
Here is what batadv-vis shows for my setup (I changed the labels "TT"
for the two clients to "CLI<IP>").
digraph {
subgraph "cluster_08:00:27:2c:10:05" {
"08:00:27:2c:10:05"
}
"08:00:27:2c:10:05" -> "08:00:27:6b:86:2e" [label="1.016"]
"08:00:27:2c:10:05" -> "0a:00:27:00:00:06" [label="TT"]
"08:00:27:2c:10:05" -> "1e:8d:d3:23:ab:d4" [label="TT"]
"08:00:27:2c:10:05" -> "08:00:27:b1:38:03" [label="CLI142"]
"08:00:27:2c:10:05" -> "1e:8d:d3:23:ab:d4" [label="TT"]
"08:00:27:2c:10:05" -> "08:00:27:1c:9b:14" [label="TT"]
subgraph "cluster_08:00:27:a2:8c:62" {
"08:00:27:a2:8c:62"
}
"08:00:27:a2:8c:62" -> "08:00:27:83:dd:41" [label="1.000"]
"08:00:27:a2:8c:62" -> "08:00:27:38:31:55" [label="1.000"]
"08:00:27:a2:8c:62" -> "08:00:27:f6:78:1a" [label="TT"]
"08:00:27:a2:8c:62" -> "d6:f4:e6:bd:22:fb" [label="TT"]
"08:00:27:a2:8c:62" -> "d6:f4:e6:bd:22:fb" [label="TT"]
"08:00:27:a2:8c:62" -> "0a:00:27:00:00:05" [label="TT"]
subgraph "cluster_08:00:27:38:31:55" {
"08:00:27:38:31:55"
}
"08:00:27:38:31:55" -> "08:00:27:83:dd:41" [label="1.000"]
"08:00:27:38:31:55" -> "08:00:27:a2:8c:62" [label="1.000"]
"08:00:27:38:31:55" -> "52:98:12:67:6b:06" [label="TT"]
"08:00:27:38:31:55" -> "0a:00:27:00:00:04" [label="TT"]
"08:00:27:38:31:55" -> "08:00:27:08:ee:90" [label="CLI121"]
"08:00:27:38:31:55" -> "52:98:12:67:6b:06" [label="TT"]
"08:00:27:38:31:55" -> "08:00:27:17:21:9d" [label="TT"]
subgraph "cluster_08:00:27:6b:86:2e" {
"08:00:27:6b:86:2e"
"08:00:27:83:dd:41" [peripheries=2]
}
"08:00:27:6b:86:2e" -> "08:00:27:2c:10:05" [label="1.000"]
"08:00:27:83:dd:41" -> "08:00:27:38:31:55" [label="1.000"]
"08:00:27:83:dd:41" -> "08:00:27:a2:8c:62" [label="1.016"]
"08:00:27:6b:86:2e" -> "a6:f4:5c:fa:06:cd" [label="TT"]
"08:00:27:6b:86:2e" -> "0a:00:27:00:00:03" [label="TT"]
"08:00:27:6b:86:2e" -> "08:00:27:9c:bb:a3" [label="TT"]
"08:00:27:6b:86:2e" -> "a6:f4:5c:fa:06:cd" [label="TT"]
}
A hard interface can be removed and then added back in quick
succession. This is particularly true for hdlc interface when changing
the protocol.
It is not possible it synchronously remove the sysfs and debugfs
entries for the hard interface when it is removed because the files
may be open. Thus removal is deferred. The files may thus already
exist in sysfs and debugfs when the hard interface is re-added, and
the operations fail.
To fix this race, synchronously rename the debugfs and sysfs files to
a unique temporary name, thus making the name available when the
interface comes back, yet keeps open files still available.
Signed-off-by: Andrew Lunn <andrew(a)lunn.ch>
---
net/batman-adv/debugfs.c | 23 +++++++++++++++++++++++
net/batman-adv/debugfs.h | 1 +
net/batman-adv/hard-interface.c | 19 +++++++++++++++++++
net/batman-adv/sysfs.c | 17 +++++++++++++++++
net/batman-adv/sysfs.h | 2 ++
5 files changed, 62 insertions(+)
diff --git a/net/batman-adv/debugfs.c b/net/batman-adv/debugfs.c
index 50545b0..8b852aa 100644
--- a/net/batman-adv/debugfs.c
+++ b/net/batman-adv/debugfs.c
@@ -55,6 +55,7 @@
#include "translation-table.h"
static struct dentry *batadv_debugfs;
+static atomic_t batadv_rename = ATOMIC_INIT(0);
#ifdef CONFIG_BATMAN_ADV_DEBUG
#define BATADV_LOG_BUFF_MASK (batadv_log_buff_len - 1)
@@ -570,6 +571,28 @@ void batadv_debugfs_del_hardif(struct batadv_hard_iface *hard_iface)
}
}
+/**
+ * batadv_debugfs_rename_hardif - rename the base directory
+ * @hard_iface: hard interface which is renamed.
+ *
+ * The interface may be removed and then quickly added back
+ * again. Rename the old instance to something temporary and unique,
+ * so avoiding a name space clash if it does reappear before the deferred
+ * work completes the removal.
+ */
+void batadv_debugfs_rename_hardif(struct batadv_hard_iface *hard_iface)
+{
+ char new_name[32];
+
+ snprintf(new_name, sizeof(*new_name) - 1, "tmp-%d",
+ atomic_inc_return(&batadv_rename));
+
+ if (batadv_debugfs && hard_iface->debug_dir) {
+ debugfs_rename(batadv_debugfs, hard_iface->debug_dir,
+ batadv_debugfs, new_name);
+ }
+}
+
int batadv_debugfs_add_meshif(struct net_device *dev)
{
struct batadv_priv *bat_priv = netdev_priv(dev);
diff --git a/net/batman-adv/debugfs.h b/net/batman-adv/debugfs.h
index 1ab4e2e..e7d19c1 100644
--- a/net/batman-adv/debugfs.h
+++ b/net/batman-adv/debugfs.h
@@ -34,6 +34,7 @@ int batadv_debugfs_add_meshif(struct net_device *dev);
void batadv_debugfs_del_meshif(struct net_device *dev);
int batadv_debugfs_add_hardif(struct batadv_hard_iface *hard_iface);
void batadv_debugfs_del_hardif(struct batadv_hard_iface *hard_iface);
+void batadv_debugfs_rename_hardif(struct batadv_hard_iface *hard_iface);
#else
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index bf4ee24..08b62d9 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -747,6 +747,23 @@ out:
return NULL;
}
+/**
+ * batadv_hardif_rename - rename the sysfs and debugfs
+ * @hard_iface: The hard interface to rename
+ *
+ * The sysfs and debugfs files cannot be removed until all users close
+ * them. So the removal is differed into a work queue. This however
+ * means if the interface comes back before the work queue runs, the
+ * files are still there, and so the create gives an EEXISTS error. To
+ * avoid this, rename them to a tempory name.
+ */
+static void batadv_hardif_rename(struct batadv_hard_iface *hard_iface)
+{
+ batadv_sysfs_rename_hardif(&hard_iface->hardif_obj,
+ hard_iface->net_dev);
+ batadv_debugfs_rename_hardif(hard_iface);
+}
+
static void batadv_hardif_remove_interface(struct batadv_hard_iface *hard_iface)
{
ASSERT_RTNL();
@@ -760,6 +777,8 @@ static void batadv_hardif_remove_interface(struct batadv_hard_iface *hard_iface)
return;
hard_iface->if_status = BATADV_IF_TO_BE_REMOVED;
+ batadv_hardif_rename(hard_iface);
+
queue_work(batadv_event_workqueue, &hard_iface->cleanup_work);
}
diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index 233abcf..37b0aae 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -47,6 +47,8 @@
#include "packet.h"
#include "soft-interface.h"
+static atomic_t batadv_rename = ATOMIC_INIT(0);
+
static struct net_device *batadv_kobj_to_netdev(struct kobject *obj)
{
struct device *dev = container_of(obj->parent, struct device, kobj);
@@ -1045,6 +1047,21 @@ out:
return -ENOMEM;
}
+void batadv_sysfs_rename_hardif(struct kobject **hardif_obj,
+ struct net_device *dev)
+{
+ char new_name[32];
+ int err;
+
+ snprintf(new_name, sizeof(*new_name) - 1, "tmp-%d",
+ atomic_inc_return(&batadv_rename));
+
+ err = kobject_rename(*hardif_obj, new_name);
+ if (err)
+ batadv_err(dev, "Can't rename sysfs dir: %s/%s: %d\n",
+ dev->name, new_name, err);
+}
+
void batadv_sysfs_del_hardif(struct kobject **hardif_obj)
{
kobject_put(*hardif_obj);
diff --git a/net/batman-adv/sysfs.h b/net/batman-adv/sysfs.h
index c76021b..64d3722 100644
--- a/net/batman-adv/sysfs.h
+++ b/net/batman-adv/sysfs.h
@@ -48,6 +48,8 @@ void batadv_sysfs_del_meshif(struct net_device *dev);
int batadv_sysfs_add_hardif(struct kobject **hardif_obj,
struct net_device *dev);
void batadv_sysfs_del_hardif(struct kobject **hardif_obj);
+void batadv_sysfs_rename_hardif(struct kobject **hardif_obj,
+ struct net_device *dev);
int batadv_sysfs_add_vlan(struct net_device *dev,
struct batadv_softif_vlan *vlan);
void batadv_sysfs_del_vlan(struct batadv_priv *bat_priv,
--
2.8.0.rc3