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
Scenario:
I have recently moved 13 nodes previously using wds to batman-adv.
One problem that i am encountering is the gateway selection and its
actually defectiveness.
This problem has be seen when the client nodes are set on class 20.
The node clients in fact select the gateways that advertise better link
quality usually above 200 but at the same time some clients end up
connecting to a gateway that is either too slow or unusable either due
to routing or other factors such as distance and even clear visibility.
In one case the node connects to something that defies logic.
I thought about ways to work around this and came up with an idea that
may help which means the creation of 2 new classes.
- Preferred gateway fall-back
- Preferred fixed gateway
For Preferred gateway fall-back:
consider the gateway's advertised quality towards the gateway and stick
with the selection until the gateway disappears returning to it soon as
it returns regardless of any other data.
- Preferred fixed gateway
Manual gateway selection by the node owner or admin. (wds type)
If these classes cannot be created; maybe a plugin of some sort could
be. could be called "gordon".
I understand that this idea may sound useless the same way blocking
originators may be for someone but in some scenarios just like blocking
originators it look to me very valid.
My current problem is that the link quality seems to go up and down like
a roller coaster in a specific order which causes the clients to switch
constantly sometimes many times per hour.
While this does not seem to be an issue for most client nodes; for 2 of
them it means no internet access at all.
The solution for now seems to be having these 2 nodes forced to use 1
specific gateway.
I am quite open to any ideas that may help resolve this issue that has
been casing some "uninformed people" to become headaches.
--
Redes wireless
http://wirelesspt.net
When adding a new hard interface (e.h. wlan0) to a soft interface (e.g. bat0)
and the former is already enslaved in another virtual interface (e.g. a software
bridge) batman-adv has to free the it first and then continue with the
adding mechanism.
In this way the behaviour becomes consistent with what "ip link set master"
does. At the moment batman-adv enslaves the hard interface without checking for
the master device, possibly causing strange behaviours which are never wanted by
the users.
Reported-by: Marek Lindner <lindner_marek(a)yahoo.de>
Signed-off-by: Antonio Quartulli <ordex(a)autistici.org>
---
v2:
- added compat code for netdev_master_upper_dev_get()
v3:
- use #define instead of a static inline function in compat code
compat.h | 5 +++++
hard-interface.c | 14 ++++++++------
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/compat.h b/compat.h
index 425b3d9..885e551 100644
--- a/compat.h
+++ b/compat.h
@@ -214,6 +214,11 @@ static int __batadv_interface_set_mac_addr(x, y)
#define netdev_master_upper_dev_link netdev_set_master
#define netdev_upper_dev_unlink(slave, master) netdev_set_master(slave, NULL)
+#define netdev_master_upper_dev_get(dev) \
+({\
+ ASSERT_RTNL();\
+ dev->master;\
+})
#endif /* < KERNEL_VERSION(3, 9, 0) */
diff --git a/hard-interface.c b/hard-interface.c
index c08e39e..fd99e42 100644
--- a/hard-interface.c
+++ b/hard-interface.c
@@ -311,7 +311,7 @@ int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
const char *iface_name)
{
struct batadv_priv *bat_priv;
- struct net_device *soft_iface;
+ struct net_device *soft_iface, *master;
__be16 ethertype = __constant_htons(ETH_P_BATMAN);
int ret;
@@ -321,11 +321,6 @@ int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
if (!atomic_inc_not_zero(&hard_iface->refcount))
goto out;
- /* hard-interface is part of a bridge */
- if (hard_iface->net_dev->priv_flags & IFF_BRIDGE_PORT)
- pr_err("You are about to enable batman-adv on '%s' which already is part of a bridge. Unless you know exactly what you are doing this is probably wrong and won't work the way you think it would.\n",
- hard_iface->net_dev->name);
-
soft_iface = dev_get_by_name(&init_net, iface_name);
if (!soft_iface) {
@@ -347,6 +342,13 @@ int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
goto err_dev;
}
+ /* check if the interface is enslaved in another virtual one and
+ * in that case unlink it first
+ */
+ master = netdev_master_upper_dev_get(hard_iface->net_dev);
+ if (master)
+ netdev_upper_dev_unlink(hard_iface->net_dev, master);
+
hard_iface->soft_iface = soft_iface;
bat_priv = netdev_priv(hard_iface->soft_iface);
--
1.8.1.2
Hi all,
I've writed a batman-adv proto in according on new netifd system of
attitude adjustement release.
With this script the batman-adv configuration could be moved on
/etc/config/network file istead of /etc/config/batman-adv and the
/lib/batman-adv/config.sh is no longer required.
es.
config interface mesh
option proto bat
option device bat0
option interfaces 'wlan0 wlan1'
option gw_mode server
...
if someone think that my work could be useful and can provide a better
and elegant integration on openwrt please keep in touch.
--
Filippo Sallemi
On Tue, Feb 19, 2013 at 03:36:46PM +0100, Antonio Quartulli wrote:
> In recv_alfred_packet() a "break" statement is missing at the end of a case
> block and the function is returning -1 instead of 0 even if the packet type was
> known. Add a break to let the function behave correctly.
>
> However this bug is harmless at the moment because the return value of the
> function is not checked anywhere.
>
> Signed-off: Antonio Quartulli <antonio(a)open-mesh.com>
> ---
> recv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/recv.c b/recv.c
> index 0170695..ffabba5 100644
> --- a/recv.c
> +++ b/recv.c
> @@ -416,6 +416,7 @@ int recv_alfred_packet(struct globals *globals)
> case ALFRED_STATUS_TXEND:
> process_alfred_status_txend(globals, &source.sin6_addr,
> (struct alfred_status_v0 *)packet);
> + break;
Applied (git rev 274133d)
Thanks,
Simon
When adding a new hard interface (e.h. wlan0) to a soft interface (e.g. bat0)
and the former is already enslaved in another virtual interface (e.g. a software
bridge) batman-adv has to free the it first and then continue with the
adding mechanism.
In this way the behaviour becomes consistent with what "ip link set master"
does. At the moment batman-adv enslaves the hard interface without checking for
the master device, possibly causing strange behaviours which are never wanted by
the users.
Reported-by: Marek Lindner <lindner_marek(a)yahoo.de>
Signed-off-by: Antonio Quartulli <ordex(a)autistici.org>
---
v2:
- added compat code for netdev_master_upper_dev_get()
compat.h | 9 +++++++++
hard-interface.c | 14 ++++++++------
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/compat.h b/compat.h
index 425b3d9..787cbcc 100644
--- a/compat.h
+++ b/compat.h
@@ -197,6 +197,8 @@ static inline void eth_hw_addr_random(struct net_device *dev)
#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 9, 0)
+#include <linux/netdevice.h>
+
#define prandom_u32() random32()
#define batadv_interface_set_mac_addr(x, y) \
@@ -214,6 +216,13 @@ static int __batadv_interface_set_mac_addr(x, y)
#define netdev_master_upper_dev_link netdev_set_master
#define netdev_upper_dev_unlink(slave, master) netdev_set_master(slave, NULL)
+static inline struct net_device *
+netdev_master_upper_dev_get(struct net_device *dev)
+{
+ ASSERT_RTNL();
+
+ return dev->master;
+}
#endif /* < KERNEL_VERSION(3, 9, 0) */
diff --git a/hard-interface.c b/hard-interface.c
index c08e39e..fd99e42 100644
--- a/hard-interface.c
+++ b/hard-interface.c
@@ -311,7 +311,7 @@ int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
const char *iface_name)
{
struct batadv_priv *bat_priv;
- struct net_device *soft_iface;
+ struct net_device *soft_iface, *master;
__be16 ethertype = __constant_htons(ETH_P_BATMAN);
int ret;
@@ -321,11 +321,6 @@ int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
if (!atomic_inc_not_zero(&hard_iface->refcount))
goto out;
- /* hard-interface is part of a bridge */
- if (hard_iface->net_dev->priv_flags & IFF_BRIDGE_PORT)
- pr_err("You are about to enable batman-adv on '%s' which already is part of a bridge. Unless you know exactly what you are doing this is probably wrong and won't work the way you think it would.\n",
- hard_iface->net_dev->name);
-
soft_iface = dev_get_by_name(&init_net, iface_name);
if (!soft_iface) {
@@ -347,6 +342,13 @@ int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
goto err_dev;
}
+ /* check if the interface is enslaved in another virtual one and
+ * in that case unlink it first
+ */
+ master = netdev_master_upper_dev_get(hard_iface->net_dev);
+ if (master)
+ netdev_upper_dev_unlink(hard_iface->net_dev, master);
+
hard_iface->soft_iface = soft_iface;
bat_priv = netdev_priv(hard_iface->soft_iface);
--
1.8.1.2