Re: [PATCH 6/7] lldpd: make management address advertisement controllable

[Jo-Philipp Wich]

Hi,

comment below.

Am 4/2/24 um 15:02 schrieb Paul Donald:

Defaults to off.

Available from >= 0.7.15

These are sent in TLV

Signed-off-by: Paul Donald 
---
  package/network/services/lldpd/files/lldpd.init | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/package/network/services/lldpd/files/lldpd.init 
b/package/network/services/lldpd/files/lldpd.init
index 3e804cd033..72baf19cc5 100644
--- a/package/network/services/lldpd/files/lldpd.init
+++ b/package/network/services/lldpd/files/lldpd.init
@@ -128,6 +128,10 @@ write_lldpd_conf()
local lldp_capadv
config_get_bool lldp_capadv 'config' 'lldp_capadv' 0
  
+	# Broadcast management address in lldpd >= 0.7.15

+   local lldp_maddradv
+   config_get_bool lldp_maddradv 'config' 'lldp_maddradv' 0


Same remark as for the previous patch, please spell out those option names. 
Nobody will be able to guess what a "maddradv" is. It is not a widely used 
acronym and it is not similar to the native 
"management-addresses-advertisements" setting name.


Also there's an existing option "lldp_mgmt_ip" and I guess this new option 
controls whether it is advertised in TLVs or not, so it should follow its 
naming, something like "lldp_mgmt_ip_advertisement" or similar.


~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 6/7] lldpd: make management address advertisement controllable

[Jo-Philipp Wich]

Hi,

comment below.

Am 4/2/24 um 15:02 schrieb Paul Donald:

Defaults to off.

Available from >= 0.7.15

These are sent in TLV

Signed-off-by: Paul Donald 
---
  package/network/services/lldpd/files/lldpd.init | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/package/network/services/lldpd/files/lldpd.init 
b/package/network/services/lldpd/files/lldpd.init
index 3e804cd033..72baf19cc5 100644
--- a/package/network/services/lldpd/files/lldpd.init
+++ b/package/network/services/lldpd/files/lldpd.init
@@ -128,6 +128,10 @@ write_lldpd_conf()
local lldp_capadv
config_get_bool lldp_capadv 'config' 'lldp_capadv' 0
  
+	# Broadcast management address in lldpd >= 0.7.15

+   local lldp_maddradv
+   config_get_bool lldp_maddradv 'config' 'lldp_maddradv' 0


Same remark as for the previous patch, please spell out those option names. 
Nobody will be able to guess what a "maddradv" is. It is not a widely used 
acronym and it is not similar to the native 
"management-addresses-advertisements" setting name.


Also there's an existing option "lldp_mgmt_ip" and I guess this new option 
controls whether it is advertised in TLVs or not, so it should follow its 
naming, something like "lldp_mgmt_ip_advertisement" or similar.


~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 5/7] lldpd: make capabilities advertisement controllable

[Jo-Philipp Wich]

Hi,

comment below.

Am 4/2/24 um 15:02 schrieb Paul Donald:

Defaults to off.

Only available from >= 1.0.15

These capabilities are sent in TLV.

Signed-off-by: Paul Donald 
---
  package/network/services/lldpd/files/lldpd.init | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/package/network/services/lldpd/files/lldpd.init 
b/package/network/services/lldpd/files/lldpd.init
index 284c37c2ef..3e804cd033 100644
--- a/package/network/services/lldpd/files/lldpd.init
+++ b/package/network/services/lldpd/files/lldpd.init
@@ -124,6 +124,10 @@ write_lldpd_conf()
local lldp_syscapabilities
config_get lldp_syscapabilities 'config' 'lldp_syscapabilities'
  
+	# Configurable capabilities in lldpd >= v1.0.15

+   local lldp_capadv
+   config_get_bool lldp_capadv 'config' 'lldp_capadv' 0


Can we at least try to aim for some consistency here? Most preexisting options 
are spelled out, even the other capability related option right next does not 
abbreviate "capabilities", so this new option should likely be called 
"lldp_advcapabilities". Also "adv" is ambiguous, could be interpreted as 
"advanced" instead of "advertise" without further context, so consider naming 
it "lldp_capability_advertisement" or similar.


~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 4/7] lldpd: note about capabilities

[Jo-Philipp Wich]

Hi,

What's the purpose of this isolated comment? Either we're shipping version >= 
v1.0.15 so this code is guaranteed to work or we don't, in which case the code 
should not be there. We're not putting such comments next to all other option 
handling code either.


~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 3/7] lldpd: fix restart

[Jo-Philipp Wich]

Hi,


redirection broke in 5364fe0f01ca11b47c55d78f756d3176748dd0c2


why did this slip through in the first place? Seems that neither the submitter 
(you) nor the comitter actually runtime tested those changes described as " No 
functionality/behaviour changes; code is synonymous".


~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 1/7] lldpd: fixed interface(s) parsing

[Jo-Philipp Wich]

Hi,


For interface type parameters, the man page documents patterns:
```
*,!eth*,!!eth1

uses all interfaces, except interfaces starting with "eth",
but including "eth1".
```


at some point, uci configuration was meant to provide a somewhat sane config 
abstraction over various damon specific native configurations, now I see the 
recurring trend to expose every last native config idiosyncrasy as-is in uci.


Is there really a need to support these weird micro formats in uci? The uci 
config for lldpd should deal with logical interface names and translate them 
into layer 2 ones as needed. People requiring complex, hand-tuned settings 
probably want to bypass uci entirly and simply start lldpd with a static 
native config file.


~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: here we are again: real name 'discussion'

[Jo-Philipp Wich]

Hi,

[resent to list]


a) It's a policy change and not a code change. Policy changes require a
vote


Then take a(nother) vote.

https://lists.openwrt.org/pipermail/openwrt-devel/2024-January/042063.html



b) Just because the kernel changed their interpretation of DCO
requirements doesn't mean this automatically applies to OpenWrt
contribution policy.


https://openwrt.org/submitting-patches


Good point, as far as I remember, the page was meant to refer to the version
of the linked document at the time of writing, not whatever version happens to
be the latest over at kernel.org. This should probably get corrected to link
to a version before that change.


Do not conflate vague with abstract. The thing we care about here is an
email address. Can anyone know it? Yes. Can everyone know it? Yes. Can two
people have an identical email address? No. ( This is distinct from two
people *using* one email address ).


This interpretation does not reflect my view at least. I personally consider
the email address to be a temporary artifact. Longevity or a given mail
address is not always given (throw-away accounts, mails at selfhosted domains
eventually disappearing, mails bearing no relation to the name of a person).

Apart from that, I want proper looking, capitalized names (which also may be
aliases resembling the syntax of legal birth names) in the git metadata. No
nicknames, no repetition of mail addresses as name, no urls or handles tied to
specific platforms.

Lavabit shut down over the FBIs pursuit of a single email address (namely 
Snowden). If an email address is good enough for the FBI, it's good enough

for DCO.

" A real name does not require a legal name, nor a birth name, nor any name
that appears on an official ID (e.g. a passport). "


Exactly, but in my view there is a difference between calling oneself "John R.
Doe" or "mangawarriorz99". I prefer the former and would vote against the
latter. I see no place for "childish" or unprofessionally looking handles in
the project.


If somebody contributes with his GitHub handle, does that already count
as known?


When they're backed by en email address, yes.


Not in my view.



Regards,
Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH V3 3/3] base-files: sysupgrade: add uci-defaults script disabling services #2

[Jo-Philipp Wich]

Hi,


[...]
Extend sysupgrade to check for disabled services, generate uci-defaults
script disabling them and include it in backup.

Cc: Christian Marangi 
Cc: Jo-Philipp Wich 
Cc: Jonas Gorski 
Signed-off-by: Rafał Miłecki 


Acked-by: Jo-Philipp Wich 

~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH V3 2/3] base-files: sysupgrade: use tar helper to include installed_packages.txt

[Jo-Philipp Wich]

Hi,


[...]
Replace mount + overlay with manually built tar archive that gets
prepended to the actual config files backup. This allows more
flexibility with including extra backup files. They can be included at
any paths and don't require writing to flash or mounting an overlay
which has its own limitations (mount points).

Signed-off-by: Rafał Miłecki 


Acked-by: Jo-Philipp Wich 

~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH V3 1/3] base-files: sysupgrade: add tar.sh with helpers for building archives

[Jo-Philipp Wich]

Hi,

[...] 
This allows building uncompressed tar archives from shell scripts (and

compressing them later if needed)

Signed-off-by: Rafał Miłecki 


Signed-off-by: Jo-Philipp Wich 


~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH V2 1/3] base-files: sysupgrade: add tar.sh with helpers for building archives

[Jo-Philipp Wich]

Hi Rafał,

comments inline. Sorry for the bikeshedding ahead.

~ Jo


[...]
+
+__tar_print_padding() {
+   dd if=/dev/zero bs=$1 count=1 2>/dev/null


$1 may be 0 which is an invalid value for `bs=`:

  root@OpenWrt:~# dd bs=0
  dd: number 0 is not in 1..2147483647 range

A value of "0" is valid for `count=` though:

  root@OpenWrt:~# dd bs=1 count=0
  0+0 records in
  0+0 records out

So either revert to the previous version or hardcode `bs` to 1, pass the
desired size via `count=` and live with slightly less efficiency (which likely
does not matter at all).


+}
+
+__tar_make_member() {
+   local name="$1"
+   local content="$2"


If you make this `local mtime=${3:-$(date +%s)}` you can rename this function 
to `tar_make_member_inline()` and drop the extra wrapper.



+   local mtime="$3"
+   local mode=644
[...]
+}
+
+tar_make_member_from_file() {


This `tar_make_member_from_file()` wrapper is unused and very trivial, the
only thing it adds is defaulting the mtime value to the mtime of the given
file path. I suggest to drop `tar_make_member_from_file()` entirely and to
rename  `tar_make_member_inline()` below to simply `tar_make_member()`.

Maybe also consider renaming it to `tar_print_member()` or
`tar_output_member()` since that is what it does.


+   local name="$1"
+
+   __tar_make_member "$name" "$(cat $name)" "$(date +%s -r "$1")"
+}
+
+tar_make_member_inline() {
+   local name="$1"
+   local content="$2"
+   local mtime="${3:-$(date +%s)}"
+
+   __tar_make_member "$name" "$content" "$mtime"
+}
+


Analogous to the suggested name change above, the `tar_close()` function 
should probably be renamed to `tar_print_trailer()` or `tar_output_trailer()`

as well.


+tar_close() {
+   __tar_print_padding 1024
+}


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 1/3] base-files: sysupgrade: add tar.sh with helpers for building archives

[Jo-Philipp Wich]

Hi Rafał,

thanks for taking are of this. Please find some comments below.

Am 2/26/24 um 15:14 schrieb Rafał Miłecki:

From: Jo-Philipp Wich 

This allows building uncompressed tar archives from shell scripts (and
compressing them later if needed)

Signed-off-by: Rafał Miłecki 
---
  package/base-files/files/lib/upgrade/tar.sh | 84 +
  1 file changed, 84 insertions(+)
  create mode 100644 package/base-files/files/lib/upgrade/tar.sh

