Hi,
we are currently starting to use batman-adv as mesh protocol on multicore
embedded devices. These usually don't have a lot of CPU power per core but
are reasonable fast when using multiple cores.
It was noticed that sending was working very well but receiving was
basically only using on CPU core per neighbor. The reason for that is
format of the (normal) incoming packet:
+--------------------+
| ip(v6)hdr |
+--------------------+
| inner ethhdr |
+--------------------+
| batadv unicast hdr |
+--------------------+
| outer ethhdr |
+--------------------+
The flow dissector will therefore stop after parsing the outer ethernet
header and will not parse the actual ipv(4|6)/... header of the packet. Our
assumption was now that it would help us to add minimal support to the flow
dissector to jump over the batman-adv unicast and inner ethernet header
(like in gre ETH_P_TEB). The patch was implemented in a slightly hacky
way [1] and the results looked quite promising.
Now we comes the actual "problematic" part. The packet format is currently
only available in net/batman-adv/packet.h. I've guessed that it should be
moved somewhere under include/ to make it accessible to the flow dissector
in a "standard way".
First we have to find the correct place for it. I am not aware of any
standard for that - so I've grepped for some packet headers which are used
by the dissector and found following files (most likely not a complete
list):
* linux/if_vlan.h
* net/gre.h
* net/tipc.h
* uapi/linux/if_arp.h
* uapi/linux/if_ether.h
* uapi/linux/if_pppox.h
* uapi/linux/ip.h
* uapi/linux/ipv6.h
* uapi/linux/mpls.h
* uapi/linux/tcp.h
So I would say that uapi/linux is the "winner" here. This would actually
also helpful for userspace tools which either inject packets or monitor
interfaces + dissect packets (for example batctl). Now to the actual
filename. batman_adv.h is already used in the moment for the netlink
definition and I would like to avoid that these two things are placed in
the same file.
A quick grep for "NL_NAME" showed me following names:
* linux/nl802154.h
* net/nl802154.h
* uapi/linux/batman_adv.h
* uapi/linux/devlink.h
* uapi/linux/dlm_netlink.h
* uapi/linux/fou.h
* uapi/linux/if_macsec.h
* uapi/linux/if_team.h
* uapi/linux/ila.h
* uapi/linux/ip_vs.h
* uapi/linux/irda.h
* uapi/linux/l2tp.h
* uapi/linux/nfc.h
* uapi/linux/nl80211.h
* uapi/linux/psample.h
* uapi/linux/seg6_genl.h
* uapi/linux/taskstats.h
* uapi/linux/tcp_metrics.h
* uapi/linux/tipc_config.h
* uapi/linux/tipc_netlink.h
There doesn't seem to be any common way to do it - so I have to guess what
the best names could be. I've decided (but please provide better
alternatives) to use batadv_genl.h (for generic netlink stuff) and batadv.h
(for the packet format).
I would really appreciate it when you would provide feedback to the
naming/placement of the files. Btw. the patches are currently based on the
top of batadv/net-next [2] and this repository contains changes which still
have to be submitted to net-next.
Changes in v2:
=============
* removed the batman-adv unicast packet header definition from flow_dissector.c
* moved the batman-adv packet.h/uapi headers around to provide the correct
definitions to flow_dissector.c
Kind regards,
Sven
[1] https://patchwork.open-mesh.org/patch/17162/
[2] https://git.open-mesh.org/linux-merge.git/shortlog/refs/heads/batadv/net-next
Sven Eckelmann (6):
batman-adv: Change nl references to genl
batman-adv: Rename batman-adv.h to batadv_genl.h
batman-adv: Let packet.h include its headers directly
batman-adv: Remove usage of BIT(x) in packet.h
batman-adv: Convert packet.h to uapi header
flow_dissector: Parse batman-adv unicast headers
MAINTAINERS | 3 +-
.../packet.h => include/uapi/linux/batadv.h | 32 ++++++++++++----------
include/uapi/linux/{batman_adv.h => batadv_genl.h} | 20 +++++++-------
net/batman-adv/bat_algo.c | 2 +-
net/batman-adv/bat_iv_ogm.c | 4 +--
net/batman-adv/bat_v.c | 4 +--
net/batman-adv/bat_v_elp.c | 2 +-
net/batman-adv/bat_v_ogm.c | 2 +-
net/batman-adv/bridge_loop_avoidance.c | 4 +--
net/batman-adv/distributed-arp-table.h | 2 +-
net/batman-adv/fragmentation.c | 2 +-
net/batman-adv/gateway_client.c | 4 +--
net/batman-adv/gateway_common.c | 2 +-
net/batman-adv/hard-interface.c | 2 +-
net/batman-adv/icmp_socket.c | 2 +-
net/batman-adv/main.c | 6 ++--
net/batman-adv/main.h | 4 +--
net/batman-adv/multicast.c | 2 +-
net/batman-adv/netlink.c | 14 ++++++----
net/batman-adv/network-coding.c | 2 +-
net/batman-adv/originator.c | 2 +-
net/batman-adv/routing.c | 2 +-
net/batman-adv/send.h | 3 +-
net/batman-adv/soft-interface.c | 2 +-
net/batman-adv/sysfs.c | 2 +-
net/batman-adv/tp_meter.c | 4 +--
net/batman-adv/translation-table.c | 4 +--
net/batman-adv/tvlv.c | 2 +-
net/batman-adv/types.h | 5 ++--
net/core/flow_dissector.c | 30 ++++++++++++++++++++
30 files changed, 101 insertions(+), 70 deletions(-)
rename net/batman-adv/packet.h => include/uapi/linux/batadv.h (96%)
rename include/uapi/linux/{batman_adv.h => batadv_genl.h} (95%)
--
2.11.0
The batman-adv netlink functionality is actually not pure netlink but is
using NETLINK_GENERIC and related API. The code should therefore also use
this "genl" instead of "nl" to make this more clear.
Signed-off-by: Sven Eckelmann <[email protected]>
---
include/uapi/linux/batman_adv.h | 14 +++++++-------
net/batman-adv/main.c | 2 +-
net/batman-adv/netlink.c | 10 ++++++----
3 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/include/uapi/linux/batman_adv.h b/include/uapi/linux/batman_adv.h
index ae00c99cbed0..a0fed6c01a46 100644
--- a/include/uapi/linux/batman_adv.h
+++ b/include/uapi/linux/batman_adv.h
@@ -25,9 +25,9 @@
#ifndef _UAPI_LINUX_BATMAN_ADV_H_
#define _UAPI_LINUX_BATMAN_ADV_H_
-#define BATADV_NL_NAME "batadv"
+#define BATADV_GENL_NAME "batadv"
-#define BATADV_NL_MCAST_GROUP_TPMETER "tpmeter"
+#define BATADV_GENL_MCAST_GROUP_TPMETER "tpmeter"
/**
* enum batadv_tt_client_flags - TT client specific flags
@@ -92,9 +92,9 @@ enum batadv_tt_client_flags {
};
/**
- * enum batadv_nl_attrs - batman-adv netlink attributes
+ * enum batadv_genl_attrs - batman-adv netlink attributes
*/
-enum batadv_nl_attrs {
+enum batadv_genl_attrs {
/**
* @BATADV_ATTR_UNSPEC: unspecified attribute to catch errors
*/
@@ -280,7 +280,7 @@ enum batadv_nl_attrs {
__BATADV_ATTR_AFTER_LAST,
/**
- * @NUM_BATADV_ATTR: total number of batadv_nl_attrs available
+ * @NUM_BATADV_ATTR: total number of batadv_genl_attrs available
*/
NUM_BATADV_ATTR = __BATADV_ATTR_AFTER_LAST,
@@ -291,9 +291,9 @@ enum batadv_nl_attrs {
};
/**
- * enum batadv_nl_commands - supported batman-adv netlink commands
+ * enum batadv_genl_commands - supported batman-adv netlink commands
*/
-enum batadv_nl_commands {
+enum batadv_genl_commands {
/**
* @BATADV_CMD_UNSPEC: unspecified command to catch errors
*/
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index fc6dcea30238..a56ae758f799 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -687,4 +687,4 @@ MODULE_DESCRIPTION(BATADV_DRIVER_DESC);
MODULE_SUPPORTED_DEVICE(BATADV_DRIVER_DEVICE);
MODULE_VERSION(BATADV_SOURCE_VERSION);
MODULE_ALIAS_RTNL_LINK("batadv");
-MODULE_ALIAS_GENL_FAMILY(BATADV_NL_NAME);
+MODULE_ALIAS_GENL_FAMILY(BATADV_GENL_NAME);
diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
index a68443d3f119..a9679bdca432 100644
--- a/net/batman-adv/netlink.c
+++ b/net/batman-adv/netlink.c
@@ -58,11 +58,13 @@ struct genl_family batadv_netlink_family;
/* multicast groups */
enum batadv_netlink_multicast_groups {
- BATADV_NL_MCGRP_TPMETER,
+ BATADV_GENL_MCGRP_TPMETER,
};
static const struct genl_multicast_group batadv_netlink_mcgrps[] = {
- [BATADV_NL_MCGRP_TPMETER] = { .name = BATADV_NL_MCAST_GROUP_TPMETER },
+ [BATADV_GENL_MCGRP_TPMETER] = {
+ .name = BATADV_GENL_MCAST_GROUP_TPMETER
+ },
};
static const struct nla_policy batadv_netlink_policy[NUM_BATADV_ATTR] = {
@@ -298,7 +300,7 @@ int batadv_netlink_tpmeter_notify(struct batadv_priv *bat_priv, const u8 *dst,
genlmsg_multicast_netns(&batadv_netlink_family,
dev_net(bat_priv->soft_iface), msg, 0,
- BATADV_NL_MCGRP_TPMETER, GFP_KERNEL);
+ BATADV_GENL_MCGRP_TPMETER, GFP_KERNEL);
return 0;
@@ -611,7 +613,7 @@ static const struct genl_ops batadv_netlink_ops[] = {
struct genl_family batadv_netlink_family __ro_after_init = {
.hdrsize = 0,
- .name = BATADV_NL_NAME,
+ .name = BATADV_GENL_NAME,
.version = 1,
.maxattr = BATADV_ATTR_MAX,
.netnsok = true,
--
2.11.0
The headers used by packet.h should also be included by it directly. main.h
is currently dealing with it in batman-adv, but this will no longer work
when this header is moved to include/uapi/linux/.
Signed-off-by: Sven Eckelmann <[email protected]>
---
net/batman-adv/main.h | 2 --
net/batman-adv/packet.h | 2 ++
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h
index 69c2ab666961..231bbc47910e 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -219,10 +219,8 @@ enum batadv_uev_type {
/* Kernel headers */
-#include <linux/bitops.h> /* for packet.h */
#include <linux/compiler.h>
#include <linux/etherdevice.h>
-#include <linux/if_ether.h> /* for packet.h */
#include <linux/if_vlan.h>
#include <linux/jiffies.h>
#include <linux/percpu.h>
diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h
index ebe0605d1505..0d011012c71a 100644
--- a/net/batman-adv/packet.h
+++ b/net/batman-adv/packet.h
@@ -22,6 +22,8 @@
#define _NET_BATMAN_ADV_PACKET_H_
#include <asm/byteorder.h>
+#include <linux/bitops.h>
+#include <linux/if_ether.h>
#include <linux/types.h>
/**
--
2.11.0
The batman-adv unicast packets contain a full layer 2 frame in encapsulated
form. The flow dissector must therefore be able to parse the batman-adv
unicast header to reach the layer 2+3 information.
+--------------------+
| ip(v6)hdr |
+--------------------+
| inner ethhdr |
+--------------------+
| batadv unicast hdr |
+--------------------+
| outer ethhdr |
+--------------------+
The obtained information from the upper layer can then be used by RPS to
schedule the processing on separate cores. This allows better distribution
of multiple flows from the same neighbor to different cores.
Signed-off-by: Sven Eckelmann <[email protected]>
---
net/core/flow_dissector.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 15ce30063765..784cc07fc58e 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -24,6 +24,7 @@
#include <linux/tcp.h>
#include <net/flow_dissector.h>
#include <scsi/fc/fc_fcoe.h>
+#include <uapi/linux/batadv.h>
static void dissector_set_key(struct flow_dissector *flow_dissector,
enum flow_dissector_key_id key_id)
@@ -696,6 +697,35 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
break;
}
+ case htons(ETH_P_BATMAN): {
+ struct {
+ struct batadv_unicast_packet batadv_unicast;
+ struct ethhdr eth;
+ } *hdr, _hdr;
+
+ hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen,
+ &_hdr);
+ if (!hdr) {
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
+
+ if (hdr->batadv_unicast.version != BATADV_COMPAT_VERSION) {
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
+
+ if (hdr->batadv_unicast.packet_type != BATADV_UNICAST) {
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
+
+ proto = hdr->eth.h_proto;
+ nhoff += sizeof(*hdr);
+
+ fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
+ break;
+ }
case htons(ETH_P_8021AD):
case htons(ETH_P_8021Q): {
const struct vlan_hdr *vlan;
--
2.11.0
The header file is used by different userspace programs to inject packets
or to decode sniffed packets. It should therefore be available to them as
userspace header.
Also other components in the kernel (like the flow dissector) require
access to the packet definitions to be able to decode ETH_P_BATMAN ethernet
packets.
Signed-off-by: Sven Eckelmann <[email protected]>
---
MAINTAINERS | 1 +
net/batman-adv/packet.h => include/uapi/linux/batadv.h | 9 +++++----
net/batman-adv/bat_iv_ogm.c | 2 +-
net/batman-adv/bat_v.c | 2 +-
net/batman-adv/bat_v_elp.c | 2 +-
net/batman-adv/bat_v_ogm.c | 2 +-
net/batman-adv/bridge_loop_avoidance.c | 2 +-
net/batman-adv/distributed-arp-table.h | 2 +-
net/batman-adv/fragmentation.c | 2 +-
net/batman-adv/gateway_client.c | 2 +-
net/batman-adv/gateway_common.c | 2 +-
net/batman-adv/hard-interface.c | 2 +-
net/batman-adv/icmp_socket.c | 2 +-
net/batman-adv/main.c | 2 +-
net/batman-adv/main.h | 2 +-
net/batman-adv/multicast.c | 2 +-
net/batman-adv/netlink.c | 2 +-
net/batman-adv/network-coding.c | 2 +-
net/batman-adv/routing.c | 2 +-
net/batman-adv/send.h | 3 +--
net/batman-adv/soft-interface.c | 2 +-
net/batman-adv/sysfs.c | 2 +-
net/batman-adv/tp_meter.c | 2 +-
net/batman-adv/translation-table.c | 2 +-
net/batman-adv/tvlv.c | 2 +-
net/batman-adv/types.h | 3 +--
26 files changed, 30 insertions(+), 30 deletions(-)
rename net/batman-adv/packet.h => include/uapi/linux/batadv.h (98%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 68abe39ee87a..10c42a978c59 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2563,6 +2563,7 @@ S: Maintained
F: Documentation/ABI/testing/sysfs-class-net-batman-adv
F: Documentation/ABI/testing/sysfs-class-net-mesh
F: Documentation/networking/batman-adv.rst
+F: include/uapi/linux/batadv.h
F: include/uapi/linux/batadv_genl.h
F: net/batman-adv/
diff --git a/net/batman-adv/packet.h b/include/uapi/linux/batadv.h
similarity index 98%
rename from net/batman-adv/packet.h
rename to include/uapi/linux/batadv.h
index 311ac6fd76ff..46a22ab79b76 100644
--- a/net/batman-adv/packet.h
+++ b/include/uapi/linux/batadv.h
@@ -1,4 +1,4 @@
-/* SPDX-License-Identifier: GPL-2.0 */
+/* SPDX-License-Identifier: (GPL-2.0 WITH Linux-syscall-note) */
/* Copyright (C) 2007-2017 B.A.T.M.A.N. contributors:
*
* Marek Lindner, Simon Wunderlich
@@ -16,10 +16,11 @@
* along with this program; if not, see <http://www.gnu.org/licenses/>.
*
* License-Filename: LICENSES/preferred/GPL-2.0
+ * License-Filename: LICENSES/exceptions/Linux-syscall-note
*/
-#ifndef _NET_BATMAN_ADV_PACKET_H_
-#define _NET_BATMAN_ADV_PACKET_H_
+#ifndef _UAPI_LINUX_BATADV_H_
+#define _UAPI_LINUX_BATADV_H_
#include <asm/byteorder.h>
#include <linux/if_ether.h>
@@ -643,4 +644,4 @@ struct batadv_tvlv_mcast_data {
u8 reserved[3];
};
-#endif /* _NET_BATMAN_ADV_PACKET_H_ */
+#endif /* _UAPI_LINUX_BATADV_H_ */
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 7727a39c4265..201529d1f015 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -54,6 +54,7 @@
#include <linux/workqueue.h>
#include <net/genetlink.h>
#include <net/netlink.h>
+#include <uapi/linux/batadv.h>
#include <uapi/linux/batadv_genl.h>
#include "bat_algo.h"
@@ -65,7 +66,6 @@
#include "netlink.h"
#include "network-coding.h"
#include "originator.h"
-#include "packet.h"
#include "routing.h"
#include "send.h"
#include "translation-table.h"
diff --git a/net/batman-adv/bat_v.c b/net/batman-adv/bat_v.c
index b43ba53daafd..4b08d2b4d66f 100644
--- a/net/batman-adv/bat_v.c
+++ b/net/batman-adv/bat_v.c
@@ -39,6 +39,7 @@
#include <linux/workqueue.h>
#include <net/genetlink.h>
#include <net/netlink.h>
+#include <uapi/linux/batadv.h>
#include <uapi/linux/batadv_genl.h>
#include "bat_algo.h"
@@ -51,7 +52,6 @@
#include "log.h"
#include "netlink.h"
#include "originator.h"
-#include "packet.h"
struct sk_buff;
diff --git a/net/batman-adv/bat_v_elp.c b/net/batman-adv/bat_v_elp.c
index 0908e313d121..9fb1718248b8 100644
--- a/net/batman-adv/bat_v_elp.c
+++ b/net/batman-adv/bat_v_elp.c
@@ -44,13 +44,13 @@
#include <linux/types.h>
#include <linux/workqueue.h>
#include <net/cfg80211.h>
+#include <uapi/linux/batadv.h>
#include "bat_algo.h"
#include "bat_v_ogm.h"
#include "hard-interface.h"
#include "log.h"
#include "originator.h"
-#include "packet.h"
#include "routing.h"
#include "send.h"
diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c
index a79e89ca5667..0f4050193e30 100644
--- a/net/batman-adv/bat_v_ogm.c
+++ b/net/batman-adv/bat_v_ogm.c
@@ -41,13 +41,13 @@
#include <linux/string.h>
#include <linux/types.h>
#include <linux/workqueue.h>
+#include <uapi/linux/batadv.h>
#include "bat_algo.h"
#include "hard-interface.h"
#include "hash.h"
#include "log.h"
#include "originator.h"
-#include "packet.h"
#include "routing.h"
#include "send.h"
#include "translation-table.h"
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index 1a9594d68db2..54d5feeb8ee2 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -52,6 +52,7 @@
#include <net/genetlink.h>
#include <net/netlink.h>
#include <net/sock.h>
+#include <uapi/linux/batadv.h>
#include <uapi/linux/batadv_genl.h>
#include "hard-interface.h"
@@ -59,7 +60,6 @@
#include "log.h"
#include "netlink.h"
#include "originator.h"
-#include "packet.h"
#include "soft-interface.h"
#include "sysfs.h"
#include "translation-table.h"
diff --git a/net/batman-adv/distributed-arp-table.h b/net/batman-adv/distributed-arp-table.h
index 351de492430d..a9303faaaf54 100644
--- a/net/batman-adv/distributed-arp-table.h
+++ b/net/batman-adv/distributed-arp-table.h
@@ -26,9 +26,9 @@
#include <linux/compiler.h>
#include <linux/netdevice.h>
#include <linux/types.h>
+#include <uapi/linux/batadv.h>
#include "originator.h"
-#include "packet.h"
struct seq_file;
struct sk_buff;
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c
index 3890186b6f70..22b98cf39cfa 100644
--- a/net/batman-adv/fragmentation.c
+++ b/net/batman-adv/fragmentation.c
@@ -35,10 +35,10 @@
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/string.h>
+#include <uapi/linux/batadv.h>
#include "hard-interface.h"
#include "originator.h"
-#include "packet.h"
#include "routing.h"
#include "send.h"
#include "soft-interface.h"
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index 23c8032ce478..9a25b840fb33 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -45,6 +45,7 @@
#include <linux/stddef.h>
#include <linux/udp.h>
#include <net/sock.h>
+#include <uapi/linux/batadv.h>
#include <uapi/linux/batadv_genl.h>
#include "gateway_common.h"
@@ -52,7 +53,6 @@
#include "log.h"
#include "netlink.h"
#include "originator.h"
-#include "packet.h"
#include "routing.h"
#include "soft-interface.h"
#include "sysfs.h"
diff --git a/net/batman-adv/gateway_common.c b/net/batman-adv/gateway_common.c
index d918a176fa7b..a0e95a78b529 100644
--- a/net/batman-adv/gateway_common.c
+++ b/net/batman-adv/gateway_common.c
@@ -29,10 +29,10 @@
#include <linux/netdevice.h>
#include <linux/stddef.h>
#include <linux/string.h>
+#include <uapi/linux/batadv.h>
#include "gateway_client.h"
#include "log.h"
-#include "packet.h"
#include "tvlv.h"
/**
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index f8a1dbc75f42..7519545cd0de 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -40,6 +40,7 @@
#include <linux/spinlock.h>
#include <net/net_namespace.h>
#include <net/rtnetlink.h>
+#include <uapi/linux/batadv.h>
#include "bat_v.h"
#include "bridge_loop_avoidance.h"
@@ -48,7 +49,6 @@
#include "gateway_client.h"
#include "log.h"
#include "originator.h"
-#include "packet.h"
#include "send.h"
#include "soft-interface.h"
#include "sysfs.h"
diff --git a/net/batman-adv/icmp_socket.c b/net/batman-adv/icmp_socket.c
index 7bac7b4ee165..6dba4fbbbf70 100644
--- a/net/batman-adv/icmp_socket.c
+++ b/net/batman-adv/icmp_socket.c
@@ -46,11 +46,11 @@
#include <linux/string.h>
#include <linux/uaccess.h>
#include <linux/wait.h>
+#include <uapi/linux/batadv.h>
#include "hard-interface.h"
#include "log.h"
#include "originator.h"
-#include "packet.h"
#include "send.h"
static struct batadv_socket_client *batadv_socket_client_hash[256];
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index c034e5c504c4..d6ca1b92afc5 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -48,6 +48,7 @@
#include <linux/workqueue.h>
#include <net/dsfield.h>
#include <net/rtnetlink.h>
+#include <uapi/linux/batadv.h>
#include <uapi/linux/batadv_genl.h>
#include "bat_algo.h"
@@ -65,7 +66,6 @@
#include "netlink.h"
#include "network-coding.h"
#include "originator.h"
-#include "packet.h"
#include "routing.h"
#include "send.h"
#include "soft-interface.h"
diff --git a/net/batman-adv/main.h b/net/batman-adv/main.h
index 231bbc47910e..44f72cb41e93 100644
--- a/net/batman-adv/main.h
+++ b/net/batman-adv/main.h
@@ -225,8 +225,8 @@ enum batadv_uev_type {
#include <linux/jiffies.h>
#include <linux/percpu.h>
#include <linux/types.h>
+#include <uapi/linux/batadv.h>
-#include "packet.h"
#include "types.h"
struct net_device;
diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
index 2bda4540466d..f95af5101a8c 100644
--- a/net/batman-adv/multicast.c
+++ b/net/batman-adv/multicast.c
@@ -57,11 +57,11 @@
#include <net/if_inet6.h>
#include <net/ip.h>
#include <net/ipv6.h>
+#include <uapi/linux/batadv.h>
#include "hard-interface.h"
#include "hash.h"
#include "log.h"
-#include "packet.h"
#include "translation-table.h"
#include "tvlv.h"
diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
index 5a66eb6c45f1..551c0bd1fecf 100644
--- a/net/batman-adv/netlink.c
+++ b/net/batman-adv/netlink.c
@@ -42,6 +42,7 @@
#include <net/genetlink.h>
#include <net/netlink.h>
#include <net/sock.h>
+#include <uapi/linux/batadv.h>
#include <uapi/linux/batadv_genl.h>
#include "bat_algo.h"
@@ -49,7 +50,6 @@
#include "gateway_client.h"
#include "hard-interface.h"
#include "originator.h"
-#include "packet.h"
#include "soft-interface.h"
#include "tp_meter.h"
#include "translation-table.h"
diff --git a/net/batman-adv/network-coding.c b/net/batman-adv/network-coding.c
index aea92d09d7e8..bd72619106ba 100644
--- a/net/batman-adv/network-coding.c
+++ b/net/batman-adv/network-coding.c
@@ -51,12 +51,12 @@
#include <linux/stddef.h>
#include <linux/string.h>
#include <linux/workqueue.h>
+#include <uapi/linux/batadv.h>
#include "hard-interface.h"
#include "hash.h"
#include "log.h"
#include "originator.h"
-#include "packet.h"
#include "routing.h"
#include "send.h"
#include "tvlv.h"
diff --git a/net/batman-adv/routing.c b/net/batman-adv/routing.c
index 36001738917b..b462442b6d59 100644
--- a/net/batman-adv/routing.c
+++ b/net/batman-adv/routing.c
@@ -36,6 +36,7 @@
#include <linux/skbuff.h>
#include <linux/spinlock.h>
#include <linux/stddef.h>
+#include <uapi/linux/batadv.h>
#include "bitarray.h"
#include "bridge_loop_avoidance.h"
@@ -46,7 +47,6 @@
#include "log.h"
#include "network-coding.h"
#include "originator.h"
-#include "packet.h"
#include "send.h"
#include "soft-interface.h"
#include "tp_meter.h"
diff --git a/net/batman-adv/send.h b/net/batman-adv/send.h
index b9ef010bfb03..a9a0ebae73da 100644
--- a/net/batman-adv/send.h
+++ b/net/batman-adv/send.h
@@ -26,8 +26,7 @@
#include <linux/compiler.h>
#include <linux/spinlock.h>
#include <linux/types.h>
-
-#include "packet.h"
+#include <uapi/linux/batadv.h>
struct sk_buff;
diff --git a/net/batman-adv/soft-interface.c b/net/batman-adv/soft-interface.c
index 64d82e90d23c..cd46ac77ff14 100644
--- a/net/batman-adv/soft-interface.c
+++ b/net/batman-adv/soft-interface.c
@@ -51,6 +51,7 @@
#include <linux/stddef.h>
#include <linux/string.h>
#include <linux/types.h>
+#include <uapi/linux/batadv.h>
#include "bat_algo.h"
#include "bridge_loop_avoidance.h"
@@ -62,7 +63,6 @@
#include "multicast.h"
#include "network-coding.h"
#include "originator.h"
-#include "packet.h"
#include "send.h"
#include "sysfs.h"
#include "translation-table.h"
diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index eebd3d1ebfd7..328b5ff15aa6 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -41,6 +41,7 @@
#include <linux/string.h>
#include <linux/stringify.h>
#include <linux/workqueue.h>
+#include <uapi/linux/batadv.h>
#include "bridge_loop_avoidance.h"
#include "distributed-arp-table.h"
@@ -49,7 +50,6 @@
#include "hard-interface.h"
#include "log.h"
#include "network-coding.h"
-#include "packet.h"
#include "soft-interface.h"
static struct net_device *batadv_kobj_to_netdev(struct kobject *obj)
diff --git a/net/batman-adv/tp_meter.c b/net/batman-adv/tp_meter.c
index 152189b6f862..8c1230ce4697 100644
--- a/net/batman-adv/tp_meter.c
+++ b/net/batman-adv/tp_meter.c
@@ -51,13 +51,13 @@
#include <linux/timer.h>
#include <linux/wait.h>
#include <linux/workqueue.h>
+#include <uapi/linux/batadv.h>
#include <uapi/linux/batadv_genl.h>
#include "hard-interface.h"
#include "log.h"
#include "netlink.h"
#include "originator.h"
-#include "packet.h"
#include "send.h"
/**
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 0065b8a2a1e4..030cfac888e3 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -54,6 +54,7 @@
#include <net/genetlink.h>
#include <net/netlink.h>
#include <net/sock.h>
+#include <uapi/linux/batadv.h>
#include <uapi/linux/batadv_genl.h>
#include "bridge_loop_avoidance.h"
@@ -62,7 +63,6 @@
#include "log.h"
#include "netlink.h"
#include "originator.h"
-#include "packet.h"
#include "soft-interface.h"
#include "tvlv.h"
diff --git a/net/batman-adv/tvlv.c b/net/batman-adv/tvlv.c
index e5857e755ffd..509f251a5e7c 100644
--- a/net/batman-adv/tvlv.c
+++ b/net/batman-adv/tvlv.c
@@ -38,9 +38,9 @@
#include <linux/stddef.h>
#include <linux/string.h>
#include <linux/types.h>
+#include <uapi/linux/batadv.h>
#include "originator.h"
-#include "packet.h"
#include "send.h"
#include "tvlv.h"
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 9b78d9364e45..afe5a3ac2f39 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -37,10 +37,9 @@
#include <linux/types.h>
#include <linux/wait.h>
#include <linux/workqueue.h>
+#include <uapi/linux/batadv.h>
#include <uapi/linux/batadv_genl.h>
-#include "packet.h"
-
struct seq_file;
#ifdef CONFIG_BATMAN_ADV_DAT
--
2.11.0
The BIT(x) macro is no longer available for uapi headers because it is
defined outside of it (linux/bitops.h). The use of it must therefore be
avoided and replaced by an appropriate other representation.
Signed-off-by: Sven Eckelmann <[email protected]>
---
net/batman-adv/packet.h | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/net/batman-adv/packet.h b/net/batman-adv/packet.h
index 0d011012c71a..311ac6fd76ff 100644
--- a/net/batman-adv/packet.h
+++ b/net/batman-adv/packet.h
@@ -22,7 +22,6 @@
#define _NET_BATMAN_ADV_PACKET_H_
#include <asm/byteorder.h>
-#include <linux/bitops.h>
#include <linux/if_ether.h>
#include <linux/types.h>
@@ -94,9 +93,9 @@ enum batadv_subtype {
* one hop neighbor on the interface where it was originally received.
*/
enum batadv_iv_flags {
- BATADV_NOT_BEST_NEXT_HOP = BIT(0),
- BATADV_PRIMARIES_FIRST_HOP = BIT(1),
- BATADV_DIRECTLINK = BIT(2),
+ BATADV_NOT_BEST_NEXT_HOP = 1UL << 0,
+ BATADV_PRIMARIES_FIRST_HOP = 1UL << 1,
+ BATADV_DIRECTLINK = 1UL << 2,
};
/**
@@ -125,9 +124,9 @@ enum batadv_icmp_packettype {
* @BATADV_MCAST_WANT_ALL_IPV6: we want all IPv6 multicast packets
*/
enum batadv_mcast_flags {
- BATADV_MCAST_WANT_ALL_UNSNOOPABLES = BIT(0),
- BATADV_MCAST_WANT_ALL_IPV4 = BIT(1),
- BATADV_MCAST_WANT_ALL_IPV6 = BIT(2),
+ BATADV_MCAST_WANT_ALL_UNSNOOPABLES = 1UL << 0,
+ BATADV_MCAST_WANT_ALL_IPV4 = 1UL << 1,
+ BATADV_MCAST_WANT_ALL_IPV6 = 1UL << 2,
};
/* tt data subtypes */
@@ -141,10 +140,10 @@ enum batadv_mcast_flags {
* @BATADV_TT_FULL_TABLE: contains full table to replace existing table
*/
enum batadv_tt_data_flags {
- BATADV_TT_OGM_DIFF = BIT(0),
- BATADV_TT_REQUEST = BIT(1),
- BATADV_TT_RESPONSE = BIT(2),
- BATADV_TT_FULL_TABLE = BIT(4),
+ BATADV_TT_OGM_DIFF = 1UL << 0,
+ BATADV_TT_REQUEST = 1UL << 1,
+ BATADV_TT_RESPONSE = 1UL << 2,
+ BATADV_TT_FULL_TABLE = 1UL << 4,
};
/**
@@ -152,7 +151,7 @@ enum batadv_tt_data_flags {
* @BATADV_VLAN_HAS_TAG: whether the field contains a valid vlan tag or not
*/
enum batadv_vlan_flags {
- BATADV_VLAN_HAS_TAG = BIT(15),
+ BATADV_VLAN_HAS_TAG = 1UL << 15,
};
/**
--
2.11.0
This file contains the relevant information to let userspace communicate
with batman-adv over generic netlink. The relevant genl enums for the
attributes and commands have the prefix batadv_genl. Renaming this file to
this name therefore represents the content better and avoids confusion with
the file which will contain the packet format definitions.
Signed-off-by: Sven Eckelmann <[email protected]>
---
MAINTAINERS | 2 +-
include/uapi/linux/{batman_adv.h => batadv_genl.h} | 6 +++---
net/batman-adv/bat_algo.c | 2 +-
net/batman-adv/bat_iv_ogm.c | 2 +-
net/batman-adv/bat_v.c | 2 +-
net/batman-adv/bridge_loop_avoidance.c | 2 +-
net/batman-adv/gateway_client.c | 2 +-
net/batman-adv/main.c | 2 +-
net/batman-adv/netlink.c | 2 +-
net/batman-adv/originator.c | 2 +-
net/batman-adv/tp_meter.c | 2 +-
net/batman-adv/translation-table.c | 2 +-
net/batman-adv/types.h | 2 +-
13 files changed, 15 insertions(+), 15 deletions(-)
rename include/uapi/linux/{batman_adv.h => batadv_genl.h} (98%)
diff --git a/MAINTAINERS b/MAINTAINERS
index aa71ab52fd76..68abe39ee87a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2563,7 +2563,7 @@ S: Maintained
F: Documentation/ABI/testing/sysfs-class-net-batman-adv
F: Documentation/ABI/testing/sysfs-class-net-mesh
F: Documentation/networking/batman-adv.rst
-F: include/uapi/linux/batman_adv.h
+F: include/uapi/linux/batadv_genl.h
F: net/batman-adv/
BAYCOM/HDLCDRV DRIVERS FOR AX.25
diff --git a/include/uapi/linux/batman_adv.h b/include/uapi/linux/batadv_genl.h
similarity index 98%
rename from include/uapi/linux/batman_adv.h
rename to include/uapi/linux/batadv_genl.h
index a0fed6c01a46..8ed28676e198 100644
--- a/include/uapi/linux/batman_adv.h
+++ b/include/uapi/linux/batadv_genl.h
@@ -22,8 +22,8 @@
* DEALINGS IN THE SOFTWARE.
*/
-#ifndef _UAPI_LINUX_BATMAN_ADV_H_
-#define _UAPI_LINUX_BATMAN_ADV_H_
+#ifndef _UAPI_LINUX_BATADV_GENL_H_
+#define _UAPI_LINUX_BATADV_GENL_H_
#define BATADV_GENL_NAME "batadv"
@@ -423,4 +423,4 @@ enum batadv_tp_meter_reason {
BATADV_TP_REASON_TOO_MANY = 133,
};
-#endif /* _UAPI_LINUX_BATMAN_ADV_H_ */
+#endif /* _UAPI_LINUX_BATADV_GENL_H_ */
diff --git a/net/batman-adv/bat_algo.c b/net/batman-adv/bat_algo.c
index 37390a21e5e9..07392a25235f 100644
--- a/net/batman-adv/bat_algo.c
+++ b/net/batman-adv/bat_algo.c
@@ -31,7 +31,7 @@
#include <linux/string.h>
#include <net/genetlink.h>
#include <net/netlink.h>
-#include <uapi/linux/batman_adv.h>
+#include <uapi/linux/batadv_genl.h>
#include "bat_algo.h"
#include "netlink.h"
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 4055338de7f0..7727a39c4265 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -54,7 +54,7 @@
#include <linux/workqueue.h>
#include <net/genetlink.h>
#include <net/netlink.h>
-#include <uapi/linux/batman_adv.h>
+#include <uapi/linux/batadv_genl.h>
#include "bat_algo.h"
#include "bitarray.h"
diff --git a/net/batman-adv/bat_v.c b/net/batman-adv/bat_v.c
index 7eea24204b55..b43ba53daafd 100644
--- a/net/batman-adv/bat_v.c
+++ b/net/batman-adv/bat_v.c
@@ -39,7 +39,7 @@
#include <linux/workqueue.h>
#include <net/genetlink.h>
#include <net/netlink.h>
-#include <uapi/linux/batman_adv.h>
+#include <uapi/linux/batadv_genl.h>
#include "bat_algo.h"
#include "bat_v_elp.h"
diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index 15d64a575034..1a9594d68db2 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -52,7 +52,7 @@
#include <net/genetlink.h>
#include <net/netlink.h>
#include <net/sock.h>
-#include <uapi/linux/batman_adv.h>
+#include <uapi/linux/batadv_genl.h>
#include "hard-interface.h"
#include "hash.h"
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index 5e2d00731e9f..23c8032ce478 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -45,7 +45,7 @@
#include <linux/stddef.h>
#include <linux/udp.h>
#include <net/sock.h>
-#include <uapi/linux/batman_adv.h>
+#include <uapi/linux/batadv_genl.h>
#include "gateway_common.h"
#include "hard-interface.h"
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index a56ae758f799..c034e5c504c4 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -48,7 +48,7 @@
#include <linux/workqueue.h>
#include <net/dsfield.h>
#include <net/rtnetlink.h>
-#include <uapi/linux/batman_adv.h>
+#include <uapi/linux/batadv_genl.h>
#include "bat_algo.h"
#include "bat_iv_ogm.h"
diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
index a9679bdca432..5a66eb6c45f1 100644
--- a/net/batman-adv/netlink.c
+++ b/net/batman-adv/netlink.c
@@ -42,7 +42,7 @@
#include <net/genetlink.h>
#include <net/netlink.h>
#include <net/sock.h>
-#include <uapi/linux/batman_adv.h>
+#include <uapi/linux/batadv_genl.h>
#include "bat_algo.h"
#include "bridge_loop_avoidance.h"
diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c
index dafc73811aa6..ec7b357e1781 100644
--- a/net/batman-adv/originator.c
+++ b/net/batman-adv/originator.c
@@ -41,7 +41,7 @@
#include <linux/stddef.h>
#include <linux/workqueue.h>
#include <net/sock.h>
-#include <uapi/linux/batman_adv.h>
+#include <uapi/linux/batadv_genl.h>
#include "bat_algo.h"
#include "distributed-arp-table.h"
diff --git a/net/batman-adv/tp_meter.c b/net/batman-adv/tp_meter.c
index d73346057310..152189b6f862 100644
--- a/net/batman-adv/tp_meter.c
+++ b/net/batman-adv/tp_meter.c
@@ -51,7 +51,7 @@
#include <linux/timer.h>
#include <linux/wait.h>
#include <linux/workqueue.h>
-#include <uapi/linux/batman_adv.h>
+#include <uapi/linux/batadv_genl.h>
#include "hard-interface.h"
#include "log.h"
diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
index 5d3ba12002df..0065b8a2a1e4 100644
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@ -54,7 +54,7 @@
#include <net/genetlink.h>
#include <net/netlink.h>
#include <net/sock.h>
-#include <uapi/linux/batman_adv.h>
+#include <uapi/linux/batadv_genl.h>
#include "bridge_loop_avoidance.h"
#include "hard-interface.h"
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index e4a79f9e2e24..9b78d9364e45 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -37,7 +37,7 @@
#include <linux/types.h>
#include <linux/wait.h>
#include <linux/workqueue.h>
-#include <uapi/linux/batman_adv.h>
+#include <uapi/linux/batadv_genl.h>
#include "packet.h"
--
2.11.0
On Tue, Dec 5, 2017 at 6:35 AM, Sven Eckelmann
<[email protected]> wrote:
> The batman-adv unicast packets contain a full layer 2 frame in encapsulated
> form. The flow dissector must therefore be able to parse the batman-adv
> unicast header to reach the layer 2+3 information.
>
> +--------------------+
> | ip(v6)hdr |
> +--------------------+
> | inner ethhdr |
> +--------------------+
> | batadv unicast hdr |
> +--------------------+
> | outer ethhdr |
> +--------------------+
>
> The obtained information from the upper layer can then be used by RPS to
> schedule the processing on separate cores. This allows better distribution
> of multiple flows from the same neighbor to different cores.
>
> Signed-off-by: Sven Eckelmann <[email protected]>
> ---
> net/core/flow_dissector.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 15ce30063765..784cc07fc58e 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -24,6 +24,7 @@
> #include <linux/tcp.h>
> #include <net/flow_dissector.h>
> #include <scsi/fc/fc_fcoe.h>
> +#include <uapi/linux/batadv.h>
>
> static void dissector_set_key(struct flow_dissector *flow_dissector,
> enum flow_dissector_key_id key_id)
> @@ -696,6 +697,35 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>
> break;
> }
> + case htons(ETH_P_BATMAN): {
> + struct {
> + struct batadv_unicast_packet batadv_unicast;
> + struct ethhdr eth;
> + } *hdr, _hdr;
> +
> + hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen,
> + &_hdr);
> + if (!hdr) {
> + fdret = FLOW_DISSECT_RET_OUT_BAD;
> + break;
> + }
> +
> + if (hdr->batadv_unicast.version != BATADV_COMPAT_VERSION) {
> + fdret = FLOW_DISSECT_RET_OUT_BAD;
> + break;
> + }
> +
> + if (hdr->batadv_unicast.packet_type != BATADV_UNICAST) {
> + fdret = FLOW_DISSECT_RET_OUT_BAD;
> + break;
> + }
> +
> + proto = hdr->eth.h_proto;
> + nhoff += sizeof(*hdr);
> +
> + fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
> + break;
> + }
> case htons(ETH_P_8021AD):
> case htons(ETH_P_8021Q): {
> const struct vlan_hdr *vlan;
> --
> 2.11.0
>
Switch statements with cases having many LOC is hard to read and
__skb_flow_dissect is aleady quite convoluted to begin with.
I suggest putting this in a static function similar to how MPLS and
GRE are handled.
Tom
On Dienstag, 5. Dezember 2017 09:19:45 CET Tom Herbert wrote:
[...]
> Switch statements with cases having many LOC is hard to read and
> __skb_flow_dissect is aleady quite convoluted to begin with.
>
> I suggest putting this in a static function similar to how MPLS and
> GRE are handled.
Thanks for the feedback.
I was not sure whether "inline" or an extra function would be preferred. I've
then decided to implement it like most of the other protocols. But since an
extra function is the preferred method for handling new protos, I will move it
to an extra function.
The change can already be found in
https://git.open-mesh.org/linux-merge.git/shortlog/refs/heads/ecsv/flowdissector
I also saw that you've introduced in
commit 823b96939578 ("flow_dissector: Add control/reporting of encapsulation")
a flag to stop dissecting when something encapsulated was detected. It is not
100% clear to me when the FLOW_DIS_ENCAPSULATION should be set and
FLOW_DISSECTOR_F_STOP_AT_ENCAP be checked. Only when there is IP/eth in IP
(like in the two examples from the commit message) or also when there is a
ethernet header, followed by batman-adv unicast header and again followed by
an ethernet header?
Right now, the flag FLOW_DISSECTOR_F_STOP_AT_ENCAP is only used by
fib_multipath_hash - so it doesn't really help me to understand it. But the
FLOW_DIS_ENCAPSULATION is checked by drivers/net/ethernet/broadcom/bnxt/bnxt.c
to set it in a special tunnel_type (anytunnel) mode for IPv4/IPv6 packets. So
my (wild) guess right now is that these flags should only be used/checked when
handling encapsulation inside IPv4/IPv6 packets. But maybe you can correct me
here.
Kind regards,
Sven
On Tue, Dec 5, 2017 at 9:35 AM, Sven Eckelmann
<[email protected]> wrote:
> This file contains the relevant information to let userspace communicate
> with batman-adv over generic netlink. The relevant genl enums for the
> attributes and commands have the prefix batadv_genl. Renaming this file to
> this name therefore represents the content better and avoids confusion with
> the file which will contain the packet format definitions.
>
> Signed-off-by: Sven Eckelmann <[email protected]>
> ---
> MAINTAINERS | 2 +-
> include/uapi/linux/{batman_adv.h => batadv_genl.h} | 6 +++---
This and the previous patch changes uapi. That might break userspace
applications that rely on it.
On Wed, Dec 6, 2017 at 5:26 AM, Sven Eckelmann
<[email protected]> wrote:
> On Dienstag, 5. Dezember 2017 09:19:45 CET Tom Herbert wrote:
> [...]
>> Switch statements with cases having many LOC is hard to read and
>> __skb_flow_dissect is aleady quite convoluted to begin with.
>>
>> I suggest putting this in a static function similar to how MPLS and
>> GRE are handled.
Perhaps it can even be behind a static key depending on whether any
devices are active, adjusted in batadv_hardif_(en|dis)able_interface.
> Thanks for the feedback.
>
> I was not sure whether "inline" or an extra function would be preferred. I've
> then decided to implement it like most of the other protocols. But since an
> extra function is the preferred method for handling new protos, I will move it
> to an extra function.
>
> The change can already be found in
> https://git.open-mesh.org/linux-merge.git/shortlog/refs/heads/ecsv/flowdissector
>
> I also saw that you've introduced in
> commit 823b96939578 ("flow_dissector: Add control/reporting of encapsulation")
> a flag to stop dissecting when something encapsulated was detected. It is not
> 100% clear to me when the FLOW_DIS_ENCAPSULATION should be set and
> FLOW_DISSECTOR_F_STOP_AT_ENCAP be checked. Only when there is IP/eth in IP
> (like in the two examples from the commit message) or also when there is a
> ethernet header, followed by batman-adv unicast header and again followed by
> an ethernet header?
Please implement FLOW_DISSECTOR_F_STOP_AT_ENCAP. It may
be used in more flow dissector paths in the future.
The features are also used by GRE, which can encap Ethernet, for an example
that is closer to this protocol.
On Mittwoch, 6. Dezember 2017 11:42:33 CET Willem de Bruijn wrote:
> On Tue, Dec 5, 2017 at 9:35 AM, Sven Eckelmann
> <[email protected]> wrote:
> > This file contains the relevant information to let userspace communicate
> > with batman-adv over generic netlink. The relevant genl enums for the
> > attributes and commands have the prefix batadv_genl. Renaming this file to
> > this name therefore represents the content better and avoids confusion
with
> > the file which will contain the packet format definitions.
> >
> > Signed-off-by: Sven Eckelmann <[email protected]>
> > ---
> > MAINTAINERS | 2 +-
> > include/uapi/linux/{batman_adv.h => batadv_genl.h} | 6 +++---
>
> This and the previous patch changes uapi. That might break userspace
> applications that rely on it.
I am not aware of any application because all (alfred, batctl and some gluon
integration) of them currently ship their own copy because distribution didn't
catch up. And this is also the reason why I want to do it now - not later.
Kind regards,
Sven
On Wed, Dec 6, 2017 at 11:55 AM, Sven Eckelmann
<[email protected]> wrote:
> On Mittwoch, 6. Dezember 2017 11:42:33 CET Willem de Bruijn wrote:
>> On Tue, Dec 5, 2017 at 9:35 AM, Sven Eckelmann
>> <[email protected]> wrote:
>> > This file contains the relevant information to let userspace communicate
>> > with batman-adv over generic netlink. The relevant genl enums for the
>> > attributes and commands have the prefix batadv_genl. Renaming this file to
>> > this name therefore represents the content better and avoids confusion
> with
>> > the file which will contain the packet format definitions.
>> >
>> > Signed-off-by: Sven Eckelmann <[email protected]>
>> > ---
>> > MAINTAINERS | 2 +-
>> > include/uapi/linux/{batman_adv.h => batadv_genl.h} | 6 +++---
>>
>> This and the previous patch changes uapi. That might break userspace
>> applications that rely on it.
>
> I am not aware of any application because all (alfred, batctl and some gluon
> integration) of them currently ship their own copy because distribution didn't
> catch up. And this is also the reason why I want to do it now - not later.
That assumes that you know all applications, including those not
publicly available. It may be true in this instance, but it is not
possible to be certain.
On Wed, Dec 6, 2017 at 8:54 AM, Willem de Bruijn
<[email protected]> wrote:
> On Wed, Dec 6, 2017 at 5:26 AM, Sven Eckelmann
> <[email protected]> wrote:
>> On Dienstag, 5. Dezember 2017 09:19:45 CET Tom Herbert wrote:
>> [...]
>>> Switch statements with cases having many LOC is hard to read and
>>> __skb_flow_dissect is aleady quite convoluted to begin with.
>>>
>>> I suggest putting this in a static function similar to how MPLS and
>>> GRE are handled.
>
> Perhaps it can even be behind a static key depending on whether any
> devices are active, adjusted in batadv_hardif_(en|dis)able_interface.
>
It's aready in a switch statement so static key shouldn't make a
difference. Also, we have made flow dissector operate indendently of
whether to end protocol is enable (for instance GRE is dissected
regarless of whether and GRE tunnels are configured).
Tom
>> Thanks for the feedback.
>>
>> I was not sure whether "inline" or an extra function would be preferred. I've
>> then decided to implement it like most of the other protocols. But since an
>> extra function is the preferred method for handling new protos, I will move it
>> to an extra function.
>>
>> The change can already be found in
>> https://git.open-mesh.org/linux-merge.git/shortlog/refs/heads/ecsv/flowdissector
>>
>> I also saw that you've introduced in
>> commit 823b96939578 ("flow_dissector: Add control/reporting of encapsulation")
>> a flag to stop dissecting when something encapsulated was detected. It is not
>> 100% clear to me when the FLOW_DIS_ENCAPSULATION should be set and
>> FLOW_DISSECTOR_F_STOP_AT_ENCAP be checked. Only when there is IP/eth in IP
>> (like in the two examples from the commit message) or also when there is a
>> ethernet header, followed by batman-adv unicast header and again followed by
>> an ethernet header?
>
> Please implement FLOW_DISSECTOR_F_STOP_AT_ENCAP. It may
> be used in more flow dissector paths in the future.
>
> The features are also used by GRE, which can encap Ethernet, for an example
> that is closer to this protocol.
On Mittwoch, 6. Dezember 2017 11:54:13 CET Willem de Bruijn wrote:
> Perhaps it can even be behind a static key depending on whether any
> devices are active, adjusted in batadv_hardif_(en|dis)able_interface.
I don't like that because we don't need batman-adv loaded to simply forward
(bridge) traffic between interfaces. And not being able to use RPS with
multiple cores just because the batman-adv module (and interfaces) is not
enabled seems to be counter-intuitive.
> Please implement FLOW_DISSECTOR_F_STOP_AT_ENCAP. It may
> be used in more flow dissector paths in the future.
>
> The features are also used by GRE, which can encap Ethernet, for an example
> that is closer to this protocol.
Thanks for the feedback. I have it now implemented like in GRE.
The change can already be found in
https://git.open-mesh.org/linux-merge.git/shortlog/refs/heads/ecsv/flowdissector
Kind regards,
Sven
> On Mittwoch, 6. Dezember 2017 11:54:13 CET Willem de Bruijn wrote:
>> Perhaps it can even be behind a static key depending on whether any
>> devices are active, adjusted in batadv_hardif_(en|dis)able_interface.
>
> I don't like that because we don't need batman-adv loaded to simply forward
> (bridge) traffic between interfaces. And not being able to use RPS with
> multiple cores just because the batman-adv module (and interfaces) is not
> enabled seems to be counter-intuitive.
Okay. Ack on Tom's points, too.
I want to be able to administratively limit the footprint of flow dissector,
but agreed that this can be configured independent from the rest of the
datapath, i.e., with knobs specific to flow dissector.
>> Please implement FLOW_DISSECTOR_F_STOP_AT_ENCAP. It may
>> be used in more flow dissector paths in the future.
>>
>> The features are also used by GRE, which can encap Ethernet, for an example
>> that is closer to this protocol.
>
> Thanks for the feedback. I have it now implemented like in GRE.
>
> The change can already be found in
> https://git.open-mesh.org/linux-merge.git/shortlog/refs/heads/ecsv/flowdissector
Great, thanks.
On Mittwoch, 6. Dezember 2017 11:58:14 CET Willem de Bruijn wrote:
[...]
> >> > ---
> >> > MAINTAINERS | 2 +-
> >> > include/uapi/linux/{batman_adv.h => batadv_genl.h} | 6 +++---
> >>
> >> This and the previous patch changes uapi. That might break userspace
> >> applications that rely on it.
> >
> > I am not aware of any application because all (alfred, batctl and some gluon
> > integration) of them currently ship their own copy because distribution didn't
> > catch up. And this is also the reason why I want to do it now - not later.
>
> That assumes that you know all applications, including those not
> publicly available. It may be true in this instance, but it is not
> possible to be certain.
I've just talked with Simon. Because you have a problem with these two
changes, he suggested that I should drop these two patches and merge packet.h
with the uapi batadv genl header batman_adv.h
Kind regards,
Sven
On Freitag, 15. Dezember 2017 11:32:05 CET Sven Eckelmann wrote:
> On Mittwoch, 6. Dezember 2017 11:58:14 CET Willem de Bruijn wrote:
> [...]
> > >> > ---
> > >> > MAINTAINERS | 2 +-
> > >> > include/uapi/linux/{batman_adv.h => batadv_genl.h} | 6 +++---
> > >>
> > >> This and the previous patch changes uapi. That might break userspace
> > >> applications that rely on it.
> > >
> > > I am not aware of any application because all (alfred, batctl and some gluon
> > > integration) of them currently ship their own copy because distribution didn't
> > > catch up. And this is also the reason why I want to do it now - not later.
> >
> > That assumes that you know all applications, including those not
> > publicly available. It may be true in this instance, but it is not
> > possible to be certain.
>
> I've just talked with Simon. Because you have a problem with these two
> changes, he suggested that I should drop these two patches and merge packet.h
> with the uapi batadv genl header batman_adv.h
No, this is also bad because batman_adv.h is MIT license and packet.h is
GPL-2. So what other name would you suggest for packet.h? batman_adv_packet.h?
Kind regards,
Sven
On Fri, Dec 15, 2017 at 6:48 AM, Sven Eckelmann
<[email protected]> wrote:
> On Freitag, 15. Dezember 2017 11:32:05 CET Sven Eckelmann wrote:
>> On Mittwoch, 6. Dezember 2017 11:58:14 CET Willem de Bruijn wrote:
>> [...]
>> > >> > ---
>> > >> > MAINTAINERS | 2 +-
>> > >> > include/uapi/linux/{batman_adv.h => batadv_genl.h} | 6 +++---
>> > >>
>> > >> This and the previous patch changes uapi. That might break userspace
>> > >> applications that rely on it.
>> > >
>> > > I am not aware of any application because all (alfred, batctl and some gluon
>> > > integration) of them currently ship their own copy because distribution didn't
>> > > catch up. And this is also the reason why I want to do it now - not later.
>> >
>> > That assumes that you know all applications, including those not
>> > publicly available. It may be true in this instance, but it is not
>> > possible to be certain.
>>
>> I've just talked with Simon. Because you have a problem with these two
>> changes, he suggested that I should drop these two patches and merge packet.h
>> with the uapi batadv genl header batman_adv.h
>
> No, this is also bad because batman_adv.h is MIT license and packet.h is
> GPL-2. So what other name would you suggest for packet.h? batman_adv_packet.h?
Sure, that sounds great. Thanks.
On Freitag, 15. Dezember 2017 11:57:55 CET Willem de Bruijn wrote:
> > No, this is also bad because batman_adv.h is MIT license and packet.h is
> > GPL-2. So what other name would you suggest for packet.h? batman_adv_packet.h?
>
> Sure, that sounds great. Thanks.
Really? Isn't include/uapi/linux/batman_adv_packet.h looking like an accident
which never should have had happened?
Kind regards,
Sven
On Fri, Dec 15, 2017 at 12:18 PM, Sven Eckelmann
<[email protected]> wrote:
> On Freitag, 15. Dezember 2017 11:57:55 CET Willem de Bruijn wrote:
>> > No, this is also bad because batman_adv.h is MIT license and packet.h is
>> > GPL-2. So what other name would you suggest for packet.h? batman_adv_packet.h?
>>
>> Sure, that sounds great. Thanks.
>
> Really? Isn't include/uapi/linux/batman_adv_packet.h looking like an accident
> which never should have had happened?
My only point was that renaming and modifying existing uapi files
can break userspace compilation.
As long as the existing files are not changed, I don't have a strong
opinion on naming for new files.