b.a.t.m.a.n.lists.open-mesh.org archive mirror
 help / color / mirror / Atom feed
* [B.A.T.M.A.N.] [PATCH next v2 1/3] batman-adv: Introduce compat-patches support
@ 2016-09-15 13:52 Sven Eckelmann
  2016-09-15 13:52 ` [B.A.T.M.A.N.] [PATCH next v2 2/3] batman-adv: Use simple script to patch minor compat problems Sven Eckelmann
  2016-09-15 13:52 ` [B.A.T.M.A.N.] [PATCH next v2 3/3] batman-adv: make netlink attributes const Sven Eckelmann
  0 siblings, 2 replies; 6+ messages in thread
From: Sven Eckelmann @ 2016-09-15 13:52 UTC (permalink / raw)
  To: b.a.t.m.a.n

compat-includes/compat.h can usually be used to solve compatibility
problems with older kernels. This works well for functions, defines/enums
and sometimes even structures that were introduced.

But this can fail when structs changed. Some of these can be solved in
crude ways but sometimes it is unavoidable to have a version specific code.
Unfortunately, this kind of code is not acceptable in the kernel and thus
the compat infrastructure of the external module has to do add it
automatically before the source is compiled.

This process works by creating a build directory which is prefilled with
the source from net/batman-adv/. The patches from compat-patches/ will be
applied on top of this copy and then the code is compiled.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
v2:
 - Rebased on top of current next

 .gitignore              | 10 ++++------
 Makefile                | 31 +++++++++++++++++++++++++++----
 compat-patches/README   | 23 +++++++++++++++++++++++
 compat-sources/Makefile |  6 +++---
 4 files changed, 57 insertions(+), 13 deletions(-)
 create mode 100644 compat-patches/README

diff --git a/.gitignore b/.gitignore
index 4c03561..15a99aa 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,10 +1,8 @@
+/build/
 /compat-autoconf.h
 /compat-autoconf.h.tmp
 /compat-sources/**/.*
 /compat-sources/**/*.o