diff --git a/package/base-files/files/lib/upgrade/tar.sh 
b/package/base-files/files/lib/upgrade/tar.sh
new file mode 100644
index 00..00057dd760
--- /dev/null
+++ b/package/base-files/files/lib/upgrade/tar.sh
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+
+__tar_print_padding() {
+   [ $1 -eq 0 ] || dd if=/dev/zero bs=$1 count=1 2>/dev/null
+}
+
+__tar_make_member() {
+   local name="$1"
+   local content="$2"
+   local username="$3"
+   local groupname="$4"
+   local mtime="$5"
+   local mode=644


I think the uid and gid values should correspond to the given username 
and groupname values. Something like this would probably work:


local uid=$(id -u "$username")
local gid=$(sed -rne "s#^$groupname:[^:]*:([0-9]+):.*\$#\1#p" /etc/group)


+   local uid=0
+   local gid=0
+   local size=${#content}
+   local type=0
+   local link=""
+
+   # 100 byte of padding bytes, using 0x01 since the shell does not tolate 
null bytes in strings
+   local 
pad=$'\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1\1'
+
+   # validate name
+   if [ "${name:0:1}" = "/" ]; then
+   name="${name:1}"
+   fi
+
+   # truncate string header values to their maximum length
+   name=${name:0:100}
+   link=${link:0:100}
+   username=${username:0:32}
+   groupname=${groupname:0:32}
+
+   # construct header part before checksum field
+   local header1="${name}${pad:0:$((100 - ${#name}))}"
+   header1="${header1}$(printf '%07d\1' $mode)"
+   header1="${header1}$(printf '%07o\1' $uid)"
+   header1="${header1}$(printf '%07o\1' $gid)"
+   header1="${header1}$(printf '%011o\1' $size)"
+   header1="${header1}$(printf '%011o\1' $mtime)"
+
+   # construct header part after checksum field
+   local header2="$(printf '%d' $type)"
+   header2="${header2}${link}${pad:0:$((100 - ${#link}))}"
+   header2="${header2}ustar  ${pad:0:1}"
+   header2="${header2}${username}${pad:0:$((32 - ${#username}))}"
+   header2="${header2}${groupname}${pad:0:$((32 - ${#groupname}))}"
+
+   # calculate checksum over header fields
+   local checksum=0
+   for byte in $(printf '%s%8s%s' "$header1" "" "$header2" | tr '\1' '\0' | hexdump 
-ve '1/1 "%u "'); do
+   checksum=$((checksum + byte))
+   done
+
+   # print member header, padded to 512 byte
+   printf '%s%06o\0 %s' "$header1" $checksum "$header2" | tr '\1' '\0'
+   __tar_print_padding 183


I checked and noticed that `dd` accepts a `count` value of 0, so we can 
inline `__tar_print_padding()` (whose sole purpose was the != 0 check) 
and get rid of the extra function:


dd if=/dev/zero bs=183 count=1 2>/dev/null


+
+   # print content data, padded to multiple of 512 byte
+   printf "%s" "$content"
+   __tar_print_padding $((512 - (size % 512)))


Inline this `__tar_print_padding()` as (count may be zero):

dd if=/dev/zero bs=1 count=$((512 - (size % 512))) 2>/dev/null


+}
+
+tar_make_member_from_file() {
+   local name="$1"
+   local username="$(ls -l "$1" | tr -s ' ' | cut -d ' ' -f 3)"
+   local groupname="$(ls -l "$1" | tr -s ' ' | cut -d ' ' -f 4)"
+
+   __tar_make_member "$name" "$(cat $name)" "$username" "$groupname" "$(date +%s -r 
"$1")"
+}
+
+tar_make_member_inline() {
+   local name="$1"
+   local content="$2"
+   local username="${3:-root}"
+   local groupname="${4:-root}"
+   local mtime="${5:-$(date +%s)}"
+
+   __tar_make_member "$name" "$content" "$username" "$groupname" "$mtime"
+}
+
+tar_close() {
+   __tar_print_padding 1024


Inline this `__tar_print_padding()` as:

dd if=/dev/zero bs=1024 count=1 2>/dev/null


+}



~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] base-files: sysupgrade: include uci-defaults script disabling services

[Jo-Philipp Wich]

Hi Rafał,


Extend sysupgrade to check for disabled services, generate uci-defaults
script disabling them and include it in backup.

Cc: Christian Marangi 
Cc: Jo-Philipp Wich 
Cc: Jonas Gorski 
Signed-off-by: Rafał Miłecki 


Acked-by: Jo-Philipp Wich 


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: ustream-ssl ABI_VERSION usage

[Jo-Philipp Wich]

Hi Paul,


While working on using APK instead of OPKG, I started to look into ABI
versions of different packages and found that ustream-ssl always install
libustream-ssl.so , with no ABI version
attached.


$(INSTALL_DATA) $(PKG_INSTALL_DIR)/usr/lib/libustream-ssl.so $(1)/lib/


Doesn’t this render the idea of ABI version obsolete since they can’t be
installed side by side?


Yes it does, somewhat. It still enforces package consistency opkg-wise (can't
upgrade libustream-ssl without upgrading all packages depending on it) but as
you correctly pointed out, it will prevent side-by-side installations and thus
graceful upgrades.

Ideally all packages specifying an ABI version should ship versioned .so files
as well.

~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Are we still use md5 as default as password hash?

[Jo-Philipp Wich]

Hi,

you will also need to enable additional ciphers in musl libc (disabling the 
crypt size hack).


Please disregard that remark, it has already been pointed out that the hack is 
disabled by default since quite some time.




~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Are we still use md5 as default as password hash?

[Jo-Philipp Wich]

Hi,


Does the following do the trick?


you will also need to enable additional ciphers in musl libc (disabling the 
crypt size hack).


~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: firewall4: loopback device is ACCEPTED before include chain-prepend input

[Jo-Philipp Wich]

Hi,

> [...]
> Is there a reason why this decision was made, to add the custom include after
> the loopback interface?

Performance considerations, mostly. It violates pola though since it deviates
from the behavior of other chain includes, so I'm okay with moving the include
before the loopback check rule.

Can you whip up a patch for that? Don't forget to do the same for `output` as
well.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

[RFC netifd 1/2] interface-ip: mask out host bits in IPv4 route targets

[Jo-Philipp Wich]

The kernel will reject attempts to install routes with target addresses
having host bits set with an "Invalid prefix for given prefix length"
error.

A route configuration like the one below will silently fail to apply:

config route
option interface lan
option target 10.40.40.1/24

Attempting to do the same with iproute2 will fail as well:

# ip route add 10.40.40.1/24 dev br-lan
Error: Invalid prefix for given prefix length.

However, for IPv6 route targets with set host bits are allowed:

# ip -6 route add 3000::1/64 via fe80::1234:5678:9abcd:ef01 dev br-lan
# ip -6 route list 3000::1/64
3000::/64 via fe80::1234:5678:9abc:def1 dev br-lan metric 1024 pref medium

In order to stay consistent here, and to avoid unecessary configuration
pitfalls, make netifd more lenient and simply mask out excess host bits
while parsing IPv4 route configuration.

Signed-off-by: Jo-Philipp Wich 
---
 interface-ip.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/interface-ip.c b/interface-ip.c
index a06a514..fee29a9 100644
--- a/interface-ip.c
+++ b/interface-ip.c
@@ -441,6 +441,10 @@ interface_ip_add_route(struct interface *iface, struct 
blob_attr *attr, bool v6)
DPRINTF("Failed to parse route target: %s\n", (char *) 
blobmsg_data(cur));
goto error;
}
+
+   /* Mask out IPv4 host bits to avoid "Invalid prefix for given 
prefix length" */
+   if (af == AF_INET && route->mask < 32)
+   route->addr.in.s_addr &= ((1u << route->mask) - 1);
}
 
if ((cur = tb[ROUTE_GATEWAY]) != NULL) {
-- 
2.39.1


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

[RFC netifd 2/2] interface-ip: allow configuring routes without explicit interface

[Jo-Philipp Wich]

Support the configuration of network routes not bound to any specific
interface. In case such a route is configured, it will be internally
owned by the loopback interface and have a new DEVROUTE_NODEV flag
set to inhibit the RTA_OIF attribute when installing the kernel route.

Signed-off-by: Jo-Philipp Wich 
---
 interface-ip.c | 16 
 interface-ip.h |  3 +++
 system-linux.c |  3 +++
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/interface-ip.c b/interface-ip.c
index fee29a9..d2fe385 100644
--- a/interface-ip.c
+++ b/interface-ip.c
@@ -405,6 +405,7 @@ interface_ip_add_route(struct interface *iface, struct 
blob_attr *attr, bool v6)
struct blob_attr *tb[__ROUTE_MAX], *cur;
struct device_route *route;
int af = v6 ? AF_INET6 : AF_INET;
+   bool no_device = false;
 
blobmsg_parse(route_attr, __ROUTE_MAX, tb, blobmsg_data(attr), 
blobmsg_data_len(attr));
 
@@ -412,10 +413,13 @@ interface_ip_add_route(struct interface *iface, struct 
blob_attr *attr, bool v6)
return;
 
if (!iface) {
-   if ((cur = tb[ROUTE_INTERFACE]) == NULL)
-   return;
+   if ((cur = tb[ROUTE_INTERFACE]) == NULL) {
+   iface = vlist_find(, "loopback", iface, 
node);
+   no_device = true;
+   } else {
+   iface = vlist_find(, blobmsg_data(cur), 
iface, node);
+   }
 
-   iface = vlist_find(, blobmsg_data(cur), iface, node);
if (!iface)
return;
 
@@ -520,7 +524,11 @@ interface_ip_add_route(struct interface *iface, struct 
blob_attr *attr, bool v6)
route->flags |= DEVROUTE_PROTO;
}
 
-   interface_set_route_info(iface, route);
+   if (no_device)
+   route->flags |= DEVROUTE_NODEV;
+   else
+   interface_set_route_info(iface, route);
+
vlist_add(>route, >node, route);
return;
 
diff --git a/interface-ip.h b/interface-ip.h
index 8843349..cc7efbd 100644
--- a/interface-ip.h
+++ b/interface-ip.h
@@ -51,6 +51,9 @@ enum device_addr_flags {
 
/* neighbor mac address */
DEVNEIGH_MAC= (1 << 11),
+
+   /* route specifies no device */
+   DEVROUTE_NODEV  = (1 << 12),
 };
 
 union if_addr {
diff --git a/system-linux.c b/system-linux.c
index 0760e73..a8add1e 100644
--- a/system-linux.c
+++ b/system-linux.c
@@ -2789,6 +2789,9 @@ static int system_rt(struct device *dev, struct 
device_route *route, int cmd)
}
}
 
+   if (route->flags & DEVROUTE_NODEV)
+   dev = NULL;
+
msg = nlmsg_alloc_simple(cmd, flags);
if (!msg)
return -1;
-- 
2.39.1


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Intention on moving board-2 blob to a separate repo

[Jo-Philipp Wich]

Hi Ansuel,

this makes a lot of sense imho.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] ubus: added ubus_handle_events function that "guaranties" execution of all polled events

[Jo-Philipp Wich]

Hi,

> In case of previous setup or calling flow ctx->cancel_poll is set to true
> function ubus_handle_event may process ONLY ONE request, though the comment
> says it processes events:
> 
> /* call this for read events on ctx->sock.fd when not using uloop */ static
> inline void ubus_handle_event(struct ubus_context *ctx) { 
> ctx->sock.cb(>sock, ULOOP_READ); }
> 
> In case if I would manually poll the ubus fd and do not use uloop to poll
> it and after that it may process ONE event and the rest will be processed
> on the next loop cycle. I would like to have a function that guarantees
> that every request will be processed in a single call to
> ubus_haubus_handle_eventndle_event.

You're already using a foreign event loop / IO notification mechanism, you
already have means to determine socket read readiness. Invoking a library
function that does it's own polling internally with arbitrary, uncontrollable
timeouts does not seem like a good design.

It would be better to implement a function that simply keeps calling
`get_next_msg(ctx, _fd)` and `ubus_process_msg(ctx, >msgbuf,
recv_fd);` until `get_next_msg()` yields false.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] fw4: add a range of icmpv6 types

[Jo-Philipp Wich]

Hi,

the patch was white-space mangled and the Signed-off-by didn't match the
author. It also introduced syntax errors in fw4.uc so it seems it hasn't been
runtime tested at all.

Superseded by https://git.openwrt.org/e6e82a5 and 
https://git.openwrt.org/30ee17a.



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] fw4: fix ipset comment field from bool to string

