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
The content of batman_adv/iface_status ends with a newline. Thus
register_interfaces has to make sure that it only compares the first line
without the \n when comparing with the status string.
Signed-off-by: Sven Eckelmann <sven(a)narfation.org>
---
vis/vis.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/vis/vis.c b/vis/vis.c
index 4d494bc..7eab781 100644
--- a/vis/vis.c
+++ b/vis/vis.c
@@ -257,6 +257,7 @@ static int register_interfaces(struct globals *globals)
DIR *iface_base_dir;
struct dirent *iface_dir;
char *path_buff, *file_content;
+ char *content_newline;
path_buff = malloc(PATH_BUFF_LEN);
if (!path_buff) {
@@ -295,6 +296,10 @@ static int register_interfaces(struct globals *globals)
if (!file_content)
continue;
+ content_newline = strstr(file_content, "\n");
+ if (content_newline)
+ *content_newline = '\0';
+
if (strcmp(file_content, "active") == 0)
get_if_index(globals, iface_dir->d_name);
--
2.8.1
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.
v5:
- changed function name to batadv_bla_check_claim
- put added include file in alphabetical order
- added check to exclude ip address 0.0.0.0 from snooping
v4:
- removed unnecessary include in soft-interface.c
- solved issue with double-use and refcounting of pointers in routing.c
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
Andreas Pape (6):
batman-adv: prevent multiple ARP replies sent by gateways if dat
enbled
batman-adv: speed up dat by snooping received ip traffic
batman-adv: prevent duplication of ARP replies when DAT is used
batman-adv: drop unicast packets from other backbone gw
batman-adv: changed debug messages for easier bla debugging
batman-adv: handle race condition for claims between gateways
net/batman-adv/bridge_loop_avoidance.c | 87 ++++++++++++++++++++++++---
net/batman-adv/bridge_loop_avoidance.h | 11 ++++
net/batman-adv/distributed-arp-table.c | 102 ++++++++++++++++++++++++++++++++
net/batman-adv/distributed-arp-table.h | 9 +++-
net/batman-adv/routing.c | 25 +++++++-
net/batman-adv/soft-interface.c | 3 +
6 files changed, 225 insertions(+), 12 deletions(-)
..................................................................
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.
___________________________________________________________________
The function batadv_send_skb_unicast is not acquiring a reference for an
orig_node nor removing it from any datastructure. It still reduces the
reference counter for an object which is still in the hands of the caller.
This is confusing and can lead in the future to problems in the reference
handling of the caller function.
Signed-off-by: Sven Eckelmann <sven(a)narfation.org>
---
v3:
- adjust commit message to sound less like an fix (thanks Linus)
- Remove " and release a reference to this orig_node" from kerneldoc of
batadv_send_skb_unicast (thanks Linus)
v2:
- remove bogus multicast example
- remove Fixes:
---
net/batman-adv/send.c | 25 +++++++++++++++++--------
net/batman-adv/soft-interface.c | 3 +++
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
index 729deec..b4294ac 100644
--- a/net/batman-adv/send.c
+++ b/net/batman-adv/send.c
@@ -307,8 +307,7 @@ out:
*
* Wrap the given skb into a batman-adv unicast or unicast-4addr header
* depending on whether BATADV_UNICAST or BATADV_UNICAST_4ADDR was supplied
- * as packet_type. Then send this frame to the given orig_node and release a
- * reference to this orig_node.
+ * as packet_type. Then send this frame to the given orig_node.
*
* Return: NET_XMIT_DROP in case of error or NET_XMIT_SUCCESS otherwise.
*/
@@ -362,8 +361,6 @@ int batadv_send_skb_unicast(struct batadv_priv *bat_priv,
ret = NET_XMIT_SUCCESS;
out:
- if (orig_node)
- batadv_orig_node_put(orig_node);
if (ret == NET_XMIT_DROP)
kfree_skb(skb);
return ret;
@@ -395,6 +392,7 @@ int batadv_send_skb_via_tt_generic(struct batadv_priv *bat_priv,
struct ethhdr *ethhdr = (struct ethhdr *)skb->data;
struct batadv_orig_node *orig_node;
u8 *src, *dst;
+ int ret;
src = ethhdr->h_source;
dst = ethhdr->h_dest;
@@ -406,8 +404,13 @@ int batadv_send_skb_via_tt_generic(struct batadv_priv *bat_priv,
}
orig_node = batadv_transtable_search(bat_priv, src, dst, vid);
- return batadv_send_skb_unicast(bat_priv, skb, packet_type,
- packet_subtype, orig_node, vid);
+ ret = batadv_send_skb_unicast(bat_priv, skb, packet_type,
+ packet_subtype, orig_node, vid);
+
+ if (orig_node)
+ batadv_orig_node_put(orig_node);
+
+ return ret;
}
/**
@@ -425,10 +428,16 @@ int batadv_send_skb_via_gw(struct batadv_priv *bat_priv, struct sk_buff *skb,
unsigned short vid)
{
struct batadv_orig_node *orig_node;
+ int ret;
orig_node = batadv_gw_get_selected_orig(bat_priv);
- return batadv_send_skb_unicast(bat_priv, skb, BATADV_UNICAST, 0,
- orig_node, vid);
+ ret = batadv_send_skb_unicast(bat_priv, skb, BATADV_UNICAST, 0,
+ orig_node, vid);
+
+ if (orig_node)
+ batadv_orig_node_put(orig_node);
+
+ return ret;
}
void batadv_forw_packet_free(struct batadv_forw_packet *forw_packet)
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 216ac03..e508bf5 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -57,6 +57,7 @@
#include "hard-interface.h"
#include "multicast.h"
#include "network-coding.h"
+#include "originator.h"
#include "packet.h"
#include "send.h"
#include "sysfs.h"
@@ -377,6 +378,8 @@ dropped:
dropped_freed:
batadv_inc_counter(bat_priv, BATADV_CNT_TX_DROPPED);
end:
+ if (mcast_single_orig)
+ batadv_orig_node_put(mcast_single_orig);
if (primary_if)
batadv_hardif_put(primary_if);
return NETDEV_TX_OK;
--
2.8.1