Hi,
After some time of playing with the B.A.T.M.A.N protocol and
net-interface on OpenWRT and Debian I was thinking to use it with
the servers I use everyday (and maybe on routers/appliances I have
nanoBSD on).
So I started an effort...
(As a background) I already ported some applications to FreeBSD [and I'm
maintaining them] and
also I did work already on the Linux emulation layer of FreeBSD (FreeBSD
has a Linux syscall-emulation and Linux-KPI layers).
So my approach (as naturally I didn't expect the build of batman-adv.ko
to be successful as is),
was based on the approach that we [at FreeBSD] did to port Linux's
drm... <https://github.com/FreeBSDDesktop/kms-drm>
I ended up in adding some header-files to FreeBSD Linux-KPI (like
average.h, percpu.h, ...).
Now I'm at a state that Netlink blocks me and I'm to determine next step :-)
[Which I don't assume it being trivial with my current approach]
So I'd like to ask:
1- Is it better approach to "rewrite" batman-adv.ko [at least
Netlink-ish (let's call "Linuxism") parts] than what I'm doing now?
2- Any other efforts are being done out there?
3- is batmand deprecated [So I should mainly focus on batman-adv.ko]?
4- any other comments do you have? :D
P.S. sorry if I'm not really good at starting conversation from scratch
and out-of-nowhere :D
but I hope by continuing the collaboration we can have better (more
enriched) FreeBSD and better (as in more portable) B.A.T.M.A.N :-)
--
Best regards, MMokhi.
From: Andreas Pape <apape(a)phoenixcontact.com>
Like in the case of the patch for batadv_bla_tx to handle a race
condition when claiming a mac address for bla, a similar situation
can occur when claiming is triggered via batadv_bla_rx. This patch
solves this with a similar approach as for batadv_bla_tx.
Signed-off-by: Andreas Pape <apape(a)phoenixcontact.com>
---
net/batman-adv/bridge_loop_avoidance.c | 31 ++++++++++++++++++++-----------
net/batman-adv/translation-table.c | 26 ++++++++++++++++++++++++++
net/batman-adv/translation-table.h | 3 +++
3 files changed, 49 insertions(+), 11 deletions(-)
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index d07e89e..cab8980 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -1847,19 +1847,28 @@ bool batadv_bla_rx(struct batadv_priv *bat_priv, struct sk_buff *skb,
if (!claim) {
/* possible optimization: race for a claim */
- /* No claim exists yet, claim it for us!
+ /* Make sure this packet is not looping back
+ * from our own backbone.
*/
- batadv_dbg(BATADV_DBG_BLA, bat_priv,
- "bla_rx(): Unclaimed MAC %pM found. Claim it. Local: %s\n",
- ethhdr->h_source,
- batadv_is_my_client(bat_priv,
- ethhdr->h_source, vid) ?
- "yes" : "no");
- batadv_handle_claim(bat_priv, primary_if,
- primary_if->net_dev->dev_addr,
- ethhdr->h_source, vid);
- goto allow;
+ if (batadv_tt_local_has_timed_out(bat_priv, ethhdr->h_source,
+ vid, 100)) {
+ /* No claim exists yet, claim it for us!
+ */
+ batadv_dbg(BATADV_DBG_BLA, bat_priv,
+ "bla_rx(): Unclaimed MAC %pM found. Claim it. Local: %s\n",
+ ethhdr->h_source,
+ batadv_is_my_client(bat_priv,
+ ethhdr->h_source, vid) ?
+ "yes" : "no");
+
+ batadv_handle_claim(bat_priv, primary_if,
+ primary_if->net_dev->dev_addr,
+ ethhdr->h_source, vid);
+ goto allow;
+ } else {
+ goto handled;
+ }
}
/* if it is our own claim ... */
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index e75b493..b908195 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -4380,3 +4380,29 @@ void batadv_tt_cache_destroy(void)
kmem_cache_destroy(batadv_tt_req_cache);
kmem_cache_destroy(batadv_tt_roam_cache);
}
+
+bool batadv_tt_local_has_timed_out(struct batadv_priv *bat_priv,
+ const u8 *addr, unsigned short vid,
+ unsigned int timeout)
+{
+ struct batadv_tt_local_entry *tt_local_entry;
+ bool ret = true;
+
+ tt_local_entry = batadv_tt_local_hash_find(bat_priv, addr, vid);
+ if (!tt_local_entry)
+ goto out;
+ /* Check if the client has been logically deleted (but is kept for
+ * consistency purpose)
+ */
+ if ((tt_local_entry->common.flags & BATADV_TT_CLIENT_PENDING) ||
+ (tt_local_entry->common.flags & BATADV_TT_CLIENT_ROAM))
+ goto out;
+ /* Check that the tt_local_entry has a certain age */
+ if (!batadv_has_timed_out(tt_local_entry->last_seen, timeout))
+ ret = false;
+
+out:
+ if (tt_local_entry)
+ batadv_tt_local_entry_put(tt_local_entry);
+ return ret;
+}
diff --git a/net/batman-adv/translation-table.h b/net/batman-adv/translation-table.h
index 411d586..b05d0d8 100644
--- a/net/batman-adv/translation-table.h
+++ b/net/batman-adv/translation-table.h
@@ -65,5 +65,8 @@ bool batadv_tt_global_is_isolated(struct batadv_priv *bat_priv,
int batadv_tt_cache_init(void);
void batadv_tt_cache_destroy(void);
+bool batadv_tt_local_has_timed_out(struct batadv_priv *bat_priv,
+ const u8 *addr, unsigned short vid,
+ unsigned int timeout);
#endif /* _NET_BATMAN_ADV_TRANSLATION_TABLE_H_ */
--
1.7.0.4
Whenever a new VLAN is created on top of batman virtual interfaces
the batman-adv kernel module creates internal structures to track
the status of said VLAN. Amongst other things, the MAC address of
the VLAN interface itself has to be stored.
Without this change a VLAN and its infrastructure could be created
while the interface MAC address is not stored without triggering
any error, thus creating issues in other parts of the code.
Prevent the VLAN from being created if the MAC address can not
be stored.
Fixes: 952cebb57518 ("batman-adv: add per VLAN interface attribute framework")
Signed-off-by: Marek Lindner <mareklindner(a)neomailbox.ch>
---
net/batman-adv/hard-interface.c | 2 +-
net/batman-adv/soft-interface.c | 105 ++++++++++++++++++++++++--------
net/batman-adv/soft-interface.h | 3 +-
3 files changed, 83 insertions(+), 27 deletions(-)
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index c405d15b..0b22cc4d 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -1000,7 +1000,7 @@ static int batadv_hard_if_event(struct notifier_block *this,
if (batadv_softif_is_valid(net_dev) && event == NETDEV_REGISTER) {
batadv_sysfs_add_meshif(net_dev);
bat_priv = netdev_priv(net_dev);
- batadv_softif_create_vlan(bat_priv, BATADV_NO_FLAGS);
+ batadv_softif_create_vlan_late(bat_priv, BATADV_NO_FLAGS);
return NOTIFY_DONE;
}
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index edeffcb9..728d9d40 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -563,16 +563,36 @@ struct batadv_softif_vlan *batadv_softif_vlan_get(struct batadv_priv *bat_priv,
}
/**
- * batadv_softif_create_vlan() - allocate the needed resources for a new vlan
+ * batadv_softif_destroy_vlan() - remove and destroy a softif_vlan object
+ * @bat_priv: the bat priv with all the soft interface information
+ * @vlan: the object to remove
+ */
+static void batadv_softif_destroy_vlan(struct batadv_priv *bat_priv,
+ struct batadv_softif_vlan *vlan)
+{
+ /* explicitly remove the associated TT local entry because it is marked
+ * with the NOPURGE flag
+ */
+ batadv_tt_local_remove(bat_priv, bat_priv->soft_iface->dev_addr,
+ vlan->vid, "vlan interface destroyed", false);
+
+ batadv_sysfs_del_vlan(bat_priv, vlan);
+ batadv_softif_vlan_put(vlan);
+}
+
+/**
+ * batadv_softif_create_vlan_early() - allocate the needed resources for a new
+ * vlan, defer sysfs creation till later
* @bat_priv: the bat priv with all the soft interface information
* @vid: the VLAN identifier
*
* Return: 0 on success, a negative error otherwise.
*/
-int batadv_softif_create_vlan(struct batadv_priv *bat_priv, unsigned short vid)
+static int batadv_softif_create_vlan_early(struct batadv_priv *bat_priv,
+ unsigned short vid)
{
struct batadv_softif_vlan *vlan;
- int err;
+ bool client_added;
vlan = batadv_softif_vlan_get(bat_priv, vid);
if (vlan) {
@@ -590,12 +610,6 @@ int batadv_softif_create_vlan(struct batadv_priv *bat_priv, unsigned short vid)
atomic_set(&vlan->ap_isolation, 0);
- err = batadv_sysfs_add_vlan(bat_priv->soft_iface, vlan);
- if (err) {
- kfree(vlan);
- return err;
- }
-
spin_lock_bh(&bat_priv->softif_vlan_list_lock);
kref_get(&vlan->refcount);
hlist_add_head_rcu(&vlan->list, &bat_priv->softif_vlan_list);
@@ -604,32 +618,63 @@ int batadv_softif_create_vlan(struct batadv_priv *bat_priv, unsigned short vid)
/* add a new TT local entry. This one will be marked with the NOPURGE
* flag
*/
- batadv_tt_local_add(bat_priv->soft_iface,
- bat_priv->soft_iface->dev_addr, vid,
- BATADV_NULL_IFINDEX, BATADV_NO_MARK);
+ client_added = batadv_tt_local_add(bat_priv->soft_iface,
+ bat_priv->soft_iface->dev_addr, vid,
+ BATADV_NULL_IFINDEX, BATADV_NO_MARK);
/* don't return reference to new softif_vlan */
batadv_softif_vlan_put(vlan);
+ if (!client_added) {
+ batadv_softif_destroy_vlan(bat_priv, vlan);
+ return -ENOENT;
+ }
+
return 0;
}
/**
- * batadv_softif_destroy_vlan() - remove and destroy a softif_vlan object
+ * batadv_softif_create_vlan_late() - complete softif vlan creation with the
+ * sysfs entries
* @bat_priv: the bat priv with all the soft interface information
- * @vlan: the object to remove
+ * @vid: the VLAN identifier
+ *
+ * Return: 0 on success, a negative error otherwise.
*/
-static void batadv_softif_destroy_vlan(struct batadv_priv *bat_priv,
- struct batadv_softif_vlan *vlan)
+int batadv_softif_create_vlan_late(struct batadv_priv *bat_priv,
+ unsigned short vid)
{
- /* explicitly remove the associated TT local entry because it is marked
- * with the NOPURGE flag
- */
- batadv_tt_local_remove(bat_priv, bat_priv->soft_iface->dev_addr,
- vlan->vid, "vlan interface destroyed", false);
+ struct batadv_softif_vlan *vlan;
+ int ret;
+
+ vlan = batadv_softif_vlan_get(bat_priv, vid);
+ if (!vlan)
+ return -ENOENT;
+
+ ret = batadv_sysfs_add_vlan(bat_priv->soft_iface, vlan);
- batadv_sysfs_del_vlan(bat_priv, vlan);
batadv_softif_vlan_put(vlan);
+ return ret;
+}
+
+/**
+ * batadv_softif_create_vlan() - allocate the needed resources for a new vlan
+ * @bat_priv: the bat priv with all the soft interface information
+ * @vid: the VLAN identifier
+ *
+ * Return: 0 on success, a negative error otherwise.
+ */
+static int batadv_softif_create_vlan(struct batadv_priv *bat_priv,
+ unsigned short vid)
+{
+ int err;
+
+ err = batadv_softif_create_vlan_early(bat_priv, vid);
+ if (err)
+ return err;
+
+ err = batadv_softif_create_vlan_late(bat_priv, vid);
+ return err;
}
/**
@@ -648,6 +693,7 @@ static int batadv_interface_add_vid(struct net_device *dev, __be16 proto,
{
struct batadv_priv *bat_priv = netdev_priv(dev);
struct batadv_softif_vlan *vlan;
+ bool client_added;
int ret;
/* only 802.1Q vlans are supported.
@@ -683,9 +729,14 @@ static int batadv_interface_add_vid(struct net_device *dev, __be16 proto,
* flag. This must be added again, even if the vlan object already
* exists, because the entry was deleted by kill_vid()
*/
- batadv_tt_local_add(bat_priv->soft_iface,
- bat_priv->soft_iface->dev_addr, vid,
- BATADV_NULL_IFINDEX, BATADV_NO_MARK);
+ client_added = batadv_tt_local_add(bat_priv->soft_iface,
+ bat_priv->soft_iface->dev_addr, vid,
+ BATADV_NULL_IFINDEX, BATADV_NO_MARK);
+
+ if (!client_added) {
+ batadv_softif_destroy_vlan(bat_priv, vlan);
+ return -ENOENT;
+ }
return 0;
}
@@ -850,6 +901,10 @@ static int batadv_softif_init_late(struct net_device *dev)
if (ret < 0)
goto unreg_debugfs;
+ ret = batadv_softif_create_vlan_early(bat_priv, BATADV_NO_FLAGS);
+ if (ret < 0)
+ goto unreg_debugfs;
+
return 0;
unreg_debugfs:
diff --git a/net/batman-adv/soft-interface.h b/net/batman-adv/soft-interface.h
index daf87f07..b8a9a3c8 100644
--- a/net/batman-adv/soft-interface.h
+++ b/net/batman-adv/soft-interface.h
@@ -36,7 +36,8 @@ struct net_device *batadv_softif_create(struct net *net, const char *name);
void batadv_softif_destroy_sysfs(struct net_device *soft_iface);
bool batadv_softif_is_valid(const struct net_device *net_dev);
extern struct rtnl_link_ops batadv_link_ops;
-int batadv_softif_create_vlan(struct batadv_priv *bat_priv, unsigned short vid);
+int batadv_softif_create_vlan_late(struct batadv_priv *bat_priv,
+ unsigned short vid);
void batadv_softif_vlan_put(struct batadv_softif_vlan *softif_vlan);
struct batadv_softif_vlan *batadv_softif_vlan_get(struct batadv_priv *bat_priv,
unsigned short vid);
--
2.17.0
Hi,
these two changes should help batman-adv during the ongoing process of
replacing the debugfs code. We already have:
* replaced most debugfs "tables" with netlink dumpit commands
* replaced the socket file code in batctl with direct captures/injects for
traceroute/ping
* made the debugfs code optional
These first two things were required to get it working in network namespaces.
The last point allows in theory to disable the debugfs code but distributions
will still continue to build the debugfs support for a while. The first patch
in this series will therefore log access requests to this deprecated
functionality in the kernel message buffer. This will hopefully cause more
developers to switch their tools to netlink in the near future:
$ cat /sys/kernel/debug/batman_adv/bat0/neighbors > /dev/null
batman_adv: [Deprecated]: cat (pid 12053) Use of debugfs file "neighbors".
Use genl command BATADV_CMD_GET_NEIGHBORS instead
But we also have some other functionality in batman-adv's debugfs which could
be integrated better in the rest of the kernel infrastructure: the logging.
The log ringbuffer from batman-adv is completely isolated from the rest of
the kernel even when the kernel itself already provides its own tracing
infrastructure. The tooling for the kernel infrastructure is also a lot
better and should replace the batman_adv/*/log file in batman-adv/batctl
completely - but let us postpone the removal for now.
Some examples for the usage of batadv_dbg via tracepoints can be found in the
commit message of this change. A lot more complex things are possible with
TRACE_EVENT and co but let us start now with the conversion of the already
existing messages. There is also a good introduction on LWN [1,2,3]
Kind regards,
Sven
[1] https://lwn.net/Articles/379903/
[2] https://lwn.net/Articles/381064/
[3] https://lwn.net/Articles/383362/
Sven Eckelmann (2):
batman-adv: Mark debugfs functionality as deprecated
batman-adv: Provide debug messages as trace events
Makefile | 3 ++
README.external.rst | 1 +
compat-include/linux/fs.h | 37 +++++++++++++++++
gen-compat-autoconf.sh | 1 +
net/batman-adv/Kconfig | 11 +++++
net/batman-adv/Makefile | 3 ++
net/batman-adv/debugfs.c | 37 +++++++++++++++++
net/batman-adv/debugfs.h | 6 +++
net/batman-adv/icmp_socket.c | 3 ++
net/batman-adv/log.c | 19 +++++++--
net/batman-adv/trace.c | 22 ++++++++++
net/batman-adv/trace.h | 78 ++++++++++++++++++++++++++++++++++++
12 files changed, 217 insertions(+), 4 deletions(-)
create mode 100644 compat-include/linux/fs.h
create mode 100644 net/batman-adv/trace.c
create mode 100644 net/batman-adv/trace.h
--
2.18.0
Hi,
over two years ago, I've added some tickets [1,2,3,4,5,6,7] about
missing/not properly locked list checks for functions which create new list
entries. Only Linus created a patch [8] and the other tickets were ignored.
I have therefore now created the minimal version of the patches.
Kind regards,
Sven
[1] https://www.open-mesh.org/issues/235
[2] https://www.open-mesh.org/issues/236
[3] https://www.open-mesh.org/issues/237
[4] https://www.open-mesh.org/issues/238
[5] https://www.open-mesh.org/issues/239
[6] https://www.open-mesh.org/issues/240
[7] https://www.open-mesh.org/issues/241
[8] https://patchwork.open-mesh.org/patch/4919/
Sven Eckelmann (5):
batman-adv: Prevent duplicated gateway_node entry
batman-adv: Prevent duplicated nc_node entry
batman-adv: Prevent duplicated softif_vlan entry
batman-adv: Prevent duplicated global TT entry
batman-adv: Prevent duplicated tvlv handler
net/batman-adv/gateway_client.c | 8 ++++++--
net/batman-adv/network-coding.c | 27 +++++++++++++++------------
net/batman-adv/soft-interface.c | 21 ++++++++++++---------
net/batman-adv/translation-table.c | 6 ++++--
net/batman-adv/tvlv.c | 8 ++++++--
5 files changed, 43 insertions(+), 27 deletions(-)
--
2.18.0
The per hardif sysfs file "batman_adv/throughput_override" prints the
resulting change as info text when the users writes to this file. It uses
the helper function batadv_info to add it at the same time to the kernel
ring buffer and to the batman-adv debug log (when CONFIG_BATMAN_ADV_DEBUG
is enabled).
The function batadv_info requires as first parameter the batman-adv softif
net_device. This parameter is then used to find the private buffer which
contains the debug log for this batman-adv interface. But
batadv_store_throughput_override used as first argument the slave
net_device. This slave device doesn't have the batadv_priv private data
which is access by batadv_info.
Writing to this file with CONFIG_BATMAN_ADV_DEBUG enabled can either lead
to a segfault or to memory corruption.
Fixes: c513176e4b7a ("batman-adv: add throughput override attribute to hard_ifaces")
Signed-off-by: Sven Eckelmann <sven(a)narfation.org>
---
net/batman-adv/sysfs.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index f2eef43b..3a76e897 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -1090,8 +1090,9 @@ static ssize_t batadv_store_throughput_override(struct kobject *kobj,
if (old_tp_override == tp_override)
goto out;
- batadv_info(net_dev, "%s: Changing from: %u.%u MBit to: %u.%u MBit\n",
- "throughput_override",
+ batadv_info(hard_iface->soft_iface,
+ "%s: %s: Changing from: %u.%u MBit to: %u.%u MBit\n",
+ "throughput_override", net_dev->name,
old_tp_override / 10, old_tp_override % 10,
tp_override / 10, tp_override % 10);
--
2.18.0