[Jo-Philipp Wich]

Hi,

the patch was whitespace mangled and didn't apply. After fixing it up
manually, the Signed-off-by didn't match the author. Also the fixed option
wasn't use anywhere so the fix was rather incomplete (or not useful by itself).

It is superseded by https://git.openwrt.org/39e8c70 now.

Regards,
Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH v3 6/7] ucode: Update to latest

[Jo-Philipp Wich]

Hi,

> Can you change that?

Could you also please omit the merge commits from the listing in the commit
message?

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] iwinfo: devices: add Qualcomm Atheros IPQ8074 WiSoC

[Jo-Philipp Wich]

Hi Robert,

I know that you're just expanding existing code (which I recently noticed for
the first time) but I think that adding more and more if/else clauses with
further hardware matches for purely cosmetic reasons* is a good way forward.

At the very least a mechanism should be added to configure this
FDT-to-PCI-ID-to-name mapping in the devices.txt file directly.


*) Many hardware entries are simply added to show a fancy radio name instead
   of "Generic MAC80211" radio but don't add non-defaults values such as power
   offsets or antenna gains.


Regards,
Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Secure cookie handling upon https to http downgrade

[Jo-Philipp Wich]

Hi,

> More generally, and regard to the earlier suggestion, I would still suggest
> splitting the http vs https cookie names in any ongoing luci rework in order
> to avoid this situation.

this also has been implemented already, see
https://github.com/openwrt/luci/commit/08fb38399f5b297be7d460703b70d3b893139f9f

Regards,
Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Secure cookie handling upon https to http downgrade

[Jo-Philipp Wich]

Hi,

> [...]
> I renamed the new cookies to "http-sysauth" and "https-sysauth", to work
> around this and it seems to do the right thing.  But there is still a fault 
> here.

Already fixed with
https://github.com/jow-/lucihttp/commit/6e68a1065f3ed1889e5fa053b206bd3aa108bd5f

~ Jow



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: ui.waitReconnect() may load over HTTP instead of HTTPS

[Jo-Philipp Wich]

Hi,

ui.awaitReconnect() tries both http:// and https:// access simultaneously and
redirects to whatever URL loads successfully.

HTTPS access might be unavailable, e.g. when flashing an image without SSL
support built in. This used to be the norm before OpenWrt enabled HTTPS by
default in 22.03.

So this probing of an http:// URL from within a page that was loaded via
https:// is intentional.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: firewall4 question

[Jo-Philipp Wich]

Hi,

try adding `option dest lan` to your block rule. Without any destination, it
is treated as input rule, not a forwarding one.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [RFC] dropping of $(AUTORELEASE) feature

[Jo-Philipp Wich]

Hi,

> The AUTORELEASE has been a nice feature from the package PR maintenance
> perspective.
> 
> Earlier there was constant trouble with concurrent PRs for the same package
> having the same PKG_RELEASE bump, or the maintainer doing a small change with
> a bump while there was an open PR with the same bump. The led to trivial
> conflicts and rebase needs for PRs. AUTORELEASE has tackled that nicely.

it tackled it by sweeping the problem under the carpet and hiding the fact
that those concurrent PRs were indeed conflicting. If the version targeted by
a PR has been changed in the meanwhile it should not be made trivially
mergable by hiding the actual revision from the sources.

I agree that this increases maintenance burden but I believe that a CI side
solution for those conflicting-because-of-deviating-PKG_RELEASE situations
would be the better course of action.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [RFC] dropping of $(AUTORELEASE) feature

[Jo-Philipp Wich]

Hi,

yes, please kill it. The $(AUTORELEASE) option does not work for sources
without Git history, it produces different results depending on the history,
it causes package bumps for even trivial cosmetic fixes.

It can also lead to situations where packages on different branches end up
with the exact same version but different sets of commits applied to each 
branch.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] fw4: fix handling of unaccepted forward packets

[Jo-Philipp Wich]

Hi,

you misunderstand the purpose of the zone forward policy. It is not meant to
catch traffic from a zone to another zone, but traffic relayed from one
interface to another interface within the same zone.

Traffic from one zone to another zone is solely handled by the global forward
policy in the defaults section (or individual rules).

Your patch would change this long standing behavior, therefor NACK from me.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: lua 5.1.5 CVEs / lua 5.3 with luci

[Jo-Philipp Wich]

Hi,

> Can one be curious and ask what is gonna be used instead of lua, or is
> that still not 100% decided yet?

you can find more details at
https://forum.openwrt.org/t/luci-rewrite-in-ucode-testers-wanted/137250

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: lua 5.1.5 CVEs / lua 5.3 with luci

[Jo-Philipp Wich]

Hi,

all errors you quoted are occurring within Lua code. The view rendering etc.
mostly happens in JavaScript on the client side, this is why things /seem/ to
work. Many backend actions are implemented as rpcd plugins in Lua code though,
and all those seem to fail (not register with rpcd in the first place, likely
because the requested interpreter /usr/bin/lua is not there).

Newer Lua versions do have various incompatibilities with Lua 5.1 and the
deprecation of setfenv(), getfenv() in favor to _ENV will require a lot of
refactoring in LuCI framework code.

Since LuCI is in the process of migrating away from Lua, only keeping an
optional compatibility Lua runtime for legacy applications, it is unlikely
that any work will be spent to convert the framework code to later Lua versions.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] netifd: add accept_ra support

[Jo-Philipp Wich]

Hi,

> Make the "Accept Router Advertisements" configurable. This is needed if
> you do not want to use odhcp6c and let the kernel handle the RAs. This
> can save some diskspace.

NACK from me. As it will interfere with odhcp6c operation in the default setup
I don't think that it is a good idea to expose this in uci. Given that this is
a nonstandard setup anyway you can as well set this value in sysctl directly.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [musl] nslookup failures with coarse CLOCK_MONOTONIC

[Jo-Philipp Wich]

Hi,

> [...]
> Which implementation of nslookup is this? Busybox? It would probably
> be useful to hear thoughts on it from their side.
assuming the OP is using standard OpenWrt nslookup, it is the "big" busybox
nslookup implementation, which is using the res_*() api and name lookup logic
borrowed from musl libc instead of the original "small" version fiddling with
the `_res` state directly (and being broken on musl libc due to that).

The proper course of action here is likely adapting the solution in
6c858d6fd4df8b5498ef2cae66c8f3c3eff1587b and porting it to the busybox "big"
nslookup code itself.

I agree that musl libc itself cannot do much more to ensure uniqueness of the
IDs generated by res_mkquery() and that it should be solved in the application
code itself in this case.

Regards,
Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] Send bad forward_zone packets to verdict_from_zone

[Jo-Philipp Wich]

Hi,

the forward policy for zones is supposed to only apply to forwarded traffic
among interfaces of the same zone. If I read it correctly, your patch would
change this long standing behavior to something else.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Add SoB tag to hack patches on generic target

[Jo-Philipp Wich]

Hi,

to be pragamtic, set a `Submitted-by`, `Added-by`, `Introduced-by` or similar
tag. This should provide enough clue to locate the original author without the
need to "forge" Sob.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: DSA Terminology

[Jo-Philipp Wich]

Hi,

>> Well, it would still be less confusing than the state we're currently in.
>> Anyway, converting "config interface" to "config network" and "config
>> device" to "config iface" is an option.

I don't like `iface`, it is also easily confused with `interface` (which - as
explained - still has to be supported in the forseeable future even after the
conversion).

