* [PATCH 0/5] pull request for net: batman-adv 2025-03-13
@ 2025-03-13 16:17 Simon Wunderlich
2025-03-13 16:17 ` [PATCH 1/5] batman-adv: fix panic during interface removal Simon Wunderlich
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Simon Wunderlich @ 2025-03-13 16:17 UTC (permalink / raw)
To: davem, kuba; +Cc: netdev, b.a.t.m.a.n, Simon Wunderlich
Hi David, hi Jakub,
here are some bugfixes for batman-adv which we would like to have integrated into net.
Please pull or let me know of any problem!
Thank you,
Simon
The following changes since commit fff8f17c1a6fc802ca23bbd3a276abfde8cc58e6:
batman-adv: Do not let TT changes list grows indefinitely (2024-12-05 22:38:26 +0100)
are available in the Git repository at:
git://git.open-mesh.org/linux-merge.git tags/batadv-net-pullrequest-20250313
for you to fetch changes up to 548b0c5de7619ef53bbde5590700693f2f6d2a56:
batman-adv: Ignore own maximum aggregation size during RX (2025-02-08 19:24:33 +0100)
----------------------------------------------------------------
Here are some batman-adv bugfixes:
- fix panic during interface removal, by Andy Strohman
- Ignore neighbor throughput metrics in error case, by Sven Eckelmann
- Drop unmanaged ELP metric worker, by Sven Eckelmann
- Fix incorrect offset in batadv_tt_tvlv_ogm_handler_v1(), by Remi Pommarel
- Ignore own maximum aggregation size during RX, Sven Eckelmann
----------------------------------------------------------------
Andy Strohman (1):
batman-adv: fix panic during interface removal
Remi Pommarel (1):
batman-adv: Fix incorrect offset in batadv_tt_tvlv_ogm_handler_v1()
Sven Eckelmann (3):
batman-adv: Ignore neighbor throughput metrics in error case
batman-adv: Drop unmanaged ELP metric worker
batman-adv: Ignore own maximum aggregation size during RX
net/batman-adv/bat_iv_ogm.c | 3 +-
net/batman-adv/bat_v.c | 2 -
net/batman-adv/bat_v_elp.c | 122 ++++++++++++++++++++++++++-----------
net/batman-adv/bat_v_elp.h | 2 -
net/batman-adv/bat_v_ogm.c | 3 +-
net/batman-adv/translation-table.c | 12 ++--
net/batman-adv/types.h | 3 -
7 files changed, 93 insertions(+), 54 deletions(-)
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/5] batman-adv: fix panic during interface removal
2025-03-13 16:17 [PATCH 0/5] pull request for net: batman-adv 2025-03-13 Simon Wunderlich
@ 2025-03-13 16:17 ` Simon Wunderlich
2025-03-13 16:17 ` [PATCH 2/5] batman-adv: Ignore neighbor throughput metrics in error case Simon Wunderlich
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Simon Wunderlich @ 2025-03-13 16:17 UTC (permalink / raw)
To: davem, kuba
Cc: netdev, b.a.t.m.a.n, Andy Strohman, stable, Sven Eckelmann,
Simon Wunderlich
From: Andy Strohman <andrew@andrewstrohman.com>
Reference counting is used to ensure that
batadv_hardif_neigh_node and batadv_hard_iface
are not freed before/during
batadv_v_elp_throughput_metric_update work is
finished.
But there isn't a guarantee that the hard if will
remain associated with a soft interface up until
the work is finished.
This fixes a crash triggered by reboot that looks
like this:
Call trace:
batadv_v_mesh_free+0xd0/0x4dc [batman_adv]
batadv_v_elp_throughput_metric_update+0x1c/0xa4
process_one_work+0x178/0x398
worker_thread+0x2e8/0x4d0
kthread+0xd8/0xdc
ret_from_fork+0x10/0x20
(the batadv_v_mesh_free call is misleading,
and does not actually happen)
I was able to make the issue happen more reliably
by changing hardif_neigh->bat_v.metric_work work
to be delayed work. This allowed me to track down
and confirm the fix.
Cc: stable@vger.kernel.org
Fixes: c833484e5f38 ("batman-adv: ELP - compute the metric based on the estimated throughput")
Signed-off-by: Andy Strohman <andrew@andrewstrohman.com>
[sven@narfation.org: prevent entering batadv_v_elp_get_throughput without
soft_iface]
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/bat_v_elp.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
index 1d704574e6bf..fbf499bcc671 100644
--- a/net/batman-adv/bat_v_elp.c
+++ b/net/batman-adv/bat_v_elp.c
@@ -66,12 +66,19 @@ static void batadv_v_elp_start_timer(struct batadv_hard_iface *hard_iface)
static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
{
struct batadv_hard_iface *hard_iface = neigh->if_incoming;
+ struct net_device *soft_iface = hard_iface->soft_iface;
struct ethtool_link_ksettings link_settings;
struct net_device *real_netdev;
struct station_info sinfo;
u32 throughput;
int ret;
+ /* don't query throughput when no longer associated with any
+ * batman-adv interface
+ */
+ if (!soft_iface)
+ return BATADV_THROUGHPUT_DEFAULT_VALUE;
+
/* if the user specified a customised value for this interface, then
* return it directly
*/
@@ -141,7 +148,7 @@ static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
default_throughput:
if (!(hard_iface->bat_v.flags & BATADV_WARNING_DEFAULT)) {
- batadv_info(hard_iface->soft_iface,
+ batadv_info(soft_iface,
"WiFi driver or ethtool info does not provide information about link speeds on interface %s, therefore defaulting to hardcoded throughput values of %u.%1u Mbps. Consider overriding the throughput manually or checking your driver.\n",
hard_iface->net_dev->name,
BATADV_THROUGHPUT_DEFAULT_VALUE / 10,
--
2.39.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/5] batman-adv: Ignore neighbor throughput metrics in error case
2025-03-13 16:17 [PATCH 0/5] pull request for net: batman-adv 2025-03-13 Simon Wunderlich
2025-03-13 16:17 ` [PATCH 1/5] batman-adv: fix panic during interface removal Simon Wunderlich
@ 2025-03-13 16:17 ` Simon Wunderlich
2025-03-13 16:17 ` [PATCH 3/5] batman-adv: Drop unmanaged ELP metric worker Simon Wunderlich
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Simon Wunderlich @ 2025-03-13 16:17 UTC (permalink / raw)
To: davem, kuba; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, stable, Simon Wunderlich
From: Sven Eckelmann <sven@narfation.org>
If a temporary error happened in the evaluation of the neighbor throughput
information, then the invalid throughput result should not be stored in the
throughtput EWMA.
Cc: stable@vger.kernel.org
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/bat_v_elp.c | 50 ++++++++++++++++++++++++++------------
1 file changed, 34 insertions(+), 16 deletions(-)
diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
index fbf499bcc671..65e52de52bcd 100644
--- a/net/batman-adv/bat_v_elp.c
+++ b/net/batman-adv/bat_v_elp.c
@@ -59,11 +59,13 @@ static void batadv_v_elp_start_timer(struct batadv_hard_iface *hard_iface)
/**
* batadv_v_elp_get_throughput() - get the throughput towards a neighbour
* @neigh: the neighbour for which the throughput has to be obtained
+ * @pthroughput: calculated throughput towards the given neighbour in multiples
+ * of 100kpbs (a value of '1' equals 0.1Mbps, '10' equals 1Mbps, etc).
*
- * Return: The throughput towards the given neighbour in multiples of 100kpbs
- * (a value of '1' equals 0.1Mbps, '10' equals 1Mbps, etc).
+ * Return: true when value behind @pthroughput was set
*/
-static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
+static bool batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh,
+ u32 *pthroughput)
{
struct batadv_hard_iface *hard_iface = neigh->if_incoming;
struct net_device *soft_iface = hard_iface->soft_iface;
@@ -77,14 +79,16 @@ static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
* batman-adv interface
*/
if (!soft_iface)
- return BATADV_THROUGHPUT_DEFAULT_VALUE;
+ return false;
/* if the user specified a customised value for this interface, then
* return it directly
*/
throughput = atomic_read(&hard_iface->bat_v.throughput_override);
- if (throughput != 0)
- return throughput;
+ if (throughput != 0) {
+ *pthroughput = throughput;
+ return true;
+ }
/* if this is a wireless device, then ask its throughput through
* cfg80211 API
@@ -111,19 +115,24 @@ static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
* possible to delete this neighbor. For now set
* the throughput metric to 0.
*/
- return 0;
+ *pthroughput = 0;
+ return true;
}
if (ret)
goto default_throughput;
- if (sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT))
- return sinfo.expected_throughput / 100;
+ if (sinfo.filled & BIT(NL80211_STA_INFO_EXPECTED_THROUGHPUT)) {
+ *pthroughput = sinfo.expected_throughput / 100;
+ return true;
+ }
/* try to estimate the expected throughput based on reported tx
* rates
*/
- if (sinfo.filled & BIT(NL80211_STA_INFO_TX_BITRATE))
- return cfg80211_calculate_bitrate(&sinfo.txrate) / 3;
+ if (sinfo.filled & BIT(NL80211_STA_INFO_TX_BITRATE)) {
+ *pthroughput = cfg80211_calculate_bitrate(&sinfo.txrate) / 3;
+ return true;
+ }
goto default_throughput;
}
@@ -142,8 +151,10 @@ static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
hard_iface->bat_v.flags &= ~BATADV_FULL_DUPLEX;
throughput = link_settings.base.speed;
- if (throughput && throughput != SPEED_UNKNOWN)
- return throughput * 10;
+ if (throughput && throughput != SPEED_UNKNOWN) {
+ *pthroughput = throughput * 10;
+ return true;
+ }
}
default_throughput:
@@ -157,7 +168,8 @@ static u32 batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh)
}
/* if none of the above cases apply, return the base_throughput */
- return BATADV_THROUGHPUT_DEFAULT_VALUE;
+ *pthroughput = BATADV_THROUGHPUT_DEFAULT_VALUE;
+ return true;
}
/**
@@ -169,15 +181,21 @@ void batadv_v_elp_throughput_metric_update(struct work_struct *work)
{
struct batadv_hardif_neigh_node_bat_v *neigh_bat_v;
struct batadv_hardif_neigh_node *neigh;
+ u32 throughput;
+ bool valid;
neigh_bat_v = container_of(work, struct batadv_hardif_neigh_node_bat_v,
metric_work);
neigh = container_of(neigh_bat_v, struct batadv_hardif_neigh_node,
bat_v);
- ewma_throughput_add(&neigh->bat_v.throughput,
- batadv_v_elp_get_throughput(neigh));
+ valid = batadv_v_elp_get_throughput(neigh, &throughput);
+ if (!valid)
+ goto put_neigh;
+
+ ewma_throughput_add(&neigh->bat_v.throughput, throughput);
+put_neigh:
/* decrement refcounter to balance increment performed before scheduling
* this task
*/
--
2.39.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/5] batman-adv: Drop unmanaged ELP metric worker
2025-03-13 16:17 [PATCH 0/5] pull request for net: batman-adv 2025-03-13 Simon Wunderlich
2025-03-13 16:17 ` [PATCH 1/5] batman-adv: fix panic during interface removal Simon Wunderlich
2025-03-13 16:17 ` [PATCH 2/5] batman-adv: Ignore neighbor throughput metrics in error case Simon Wunderlich
@ 2025-03-13 16:17 ` Simon Wunderlich
2025-03-13 16:17 ` [PATCH 4/5] batman-adv: Fix incorrect offset in batadv_tt_tvlv_ogm_handler_v1() Simon Wunderlich
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Simon Wunderlich @ 2025-03-13 16:17 UTC (permalink / raw)
To: davem, kuba; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, stable, Simon Wunderlich
From: Sven Eckelmann <sven@narfation.org>
The ELP worker needs to calculate new metric values for all neighbors
"reachable" over an interface. Some of the used metric sources require
locks which might need to sleep. This sleep is incompatible with the RCU
list iterator used for the recorded neighbors. The initial approach to work
around of this problem was to queue another work item per neighbor and then
run this in a new context.
Even when this solved the RCU vs might_sleep() conflict, it has a major
problems: Nothing was stopping the work item in case it is not needed
anymore - for example because one of the related interfaces was removed or
the batman-adv module was unloaded - resulting in potential invalid memory
accesses.
Directly canceling the metric worker also has various problems:
* cancel_work_sync for a to-be-deactivated interface is called with
rtnl_lock held. But the code in the ELP metric worker also tries to use
rtnl_lock() - which will never return in this case. This also means that
cancel_work_sync would never return because it is waiting for the worker
to finish.
* iterating over the neighbor list for the to-be-deactivated interface is
currently done using the RCU specific methods. Which means that it is
possible to miss items when iterating over it without the associated
spinlock - a behaviour which is acceptable for a periodic metric check
but not for a cleanup routine (which must "stop" all still running
workers)
The better approch is to get rid of the per interface neighbor metric
worker and handle everything in the interface worker. The original problems
are solved by:
* creating a list of neighbors which require new metric information inside
the RCU protected context, gathering the metric according to the new list
outside the RCU protected context
* only use rcu_trylock inside metric gathering code to avoid a deadlock
when the cancel_delayed_work_sync is called in the interface removal code
(which is called with the rtnl_lock held)
Cc: stable@vger.kernel.org
Fixes: c833484e5f38 ("batman-adv: ELP - compute the metric based on the estimated throughput")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/bat_v.c | 2 --
net/batman-adv/bat_v_elp.c | 71 ++++++++++++++++++++++++++------------
net/batman-adv/bat_v_elp.h | 2 --
net/batman-adv/types.h | 3 --
4 files changed, 48 insertions(+), 30 deletions(-)
diff --git a/net/batman-adv/bat_v.c b/net/batman-adv/bat_v.c
index ac11f1f08db0..d35479c465e2 100644
--- a/net/batman-adv/bat_v.c
+++ b/net/batman-adv/bat_v.c
@@ -113,8 +113,6 @@ static void
batadv_v_hardif_neigh_init(struct batadv_hardif_neigh_node *hardif_neigh)
{
ewma_throughput_init(&hardif_neigh->bat_v.throughput);
- INIT_WORK(&hardif_neigh->bat_v.metric_work,
- batadv_v_elp_throughput_metric_update);
}
/**
diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
index 65e52de52bcd..b065578b4436 100644
--- a/net/batman-adv/bat_v_elp.c
+++ b/net/batman-adv/bat_v_elp.c
@@ -18,6 +18,7 @@
#include <linux/if_ether.h>
#include <linux/jiffies.h>
#include <linux/kref.h>
+#include <linux/list.h>
#include <linux/minmax.h>
#include <linux/netdevice.h>
#include <linux/nl80211.h>
@@ -26,6 +27,7 @@
#include <linux/rcupdate.h>
#include <linux/rtnetlink.h>
#include <linux/skbuff.h>
+#include <linux/slab.h>
#include <linux/stddef.h>
#include <linux/string.h>
#include <linux/types.h>
@@ -41,6 +43,18 @@
#include "routing.h"
#include "send.h"
+/**
+ * struct batadv_v_metric_queue_entry - list of hardif neighbors which require
+ * and metric update
+ */
+struct batadv_v_metric_queue_entry {
+ /** @hardif_neigh: hardif neighbor scheduled for metric update */
+ struct batadv_hardif_neigh_node *hardif_neigh;
+
+ /** @list: list node for metric_queue */
+ struct list_head list;
+};
+
/**
* batadv_v_elp_start_timer() - restart timer for ELP periodic work
* @hard_iface: the interface for which the timer has to be reset
@@ -137,10 +151,17 @@ static bool batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh,
goto default_throughput;
}
+ /* only use rtnl_trylock because the elp worker will be cancelled while
+ * the rntl_lock is held. the cancel_delayed_work_sync() would otherwise
+ * wait forever when the elp work_item was started and it is then also
+ * trying to rtnl_lock
+ */
+ if (!rtnl_trylock())
+ return false;
+
/* if not a wifi interface, check if this device provides data via
* ethtool (e.g. an Ethernet adapter)
*/
- rtnl_lock();
ret = __ethtool_get_link_ksettings(hard_iface->net_dev, &link_settings);
rtnl_unlock();
if (ret == 0) {
@@ -175,31 +196,19 @@ static bool batadv_v_elp_get_throughput(struct batadv_hardif_neigh_node *neigh,
/**
* batadv_v_elp_throughput_metric_update() - worker updating the throughput
* metric of a single hop neighbour
- * @work: the work queue item
+ * @neigh: the neighbour to probe
*/
-void batadv_v_elp_throughput_metric_update(struct work_struct *work)
+static void
+batadv_v_elp_throughput_metric_update(struct batadv_hardif_neigh_node *neigh)
{
- struct batadv_hardif_neigh_node_bat_v *neigh_bat_v;
- struct batadv_hardif_neigh_node *neigh;
u32 throughput;
bool valid;
- neigh_bat_v = container_of(work, struct batadv_hardif_neigh_node_bat_v,
- metric_work);
- neigh = container_of(neigh_bat_v, struct batadv_hardif_neigh_node,
- bat_v);
-
valid = batadv_v_elp_get_throughput(neigh, &throughput);
if (!valid)
- goto put_neigh;
+ return;
ewma_throughput_add(&neigh->bat_v.throughput, throughput);
-
-put_neigh:
- /* decrement refcounter to balance increment performed before scheduling
- * this task
- */
- batadv_hardif_neigh_put(neigh);
}
/**
@@ -273,14 +282,16 @@ batadv_v_elp_wifi_neigh_probe(struct batadv_hardif_neigh_node *neigh)
*/
static void batadv_v_elp_periodic_work(struct work_struct *work)
{
+ struct batadv_v_metric_queue_entry *metric_entry;
+ struct batadv_v_metric_queue_entry *metric_safe;
struct batadv_hardif_neigh_node *hardif_neigh;
struct batadv_hard_iface *hard_iface;
struct batadv_hard_iface_bat_v *bat_v;
struct batadv_elp_packet *elp_packet;
+ struct list_head metric_queue;
struct batadv_priv *bat_priv;
struct sk_buff *skb;
u32 elp_interval;
- bool ret;
bat_v = container_of(work, struct batadv_hard_iface_bat_v, elp_wq.work);
hard_iface = container_of(bat_v, struct batadv_hard_iface, bat_v);
@@ -316,6 +327,8 @@ static void batadv_v_elp_periodic_work(struct work_struct *work)
atomic_inc(&hard_iface->bat_v.elp_seqno);
+ INIT_LIST_HEAD(&metric_queue);
+
/* The throughput metric is updated on each sent packet. This way, if a
* node is dead and no longer sends packets, batman-adv is still able to
* react timely to its death.
@@ -340,16 +353,28 @@ static void batadv_v_elp_periodic_work(struct work_struct *work)
/* Reading the estimated throughput from cfg80211 is a task that
* may sleep and that is not allowed in an rcu protected
- * context. Therefore schedule a task for that.
+ * context. Therefore add it to metric_queue and process it
+ * outside rcu protected context.
*/
- ret = queue_work(batadv_event_workqueue,
- &hardif_neigh->bat_v.metric_work);
-
- if (!ret)
+ metric_entry = kzalloc(sizeof(*metric_entry), GFP_ATOMIC);
+ if (!metric_entry) {
batadv_hardif_neigh_put(hardif_neigh);
+ continue;
+ }
+
+ metric_entry->hardif_neigh = hardif_neigh;
+ list_add(&metric_entry->list, &metric_queue);
}
rcu_read_unlock();
+ list_for_each_entry_safe(metric_entry, metric_safe, &metric_queue, list) {
+ batadv_v_elp_throughput_metric_update(metric_entry->hardif_neigh);
+
+ batadv_hardif_neigh_put(metric_entry->hardif_neigh);
+ list_del(&metric_entry->list);
+ kfree(metric_entry);
+ }
+
restart_timer:
batadv_v_elp_start_timer(hard_iface);
out:
diff --git a/net/batman-adv/bat_v_elp.h b/net/batman-adv/bat_v_elp.h
index 9e2740195fa2..c9cb0a307100 100644
--- a/net/batman-adv/bat_v_elp.h
+++ b/net/batman-adv/bat_v_elp.h
@@ -10,7 +10,6 @@
#include "main.h"
#include <linux/skbuff.h>
-#include <linux/workqueue.h>
int batadv_v_elp_iface_enable(struct batadv_hard_iface *hard_iface);
void batadv_v_elp_iface_disable(struct batadv_hard_iface *hard_iface);
@@ -19,6 +18,5 @@ void batadv_v_elp_iface_activate(struct batadv_hard_iface *primary_iface,
void batadv_v_elp_primary_iface_set(struct batadv_hard_iface *primary_iface);
int batadv_v_elp_packet_recv(struct sk_buff *skb,
struct batadv_hard_iface *if_incoming);
-void batadv_v_elp_throughput_metric_update(struct work_struct *work);
#endif /* _NET_BATMAN_ADV_BAT_V_ELP_H_ */
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 04f6398b3a40..85a50096f5b2 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -596,9 +596,6 @@ struct batadv_hardif_neigh_node_bat_v {
* neighbor
*/
unsigned long last_unicast_tx;
-
- /** @metric_work: work queue callback item for metric update */
- struct work_struct metric_work;
};
/**
--
2.39.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/5] batman-adv: Fix incorrect offset in batadv_tt_tvlv_ogm_handler_v1()
2025-03-13 16:17 [PATCH 0/5] pull request for net: batman-adv 2025-03-13 Simon Wunderlich
` (2 preceding siblings ...)
2025-03-13 16:17 ` [PATCH 3/5] batman-adv: Drop unmanaged ELP metric worker Simon Wunderlich
@ 2025-03-13 16:17 ` Simon Wunderlich
2025-03-13 16:17 ` [PATCH 5/5] batman-adv: Ignore own maximum aggregation size during RX Simon Wunderlich
2025-03-18 11:05 ` [PATCH 0/5] pull request for net: batman-adv 2025-03-13 Paolo Abeni
5 siblings, 0 replies; 9+ messages in thread
From: Simon Wunderlich @ 2025-03-13 16:17 UTC (permalink / raw)
To: davem, kuba
Cc: netdev, b.a.t.m.a.n, Remi Pommarel, stable, Sven Eckelmann,
Simon Wunderlich
From: Remi Pommarel <repk@triplefau.lt>
Since commit 4436df478860 ("batman-adv: Add flex array to struct
batadv_tvlv_tt_data"), the introduction of batadv_tvlv_tt_data's flex
array member in batadv_tt_tvlv_ogm_handler_v1() put tt_changes at
invalid offset. Those TT changes are supposed to be filled from the end
of batadv_tvlv_tt_data structure (including vlan_data flexible array),
but only the flex array size is taken into account missing completely
the size of the fixed part of the structure itself.
Fix the tt_change offset computation by using struct_size() instead of
flex_array_size() so both flex array member and its container structure
sizes are taken into account.
Cc: stable@vger.kernel.org
Fixes: 4436df478860 ("batman-adv: Add flex array to struct batadv_tvlv_tt_data")
Signed-off-by: Remi Pommarel <repk@triplefau.lt>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/translation-table.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 760d51fdbdf6..7d5de4cbb814 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -3959,23 +3959,21 @@ static void batadv_tt_tvlv_ogm_handler_v1(struct batadv_priv *bat_priv,
struct batadv_tvlv_tt_change *tt_change;
struct batadv_tvlv_tt_data *tt_data;
u16 num_entries, num_vlan;
- size_t flex_size;
+ size_t tt_data_sz;
if (tvlv_value_len < sizeof(*tt_data))
return;
tt_data = tvlv_value;
- tvlv_value_len -= sizeof(*tt_data);
-
num_vlan = ntohs(tt_data->num_vlan);
- flex_size = flex_array_size(tt_data, vlan_data, num_vlan);
- if (tvlv_value_len < flex_size)
+ tt_data_sz = struct_size(tt_data, vlan_data, num_vlan);
+ if (tvlv_value_len < tt_data_sz)
return;
tt_change = (struct batadv_tvlv_tt_change *)((void *)tt_data
- + flex_size);
- tvlv_value_len -= flex_size;
+ + tt_data_sz);
+ tvlv_value_len -= tt_data_sz;
num_entries = batadv_tt_entries(tvlv_value_len);
--
2.39.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 5/5] batman-adv: Ignore own maximum aggregation size during RX
2025-03-13 16:17 [PATCH 0/5] pull request for net: batman-adv 2025-03-13 Simon Wunderlich
` (3 preceding siblings ...)
2025-03-13 16:17 ` [PATCH 4/5] batman-adv: Fix incorrect offset in batadv_tt_tvlv_ogm_handler_v1() Simon Wunderlich
@ 2025-03-13 16:17 ` Simon Wunderlich
2025-03-18 11:05 ` [PATCH 0/5] pull request for net: batman-adv 2025-03-13 Paolo Abeni
5 siblings, 0 replies; 9+ messages in thread
From: Simon Wunderlich @ 2025-03-13 16:17 UTC (permalink / raw)
To: davem, kuba; +Cc: netdev, b.a.t.m.a.n, Sven Eckelmann, stable, Simon Wunderlich
From: Sven Eckelmann <sven@narfation.org>
An OGMv1 and OGMv2 packet receive processing were not only limited by the
number of bytes in the received packet but also by the nodes maximum
aggregation packet size limit. But this limit is relevant for TX and not
for RX. It must not be enforced by batadv_(i)v_ogm_aggr_packet to avoid
loss of information in case of a different limit for sender and receiver.
This has a minor side effect for B.A.T.M.A.N. IV because the
batadv_iv_ogm_aggr_packet is also used for the preprocessing for the TX.
But since the aggregation code itself will not allow more than
BATADV_MAX_AGGREGATION_BYTES bytes, this check was never triggering (in
this context) prior of removing it.
Cc: stable@vger.kernel.org
Fixes: c6c8fea29769 ("net: Add batman-adv meshing protocol")
Fixes: 9323158ef9f4 ("batman-adv: OGMv2 - implement originators logic")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
net/batman-adv/bat_iv_ogm.c | 3 +--
net/batman-adv/bat_v_ogm.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 07ae5dd1f150..b12645949ae5 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -325,8 +325,7 @@ batadv_iv_ogm_aggr_packet(int buff_pos, int packet_len,
/* check if there is enough space for the optional TVLV */
next_buff_pos += ntohs(ogm_packet->tvlv_len);
- return (next_buff_pos <= packet_len) &&
- (next_buff_pos <= BATADV_MAX_AGGREGATION_BYTES);
+ return next_buff_pos <= packet_len;
}
/* send a batman ogm to a given interface */
diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c
index e503ee0d896b..8f89ffe6020c 100644
--- a/net/batman-adv/bat_v_ogm.c
+++ b/net/batman-adv/bat_v_ogm.c
@@ -839,8 +839,7 @@ batadv_v_ogm_aggr_packet(int buff_pos, int packet_len,
/* check if there is enough space for the optional TVLV */
next_buff_pos += ntohs(ogm2_packet->tvlv_len);
- return (next_buff_pos <= packet_len) &&
- (next_buff_pos <= BATADV_MAX_AGGREGATION_BYTES);
+ return next_buff_pos <= packet_len;
}
/**
--
2.39.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/5] pull request for net: batman-adv 2025-03-13
2025-03-13 16:17 [PATCH 0/5] pull request for net: batman-adv 2025-03-13 Simon Wunderlich
` (4 preceding siblings ...)
2025-03-13 16:17 ` [PATCH 5/5] batman-adv: Ignore own maximum aggregation size during RX Simon Wunderlich
@ 2025-03-18 11:05 ` Paolo Abeni
2025-03-18 13:56 ` Sven Eckelmann
5 siblings, 1 reply; 9+ messages in thread
From: Paolo Abeni @ 2025-03-18 11:05 UTC (permalink / raw)
To: Simon Wunderlich, davem, kuba; +Cc: netdev, b.a.t.m.a.n
Hi,
On 3/13/25 5:17 PM, Simon Wunderlich wrote:
> here are some bugfixes for batman-adv which we would like to have integrated into net.
>
> Please pull or let me know of any problem!
I'm sorry, we are lagging behind.
The series does not apply cleanly to the net tree, could you please
rebase it?
While at it, could you please include the target tree in the subj prefix?
Thanks!
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/5] pull request for net: batman-adv 2025-03-13
2025-03-18 11:05 ` [PATCH 0/5] pull request for net: batman-adv 2025-03-13 Paolo Abeni
@ 2025-03-18 13:56 ` Sven Eckelmann
2025-03-18 14:27 ` Paolo Abeni
0 siblings, 1 reply; 9+ messages in thread
From: Sven Eckelmann @ 2025-03-18 13:56 UTC (permalink / raw)
To: Simon Wunderlich, davem, Paolo Abeni; +Cc: kuba, netdev, b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 1267 bytes --]
On Tuesday, 18 March 2025 12:05:52 CET Paolo Abeni wrote:
> The series does not apply cleanly to the net tree, could you please
> rebase it?
$ git log -1 --oneline
9a81fc3480bf (HEAD, net/main) ipv6: Set errno after ip_fib_metrics_init() in ip6_route_info_create().
$ git pull --no-ff git://git.open-mesh.org/linux-merge.git tags/batadv-net-pullrequest-20250313
From git://git.open-mesh.org/linux-merge
* tag batadv-net-pullrequest-20250313 -> FETCH_HEAD
Merge made by the 'ort' strategy.
net/batman-adv/bat_iv_ogm.c | 3 +--
net/batman-adv/bat_v_ogm.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
So, it works perfectly fine for me .
I understand that it is confusing that that Simon send a PR with 5 patches
mentioned. It is actually only 1 patch - 4 were already submitted in the last
PR. But still, the PR seems to apply cleanly for me.
Any hints how to reproduce your problem?
> While at it, could you please include the target tree in the subj prefix?
It currently mentions net in the subject. But I think you mean to change it
from "[PATCH 0/5] pull request for net: batman-adv 2025-03-13" to
"[PATCH net 0/5] pull request: batman-adv 2025-03-13". Or which exact format
do you prefer?
Kind regards,
Sven
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/5] pull request for net: batman-adv 2025-03-13
2025-03-18 13:56 ` Sven Eckelmann
@ 2025-03-18 14:27 ` Paolo Abeni
0 siblings, 0 replies; 9+ messages in thread
From: Paolo Abeni @ 2025-03-18 14:27 UTC (permalink / raw)
To: Sven Eckelmann, Simon Wunderlich, davem; +Cc: kuba, netdev, b.a.t.m.a.n
On 3/18/25 2:56 PM, Sven Eckelmann wrote:
> On Tuesday, 18 March 2025 12:05:52 CET Paolo Abeni wrote:
>> The series does not apply cleanly to the net tree, could you please
>> rebase it?
>
> $ git log -1 --oneline
> 9a81fc3480bf (HEAD, net/main) ipv6: Set errno after ip_fib_metrics_init() in ip6_route_info_create().
>
> $ git pull --no-ff git://git.open-mesh.org/linux-merge.git tags/batadv-net-pullrequest-20250313
> From git://git.open-mesh.org/linux-merge
> * tag batadv-net-pullrequest-20250313 -> FETCH_HEAD
> Merge made by the 'ort' strategy.
> net/batman-adv/bat_iv_ogm.c | 3 +--
> net/batman-adv/bat_v_ogm.c | 3 +--
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> So, it works perfectly fine for me .
>
> I understand that it is confusing that that Simon send a PR with 5 patches
> mentioned. It is actually only 1 patch - 4 were already submitted in the last
> PR. But still, the PR seems to apply cleanly for me.
>
> Any hints how to reproduce your problem?
My scripts fail to apply the posted patches (when the patches are posted
on the ML, the script try to apply them and compare to the PR tag).
Direct pulling the tag is successful, but I find a single patch there:
548b0c5de761 ("batman-adv: Ignore own maximum aggregation size during R")
instead of the mentioned 5.
>> While at it, could you please include the target tree in the subj prefix?
>
> It currently mentions net in the subject. But I think you mean to change it
> from "[PATCH 0/5] pull request for net: batman-adv 2025-03-13" to
> "[PATCH net 0/5] pull request: batman-adv 2025-03-13". Or which exact format
> do you prefer?
The preferred format is the latter: "[PATCH net ...]". That additionally
helps the CI to do the right things.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-03-18 14:28 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-13 16:17 [PATCH 0/5] pull request for net: batman-adv 2025-03-13 Simon Wunderlich
2025-03-13 16:17 ` [PATCH 1/5] batman-adv: fix panic during interface removal Simon Wunderlich
2025-03-13 16:17 ` [PATCH 2/5] batman-adv: Ignore neighbor throughput metrics in error case Simon Wunderlich
2025-03-13 16:17 ` [PATCH 3/5] batman-adv: Drop unmanaged ELP metric worker Simon Wunderlich
2025-03-13 16:17 ` [PATCH 4/5] batman-adv: Fix incorrect offset in batadv_tt_tvlv_ogm_handler_v1() Simon Wunderlich
2025-03-13 16:17 ` [PATCH 5/5] batman-adv: Ignore own maximum aggregation size during RX Simon Wunderlich
2025-03-18 11:05 ` [PATCH 0/5] pull request for net: batman-adv 2025-03-13 Paolo Abeni
2025-03-18 13:56 ` Sven Eckelmann
2025-03-18 14:27 ` Paolo Abeni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).