17 Apr
2013
17 Apr
'13
9:54 p.m.
When removing a single interface while a broadcast or ogm packet is
still pending then we will free the forward packet without releasing the
queue slots again.
This patch is supposed to fix this issue.
Signed-off-by: Linus Lüssing <linus.luessing(a)web.de>
---
send.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/send.c b/send.c
index ed7072a..2d539d6 100644
--- a/send.c
+++ b/send.c
@@ -356,6 +356,9 @@ batadv_purge_outstanding_packets(struct batadv_priv *bat_priv,
if (pending) {
hlist_del(&forw_packet->list);
+ if (!forw_packet->own)
+ atomic_inc(&bat_priv->bcast_queue_left);
+
batadv_forw_packet_free(forw_packet);
}
}
@@ -382,6 +385,9 @@ batadv_purge_outstanding_packets(struct batadv_priv *bat_priv,
if (pending) {
hlist_del(&forw_packet->list);
+ if (!forw_packet->own)
+ atomic_inc(&bat_priv->batman_queue_left);
+
batadv_forw_packet_free(forw_packet);
}
}
--
1.7.10.4
17 Apr
17 Apr
9:54 p.m.
New subject: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Refactor forward packet creation/destruction
This patch abstracts the forward packet creation and destruction in the
functions batadv_forw_packet_alloc() and batadv_forw_packet_free().
That way there is less complexity to wrap the head around when freeing a
forward packet.
For instance broadcast/ogm queue left counting will not need
to be done with "manual" atomic_inc/dec calls anymore, which should
greatly reduce the risk of having or reintroducing another
queue-left-miscounting again.
Signed-off-by: Linus Lüssing <linus.luessing(a)web.de>
---
bat_iv_ogm.c | 32 ++++--------------
send.c | 103 +++++++++++++++++++++++++++++++++++++++++-----------------
send.h | 5 +++
types.h | 1 +
4 files changed, 86 insertions(+), 55 deletions(-)
diff --git a/bat_iv_ogm.c b/bat_iv_ogm.c
index 42b7a94..c113627 100644
--- a/bat_iv_ogm.c
+++ b/bat_iv_ogm.c
@@ -427,26 +427,13 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char
*packet_buff,
struct batadv_forw_packet *forw_packet_aggr;
unsigned char *skb_buff;
unsigned int skb_size;
+ atomic_t *queue_left = own_packet ? NULL : &bat_priv->batman_queue_left;
- if (!atomic_inc_not_zero(&if_incoming->refcount))
+ forw_packet_aggr = batadv_forw_packet_alloc(if_incoming, queue_left,
+ bat_priv);
+ if (!forw_packet_aggr)
return;
- /* own packet should always be scheduled */
- if (!own_packet) {
- if (!batadv_atomic_dec_not_zero(&bat_priv->batman_queue_left)) {
- batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
- "batman packet queue full\n");
- goto out;
- }
- }
-
- forw_packet_aggr = kmalloc(sizeof(*forw_packet_aggr), GFP_ATOMIC);
- if (!forw_packet_aggr) {
- if (!own_packet)
- atomic_inc(&bat_priv->batman_queue_left);
- goto out;
- }
-
if ((atomic_read(&bat_priv->aggregated_ogms)) &&
(packet_len < BATADV_MAX_AGGREGATION_BYTES))
skb_size = BATADV_MAX_AGGREGATION_BYTES;
@@ -457,10 +444,8 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char
*packet_buff,
forw_packet_aggr->skb = dev_alloc_skb(skb_size);
if (!forw_packet_aggr->skb) {
- if (!own_packet)
- atomic_inc(&bat_priv->batman_queue_left);
- kfree(forw_packet_aggr);
- goto out;
+ batadv_forw_packet_free(forw_packet_aggr);
+ return;
}
skb_reserve(forw_packet_aggr->skb, ETH_HLEN + NET_IP_ALIGN);
@@ -471,7 +456,6 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char
*packet_buff,
memcpy(skb_buff, packet_buff, packet_len);
forw_packet_aggr->own = own_packet;
- forw_packet_aggr->if_incoming = if_incoming;
forw_packet_aggr->num_packets = 0;
forw_packet_aggr->direct_link_flags = BATADV_NO_FLAGS;
forw_packet_aggr->send_time = send_time;
@@ -491,10 +475,6 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char
*packet_buff,
queue_delayed_work(batadv_event_workqueue,
&forw_packet_aggr->delayed_work,
send_time - jiffies);
-
- return;
-out:
- batadv_hardif_free_ref(if_incoming);
}
/* aggregate a new packet into the existing ogm packet */
diff --git a/send.c b/send.c
index 2d539d6..36a1c4c 100644
--- a/send.c
+++ b/send.c
@@ -138,12 +138,77 @@ void batadv_schedule_bat_ogm(struct batadv_hard_iface *hard_iface)
bat_priv->bat_algo_ops->bat_ogm_schedule(hard_iface);
}
-static void batadv_forw_packet_free(struct batadv_forw_packet *forw_packet)
+/**
+ * batadv_forw_packet_alloc - Allocates a forwarding packet
+ * @if_incoming: The (optional) if_incoming to be grabbed
+ * @queue_left: The (optional) queue counter to decrease
+ * @bat_priv: The bat_priv for the mesh of this forw_packet
+ *
+ * Allocates a forwarding packet and tries to get a reference to the
+ * (optional) if_incoming and queue_left. If queue_left is NULL then
+ * bat_priv is optional, too.
+ *
+ * On success, returns the allocated forwarding packet. Otherwise returns
+ * NULL.
+ */
+struct batadv_forw_packet *
+batadv_forw_packet_alloc(struct batadv_hard_iface *if_incoming,
+ atomic_t *queue_left,
+ struct batadv_priv *bat_priv)
+{
+ struct batadv_forw_packet *forw_packet = NULL;
+
+ if (if_incoming && !atomic_inc_not_zero(&if_incoming->refcount))
+ goto out;
+
+ if (queue_left && !batadv_atomic_dec_not_zero(queue_left)) {
+ if (queue_left == &bat_priv->bcast_queue_left)
+ batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
+ "bcast queue full\n");
+ else if (queue_left == &bat_priv->batman_queue_left)
+ batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
+ "batman queue full\n");
+ else
+ batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
+ "a mysterious queue is full\n");
+ goto err;
+ }
+
+ forw_packet = kmalloc(sizeof(struct batadv_forw_packet), GFP_ATOMIC);
+ if (!forw_packet)
+ goto err2;
+
+ forw_packet->skb = NULL;
+ forw_packet->if_incoming = if_incoming;
+ forw_packet->queue_left = queue_left;
+
+ goto out;
+
+err2:
+ if (queue_left)
+ atomic_inc(queue_left);
+err:
+ if (if_incoming)
+ batadv_hardif_free_ref(if_incoming);
+out:
+ return forw_packet;
+}
+
+/**
+ * batadv_forw_packet_free - Frees a forwarding packet
+ * @forw_packet: The packet to free
+ *
+ * This frees a forwarding packet and releases any ressources it might
+ * have claimed.
+ */
+void batadv_forw_packet_free(struct batadv_forw_packet *forw_packet)
{
if (forw_packet->skb)
kfree_skb(forw_packet->skb);
if (forw_packet->if_incoming)
batadv_hardif_free_ref(forw_packet->if_incoming);
+ if (forw_packet->queue_left)
+ atomic_inc(forw_packet->queue_left);
kfree(forw_packet);
}
@@ -177,25 +242,21 @@ int batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
const struct sk_buff *skb,
unsigned long delay)
{
- struct batadv_hard_iface *primary_if = NULL;
+ struct batadv_hard_iface *primary_if;
struct batadv_forw_packet *forw_packet;
struct batadv_bcast_packet *bcast_packet;
struct sk_buff *newskb;
- if (!batadv_atomic_dec_not_zero(&bat_priv->bcast_queue_left)) {
- batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
- "bcast packet queue full\n");
- goto out;
- }
-
primary_if = batadv_primary_if_get_selected(bat_priv);
if (!primary_if)
- goto out_and_inc;
-
- forw_packet = kmalloc(sizeof(*forw_packet), GFP_ATOMIC);
+ goto out;
+ forw_packet = batadv_forw_packet_alloc(primary_if,
+ &bat_priv->bcast_queue_left,
+ bat_priv);
+ batadv_hardif_free_ref(primary_if);
if (!forw_packet)
- goto out_and_inc;
+ goto out;
newskb = skb_copy(skb, GFP_ATOMIC);
if (!newskb)
@@ -208,7 +269,6 @@ int batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
skb_reset_mac_header(newskb);
forw_packet->skb = newskb;
- forw_packet->if_incoming = primary_if;
/* how often did we send the bcast packet ? */
forw_packet->num_packets = 0;
@@ -220,12 +280,8 @@ int batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
return NETDEV_TX_OK;
packet_free:
- kfree(forw_packet);
-out_and_inc:
- atomic_inc(&bat_priv->bcast_queue_left);
+ batadv_forw_packet_free(forw_packet);
out:
- if (primary_if)
- batadv_hardif_free_ref(primary_if);
return NETDEV_TX_BUSY;
}
@@ -282,7 +338,6 @@ static void batadv_send_outstanding_bcast_packet(struct work_struct
*work)
out:
batadv_forw_packet_free(forw_packet);
- atomic_inc(&bat_priv->bcast_queue_left);
}
void batadv_send_outstanding_bat_ogm_packet(struct work_struct *work)
@@ -312,10 +367,6 @@ void batadv_send_outstanding_bat_ogm_packet(struct work_struct
*work)
batadv_schedule_bat_ogm(forw_packet->if_incoming);
out:
- /* don't count own packet */
- if (!forw_packet->own)
- atomic_inc(&bat_priv->batman_queue_left);
-
batadv_forw_packet_free(forw_packet);
}
@@ -356,9 +407,6 @@ batadv_purge_outstanding_packets(struct batadv_priv *bat_priv,
if (pending) {
hlist_del(&forw_packet->list);
- if (!forw_packet->own)
- atomic_inc(&bat_priv->bcast_queue_left);
-
batadv_forw_packet_free(forw_packet);
}
}
@@ -385,9 +433,6 @@ batadv_purge_outstanding_packets(struct batadv_priv *bat_priv,
if (pending) {
hlist_del(&forw_packet->list);
- if (!forw_packet->own)
- atomic_inc(&bat_priv->batman_queue_left);
-
batadv_forw_packet_free(forw_packet);
}
}
diff --git a/send.h b/send.h
index 38e662f..7004486 100644
--- a/send.h
+++ b/send.h
@@ -27,6 +27,11 @@ bool batadv_send_skb_to_orig(struct sk_buff *skb,
struct batadv_orig_node *orig_node,
struct batadv_hard_iface *recv_if);
void batadv_schedule_bat_ogm(struct batadv_hard_iface *hard_iface);
+struct batadv_forw_packet *batadv_forw_packet_alloc(
+ struct batadv_hard_iface *if_incoming,
+ atomic_t *queue_left,
+ struct batadv_priv *bat_priv);
+void batadv_forw_packet_free(struct batadv_forw_packet *forw_packet);
int batadv_add_bcast_packet_to_list(struct batadv_priv *bat_priv,
const struct sk_buff *skb,
unsigned long delay);
diff --git a/types.h b/types.h
index 5f542bd..8c28142 100644
--- a/types.h
+++ b/types.h
@@ -863,6 +863,7 @@ struct batadv_forw_packet {
uint8_t num_packets;
struct delayed_work delayed_work;
struct batadv_hard_iface *if_incoming;
+ atomic_t *queue_left;
};
/**
--
1.7.10.4
10 Mar
10 Mar
5:08 p.m.
New subject: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Refactor forward packet creation/destruction
On Wednesday 17 April 2013 23:54:33 Linus Lüssing wrote:
This patch abstracts the forward packet creation and
destruction in the
functions batadv_forw_packet_alloc() and batadv_forw_packet_free().
That way there is less complexity to wrap the head around when freeing a
forward packet.
For instance broadcast/ogm queue left counting will not need
to be done with "manual" atomic_inc/dec calls anymore, which should
greatly reduce the risk of having or reintroducing another
queue-left-miscounting again.
Signed-off-by: Linus Lüssing <linus.luessing(a)web.de>
---
It looks like this patch doesn't apply anymore. Can you please resent it or
mark it correctly in patchwork [1].
Thanks,
Sven
[1] https://patchwork.open-mesh.org/patch/2891/
9 Apr
9 Apr
4:23 p.m.
New subject: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Refactor forward packet creation/destruction
On Thursday 10 March 2016 18:08:27 Sven Eckelmann wrote:
[...]
It looks like this patch doesn't apply anymore.
Can you please resent it or
mark it correctly in patchwork [1].
Thanks,
Sven
[1] https://patchwork.open-mesh.org/patch/2891/
What is the state here?
Kind regards,
Sven
17 Apr
17 Apr
10:08 p.m.
New subject: [B.A.T.M.A.N.] [PATCHv2 1/2] batman-adv: Fix broadcast/ogm queue limit on a removed interface
When removing a single interface while a broadcast or ogm packet is
still pending then we will free the forward packet without releasing the
queue slots again.
This patch is supposed to fix this issue.
Signed-off-by: Linus Lüssing <linus.luessing(a)web.de>
---
send.c | 6 ++++++
1 file changed, 6 insertions(+)
* v2: Fix summary line of commit message: This issue can happen for both
OGMs and broadcast packets.
diff --git a/send.c b/send.c
index ed7072a..2d539d6 100644
--- a/send.c
+++ b/send.c
@@ -356,6 +356,9 @@ batadv_purge_outstanding_packets(struct batadv_priv *bat_priv,
if (pending) {
hlist_del(&forw_packet->list);
+ if (!forw_packet->own)
+ atomic_inc(&bat_priv->bcast_queue_left);
+
batadv_forw_packet_free(forw_packet);
}
}
@@ -382,6 +385,9 @@ batadv_purge_outstanding_packets(struct batadv_priv *bat_priv,
if (pending) {
hlist_del(&forw_packet->list);
+ if (!forw_packet->own)
+ atomic_inc(&bat_priv->batman_queue_left);
+
batadv_forw_packet_free(forw_packet);
}
}
--
1.7.10.4
10 Mar
10 Mar
5:44 p.m.
New subject: [B.A.T.M.A.N.] [PATCHv2 1/2] batman-adv: Fix broadcast/ogm queue limit on a removed interface
On Thursday 18 April 2013 00:08:36 Linus Lüssing wrote:
When removing a single interface while a broadcast or
ogm packet is
still pending then we will free the forward packet without releasing the
queue slots again.
This patch is supposed to fix this issue.
Signed-off-by: Linus Lüssing <linus.luessing(a)web.de>
---
You are talking a batman-adv interface when it contains multiple slave
interfaces, right? So batadv_purge_outstanding_packets would be called in
batadv_hardif_disable_interface and not in batadv_mesh_free (which is only
done when the batX interface will be removed).
This at least sounds legit and I cannot find where else this imbalance would
be fixed.
Acked-by: Sven Eckelmann <sven(a)narfation.org>
Kind regards,
Sven
PS: This patch only requires the path change from / to /net/batman-adv/ to
apply via git-am
2543
days inactive
3631
days old
b.a.t.m.a.n@lists.open-mesh.org
5 comments
2 participants
participants (2)
-
Linus Lüssing -
Sven Eckelmann