Apart from that we should try to rid of abbreviations in configuration or at
the very least follow a consistent set of rules (if there'd be a `config
iface` then networks should be called `option net`).


Regards,
Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: DSA Terminology

[Jo-Philipp Wich]

Hi,

> IMHO changing, in /etc/config/network:
> "config interface" -> "config network"
> "config device" -> "config interface"
> would eliminate this semantic inconsistency and bring the naming
> convention more in line with what Rich referred to in his comments
> above.

This cannot be done in a sane manner though as it would render future versions
entirely backwards incompatible.

Renaming `config interface` to `config network` makes sense and can be
implemented easily. However we would still need to treat `config interface` as
synonym for it in the forseeable future in order to retain compatibility,
which means that we cannot reuse `interface` for something else.

So changing `config interface` to `config network` would be possible assuming
that `config device` remains `config device` (or is renamed to something other
than `interface`).

At the same time, the `wifi-iface` section type in /e/c/network should be
changed to `wifi-network` in order to remain consistent.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: DSA Mini-tutorial

[Jo-Philipp Wich]

Hi,

> [...]
> Meanwhile I wonder if a few specific cases wouldn't need to be better
> addressed. For instance ppp or wwan create extra « Interfaces » (in current
> Luci lingo), some having the related protocol - PPPoE/NCM, some having
> protocol « Virtual Dynamic Interface » (for ipv4/ipv6) - which I personally
> find confusing, especially as the virtual ones only show up in Luci and not
> on commandline.

Logical networks do not show up on the command line either (in the sense of
netdevs you can query with ifconfig or ip). You can see them with "ifstatus"
or "ubus list network", but the same applies to "virtual dynamic interfaces".
You can see them in ubus, you can query them with "ifstatus", you should be
able to "ifdown" them - so they are not more or less visible than other
explicitly declared logical interfaces.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: DSA Mini-tutorial still marked as Work In Progress

[Jo-Philipp Wich]

Hi Rich,

that tutorial is good ground work imho. One thing I repeatedly noticed (not in
the document, but in forum and irc chatter) is that over the time, DSA and
bridge VLAN filtering became conflated into one concept while they're actually
different pieces; one can do bridge VLAN filtering without DSA and one can
utilize DSA without doing bridge VLAN filtering.

Bluntly speaking, DSA is the thing that gives you one Linux network device per
switch port and bridge VLAN filtering is the stuff that allows you declaring
swconfig-esque VLAN port groups on top of an arbitrary bridge interface.

I think this is something we should try to better convey in the documentation.

For example simple common use cases like:

 - Making each switch port it's own independent interface with own subnet

or

 - Break out one switch port to turn it into some kind of restricted IoT or
   guest network access port

or

 - Bridge each ethernet port to another SSID

don't require bridge VLAN filtering or touching VLANs in general at all (in
contrast to former swconfig). The per-port net devices just have to be taken
out of the br-lan bridge and either be put into another bridge or configured
as independent network devices.

Bridge VLAN filtering on the other hand is only actually needed if you want to
deal with VLAN tagged traffic inside the bridge. And even then there's
sometimes alternative ways, for example the following two scenarios should be
functionally equivalent:

- Bridge device "br-vlan10" containing "lan1.10 lan2.10 lan3.10"
  - VLAN filtering disabled

vs.

- Bridge device "br-lan" containing "lan1 lan2 lan3"
  - VLAN filtering enabled
  - Bridge VLAN #10 containing lan1 as tagged, lan2 as tagged, lan3 as tagged
- VLAN device br-lan.10 on top of br-lan


In the former case you would put your IP address settings onto the dedicated
"br-vlan10" bridge device while in the latter case you would configure the IP
addressing on the "br-lan.10" subinterface of the "br-lan" bridge.

So maybe it makes sense to focus on the "with DSA, your switch just becomes a
linux bridge over a bunch of netdevs" aspect in the mini tutorial and break
out any bridge-VLAN related information into a separate advanced VLAN tutorial.

Another conceptual issue I see is that people came to expect a dedicated
"switch" configuration ui which is something that does not really work with
DSA devices anymore since there is no dedicated switch hardware entity to
interact with anymore (DSA takes care of completely abstracting this away from
the user point of view) and that bridge-vlans just happen to be a
configuration detail of a bridge, and that there happens to be a bridge
"br-lan" by default, but a system could have multiple bridges, or none at all.

So we should also explain why there is no central "switch configuration"
anymore and that this does not translate into a loss of functionality, but
that the former semi opague swconfig switch configuration entity was dissolved
into a bunch of ethernet devices inside a bridge...



~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: DSA Mini-tutorial

[Jo-Philipp Wich]

Hi,

>> I wrote this mostly because the LuCI interface itself makes a distinction
>> between the "Devices" tab and the "Interfaces" tab. But maybe this isn't the
>> best way to describe what goes there.
> 
> I agree that there are inconsistencies in LuCI. The only place I see the
> terminology correctly stated is on the wireless section:
> 
> Network: Lists all entries on the Interfaces section.
> 
>> Choose the network(s) you want to attach to this wireless interface or >
> fill out the custom field to define a new network.
> 
> Which correctly states that:
> 
> Interfaces section should be called Networks.
> Devices section should be called Interfaces.

I agree that there's inconsistencies, I eventually want to clean that up.
Part of the terminology stems from uci itself, where `config interface`
declares logical *networks* while *interfaces* (as in Linux netdevs) are
referred to as `config device` (and `config wifi-*iface*` for wireless, which
probably should be named differently too).

Personally I am fine with rebranding the logical network entities (lan, wan,
loopback etc.) to "networks" but I am unsure about renaming what is currently
"devices" to "interfaces" as it would then intersect with the current meaning
of interfaces...

Anyhow, so while I agree that:

> Interfaces section should be called Networks.
> Devices section should be called Interfaces.

... it will directly contradict /etc/config/network, where networks are called
`config interface` and interfaces are called `config device` likely leading to
even more confusion.


~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH rpcd] sys: mitigate possible strncpy string truncation

[Jo-Philipp Wich]

Hi,

comment below.

~ Jo

On 8/24/22 10:14 AM, Petr Štetiar wrote:
> [...]
> --- a/sys.c
> +++ b/sys.c
> @@ -224,7 +224,8 @@ procstr:
>   continue;
>  
>   if (!strcmp(var, "Package:")) {
> - strncpy(pkg, p1, sizeof(pkg));
> + strncpy(pkg, p1, sizeof(pkg) - 1);
> + pkg[sizeof(pkg) - 1] = '\0';

I suggest to change declarations of `pkg` and `ver` to:

   char pkg[128] = { 0 }, ver[128] = { 0 };

This way you can omit this explicit '\0' assignment since the dest char array
will be zero initialized and the `sizeof(...) - 1` above will ensure that the
last '\0' is never overwritten.

>   continue;
>   }
>  
> @@ -241,7 +242,8 @@ procstr:
>   }
>  
>   if (!strcmp(var, "Version:")) {
> - strncpy(ver, p1, sizeof(ver));
> + strncpy(ver, p1, sizeof(ver) - 1);
> + ver[sizeof(ver) - 1] = '\0';
>   continue;
>   }
>  
> 
> ___
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
> 



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] build: always set CONFIG_IPV6

[Jo-Philipp Wich]

Hi,

> [...]
> This patch sets the related CONFIG to always true and removes the
> config prompt, keeping the change minimal, and, should !CONFIG_IPV6 ever
> be fixed, easy to revert.
> 
> Signed-off-by: Thibaut VARÈNE 
Acked-by: Jo-Philipp Wich 



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH firewall4] fw4: add support for include.d dir

[Jo-Philipp Wich]

Hi,

> [...]
> 
> Sorry, but I don't agree with the following reasons. Let me briefly explain
> why.
> 
> All files under '/usr/share/firewall4/includes.d' are uci files. I can see 
> all relevant options.

One problem with your suggested implementation is that you introduce a generic
directory (/usr/share/firewall4/includes.d) containing uci partials that do
look like a full /etc/config/firewall but actually ignore all section types
except `config include` ones, that will lead to confusion.

> I can set in the includes my own path. This is
> relevant for packages that updates the ruleset on events. If I do not want
> to be this rules persistent saved I could use a tmp file in the system
> (strongswan).

Using my proposal you could achieve the same by placing a symlink to a
temporary file path

> Also the new include add by you already does have the state file feature. 
> Which is relevant on reloading the ruleset.

Not sure what you mean with that.

> In addition, it would make the ruleset.uc file confusing if I added the
> same feature again.

The ruleset.uc file wouldn't change at all. The implementation would be
confined to fw4.uc, like your approach.

> Also, I would have to add two sections on include to support temporary
> rules, which I would not have to store under 
> '/usr/share/firewall4/includes.d/' but under '/tmp/firewall4/includes.d/'
> for example to support the not persistent feature.

ln -s /tmp/strongswan/nftables.d/pre-input.nft \
  /usr/share/nftables.d/chain-pre/input/10_strongswan.nft

ln -s /tmp/strongswan/nftables.d/pre-output.nft \
  /usr/share/nftables.d/chain-pre/output/10_strongswan.nft

ln -s /tmp/strongswan/nftables.d/pre-forward.nft \
  /usr/share/nftables.d/chain-pre/forward/10_strongswan.nft

> We also use the include to add the helpers.

Can you elaborate?

> Last but not leased, if we now add an other possibility, then I don't
> think anyone knows where which file adds which rule!

The same applies to /usr/share/firewall4/includes.d/ as well. You do need to
be aware of it to know that includes can stem from there.

> From my point of view, it makes sense to use the include function from you 
> with my extension. So I think the include feature is the better and
> unified solution.

What I do not like about it are several facts:

 - It introduces uci in a non-standard location
 - It implies configurability where there isn't any (it is package managed
   resource files, like init scripts. Technically you can edit init scripts
   but they're certainly not configuration but package integration code).
 - It introduces overhead; you do have to read and parse a uci file to extract
   a path from it which you then write into the ruleset so that nftables can
   include it
 - It creates the false impression of being a general-purpose uci partial
   solution to be able to modularize /etc/config/firewall which it isn't since
   it'll silently ignore any non-include section type being put there


I have attached my proposal as patch for reference.


Kind regards,
Jo
From 5ab0f61350f02590c5e6c1981bce4531510517de Mon Sep 17 00:00:00 2001
From: Jo-Philipp Wich 
Date: Thu, 11 Aug 2022 13:48:14 +0200
Subject: [PATCH] fw4: support automatic includes

Introduce a new directory tree /usr/share/nftables/includes.d/ which may
contain partial nftables files being included into the rendered ruleset.

The include position is derived from the file path;

 - Files in .../includes.d/table-pre/ and .../includes.d/table-post/ are
   included before and after the `table inet fw4 { ... }` declaration
   respectively

 - Files in .../includes.d/ruleset-pre/ and .../includes.d/ruleset-post/
   are included before the first chain and after the last chain
   declaration within the fw4 table respectively

 - Files in .../includes.d/chain-pre/${chain}/ and .../chain-post/${chain}/
   are included before the first and after the last rule within the mentioned
   chain of the fw4 table respectively

Automatic includes can be disabled by setting the `auto_includes` option to
`0` in the global defaults section.

Signed-off-by: Jo-Philipp Wich 
---
 root/usr/share/ucode/fw4.uc | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/root/usr/share/ucode/fw4.uc b/root/usr/share/ucode/fw4.uc
index 2dc44ac..86e3331 100644
--- a/root/usr/share/ucode/fw4.uc
+++ b/root/usr/share/ucode/fw4.uc
@@ -733,6 +733,19 @@ return {
 		this.cursor.foreach("firewall", "include", i => self.parse_include(i));
 
 
+		//
+		// Discover automatic includes
+		//
+
+		if (this.default_option("auto_includes")) {
+			for (let position in [ 'ruleset-pre', 'ruleset-post', 'table-pre', 'table-post', 'chain-pre', 'chain-post' ])
+for (let chain in (position in [ 'chain-pre', 'chain-post' ]) ? fs.lsdir(`/usr/share/nftables/includes.d/${position}`) : [ null ])
+	for (let path in fs.glob(`/usr/share/nf

Re: Contributions to netifd

[Jo-Philipp Wich]

Hi Joerg,


> we are a company of experts for automotive software and are working for
> many customers in the automotive industry.
> 
> One of our customers (a big german automotive supplier) is looking for a
> network configuration solution with some specific limitations and after
> evaluating several alternatives (e.g. network manager, wicked, ...) we
> ended up with netfid being the best solution giving the requirements.

That sounds really awesome!

> However netifd does not (yet) fully fit the requirements and instead of
> developing a fork, our customer would like to contribute to OpenWRT
> directly and integrate required features/changes.

I am really happy to hear that, and it's certainly the correct approach imho.

> Some examples of what we know at the moment what would be needed:
>  - We may need static linking of the libraries used in netifd. I saw
> that this is basically possible, but a bit broken IIRC.

It is entirely possible that this is broken, at least it is not something that
is frequently tested (or tested at all). But I don't foresee any big issues in
solving that.

>  - One of the requirement (For security reasons) is, that the system
> should be able to run without a script interpreter (not even a shell),
> but we need dhcp. So one of the first proposals would be creating a dhcp
> protocol implementation in c

Is the security policy also prohibiting embedded VMs that execute precompiled,
immutable byte code? Using ucode would allow for this and implementing the
higher level proto handling in it vastly simplifies things

>  - VPN configuration (possible ipsec with strongswan) will be required
> and we would also need to implement this directly in c.

That is a lot of complexity / a huge amount of code you're going to implement
in C there, and it's mostly going to lie dormant upstream I suppose, since
OpenWrt will likely stick to the existing external solutions integrated via
scripting.

>  - As far as I can see, there is no tooling for QoS/tc in openwrt at the
> moment. This is probably also required and could be implemented as part
> of netifd.

There's SQM scripts and the like. Or do you refer to QoS/tc integration
directly in netifd?

>  - Traffic filtering is one more point on our list of requirements. I
> know that you use fw3 and I saw fw4 for nftables. However I guess fw4
> will not work for us, because of ucode, but I could be wrong here.

Same question as above. Theoretically the entire fw4 ucode sources could be
precompiled into a single bytecode image which you could then embed into an
ELF executable providing the ucode VM. The end result would also be a compiled
executable running its own code and not externally modifiable source files.

> > I guess most of the changes could also help the OpenWRT community. If
> some of the changes are not really what you want for OpenWRT users, we
> could also think about implementing a plugin API, much like the proto
> handler API, but without additional exec-calls.

That definitely makes sense. However to be usable in practice for the kind of
use-cases OpenWrt aims for, that plugin API would need to rely on dlopen() to
be able to load plugins at runtime, I guess that would violate your security
architecture constraints, right?

Obviously it could be made a compile time option to build-in modules directly
in the executable - it's just something we should have in mind when designaing
an implementation.

> Would these kind of contributions be wanted by OpenWRT?

I can not speak for the others, but personally I do think that (at least some
of) the proposals are useful and I'd love to help.

I am a bit wary of doing a custom DHCP implementation, Busybox udhcpc for
example is quite old already and there's still issues being discovered with it
nowadays. I guess such a custom implementation would require a lot of time to
mature. On the other hand it's probably going to run in a very controlled
environment so it might not be as problematic after all.


Kind regards,
Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Question about ancient TARGET_CFLAGS in rules.mk?

[Jo-Philipp Wich]

Hi,

> I mean I know they are gigantic corner case where you can build an entire 
> house in the corner... But what are the drawbacks of such small fix? The 
> NULL check one for example seems pretty important...

Adding that null check there seems redundant, any code path that could lead to
blob_next() returning `NULL` would've already crashed due to a null pointer
dereference in blob_len(), invoked through blob_raw_len() and blob_pad_len()
by blob_next().

So adding this check will solve nothing apart from silencing a random warning,
without actually fixing any potential problem. It might even create the
incorrect impression that a possible NULL pointer situation is handled /
that blobmsg_add_string_buffer() is NULL-safe, while it actually is not.


~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH firewall4] fw4: add support for include.d dir

[Jo-Philipp Wich]

Hi,

instead of introducing uci includes that configure nft includes, why not
encode the chain/position etc. values directly into the path/filename and
directly include the file if it exists at the expected location?

A potential pattern could be
"[0-9][0-9]_{ruleset_pre,ruleset_post,table_pre,table_post,chain_pre_*,chain_post_*}_*.nft".