-/net/batman-adv/.*
-/net/batman-adv/batman-adv.ko
-/net/batman-adv/batman-adv.mod.c
-/net/batman-adv/modules.order
-/net/batman-adv/Module.symvers
-/net/batman-adv/*.o
+/modules.order
+/Module.symvers
+/.tmp_versions
diff --git a/Makefile b/Makefile
index d42bb56..b105290 100644
--- a/Makefile
+++ b/Makefile
@@ -33,6 +33,7 @@ export CONFIG_BATMAN_ADV_MCAST=y
 export CONFIG_BATMAN_ADV_BATMAN_V=n
 
 PWD:=$(shell pwd)
+BUILD_DIR=$(PWD)/build
 KERNELPATH ?= /lib/modules/$(shell uname -r)/build
 # sanity check: does KERNELPATH exist?
 ifeq ($(shell cd $(KERNELPATH) && pwd),)
@@ -41,6 +42,14 @@ endif
 
 export KERNELPATH
 RM ?= rm -f
+MKDIR := mkdir -p
+PATCH_FLAGS = --batch --fuzz=0 --forward --strip=1 --unified --version-control=never -g0 --remove-empty-files --no-backup-if-mismatch --reject-file=-
+PATCH := patch $(PATCH_FLAGS) -i
+CP := cp -fpR
+
+SOURCE = $(wildcard net/batman-adv/*.[ch]) net/batman-adv/Makefile
+SOURCE_BUILD = $(wildcard $(BUILD_DIR)/net/batman-adv/*.[ch]) $(BUILD_DIR)/net/batman-adv/Makefile
+SOURCE_STAMP = $(BUILD_DIR)/net/batman-adv/.compat-prepared
 
 REVISION= $(shell	if [ -d "$(PWD)/.git" ]; then \
 				echo $$(git --git-dir="$(PWD)/.git" describe --always --dirty --match "v*" |sed 's/^v//' 2> /dev/null || echo "[unknown]"); \
@@ -57,7 +66,7 @@ endif
 
 include $(PWD)/compat-sources/Makefile
 
-obj-y += net/batman-adv/
+obj-y += build/net/batman-adv/
 
 export batman-adv-y
 
@@ -76,18 +85,32 @@ BUILD_FLAGS := \
 	CONFIG_BATMAN_ADV_BATMAN_V=$(CONFIG_BATMAN_ADV_BATMAN_V) \
 	INSTALL_MOD_DIR=updates/
 
-all: config
+all: config $(SOURCE_STAMP)
 	$(MAKE) -C $(KERNELPATH) $(BUILD_FLAGS)	modules
 
 clean:
 	$(RM) compat-autoconf.h*
-	$(MAKE) -C $(KERNELPATH) $(BUILD_FLAGS) clean
+	$(RM) -r $(BUILD_DIR)
 
-install: config
+install: config $(SOURCE_STAMP)
 	$(MAKE) -C $(KERNELPATH) $(BUILD_FLAGS) modules_install
 	depmod -a
 
 config:
 	$(PWD)/gen-compat-autoconf.sh $(PWD)/compat-autoconf.h
 
+$(SOURCE_STAMP): $(SOURCE) compat-patches/*
+	$(MKDIR) $(BUILD_DIR)/net/batman-adv/
+	@$(RM) $(SOURCE_BUILD)
+	@$(CP) $(SOURCE) $(BUILD_DIR)/net/batman-adv/
+	@set -e; \
+	patches="$$(ls -1 compat-patches/|grep '.patch$$'|sort)"; \
+	for i in $${patches}; do \
+		echo '  COMPAT_PATCH '$${i};\
+		cd $(BUILD_DIR); \
+		$(PATCH) ../compat-patches/$${i}; \
+		cd - > /dev/null; \
+	done
+	touch $(SOURCE_STAMP)
+
 .PHONY: all clean install config
diff --git a/compat-patches/README b/compat-patches/README
new file mode 100644
index 0000000..3bbddb3
--- /dev/null
+++ b/compat-patches/README
@@ -0,0 +1,23 @@
+WARNING
+=======
+
+Please avoid using the compat-patches/ to implement support for old kernels.
+This should be the last resort.
+
+ * it is nearly always possible to use compat-includes/ to do the same with a
+   lot less problems
+
+ * maintaining these patches is *censored*
+
+GENERATING A PATCH
+==================
+
+If it not possible to avoid a patch then please make the patch as small as
+possible. Even refactor the code which has to be patched to reduce the
+size/number of the changes.
+
+Please use git-format-patches to generate them and order same inside via the
+XXXX- prefix of the patch name.
+
+    git format-patch --abbrev=7 -U3 --diff-algorithm=histogram --no-signature \
+	--format=format:'From: %an <%ae>%nDate: %aD%nSubject: [PATCH] %B' -1
diff --git a/compat-sources/Makefile b/compat-sources/Makefile
index a45c202..c8bae49 100644
--- a/compat-sources/Makefile
+++ b/compat-sources/Makefile
@@ -1,3 +1,3 @@
-batman-adv-$(CONFIG_BATMAN_ADV_MCAST) += ../../compat-sources/net/core/skbuff.o
-batman-adv-$(CONFIG_BATMAN_ADV_MCAST) += ../../compat-sources/net/ipv4/igmp.o
-batman-adv-$(CONFIG_BATMAN_ADV_MCAST) += ../../compat-sources/net/ipv6/mcast_snoop.o
+batman-adv-$(CONFIG_BATMAN_ADV_MCAST) += ../../../compat-sources/net/core/skbuff.o
+batman-adv-$(CONFIG_BATMAN_ADV_MCAST) += ../../../compat-sources/net/ipv4/igmp.o
+batman-adv-$(CONFIG_BATMAN_ADV_MCAST) += ../../../compat-sources/net/ipv6/mcast_snoop.o
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [B.A.T.M.A.N.] [PATCH next v2 2/3] batman-adv: Use simple script to patch minor compat problems
  2016-09-15 13:52 [B.A.T.M.A.N.] [PATCH next v2 1/3] batman-adv: Introduce compat-patches support Sven Eckelmann
@ 2016-09-15 13:52 ` Sven Eckelmann
  2016-09-16 15:34   ` Sven Eckelmann
  2016-09-15 13:52 ` [B.A.T.M.A.N.] [PATCH next v2 3/3] batman-adv: make netlink attributes const Sven Eckelmann
  1 sibling, 1 reply; 6+ messages in thread
From: Sven Eckelmann @ 2016-09-15 13:52 UTC (permalink / raw)
  To: b.a.t.m.a.n

Unified diff base patches have the problem that they fix only a single line
and depend on the surrounding. This approach can fail relative often when
the surrounding code will be changed often. Thus semantic patches (like
spatch from coccinelle) or simple find+replace scripts are better for this
approach.

Introduce a simple script file which will store these kind of find+replace
instructions.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
v2:
 - introduce this patch to support the patch 3

 Makefile                       | 3 ++-
 compat-patches/replacements.sh | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)
 create mode 100755 compat-patches/replacements.sh

diff --git a/Makefile b/Makefile
index b105290..7ef2569 100644
--- a/Makefile
+++ b/Makefile
@@ -99,7 +99,7 @@ install: config $(SOURCE_STAMP)
 config:
 	$(PWD)/gen-compat-autoconf.sh $(PWD)/compat-autoconf.h
 
-$(SOURCE_STAMP): $(SOURCE) compat-patches/*
+$(SOURCE_STAMP): $(SOURCE) compat-patches/* compat-patches/replacements.sh
 	$(MKDIR) $(BUILD_DIR)/net/batman-adv/
 	@$(RM) $(SOURCE_BUILD)
 	@$(CP) $(SOURCE) $(BUILD_DIR)/net/batman-adv/
@@ -111,6 +111,7 @@ $(SOURCE_STAMP): $(SOURCE) compat-patches/*
 		$(PATCH) ../compat-patches/$${i}; \
 		cd - > /dev/null; \
 	done
+	compat-patches/replacements.sh
 	touch $(SOURCE_STAMP)
 
 .PHONY: all clean install config
diff --git a/compat-patches/replacements.sh b/compat-patches/replacements.sh
new file mode 100755
index 0000000..c7875c0
--- /dev/null
+++ b/compat-patches/replacements.sh
@@ -0,0 +1,3 @@
+#! /bin/sh
+
+set -e
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [B.A.T.M.A.N.] [PATCH next v2 3/3] batman-adv: make netlink attributes const
  2016-09-15 13:52 [B.A.T.M.A.N.] [PATCH next v2 1/3] batman-adv: Introduce compat-patches support Sven Eckelmann
  2016-09-15 13:52 ` [B.A.T.M.A.N.] [PATCH next v2 2/3] batman-adv: Use simple script to patch minor compat problems Sven Eckelmann
@ 2016-09-15 13:52 ` Sven Eckelmann
  2016-10-09  2:07   ` Linus Lüssing
  1 sibling, 1 reply; 6+ messages in thread
From: Sven Eckelmann @ 2016-09-15 13:52 UTC (permalink / raw)
  To: b.a.t.m.a.n

From: stephen hemminger <stephen@networkplumber.org>

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
[sven@narfation.org: Add compat-patch script]
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
v2:
 - new patch picked from the net-next.git

 compat-patches/replacements.sh | 6 ++++++
 net/batman-adv/netlink.c       | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/compat-patches/replacements.sh b/compat-patches/replacements.sh
index c7875c0..974aa96 100755
--- a/compat-patches/replacements.sh
+++ b/compat-patches/replacements.sh
@@ -1,3 +1,9 @@
 #! /bin/sh
 
 set -e
+
+# for kernel < 3.13 to make netlink compat code work
+sed -i \
+	-e 's/^static const struct genl_multicast_group batadv_netlink_mcgrps/static struct genl_multicast_group batadv_netlink_mcgrps/' \
+	-e 's/^static const struct nla_policy batadv_netlink_policy/static const struct nla_policy batadv_netlink_policy/' \
+	build/net/batman-adv/netlink.c
diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
index 18831e7..64cb6ac 100644
--- a/net/batman-adv/netlink.c
+++ b/net/batman-adv/netlink.c
@@ -62,11 +62,11 @@ enum batadv_netlink_multicast_groups {
 	BATADV_NL_MCGRP_TPMETER,
 };
 
-static struct genl_multicast_group batadv_netlink_mcgrps[] = {
+static const struct genl_multicast_group batadv_netlink_mcgrps[] = {
 	[BATADV_NL_MCGRP_TPMETER] = { .name = BATADV_NL_MCAST_GROUP_TPMETER },
 };
 
-static struct nla_policy batadv_netlink_policy[NUM_BATADV_ATTR] = {
+static const struct nla_policy batadv_netlink_policy[NUM_BATADV_ATTR] = {
 	[BATADV_ATTR_VERSION]		= { .type = NLA_STRING },
 	[BATADV_ATTR_ALGO_NAME]		= { .type = NLA_STRING },
 	[BATADV_ATTR_MESH_IFINDEX]	= { .type = NLA_U32 },
-- 
2.9.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH next v2 2/3] batman-adv: Use simple script to patch minor compat problems
  2016-09-15 13:52 ` [B.A.T.M.A.N.] [PATCH next v2 2/3] batman-adv: Use simple script to patch minor compat problems Sven Eckelmann
@ 2016-09-16 15:34   ` Sven Eckelmann
  0 siblings, 0 replies; 6+ messages in thread
From: Sven Eckelmann @ 2016-09-16 15:34 UTC (permalink / raw)
  To: b.a.t.m.a.n

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

On Donnerstag, 15. September 2016 15:52:48 CEST Sven Eckelmann wrote:
> Unified diff base patches have the problem that they fix only a single line
> and depend on the surrounding. This approach can fail relative often when
> the surrounding code will be changed often. Thus semantic patches (like
> spatch from coccinelle) or simple find+replace scripts are better for this
> approach.
> 
> Introduce a simple script file which will store these kind of find+replace
> instructions.
> 
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---

We could also skip this step and directly use coccinelle. I started a branch
some examples to cleanup our compat hacks [1]. The change for the netlink
const change is also already available [2].

Kind regards,
	Sven


[1] https://git.open-mesh.org/batman-adv.git/shortlog/refs/heads/ecsv/buildpatch
[2] https://git.open-mesh.org/batman-adv.git/commitdiff/2895226999760f2f2b34b506d33dc54c3bb43ca4?hp=7582ba83a3c7870dcb00f166259fbbb65600566c

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH next v2 3/3] batman-adv: make netlink attributes const
  2016-09-15 13:52 ` [B.A.T.M.A.N.] [PATCH next v2 3/3] batman-adv: make netlink attributes const Sven Eckelmann
@ 2016-10-09  2:07   ` Linus Lüssing
  2016-10-09  6:25     ` Sven Eckelmann
  0 siblings, 1 reply; 6+ messages in thread
From: Linus Lüssing @ 2016-10-09  2:07 UTC (permalink / raw)
  To: The list for a Better Approach To Mobile Ad-hoc Networking

On Thu, Sep 15, 2016 at 03:52:49PM +0200, Sven Eckelmann wrote:
> +sed -i \
> +	-e 's/^static const struct genl_multicast_group batadv_netlink_mcgrps/static struct genl_multicast_group batadv_netlink_mcgrps/' \
> +	-e 's/^static const struct nla_policy batadv_netlink_policy/static const struct nla_policy batadv_netlink_policy/' \

For the second thing: Replacing the string with itself?


Just wondering... I simply tried casting the const away and that
seems to compile without a warning:

-----
static inline int
 batadv_genl_register_family_with_ops_grps(struct genl_family *family,
                                          struct genl_ops *ops, size_t n_ops,
-                                         struct genl_multicast_group *mcgrps,
+                                         const struct genl_multicast_group *mcgrps,
                                          size_t n_mcgrps)
 {
        family->ops = ops;
        family->n_ops = n_ops;
-       family->mcgrps = mcgrps;
+       family->mcgrps = (struct genl_multicast_group *)mcgrps;
        family->n_mcgrps = n_mcgrps;
        family->module = THIS_MODULE;
-----

Or does this lead to some dangerous behaviour on the compiler side?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [B.A.T.M.A.N.] [PATCH next v2 3/3] batman-adv: make netlink attributes const
  2016-10-09  2:07   ` Linus Lüssing
@ 2016-10-09  6:25     ` Sven Eckelmann
  0 siblings, 0 replies; 6+ messages in thread
From: Sven Eckelmann @ 2016-10-09  6:25 UTC (permalink / raw)
  To: b.a.t.m.a.n

[-- Attachment #1: Type: text/plain, Size: 927 bytes --]

On Sonntag, 9. Oktober 2016 04:07:10 CEST Linus Lüssing wrote:
> On Thu, Sep 15, 2016 at 03:52:49PM +0200, Sven Eckelmann wrote:
> > +sed -i \
> > +	-e 's/^static const struct genl_multicast_group batadv_netlink_mcgrps/static struct genl_multicast_group batadv_netlink_mcgrps/' \
> > +	-e 's/^static const struct nla_policy batadv_netlink_policy/static const struct nla_policy batadv_netlink_policy/' \
> 
> For the second thing: Replacing the string with itself?

Damn, you are right. This line can be removed anyway. So it is not
"a big problem" but ugly as hell :)

> Just wondering... I simply tried casting the const away and that
> seems to compile without a warning:
[...]
> Or does this lead to some dangerous behaviour on the compiler side?
> 
Yes, this will crash because it is const aka read-only and the compat
code cannot write to the struct (actually, the read-only page).

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-10-09  6:25 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-15 13:52 [B.A.T.M.A.N.] [PATCH next v2 1/3] batman-adv: Introduce compat-patches support Sven Eckelmann
2016-09-15 13:52 ` [B.A.T.M.A.N.] [PATCH next v2 2/3] batman-adv: Use simple script to patch minor compat problems Sven Eckelmann
2016-09-16 15:34   ` Sven Eckelmann
2016-09-15 13:52 ` [B.A.T.M.A.N.] [PATCH next v2 3/3] batman-adv: make netlink attributes const Sven Eckelmann
2016-10-09  2:07   ` Linus Lüssing
2016-10-09  6:25     ` Sven Eckelmann

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).