DHCP connectivity issues can currently occur if the following conditions
are met:
1) A DHCP packet from a client to a server
2) This packet has a multicast destination
3) This destination has a matching entry in the translation table
(FF:FF:FF:FF:FF:FF for IPv4, 33:33:00:01:00:02/33:33:00:01:00:03
for IPv6)
4) The orig-node determined by TT for the multicast destination
does not match the orig-node determined by best-gateway-selection
In this case the DHCP packet will be dropped.
The "gateway-out-of-range" check is supposed to only be applied to
unicasted DHCP packets to a specific DHCP server.
In that case dropping the the unicasted frame forces the client to
retry via a broadcasted one, but now directed to the new best
gateway.
A DHCP packet with broadcast/multicast destination is already ensured to
always be delivered to the best gateway. Dropping a multicasted
DHCP packet here will only prevent completing DHCP as there is no
other fallback.
So far, it seems the unicast check was implicitly performed by
expecting the batadv_transtable_search() to return NULL for multicast
destinations. However, a multicast address could have always ended up in
the translation table and in fact is now common.
To fix this potential loss of a DHCP client-to-server packet to a
multicast address this patch adds an explicit multicast destination
check to reliably bail out of the gateway-out-of-range check for such
destinations.
The issue and fix were tested in the following three node setup:
- Line topology, A-B-C
- A: gateway client, DHCP client
- B: gateway server, hop-penalty increased: 30->60, DHCP server
- C: gateway server, code modifications to announce FF:FF:FF:FF:FF:FF
Without this patch, A would never transmit its DHCP Discover packet
due to an always "out-of-range" condition. With this patch,
a full DHCP handshake between A and B was possible again.
Fixes: afae4e42aae6 ("batman-adv: refactoring gateway handling code")
Signed-off-by: Linus Lüssing <linus.luessing(a)c0d3.blue>
---
Changes since v1:
- now really fixing orig_dst_node initialization
Changes since RFC:
- fixed an uninitialized variable error for orig_dst_node by
initializing it to NULL
- Verified the issue and tested the fix in VMs, added the test
setup description to the commit message
---
net/batman-adv/gateway_client.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index c294f6fd..8b198ee7 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -746,7 +746,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
{
struct batadv_neigh_node *neigh_curr = NULL;
struct batadv_neigh_node *neigh_old = NULL;
- struct batadv_orig_node *orig_dst_node;
+ struct batadv_orig_node *orig_dst_node = NULL;
struct batadv_gw_node *gw_node = NULL;
struct batadv_gw_node *curr_gw = NULL;
struct batadv_neigh_ifinfo *curr_ifinfo, *old_ifinfo;
@@ -757,6 +757,9 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
vid = batadv_get_vid(skb, 0);
+ if (is_multicast_ether_addr(ethhdr->h_dest))
+ goto out;
+
orig_dst_node = batadv_transtable_search(bat_priv, ethhdr->h_source,
ethhdr->h_dest, vid);
if (!orig_dst_node)
--
2.11.0
For multicast frames AP isolation is only supposed to be checked on
the receiving nodes and never on the originating one.
Furthermore, the isolation or wifi flag bits should only be intepreted
as such for unicast and never multicast TT entries.
By injecting flags to the multicast TT entry claimed by a single
target node it was verified in tests that this multicast address
becomes unreachable, leading to packet loss.
Omitting the "src" parameter to the batadv_transtable_search() call
successfully skipped the AP isolation check and made the target
reachable again.
Fixes: 405cc1e5a81e ("batman-adv: Modified forwarding behaviour for multicast packets")
Signed-off-by: Linus Lüssing <linus.luessing(a)c0d3.blue>
---
This issue currently cannot appear in the wild. See explanation here:
https://www.open-mesh.org/issues/335#note-16
However if we were to legitimately start using these flags for
multicast's own purposes then we would start to see issues in AP
isolation setups. Therefore, and because the fix is tiny and "obvious",
I think it would still make sense to send it through stable@.
---
net/batman-adv/multicast.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
index de3a055f..a11d3d89 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -869,8 +869,8 @@ static struct batadv_orig_node *
batadv_mcast_forw_tt_node_get(struct batadv_priv *bat_priv,
struct ethhdr *ethhdr)
{
- return batadv_transtable_search(bat_priv, ethhdr->h_source,
- ethhdr->h_dest, BATADV_NO_FLAGS);
+ return batadv_transtable_search(bat_priv, NULL, ethhdr->h_dest,
+ BATADV_NO_FLAGS);
}
/**
--
2.11.0
Hi David,
here is another late feature/cleanup pull request of batman-adv to go into net-next.
Please pull or let me know of any problem!
Thank you,
Simon
The following changes since commit a163dc22d515d17844435c8217ff66193d35b3fa:
batman-adv: always assume 2-byte packet alignment (2018-02-27 13:02:54 +0100)
are available in the git repository at:
git://git.open-mesh.org/linux-merge.git tags/batadv-next-for-davem-20180319
for you to fetch changes up to 53dd9a68ba683986ec90497586f94b941bb748a0:
batman-adv: add multicast flags netlink support (2018-03-14 10:15:34 +0100)
----------------------------------------------------------------
This feature/cleanup patchset includes the following patches:
- avoid redundant multicast TT entries, by Linus Luessing
- add netlink support for distributed arp table cache and multicast flags,
by Linus Luessing (2 patches)
----------------------------------------------------------------
Linus Lüssing (3):
batman-adv: Avoid redundant multicast TT entries
batman-adv: add DAT cache netlink support
batman-adv: add multicast flags netlink support
include/uapi/linux/batman_adv.h | 82 +++++++++
net/batman-adv/distributed-arp-table.c | 152 +++++++++++++++++
net/batman-adv/distributed-arp-table.h | 8 +
net/batman-adv/multicast.c | 293 ++++++++++++++++++++++++++++++++-
net/batman-adv/multicast.h | 18 ++
net/batman-adv/netlink.c | 88 ++++++----
6 files changed, 604 insertions(+), 37 deletions(-)
Hi David,
here are some more bugfixes for net.
Please pull or let me know of any problem!
Thank you,
Simon
The following changes since commit f22e08932c2960f29b5e828e745c9f3fb7c1bb86:
batman-adv: Fix internal interface indices types (2018-02-25 20:19:34 +0100)
are available in the git repository at:
git://git.open-mesh.org/linux-merge.git tags/batadv-net-for-davem-20180319
for you to fetch changes up to fc04fdb2c8a894283259f5621d31d75610701091:
batman-adv: Fix skbuff rcsum on packet reroute (2018-03-18 13:20:32 +0100)
----------------------------------------------------------------
Here are some batman-adv bugfixes:
- fix possible IPv6 packet loss when multicast extension is used, by Linus Luessing
- fix SKB handling issues for TTVN and DAT, by Matthias Schiffer (two patches)
- fix include for eventpoll, by Sven Eckelmann
- fix skb checksum for ttvn reroutes, by Sven Eckelmann
----------------------------------------------------------------
Linus Lüssing (1):
batman-adv: Fix multicast packet loss with a single WANT_ALL_IPV4/6 flag
Matthias Schiffer (2):
batman-adv: update data pointers after skb_cow()
batman-adv: fix header size check in batadv_dbg_arp()
Sven Eckelmann (2):
batman-adv: Add missing include for EPOLL* constants
batman-adv: Fix skbuff rcsum on packet reroute
net/batman-adv/distributed-arp-table.c | 2 +-
net/batman-adv/icmp_socket.c | 1 +
net/batman-adv/log.c | 1 +
net/batman-adv/multicast.c | 4 ++--
net/batman-adv/routing.c | 25 +++++++++++++++----------
5 files changed, 20 insertions(+), 13 deletions(-)
DHCP connectivity issues can currently occur if the following conditions
are met:
1) A DHCP packet from a client to a server
2) This packet has a multicast destination
3) This destination has a matching entry in the translation table
(FF:FF:FF:FF:FF:FF for IPv4, 33:33:00:01:00:02/33:33:00:01:00:03
for IPv6)
4) The orig-node determined by TT for the multicast destination
does not match the orig-node determined by best-gateway-selection
In this case the DHCP packet will be dropped.
The "gateway-out-of-range" check is supposed to only be applied to
unicasted DHCP packets to a specific DHCP server.
In that case dropping the the unicasted frame forces the client to
retry via a broadcasted one, but now directed to the new best
gateway.
A DHCP packet with broadcast/multicast destination is already ensured to
always be delivered to the best gateway. Dropping a multicasted
DHCP packet here will only prevent completing DHCP as there is no
other fallback.
So far, it seems the unicast check was implicitly performed by
expecting the batadv_transtable_search() to return NULL for multicast
destinations. However, a multicast address could have always ended up in
the translation table and in fact is now common.
To fix this potential loss of a DHCP client-to-server packet to a
multicast address this patch adds an explicit multicast destination
check to reliably bail out of the gateway-out-of-range check for such
destinations.
The issue and fix were tested in the following three node setup:
- Line topology, A-B-C
- A: gateway client, DHCP client
- B: gateway server, hop-penalty increased: 30->60, DHCP server
- C: gateway server, code modifications to announce FF:FF:FF:FF:FF:FF
Without this patch, A would never transmit its DHCP Discover packet
due to an always "out-of-range" condition. With this patch,
a full DHCP handshake between A and B was possible again.
Fixes: afae4e42aae6 ("batman-adv: refactoring gateway handling code")
Signed-off-by: Linus Lüssing <linus.luessing(a)c0d3.blue>
---
Changes since RFC:
- fixed an uninitialized variable error for orig_dst_node by
initializing it to NULL
- Verified the issue and tested the fix in VMs, added the test
setup description to the commit message
---
net/batman-adv/gateway_client.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index c294f6fd..e7a245bd 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -757,6 +757,9 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
vid = batadv_get_vid(skb, 0);
+ if (is_multicast_ether_addr(ethhdr->h_dest))
+ goto out;
+
orig_dst_node = batadv_transtable_search(bat_priv, ethhdr->h_source,
ethhdr->h_dest, vid);
if (!orig_dst_node)
--
2.11.0
DHCP connectivity issues can currently occur if the following conditions
are met:
1) A DHCP packet from a client to a server
2) This packet has a multicast destination
3) This destination has a matching entry in the translation table
(FF:FF:FF:FF:FF:FF for IPv4, 33:33:00:01:00:02/33:33:00:01:00:03
for IPv6)
4) The orig-node determined by TT for the multicast destination
does not match the orig-node determined by best-gateway-selection
In this case the DHCP packet will be dropped.
The "gateway-out-of-range" check is supposed to only be applied to
unicasted DHCP packets to a specific DHCP server.
In that case dropping the the unicasted frame forces the client to
retry via a broadcasted one, but now directed to the new best
gateway.
A DHCP packet with broadcast/multicast destination is already ensured to
always be delivered to the best gateway. Dropping a multicasted
DHCP packet here will only prevent completing DHCP as there is no
other fallback.
So far, it seems the unicast check was implicitly performed by
expecting the batadv_transtable_search() to return NULL for multicast
destinations. However, a multicast address could have always ended up in
the translation table and in fact is now common.
To fix this potential loss of a DHCP client-to-server packet to a
multicast address this patch adds an explicit multicast destination
check to reliably bail out of the gateway-out-of-range check for such
destinations.
Fixes: afae4e42aae6 ("batman-adv: refactoring gateway handling code")
Not-yet-signed-off-because-untested
---
net/batman-adv/gateway_client.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index c294f6fd..e7a245bd 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -757,6 +757,9 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
vid = batadv_get_vid(skb, 0);
+ if (is_multicast_ether_addr(ethhdr->h_dest))
+ goto out;
+
orig_dst_node = batadv_transtable_search(bat_priv, ethhdr->h_source,
ethhdr->h_dest, vid);
if (!orig_dst_node)
--
2.11.0
On Mon, Mar 19, 2018 at 01:17:37AM +0000, scan-admin(a)coverity.com wrote:
[...]
> ________________________________________________________________________________________________________
> *** CID 174547: Incorrect expression (COPY_PASTE_ERROR)
> /netlink.c: 255 in info_callback()
> 249 else
> 250 mcast_flags = -EOPNOTSUPP;
> 251
> 252 if (attrs[BATADV_ATTR_MCAST_FLAGS_PRIV])
> 253 mcast_flags_priv = nla_get_u32(attrs[BATADV_ATTR_MCAST_FLAGS_PRIV]);
> 254 else
> >>> CID 174547: Incorrect expression (COPY_PASTE_ERROR)
> >>> "mcast_flags" in "mcast_flags = -95L" looks like a copy-paste error.
> 255 mcast_flags = -EOPNOTSUPP;
[...]
Now this is officially the second time a code/patch reviewing
algorithm creeps me out :D.
"Looks like", yeah... and even worse, the AI is actually right...
I will send a fix :).
Cheers
The switch to enable/disable throughput meter debug messages in `batctl
log` is supported since batman-adv 2016.3 but was never adopted in batctl.
Signed-off-by: Sven Eckelmann <sven(a)narfation.org>
---
README.rst | 1 +
man/batctl.8 | 7 ++++---
sys.c | 7 ++++++-
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/README.rst b/README.rst
index 256e153..4c0f544 100644
--- a/README.rst
+++ b/README.rst
@@ -428,6 +428,7 @@ Example::
[ ] messages related to arp snooping and distributed arp table (dat)
[ ] messages related to network coding (nc)
[ ] messages related to multicast (mcast)
+ [ ] messages related to throughput meter (tp)
batctl nc_nodes
diff --git a/man/batctl.8 b/man/batctl.8
index 7cf66ca..d5fc530 100644
--- a/man/batctl.8
+++ b/man/batctl.8
@@ -107,9 +107,10 @@ level. Level 'none' disables all verbose logging. Level 'batman' enables message
Level 'routes' enables messages related to routes being added / changed / deleted. Level 'tt' enables messages related to
translation table operations. Level 'bla' enables messages related to the bridge loop avoidance. Level 'dat' enables
messages related to ARP snooping and the Distributed Arp Table. Level 'nc' enables messages related to network coding.
-Level 'mcast' enables messages related to multicast optimizations. Level 'all' enables all messages. The messages
-are sent to the batman-adv debug log. Use \fBbatctl log\fP to retrieve it. Make sure to have debugging output enabled
-when compiling the module otherwise the output as well as the loglevel options won't be available.
+Level 'mcast' enables messages related to multicast optimizations. Level 'tp' enables messages related to throughput meter.
+Level 'all' enables all messages. The messages are sent to the batman-adv debug log. Use \fBbatctl log\fP to retrieve it.
+Make sure to have debugging output enabled when compiling the module otherwise the output as well as the loglevel options
+won't be available.
.br
.IP "\fBlog\fP|\fBl\fP [\fB\-n\fP]\fP"
batctl will read the batman-adv debug log which has to be compiled into the kernel module. If "\-n" is given batctl will not
diff --git a/sys.c b/sys.c
index 068aa31..e0c2073 100644
--- a/sys.c
+++ b/sys.c
@@ -132,6 +132,7 @@ static void log_level_usage(void)
fprintf(stderr, " \t dat Messages related to arp snooping and distributed arp table\n");
fprintf(stderr, " \t nc Messages related to network coding\n");
fprintf(stderr, " \t mcast Messages related to multicast\n");
+ fprintf(stderr, " \t tp Messages related to throughput meter\n");
}
int handle_loglevel(char *mesh_iface, int argc, char **argv)
@@ -169,7 +170,7 @@ int handle_loglevel(char *mesh_iface, int argc, char **argv)
log_level = 0;
break;
} else if (strcmp(argv[i], "all") == 0) {
- log_level = 127;
+ log_level = 255;
break;
} else if (strcmp(argv[i], "batman") == 0)
log_level |= BIT(0);
@@ -185,6 +186,8 @@ int handle_loglevel(char *mesh_iface, int argc, char **argv)
log_level |= BIT(5);
else if (strcmp(argv[i], "mcast") == 0)
log_level |= BIT(6);
+ else if (strcmp(argv[i], "tp") == 0)
+ log_level |= BIT(7);
else {
log_level_usage();
goto out;
@@ -220,6 +223,8 @@ int handle_loglevel(char *mesh_iface, int argc, char **argv)
"messages related to network coding", "nc");
printf("[%c] %s (%s)\n", (log_level & BIT(6)) ? 'x' : ' ',
"messages related to multicast", "mcast");
+ printf("[%c] %s (%s)\n", (log_level & BIT(7)) ? 'x' : ' ',
+ "messages related to throughput meter", "tp");
out:
free(path_buff);
--
2.16.2
The BIT(6) has to be set when the "all" loglevel is used to really get all
the log output which is known to batctl.
Fixes: 55042b3a3bbf ("batctl: adding multicast debug level")
Signed-off-by: Sven Eckelmann <sven(a)narfation.org>
---
sys.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sys.c b/sys.c
index bf77b7d..068aa31 100644
--- a/sys.c
+++ b/sys.c
@@ -169,7 +169,7 @@ int handle_loglevel(char *mesh_iface, int argc, char **argv)
log_level = 0;
break;
} else if (strcmp(argv[i], "all") == 0) {
- log_level = 63;
+ log_level = 127;
break;
} else if (strcmp(argv[i], "batman") == 0)
log_level |= BIT(0);
--
2.16.2