Taking the example from your mail, these *.nft includes would be stored at

/usr/share/firewall4/include.d/01_chain_pre_input_strongswan.nft
/usr/share/firewall4/include.d/02_chain_pre_output_strongswan.nft
/usr/share/firewall4/include.d/03_chain_pre_forward_strongswan.nft

Alternatively, the hooks could be moved into a subdirectory structure for
better clarity:

  /usr/share/firewall4/includes.d/
+ ruleset-pre/
   + 99_custom_named_set_declarations.nft
+ ruleset-post/
   + ...
+ table-pre/
   + ...
+ table-post/
   + ...
+ chain-pre/
   + input/
 + 29_strongswan.nft
   + output/
 + 29_strongswan.nft
   + forward/
 + 29_strongswan.nft
+ chain-post/
   + mangle_output/
 + 99_custom_dscp_fiddling.nft

(The numeric prefixes carry no semantic meaning in this structure, they'd just
be there to enforce a certain order within a given hook directory)


I think the above would be a lot more manageable since you'd just have to
place partial .nft files which are then folded into the final ruleset on fw4
start/reload.


~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 1/2] ubox: fix GCC fanalyzer warnings

[Jo-Philipp Wich]

Hi,

> [...]
> - free(aliases);
> + if (aliases)
> + free(aliases);

This check is redundant, the free() function is guaranteed to be NULL-safe in
the standard:

   The free() function shall cause the space pointed to by ptr to be
   deallocated; that is, made available for further allocation. If ptr is a
   null pointer, no action shall occur.

> [...]

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Query regd. rw rootfs

[Jo-Philipp Wich]

Hi,

> As mentioned in https://openwrt.org/docs/techref/preinit_mount#failsafeBy,
> rootfs is made rw via overlay. So, even a non-root user will have the
> ability to modify (for e.g. delete dnsmasq entry in /etc/passwd).

no, a non-root user will lack sufficient permission to modify these files.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Query regd. rw rootfs

[Jo-Philipp Wich]

Hi,

> If the behavior is not same, can you let me know how "below" is 
> handled/taken care in OpenWRT? "changes made to sensitive files for e.g. 
> /etc/passwd (deleting a line. Deleting passwd file etc.) will have adverse 
> impact on security and some init scripts may not start etc."

OpenWrt does not have special handling for such situations. Users deleting
parts of vital system configuration files (or even entire files such as
libraries, init scripts etc.) will need to recover their system
manually.

What is the actual problem you are trying to solve? If a user possesses
enough permissions to modify /etc/passwd he might as well modify init scripts,
replace legitimate executables with malicious ones or simply replace the
running firmware altogether by reflashing another image.

I don't see the point in adding such "protections" aside from increasing code
complexity, bug and attack surface as well as required storage footprint.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Query regd. rw rootfs

[Jo-Philipp Wich]

Hi,

> [...]
> Is this behavior the same with procd?

it is not the same. There is no special handling for specific overlay file
paths in procd.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Wrong hash for firewall package?

[Jo-Philipp Wich]

Hi again,

please ignore my previous message, that was an incorrect observation on my
side. In fact my locally generated source archive matches the one on the
source mirror, so I assume Rui's recent bump simply added a wrong checksum.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Wrong hash for firewall package?

[Jo-Philipp Wich]

Hi,

I compared the package stored on the source mirror with the locally generated
mismatching one. The contained .tar has the same checksum but the compressed
xz file is different. I suppose this is due to the recent enabling of
multithreaded xz compression which yields different results on different 
machines.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH firewall4] ruleset: add missing pre_* chains

[Jo-Philipp Wich]

Hi,

can't you do the same by dropping a snippet into /etc/nftables.d/ which simply
registers chain with lower priority hooks? In fact there already is an example
[1].

