oonf-olsrd2: reinstate and update to OONF master#1176
Conversation
parasew
force-pushed
the
reinstate-oonf-olsrd2
branch
from
c17f401 to
d308f19
Compare
|
Given that the package was removed nearly a year ago and we want to add it back, it basically needs a complete rebirth at this point. We should approach it and review it from scratch to ensure it’s clean and makes sense. If we don’t do it now, it’ll never happen. I was hoping to convince Copilot to take a look at it, but apparently, it’s a no. :) We’ll see, maybe it’ll get moving. As a next step, I suggest we set up a quick review of the original requirements so we don't bring back any old technical debt. |
| define Package/oonf-init-scripts | ||
| SECTION:=net | ||
| CATEGORY:=Network | ||
| MAINTAINER:=Henning Rogge <hrogge@gmail.com> |
There was a problem hiding this comment.
Does anyone know if this maintainer is still active?
There was a problem hiding this comment.
I can't confirm whether Henning is still active on the packaging side. To avoid these going unmaintained again, I'm willing to take over maintenance of oonf-olsrd2 and oonf-init-scripts and have set PKG_MAINTAINER accordingly. I've also emailed Henning directly as a courtesy in case he'd like to stay on. I have several OpenWrt devices and will run olsrd2 on real hardware, not just the cross-compile and QEMU validation I did for this PR.
There was a problem hiding this comment.
As far as I remember he does not maintain the package anymore. But maybe he responds.
| $(INSTALL_BIN) -D $(PKG_BUILD_DIR)/olsrd2_static $(1)/usr/sbin/olsrd2 | ||
| $(INSTALL_BIN) -D ./files/olsrd2.init $(1)/etc/init.d/olsrd2 | ||
| $(INSTALL_BIN) -D ./files/olsrd2.hotplug $(1)/etc/hotplug.d/iface/50-olsrd2 | ||
| $(INSTALL_DATA) -D ./files/olsrd2.uci $(1)/etc/config/olsrd2 |
There was a problem hiding this comment.
Could you clarify the purpose of the -d parameter here?
There was a problem hiding this comment.
-D (uppercase) is the coreutils install create-leading-directories flag. I've switched to the idiomatic form: an explicit $(INSTALL_DIR) for the target dirs followed by plain $(INSTALL_BIN)/$(INSTALL_DATA)/$(INSTALL_CONF) without -D.
| define Build/Install | ||
| $(INSTALL_BIN) -D $(PKG_BUILD_DIR)/$(MAKE_PATH)/olsrd2_static $(PKG_INSTALL_DIR)/usr/sbin/olsrd2; | ||
| endef |
There was a problem hiding this comment.
I tried removing this (along with CMAKE_INSTALL), but it breaks the build: without it, os_vif_linux.c (pulled in by the lan_import plugin) fails to compile with a "redefinition of struct ethhdr" error under the musl toolchain. So the cmake install handling is load-bearing here, not dead code, and I've kept it. I haven't traced the exact mechanism by which CMAKE_INSTALL affects that compile, but it's empirically required for a clean build across the CI architectures.
| # PKG_VERSION is a synthetic <date>~<short-commit> string built from | ||
| # PKG_SOURCE_DATE/PKG_SOURCE_VERSION. olsrd2 reports the OONF library | ||
| # version via -v/--version (see src/libcore/oonf_main.c), which does not | ||
| # contain that synthetic string, so the generic version check cannot | ||
| # match. Skip it explicitly. |
There was a problem hiding this comment.
Im not entirely comfortable with this comment. Why does our versioning diverge from upstream, and how should we verify it? The logic here basically means: we have generic version testing, upstream provides one version, downstream has a different one, and because they don't match, we just completely bypass the generic CI/CD checks. That doesn't seem like the right approach, does it?
There was a problem hiding this comment.
You're right that bypassing the check was wrong. The version diverges only because there's no upstream release tag to track, so this follows OpenWrt's standard ~ snapshot convention for the pinned master commit. But the build injects that commit via -D OONF_LIB_GIT, and olsrd2 reports it in its -v output. So I replaced the skip with a real check that verifies the binary reports the pinned commit. Validated in QEMU: olsrd2 -v prints "Git commit: b2164126e12340...", which the check matches.
| CMAKE_OPTIONS+=-D CMAKE_BUILD_TYPE:String=$(BUILD_TYPE) \ | ||
| -D OONF_NO_WERROR:Bool=true \ | ||
| -D OONF_LOGGING_LEVEL:String=debug \ | ||
| -D OONF_NO_TESTING:Bool=true \ | ||
| -D UCI:Bool=true \ | ||
| -D OONF_APP_DEFAULT_CFG_HANDLER:String=uci \ | ||
| -D OONF_STATIC_PLUGINS:String="class;callback;clock;duplicate_set;layer2;packet_socket;rfc5444;socket;stream_socket;telnet;timer;viewer;os_clock;os_fd;os_interface;os_routing;os_system;nhdp;olsrv2;ff_dat_metric;neighbor_probing;nl80211_listener;link_config;layer2info;systeminfo;cfg_uciloader;cfg_compact;nhdpinfo;olsrv2info;netjsoninfo;${CMAKE_OPTIONAL_PLUGINS}" \ | ||
| -D OONF_LIB_GIT:String=$(PKG_SOURCE_VERSION) \ | ||
| -D VERSION_SUB_TAG:String=$(PKG_SOURCE_DATE) \ | ||
| -D INSTALL_LIB_DIR:Path=lib/oonf \ | ||
| -D INSTALL_INCLUDE_DIR:Path=include/oonf \ | ||
| -D INSTALL_CMAKE_DIR:Path=lib/oonf \ | ||
| -D CMAKE_PREFIX_PATH=$(STAGING_DIR)/usr \ | ||
| -D CMAKE_GENERATOR=Ninja |
There was a problem hiding this comment.
cmake accepts both -DVAR=value and -D VAR=value; the space form is documented and used elsewhere in the feed. The package builds cleanly across all CI architectures as-is, so this isn't an error. Left the spacing unchanged.
| case "$( uci -q get "${DAEMON}.@[${i}].ignore" )" in | ||
| 1|on|true|enabled|yes) | ||
| oonf_log "removing/ignore section '$section'" | ||
| uci -q -c /var/run delete "${DAEMON}_dev.@[${j}]" | ||
| i=$(( i + 1 )) | ||
|
|
||
| continue | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
Good catch, real bug. ${j} was never defined, and as you noted deleting mid-loop shifts the anonymous section indexes. Note Copilot's suggested fix (dropping the delete) would silently disable the ignore feature, so I didn't take it. Instead I collect the ignored indexes during the loop and delete them afterwards in descending order, which removes the section without disturbing the others. Validated in QEMU with a three-section config where the middle section is ignored: that section is removed from /var/run/olsrd2_dev and the surrounding sections stay intact and correctly named.
| uci export ${DAEMON} >"/var/run/${DAEMON}_dev" | ||
|
|
||
| while section="$( uci -q -c /etc/config get "${DAEMON}.@[${i}]" )"; do { | ||
| echo "section: ${section}" |
| for single_interface in ${interface}; do { | ||
| device_name="$( oonf_get_layer3_device "${single_interface}" )" | ||
|
|
||
| echo "Interface: ${single_interface} = ${device_name}" |
| running() | ||
| { | ||
| # check if we have a pidfile and then check if that pid still exists. | ||
| # since we don't use -e this has to be explicitly returned. exit would stop the process. | ||
| test -e "/tmp/run/olsrd2.pid" && test -e "/proc/$(cat "/tmp/run/olsrd2.pid")" && return 0 | ||
| return 1 | ||
| } |
There was a problem hiding this comment.
Fixed. running() now uses /var/run/${DAEMON}.pid instead of the hardcoded olsrd2 path, consistent with oonf_reread_config elsewhere in the script.
|
Agreed, let's treat it as a fresh package rather than a revert. I'll go through both Makefiles against current conventions and push a cleaned version. For the requirements review: the default plugin set is driven by Config.in (lan_import, route_modifier, mpr, etc. as compile-time options folded into the single static binary), oonf-init-scripts ships two sourced shell helpers to /lib/functions/, and oonf-olsrd2 carries the procd init + hotplug + a default UCI config. Happy to align on what the default-enabled plugin set should be before I resubmit, so we don't bring back anything stale. |
Reverts the olsrd2 portion of the oonf-* removal in 58070aa. The two issues that motivated removal are fixed in current OONF master: - GCC 14 -Wint-conversion in os_system_linux.c: master uses designated initializers for struct msghdr. The fix existed on develop and landed on master in the 2025-12 develop->master merge. - CMake 4.x: root CMakeLists.txt now sets cmake_minimum_required 3.10. Bumps the source pin from the stale 2022-08-25 commit to master b2164126 (2025-12-04), bumps PKG_RELEASE, and drops 100-enable-lan-import-plugin.patch, which upstream has absorbed (lan_import is now in src/olsrv2/CMakeLists.txt). Verified: clean build and hash verification with the OpenWrt x86-64 gcc-14.3.0_musl SDK. Signed-off-by: Matthias Tarasiewicz <mt@riat.at>
parasew
force-pushed
the
reinstate-oonf-olsrd2
branch
from
d308f19 to
e2f7443
Compare
|
Pushed a cleaned-up version treating this as a fresh package per your suggestion, rather than a revert. Summary of changes: dropped the PKG_BUILD_DIR override, kernel.mk include, redundant VERSION lines, and the oonf-git/template indirection; added SPDX headers, PKG_MAINTAINER, PKG_LICENSE; switched URLs to https; fixed install macros (INSTALL_DIR + INSTALL_CONF for the config file); reset PKG_RELEASE to 1; added dependencies. Replaced the test-version.sh skip with a real check. Fixed several pre-existing bugs in oonf_init.sh that Copilot caught. I built and validated everything in QEMU on the x86-64 snapshot target: the package installs, jshn resolves as a dependency, olsrd2 reports the pinned commit, and the ignore-section logic works correctly. Details in the per-line replies. |
Summary
Reinstates
oonf-olsrd2(and itsoonf-init-scriptsdependency), which were removed in #1145. The two issues that motivated the removal are both fixed in current OONF master, and I've verified a clean build.Why this is safe now
The removal in #1145 cited three problems. Status of each:
-Wint-conversion(radio and proxy can not be compiled with GCC 14 (error: initialization of 'int' from 'void *' makes integer from pointer without a cast [-Wint-conversion]) OLSR/OONF#71): fixed upstream. Thestruct msghdrinitializers insrc/base/os_linux/os_system_linux.cnow use designated initializers, which skip musl's padding fields. The fix existed on thedevelopbranch and only reachedmasterin the 2025-12develop→masterreconciliation, which is why the old 2022 pin never had it.CMakeLists.txtnow setscmake_minimum_required(VERSION 3.10 FATAL_ERROR), above the 3.5 floor CMake 4.x enforces.masterbranch (notice dated 2025-12-04).What this PR changes
2022-08-25commit to masterb2164126e12340f19ea33070e1e11eb469a051e5(2025-12-04), with a refreshedPKG_MIRROR_HASH.PKG_RELEASE.100-enable-lan-import-plugin.patch. Upstream has absorbed it:add_subdirectory(lan_import)is already present insrc/olsrv2/CMakeLists.txt, so the patch no longer applies and is redundant.No source code changes and no plugin-list changes were needed.
Verification
Clean build and
PKG_MIRROR_HASHverification with the OpenWrtx86-64gcc-14.3.0_muslSDK (snapshots). x86-64 is 64-bit musl, the same ABI condition that triggered the original-Wint-conversionfailure, so the build exercises the relevant code path. No-Wint-conversionand no CMake minimum-version error.Scope
This PR covers
oonf-olsrd2only. The DLEP packages (oonf-dlep-radio,oonf-dlep-proxy) were the actual subjects of OLSR/OONF#71 and remain removed; they can follow in a separate PR once each is build-tested individually.@XDjackieXD noted intent to re-add the packages in #1145.