-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use own namespace #13
base: master
Are you sure you want to change the base?
Conversation
The build fails here against /usr/src/linux-headers-4.18.0-2-amd64 with
This really seems to be the case:
I've worked around it via
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are some things that should be fixed if we decide to merge this.
Makefile
Outdated
@@ -20,13 +20,13 @@ | |||
|
|||
# read README.external for more information about the configuration | |||
# B.A.T.M.A.N. debugging: | |||
export CONFIG_BATMAN_ADV_DEBUG=n | |||
export CONFIG_BATMAN_ADV_LEGACY_DEBUG=n |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this is always built out-of-tree, I don't see a point in renaming these config symbols.
batman-adv-y += soft-interface.o | ||
batman-adv-y += sysfs.o | ||
batman-adv-y += translation-table.o | ||
batman-adv-y += unicast.o |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not touch this file and generate a batman-adv.ko from this build step.
Makefile
Outdated
$(OBJDUMP) -t batman-adv-legacy.ko | grep batadv_ | grep -v "batadv_legacy" | \ | ||
sed "s/.* \([^ ]*\)batadv_\([^ ]*\)$$/\1batadv_\2 \1batadv_legacy_\2/" \ | ||
>> $(PWD)/updated-syms.txt; \ | ||
$(OBJCOPY) --redefine-syms=$(PWD)/updated-syms.txt $(PWD)/batman-adv-legacy.ko; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the same filename for input and output of the objcopy is making this more fragile than necessary. Just use batman-adv.ko as input, batman-adv-legacy.ko as output, and drop the whole if conditional.
We can use proper file targets then, which will also improve error handling:
- rename build to batman-adv.ko
- rename updatesyms to batman-adv-legacy.ko
- create a proper target to updated-syms.txt
- properly define dependencies between the different targets
main.h
Outdated
@@ -22,8 +22,8 @@ | |||
|
|||
#define BATADV_DRIVER_AUTHOR "Marek Lindner <[email protected]>, " \ | |||
"Simon Wunderlich <[email protected]>" | |||
#define BATADV_DRIVER_DESC "B.A.T.M.A.N. advanced" | |||
#define BATADV_DRIVER_DEVICE "batman-adv" | |||
#define BATADV_DRIVER_DESC "B.A.T.M.A.N. advanced legacy (compat14)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a space between compat and 14?
I'm not really a fan of these changes. I don't see why it should be necessary to load both modules at the same time to allow a scheduled domain switch - shouldn't it be enough to be able to install both modules into the same image, and decide at runtime which one to load? That way we would need to adjust way less code to the "-legacy" namespace (batctl calls, netlink queries, ...). |
Good point. You are right, the scheduled domain switcher will trigger a reboot anyway. And not making both loadable at the same time would indeed make this pull-request way simpler. The one advantage to be able to load both modules simultaneously I can think of right now would to ease migration on the gateway side. Summing up the list in the wiki there are currently 39 site.conf/mk's of which 13 are still using batman-adv-legacy/compat-14 (counting Freifunk Rheinland as one). With being able to load modules simultaneously it would probably just be a "git clone, run these dkms commands to build and install, update a few config lines". Compared to a setting up and integrating a new, additional system/OS somehow. Depending on how active these 13 communities still are and how quick they'd be in setting up a new VM or root server we would need to maintain the batman-adv-legacy fork for a longer or shorter amount of time. (anyone from these 13 communities having some opinion or strong preference?) |
Signed-off-by: Linus Lüssing <[email protected]>
sed -i "s/^batman-adv-/batman-adv-legacy-/g" Makefile* sed -i "s/ batman-adv.o/ batman-adv-legacy.o/g" Makefile.kbuild Signed-off-by: Linus Lüssing <[email protected]>
36e25e1
to
1068f3f
Compare
Signed-off-by: Linus Lüssing <[email protected]>
Signed-off-by: Linus Lüssing <[email protected]>
Signed-off-by: Linus Lüssing <[email protected]>
This creates a new batman-adv-legacy.ko which has its symbols updated accordingly. Signed-off-by: Linus Lüssing <[email protected]>
sed -i "s/^#define BATADV_NL_NAME \"batadv\"/#define BATADV_NL_NAME \"batadv_legacy\"/" uapi/linux/batman_adv.h Signed-off-by: Linus Lüssing <[email protected]>
1068f3f
to
f18bcec
Compare
Changelog v2:
Edit:
|
This patchset allows loading and running the mainline batman-adv (compat 15) and the batman-adv-legacy (compat 14) fork simultaneously.
Together with the gluon-scheduled-domain-switch package (freifunk-gluon/gluon#1555) this should ease the migration from compat 14 to compat 15 and speed-up the deprecation of this fork.
(Changing the module name was first proposed in #3 three years ago.)