So in order to achieve what you want, create an
`/etc/nftables.d/10-pre-chains.nft with the following contents:

-- 8< --
chain pre_input {
  type filter hook input priority -1; policy accept;
}

chain pre_forward {
  type filter hook forward priority -1; policy accept;
}

chain pre_output {
  type filter hook output priority -1; policy accept;
}
-- >8 --


Since nftables makes it easy to have many hooks, I doubt that such a generic,
mostly unused facility is needed at all. If your package/process/use case
requires staging custom rules before the default ones, drop a partial into
/etc/nftables.d/ which declares own chains with lower priority hooks along
with the rules you need.

~ Jo


1:
https://git.openwrt.org/?p=project/firewall4.git;a=blob;f=root/etc/nftables.d/10-custom-filter-chains.nft;h=4cb421308f2f8b084add77fc51f8430596d83abf;hb=b2682251a173103490a4a2556fb87e4df1d627b3#l8



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Question about DNS-Tap integration

[Jo-Philipp Wich]

Hi,
> My question is about whether are you planning to introduce the DNS-Tap
> over TCP/IP to some version of the OpenWRT Router firmware. It's the emerging
> technology which might be very useful in the future.

you probably should direct these questions to the upstream DNSmasq project.

> You might try to have a look to the link [1] and evaluate but having the
> option to set DNS-Tap sender to some IP address and port using OpenWRT
> configuration interface (LuCI) would be a pretty nice thing.

LuCI integration will only happen once there is a base system facility in place.

> Are you planning to add such a support?

I don't think there's any specific plans. If at all, it might become available
by an update on OpenWrt once it is integrated by upstream DNSmasq.


Regards,
Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 1/4] uclient-fetch: --header option to pass additional raw HTTP headers

[Jo-Philipp Wich]

Hi Sergey,

a minor nitpick inline below.

On 5/9/22 11:59 PM, Sergey Ponomarev wrote:
> You can add a custom HTTP header(s) to request:
> 
> wget --header='Authorization: Bearer TOKEN' \
> --header='If-Modified-Since: Wed, 9 May 2021 12:16:00 GMT' \
> https://example.com/
> 
> Some headers like Authorization or User-Agent may be already set by 
> --password or --user-agent.
> We may override them but it's a protection from user itself.
> To keep code concise the logic omitted.
> 
> Signed-off-by: Sergey Ponomarev 
> ---
> [...]
> + case L_HEADER:
> + if (!raw_headers) {
> + /* Max possible count of headers is the 
> count of args (argc) - 2
> +  Since the first arg is program and 
> last is a URL.
> +  But user may forget the URL and 
> raw_headers is null terminated so allocate argc */
> + raw_headers = calloc(argc, sizeof(char 
> *));

Please handle a possible calloc() error here. Maybe consider introducing an
"xalloc()" or similar helper which wraps calloc() and invokes abort() on NULL
return.

> + }
> + raw_headers[raw_headers_count] = optarg;
> + raw_headers_count++;
> + break;
>   case L_POST_DATA:
>   post_data = optarg;
>   break;
> [...]



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Unpatched DNS bug affects millions of routers and IoT devices

[Jo-Philipp Wich]

Hi,

> [QUOTE]
> 
> The library uClibc and its fork from the OpenWRT team, uClibc-ng. Both
> variants are widely used by major vendors like Netgear, Axis, and
> Linksys, as well as Linux distributions suitable for embedded
> applications.
> 
> [/QUOTE]
> 
> Does it affect my TP-link AC1200 wireless router?

Neither does current OpenWrt use uClibc, not did the OpenWrt team fork uClibc
into uClibc-ng, both projects are completely unaffiliated with OpenWrt.

It likely does not affect your TP-link AC1200 wireless router if it runs one
of the currnetly supported OpenWrt version series 19.07, 21.02 or 22.03.

OpenWrt switched the default libc implementation to musl in June 2015 and
dropped the last remaining ARC-only support for uClibc-ng in December 2020.

Many vendors use proprietary forks based on a very old OpenWrt 15.05 ("Chaos
Calmer") version, which still used uClibc. This version is long EOL though and
not maintained by the OpenWrt project anymore. It is up to the vendors to
provide patches for their OpenWrt based firmware derivates.

Regards,
Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] packages: nvram: add NVRAM quirks for bcm53xx target

[Jo-Philipp Wich]

Hi,

> I think boot() would look better, but it would be good to keep this consistent
> with the bcm47xx code. Using boot() or start() does not make a big difference,
> boot() calls start() anyway.

an "/etc/init.d/whatever restart" will not execute boot(), which would be a
reason to prefer it over start() for code that is not meant to be (re)executed
by the user.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] kernel/kmod-lib-lzo: include the lzo-rle kmod in the package

[Jo-Philipp Wich]

Hi,

> Albeit a separate crypto module, lzo-rle uses the same kernel library as lzo.
> Crypto API users (zram, for example) expect both lzo and lzo-rle to be
> available, so let's include lzo-rle (about 5.5 kiB) in the lib-lzo package.
> 
> Based on e9hack's original patch: 
> https://patchwork.ozlabs.org/project/openwrt/patch/541cbfbd-76f2-59b3-a867-47b6f0fc7...@gmail.com/
> 
> Signed-off-by: Rui Salvaterra 

Acked-by: Jo-Philipp Wich 



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Drop CONFIG_IPV6 ?

[Jo-Philipp Wich]

Hi,

> Is anyone disabling CONFIG_IPV6 ?

no.

> Do people agree we can drop CONFIG_IPV6 ?

yes.

> Should we do this before we branch 22.x ?

yes please. I am tired of sprinkling ifdefs, maintaining separate package
build flavors, conditional dependency forests and runtime checks for IPv6
presence everywhere.

Even if it is decided to leave CONFIG_IPV6 in place I'll probably stop
respecing it for things like uhttpd, rpcd, LuCI etc. soon since I consider
builds with CONFIG_IPV6=n to be an unsupported configuration.


~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Firewall question

[Jo-Philipp Wich]

Hello Hartmut,

[...]

> If I check the logs from apache2, I see from lan network only the gateway
> or router ip and no client ip's. From guest network, I see every individual
> client ip's.

Yeah, for lan this is expected. The automatic NAT reflection consists of one
DNAT rule to forward the current_ext_iface_addr:port to the internal
destination and one SNAT rule to rewrite the source of such reflected traffic
to the external (or internal) router IP in order to force responses via the
router which in turn forwards them back to the original requesting client.

Without that additional SNAT, the replies would come from a different source
(DNAT target's internal IP) than were the request was initially sent to
(external WAN IP) from a requesting host's pov. Such unexpected response is
usually ignored and discard by the network stack of the requesting host.

Unfortunately that also means that the DNAT target never sees the actual
source IP for reflected traffic.

I do not understand however how the guest routing works in your network
currently...

> Does exist a way to see the individual client ip's from lan network too? Or
> is this only possible, if I configure a third network for the linux
> server?

I am not sure what you mean with 3rd network. Does it mean the server has an
address in the guest subnet? Maybe you can describe its network setup in more
detail...

> 
> It is possible to do the things of hotplug script by a standard firewall
> rule in '/etc/config/firewall' ?

You mean a DNAT rule that specifically uses the current WAN IP as destination
match (not all WAN interfaces) and not doing reflection?

The following should be equivalent:

config redirect
option name 'guest: Redirect wan HTTPS from port 443 to 192.168.199.80 on
port 8443'
option target 'DNAT'
option src 'wan'
option dest 'lan'
option proto 'tcp'
option family 'ipv4'
option src_ip '10.1.0.0/16'
#option src_ip 'guest' # alternative that does not hardcode guest subnet
option src_dip 'wan' # sic! "wan" is resolved to the current IP
option src_dport '443'
option dest_ip '192.168.199.80'
option dest_port '8443'
option reflection '0'


~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Merged: opkg_remove: avoid remove pkg repeatly with option--force-removal-of-dependent-packages

[Jo-Philipp Wich]

Merged into project/opkg-lede.git, branch master.
Thank you!


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] toolchain: musl: disable crypt size hack on !SMALL_FLASH devices

[Jo-Philipp Wich]

Hi,

while the decision to do that seems obvious on first sight, I think that
supporting different password hashing algorithms on different targets might
lead to unexpected surprises for downstream users. E.g. when precalculated
password hashes taken from one device are built inside custom firmware images
for another small flash device and suddenly the login is not working anymore.

I'd rather reenable all hash types for all targets, even small flash ones to
maintain consistency.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

[RFC] Stop providing binary package updates for release builds?

[Jo-Philipp Wich]

Hi,

since the release of LEDE 17.01.0, OpenWrt started offering updated binary
packages for released versions, means the HEAD of a released stable branch is
continuously getting rebuilt and the resulting binaries are uploaded to the
release repositories. Users will see those updated packages as "upgrades" and
are able to install them via opkg or LuCI.

There were several motivations to work towards providing these "continuous"
updates:

 - Ability to quickly push security or bug fixes without requiring users to
   wait for the next release and forcing them to fully reflash their
   deployments (also having to accept unwanted Kernel updates, potential
   regressions or potential flash failures)

 - Ability to de-duplicate required disk space, no need to keep a ~90%
   identical copy of the entire package universe for each minor release

 - Ability for package maintainers to push important/useful updates to
   stable releases without needing to wait for the next release cycle to
   have these changes available for a wider, non-snapshot using audience

However after over four years of providing these updates it appears that the
overall usefulness of this approach to the user base is not as certain:

 - The wiki actively discourages users from upgrading packages [1]

 - The overall sentiment in the forum is to instruct users to not/never
   upgrade packages [2] but to always upgrade to the next release, build from
   source [3] or use snapshots/IB

 - Many contributors are not aware of the additional responsibilities and
   potential pitfalls when backporting features, fixes or updates to stable
   branches [4]

 - Many developers or active community members do not appear to care about
   binary releases at all, only using the full buildroot to build images
   completely from source

 - In my view, an overall sentiment among most contributors that OpenWrt is a
   source based distribution, with precompiled images and binary package
   repositories being an after thought at best

Given these facts I would like to propose rolling back continuous package
build changes and revert mostly to the pre-LEDE 17.01.0 approach for future
release processes. In particular this entails:

 - Stop providing binary package updates; build release images and associated
   repositories once only, archive the artifacts and redelegate build
   resources back to master snapshots

   - Maintainers must ensure that packages/device images that should be part
 of a release build properly during the RC phase, whatever is broken will
 not be part of the repositories but might/might not reappear with the
 next maintenance release

 - Stop dividing the build process into distinct image and package phases,
   simply build the entire release once from a complete source tree

 - Stop sharing package repositories among different targets

 - Discontinue release branch HEAD builders and only do branch build testing
   during the pre- and RC build phases

Once implemented, this would allow us to:

 - Massively reduce the complexity of the build logic, it will basically
   become a "make world" with no need to track changes, iterative cleanups
   etc.

 - Reduce the amount of needed compute resources, we only need to build master
   snapshots and occasionally ramp up resources to build release images

 - Focus on source code development and master branch features without having
   to worry about binary package peculiarities such as ABI- and feature
   compatibility, versioning or dependency handling

 - Simplify release branch maintenance, build images every few months and in
   case of security issues in vital components like openssl or dropbear we
   could instruct users to wait for the next maintenance release or switch to
   snapshots

 - Optimize packages. Ship different package feature selections for each
   target, e.g. small ones for restricted targets, big fully featured packages
   with hand written assembly optimizations for mvebu. Full coreutils, glibc
   for x86 builds

Potential alternatives for users:

 - Attended sysupgrade instead of releases and package updates to reflash
   master snapshots with updated packages from time to time

 - Build from source

 - Use another downstream distribution


In my opinion, there is a fundamental decision to be made on whether we would
like to pursue the goal of (also) being a binary distribution for
routers/embedded appliances or if we simply concentrate on the build-from-
source aspect and leave the binary package business to forks or other
downstream distributions projects.

Solutions like Debian/Alpine chroots or entware maybe could also be an
alternative for OpenWrt's own binary package ecosystem.

Given that maintaining binary package distributions is hard and boring work we
need to decide whether we want to fully and properly pursue this goal or
rather not at all and focus on always porting master to the latest minor
Kernel version and finding the optimal compilation flags for 

Re: [PATCH] netifd: system-linux: add dev_type info for ubus network.device status

[Jo-Philipp Wich]

Hi,

> I have now taken a look at your suggestion.
> Unfortunately, I found that not all network interfaces have set the DEVTYPE
> attribute set in their uevent file. I have not yet found any information
> who sets this value. Does this do the driver or the subsystem?

afair it is set by the responsible driver.

> [...]
> The question now if this DEVTYPE is not set what should we do?

I think in case of an absent DEVTYPE variable you can assume "ethernet" as
fallback (cross check with /sys/class/net/*/type == 1).

Another approach would be falling back to your original mapping in case
DEVTYPE is not present (so 1->ethernet, 24->ppp, etc.)


~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] netifd: system-linux: add dev_type info for ubus network.device status

[Jo-Philipp Wich]

Hi,

imho these types are not that useful in practice (e.g. tap devices etc. are
all reported as "ethernet". Maybe expose /sys/class/net/$devname/uevent
DEVTYP= instead.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: dnsmasq issue

[Jo-Philipp Wich]

Hi,

> Or does this phenomenon also happen if running without ujail?

afair it also happens without ujail. Whenever handler scripts are enabled,
dnsmasq will spawn a secondary helper process. This was also the reason why
/etc/hotplug.d/*/ support is only conditionally enabled if installed scripts
are found since users complained about the increased resource usage [1].

Regards,
Jo

1: https://git.openwrt.org/8b486ec2b52056b737a4ce64a2040a9a27a6bd60



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

[PATCH] procd: procd.sh: make no assumptions about init script path

[Jo-Philipp Wich]

Init scripts in /etc/init.d/ may be symlinks pointing elsewhere, so it
is not safe to assume that the basename of the real path is existing.

Instead of trying to reassemble the target path from the basename when
setting up triggers, trust the result of readlink and fall back to
`$initscript` which corresponds to `argv[0]` when readlink failed.

This fixes reload trigger setup for init script symlinks that point
to files with different base names.

Signed-off-by: Jo-Philipp Wich 
---
 package/system/procd/files/procd.sh | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/package/system/procd/files/procd.sh 
b/package/system/procd/files/procd.sh
index 3549a5a914..5fc5441be0 100644
--- a/package/system/procd/files/procd.sh
+++ b/package/system/procd/files/procd.sh
@@ -299,11 +299,10 @@ _procd_add_interface_trigger() {
 }
 
 _procd_add_reload_interface_trigger() {
-   local script=$(readlink "$initscript")
-   local name=$(basename ${script:-$initscript})
+   local script=$(readlink -f "$initscript")
 
_procd_open_trigger
-   _procd_add_interface_trigger "interface.*" $1 /etc/init.d/$name reload
+   _procd_add_interface_trigger "interface.*" $1 "${script:-$initscript}" 
reload
_procd_close_trigger
 }
 
@@ -424,13 +423,12 @@ _procd_add_raw_trigger() {
 }
 
 _procd_add_reload_trigger() {
-   local script=$(readlink "$initscript")
-   local name=$(basename ${script:-$initscript})
+   local script=$(readlink -f "$initscript")
local file
 
_procd_open_trigger
for file in "$@"; do
-   _procd_add_config_trigger "config.change" "$file" 
/etc/init.d/$name reload
+   _procd_add_config_trigger "config.change" "$file" 
"${script:-$initscript}" reload
done
_procd_close_trigger
 }
-- 
2.30.2


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

[RFC PATCH] treewide: drop librt and libpthread packages

[Jo-Philipp Wich]

Since OpenWrt's main libc library, musl, does not provide separate shared
object files for libpthread and librt, the existing binary packages for
them are empty placeholders which provide no runtime functionality and
frequently cause confusion among users who attempt to build software
linking -lrt or -lpthread on target.

To clean this situation up somewhat and to simplify binary package
dependecies for all of the potential musl, glibc and uclibc cases, drop
those packages and move libpthread.so as well as librt.so into the main
libc package for those libc implementations that happen ship them as
extra shared libraries.

Signed-off-by: Jo-Philipp Wich 
---
 package/devel/perf/Makefile  |   2 +-
 package/devel/valgrind/Makefile  |   2 +-
 package/libs/libevent2/Makefile  |   2 +-
 package/libs/libnl/Makefile  |   1 -
 package/libs/libusb/Makefile |   1 -
 package/libs/musl-fts/Makefile   |   1 -
 package/libs/toolchain/Makefile  | 108 ++-
 package/network/config/ltq-adsl-app/Makefile |   2 +-
 package/network/config/ltq-vdsl-app/Makefile |   2 +-
 package/network/ipv6/thc-ipv6/Makefile   |   5 +-
 package/system/opkg/Makefile |   2 +-
 package/utils/adb/Makefile   |   2 +-
 package/utils/mdadm/Makefile |   2 +-
 package/utils/util-linux/Makefile|   1 -
 14 files changed, 16 insertions(+), 117 deletions(-)

diff --git a/package/devel/perf/Makefile b/package/devel/perf/Makefile
index bbf3aaf9ce..a5d253e94d 100644
--- a/package/devel/perf/Makefile
+++ b/package/devel/perf/Makefile
@@ -27,7 +27,7 @@ include $(INCLUDE_DIR)/nls.mk
 define Package/perf
   SECTION:=devel
   CATEGORY:=Development
-  DEPENDS:= +libelf +libdw +PACKAGE_libunwind:libunwind +libpthread +librt 
+objdump @!IN_SDK @!TARGET_arc770 @KERNEL_PERF_EVENTS
+  DEPENDS:= +libelf +libdw +PACKAGE_libunwind:libunwind +objdump @!IN_SDK 
@!TARGET_arc770 @KERNEL_PERF_EVENTS
   TITLE:=Linux performance monitoring tool
   VERSION:=$(LINUX_VERSION)-$(PKG_RELEASE)
   URL:=http://www.kernel.org
diff --git a/package/devel/valgrind/Makefile b/package/devel/valgrind/Makefile
index e6ebff4b30..04a261a5c8 100644
--- a/package/devel/valgrind/Makefile
+++ b/package/devel/valgrind/Makefile
@@ -33,7 +33,7 @@ include $(INCLUDE_DIR)/kernel.mk
 define Package/valgrind
   SECTION:=devel
   CATEGORY:=Development
-  
DEPENDS:=@mips||mipsel||mips64||mips64el||i386||x86_64||powerpc||arm_v7||aarch64
 +libpthread +librt
+  
DEPENDS:=@mips||mipsel||mips64||mips64el||i386||x86_64||powerpc||arm_v7||aarch64
   TITLE:=debugging and profiling tools for Linux
   URL:=http://www.valgrind.org
 endef
diff --git a/package/libs/libevent2/Makefile b/package/libs/libevent2/Makefile
index 85c159c2a6..b6b2fdfa50 100644
--- a/package/libs/libevent2/Makefile
+++ b/package/libs/libevent2/Makefile
@@ -102,7 +102,7 @@ endef
 define Package/libevent2-pthreads
   $(call Package/libevent2/Default)
   TITLE+= Pthreads library (version 2.1)
-  DEPENDS+=+libpthread +libevent2-core
+  DEPENDS+=+libevent2-core
 endef
 
 define Package/libevent2-pthreads/description
diff --git a/package/libs/libnl/Makefile b/package/libs/libnl/Makefile
index db0c65c7a7..64e37a28d5 100644
--- a/package/libs/libnl/Makefile
+++ b/package/libs/libnl/Makefile
@@ -31,7 +31,6 @@ endef
 define Package/libnl-core
 $(call Package/libnl/default)
   TITLE:=Core Netlink Library
-  DEPENDS:=+libpthread
 endef
 
 define Package/libnl-genl
diff --git a/package/libs/libusb/Makefile b/package/libs/libusb/Makefile
index 6b80b3848d..abf966a384 100644
--- a/package/libs/libusb/Makefile
+++ b/package/libs/libusb/Makefile
@@ -30,7 +30,6 @@ define Package/libusb-1.0
   SECTION:=libs
   CATEGORY:=Libraries
   TITLE:=A library for accessing Linux USB devices
-  DEPENDS:=+libpthread +librt
   URL:=http://libusb.info/
   ABI_VERSION:=0
 endef
diff --git a/package/libs/musl-fts/Makefile b/package/libs/musl-fts/Makefile
index 494f700f8a..b2d9843bb3 100644
--- a/package/libs/musl-fts/Makefile
+++ b/package/libs/musl-fts/Makefile
@@ -36,7 +36,6 @@ define Package/musl-fts
   CATEGORY:=Libraries
   TITLE:=fts implementation for musl libc
   URL:=https://github.com/pullmoll/musl-fts
-  DEPENDS:= +libpthread
 endef
 
 define Package/musl-fts/description
diff --git a/package/libs/toolchain/Makefile b/package/libs/toolchain/Makefile
index dea99060f9..d3c571a80f 100644
--- a/package/libs/toolchain/Makefile
+++ b/package/libs/toolchain/Makefile
@@ -115,7 +115,7 @@ define Package/libasan
 $(call Package/gcc/Default)
   NAME:=libasan
   TITLE:=Runtime library for AddressSanitizer in GCC
-  DEPENDS:=@USE_GLIBC +librt +libstdcpp @!mips64 @!mips64el @!arc
+  DEPENDS:=@USE_GLIBC +libstdcpp @!mips64 @!mips64el @!arc
   ABI_VERSION:=5
 endef
 
@@ -144,7 +144,7 @@ define Package/libtsan
 $(call Package/gcc/Default)
   NAME:=libtsan
   TITLE:=Runtime library for ThreadSanitizer in GCC
-  DEPENDS

[PATCH] procd: setup /dev/stdin, /dev/stdout and /dev/stderr symlinks

[Jo-Philipp Wich]

Extend the hotplug.json ruleset to setup the common /dev/std{in,out,err}
symbolic links which are needed by some applications, e.g. nftables when
applying rulesets from stdin.

Signed-off-by: Jo-Philipp Wich 
---
 package/system/procd/files/hotplug.json | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/package/system/procd/files/hotplug.json 
b/package/system/procd/files/hotplug.json
index 7e0f129d35..b930b307a4 100644
--- a/package/system/procd/files/hotplug.json
+++ b/package/system/procd/files/hotplug.json
@@ -7,9 +7,19 @@
[ "has", "MINOR" ]
],
[
+   [ "if",
+   [ "eq", "DEVNAME", "null" ],
+   [
+   [ "makedev", 
"/dev/%DEVNAME%", "0666" ],
+   [ "exec", "/bin/ln", 
"-s", "/proc/self/fd/0", "/dev/stdin" ],
+   [ "exec", "/bin/ln", 
"-s", "/proc/self/fd/1", "/dev/stdout" ],
+   [ "exec", "/bin/ln", 
"-s", "/proc/self/fd/2", "/dev/stderr" ],
+   [ "return" ]
+   ]
+   ],
[ "if",
[ "eq", "DEVNAME",
-   [ "null", "full", 
"ptmx", "zero", "tty", "net", "random", "urandom" ]
+   [ "full", "ptmx", 
"zero", "tty", "net", "random", "urandom" ]
],
[
[ "makedev", 
"/dev/%DEVNAME%", "0666" ],
-- 
2.30.2


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Missing 'libubus.so' and change of ABI in 19.07.8

[Jo-Philipp Wich]

Hi,

> But I really can't see the breaking change, could you please point it out for 
> me?

struct ubus_client changed its layout.

> The second question is the change of the library name from 'libubus.so' to
> 'libubus.so.20210603'. Was this intended? 

Yes, it was intentional. Programs should link against the versioned shared
library at build time.

> Could we at least add the symlink
> 'libubus.so'?

No, as it will break coexistence of different versions of libubus.so. It will
also cause programs linking against the old libubus.so to pick up the new one
with a different `struct ubus_client` memory layout, leading to undefined
behavior and segmentation faults.

> My take is that both of these changes, together with unsharing the ubus
> package, were really unnecessary in a stable branch nearing its end of life.

The changes solve multiple denial of service and invalid memory access issues,
they were important enough to warrant this breaking change.

> Thank you for insights and kind regards.

You can find more details about library ABI versioning in
https://openwrt.org/docs/guide-developer/package-policies#shared_libraries


Kind regards,
Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH firewall4 1/2] tests: adapt test to new ICMP print logic

[Jo-Philipp Wich]

Hi,

merged both patches, thank you!

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] nftables: bump to 1.0.0

[Jo-Philipp Wich]

Hi,

> Signed-off-by: Stijn Tintel 

Acked-by: Jo-Philipp Wich 




signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 2/2] hostapd: force ieee80211w instead of setting a default

[Jo-Philipp Wich]

Hi,

> right now luci will force 2

nope, it will not. It calculates the dynamic default according to the logic in
hostapd.sh and will remove the value from uci if the selected user value
matches the dynamic default [1].

So with WPA3 selected and choosing 11w ...
 ... required -> remove option ieee80211w
 ... optional -> write option ieee80211w 1
 ... disabled -> write option ieee80211w 0 [2]

With WPA3 mixed mode selected and choosing 11w ...
 ... required -> write option ieee80211w 2
 ... optional -> remove option ieee80211w
 ... disabled -> write option ieee80211w 0 [2]

With any other WPA mode selected and choosing 11w ...
 ... required -> write option ieee80211w 2
 ... optional -> write option ieee80211w 1
 ... disabled -> remove option ieee80211w


1: See defaults at:
https://github.com/openwrt/luci/blob/master/modules/luci-mod-network/htdocs/luci-static/resources/view/network/wireless.js#L1685
2: Disabling (0) is bugged in 21.02.0 and has been fixed in master with:
https://github.com/openwrt/luci/commit/0b49ed45c6e9f4bc10abdcea392125aec3794e58


~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 2/2] hostapd: force ieee80211w instead of setting a default

[Jo-Philipp Wich]

Hi,


> Doesn't that downgrade PSK-SAE which the user configured to be with
> mandatory MFP to now only have optional MFP?

yes. The way I read it, it also prevents using the WPA3 protocol without
obligatory MFP (afair some users use that for limited drivers)

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: RFC: toolchain for building eBPF modules within the OpenWrt build system

[Jo-Philipp Wich]

Hi Felix,

I'd suggest to require a preinstalled host toolchain for the time being.

~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] base-files: make os-release symbolic link absolute

[Jo-Philipp Wich]

Hi,

> Did you check that none of the build scripts try to access this file? In
> case they do, they'd fail since an absolute path wouldn't exists on the
> building host system.

apart from that it might break 3rd party workflows for no obvious
reason. The existing absolute symlinks mentioned all point to ephemeral
resources while os-release points to an actual fixed file.

In doubt, I'd personally favor keeping things as-is as this change is
not actually fixing or improving things, just breaking potential
use-cases (e.g. `cat /some/mounted_rootfs/etc/os-release`)

~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH luci] luci-mod-network: wireless.js: allow changing the 60 GHz radio's frequency

[Jo-Philipp Wich]

Hi Alex,

thank you for this patch. Based on your submission I pushed a somewhat
smaller fix to LuCI master which omits the last two hunks:
https://github.com/openwrt/luci/commit/e5626ece12236f6be9dbb6da6eb90fcbb469a1f0

The code related to the `hwmode` option is there for backwards
compatibility reasons and should only deal with 11a and 11g.

OpenWrt master uses the new `option band` to specify wireless frequency
bands and that should be handled correctly already without the
additional changes.

~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 19.07 4/4] treewide: mark selected packages nonshared

[Jo-Philipp Wich]

Hi,

> I've never seen a non integer release, is there a special reason for this?

I used to do that in the past when backporting master changes to
branches that didn't apply cleanly anymore.

In this case the package contents between master and release branches
deviated while keeping the same software version. To denote that fact I
kept the last common PKG_RELEASE as appended a minor version instead.

So assuming a package "foo" had version 1.2.3 with PKG_RELEASE 4 in
master and branch, and that the contents deviated over time (different
patches, init script fixes, system integrations) while both remaining at
version 1.2.3, the master package would've get bumped to 1.2.3-5,
1.2.3-6 and so on while the "forked" branch version gets bumped from
1.2.3-4 to 1.2.3-4.1, 1.2.3-4.2 etc.

HTH,
Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: ip rule processing partly broken (21.02 and Master)

[Jo-Philipp Wich]

Hi,

the ip rules encoded in /etc/config/network are processed by netifd C
code directly, they're not translated into busybox ip calls.

The entire busybox ip.c code contains not a single instance of
FIB_RULE_INVERT so it simply does not implement inversion. It will also
not be able to report inverted rules properly, since there is no code to
print the FIB_RULE_INVERT flag.

Are you absolutely sure that the uci rules are improperly applied or is
this just a case of busybox ip rule displaying them wrongly without the
"not" flag?

Can you install ip-full or ip-tiny from the iproute2 suite and verify
the "ip rule" output again?

~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Luci->Network->Interfaces is broken

[Jo-Philipp Wich]

Hi,

> It works now (no error message), but the bad thing is, it isn't visible in
> LuCI, which wireless adapter's are attached to the bridge and it needs some
> clicks to see which interfaces (wired ports) are attached to the bridge.
> 
> I don't like this solution in LuCI.

Fixed with
https://github.com/openwrt/luci/commit/608f89429b4b8537ddaefd070c777a6d4fb1c7a1

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 19.07] ubus: update to version 2021-06-03

[Jo-Philipp Wich]

Hi Petr,

does that change the libubus abi?

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] busybox: sysntpd: add trigger to reload server

[Jo-Philipp Wich]

Hi,

>> start_service() {
>> +. /lib/functions/network.sh
> 
> 
> This doesn't look right.  It's usually added at the top of the file, unnested.

Which would be the wrong thing to do here. Since the init script is run
on the host system during build (to enable it), it must not source files
which are only available on target.

Sourcing the library inside the procedure is correct.



~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Interface names when putting 802.1q VLAN on top of bonding configuration

[Jo-Philipp Wich]

Hi,

> One more question, now I'm trying to put a bridge on top of each of these 
> vlan* interfaces so that I can map those to a few physical interfaces. I also 
> need several vlans to map to one of the interfaces (tagged).. not sure how to 
> do that yet either. Any suggestions with this config? When I apply it, I lose 
> network access.

If you move an interface into a bridge (e.g. vlan10 into bv10) then you need
to put the IP addresses (172.20.32.250/255.255.255.0 for vlan10) onto the
bridge and remove it from vlan10. There is also no need to retain the `config
interface vlan10` section then anymore. Netdevs used as bridge ports cannot be
used standalone anymore.

Your config should look like that:

-- 8< --
config interface 'loopback'
option ifname 'lo'
option proto 'static'
option ipaddr '127.0.0.1'
option netmask '255.0.0.0'

config globals 'globals'
option ula_prefix 'fdb9:bf48:0362::/48'

config interface lanxge
option proto 'bonding'
option auto '1'
option bonding_policy '802.3ad'
option link_monitoring 'mii'
option slaves 'eth4 eth5'
option lacp_rate 'fast'
option miimon '100'
option use_carrier 1
option xmit_hash_policy 'layer3+4'
option force_link '1'
option ipaddr 127.0.0.2

config device
option type 8021q
option ifname bonding-lanxge
option vid 10
option name vlan10

config device
option type 8021q
option ifname bonding-lanxge
option vid 20
option name vlan20

config device
option type 8021q
option ifname bonding-lanxge
option vid 21
option name vlan21

config device
option type 8021q
option ifname bonding-lanxge
option vid 30
option name vlan30

config interface 'wan'
option ifname 'eth0.0'
option proto 'dhcp'

config interface bv10
option type 'bridge'
option ifname vlan10
option proto static
option ipaddr 172.20.32.250
option netmask 255.255.255.0

config interface bv20
option type 'bridge'
option ifname vlan20
option proto static
option ipaddr 172.20.34.2
option netmask 255.255.255.128  

config interface bv21
option type 'bridge'
option ifname vlan21
option proto static
option ipaddr 172.20.35.3
option netmask 255.255.255.240

config interface bv30
option type 'bridge'
option ifname vlan30
option proto static
option ipaddr 172.20.34.130
option netmask 255.255.255.128
-- >8 --


Since legacy bridge declarations (through option type bridge in config
interface) are being phased out, your config ideally should look like that
(requires current master or OpenWrt 21.02 branch):

-- 8< --
config interface 'loopback'
option ifname 'lo'
option proto 'static'
option ipaddr '127.0.0.1'
option netmask '255.0.0.0'

config globals 'globals'
option ula_prefix 'fdb9:bf48:0362::/48'

config interface lanxge
option proto 'bonding'
option auto '1'
option bonding_policy '802.3ad'
option link_monitoring 'mii'
option slaves 'eth4 eth5'
option lacp_rate 'fast'
option miimon '100'
option use_carrier 1
option xmit_hash_policy 'layer3+4'
option force_link '1'
option ipaddr 127.0.0.2

config device
option type 8021q
option ifname bonding-lanxge
option vid 10
option name vlan10

config device
option type 8021q
option ifname bonding-lanxge
option vid 20
option name vlan20

config device
option type 8021q
option ifname bonding-lanxge
option vid 21
option name vlan21

config device
option type 8021q
option ifname bonding-lanxge
option vid 30
option name vlan30

config device
option type bridge
option name bv10
list ports vlan10

config device
option type bridge
option name bv20
list ports vlan20

config device
option type bridge
option name bv21
list ports vlan21   

config device
option type bridge
option name bv30
list ports vlan30

config interface 'wan'
option ifname 'eth0.0'
option proto 'dhcp'

config interface bv10
option device bv10
option proto static
option ipaddr 172.20.32.250
option netmask 255.255.255.0

config interface bv20
option device bv20
option proto static
option ipaddr 172.20.34.2
option netmask 255.255.255.128  

config interface bv21
option device bv21
option proto static
option ipaddr 172.20.35.3
option netmask 255.255.255.240

config interface bv30
option device bv30
option proto 

Re: [PATCH] base-files: simplify setting device MAC

[Jo-Philipp Wich]

Hi,

> Ideally you should be able to use jsonfilter too but I don't know how to
> deal with "-" in a property name.

Use bracket notation.

> Following doesn't work for me:
> 
> ubus call network.device status '{ "name": "br-lan" }' | jsonfilter -e
> "$.bridge-members"

ubus call network.device status '{ "name": "br-lan" }' | jsonfilter -e
> "$['bridge-members']"

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Luci->Network->Interfaces is broken

[Jo-Philipp Wich]

Hi,

> This is the reason. Long time ago, I did select the option 'Remove ipkg/opkg
> status data files in final images' to reduce the image size. Since such an
> option can be selected, LuCI cannot assume, that the file netifd.control 
> exists.

fixed.


~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Interface names when putting 802.1q VLAN on top of bonding configuration

[Jo-Philipp Wich]

Hi,

the following should do what you want.

config device
  option type 8021q
  option ifname bonding-lan
  option vid 20
  option name vlan20

config interface vlan20
  option ifname vlan20
  option proto static
  option ipaddr 172.20.34.2
  option netmask 255.255.255.128


~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Merged: rules: fix device and chain usage forDSCP/MARK targets

[Jo-Philipp Wich]

Merged into project/firewall3.git, branch master at
http://git.openwrt.org/?p=project/firewall3.git.

Thank you!


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Merged: zone: avoid duplicates in devices list

[Jo-Philipp Wich]

Merged into project/firewall3.git, branch master at
http://git.openwrt.org/?p=project/firewall3.git.

Thank you!


___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 1/2] uhttpd: Reload config after uhttpd-mod-ubus was added

[Jo-Philipp Wich]

Hi Hauke,

thanks for looking into it!

I have a couple of remarks...

> [...]
>  [ "$(uci -q get uhttpd.main.ubus_socket)" = "/var/run/ubus.sock" ] && {
>   uci set uhttpd.main.ubus_socket='/var/run/ubus/ubus.sock'
>   uci commit uhttpd
> + reload_config

That might reload other, unrelated changes. It likely isn't that much of
a problem in practise for most users but could interfere with other
provisioning setups or custom logic.

I think it is better if we simply perform a selective /etc/init.d/uhttpd
reload call here.

>  }
>  
>  exit 0
> 

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Question about LuCI state (incl DSA) & request for testing

[Jo-Philipp Wich]

Hi,

> I'd like to ask: what's the current state of LuCI?
> One thing that probably requires some extra focus is DSA. Are there
> any remaining issues regarding it?

some parts of the DSA/device config code are intentionally disabled since
netifd didn't handle these things at the time it was written. That needs to be
revised yet. Also support for wireless bridge vlans is missing yet.

We also need to revise the default network configuration of DSA devices to
declare a "switch0" bridge device by default.

While this is not strictly necessary it simplifies adding further VLANs a lot
since users are not required to redo their entire network config beforehand.


~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Revisiting ABI_VERSION handling policy

[Jo-Philipp Wich]

Hi,

> The last stable update didn't change SONAME, but it included changes to
> the same data structure that triggered the bug that I ran into. It could
> very likely have introduced very similar subtle breakage.

this is a clear upstream bug then. Might make sense to look into how other
distributions deal with that.

> Looking at the abi-laboratory timeline, there have been many incremental
> wolfssl releases that did not change the SONAME, but introduced
> potentially ABI incompatible changes. In fact, it seems to me that there
> are more breaking than non-breaking updates.

That doesn't inspire confidence into wolfSSL at all then. Maybe we should
reconsider using it by default. If upstream is not able to deliver a stable
ABI for a crypto library, I'd say it isn't really fit for our purposes.

> If we were to peg the ABI_VERSION to the package version, we could still
> deliver security updates by backporting relevant patches and review
> potential ABI isuses of those patches.

If we at the same time ensure that the library SONAME is properly modified to
always match ABI_VERSION and that the packaged shared object filename matches
SONAME then it is fine for me. Merely bumping ABI_VERSION while continue
shipping the same lib*.so.123 would not work as it'd lead to the previously
mentioned file conflicts (e.g. libfoobar1.1 (ABI_VERSION:=1.1) and
libfoobar1.2 (ABI_VERSION:=1.2) both shipping libfoobar.so.123 as incorrectly
specified by upstream)

>> [...]
> The breakage that I was seeing has nothing to do with exported symbols.
> The layout of a data structure changed. I'm not sure how to detect that
> from readelf/objdump output.

Okay, I missed that. Still, not changing exported data structures while
keeping the current SONAME should be common sense for any library maintainer.

The ABI_VERSION mechanism as it is right now relies on upstream being sane
obviously. I wrongly assumed that wolfSSL does proper ABI hygiene. Apparently
it doesn't so no wonder it breaks down in subtle ways.

As for the ability to detect it - it should be possible to reconstruct
effective binary struct layout from DWARF debug info. So given debug builds of
both the old and the new library versions, the output of a utility such as
dwarfdump can be compared.

>> [...]
> How do we address this in a proper way that isn't a potential foot-gun
> on every version upgrade?

Apart from investing time on our end to paper over versioning deficiencies, we
should talk to upstream, raise awareness for proper SONAME versioning. Also
look how Debian deals with it (if at all).

~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Revisiting ABI_VERSION handling policy

[Jo-Philipp Wich]

Hi,

> The goal of reducing unnecessary build churn makes perfect sense to me,
> but I think we need to reconsider the trade-off we're making here, and
> the failure modes of each option.

the motivation wasn't about build-churn at all but to ensure that library
packages can be cleanly upgraded and that different versions of libraries can
coexist on the same system without trashing userspace mid-upgrade. The other
major consideration is hard package version dependencies to ensure that
library users actually depend on the binary library ABI version they've been
linked against at build time.

> I believe that having to manually consider whether the ABI changed when
> updating a package comes with a nasty foot-gun that can deliver very
> subtle and hard to debug failure cases.

Upgrading a very central library package like wolfSSL which is used by uclient
(and thus opkg), wpad (wireless connectivity) and uhttpd through ustream
(webinterface access) is a sensible operation. In my view, sanely maintaining
the ABI_VERSION is simply part of the work when dealing with such a package.

Throwing all that over board just because you've been annoyed by a bug that
cost you a few hours is not the right way forward. By that metric, we
should've dropped most of buildroot years ago already.

> To fix this situation, I would like to switch wolfssl and maybe a few
> other packages prone to ABI breakage back to version based ABI_VERSION.
> At least wolfssl also needs a short suffix generated from config vars.

It makes little sense to peg the ABI_VERSION to the package version, at least
without revising the packaging at the same time. The ABI version should only
change if the shipped shared objects become incompatible (or more
specifically, if the SONAME changes).

By tying the ABI version to the package version without also ensuring that the
shipped .so files are differently named, you'll also introduce file level
package conflicts. A sanely managed ABI_VERSION should ensure that it is
possible for libfoo1 (ABI_VERSION 1) and libfoo2 (ABI_VERSION 2) to coexist on
the system which is a precondition for being able to cleanly opkg-upgrading
library packages without trashing the currently running userspace.

Especially for libraries with frequent security updates like wolfSSL, I'd
rather retain this ability.

> If we want to reduce build churn, we should find a way to automate
> setting the ABI version, since clearly our package maintainers + review
> process cannot (and in my opinion also should not) be trusted with this
> extra responsibility.

It should be possible to use readelf/objdump to examine the exported symbols
of a given shared object and see if either the size of a symbol changed or if
previously present ones were removed - addition should be fine.

Maybe this is something that can be added to the FIXUP=1 stuff.

> Maybe we can replace it with a hash of all installed header files. This
> might be a better way to resolve cases like this wolfssl instance, but
> we likely will still be rebuilding dependent packages on most updates.

A hash of installed header files makes little sense imho as it does not really
tell which symbols are exported by a shared object in the end. To me it seems
you conflate two different problems here:

a) maintaining ABI versions
b) ABI depending on compile time options

Unless I missed something, it appears your original issue was due to b) and it
can be solved by appending a hash of CONFIG_* symbol values to the explicitly
set ABI version.



Regards,
Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Job board support on openwrt.org?

[Jo-Philipp Wich]

Hi,

I don't think this is a good idea due to legal obligations,
administrative hassle, quality of work issues and so on.

NACK from me.

~ Jo

___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Dnsmasq 2.83 causes log spam

[Jo-Philipp Wich]

Hi Hannu,

unfortunately we lack a reliable reproducer so far.

A packet capture would be most helpful, another option would be a bisect of
the intermediate dnsmasq Git revisions.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: Add package version dependency for point releases

[Jo-Philipp Wich]

Hi,

why not simply wrap the workaround in a uname or /proc/version check?

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH] wireguard-tools: allow generating private_key

[Jo-Philipp Wich]

Hi,

I'm afraid this approach is not really acceptable. Automatically calling `uci
commit` outside of the early-boot / uci-defaults context is not safe.

There could be arbitrary user defined, intentionally uncommitted changes
stashed when the ifup sequence is running which you would inadvertently commit
along with the key option change.

So, a NACK from me.

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Re: [PATCH 1/3] rules: add commitcount function

[Jo-Philipp Wich]

Hi,

I think this change will introduce massive overhead during the DUMP phase of
the buildroot when scanning package metadata.

You should wrap the logic into an `ifneq ($(DUMP),1)` or similar and simply
return a static placeholder value if DUMP==1

~ Jo



signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel