Re: uci: unnecessary write accesses

2021-09-21 Thread Felix Fietkau
On 2021-09-21 13:19, Florian Eckert wrote:
> 
> I have written a small shell script, to track write access to the 
> '/etc/config' directory.
> For this task I am using the inotify-tool package [1].
> I am using the inotifywait tool to add the watchers [2] with a small 
> shell script and log them to the syslog.
> 
> If I change a 'uci' option with the  the 'uci' commands, then I get 
> create and write events for a temporary file in the '/etc/config/' 
> directory.
> I didn't expect it to be like this.
> 
> For example, if I now change a file using the 'uci' command, a tmp file 
> is created under '/etc/config'.
> Is this correct? Or do we have a problem there.
> I have always thought that for embedded systems with flashes it is 
> important to make as few write accesses as possible.
> 
> Shell commands to change uci config:
> 
> root@st-dev-07 ~ # uci set system.led_Power.trigger=none
> root@st-dev-07 ~ # uci commit system
> 
> Log output of my Script:
> 
> Tue Sep 21 12:50:38 2021 daemon.info loginotify[17375]: 
> {"directory":"/etc/config/","file":".system.uci-fEGgbp","event":"CREATE","time":"09/21/21
>  
> 12:50:38"}
> Tue Sep 21 12:50:38 2021 daemon.info loginotify[17375]: 
> {"directory":"/etc/config/","file":".system.uci-fEGgbp","event":"MODIFY","time":"09/21/21
>  
> 12:50:38"}
> Tue Sep 21 12:50:38 2021 daemon.info loginotify[17375]: 
> {"directory":"/etc/config/","file":".system.uci-fEGgbp","event":"MODIFY","time":"09/21/21
>  
> 12:50:38"}
> Tue Sep 21 12:50:38 2021 daemon.info loginotify[17375]: 
> {"directory":"/etc/config/","file":".system.uci-fEGgbp","event":"CLOSE_WRITE,CLOSE","time":"09/21/21
>  
> 12:50:38"}
> Tue Sep 21 12:50:38 2021 daemon.info loginotify[17375]: 
> {"directory":"/etc/config/","file":"system","event":"CLOSE_WRITE,CLOSE","time":"09/21/21
>  
> 12:50:38"}
> Tue Sep 21 12:50:38 2021 daemon.info loginotify[17375]: 
> {"directory":"/etc/config/","file":".system.uci-fEGgbp","event":"ATTRIB","time":"09/21/21
>  
> 12:50:38"}
> 
> I am bothered about the file '.system.uci-fEGgbp'?
The extra temporary file in /etc/config is intentional. uci is simply
using a common pattern in order to make rewriting config files atomic
and avoid leaving behind empty or incomplete files in case of a system
crash during commit.
The new file is written under a temporary name, and after a fsync, it is
renamed to the final filename. This only works if the temporary file and
the config file are on the same filesystem, that's why the temp file is
put in /etc/config as well.

- Felix

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


Re: WDS stopped working in 21.02, looking for bug in netifd

2021-09-21 Thread Felix Fietkau
On 2021-09-20 22:56, Daniel Haid wrote:
> Felix, I took the last openwrt snapshot and compiled netifd from master 
> with your patch applied and installed it.
> 
> Result:
> After boot wlan0.sta1 was DOWN.
> After "/etc/init.d/network restart" it was UP and the connection worked!
> After another "/etc/init.d/network restart" it was DOWN again.
> After reboot it was UP again and working.
> After "/etc/init.d/network restart" it was DOWN again.
Can you please send me the config that you're using? I'd like to try to
reproduce it myself.

Thanks,

- Felix

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


Re: WDS stopped working in 21.02, looking for bug in netifd

2021-09-20 Thread Felix Fietkau
On 2021-09-20 16:46, Daniel Haid wrote:
> I have continued investigating.
> 
> After all, it seems that the interface being down is just a symptom.
> 
> I summarize my current findings:
> 
> With the 21.02 netifd version, there seems to be a bug concerting WDS. 
> The bug has the following effect:
> 
> I have openwrt 21.02 running on one system running as WDS AP and another 
> one running as WDS Client. The WDS Client is running and its 
> configuration never changed.
> 
> After booting the WDS AP, there are two possibilities for in what state 
> the system can be, I call them NON-WORKING and WORKING. The probability 
> seems to be about 50% to be in one or the other state after booting.
> 
> To find out in which state I am after booting, I look for the interface 
> wlan0.sta1. If it is UP, then we are in state WORKING. If it is DOWN, 
> then we are in state NON-WORKING.
> 
> Using ping, in state WORKING, the AP can reach the Client. In state 
> NON-WORKING, the AP cannot reach the Client.
> 
> In state WORKING, the interface wlan0.sta1 can be set to DOWN and UP 
> again, and the AP can then again ping the Client, but only after waiting 
> about 4 minutes for the Client to reconnect to the AP (in my last mail, 
> I wrote that it did not work, but I just did not wait long enough).
> 
> In state NON-WORKING, I can set the interface wlan0.sta1 to UP, but this 
> will not help. The ip command will show that the interface is UP, but 
> the AP can not ping the Client, no matter how long I wait after setting 
> the state to UP.
> 
> If I turn off the Client, wait for the interface wlan0.sta1 to be 
> removed on the AP, and then restart the Client, then the interface 
> wlan0.sta1 will be created again, in state DOWN. Everything is again as 
> in the state NON-WORKING.
> 
> To reliably reach the state NON-WORKING, run "/etc/init.d/network restart".
> 
> Changing the function wireless_interface_handle_link such that it does 
> not call interface_handle_link when it is called from 
> wireless_device_hotplug_event fixes the bug.
> 
> But I do not understand what is happening.
> 
> I am not subscribed to the list; please send Cc to me.
Please test if applying this change to netifd fixes the issue.

Thanks,

- Felix

---
--- a/wireless.c
+++ b/wireless.c
@@ -328,14 +328,14 @@ static void wireless_interface_handle_link(struct 
wireless_interface *vif, const
if (!ifname)
ifname = vif->ifname;
 
-   if (up) {
+   if (up && ifname != vif->ifname) {
struct device *dev = device_get(ifname, 2);
if (dev) {
dev->wireless_isolate = vif->isolate;
dev->wireless_proxyarp = vif->proxyarp;
dev->wireless = true;
dev->wireless_ap = vif->ap_mode;
-   dev->bpdu_filter = dev->wireless_ap && ifname == 
vif->ifname;
+   dev->bpdu_filter = dev->wireless_ap;
}
}
 
@@ -793,6 +793,13 @@ wireless_interface_init_config(struct wireless_interface 
*vif)
if ((cur = tb[VIF_ATTR_NETWORK]))
vif->network = cur;
 
+   cur = tb[VIF_ATTR_MODE];
+   if (cur)
+   vif->ap_mode = !strcmp(blobmsg_get_string(cur), "ap");
+
+   if (!vif->ap_mode)
+   return;
+
cur = tb[VIF_ATTR_ISOLATE];
if (cur)
vif->isolate = blobmsg_get_bool(cur);
@@ -801,9 +808,6 @@ wireless_interface_init_config(struct wireless_interface 
*vif)
if (cur)
vif->proxyarp = blobmsg_get_bool(cur);
 
-   cur = tb[VIF_ATTR_MODE];
-   if (cur)
-   vif->ap_mode = !strcmp(blobmsg_get_string(cur), "ap");
 }
 
 /* vlist update call for wireless interface list */

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


Re: Broken ARP broadcast on master (commit 0f688797)

2021-09-06 Thread Felix Fietkau

> On 6. Sep 2021, at 02:03, David Bauer  wrote:
> 
> Hi Felix,
> 
> updating my Wireless APs (ath9k+ath10k / mt7603+mt7915) broke ARP broadcast 
> delivery to clients
> connected to the radios with SW rate control.
> 
> Bisecting this problem revealed commit 0f688797 ("mac80211: add missing 
> change for encap offload
> on devices with sw rate control") introduces this problem, reverting it 
> restored the desired
> functionality.
> 
> Can you have a look what is going wrong here?


Hi David,

Please try the latest version, it should resolve the issue

Thanks,
- Felix

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


Re: [PATCH] Revert "initd: fix off-by-one error in mkdev.c"

2021-08-31 Thread Felix Fietkau


On 2021-08-31 10:25, vinc...@systemli.org wrote:
> From: Nick Hainke 
> 
> This reverts commit 8eb1d783cca6e0d501dd3a2f94262ffc36ae6482.
> 
> This line reads a symbolic link into the string buffer "buf".
>   len = readlink(buf2, buf, sizeof(buf));
> The commit replaced now
>   buf[len] = 0;
> with
>   buf[sizeof(buf) - 1] = '\0';
> 
> However, that does not work since readlink does not null-terminate
> the string written into "buf" and  "buf[len] = 0" was used for that.
> 
> What happens if the buffer is to small?
> "If the buf argument is not large enough to contain the link content,
> the first bufsize bytes shall be placed in buf."
> (Source: 
> https://pubs.opengroup.org/onlinepubs/009695399/functions/readlink.htm)
That revert adds back the original off-by-one error, since len will be
sizeof(buf) in case of an undersized buffer.
I agree that 'buf[len] = 0' is correct, but only if you also use
sizeof(buf)-1 as size argument in the readlink() call.

- Felix

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


Re: [PATCH] uhttpd: add config option for json_script

2021-08-24 Thread Felix Fietkau
On 2021-08-20 15:11, Stijn Tintel wrote:
> Add a config option for json_script instead of unconditionally including
> all json files in /etc/uhttpd in every uhttpd instance. This makes it
> possible to configure a single instance with an unconditional redirect,
> which is currently not possible as it would render all other uhttpd
> instances unusable.
> 
> Signed-off-by: Stijn Tintel 
Acked-by: Felix Fietkau 

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


Re: [PATCH 21.02] Revert "netifd: update to the latest version"

2021-07-26 Thread Felix Fietkau
On 2021-07-26 17:09, Petr Štetiar wrote:
> Petr Štetiar  [2021-07-26 17:01:32]:
> 
>> This reverts commit 089efd61e9a6cdc0ea39c184d37bc8ebbe03175c as it
>> breaks LAN network on at least mvebu/turris-omnia device.
>> 
>> Confirmed-by: Josef Schlehofer 
>> Signed-off-by: Petr Štetiar 
>> ---
>> 
>> I've discovered this today in the morning in my testbed where I perform daily
>> runtime tests of initramfs images on several devices, where images/tests for
>> snapshot and 21.02-SNAPSHOT failed on turris-omnia device.
> 
> Sorry, forgot to provide more detials. The problem is with initialization of
> LAN network, where the interface readiness is tested by following command:
> 
>  ifstatus lan | jsonfilter -qe "@.up"
> 
> then the LAN network availability is tested with simple `ping -c1 192.168.1.2`
> command which timeouts. This is pristine image with no config changes, so the
> device has 192.168.1.1/24 network config and the other end 192.168.1.2/24.
> 
> Console logs are available:
> 
>  snapshot 
> https://gitlab.com/ynezz/openwrt-device-runtime-testing/-/jobs/1451323563/artifacts/file/console_turris-omnia-initramfs.txt
>  21.02
> https://gitlab.com/ynezz/openwrt-device-runtime-testing/-/jobs/1451222403/artifacts/file/console_turris-omnia-initramfs.txt
Fixed the regression in master and 21.02, sorry about the mess.
For some reason, it did not trigger in my configurations...

- Felix

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


Re: OpenWrt 21.02 status

2021-07-17 Thread Felix Fietkau


On 2021-07-17 17:45, Hauke Mehrtens wrote:
> Hi,
> 
> In general the 21.02-rc3 looks good, but we still have some problems.
> 
> Currently we still have these problem:
> 
> - IPv6 broken with flow offloading (according to reports, potentially 
> related to hw flow offloading)
> - PPPoE allegedly broken (according to reports, not fully reproducible, 
> likely related to hw flow offloading too)
>- https://bugs.openwrt.org/index.php?do=details_id=3909
>- https://bugs.openwrt.org/index.php?do=details_id=3835
>- 
> https://forum.openwrt.org/t/21-02-snapshot-ipv4-pppoe-session-keeps-terminating/90552
> - DHCPv6 broken if lease times < 12h chosen
>- 
> https://forum.openwrt.org/t/openwrt-21-02-0-third-release-candidate/99363/137
>- 
> https://forum.openwrt.org/t/openwrt-21-02-0-third-release-candidate/99363/162
> - WDS with bridge-vlan broken (missing backport from master)
I added a netifd backport in fe498dd3f108 (with a fix in f3f70fb9567d)

- Felix

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


Re: [PATCH] ubus: add a private field to event handlers

2021-07-14 Thread Felix Fietkau
On 2021-07-13 13:57, louis_hamp...@sercomm.com wrote:
> 
> This is particularly useful in multithreaded non-C FFI, where
> additional context from where the event handler was registered may be
> critical to ensuring the output of the event is sent to the right
> location.
> 
> Signed-off-by: Louis Hampton 
In order to get the additional context, the normal solution is to embed
struct ubus_event_handler into another data structure, which contains
the extra context and can be accessed via container_of.
Is there a reason why this can't be done in your code as well?

- Felix

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


Re: wrong activation of internal radius server

2021-06-04 Thread Felix Fietkau


Hi Hartmut,

On 2021-06-04 15:22, e9hack wrote:
> OpenWrt has been supporting hostapd's internal radius server for a few days. 
> For several years there has been a line in the init-script that activates the 
> internal radius server too. I think this is wrong (from hostapd.sh):
> 
>   [ -n "$wps_possible" -a -n "$config_methods" ] && {
>   set_default ext_registrar 0
>   set_default wps_device_type "6-0050F204-1"
>   set_default wps_device_name "OpenWrt AP"
>   set_default wps_manufacturer "www.openwrt.org"
>   set_default wps_independent 1
> 
>   wps_state=2
>   [ -n "$wps_not_configured" ] && wps_state=1
> 
>   [ "$ext_registrar" -gt 0 -a -n "$network_bridge" ] && append 
> bss_conf "upnp_iface=$network_bridge" "$N"
> 
> ===>  append bss_conf "eap_server=1" "$N"
I just checked the hostapd WPS README file, and it seems that the
eap_server=1 line is actually required:
https://w1.fi/cgit/hostap/tree/hostapd/README-WPS#n94

- Felix

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


Re: [PATCH V2 netifd] interface: rename "ifname" attribute to "device"

2021-05-25 Thread Felix Fietkau

On 2021-05-24 15:35, Rafał Miłecki wrote:
> From: Rafał Miłecki 
> 
> Interfaces need to be assigned to devices. For that purpose a "device"
> option should be more accurate than "ifname" one.
> 
> For backward compatibility add a temporary config translation.
> 
> Config example:
> 
> config device
>   option name 'br-lan'
>   option type 'bridge'
>   list ports 'lan1'
>   list ports 'lan2'
> 
> config interface 'lan'
>   option device 'br-lan'
>   option proto 'static'
> 
> Signed-off-by: Rafał Miłecki 
I think the code for backwards compatibility would be a lot simpler if
you add both IFACE_ATTR_DEVICE and IFACE_ATTR_IFNAME and initialize
tb[IFACE_ATTR_DEVICE] to tb[IFACE_ATTR_IFNAME] if it's NULL.

- Felix

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


Re: [PATCH netifd] interface: rename "ifname" attribute to "device"

2021-05-22 Thread Felix Fietkau

On 2021-05-17 16:49, Rafał Miłecki wrote:
> From: Rafał Miłecki 
> 
> Interfaces need to be assigned to devices. For that purpose a "device"
> option should be more accurate than "ifname" one.
> 
> For backward compatibility add a temporary config translation.
> 
> Config example:
> 
> config device
>   option name 'lan'
>   option type 'bridge'
>   list ports 'lan1'
>   list ports 'lan2'
> 
> config interface 'home'
>   option device 'lan'
>   option proto 'static'
> 
> Signed-off-by: Rafał Miłecki 
While I agree that the 'device' name would fit better than 'ifname',
the fallback config translation is not enough in this case.
netifd supports adding dynamic interfaces via ubus, where the config
data is provided as blobmsg. You could update the in-tree users of that,
but I don't know if there are any out-of-tree downstream users of this
functionality that you would be breaking with this change.

- Felix

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


Re: netifd: redesigning UCI config & interfaces

2021-05-13 Thread Felix Fietkau
Hi Rafał,

Thanks for the proposal

On 2021-05-13 15:58, Rafał Miłecki wrote:
> Current /etc/config/network design and netifd implementation are quite a
> bit messy:
> 1. There is no clear layer 2 vs. layer 3 distinction
I think aside from a few legacy compatibility hacks (e.g. the infamous
interface option type bridge, which we really should phase out at some
point), there actually is a clear distinction.
The device refers to the underlying layer 2 linux device
The interface refers to a logical layer 3 configuration on top of that,
which may or may not have its own layer 3 netdev.
It may not seem as clear when viewing the whole thing through the lens
of just looking at the individual linux netdevs, but that's because it's
different data model.

> 2. UCI sections are inconsistent
> 3. For some setups there are few ways of defining them
> 4. A lot of netifd states are implicit (magic behaviour)
> 5. It's really hard to handle all above with a simple & clear UI
I think this strongly depends on your definition of 'simple' and the
level of abstraction. If your definition of a simple UI is simply a
fancier display of netdev stats and individual linux netdev
configurations, then your statement makes some sense to me.

For something more high level than that, I don't think it makes things
any easier.

> I'm working on network & netifd cleanup and wanted to describe my plans.
> Please review my following ideas.
> 
> 
> *** ubus objects ***
> 
> Linux uses interfaces (e.g. eth0, br-lan) for laver 2 devices as well as
> layer 3 interfaces. Tools like ifconfig, brctl, ip work on top of such
> interfaces.
> 
> OpenWrt tools ifup & ifdown are expected to work on Linux interfaces
> just like ifconfig. They call ubus objects methods "up" & "down".
> 
> That's why we need network.interface. ubus object for every used
> Linux interface. No matter if that Linux interface is layer 2 or layer
> 3. Even if Linux interface "lan" is a bridge (layer 2) we still need a
> way to bring it up & down (so network.interface.lan is required).
> 
> Adding a new set of ubus objects (like network.device.lan) would result
> in duplication. We don't want that. E.g. both commands would do the same
> 1. ubus call network.device.lan up
> 1. ubus call network.interface.lan up
In my opinion, forcing the interface to have the same name as the
explicitly specified l3 device is a big step back in usability.

In the current default config, interface sections are named after their
logical function, e.g. lan or wan. Users can use ifup/ifdown wan having
to worry about whether their wan device is a DSA port called wan, a
pppoe interface called pppoe-wan, or a switch VLAN like switch0.1,
eth0.1 or a named VLAN device like sw0-wan.

It also makes it easy to bring up/down individual logical configurations
on top of the same device.

Your proposal wouldn't just affect what users need to write in the
network config. We also have other services that refer to the logical
configurations (and then look up the actual linux device later).

Here's a simple example of how your proposed change would make things a
lot more difficult for users and/or the UI:

Let's assume a more modern target already using 'wan' as netdev for the
WAN port via DSA. Let's also assume that it is currently configured to
use DHCP on WAN and has a few services running that refer to the wan
interface. Now the user has moved the device somewhere else and wants to
use PPPoE for WAN.
This means that the user doesn't just have to add an extra device
section and change the 'device' option for the ifname, they also have to
go through all config sections of services potentially depending on the
old netdev name and rewrite those.

> *** devices (layer 2) ***
> 
> We should have designed UCI sections for layer 2 devices. Use UCI
> section types instead of "option type foo". Reasons:
> 1. Each device requires different handling
> 2. Options have different meaning depending on device type
> 
> While switching to new UCI sections we should also cleanup some UCI
> options and behaviour.
> 
> Example:
> 
> config bridge
>   option name 'foo'
>   list ports 'lan1'
>   list ports 'lan2'
> 
> Above you can see new UCI section for bridge device with "ports" list
> instead of "ifname" string.
> 
> Above config should result in creating "foo" bridge Linux interface and
> network.interface.foo ubus object for bringing it up / down.
> 
> Other devices would work similarly, e.g. "config vlan" for vlan.
I'm not sure if this part is really worth the effort or makes things
better in any meaningful way. Even if you get rid of most of the type
dependent sets of accepted options, some will likely remain for tunnel
devices, and interfaces will also still have varying options depending
on the proto.

- Felix

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


Re: FRAG Attacks (new vuln for wifi)

2021-05-13 Thread Felix Fietkau


On 2021-05-13 11:10, Mirko Parthey wrote:
> On Thu, May 13, 2021 at 05:57:01AM +0200, Felix Fietkau wrote:
>> >> > Regarding ath10k-ct, what kernel-versions of the ath10k-ct driver need 
>> >> > to be patched?
>> >> > Is 4.19 the oldest that owrt will consume?
>> >> I think so. 4.19 is used by OpenWrt 19.07 and I don't think we'll update
>> >> anything older than that.
>> >
>> > So, to summarise, the 4.19, 5.4 and 5.10 branch kernels are currently in 
>> > use.
>> I don't see where we're using the ath10k-ct 5.4 kernel at all. It should
>> be just 4.19 and 5.10.
> 
> I am just commenting from the sideline, but I see other Kernel versions:
> 
> OpenWrt 19.07 = Kernel 4.14
> OpenWrt 21.02 = Kernel 5.4
> OpenWrt master = mostly still Kernel 5.4, but moving towards 5.10
We're talking about the version of the kernel tree used to create the
ath10k-ct source, not the kernel version used by OpenWrt.

- Felix

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


Re: FRAG Attacks (new vuln for wifi)

2021-05-12 Thread Felix Fietkau


On 2021-05-13 01:45, Nick Lowe wrote:
>> > Regarding ath10k-ct, what kernel-versions of the ath10k-ct driver need to 
>> > be patched?
>> > Is 4.19 the oldest that owrt will consume?
>> I think so. 4.19 is used by OpenWrt 19.07 and I don't think we'll update
>> anything older than that.
> 
> So, to summarise, the 4.19, 5.4 and 5.10 branch kernels are currently in use.
I don't see where we're using the ath10k-ct 5.4 kernel at all. It should
be just 4.19 and 5.10.

- Felix

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


Re: FRAG Attacks (new vuln for wifi)

2021-05-12 Thread Felix Fietkau


On 2021-05-12 19:13, Ben Greear wrote:
> On 5/12/21 9:00 AM, Felix Fietkau wrote:
>> 
>> Hi,
>> 
>> On 2021-05-12 01:34, Paul D wrote:
>>> https://www.fragattacks.com/
>>>
>>> https://lore.kernel.org/linux-wireless/20210511180259.159598-1-johan...@sipsolutions.net/
>> I've merged those fixes in openwrt commit 025bd93f36c9.
>> After some testing in master, we should backport them soon.
>> 
>> ath10k-ct will need to be fixed too, and I plan on pushing an mt76 fix
>> very soon. Not sure what other drivers may need fixing...
>> 
>> - Felix
>> 
> 
> Regarding ath10k-ct, what kernel-versions of the ath10k-ct driver need to be 
> patched?
> Is 4.19 the oldest that owrt will consume?
I think so. 4.19 is used by OpenWrt 19.07 and I don't think we'll update
anything older than that.

- Felix

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


Re: FRAG Attacks (new vuln for wifi)

2021-05-12 Thread Felix Fietkau


Hi,

On 2021-05-12 01:34, Paul D wrote:
> https://www.fragattacks.com/
> 
> https://lore.kernel.org/linux-wireless/20210511180259.159598-1-johan...@sipsolutions.net/
I've merged those fixes in openwrt commit 025bd93f36c9.
After some testing in master, we should backport them soon.

ath10k-ct will need to be fixed too, and I plan on pushing an mt76 fix
very soon. Not sure what other drivers may need fixing...

- Felix

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


Re: [PATCH] build: fix regression for kernels < 5.10

2021-04-13 Thread Felix Fietkau


On 2021-04-13 14:22, Sebastian Kemper wrote:
> This fixes a regression introduced with commit
> 5ed1e5140a80558ab47fd70410ae3242bed5becf ("build: build kernel image
> before building modules/packages").
> 
> Before this commit the make target would always include "modules",
> resulting in a MODPOST and a complete Module.symvers file. Since this
> commit a MODPOST of the kernel modules is not guaranteed for kernels <
> 5.10. This results in some broken SDKs in which external packages that
> depend on exported symbols from kernel modules fail to compile.
Why is it not enough to do this in the CompileModules step?

> Adding "modules" back to the calls to the CompileImage defines fixes the
> regression. For kernels > 5.10 this is not needed, but it doesn't cause
> any harm either.
Why is >5.10 not affected? Can we backport the fix? I'd like to avoid
adding extra unnecessary build step that slow down running make
target/install.

> Tested with kernels 5.4.x and 5.10.x.
> 
> Signed-off-by: Sebastian Kemper 
> ---
>  include/kernel-defaults.mk | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/include/kernel-defaults.mk b/include/kernel-defaults.mk
> index 4b0b136a03..1b3b4497a2 100644
> --- a/include/kernel-defaults.mk
> +++ b/include/kernel-defaults.mk
> @@ -147,12 +147,17 @@ define Kernel/CopyImage
>   }
>  endef
> 
> +# Always add "modules" so a proper Module.symvers file is written that
> +# also contains symbols from the kernel modules. Without these symbols
> +# external packages that depend on exported symbols from kernel modules
> +# will fail to build.
>  define Kernel/CompileImage/Default
>   rm -f $(TARGET_DIR)/init
> - +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if 
> $(KERNELNAME),$(KERNELNAME),all)
> + +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if 
> $(KERNELNAME),$(KERNELNAME),all) modules
>   $(call Kernel/CopyImage)
>  endef
> 
> +# Here as well, always add "modules", see comment above.
>  ifneq ($(CONFIG_TARGET_ROOTFS_INITRAMFS),)
>  define Kernel/CompileImage/Initramfs
>   $(call Kernel/Configure/Initramfs)
> @@ -173,7 +178,7 @@ endif
>  # ?  $(if $(CONFIG_TARGET_INITRAMFS_COMPRESSION_LZ4),)
>   $(if 
> $(CONFIG_TARGET_INITRAMFS_COMPRESSION_ZSTD),$(STAGING_DIR_HOST)/bin/zstd -T0 
> -f -o $(KERNEL_BUILD_DIR)/initrd.cpio.zstd $(KERNEL_BUILD_DIR)/initrd.cpio)
>  endif
> - +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if 
> $(KERNELNAME),$(KERNELNAME),all)
> + +$(KERNEL_MAKE) $(KERNEL_MAKEOPTS_IMAGE) $(if 
> $(KERNELNAME),$(KERNELNAME),all) modules
Why do this for initramfs as well?

- Felix

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


Re: [PATCH] kernel: disable crypto-hw-ccp on BCM4908

2021-04-13 Thread Felix Fietkau

On 2021-04-13 11:46, Rafał Miłecki wrote:
> From: Rafał Miłecki 
> 
> CONFIG_CRYPTO_DEV_SP_CCP depends on DMA_ENGINE which depends on
> DMADEVICES=y which bcm4908 doesn't need and doesn't use at this point.
> 
> Trying to build this package on bcm4908 was causing:
> ERROR: module 
> 'build_dir/target-aarch64_cortex-a53_musl/linux-bcm4908_generic/linux-5.4.110/drivers/crypto/ccp/ccp-crypto.ko'
>  is missing.
> 
> Signed-off-by: Rafał Miłecki 
> ---
>  package/kernel/linux/modules/crypto.mk | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/package/kernel/linux/modules/crypto.mk 
> b/package/kernel/linux/modules/crypto.mk
> index cbaa8d3ce5..0f696c3129 100644
> --- a/package/kernel/linux/modules/crypto.mk
> +++ b/package/kernel/linux/modules/crypto.mk
> @@ -319,7 +319,8 @@ $(eval $(call KernelPackage,crypto-hmac))
>  
>  define KernelPackage/crypto-hw-ccp
>TITLE:=AMD Cryptographic Coprocessor
> -  DEPENDS:=+kmod-crypto-authenc +kmod-crypto-hash +kmod-crypto-manager 
> +kmod-random-core +kmod-crypto-sha1 +kmod-crypto-sha256 +kmod-crypto-rsa
> +  DEPENDS:=@!TARGET_bcm4908
> +  DEPENDS+=+kmod-crypto-authenc +kmod-crypto-hash +kmod-crypto-manager 
> +kmod-random-core +kmod-crypto-sha1 +kmod-crypto-sha256 +kmod-crypto-rsa
Wouldn't it make a lot more sense to depend on @TARGET_x86_64 instead?

- Felix

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


Re: Regression in auto-mounting mtd squashfs partitions

2021-04-10 Thread Felix Fietkau
On 2021-04-10 16:33, Daniel Golle wrote:
> On Sat, Apr 10, 2021 at 04:04:28PM +0200, Felix Fietkau wrote:
>> Hi Daniel,
>> 
>> it seems to me that your commit 2809d744 ("kernel: support FIT
>> partition parser on mtdblock devices") is causing a regression in
>> mounting squashfs rootfs when CONFIG_FIT_PARTITION is enabled.
>> The following workaround fixes it, but maybe you can make a better fix,
>> since you're more familiar with the code.
> 
> No, I just forgot that we are also making assumptions about the mtdblock
> device minor number in our own patches...
> The fix looks good to me and certainly won't break devices that are
> actually making of the new external-data FIT images (as ROOT_DEV is
> then set by the FIT block partition parser in a generic way and no
> 'rootfs' MTD partition exists in that case).
> 
> Acked-by: Daniel Golle 
Fix pushed, thanks for the fast reply.

- Felix

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


Regression in auto-mounting mtd squashfs partitions

2021-04-10 Thread Felix Fietkau
Hi Daniel,

it seems to me that your commit 2809d744 ("kernel: support FIT
partition parser on mtdblock devices") is causing a regression in
mounting squashfs rootfs when CONFIG_FIT_PARTITION is enabled.
The following workaround fixes it, but maybe you can make a better fix,
since you're more familiar with the code.

- Felix

--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -699,9 +699,13 @@ int add_mtd_device(struct mtd_info *mtd)
if (!strcmp(mtd->name, "rootfs") &&
IS_ENABLED(CONFIG_MTD_ROOTFS_ROOT_DEV) &&
ROOT_DEV == 0) {
+   unsigned int index = mtd->index;
pr_notice("mtd: device %d (%s) set to be root filesystem\n",
  mtd->index, mtd->name);
-   ROOT_DEV = MKDEV(MTD_BLOCK_MAJOR, mtd->index);
+#ifdef CONFIG_FIT_PARTITION
+   index <<= 1;
+#endif
+   ROOT_DEV = MKDEV(MTD_BLOCK_MAJOR, index);
}
 
return 0;





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


Re: [PATCH] kernel: drop the conntrack rtcache patch

2021-04-09 Thread Felix Fietkau
On 2021-04-09 00:57, Rui Salvaterra wrote:
> It's in backports-5.4, but it wasn't ever merged. Upstream followed another
> approach, with flow offloading, which has much better performance. Drop this
> obsolete patch and refresh the kernel patches.
> 
> Signed-off-by: Rui Salvaterra 
Acked-by: Felix Fietkau 

- Felix

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


Re: [PATCH] openssl: don't rename a file with quilt

2021-03-26 Thread Felix Fietkau


On 2021-03-26 22:39, Eneas U de Queiroz wrote:
> On Fri, Mar 26, 2021 at 4:28 PM Rosen Penev  wrote:
>>
>> On Fri, Mar 26, 2021 at 5:55 AM Eneas U de Queiroz
>>  wrote:
>> >
>> > On Fri, Mar 26, 2021 at 6:26 AM Rosen Penev  wrote:
>> > > +ifeq ($(QUILT),)
>> > > +   mv $(PKG_BUILD_DIR)/crypto/engine/eng_devcrypto.c 
>> > > $(PKG_BUILD_DIR)/engines/e_devcrypto.c
>> > > +endif
>> >
>> > This will break compilation with QUILT, as the rename will never
>> > happen then.  You're using this strategy with other packages, so I
>> > won't mention them individually, but this applies to all.
>> > I would handle it at the patch level by removing the old file and
>> > creating the new one.
>> It doesn't break quilt as the rename happens separately. Note that the
>> patches were modified to refer to the old name.
> It breaks 'make package/openssl/compile QUILT=1', because the 'mv'
> command will never run when QUILT is not empty.  You can't run the
> 'mv' line with quilt because the patches are not applied in
> Build/Prepare.  However, you must ensure it is run later, or the
> package will not compile with QUILT=1.  I can point you to
> openwrt/packages#14894 to see why you can't just skip running stuff
> when QUILT is defined.
I fully agree with Eneas here (though I don't like his patch for this
issue either). Here's a way to fix this:

include/package-defaults.mk has this:

  define Build/Prepare/Default
$(PKG_UNPACK)
[ ! -d ./src/ ] || $(CP) ./src/. $(PKG_BUILD_DIR)
$(Build/Patch)
  endef

You can adjust it to define this in the package Makefile:

define Build/Prepare
$(PKG_UNPACK)
[ ! -d ./src/ ] || $(CP) ./src/. $(PKG_BUILD_DIR)
mv $(PKG_BUILD_DIR)/crypto/engine/eng_devcrypto.c 
$(PKG_BUILD_DIR)/engines/e_devcrypto.c
$(Build/Patch)
endef

- Felix

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


Re: [RFC PATCH v2 0/1] Introduce UCI support for configuring DSA VLAN filter rules

2021-03-26 Thread Felix Fietkau


On 2021-03-26 09:55, Martin Schiller wrote:
> On 2021-03-26 09:42, Felix Fietkau wrote:
>> On 2021-03-26 09:34, Martin Schiller wrote:
>>> On 2020-07-24 19:13, Felix Fietkau wrote:
>>>> On 2020-07-24 18:44, Jo-Philipp Wich wrote:
>>>>> Hi Felix,
>>>>> 
>>>>>> [...]
>>>>>> 
>>>>>> For a simple default config, you could have this:
>>>>>> 
>>>>>> # network
>>>>>> config device
>>>>> option type bridge  # I assume this is needed as well
>>>>>>  option name switch0
>>>> Correct.
>>>> 
>>>>>> config bridge-vlan
>>>>>>  option vlan 1
>>>>>>  option ports "lan1 lan2 lan3 lan4"
>>>>>> 
>>>>>> config interface lan
>>>>>>  option ifname switch0.1
>>>>>> 
>>>>>> 
>>>>>> # wireless
>>>>>> 
>>>>>> config wifi-iface
>>>>>>  option network lan
>>>>>> 
>>>>>> 
>>>>>> In this case, wlan0 would be added to switch0 and set to VLAN 1
>>>>>> untagged
>>>>>> by default.
>>>>>> 
>>>>>> If you want it on VLAN 10 tagged/PVID instead, you could do:
>>>>>>  option network-vlan "10:t*"
>>>>>> 
>>>>>> 
>>>>>> What do you think?
>>>>> 
>>>>> I did think about it some more, also in context of a LuCI
>>>>> implementation and
>>>>> the special role of wifi and I am convinced now that this approach
>>>>> generally
>>>>> makes sense.
>>>>> 
>>>>> However for the vlan I wonder if we should simply use "option vid 
>>>>> 10"
>>>>> since
>>>>> setting anything besides an egress untagged pvid does not make sense
>>>>> for wifi.
>>>> I think more complex VLAN settings make sense for WDS if you want to
>>>> carry multiple networks over the link.
>>>> 
>>>>> So your second example above would become:
>>>>> 
>>>>>   config wifi-iface
>>>>> option network lan
>>>>> option vid 10  # instead of inheriting vid 1, use 10 as pvid
>>>>> 
>>>>> 
>>>>> Also, just to clarify... assuming a:
>>>>> 
>>>>>   config interface foo
>>>>> option ifname somevlanbridge0.456
>>>>> 
>>>>> and an wifi iface without an explicit vid override:
>>>>> 
>>>>>   config wifi-iface
>>>>> option network foo
>>>>> 
>>>>> ... we would inherit vid 456 and set as pvid, right? Or are we are
>>>>> always
>>>>> going to default to 1?
>>>> It would inherit 456 to keep it in sync with the VLAN based network.
>>>> 
>>> 
>>> Is this functionality already integrated?
>>> I am testing with a xrx200 based system with the DSA mainline driver 
>>> and
>>> a wifi interface and have the problem that the wlan0 interface is 
>>> added
>>> to the bridge switch0 but the bridge vlan configuration for the wlan0
>>> interface is not set.
>> It's handled differently now.
>> 
>> You can set lan's ifname to switch0.1 (without option type bridge) and
>> use 'option network lan' in the wifi-iface. It will detect that the lan
>> ifname is a vlan on top of a vlan-filtering bridge and will add wlan0 
>> to
>> switch0 and make it a member of lan's vlan.
>> 
> 
> Hmmm... I think that's what I've alread done. Here is my config:
> 
> network:
> -
> config interface 'lan'
>   option proto 'static'
>   option ipaddr '192.168.X.Y'
>   option netmask '255.255.255.0'
>   option ifname 'switch0.1'
> 
> config device
>   option type 'bridge'
>   option name 'switch0'
>   list ifname 'lan1'
>   list ifname 'lan2'
>   list ifname 'lan3'
>   list ifname 'lan4'
> 
> config bridge-vlan
>   option device 'switch0'
>   option vlan '1'
>   list ports 'lan1:u*'
>   list ports 'lan2:u*'
>   list ports 'lan3:u*'
>   list ports 'lan4:u*'
> 
> wireless:
> --
> config wifi-iface 'default_radio0'
>   option device 'radio0'
>   option mode 'ap'
>   option encryption 'psk2'
>   option ssid 'TETS-AP'
>   option network 'lan'
>   option key 'xxx'
>   option wpa_disable_eapol_key_retries '1'
> 
> 
> Did I forget anything?
Looks right to me. I'll see if I can find the time to reproduce this.
You're using a recent version of OpenWrt, right?

- Felix

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


Re: [RFC PATCH v2 0/1] Introduce UCI support for configuring DSA VLAN filter rules

2021-03-26 Thread Felix Fietkau


On 2021-03-26 09:34, Martin Schiller wrote:
> On 2020-07-24 19:13, Felix Fietkau wrote:
>> On 2020-07-24 18:44, Jo-Philipp Wich wrote:
>>> Hi Felix,
>>> 
>>>> [...]
>>>> 
>>>> For a simple default config, you could have this:
>>>> 
>>>> # network
>>>> config device
>>> option type bridge  # I assume this is needed as well
>>>>option name switch0
>> Correct.
>> 
>>>> config bridge-vlan
>>>>option vlan 1
>>>>option ports "lan1 lan2 lan3 lan4"
>>>> 
>>>> config interface lan
>>>>option ifname switch0.1
>>>> 
>>>> 
>>>> # wireless
>>>> 
>>>> config wifi-iface
>>>>option network lan
>>>> 
>>>> 
>>>> In this case, wlan0 would be added to switch0 and set to VLAN 1 
>>>> untagged
>>>> by default.
>>>> 
>>>> If you want it on VLAN 10 tagged/PVID instead, you could do:
>>>>option network-vlan "10:t*"
>>>> 
>>>> 
>>>> What do you think?
>>> 
>>> I did think about it some more, also in context of a LuCI 
>>> implementation and
>>> the special role of wifi and I am convinced now that this approach 
>>> generally
>>> makes sense.
>>> 
>>> However for the vlan I wonder if we should simply use "option vid 10" 
>>> since
>>> setting anything besides an egress untagged pvid does not make sense 
>>> for wifi.
>> I think more complex VLAN settings make sense for WDS if you want to
>> carry multiple networks over the link.
>> 
>>> So your second example above would become:
>>> 
>>>   config wifi-iface
>>> option network lan
>>> option vid 10  # instead of inheriting vid 1, use 10 as pvid
>>> 
>>> 
>>> Also, just to clarify... assuming a:
>>> 
>>>   config interface foo
>>> option ifname somevlanbridge0.456
>>> 
>>> and an wifi iface without an explicit vid override:
>>> 
>>>   config wifi-iface
>>> option network foo
>>> 
>>> ... we would inherit vid 456 and set as pvid, right? Or are we are 
>>> always
>>> going to default to 1?
>> It would inherit 456 to keep it in sync with the VLAN based network.
>> 
> 
> Is this functionality already integrated?
> I am testing with a xrx200 based system with the DSA mainline driver and
> a wifi interface and have the problem that the wlan0 interface is added
> to the bridge switch0 but the bridge vlan configuration for the wlan0
> interface is not set.
It's handled differently now.

You can set lan's ifname to switch0.1 (without option type bridge) and
use 'option network lan' in the wifi-iface. It will detect that the lan
ifname is a vlan on top of a vlan-filtering bridge and will add wlan0 to
switch0 and make it a member of lan's vlan.

- Felix

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


Re: [PATCH v2] ubusd: convert tx_queue to linked list

2021-03-25 Thread Felix Fietkau


On 2021-03-25 21:43, Arnout Vandecappelle wrote:
> 
> 
> On 25/03/2021 10:48, Felix Fietkau wrote:
>> 
>> On 2021-03-24 13:27, Arnout Vandecappelle (Essensium/Mind) wrote:
>>> ubusd maintains a per-client tx_queue containing references to message
>>> buffers that have not been sent yet (due to the socket blocking). This
>>> is a fixed-size, 64-element queue.
>>>
>>> When more than 64 elements are queued, subsequent elements are simply
>>> dropped. Thus, a client that is waiting for those messages will block
>>> indefinitely. In particular, this happens when more than +- 250 objects
>>> are registered on the bus and either "ubus list" or "ubus wait_for" is
>>> called. The responses to these requests consist of a message buffer per
>>> object. Since in practice, ubusd will not yield between the sends of
>>> these message buffers, the client has no time to process them and
>>> eventually the output socket blocks. After 64 more objects, the rest is
>>> dropped, including the final message that indicates termination. Thus,
>>> the client waits indefinitely for the termination message.
>>>
>>> To solve this, turn the tx_queue into a variable-sized linked list
>>> instead of a fixed-size queue.
>> In order to reduce memory allocation overhead, I would propose the
>> following:
>> 
>> struct ubus_msg_backlog {
>>  struct ubus_msg_backlog *next;
>>  struct ubus_msg_buf *msg[UBUSD_CLIENT_BACKLOG];
>>  int tail;
>> };
> 
>  This saves space by open-coding a single-linked list rather than using the
> normal double-linked list. This comes at the cost of iterating over the entire
> list in order to append to it.
> 
>  It also saves space by dropping the offset member. But for that, we don't 
> need
> to go to this extra structure.
> 
>  Applying those to "optimisations" to struct ubus_msg_buf_list would make that
> one 8 bytes (compared to 16 now). So that's 8 bytes per queued buffer, in
> addition to the 24 bytes of struct ubus_msg_buf and the actual buffer itself,
> which you can expect to be hundreds of bytes.
> 
>  struct ubus_msg_backlog is 24 bytes (assuming UBUSD_CLIENT_BACKLOG is 4),
> that's 6 bytes per element. So not much gained here. Increasing
> UBUSD_CLIENT_BACKLOG will decrease the overhead for large backlog, but gives a
> much bigger overhead for smaller backlog. So not a clear win.
UBUSD_CLIENT_BACKLOG is currently 32. I'm not so much counting bytes for
the optimization, I'm more interested in reducing the number of memory
allocations. This helps reduce memory allocator overhead and fragmentation.

>  Finally, the backlog should normally be empty. It's pretty unusual for a ubus
> message to take more then one write to transfer.
> 
>  In conclusion:
> - I think your proposal doesn't save much;
> - and it complicates things quite a bit;
> - in addition, the single-linked list potentially poses significant time 
> overhead.
With a reasonable upper limit for the maximum number of messages and the
current UBUSD_CLIENT_BACKLOG of 32, the time overhead for walking the
pointers is actually insignificant. If it ever becomes significant, we
can simply add a tail pointer to struct ubus_client and still save space.
I also expect it to be less than the malloc overhead for lots of
single-message list entries.

>  If you want to save a few bytes, it does make sense to move the offset back 
> to
> struct ubus_client.
> 
>  If you *really* want to save space, and time as well, it would be better to
> optimise ubusd_handle_lookup. That currently sends a separate, relatively 
> small,
> message for every object. The overhead of the struct ubus_msg_buf dwarfs the
> overhead of struct ubus_msg_buf_list by quite a lot, and in addition there's
> overhead on the wire as well. It shouldn't be too hard to change 
> ubus_lookup_cb
> to handle a list rather than a single object.
> 
>  Maybe I should have gone down that path. I hadn't thought of it at the time.
I think that's a good idea.
>>> The txq_off variable was used to keep track of which part of the current
>>> message was already written, in case only a partial write was possible.
>>> We take this opportunity to also move that variable under the
>>> ubus_msg_buf_list structure (and rename it to "offset", and change its
>>> type to size_t). This makes it clearer that it is related to that
>>> particular queued message and not to the queue as a whole.
>>>
>>> Note that this infinite tx_queue opens the door to a DoS attack. You can
>>> open a client and a server connection, then send messages from the
>

Re: [PATCH v2] ubusd: convert tx_queue to linked list

2021-03-25 Thread Felix Fietkau


On 2021-03-24 13:27, Arnout Vandecappelle (Essensium/Mind) wrote:
> ubusd maintains a per-client tx_queue containing references to message
> buffers that have not been sent yet (due to the socket blocking). This
> is a fixed-size, 64-element queue.
> 
> When more than 64 elements are queued, subsequent elements are simply
> dropped. Thus, a client that is waiting for those messages will block
> indefinitely. In particular, this happens when more than +- 250 objects
> are registered on the bus and either "ubus list" or "ubus wait_for" is
> called. The responses to these requests consist of a message buffer per
> object. Since in practice, ubusd will not yield between the sends of
> these message buffers, the client has no time to process them and
> eventually the output socket blocks. After 64 more objects, the rest is
> dropped, including the final message that indicates termination. Thus,
> the client waits indefinitely for the termination message.
> 
> To solve this, turn the tx_queue into a variable-sized linked list
> instead of a fixed-size queue.
In order to reduce memory allocation overhead, I would propose the
following:

struct ubus_msg_backlog {
struct ubus_msg_backlog *next;
struct ubus_msg_buf *msg[UBUSD_CLIENT_BACKLOG];
int tail;
};

To struct ubus_client add these:

struct ubus_msg_backlog *backlog;
int backlog_head;

After sending messages from tx_queue, you pull in messages from
cl->backlog, incrementing backlog_head.
Once cl->backlog_head == backlog->tail, you set cl->backlog to
backlog->next and free backlog.

> To maintain the linked list, an additional structure ubus_msg_buf_list
> is created. We could also have added the linked list to the ubus_msg_buf
> struct itself, but it is not relevant in the many other uses of the
> ubus_msg_buf struct so it would just complicate things.
Adding the linked list to ubus_msg_buf doesn't work, because a single
message can be queued for multiple receivers. This mistake was already
made by previous attempts at introducing a linked list for messages.

> The txq_off variable was used to keep track of which part of the current
> message was already written, in case only a partial write was possible.
> We take this opportunity to also move that variable under the
> ubus_msg_buf_list structure (and rename it to "offset", and change its
> type to size_t). This makes it clearer that it is related to that
> particular queued message and not to the queue as a whole.
> 
> Note that this infinite tx_queue opens the door to a DoS attack. You can
> open a client and a server connection, then send messages from the
> client to the server without ever reading anything on the server side.
> This will eventually lead to an out-of-memory. However, such a DoS
> already existed anyway, it just requires opening multiple server
> connections and filling up the fixed-size queue on each one. To protect
> against such DoS attacks, we'd need to:
I don't fully agree with your reasoning regarding the potential for DoS
attacks. It's true that the potential for *intentional* DoS attacks
exists in the current code already. What I'm worried about with this
patch is that you're adding extra potential for *accidental* DoS attacks
(i.e. excessive ubusd memory use for hanging processes receiving events).

> - keep a global maximum queue size that applies to all rx and tx queues
>   together;
> - stop reading from any connection when the maximum is reached;
> - close any connection when it hasn't become writeable after some
>   timeout.
I think we should have both a local and a global maximum queue size.
Other than that, I agree with the above.

- Felix

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


Re: [PATCH] gmp: compile with -DPIC to use correct asm code

2021-03-12 Thread Felix Fietkau


On 2021-03-12 11:34, Stijn Tintel wrote:
> On 11/03/2021 23:46, Eneas U de Queiroz wrote:
>> The library is always compiled with $(FPIC) (-fPIC or -fpic), even for
>> the static library.
>>
>> There are some assembly sources that decide whether or not to enable
>> PIC code by checking if PIC is defined.  It counts on libtool to define
>> it, but libtool does it only when producing code for the dynamic
>> library, while we need it for both.
>>
>> Ensure it is defined by adding it to CFLAGS next to $(FPIC).
>>
>> It avoids linking errors with strongswan on x86_64:
>>
>> ld: libgmp.a(bdiv_q_1.o): relocation R_X86_64_PC32 against symbol
>> `__gmp_binvert_limb_table' can not be used when making a shared object;
>> recompile with -fPIC
>>
>> Cc: Stijn Tintel 
>> Signed-off-by: Eneas U de Queiroz 
>> ---
>>
>> There's an error on one architecture, and all others work fine without
>> this, so I'm uneasy changing this and then breaking stuff that was
>> working fine otherwise.  However, it feels wrong to me to generate PIC
>> code from C files, but not use it in asm sources, which is essentially
>> what I am changing here.
>>
>> I've looked at asm sources for different chitectures, and there are
>> checks for PIC in: arm64, arm, x86_64, x86, and ppc asm sources, but the
>> error only appears on x86_64.
>>
>> For most CPUs, ifdef(`PIC'), is just used to do different definitions of
>> LEA (Load Effective Address).  However, both x86 and x86_64 have many
>> other checks.
>>
>> I've looked at bdiv_q_1.asm for different CPUs, and they all do some
>> form of LEA(binvert_limb_table), except for x86, where it will do it
>> only when PIC is defined.  That may explain why x86_64 is affected, and
>> x86 is not.
>>
>> I have not investigated further details.
>>
>> Alternatively, we can define it only for x86_64, which is where we know
>> there's a build failure with the linker asking to recompile with -fPIC.
>>
> I'm sorry, but I lack the knowledge to review this.

I think the patch makes sense and -DPIC should be used along with -fPIC.
I don't see any reason to make it x86_64 specific.

- Felix

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


Re: [PATCH] uclibc++: remove

2021-03-01 Thread Felix Fietkau


On 2021-02-27 05:17, Rosen Penev wrote:
> No package here depends on it. Furthermore, uClibc++ is a fairly buggy
> C++ library and seems to be relatively inactive upstream.
> 
> It also lacks proper support for modern C++11 features.
> 
> The main benefit of it is size: 66.6 KB   vs 287.3 KB on mips24kc. Static
> linking and LTO can help bring the size down of packages that need it.
> 
> Added warning message to uclibc++.mk
> 
> Signed-off-by: Rosen Penev 
I've been using iperf with uclibc++ for a long time. I know quite a few
other people using it as well.
I'd really appreciate it if you could change iperf to link just
libstdc++ statically (rest should still be dynamic) in order to avoid
pushing >200 KB of extra bloat (after compression!) onto iperf users.
I realize that removing unmaintained packages is tempting, but please
consider the fallout.

- Felix

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


Re: Please consider reverting autoconf-lean from toolchain

2021-03-01 Thread Felix Fietkau

> On 1. Mar 2021, at 23:47, Hauke Mehrtens  wrote:
> 
> On 3/1/21 3:54 PM, Hannu Nyman wrote:
>> Apparently autoconf-lean, merged into the toolchain during last weekend, 
>> causes problems with various packages at buildbot.
>> I am not sure if that is due to underlying problems in the specific failing 
>> packages, or due to a wrong site config from autoconf-lean, but buildbot is 
>> gradually hitting the problems as packages are built with faulty SDKs.
>> I suggest that the new autoconf-lean stuff is reverted until the failures 
>> are understood better.
>> I have filed bug FS#3655 and earlieralso a packages issue as that was how I 
>> found the issue in my own build.
>> There are several buildbot failure logs examples in the bug FS#3655.
>> At least ntfs-3g, rsync, nedata, and possibly also mac80211 and mwlwifi are 
>> failing for some targets.
>> (not sure about mac80211 and mwlwifi failures' relation)
>> I reverted autoconf-lean from my buildhost and I was again able to build 
>> ntfs-3g.
>> Extract from bug FS#3655:
>> https://bugs.openwrt.org/index.php?do=details_id=3655=id=desc
>>  ---
>> autoconf-lean stuff from Felix was merged by Daniel a few days ago to master 
>> with commits
>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=commit;h=32c664ff02910bf39a3fbd5a5a4a8bff3191dd03�
>>  and
>> https://git.openwrt.org/?p=openwrt/openwrt.git;a=commit;h=f439e291304a93b982e912dc91b80ca950a594f3
>>  That seems to cause buildhost-specific variation in the generated site 
>> config files, and causes variating breakage from some packages.
>> I stumbled upon this on weekend with ntfs-3g, which suddenly does not 
>> compile in my Ubuntu 20.10 in Virtualbox. I filed a bug about ntfs-3g to 
>> packages feed, but this is more generic:
>> https://github.com/openwrt/packages/issues/14940
>> The same buildhost builds 21.02 without problems (and that is still pretty 
>> identical to master regarding packages)
>> Reverting autoconf-lean commits fixes things for me.
>> There are now others with the same symptoms, based on responses to the 
>> packages feed bug.
>> Buildbot is now meeting the same problem. At least ntfs-3g, rsync, nedata, 
>> and possibly also mac80211 and mwlwifi are failing for some targets with 
>> some buildhosts (with the new SDK).
>> E.g.
>> https://downloads.openwrt.org/snapshots/faillogs/aarch64_cortex-a72/packages/ntfs-3g/compile.txt
>>  
> 
> OpenWrt does not build with glibc for me and the build bots.
> 
> When I remove this setting at least the autoconf-lean packet links, but I 
> still have some build problems.
> ac_cv_lib_xnet_main=${ac_cv_lib_xnet_main=yes}
> 
Please go ahead and revert it. 
It’s definiely not ready, and I don’t know why Daniel even merged it.

- Felix

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


Re: vm.min_free_kbytes seems very big

2021-02-27 Thread Felix Fietkau


Hi Adrian,

On 2021-02-27 16:40, Adrian Schmutzler wrote:
> Hi Felix,
> 
> due to some OOM issues I recently became aware that OpenWrt (or rather, you 
> in [1]) set the vm.min_free_kbytes to quite excessive values:
> 
> 16M for 128M RAM
> 8M for 64M RAM
> 
> The value for 32M devices has been reduced in a later patch [2].
> 
> These values seem high (1/8 or 12.5 %), and in a kernel comment here [3] we 
> found a much lower recommendation, even addressing low-mem devices.
> 
> Apart from theoretical discussions, this is of cause a real problem on 64MB 
> RAM devices (particularly with ath10k active), where RAM is scarce.
> On a typical OpenWrt installation, OOM-Killer becoming active will typically 
> be a problem in almost every case, since there are not many applications 
> running that are not essential.
> 
> I wonder whether we could not reduce these values to something much lower, 
> e.g.
> 2M for 128M RAM
> 1.5M for 64M RAM
> (1M for 32M RAM is set already)
> 
> That would still be above the recommended values in [3], but increase the 
> amount of "available" memory by 6.5 M on 64M devices.
> 
> Would you (or others) comment on this please, so I don't prepare a patch just 
> to be told I overlooked something.
The reason why these values are so high is the fact that network buffer
allocations during routing/bridging tend to be almost entirely atomic.
Under load, using the "recommended" values can easily lead to lots of
allocation failures in NAPI poll functions, because the context doing
the atomic allocation cannot free up reclaimable memory by itself.

Maybe we should use custom settings on devices with ath10k (which is a
really nasty RAM hog).

- Felix

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


Re: [PATCH v3] fakeroot: fix to work with glibc 2.33

2021-02-15 Thread Felix Fietkau


On 2021-02-14 05:41, Ilya Lipnitskiy wrote:
> The following commit removed _STAT_VER definitions from glibc:
> https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=8ed005daf0ab03e142500324a34087ce179ae78e
> 
> That subsequently broke fakeroot:
> https://bugs.archlinux.org/task/69572
> https://bugzilla.redhat.com/show_bug.cgi?id=1889862#c13
> https://forum.openwrt.org/t/unable-to-build-toolchain-fakeroot-fails-perhaps-others-after-it/87966
> 
> Make the patch based on Jan Pazdziora's suggestion from here:
> https://lists.fedoraproject.org/archives/list/de...@lists.fedoraproject.org/message/SMQ3RYXEYTVZH6PLQMKNB3NM4XLPMNZO/
> 
> Add wrappers for newly exported symbols in glibc.
> 
> Apply patch from Debian to fix warnings in fts_read and fts_children:
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=676428
> https://sources.debian.org/patches/fakeroot/1.25.3-1.1/eglibc-fts-without-LFS/
> 
> Fix __xmknod{,at} dev pointer argument. Switch default to assume * and
> not the absence of *. On glibc 2.33+, there is no definition for these
> functions in header files, so the compile test doesn't work. But, we
> can default to using the pointer (as is the case with newer glibc), and
> use the header file on older platforms to fail the test and use no pointer.
> 
> Tested on my x86_64 Arch Linux machine, fakeroot unit tests pass.
> Also tested by building various .ipks and examining the tar contents, to
> ensure that the owner uid/gid was 0/0.
> 
> Signed-off-by: Ilya Lipnitskiy 
> ---
>  .../300-glibc-2.33-compatibility.patch| 145 ++
>  1 file changed, 145 insertions(+)
>  create mode 100644 tools/fakeroot/patches/300-glibc-2.33-compatibility.patch
> 
> diff --git a/tools/fakeroot/patches/300-glibc-2.33-compatibility.patch 
> b/tools/fakeroot/patches/300-glibc-2.33-compatibility.patch
> new file mode 100644
> index 00..a460cace0c
> --- /dev/null
> +++ b/tools/fakeroot/patches/300-glibc-2.33-compatibility.patch
> @@ -0,0 +1,145 @@
> +--- a/libfakeroot.c
>  b/libfakeroot.c
> +@@ -1358,6 +1368,54 @@ int renameat(int olddir_fd, const char *
> + #endif /* HAVE_FSTATAT */
> + 
> + 
> ++#if defined(__GLIBC__) && __GLIBC_PREREQ(2,33)
Turns out combining those two into a single #if was not such a good
idea. It breaks on macOS, because __GLIBC_PREREQ is evaluated though
__GLIBC__ is not defined.
Splitting it into two #if lines makes things work again.
The change to the __xmknod dev pointer argument checks also broke.
I've pushed a fix for both, please check it and submit it upstream.

Thanks,

- Felix

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


Re: [PATCH 4/4] mac80211: add AX support

2021-02-15 Thread Felix Fietkau


On 2021-02-15 11:19, John Crispin wrote:
> Signed-off-by: John Crispin 
> ---
>  .../files/lib/netifd/wireless/mac80211.sh | 193 +-
>  .../mac80211/files/lib/wifi/mac80211.sh   |  19 +-
>  2 files changed, 195 insertions(+), 17 deletions(-)
> 
> diff --git a/package/kernel/mac80211/files/lib/netifd/wireless/mac80211.sh 
> b/package/kernel/mac80211/files/lib/netifd/wireless/mac80211.sh
> index 92c56afd24..b717770d5e 100644
> --- a/package/kernel/mac80211/files/lib/netifd/wireless/mac80211.sh
> +++ b/package/kernel/mac80211/files/lib/netifd/wireless/mac80211.sh
> @@ -56,6 +56,13 @@ drv_mac80211_init_device_config() {
[...]
> +mac80211_get_seg0() {
> + local ht_mode="$1"
> + local seg0=0
> +
> + case "$ht_mode" in
> + 40)
> + if [ $freq -gt 5950 ] && [ $freq -le 7115 ]; then
> + case "$(( ($channel / 4) % 2 ))" in
> + 1) seg0=$(($channel - 2));;
> + 0) seg0=$(($channel + 2));;
> + esac
> + elif [ $freq != 5935 ]; then
> + case "$(( ($channel / 4) % 2 ))" in
> + 1) seg0=$(($channel + 2));;
> + 0) seg0=$(($channel - 2));;
> + esac
> + fi
> + ;;
> + 80)
> + if [ $freq -gt 5950 ] && [ $freq -le 7115 ]; then
> + case "$(( ($channel / 4) % 4 ))" in
> + 0) seg0=$(($channel + 6));;
> + 1) seg0=$(($channel + 2));;
> + 2) seg0=$(($channel - 2));;
> + 3) seg0=$(($channel - 6));;
> + esac
> + elif [ $freq != 5935 ]; then
> + case "$(( ($channel / 4) % 4 ))" in
> + 1) seg0=$(($channel + 6));;
> + 2) seg0=$(($channel + 2));;
> + 3) seg0=$(($channel - 2));;
> + 0) seg0=$(($channel - 6));;
> + esac
> + fi
> + ;;
> + 160)
> + if [ $freq -gt 5950 ] && [ $freq -le 7115 ]; then
> + case "$channel" in
> + 1|5|9|13|17|21|25|29) seg0=15;;
> + 33|37|41|45|49|53|57|61) seg0=47;;
> + 65|69|73|77|81|85|89|93) seg0=79;;
> + 97|101|105|109|113|117|121|125) 
> seg0=111;;
> + 129|133|137|141|145|149|153|157) 
> seg0=143;;
> + 161|165|169|173|177|181|185|189) 
> seg0=175;;
> + 193|197|201|205|209|213|217|221) 
> seg0=207;;
> + esac
> + elif [ $freq != 5935 ]; then
> + case "$channel" in
> + 36|40|44|48|52|56|60|64) seg0=50;;
> + 100|104|108|112|116|120|124|128) 
> seg0=114;;
> + esac
> + fi
> + ;;
> + esac
> + printf "$seg0"
> +}
This function duplicates the seg0 that's already done for VHT+HE and
simply adds some extra conditions for 6 GHz. Those extra conditions
should be folded into the existing code.

> +
>  mac80211_hostapd_setup_base() {
>   local phy="$1"
>  
> @@ -333,20 +412,105 @@ mac80211_hostapd_setup_base() {
>   # 802.11ax
>   enable_ax=0
>   case "$htmode" in
> - HE*) enable_ax=1 ;;
> + HE20)   enable_ax=1
> + if [ $freq -gt 5950 ] && [ $freq -le 7115 ]; then
> + append base_cfg "op_class=131" "$N"
> + fi
> + ;;
> + HE40)
> + enable_ax=1
> + idx="$(mac80211_get_seg0 "40")"
> + if [ $freq -ge 5180 ] && [ $freq != 5935 ]; then
> + if [ $freq -gt 5950 ] && [ $freq -le 7115 ]; 
> then
> + append base_cfg "op_class=132" "$N"
> + fi
> + append base_cfg "he_oper_chwidth=0" "$N"
> + append base_cfg 
> "he_oper_centr_freq_seg0_idx=$idx" "$N"
> + fi
> + ;;
> + HE80)
> + enable_ax=1
> + idx="$(mac80211_get_seg0 "80")"
> + if [ $freq != 5935 ]; then
> + if [ $freq -gt 

Re: ath10k-ct iw missing rx stats

2021-02-14 Thread Felix Fietkau
On 2021-02-14 18:28, Ansuel Smith wrote:
> With recent mac80211 bump I notice that rx stats
> are no longer displayed.
> I ported the atk10-ct patches to the version 5.10
> and I noticed this. It's only me?
> Also this is only to report that ath10k-ct patches can
> be ported with minimal changes and works normally
> (except this problem with ubus not reporting rx stats)
It was an upstream regression. I backported the fix for it and it should
work now.

- Felix

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


[PATCH 2/5] build: add support for fixing up library soname

2021-02-12 Thread Felix Fietkau
This makes it possible to declare a package ABI_VERSION independent from the
upstream soname by setting PKG_ABI_VERSION in the package makefile.
The library filename is fixed up for files installed to packages and to the
staging dir. References to the original from executables within the same
package are also fixed up

Signed-off-by: Felix Fietkau 
---
 include/package-ipkg.mk|  5 +++
 include/package.mk |  2 ++
 rules.mk   |  6 
 scripts/set-abi-version.sh | 73 ++
 4 files changed, 86 insertions(+)
 create mode 100755 scripts/set-abi-version.sh

diff --git a/include/package-ipkg.mk b/include/package-ipkg.mk
index ead4b5742cb5..fb1b7549fbf8 100644
--- a/include/package-ipkg.mk
+++ b/include/package-ipkg.mk
@@ -152,6 +152,7 @@ ifeq ($(DUMP),)
mkdir -p $(PKG_BUILD_DIR)/.pkgdir/$(1)
$(call Package/$(1)/install,$(PKG_BUILD_DIR)/.pkgdir/$(1))
$(call Package/$(1)/install_lib,$(PKG_BUILD_DIR)/.pkgdir/$(1))
+   $(if $(PKG_ABI_VERSION),$(SET_ABI_VERSION) "$(PKG_ABI_VERSION)" 
"$(PKG_BUILD_DIR)/.pkgdir/$(1)" "$(STAGING_DIR)/packages/$(STAGING_FILES_LIST)")
touch $$@
 
 $(STAGING_DIR_ROOT)/stamp/.$(1)_installed: 
$(PKG_BUILD_DIR)/.pkgdir/$(1).installed
@@ -191,11 +192,15 @@ $(_endef)
 $$(IPKG_$(1)) : export DESCRIPTION=$$(Package/$(1)/description)
 $$(IPKG_$(1)) : export PATH=$$(TARGET_PATH_PKG)
 $$(IPKG_$(1)) : export PKG_SOURCE_DATE_EPOCH:=$(PKG_SOURCE_DATE_EPOCH)
+ifdef Build/InstallDev
+  $$(IPKG_$(1)): $(STAMP_INSTALLED)
+endif
 $(PKG_INFO_DIR)/$(1).provides $$(IPKG_$(1)): $(STAMP_BUILT) 
$(INCLUDE_DIR)/package-ipkg.mk
@rm -rf $$(IDIR_$(1)); \
$$(call remove_ipkg_files,$(1),$$(call 
opkg_package_files,$(call gen_ipkg_wildcard,$(1
mkdir -p $(PACKAGE_DIR) $$(IDIR_$(1))/CONTROL $(PKG_INFO_DIR)
$(call Package/$(1)/install,$$(IDIR_$(1)))
+   $(if $(PKG_ABI_VERSION),$(SET_ABI_VERSION) "$(PKG_ABI_VERSION)" 
"$$(IDIR_$(1))" "$(STAGING_DIR)/packages/$(STAGING_FILES_LIST)")
$(if $(Package/$(1)/install-overlay),mkdir -p $(PACKAGE_DIR) 
$$(IDIR_$(1))/rootfs-overlay)
$(call Package/$(1)/install-overlay,$$(IDIR_$(1))/rootfs-overlay)
-find $$(IDIR_$(1)) -name 'CVS' -o -name '.svn' -o -name '.#*' -o -name 
'*~'| $(XARGS) rm -rf
diff --git a/include/package.mk b/include/package.mk
index b23826bd7817..2ca0c54274d7 100644
--- a/include/package.mk
+++ b/include/package.mk
@@ -178,6 +178,7 @@ Build/Exports=$(Build/Exports/Default)
 define Build/CoreTargets
   STAMP_PREPARED:=$$(STAMP_PREPARED)
   STAMP_CONFIGURED:=$$(STAMP_CONFIGURED)
+  PKG_ABI_VERSION:=$$(PKG_ABI_VERSION)
 
   $(if $(QUILT),$(Build/Quilt))
   $(call Build/Autoclean)
@@ -236,6 +237,7 @@ define Build/CoreTargets
"$(STAGING_DIR)/packages/$(STAGING_FILES_LIST)" \
"$(STAGING_DIR)"; \
fi
+   $(if $(PKG_ABI_VERSION),$(SET_ABI_VERSION) "$(PKG_ABI_VERSION)" 
$(TMP_DIR)/stage-$(PKG_DIR_NAME))
if [ -d $(TMP_DIR)/stage-$(PKG_DIR_NAME) ]; then \
(cd $(TMP_DIR)/stage-$(PKG_DIR_NAME); find ./ > 
$(TMP_DIR)/stage-$(PKG_DIR_NAME).files); \
$(call locked, \
diff --git a/rules.mk b/rules.mk
index cbe1b0cb4c7f..c6bd7327c585 100644
--- a/rules.mk
+++ b/rules.mk
@@ -339,6 +339,10 @@ else
 $(SCRIPT_DIR)/rstrip.sh
 endif
 
+SET_ABI_VERSION= \
+PATCHELF="$(STAGING_DIR_HOST)/bin/patchelf" \
+   $(SCRIPT_DIR)/set-abi-version.sh
+
 ifeq ($(CONFIG_IPV6),y)
   DISABLE_IPV6:=
 else
@@ -428,6 +432,8 @@ $(shell \
 )
 endef
 
+abi_version_str = $(subst -,,$(subst _,,$(subst .,,$(1
+
 COMMITCOUNT = $(if $(DUMP),0,$(call commitcount))
 AUTORELEASE = $(if $(DUMP),0,$(call commitcount,1))
 
diff --git a/scripts/set-abi-version.sh b/scripts/set-abi-version.sh
new file mode 100755
index ..a61c842c6dcb
--- /dev/null
+++ b/scripts/set-abi-version.sh
@@ -0,0 +1,73 @@
+#!/usr/bin/env bash
+# 
+# Copyright (C) 2020 OpenWrt.org
+#
+# This is free software, licensed under the GNU General Public License v2.
+# See /LICENSE for more information.
+#
+SELF=${0##*/}
+
+[ -n "$PATCHELF" ] || {
+  echo "$SELF: patchelf command not defined (PATCHELF variable not set)"
+  exit 1
+}
+
+ABI_VER="$1"
+PATCH_DIR="$2"
+REF_LIST="$3"
+
+[ -n "$ABI_VER" -a -n "$PATCH_DIR" ] || {
+   echo "$SELF: no ABI version or files/directories specified"
+   echo "usage: $SELF  [...]"
+   exit 1
+}
+
+cmd() {
+   echo "$@" >&2
+   "$@" || exit 1
+}
+
+gen_lib_list() {
+   while read F; do
+   F="${F##*/}"
+   case "$F" in
+   lib*.so*);;
+   *) continue;;
+ 

[PATCH 4/5] build: drop ABI version from metadata

2021-02-12 Thread Felix Fietkau
Preparation for supporting dynamic ABI versions that depend on the runtime
configuration. Read the suffix from the staging dir pkginfo version files.

Signed-off-by: Felix Fietkau 
---
 include/feeds.mk|  2 +-
 include/package-dumpinfo.mk |  3 +--
 include/package-ipkg.mk |  9 +++--
 scripts/metadata.pm |  1 -
 scripts/package-metadata.pl | 15 ---
 5 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/include/feeds.mk b/include/feeds.mk
index ef5fef7f4ba8..5676cc4a8f6b 100644
--- a/include/feeds.mk
+++ b/include/feeds.mk
@@ -43,5 +43,5 @@ endef
 
 # 1: package name
 define GetABISuffix
-$(if $(filter-out kmod-%,$(1)),$(if $(Package/$(1)/abiversion),$(if $(filter 
%0 %1 %2 %3 %4 %5 %6 %7 %8 %9,$(1)),-)$(Package/$(1)/abiversion)))
+$(if $(filter-out kmod-%,$(1)),$(foreach v,$(wildcard 
$(STAGING_DIR)/pkginfo/$(1).version),$(shell cat $(v
 endef
diff --git a/include/package-dumpinfo.mk b/include/package-dumpinfo.mk
index 30b0187f119e..597452e4b930 100644
--- a/include/package-dumpinfo.mk
+++ b/include/package-dumpinfo.mk
@@ -21,8 +21,7 @@ $(if $(MENU),Menu: $(MENU)
 )$(if $(DEFAULT),Default: $(DEFAULT)
 )$(if $(findstring $(PREREQ_CHECK),1),Prereq-Check: 1
 )Version: $(VERSION)
-$(if $(ABI_VERSION),ABIVersion: $(ABI_VERSION)
-)Depends: $(call PKG_FIXUP_DEPENDS,$(1),$(DEPENDS))
+Depends: $(call PKG_FIXUP_DEPENDS,$(1),$(DEPENDS))
 Conflicts: $(CONFLICTS)
 Menu-Depends: $(MDEPENDS)
 Provides: $(PROVIDES)
diff --git a/include/package-ipkg.mk b/include/package-ipkg.mk
index fb1b7549fbf8..e89276ed1a03 100644
--- a/include/package-ipkg.mk
+++ b/include/package-ipkg.mk
@@ -99,7 +99,7 @@ _endef=endef
 
 ifeq ($(DUMP),)
   define BuildTarget/ipkg
-ABIV_$(1):=$(call GetABISuffix,$(1))
+ABIV_$(1):=$(if $(filter-out kmod-%,$(1)),$(ABI_VERSION))
 PDIR_$(1):=$(call FeedPackageDir,$(1))
 IPKG_$(1):=$$(PDIR_$(1))/$(1)$$(ABIV_$(1))_$(VERSION)_$(PKGARCH).ipk
 IDIR_$(1):=$(PKG_BUILD_DIR)/ipkg-$(PKGARCH)/$(1)
@@ -157,7 +157,12 @@ ifeq ($(DUMP),)
 
 $(STAGING_DIR_ROOT)/stamp/.$(1)_installed: 
$(PKG_BUILD_DIR)/.pkgdir/$(1).installed
mkdir -p $(STAGING_DIR_ROOT)/stamp
-   $(if $(ABI_VERSION),echo '$(ABI_VERSION)' | cmp -s - 
$(PKG_INFO_DIR)/$(1).version || echo '$(ABI_VERSION)' > 
$(PKG_INFO_DIR)/$(1).version)
+   $(if $(ABI_VERSION),echo '$(ABI_VERSION)' | cmp -s - 
$(PKG_INFO_DIR)/$(1).version || \
+   echo '$(ABI_VERSION)' > $(PKG_INFO_DIR)/$(1).version \
+   $(foreach pkg,$(filter-out $(1),$(PROVIDES)),; \
+   cp $(PKG_INFO_DIR)/$(1).version 
$(PKG_INFO_DIR)/$(pkg).version \
+   ) \
+   )
$(call locked,$(CP) $(PKG_BUILD_DIR)/.pkgdir/$(1)/. 
$(STAGING_DIR_ROOT)/,root-copy)
touch $$@
 
diff --git a/scripts/metadata.pm b/scripts/metadata.pm
index f6dce39662d2..5fbb77a36c4c 100644
--- a/scripts/metadata.pm
+++ b/scripts/metadata.pm
@@ -249,7 +249,6 @@ sub parse_package_metadata($) {
/^Build-Types:\s*(.+)\s*$/ and $src->{buildtypes} = [ split 
/\s+/, $1 ];
next unless $pkg;
/^Version: \s*(.+)\s*$/ and $pkg->{version} = $1;
-   /^ABIVersion: \s*(.+)\s*$/ and $pkg->{abiversion} = $1;
/^Title: \s*(.+)\s*$/ and $pkg->{title} = $1;
/^Menu: \s*(.+)\s*$/ and $pkg->{menu} = $1;
/^Submenu: \s*(.+)\s*$/ and $pkg->{submenu} = $1;
diff --git a/scripts/package-metadata.pl b/scripts/package-metadata.pl
index 5abc1bdef3f7..02102be8f39d 100755
--- a/scripts/package-metadata.pl
+++ b/scripts/package-metadata.pl
@@ -532,21 +532,6 @@ sub gen_package_auxiliary() {
if ($pkg->{name} && $pkg->{repository}) {
print "Package/$name/subdir = $pkg->{repository}\n";
}
-   if ($pkg->{name} && defined($pkg->{abiversion}) && 
length($pkg->{abiversion})) {
-   my $abiv;
-
-   if ($pkg->{abiversion} =~ 
m!^(\d{4})-(\d{2})-(\d{2})-[0-9a-f]{7,40}$!) {
-   print STDERR "WARNING: Reducing ABI version 
'$pkg->{abiversion}' of package '$name' to '$1$2$3'\n";
-   $abiv = "$1$2$3";
-   }
-   else {
-   $abiv = $pkg->{abiversion};
-   }
-
-   foreach my $n (@{$pkg->{provides}}) {
-   print "Package/$n/abiversion = $abiv\n";
-   }
-   }
my %depends;
foreach my $dep (@{$pkg->{depends} || []}) {
if ($dep =~ m!^\+?(?:[^:]+:)?([^@]+)$!) {
-- 
2.28.0


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


[PATCH 3/5] libubox: use PKG_ABI_VERSION

2021-02-12 Thread Felix Fietkau
Signed-off-by: Felix Fietkau 
---
 package/libs/libubox/Makefile | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/package/libs/libubox/Makefile b/package/libs/libubox/Makefile
index d9c239a50d15..19d91455097e 100644
--- a/package/libs/libubox/Makefile
+++ b/package/libs/libubox/Makefile
@@ -8,6 +8,7 @@ PKG_SOURCE_URL=$(PROJECT_GIT)/project/libubox.git
 
PKG_MIRROR_HASH:=97dc4eba01cf2c5d6a6d0db3747e0cdc0d95cb87e51b3115272e7d3e69a8b255
 PKG_SOURCE_DATE:=2020-12-12
 PKG_SOURCE_VERSION:=357877693ca363b12e6e7e14d345639b2440cd07
+PKG_ABI_VERSION:=$(call abi_version_str,$(PKG_SOURCE_DATE))
 CMAKE_INSTALL:=1
 
 PKG_LICENSE:=ISC
@@ -27,7 +28,7 @@ define Package/libubox
   SECTION:=libs
   CATEGORY:=Libraries
   TITLE:=Basic utility library
-  ABI_VERSION:=20201212
+  ABI_VERSION:=$(PKG_ABI_VERSION)
   DEPENDS:=
 endef
 
@@ -35,6 +36,7 @@ define Package/libblobmsg-json
   SECTION:=libs
   CATEGORY:=Libraries
   TITLE:=blobmsg <-> json conversion library
+  ABI_VERSION:=$(PKG_ABI_VERSION)
   DEPENDS:=+libjson-c +libubox
 endef
 
@@ -53,6 +55,7 @@ define Package/libjson-script
   SECTION:=utils
   CATEGORY:=Utilities
   DEPENDS:=+libubox
+  ABI_VERSION:=$(PKG_ABI_VERSION)
   TITLE:=Minimalistic JSON based scripting engine
 endef
 
-- 
2.28.0


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


[PATCH 1/5] build: fix ABI version rebuild dependency

2021-02-12 Thread Felix Fietkau
A stray comma was being appended to the last package version dependency,
causing it to be missed for ABI version checks

Signed-off-by: Felix Fietkau 
---
 include/package.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/package.mk b/include/package.mk
index a5c37b4268f3..b23826bd7817 100644
--- a/include/package.mk
+++ b/include/package.mk
@@ -73,7 +73,7 @@ find_library_dependencies = \
$(Package/$(dep3)/depends) $(dep3) \
)), \
$(Package/$(dep4)/depends) $(dep4) \
-   )), \
+   )) \
))
 
 
-- 
2.28.0


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


[PATCH 5/5] wolfssl: use dynamic ABI_VERSION depending on the configuration and package version

2021-02-12 Thread Felix Fietkau
Signed-off-by: Felix Fietkau 
---
 package/libs/wolfssl/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/package/libs/wolfssl/Makefile b/package/libs/wolfssl/Makefile
index ff01fca66310..51dc4d5a0933 100644
--- a/package/libs/wolfssl/Makefile
+++ b/package/libs/wolfssl/Makefile
@@ -33,6 +33,8 @@ PKG_CONFIG_DEPENDS:=\
CONFIG_WOLFSSL_HAS_SESSION_TICKET CONFIG_WOLFSSL_HAS_TLSV10 \
CONFIG_WOLFSSL_HAS_TLSV13 CONFIG_WOLFSSL_HAS_WPAS 
CONFIG_WOLFSSL_HAS_CERTGEN
 
+PKG_ABI_VERSION=$(patsubst %-stable,%,$(PKG_VERSION)).$(call 
version_abbrev,$(call confvar,$(PKG_CONFIG_DEPENDS)))
+
 include $(INCLUDE_DIR)/package.mk
 
 define Package/libwolfssl
@@ -44,7 +46,7 @@ define Package/libwolfssl
   MENU:=1
   PROVIDES:=libcyassl
   DEPENDS:=+WOLFSSL_HAS_DEVCRYPTO:kmod-cryptodev 
+WOLFSSL_HAS_AFALG:kmod-crypto-user
-  ABI_VERSION:=24
+  ABI_VERSION:=$(PKG_ABI_VERSION)
 endef
 
 define Package/libwolfssl/description
-- 
2.28.0


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


Re: [PATCH] fakeroot: fix to work with glibc 2.33

2021-02-11 Thread Felix Fietkau


Hi Ilya,

On 2021-02-11 04:14, Ilya Lipnitskiy wrote:
> The following commit removed _STAT_VER definitions from glibc:
> https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=8ed005daf0ab03e142500324a34087ce179ae78e
> 
> That subsequently broke fakeroot:
> https://bugs.archlinux.org/task/69572
> https://bugzilla.redhat.com/show_bug.cgi?id=1889862#c13
> https://forum.openwrt.org/t/unable-to-build-toolchain-fakeroot-fails-perhaps-others-after-it/87966
> 
> Make the patch based on Jan Pazdziora's suggestion from here:
> https://lists.fedoraproject.org/archives/list/de...@lists.fedoraproject.org/message/SMQ3RYXEYTVZH6PLQMKNB3NM4XLPMNZO/
> 
> Tested on my x86_64 Arch Linux machine, fakeroot unit tests pass.
> 
> Signed-off-by: Ilya Lipnitskiy 
If I'm not misreading the code, it seems to me that this patch might be
incomplete. The old glibc version was carrying inline wrappers for stat,
stat64, etc. Because binaries compiled against the old glibc version
were only containing references to the wrapped internal functions and
never the main functions directly, fakeroot seems to be handling only
the __ prefixed internal functions.
Defining _STAT_VER fixes the compile error and should work with binaries
compiled against old glibc, but I wonder if this also works with newer
binaries that now reference different functions.
Did you explicitly test that?

Thanks for looking into this,

- Felix

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


Re: Revisiting ABI_VERSION handling policy

2021-02-09 Thread Felix Fietkau


On 2021-02-09 23:56, Jo-Philipp Wich wrote:
> 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)
It looks to me like we already have the above problem in most of those
packages where you set the ABI_VERSION explicitly. Quite a few of our
packages intentionally have no stable ABI.

I think adjusting soname a good idea. I will look into automating it
using something like this: https://github.com/NixOS/patchelf

>>> [...]
>> 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.
I'm pretty sure this will catch changes to non-exposed structs as well,
possibly with enough false positives to render it completely useless.

For now I think it's much safer to simply assume every version update to
be ABI-breaking unless it has explicitly been proven to be compatible.

>>> [...]
>> 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).
Sounds good. While some core upstream libraries might be able to do a
good job on maintaining a stable ABI, I don't think we can rely on this
being the case for most of them, regardless of how much awareness we raise.

To sum it up, here's my current plan for resolving the immediate issues:
1. implement automatic soname patching to include ABI_VERSION
2. add config based suffix to wolfssl ABI_VERSION
3. switch back packages to automatic ABI_VERSION

When we want to deliver security updates or critical bugfixes, they
should either come in patch form (which won't affect ABI_VERSION), or
should be verified to be fully ABI compatible and override ABI_VERSION
for just that one upgrade with a comment to remove it on the next major
upgrade.

If we need to fix ABI_VERSION to a particular value for an incremental
update, we should only ever do it in the release branch and only after a
thorough review.

Do you agree?

- Felix

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


Re: Revisiting ABI_VERSION handling policy

2021-02-09 Thread Felix Fietkau


On 2021-02-09 13:00, Jo-Philipp Wich wrote:
> 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.
Ah, that wasn't really clear to me from reading the commit I pointed out.

>> 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.
This has nothing to do with being annoyed by that particular bug. The
issue that I'm annoyed about is how easy it is for these things to creep
in, even with careful checks.

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

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

>> 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 optionsI'm not conflating the two. I'm 
> saying that in the instance that I ran
into, the compile time options were the cause, but the last package
version update (which did not increase ABI_VERSION or soname) introduced
ABI changes that could have very similar effects.
Simple runtime tests often don't catch these issues (in my test only a
very specific runtime config ran into it), soname version changes are
not a reliable indicator, symbol 

Revisiting ABI_VERSION handling policy

2021-02-08 Thread Felix Fietkau


Hi,

I just wasted a few hours of debugging on an issue that turned out to be
wolfssl ABI breakage related.

At first I thought it was the version bump. While the commit that
updates wolfssl claims that the ABI didn't change, a quick look at
https://abi-laboratory.pro/index.php?view=timeline=wolfssl indicates
that this isn't true and the ABI did change after all.

When I looked into the details of the ABI change, I found out that this
wasn't the cause of my breakage (I had already cleaned my hostapd build
after updating wolfssl), but there is something even nastier lurking
here: the ABI isn't just affected by the version bump, but by wolfssl
configuration changes as well. There are plenty of #if statements inside
structs like Aes which are sometimes declared on-stack in hostapd, many
of which are affected by the OpenWrt configuration for wolfssl.

I think it's very likely that I've been affected by similar breakages in
the past, but I can't be absolutely sure now, because I didn't have the
time to investigate back then and a lazy make clean made things work again.

Either way, this whole ordeal caused me to rethink Jo's commit
"packages: set more explicit ABI_VERSION values"

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.

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.
In my case, hostapd worked just fine and wpa_supplicant was crashing due
to a stack overflow whenever I turned on 802.11w on a client interface.

Stuff like this makes our trade-off seem very asymmetric to me...

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.

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.

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.

Thoughts?

- Felix

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


Re: Fwd: C++ libubus wrapper

2021-02-04 Thread Felix Fietkau

On 2021-02-04 18:25, Wojciech Jowsa wrote:
> czw., 4 lut 2021 o 10:32 Felix Fietkau  napisał(a):
>>
>>
>> On 2021-02-03 20:02, Wojciech Jowsa wrote:
>> > Hi,
>> >
>> > I would like to write a libubus wrapper in C++. When looking into
>> > ubus_method and ubus_handler_t structures I don't see a place where I
>> > could pass a this pointer. It makes the handling of ubus calls in C++
>> > a bit complicated.
>> > The solution to that case would be adding a void pointer to the
>> > ubus_method structure (like in the ubus_request struct). This way it
>> > would be possible to pass e.g function pointer.
>> > Would it be possible to add a void pointer to ubus_method structure?
>> > If yes then I will provide the patch asap.
>> I don't think a 'this' pointer should be in the ubus_method. Methods can
>> typically be shared between multiple ubus_objects, and the 'this'
>> pointer would typically go into the ubus_object.
> 
> The pointer would share the same concept as the .handler member of
> ubus_method structure.
> It means that C++ handler method would be called based on the ubus method 
> name.
> Void pointer could also be the last argument of ubus_handler_t. Then
> I see that e.g. ubus_request structure has a void pointer as a member
> or ubus_lookup has a void pointer
> parameter so the idea is implemented but only partially.
The idea has been implemented selectively where it makes sense.
In the ubus request it is needed, because functions like ubus_invoke()
create it on-stack, and there is no embedding into other data structures.

>> However, even there it is not necessary to have a void pointer directly
>> in the struct. You can simply embed the ubus_object in another data
>> structure which contains the 'this' pointer (or maybe is even embedded
>> in the C++ object directly)
> 
> Then obtain parent struct with container_of? This might somehow work
> but personally I have never tried that.
> I think that the common pattern for C libraries API methods is to
> allow passing a user data through a void pointer either as
> a function parameter or as a structure member.
For me, putting void * pointers everywhere is an anti-pattern which I
discourage with the way the ubus api is set up. It is a lazy way to
avoid making the code more type safe, and it adds a bit of unnecessary
bloat to data structures.

The ubus API is designed in a way that you typically embed the struct
ubus_object into your own object-like data structure.
Simple pseudo-code example:

struct my_obj {
...
struct ubus_object ubus;
...
};


static int
my_obj_test(struct ubus_context *ctx, struct ubus_object *obj,
struct ubus_request_data *req, const char *method,
struct blob_attr *msg)
{
struct my_obj *obj = container_of(obj, struct my_obj, ubus);

...
}

static struct ubus_method my_obj_methods[] = {
{ .name = "test", .handle = my_obj_test },
};

As you can see, this pattern preserves some type-safety when going from
struct ubus_object to struct my_obj and it does not waste space for an
extra pointer. It is also more efficient, since the compiler will turn
it into simple pointer arithmetic instead of an extra memory access.

I think it would be very useful if your C++ wrapper could preserve this
pattern and make it just as convenient to use.

Maybe a C++ wrapper around ubus_object could simply be embedded as a
class attribute in user-defined classes and you could write some code to
generate thin wrappers that derive the object pointer from the address
of the struct ubus_object pointer and call plain C++ methods with a
fixed type.

I don't know enough C++ to come up with an example of what that wrapper
would look like, but it seems to me that if C can do this, it should be
possible in C++ as well.

Does that make sense?

- Felix

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


Re: C++ libubus wrapper

2021-02-04 Thread Felix Fietkau


On 2021-02-03 20:02, Wojciech Jowsa wrote:
> Hi,
> 
> I would like to write a libubus wrapper in C++. When looking into
> ubus_method and ubus_handler_t structures I don't see a place where I
> could pass a this pointer. It makes the handling of ubus calls in C++
> a bit complicated.
> The solution to that case would be adding a void pointer to the
> ubus_method structure (like in the ubus_request struct). This way it
> would be possible to pass e.g function pointer.
> Would it be possible to add a void pointer to ubus_method structure?
> If yes then I will provide the patch asap.
I don't think a 'this' pointer should be in the ubus_method. Methods can
typically be shared between multiple ubus_objects, and the 'this'
pointer would typically go into the ubus_object.
However, even there it is not necessary to have a void pointer directly
in the struct. You can simply embed the ubus_object in another data
structure which contains the 'this' pointer (or maybe is even embedded
in the C++ object directly)

I don't write code in C++ myself, so if there's something I'm missing
here, please let me know and show me some details.

- Felix

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


Re: [PATCHv3] ccache: update to 4.1

2020-12-23 Thread Felix Fietkau


On 2020-12-23 09:14, Rosen Penev wrote:
> On Tue, Dec 22, 2020 at 11:53 PM Hannu Nyman  wrote:
>>
>> Rosen Penev kirjoitti 23.12.2020 klo 8.33:
>> > Upstream switched to building with CMake. Adjust accordingly.
>> >
>> > Reapplied patch as upstream changed the file format.
>> >
>> > Added HOST_BUILD_PARALLEL for faster compilation.
>> >
>> > Added cmake tool dependency.
>> >
>> > Adjusted dependent tools to use NOCACHE as they are needed to build
>> > ccache.
>> >
>> > Signed-off-by: Rosen Penev 
>> > ---
>> >   v3: zstd was missing in the commit for some reason
>> >   v2: fix compilation issues without OS tools.
>> >   tools/Makefile  |  1 +
>> >   tools/ccache/Makefile   | 17 +
>> >   tools/ccache/patches/100-honour-copts.patch | 20 ++--
>> >   tools/libressl/Makefile |  1 +
>> >   tools/pkgconf/Makefile  |  2 ++
>> >   tools/zstd/Makefile |  1 +
>> >   6 files changed, 24 insertions(+), 18 deletions(-)
>> >
>> > diff --git a/tools/Makefile b/tools/Makefile
>> > index c66d4fb991..316ffb5ea6 100644
>> > --- a/tools/Makefile
>> > +++ b/tools/Makefile
>> > @@ -82,6 +82,7 @@ endif
>> >   ifneq ($(CONFIG_CCACHE)$(CONFIG_SDK),)
>> >   $(foreach tool, $(filter-out xz patch,$(tools-y)), $(eval 
>> > $(curdir)/$(tool)/compile += $(curdir)/ccache/compile))
>> >   tools-y += ccache
>> > +$(curdir)/ccache/compile := $(curdir)/cmake/compile $(curdir)/zstd/compile
>> >   endif
>> >
>> >   # in case there is no patch tool on the host we need to make patch tool a
>> >
>>
>> I am not sure if that is right. The v3 patch may create a circular 
>> dependency.
> That doesn't seem to be the case.
> 
> make tools/ccache/compile works perfectly fine with build/staging_dir removed.
> 
> Without that addition, ccache will not compile. Removing that line and
> moving the tool names up to line 83 also does not compile.
Yes, it may work in your tests, but the build shows a lot of warnings
like this one:
make[1]: Circular tools/libressl/compile <- tools/ccache/compile
dependency dropped.

Hannu is right about the circular dependency and his proposed solution.
Please fix your patch

- Felix

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


Re: [PATCH] build/prereq: requie make 4.1 or later

2020-12-13 Thread Felix Fietkau


On 2020-12-13 12:35, Hauke Mehrtens wrote:
> On 12/13/20 8:17 AM, Paul Spooren wrote:
>> FS#2086 "IS_TTY in the makefile is broken" reports flawed detection of
>> stdout piping to a file. The issue describes how e.g. terminal color
>> codes and up in log files if running make like `make > log.txt`.
>> 
>> The proposed solution uses the make variable "MAKE_TERMOUT", which was
>> introduced in make 4.1. All major distributions seem to updated to 4.1
>> or later, so this ideally dosn't break anything.
> 
> Ubuntu uses make >= 4.1 since 16.04 LTS. (all supported versions)
> https://distrowatch.com/table.php?distribution=ubuntu
> 
> RedHat 8 uses make 4.2.1, RedHat 7 uses 3.82 which would be too old.
> https://distrowatch.com/table.php?distribution=redhat
> 
> Debian uses make >= 4.1 since 9 stretch (oldstable)
> https://distrowatch.com/table.php?distribution=debian
> 
> Please check what is used on MacOS with Felix or Kevin.
macOS still ships an ancient and broken GNU make 3.81 version, which
forces you to install a newer version via homebrew, even without this
patch. No objection from me on raising the minimum version.

- Felix

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


Re: [PATCH] ramips/mt7621: refresh the kernel config

2020-12-02 Thread Felix Fietkau


On 2020-12-02 19:45, Adrian Schmutzler wrote:
>> -Original Message-
>> From: Felix Fietkau [mailto:n...@nbd.name]
>> Sent: Mittwoch, 2. Dezember 2020 19:25
>> To: Adrian Schmutzler ; 'Rui Salvaterra'
>> ; openwrt-devel@lists.openwrt.org
>> Subject: Re: [PATCH] ramips/mt7621: refresh the kernel config
>> 
>> 
>> Hi,
>> 
>> 
>> On 2020-12-02 18:32, Adrian Schmutzler wrote:
>> >> -Original Message-
>> >> From: openwrt-devel [mailto:openwrt-devel-
>> boun...@lists.openwrt.org]
>> >> On Behalf Of Rui Salvaterra
>> >> Sent: Mittwoch, 2. Dezember 2020 16:42
>> >> To: openwrt-devel@lists.openwrt.org
>> >> Cc: Rui Salvaterra 
>> >> Subject: [PATCH] ramips/mt7621: refresh the kernel config
>> >>
>> >> The removed config symbols are already enabled by the generic kernel
>> >> configuration (or by default), while the added ones are forcefully
>> >> enabled by the specific architecture.
>> >
>> > as already discussed on IRC, this is removing a lot of symbols, which makes
>> not much sense to me.
>> >
>> > It appears that this commit broke the behavior of "make kernel_oldconfig":
>> >
>> >
>> https://github.com/openwrt/openwrt/commit/a8fb12a7d62c94fd1bbda3327
>> 52e
>> > 0f8c794f3702
>> >
>> > Just reverting that commit results in something that "looks reasonable"
>> after make kernel_oldconfig.
>> >
>> > I'm marking that patch with "changes requested", though it's not really
>> Rui's fault in the first place.
>> >
>> > Let's see where this evolves to.
>> Removing a lot of symbols was the whole point behind my commit, and it
>> makes sense that any subsequent refresh has a big negative delta.
>> 
>> I tried to be careful about only filtering out auto-generated symbols.
> 
> But then I don't understand why target/linux/generic/config-filter
> contains all patterns followed by "is not set", when it's meant to match
> =y as well?
That's just an artifact to keep it in kconfig.pl syntax (since that's
what's being used to filter). The value of the symbols is irrelevant,
symbols matching the name will be filtered.

- Felix

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


Re: [PATCH] ramips/mt7621: refresh the kernel config

2020-12-02 Thread Felix Fietkau


Hi,


On 2020-12-02 18:32, Adrian Schmutzler wrote:
>> -Original Message-
>> From: openwrt-devel [mailto:openwrt-devel-boun...@lists.openwrt.org]
>> On Behalf Of Rui Salvaterra
>> Sent: Mittwoch, 2. Dezember 2020 16:42
>> To: openwrt-devel@lists.openwrt.org
>> Cc: Rui Salvaterra 
>> Subject: [PATCH] ramips/mt7621: refresh the kernel config
>> 
>> The removed config symbols are already enabled by the generic kernel
>> configuration (or by default), while the added ones are forcefully enabled by
>> the specific architecture.
> 
> as already discussed on IRC, this is removing a lot of symbols, which makes 
> not much sense to me.
> 
> It appears that this commit broke the behavior of "make kernel_oldconfig":
> 
> https://github.com/openwrt/openwrt/commit/a8fb12a7d62c94fd1bbda332752e0f8c794f3702
> 
> Just reverting that commit results in something that "looks reasonable" after 
> make kernel_oldconfig.
> 
> I'm marking that patch with "changes requested", though it's not really Rui's 
> fault in the first place.
> 
> Let's see where this evolves to.
Removing a lot of symbols was the whole point behind my commit, and it
makes sense that any subsequent refresh has a big negative delta.

I tried to be careful about only filtering out auto-generated symbols.
Did I accidentally hit any symbol with a prompt that shows up on make
afterwards?

- Felix

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


Re: [PATCH] libroxml: remove

2020-11-27 Thread Felix Fietkau


On 2020-11-27 10:36, Paul Spooren wrote:
> I'm happy to handle this but you're the maintainer. Please give your 
> blessing.
I hereby bless you :)

Thanks for taking care of this

- Felix

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


Re: [PATCH v2] build: create $(PKG_SYMVERS_DIR) if non-existent

2020-11-17 Thread Felix Fietkau

On 2020-11-18 06:54, Oldřich Jedlička wrote:
> út 17. 11. 2020 v 23:00 odesílatel Sebastian Kemper
>  napsal:
>>
>> Commit 5d76065 moved the creation of the symvers directory to
>> include/kernel-build.mk. This is fine when building from scratch. But
>> when unpacking an SDK the directory doesn't exist and because the kernel
>> won't be built (again) this directory will not be created by the build
>> system, causing build failure if make tries to copy files into it.
>>
>> This moves the creation of the symvers directory back into
>> include/kernel.mk so that the directory is created in any case.
>>
>> Signed-off-by: Sebastian Kemper 
>> ---
>>  include/kernel-build.mk | 1 -
>>  include/kernel.mk   | 1 +
>>  2 files changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/kernel-build.mk b/include/kernel-build.mk
>> index a698deec3c..22f7c4c7c7 100644
>> --- a/include/kernel-build.mk
>> +++ b/include/kernel-build.mk
>> @@ -136,7 +136,6 @@ define BuildKernel
>>$(LINUX_DIR)/.modules: export 
>> PKG_CONFIG_LIBDIR=$$(STAGING_DIR_HOST)/lib/pkgconfig
>>$(LINUX_DIR)/.modules: $(STAMP_CONFIGURED) $(LINUX_DIR)/.config FORCE
>> $(Kernel/CompileModules)
>> -   mkdir -p $(PKG_SYMVERS_DIR)
>> touch $$@
>>
>>$(LINUX_DIR)/.image: export STAGING_PREFIX=$$(STAGING_DIR_HOST)
>> diff --git a/include/kernel.mk b/include/kernel.mk
>> index 1ae9c6be29..e803ff44e7 100644
>> --- a/include/kernel.mk
>> +++ b/include/kernel.mk
>> @@ -140,6 +140,7 @@ endif
>>  PKG_EXTMOD_SUBDIRS ?= .
>>
>>  PKG_SYMVERS_DIR = $(KERNEL_BUILD_DIR)/symvers
>> +$(shell mkdir -p $(PKG_SYMVERS_DIR))
> 
> Wouldn't it be better to move this few lines below to the place which actually
> uses the folder - into collect_module_symvers? Just before
> 
>   mv $(PKG_BUILD_DIR)/Module.symvers $(PKG_SYMVERS_DIR)/$(PKG_NAME).symvers
Yes, that's a much better solution. Running mkdir inside make context
shell expansion can easily trigger some really nasty build system
performance issues.

- Felix

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


Re: [PATCH] Revert "netifd: update to the latest version"

2020-11-13 Thread Felix Fietkau


On 2020-11-13 11:34, Rui Salvaterra wrote:
> Hi, Felix,
> 
> On Fri, 13 Nov 2020 at 10:25, Felix Fietkau  wrote:
>>
>> Please git-bisect this and show me the config that doesn't work anymore,
>> so that I can fix it properly.
> 
> I can't bisect this now, as I'm using the router. The config (in
> /etc/config/network), however, is trivial:
> 
> config switch
> option enable_vlan '1'
> option name 'switch0'
> option reset '1'
> 
> config switch_vlan
> option device 'switch0'
> option ports '2 3 4 5 0t'
> option vlan '1'
> 
> config switch_vlan
> option device 'switch0'
> option ports '1 0t'
> option vlan '2'
> 
> Symptom: with the netifd update, only the VLAN 1 (eth0.1@eth0)
> interface exists, VLAN 2 (eth0.2@eth0) doesn't.
Please show me the full /etc/config/network so I can test with it as-is.

Thanks,

- Felix

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


Re: [PATCH] Revert "netifd: update to the latest version"

2020-11-13 Thread Felix Fietkau


On 2020-11-13 11:19, Rui Salvaterra wrote:
> This broke the switch VLAN configuration on at least Archer C6 v2 EU and 
> Archer
> C7 (both switchdev-based one-armed routers).
> 
> Signed-off-by: Rui Salvaterra 
Please git-bisect this and show me the config that doesn't work anymore,
so that I can fix it properly.

Thanks,

- Felix

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


Re: Someone working on kernel 5.9?

2020-10-29 Thread Felix Fietkau
On 2020-10-29 13:11, Koen Vandeputte wrote:
> 
> On 29.10.20 11:37, Andrey Jr. Melnikov wrote:
>> Koen Vandeputte  wrote:
>>
>>> On 03.10.20 17:11, Vincent Wiemann wrote:
 Hi folks,

 is anybody working on 5.9, already?
 I'd like to do some testing with io_uring on ath79 devices,
 but the features needed require a version > 5.7.
 Please let me know!
>>> Not yet currently as I'm pretty occupied with AI stuff, but I might give
>>> it a try within 1 .. 2 weeks.
>> before you start - in 5.8 kernel build process slightly changed, so openwrt
>> "build module first, kernel last" not working, vmlinux must be build before
>> modules now.
>> mtd subsystem partition code massive changed - mtdsplit drivers need rewrite.
> 
> Thanks,
> 
> I'll take a look at it.
> I did encounter the mtdsplit stuff you mention.
> 
> Just swapped to 5.10-rc1 as it will be the next LTS.
I have 5.9 working on the mediatek target in my staging tree:
https://git.openwrt.org/?p=openwrt/staging/nbd.git;a=summary

- Felix

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


Re: [RFC PATCH] ath79: remove model name from LED labels

2020-09-30 Thread Felix Fietkau


On 2020-09-30 13:43, Adrian Schmutzler wrote:
>> -Original Message-
>> From: openwrt-devel [mailto:openwrt-devel-boun...@lists.openwrt.org]
>> On Behalf Of Adrian Schmutzler
>> Sent: Sonntag, 27. September 2020 23:55
>> To: openwrt-devel@lists.openwrt.org
>> Subject: RE: [RFC PATCH] ath79: remove model name from LED labels
> 
> Hi all,
> 
> since there doesn't seem to be any interest in the subject initially, I'd 
> also be interested if there are any general objections towards dropping 
> "devicename" and using "color:function" scheme on LEDs.
> 
> I'd actually be quite happy to get rid of this obstacle that annoyed me 
> consistently throughout my involvement with OpenWrt.
The devicename in the LEDs has also annoyed me often enough. No idea why
it was added in the first place.
I think getting rid of it is a really good idea.

- Felix

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


Re: [PATCH v2] mac80211: fix regression in sta connection monitor

2020-09-26 Thread Felix Fietkau

Hi Georgi,

thanks for testing and for your insight into this issue.

On 2020-09-26 06:41, Georgi Valkov wrote:
> Hi Felix!
> 
> With your latest suggestion, it takes between 10 and 17 hours for the 
> connection to drop, then long five minutes to reconnect.
> Notice the order of code execution in the original code of 
> ieee80211_sta_tx_notify():
> probe_send_count is always cleared when ack is true. But before clearing 
> probe_send_count, we need to check if ieee80211_is_any_nullfunc() returns 
> true and probe_send_count > 0: if yes, we must call ieee80211_queue_work(). 
> You cleared probe_send_count ahead of time, so the condition to run 
> ieee80211_queue_work() will never be met.
The condition in ieee80211_sta_tx_notify may not be met, but
ieee80211_queue_work is called from ieee80211_tx_status_ext in that
case. Any idea why that is not enough?

> To spare your time, I did spend one week to find the cause, then another 
> learning every detail about the code and testing various solutions, including 
> those you proposed. While I do not have experience with mac80211’s design, 
> I’m quite good at preserving the exact behaviour during large scale 
> refactoring. And in my fix I tried changing as little as possible to keep the 
> patch small, preserving both your changes and the original design behaviour.
And the reason I'm still proposing changes to it is because your patch
does not take into account no-skb or 802.3 tx status.
I'll try to make a v3 patch that leaves more of the original code intact
at the cost of a little more duplication.

- Felix

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


Re: [PATCH] mac80211: fix regression in sta connection monitor

2020-09-22 Thread Felix Fietkau
On 2020-09-22 09:28, Georgi Valkov wrote:
> Hi Felix!
> 
> In the original code before the regression, as well as in the fix I
> proposed, probe_send_count was always reset if ack is true, while in
> your fix it will not be reset when non-null frames are received. The
> connection failed again after 10 hours and the STA remained offline for
> two minutes instead of two seconds. I should also note that I spent
> multiple days testing the fix I proposed and I had no disconnects.
> Please revise your fix again!
Thanks for the info. Handling non-null frames in that function is
incomplete, because it does not cover the 802.3 tx status path.
I'll make v2 patch that checks the timestamp of the last ACK in
ieee80211_sta_work instead.

- Felix

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


Re: [PATCH] ath79: ar8216: make switch register access atomic

2020-09-21 Thread Felix Fietkau


On 2020-09-21 18:26, Chuanhong Guo wrote:
> Hi!
> 
> On Mon, Sep 21, 2020 at 9:24 PM Felix Fietkau  wrote:
>> > +
>> > + split_addr((u32) reg, , , );
>> > +
>> > ++local_irq_save(flags);
>> > + mutex_lock(>mdio_lock);
>> Taking a mutex in an irq-disabled section is basically asking for a
>> deadlock. What router is this issue reproduced on?
> 
> Any ar934x/qca953x router using built-in switch, based on
> Flyspray bug reports. I can reproduce it on ar9341/tp-link-wr841n-v8
> As there's an ancient commit about the same issue I guess
> it appears on ar724x/ar933x as well.
> here's the commit message:
> commit 5d77f370d695c9a70f25ffb8367db64efadaaedd
> Author: Gabor Juhos 
> Date:   Sun May 8 16:32:53 2011 +
> 
> ar71xx: ag71xx: make switch register access atomic
> 
> Reading of the PHY registers occasionally returns with bogus values
> under heavy load. This misleads the PHY driver and thus causes false
> link/speed change notifications which leads to performance loss.
> [and some dmesg after this]
> 
>> What else is running at the time this happens?
> 
> I'm using a minimal wr841n-v8 image. (make menuconfig,
> select the device, save and build the firmware)
> no extra background tasks are running.
> swconfig led trigger is used so the kernel is constantly
> polling port status from switch in the background.
I have another idea what you could try. Many SoCs enable both MDIO
busses, and maybe they have some shared resources/state in the hw.

You could try making a global spinlock in ag71xx_mdio.c and do
spin_lock_bh(_lock) in ag71xx_mdio_mii_read and ag71xx_mdio_mii_write

- Felix

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


Re: [PATCH] ath79: ar8216: make switch register access atomic

2020-09-21 Thread Felix Fietkau
On 2020-09-21 08:57, Chuanhong Guo wrote:
> reg accesses on integrated ar8229 sometimes fails. As a result, phy read
> got incorrect port status and wan link goes down and up mysteriously.
> After comparing ar8216 with the old driver, these local_irq_save/restore
> calls are the only meaningful differences I could find and it does fix
> the issue.
> The same changes were added in svn r26856 by Gabor Juhos:
> ar71xx: ag71xx: make switch register access atomic
> 
> As I can't find the underlying problem either, this hack is broght
> back to fix the unstable link issue.
> This hack is only suitable for ath79 mdio and may easily break the
> driver on other platform. Limit it to ath79-only as a target patch.
> 
> Fixes: FS#2216
> Fixes: FS#3226
> Signed-off-by: Chuanhong Guo 
> ---
>  .../930-ar8216-make-reg-access-atomic.patch   | 61 +++
>  1 file changed, 61 insertions(+)
>  create mode 100644 
> target/linux/ath79/patches-5.4/930-ar8216-make-reg-access-atomic.patch
> 
> diff --git 
> a/target/linux/ath79/patches-5.4/930-ar8216-make-reg-access-atomic.patch 
> b/target/linux/ath79/patches-5.4/930-ar8216-make-reg-access-atomic.patch
> new file mode 100644
> index 00..42f3305195
> --- /dev/null
> +++ b/target/linux/ath79/patches-5.4/930-ar8216-make-reg-access-atomic.patch
> @@ -0,0 +1,61 @@
> +From b3797d1a92afe97c173b00fdb7824cedba24eef0 Mon Sep 17 00:00:00 2001
> +From: Chuanhong Guo 
> +Date: Sun, 20 Sep 2020 01:00:45 +0800
> +Subject: [PATCH] ath79: ar8216: make switch register access atomic
> +
> +due to some unknown reason these register accesses sometimes fail
> +on the integrated switch without this patch.
> +
> +THIS ONLY WORKS ON ATH79 AND MAY BREAK THE DRIVER ON OTHER PLATFORMS!
> +The mdio bus on ath79 works in polling mode and doesn't rely on
> +any interrupt. This patch breaks the driver on any mdio master
> +with interrupts used.
> +
> +---
> +--- a/drivers/net/phy/ar8216.c
>  b/drivers/net/phy/ar8216.c
> +@@ -253,12 +253,14 @@ ar8xxx_mii_write32(struct ar8xxx_priv *p
> + u32
> + ar8xxx_read(struct ar8xxx_priv *priv, int reg)
> + {
> ++unsigned long flags;
> + struct mii_bus *bus = priv->mii_bus;
> + u16 r1, r2, page;
> + u32 val;
> + 
> + split_addr((u32) reg, , , );
> + 
> ++local_irq_save(flags);
> + mutex_lock(>mdio_lock);
Taking a mutex in an irq-disabled section is basically asking for a
deadlock. What router is this issue reproduced on? What else is running
at the time this happens?

- Felix

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


Re: [PATCH 3/3] mediatek: mt7623: use bash for generating bootable images

2020-09-08 Thread Felix Fietkau
On 2020-09-08 10:39, David Woodhouse wrote:
> On Tue, 2020-09-08 at 10:31 +0200, Felix Fietkau wrote:
>> On 2020-09-08 10:26, David Woodhouse wrote:
>> > It turns out that 'echo -e' isn't portable; it doesn't work in the dash
>> > builtin echo and Ubuntu users are complaining.
>> > 
>> > I can't even get octal (specified by POSIX) to work consistently because
>> > those  variants of 'echo' which *do* support -e don't seem to interpret
>> > octalwithout it.
>> > 
>> > I could switch to /bin/echo but using -e with that isn't actually
>> > portable *either* even though it works today.
>> > 
>> > For now just stick with bash, and use its builtin. We may end up using
>> > something else entirely; perhaps perl.
>> > 
>> > Signed-off-by: David Woodhouse 
>> 
>> Have you considered using printf instead of echo?
> 
> I played with that for a while a few weeks ago and didn't manage to get
> it working in all environments either. Sometimes I need '\\x' and
> sometimes just '\x'.
> 
> At the time I gave up as it was mostly a theoretical problem and our
> own builds worked, and I had more important things to fix.
> 
> But now users are hitting this in practice¹ so I need to at least come
> up with a stopgap solution, and this was it.
> 
> Better solutions welcome — and we do have a handful of other places in
> the tree that need fixing too, when we work out what the best approach
> is.
Sure, using bash is fine. We already require it for many other things.

- Felix

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


Re: [PATCH 3/3] mediatek: mt7623: use bash for generating bootable images

2020-09-08 Thread Felix Fietkau
On 2020-09-08 10:26, David Woodhouse wrote:
> It turns out that 'echo -e' isn't portable; it doesn't work in the dash
> builtin echo and Ubuntu users are complaining.
> 
> I can't even get octal (specified by POSIX) to work consistently because
> those  variants of 'echo' which *do* support -e don't seem to interpret
> octalwithout it.
> 
> I could switch to /bin/echo but using -e with that isn't actually
> portable *either* even though it works today.
> 
> For now just stick with bash, and use its builtin. We may end up using
> something else entirely; perhaps perl.
> 
> Signed-off-by: David Woodhouse 
Have you considered using printf instead of echo?

- Felix

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


Re: [PATCH 2/2] toolchain/musl: remove several GNU headers

2020-08-28 Thread Felix Fietkau
On 2020-08-29 05:28, Rosen Penev wrote:
> Remove GLOB_ONLYDIR patch. Only fstools relies on it. fstools can be fixed
> separately.
> 
> Remove woresize.h file. It seems to be for an old version of GCC.
> 
> Remove features.h and glibc-types files. Same as above.
> 
> Remove sys/cdefs.h. This is a deprecated header. Patches to fix packages
> that use it have already been patched.
> 
> Tested with all packages in the base tree. They all compile.
Did you verify all packages in the feeds as well? When I added some of
these headers years ago, it was because some packages there were still
relying on them.

- Felix

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


Re: [Bridge] [RFC PATCH net-next] bridge: Implement MLD Querier wake-up calls / Android bug workaround

2020-08-23 Thread Felix Fietkau
On 2020-08-23 17:42, Linus Lüssing wrote:
> On Sun, Aug 16, 2020 at 03:08:13PM -0700, Stephen Hemminger wrote:
>> Rather than adding yet another feature to the bridge, could this hack be 
>> done by
>> having a BPF hook? or netfilter module?
> 
> Hi Stephen,
> 
> Thanks for the constructive feedback and suggestions!
> 
> The netfilter approach sounds tempting. However as far as I know
> OpenWrt's firewall has no easy layer 2 netfilter integration yet.
> So it has default layer 3 netfilter rules, but not for layer 2.
> 
> Ideally I'd want to work towards a solution where things "just
> work as expected" when a user enables "IGMP Snooping" in the UI.
> I could hack the netfilter rules into netifd, the OpenWrt network
> manager, when it configures the bridge. But not sure if the
> OpenWrt maintainers would like that...
> 
> Any preferences from the OpenWrt maintainers side?
Enabling bridge netfilter comes with a very significant performance
cost, so it's not something that should be done in a default configuration.

- Felix

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


Re: [PATCH 3/3] toolchain: remove BROKEN from uClibc-ng

2020-08-15 Thread Felix Fietkau
On 2020-08-16 00:27, Rosen Penev wrote:
> On Sat, Aug 15, 2020 at 2:02 AM Felix Fietkau  wrote:
>>
>> On 2020-08-14 23:30, Rosen Penev wrote:
>> > uClibc-ng works fine. Additionally, most packages have been fixed to
>> > compile with uClibc-ng.
>> >
>> > Signed-off-by: Rosen Penev 
>> It does not make sense to maintain uClibc-ng for anything other than arc
>> (the only arch that needs it). In fact, once arc is switched to glibc,
>> we should remove uClibc-ng entirely.
> My opinion is that it's fairly useful for testing purposes.
> 
> More to the patch, there's nothing broken about uClibc-ng.
"testing purposes" is exactly why the dependency says
"depends on BROKEN || arc" instead of "depends on !arc"

This makes it clear that it's an unsupported configuration, but it still
allows you to enable it with a few more steps.

I guess a way to make it even more clear would be to introduce an
'UNSUPPORTED' config symbol to distinguish it from 'BROKEN', but I'm not
sure if it's worth adding.

- Felix

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


Re: [PATCH 3/3] toolchain: remove BROKEN from uClibc-ng

2020-08-15 Thread Felix Fietkau
On 2020-08-14 23:30, Rosen Penev wrote:
> uClibc-ng works fine. Additionally, most packages have been fixed to
> compile with uClibc-ng.
> 
> Signed-off-by: Rosen Penev 
It does not make sense to maintain uClibc-ng for anything other than arc
(the only arch that needs it). In fact, once arc is switched to glibc,
we should remove uClibc-ng entirely.

- Felix

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


Re: ath25 and rb532 fail after mac80211 changes

2020-08-14 Thread Felix Fietkau
On 2020-08-14 11:16, Adrian Schmutzler wrote:
> Hi,
> 
> after "mac80211: merge performance improvement patches" [1], build fails on a 
> few 4.14 targets, namely ath25, rb532 (and ar71xx):
> 
> 
> 
> So, we have to decide whether
> 1. we fix this
> 2. we leave the buildbots broken
> 3. we remove those 4.14 targets in master (ath25, rb532)
> 
> Personally, I prefer option 3, as it's only a matter of time when we drop 
> those targets anyway.
I'm going to push backports for 4.14 soon. I'm testing them at the
moment. Sorry about the build breakage.

- Felix

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


Re: [PATCH] kernel: add ar5523 driver

2020-08-11 Thread Felix Fietkau
On 2020-08-11 14:41, mohammad rasim wrote:
> The driver currently only support managed and monitor mode
> 
> Signed-off-by: mohammad rasim 
Are you planning on using this, or did you just add it because it's in
the source tree?

- Felix

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


Re: [PATCH 1/4] tools: add fakeroot

2020-08-08 Thread Felix Fietkau
On 2020-08-07 23:12, Paul Spooren wrote:
> From: Thomas Petazzoni 
> 
> SELinux support requires setting the appropriate SELinux security context
> to files and directories, which needs to happen at build time in order
> to support read-only root filesystem scenarios. In order to create these
> security contexts, we will have to run some SELinux-specific tools on
> the host machine, but that requires root access. This adds support for
> fakeroot, which the build process will use to run the SELinux security
> context creation and the image creation.
> 
> Signed-off-by: Thomas Petazzoni 
> 
> Apply to current master, and adjust commit message
> 
> Thomas' original work is available at
> http://lists.infradead.org/pipermail/openwrt-devel/2019-November/025976.html.
> 
> Signed-off-by: W. Michael Petullo 
> [add rules.mk FAKEROOT variable]
> Signed-off-by: Paul Spooren 
> ---
> --- /dev/null
> +++ b/tools/fakeroot/Makefile
> @@ -0,0 +1,20 @@
> +#
> +# This is free software, licensed under the GNU General Public License v2.
> +# See /LICENSE for more information.
> +#
> +include $(TOPDIR)/rules.mk
> +
> +PKG_NAME:=fakeroot
> +PKG_VERSION:=1.20.2
This version is rather ancient and also breaks on macOS.
I've updated it, fixed macOS support and pushed the result to my staging
tree at https://git.openwrt.org/?p=openwrt/staging/nbd.git;a=summary

Feel free to pick my version from there if you respin or merge the
series. When you do, please test it on a Linux machine again to make
sure I didn't break anything with my fixes :)

- Felix

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


[RFC netifd 1/3] bridge: add support for defining port member vlans via hotplug ops

2020-08-07 Thread Felix Fietkau
Signed-off-by: Felix Fietkau 
---
 bridge.c| 105 +++-
 config.c|   1 +
 device.h|   4 +-
 interface.c |  10 +++--
 interface.h |   2 +-
 ubus.c  |   5 ++-
 wireless.c  |   4 +-
 7 files changed, 120 insertions(+), 11 deletions(-)

diff --git a/bridge.c b/bridge.c
index 14d497298352..92eea9f7e22e 100644
--- a/bridge.c
+++ b/bridge.c
@@ -122,6 +122,11 @@ struct bridge_member {
char name[];
 };
 
+struct bridge_vlan_hotplug_port {
+   struct list_head list;
+   struct bridge_vlan_port port;
+};
+
 static void
 bridge_reset_primary(struct bridge_state *bst)
 {
@@ -153,6 +158,7 @@ bridge_reset_primary(struct bridge_state *bst)
 static struct bridge_vlan_port *
 bridge_find_vlan_member_port(struct bridge_member *bm, struct bridge_vlan 
*vlan)
 {
+   struct bridge_vlan_hotplug_port *port;
const char *ifname = bm->dev.dev->ifname;
int i;
 
@@ -163,6 +169,13 @@ bridge_find_vlan_member_port(struct bridge_member *bm, 
struct bridge_vlan *vlan)
return >ports[i];
}
 
+   list_for_each_entry(port, >hotplug_ports, list) {
+   if (strcmp(port->port.ifname, ifname) != 0)
+   continue;
+
+   return >port;
+   }
+
return NULL;
 }
 
@@ -415,9 +428,25 @@ bridge_remove_member(struct bridge_member *bm)
 static void
 bridge_free_member(struct bridge_member *bm)
 {
+   struct bridge_state *bst = bm->bst;
struct device *dev = bm->dev.dev;
+   const char *ifname = dev->ifname;
+   struct bridge_vlan *vlan;
 
bridge_remove_member(bm);
+
+   vlist_for_each_element(>dev.vlans, vlan, node) {
+   struct bridge_vlan_hotplug_port *port, *tmp;
+
+   list_for_each_entry_safe(port, tmp, >hotplug_ports, list) 
{
+   if (strcmp(port->port.ifname, ifname) != 0)
+   continue;
+
+   list_del(>list);
+   free(port);
+   }
+   }
+
device_remove_user(>dev);
 
/*
@@ -616,11 +645,66 @@ bridge_add_member(struct bridge_state *bst, const char 
*name)
bridge_create_member(bst, name, dev, false);
 }
 
+static void
+bridge_hotplug_create_member_vlans(struct bridge_state *bst, struct blob_attr 
*vlans, const char *ifname)
+{
+   struct bridge_vlan *vlan;
+   struct blob_attr *cur;
+   int rem;
+
+   if (!vlans)
+   return;
+
+   blobmsg_for_each_attr(cur, vlans, rem) {
+   struct bridge_vlan_hotplug_port *port;
+   uint16_t flags = BRVLAN_F_UNTAGGED;
+   char *name_buf;
+   unsigned int vid;
+   char *end;
+
+   if (blobmsg_type(cur) != BLOBMSG_TYPE_STRING)
+   continue;
+
+   vid = strtoul(blobmsg_get_string(cur), , 0);
+   if (!vid || vid > 4095)
+   continue;
+
+   vlan = vlist_find(>dev.vlans, , vlan, node);
+   if (!vlan)
+   continue;
+
+   if (end && *end) {
+   if (*end != ':')
+   continue;
+
+   for (end++; *end; end++) {
+   switch (*end) {
+   case 't':
+   flags &= ~BRVLAN_F_UNTAGGED;
+   break;
+   case '*':
+   flags |= BRVLAN_F_PVID;
+   break;
+   }
+   }
+   }
+
+   port = calloc_a(sizeof(*port), _buf, strlen(ifname) + 1);
+   if (!port)
+   continue;
+
+   port->port.flags = flags;
+   port->port.ifname = strcpy(name_buf, ifname);
+   list_add_tail(>list, >hotplug_ports);
+   }
+}
+
 static int
-bridge_hotplug_add(struct device *dev, struct device *member)
+bridge_hotplug_add(struct device *dev, struct device *member, struct blob_attr 
*vlan)
 {
struct bridge_state *bst = container_of(dev, struct bridge_state, dev);
 
+   bridge_hotplug_create_member_vlans(bst, vlan, member->ifname);
bridge_create_member(bst, member->ifname, member, true);
 
return 0;
@@ -899,6 +983,20 @@ bridge_vlan_equal(struct bridge_vlan *v1, struct 
bridge_vlan *v2)
return true;
 }
 
+static void
+bridge_vlan_free(struct bridge_vlan *vlan)
+{
+   struct bridge_vlan_hotplug_port *port, *tmp;
+
+   if (!vlan)
+   return;
+
+   list_for_each_entry_safe(port, tmp, >hotplug_ports, list)
+   free(port);
+
+   free(vlan);
+}
+
 static void
 bridge_vlan_update(struct vlist_tree *tree, struct vlist_no

[RFC netifd 2/3] vlan: add pass-through hotplug ops that pass the VLAN info to the bridge

2020-08-07 Thread Felix Fietkau
Signed-off-by: Felix Fietkau 
---
 vlan.c | 64 ++
 1 file changed, 64 insertions(+)

diff --git a/vlan.c b/vlan.c
index 6270d95d755d..7d37fef33e2f 100644
--- a/vlan.c
+++ b/vlan.c
@@ -18,6 +18,8 @@
 #include "netifd.h"
 #include "system.h"
 
+static struct blob_buf b;
+
 struct vlan_device {
struct device dev;
struct device_user dep;
@@ -36,6 +38,66 @@ static void free_vlan_if(struct device *iface)
free(vldev);
 }
 
+static int
+vlan_hotplug_add(struct device *dev, struct device *member, struct blob_attr 
*vlan)
+{
+   struct vlan_device *vldev = container_of(dev, struct vlan_device, dev);
+   void *a;
+
+   dev = vldev->dep.dev;
+   if (!dev || !dev->hotplug_ops)
+   return UBUS_STATUS_NOT_SUPPORTED;
+
+   blob_buf_init(, 0);
+   a = blobmsg_open_array(, "vlans");
+   blobmsg_printf(, NULL, "%d", vldev->id);
+   blobmsg_close_array(, a);
+
+   return dev->hotplug_ops->add(dev, member, blobmsg_data(b.head));
+}
+
+static int
+vlan_hotplug_del(struct device *dev, struct device *member)
+{
+   struct vlan_device *vldev = container_of(dev, struct vlan_device, dev);
+
+   dev = vldev->dep.dev;
+   if (!dev || !dev->hotplug_ops)
+   return UBUS_STATUS_NOT_SUPPORTED;
+
+   return dev->hotplug_ops->del(dev, member);
+}
+
+static int
+vlan_hotplug_prepare(struct device *dev)
+{
+   struct vlan_device *vldev = container_of(dev, struct vlan_device, dev);
+
+   dev = vldev->dep.dev;
+   if (!dev || !dev->hotplug_ops)
+   return UBUS_STATUS_NOT_SUPPORTED;
+
+   return dev->hotplug_ops->prepare(dev);
+}
+
+static void vlan_hotplug_check(struct vlan_device *vldev)
+{
+   static const struct device_hotplug_ops hotplug_ops = {
+   .prepare = vlan_hotplug_prepare,
+   .add = vlan_hotplug_add,
+   .del = vlan_hotplug_del
+   };
+   struct device *dev = vldev->dep.dev;
+
+   if (!dev || !dev->hotplug_ops || avl_is_empty(>vlans.avl)) {
+   vldev->dev.hotplug_ops = NULL;
+   return;
+   }
+
+   vldev->dev.hotplug_ops = _ops;
+}
+
+
 static int vlan_set_device_state(struct device *dev, bool up)
 {
struct vlan_device *vldev;
@@ -75,6 +137,7 @@ static void vlan_dev_cb(struct device_user *dep, enum 
device_event ev)
device_set_present(>dev, false);
break;
case DEV_EVENT_UPDATE_IFNAME:
+   vlan_hotplug_check(vldev);
vldev->dev.hidden = dep->dev->hidden;
if (snprintf(name, sizeof(name), "%s.%d", dep->dev->ifname,
 vldev->id) >= sizeof(name) - 1 ||
@@ -139,6 +202,7 @@ static struct device *get_vlan_device(struct device *dev, 
int id, bool create)
 
vldev->dep.cb = vlan_dev_cb;
device_add_user(>dep, dev);
+   vlan_hotplug_check(vldev);
 
return >dev;
 
-- 
2.28.0


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


[RFC netifd 3/3] vlandev: add pass-through hotplug ops that pass the VLAN info to the bridge

2020-08-07 Thread Felix Fietkau
Only used for 802.1q devices

Signed-off-by: Felix Fietkau 
---
 vlandev.c | 66 +++
 1 file changed, 66 insertions(+)

diff --git a/vlandev.c b/vlandev.c
index 10b78e241576..7efb5f42b5c4 100644
--- a/vlandev.c
+++ b/vlandev.c
@@ -45,6 +45,7 @@ static const struct uci_blob_param_list vlandev_attr_list = {
 };
 
 static struct device_type vlan8021q_device_type;
+static struct blob_buf b;
 
 struct vlandev_device {
struct device dev;
@@ -57,6 +58,67 @@ struct vlandev_device {
struct vlandev_config config;
 };
 
+static int
+vlandev_hotplug_add(struct device *dev, struct device *member, struct 
blob_attr *vlan)
+{
+   struct vlandev_device *mvdev = container_of(dev, struct vlandev_device, 
dev);
+   void *a;
+
+   dev = mvdev->parent.dev;
+   if (!dev || !dev->hotplug_ops)
+   return UBUS_STATUS_NOT_SUPPORTED;
+
+   blob_buf_init(, 0);
+   a = blobmsg_open_array(, "vlans");
+   blobmsg_printf(, NULL, "%d", mvdev->config.vid);
+   blobmsg_close_array(, a);
+
+   return dev->hotplug_ops->add(dev, member, blobmsg_data(b.head));
+}
+
+static int
+vlandev_hotplug_del(struct device *dev, struct device *member)
+{
+   struct vlandev_device *mvdev = container_of(dev, struct vlandev_device, 
dev);
+
+   dev = mvdev->parent.dev;
+   if (!dev || !dev->hotplug_ops)
+   return UBUS_STATUS_NOT_SUPPORTED;
+
+   return dev->hotplug_ops->del(dev, member);
+}
+
+static int
+vlandev_hotplug_prepare(struct device *dev)
+{
+   struct vlandev_device *mvdev = container_of(dev, struct vlandev_device, 
dev);
+
+   dev = mvdev->parent.dev;
+   if (!dev || !dev->hotplug_ops)
+   return UBUS_STATUS_NOT_SUPPORTED;
+
+   return dev->hotplug_ops->prepare(dev);
+}
+
+static void vlandev_hotplug_check(struct vlandev_device *mvdev)
+{
+   static const struct device_hotplug_ops hotplug_ops = {
+   .prepare = vlandev_hotplug_prepare,
+   .add = vlandev_hotplug_add,
+   .del = vlandev_hotplug_del
+   };
+   struct device *dev = mvdev->parent.dev;
+
+   if (!dev || !dev->hotplug_ops || avl_is_empty(>vlans.avl) ||
+   dev->type != _device_type) {
+   mvdev->dev.hotplug_ops = NULL;
+   return;
+   }
+
+   mvdev->dev.hotplug_ops = _ops;
+}
+
+
 static void
 vlandev_base_cb(struct device_user *dev, enum device_event ev)
 {
@@ -69,6 +131,9 @@ vlandev_base_cb(struct device_user *dev, enum device_event 
ev)
case DEV_EVENT_REMOVE:
device_set_present(>dev, false);
break;
+   case DEV_EVENT_UPDATE_IFNAME:
+   vlandev_hotplug_check(mvdev);
+   break;
default:
return;
}
@@ -179,6 +244,7 @@ vlandev_config_init(struct device *dev)
basedev = device_get(blobmsg_data(mvdev->ifname), true);
 
device_add_user(>parent, basedev);
+   vlandev_hotplug_check(mvdev);
 }
 
 static void vlandev_qos_mapping_list_apply(struct vlist_simple_tree 
*qos_mapping_li, struct blob_attr *list)
-- 
2.28.0


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


Re: [RFC PATCH v2 0/1] Introduce UCI support for configuring DSA VLAN filter rules

2020-07-24 Thread Felix Fietkau
On 2020-07-24 18:44, Jo-Philipp Wich wrote:
> Hi Felix,
> 
>> [...]
>> 
>> For a simple default config, you could have this:
>> 
>> # network
>> config device
> option type bridge  # I assume this is needed as well
>>  option name switch0
Correct.

>> config bridge-vlan
>>  option vlan 1
>>  option ports "lan1 lan2 lan3 lan4"
>> 
>> config interface lan
>>  option ifname switch0.1
>> 
>> 
>> # wireless
>> 
>> config wifi-iface
>>  option network lan
>> 
>> 
>> In this case, wlan0 would be added to switch0 and set to VLAN 1 untagged
>> by default.
>> 
>> If you want it on VLAN 10 tagged/PVID instead, you could do:
>>  option network-vlan "10:t*"
>> 
>> 
>> What do you think?
> 
> I did think about it some more, also in context of a LuCI implementation and
> the special role of wifi and I am convinced now that this approach generally
> makes sense.
> 
> However for the vlan I wonder if we should simply use "option vid 10" since
> setting anything besides an egress untagged pvid does not make sense for wifi.
I think more complex VLAN settings make sense for WDS if you want to
carry multiple networks over the link.

> So your second example above would become:
> 
>   config wifi-iface
> option network lan
> option vid 10  # instead of inheriting vid 1, use 10 as pvid
> 
> 
> Also, just to clarify... assuming a:
> 
>   config interface foo
> option ifname somevlanbridge0.456
> 
> and an wifi iface without an explicit vid override:
> 
>   config wifi-iface
> option network foo
> 
> ... we would inherit vid 456 and set as pvid, right? Or are we are always
> going to default to 1?
It would inherit 456 to keep it in sync with the VLAN based network.

- Felix

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


Re: [RFC PATCH v2 0/1] Introduce UCI support for configuring DSA VLAN filter rules

2020-07-24 Thread Felix Fietkau
On 2020-07-24 10:37, Jo-Philipp Wich wrote:
> Hi Luiz,
> 
> I mostly agree with your proposal (though I'd call "device_for" simply
> "bridge" instead but that's details).
> 
> I don't think everything can be simply switched in one go but I do think your
> proposal could be broken down into the following measures.
> 
> The simple things:
> 
>  - Rename "config wifi-device" to "config radio", keep supporting the old
>name for backwards compatibility
Not sure how much that would help

>  - For network.device sections, make "option name" optional and default it
>to the section name, ignore section if it is anonymous and no option name
>is set
ACK

> The harder things:
> 
>  - Untangle the meaning of "option ifname" - for some protocols it specifies
>the existing device to use, for some protocols, e.g. tunnel ones, it
>specifies the name of the device to create. Ideally we should switch to
>"option device"  instead and introduce an "option name" for those cases
>where the resulting device name is controllable. This would also clear up
>some quirks with protocol types that both use a physical link and create a
>tunnel interface on top like PPPoE. One could then use "option device eth0"
>and "option name pppoe-wan" to specify both the L2 device to use and the
>name of the L3 device to create.
ACK

>  - Support "@" style notation for "option device" to resolve netdev names once
>the above untangling is done>
>  - Handling name clashes and device name limitations. Given tunnel devices
>with arbitrary names and - since the introduction of DSA - existing netdevs
>with generic names like "wan", "lan1" etc. clashes between configured
>bridge or tunnel devices and existing netdevs become likely. What should
>happen if a bridge with name "wan" is declared and a "wan" DSA port device
>already exists on the system? What should happen if a chosen name exceeds
>the 15 byte ifname limit?
> 
>  - Transform "config wifi-iface" in the wireless config into "config device"
>in the network config and handle it like yet another device type which
>happens to understand the various logical wifi network properties like
>SSID, encryption etc. Problem here: a logical wifi network does not really
>represent a 1:1 relationship to a netdev in all cases, an AP/VLAN interface
>for example will create a new vlan-subdevice for each associated station,
>so one wifi network can result in N+1 wifi netdevs. The only sensible
>use case for such uci "devices" would be associating them with a bridge
>or to leave them alone in case something outside of netifd's realm is
>somehow dealing with the existing vlan netdevs. What likely would not work
>is associating them with a "config interface" that specifies "proto static"
>since multiple netdev would get the same subnets assigned then
I think this one is very likely to cause a lot more problems than it
would solve:
You already mentioned the lack of a 1:1 relationship between wifi
interfaces and netdevs. That alone would probably introduce a lot of
implementation complexity (and possibly subtle bugs).
Another issue is that some drivers have limitations on the order in
which devices are brought up, and 802.11ax introduces implicit
dependencies between interfaces (transmitted vs non-transmitted BSS).
All of this does not map well to how netifd treats devices at the moment.

I'd strongly prefer keeping the current wireless config structure
(separate from network interfaces/devices).

- Felix

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


Re: [RFC PATCH v2 0/1] Introduce UCI support for configuring DSA VLAN filter rules

2020-07-23 Thread Felix Fietkau

> On 23. Jul 2020, at 14:17, Jo-Philipp Wich  wrote:
> 
> Hi,
> 
>> 1. Have VLAN devices on top of vlan-enabled bridges to define hotplug
>> ops where applicable, so LAN could be a plain VLAN interface switch0.1
>> instead of its own bridge.
>> 2. With these wrapper hotplug ops, a default VLAN would be passed as
>> well, unless overwritten by other VLAN settings (see below)
>> 3. In addition to option network, allow specifying option network-vlan
>> in the wifi-iface section and have it contain a list of VLANs +
>> modifiers (tagged/PVID).
> 
> I am a little bit torn. While the simplification of the config is nice, I
> don't like how "option network" acts completely different compared to when the
> target interface does not happen to be a (vlan enabled) bridge. People might
> assume that merely specifying "option network" somehow creates a bridge.
We already use the parameter for bridge and non-bridge cases, so I’d argue that 
my proposal fits well into the existing pattern.

> What about spelling out the dependency explicitly? Instead of overloading the
> meaning of "option network", add an "option bridge" instead which reuses the
> existing vlan notation followed by a vlan id spec as defined by the "option
> ports" notation for bridge devices, e.g.
> 
>  option bridge switch0  (default vlan 1 untagged)
> 
> or
> 
>  option bridge switch0
>  option vlan 1:u*  # equivalent to just switch0
> 
> or
> 
>  option bridge switch0
>  option vlan 20:t
> 
> 
> The advantage would also be, that is is completely clear that we're not
> inheriting any layer 3 ip settings but that we make the wifi netdev act as
> bridge port. It is also somewhat aligned with native hostapd configuration.
The disadvantage is that the wireless config has to be different if we switch 
to a default config with no separate LAN bridge, and it becomes potentially 
more confusing for common use cases.
I think semantically it fits quite well to just assign a Wifi interface to the 
lan network and hide some implementation details of what that means.

- Felix

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


Re: [RFC PATCH v2 0/1] Introduce UCI support for configuring DSA VLAN filter rules

2020-07-23 Thread Felix Fietkau

> On 23. Jul 2020, at 13:44, Josh Bendavid  wrote:
> 
> "1. Have VLAN devices on top of vlan-enabled bridges to define hotplug
> ops where applicable, so LAN could be a plain VLAN interface switch0.1
> instead of its own bridge."
> 
> By this you mean that the creation of the switch0.1 device will
> trigger automatically through the hotplug op
> "bridge vlan add dev switch0 vid 1 tagged self" or its equivalent?
No, it refers to how wifi interfaces are added dynamically in netifd.
If lan is a bridge and the wifi-iface has option network lan, it means that 
wlan* is added to br-lan automatically. I want to make that same mechanism work 
with lan ifname switch0.1 (and no extra lan bridge) as well.

- Felix

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


Re: [RFC PATCH v2 0/1] Introduce UCI support for configuring DSA VLAN filter rules

2020-07-23 Thread Felix Fietkau
Hi,

On 2020-07-23 12:10, Jo-Philipp Wich wrote:
> yeah I forgot to elaborate that in my last mail. The problem of dynamic / not
> explicitly addressable wifi interface names in the network config remains.
> 
> The best solution I can think of is fixing the wifi ifnames using "option
> ifname" in the wifi-iface sections (which causes some very interesting effects
> in my tests when not applied to *all* wifi-ifaces) and then using those fixes
> names in /etc/config/network.
> 
> The other alternative I see would be a hypothetical new notation comparable to
> the existing alias notation that resolves ifnames to their actual values at
> runtime... something like "@wifi-network(ssid=foo)" but this is just a very
> vague idea I had from time to time throughout the last few years which never
> gained any traction.
I'm considering the following extensions to netifd:

1. Have VLAN devices on top of vlan-enabled bridges to define hotplug
ops where applicable, so LAN could be a plain VLAN interface switch0.1
instead of its own bridge.
2. With these wrapper hotplug ops, a default VLAN would be passed as
well, unless overwritten by other VLAN settings (see below)
3. In addition to option network, allow specifying option network-vlan
in the wifi-iface section and have it contain a list of VLANs +
modifiers (tagged/PVID).

For a simple default config, you could have this:

# network
config device
option name switch0

config bridge-vlan
option vlan 1
option ports "lan1 lan2 lan3 lan4"

config interface lan
option ifname switch0.1


# wireless

config wifi-iface
option network lan


In this case, wlan0 would be added to switch0 and set to VLAN 1 untagged
by default.

If you want it on VLAN 10 tagged/PVID instead, you could do:
option network-vlan "10:t*"


What do you think?

- Felix

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


Re: [PATCH] iftop: fix compilation with GCC 10

2020-07-17 Thread Felix Fietkau
On 2020-07-14 07:43, Rosen Penev wrote:
> GCC 10 defaults to fno-common, which demains unique defenitions.
> 
> Signed-off-by: Rosen Penev 
> ---
>  package/network/utils/iftop/Makefile  |  2 +-
>  .../utils/iftop/patches/010-gcc10.patch   | 20 +++
>  2 files changed, 21 insertions(+), 1 deletion(-)
>  create mode 100644 package/network/utils/iftop/patches/010-gcc10.patch
> 
> diff --git a/package/network/utils/iftop/Makefile 
> b/package/network/utils/iftop/Makefile
> index 98fe15c8f5..aa467c2876 100644
> --- a/package/network/utils/iftop/Makefile
> +++ b/package/network/utils/iftop/Makefile
> @@ -8,7 +8,7 @@
>  include $(TOPDIR)/rules.mk
>  
>  PKG_NAME:=iftop
> -PKG_RELEASE:=1
> +PKG_RELEASE:=2
>  
>  PKG_SOURCE_PROTO:=git
>  PKG_SOURCE_URL:=https://code.blinkace.com/pdw/iftop.git
> diff --git a/package/network/utils/iftop/patches/010-gcc10.patch 
> b/package/network/utils/iftop/patches/010-gcc10.patch
> new file mode 100644
> index 00..882565a039
> --- /dev/null
> +++ b/package/network/utils/iftop/patches/010-gcc10.patch
> @@ -0,0 +1,20 @@
> +--- a/ui_common.h
>  b/ui_common.h
> +@@ -33,12 +33,12 @@ typedef struct host_pair_line_tag {
> + 
> + extern options_t options;
> + 
> +-sorted_list_type screen_list;
> +-host_pair_line totals;
> +-int peaksent, peakrecv, peaktotal;
> ++static sorted_list_type screen_list;
> ++static host_pair_line totals;
> ++static int peaksent, peakrecv, peaktotal;
> + extern history_type history_totals;
> +-hash_type* screen_hash;
> +-hash_type* service_hash;
> ++static hash_type* screen_hash;
> ++static hash_type* service_hash;
Declaring these variables as static in a header file seems wrong to me.
Shouldn't this be declared as a global variable in one of the .c files
and extern here?

- Felix

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


Re: [RFC] usage of mkhash, sha256sum and md5sum

2020-07-16 Thread Felix Fietkau
On 2020-07-16 21:18, Paul Spooren wrote:
> Hi,
> 
> On 15.07.20 22:54, Felix Fietkau wrote:
>> On 2020-07-16 04:06, Paul Spooren wrote:
>>> Hi,
>>>
>>> the OpenWrt system requires the calculation of both md5 and sha256 sums
>>> at various places, this is partly done via a small C file in
>>> ./scripts/mkhash.c and partly by using a sha256sum binary. A ancient
>>> wrapper ./scripts/md5sum is added for Mac OS X compatibility.
>>>
>>> * Should we create our own crypto by using ./scripts/mkhash.c? I
>>> remember from some previous discussions on IRC and GitHub that there are
>>> generally concerns against it, also a motivation for[0]. I understand
>>> that Felix just reinvent the code but used established sources, however
>>> it is used for package signing (not image signing). I'm fairly sure less
>>> eyes look through that code than e.g. the Debian implementation.
>> This is not "creating our own crypto" at all. I used existing widely
>> used implementations of MD5/SHA256 (mostly FreeBSD code, if I remember
>> correctly).
> Maybe the wording here is wrong, "maintain your own crypto" rather than 
> "create".
I don't see anything wrong with that when it's just simple commonly used
hash functions.

>>> * Currently include/package-ipkg.mk uses a host installed `sha256sum`
>>> binary which is not covered via include/prereq{,-build}.mk. Should it be
>>> added to prereq or replaced by mkhash?
>>>
>>> * Can ./scripts/md5sum be removed or is it still required for Mac OS X
>>> builds.
>> I'm not sure if build/host code for some packages still relies on it.
> I'll ask some fellow Mac OS X builders. However a cleaner solution would 
> be to just remove it and rely on `mkhash md5` only.
>>> * Any reason not to replace `mkhash ` by using `sum | cut -d '
>>> ' -f 1`? Both sha256sum and md5sum seem to be available per default on
>>> Debian, Alpine and OpenWrt.
>> There are many calls to mkhash from the build system, some from
>> performance sensitive parts. Changing it that way will likely make the
>> build slower (especially in cases where it only checks stamps but
>> doesn't rebuild anything).
> 
> I did a quick benchmark and mkhash & sha256sum seem to be the same speed 
> while md5sum is about 8% faster than `mkhash md5`.
> 
> Details here if anyone cares http://sprunge.us/l7amiR
You ran the benchmark on a large file, which mostly ignores startup
overhead. mkhash is frequently called on small files, so startup
performance matters much more than raw throughput.

I just tried the same benchmark on a small file on my machine, and there
mkhash is 1.48x faster than *sum | cut
Results here: http://sprunge.us/KtJTSe

- Felix


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


Re: [RFC] usage of mkhash, sha256sum and md5sum

2020-07-16 Thread Felix Fietkau
On 2020-07-16 04:06, Paul Spooren wrote:
> Hi,
> 
> the OpenWrt system requires the calculation of both md5 and sha256 sums 
> at various places, this is partly done via a small C file in 
> ./scripts/mkhash.c and partly by using a sha256sum binary. A ancient 
> wrapper ./scripts/md5sum is added for Mac OS X compatibility.
> 
> * Should we create our own crypto by using ./scripts/mkhash.c? I 
> remember from some previous discussions on IRC and GitHub that there are 
> generally concerns against it, also a motivation for[0]. I understand 
> that Felix just reinvent the code but used established sources, however 
> it is used for package signing (not image signing). I'm fairly sure less 
> eyes look through that code than e.g. the Debian implementation.
This is not "creating our own crypto" at all. I used existing widely
used implementations of MD5/SHA256 (mostly FreeBSD code, if I remember
correctly).

> * Currently include/package-ipkg.mk uses a host installed `sha256sum` 
> binary which is not covered via include/prereq{,-build}.mk. Should it be 
> added to prereq or replaced by mkhash?
> 
> * Can ./scripts/md5sum be removed or is it still required for Mac OS X 
> builds.
I'm not sure if build/host code for some packages still relies on it.

> * Any reason not to replace `mkhash ` by using `sum | cut -d ' 
> ' -f 1`? Both sha256sum and md5sum seem to be available per default on 
> Debian, Alpine and OpenWrt.
There are many calls to mkhash from the build system, some from
performance sensitive parts. Changing it that way will likely make the
build slower (especially in cases where it only checks stamps but
doesn't rebuild anything).

I'd like to keep mkhash as-is, since it's fast and shouldn't cause any
issues.

- Felix

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


Re: [RFC PATCH v2 0/1] Introduce UCI support for configuring DSA VLAN filter rules

2020-07-15 Thread Felix Fietkau
On 2020-07-15 13:52, John Crispin wrote:
> 
> On 15.07.20 13:39, Jo-Philipp Wich wrote:
>> Hi,
>>
>>> If we can't come up with a working automatic scheme, maybe we could have
>>> an option to disable the cpu port per vlan?
>> Having a default-enabled "option self" or "option local" was my idea as
>> well. Any idea which name fits better?
>>
>> ~ Jo
>>
> self is what ip-bridge uses as a parameter name for this feature
I think local is a better fit - self/master seems to be used for bridge
vlan filter command target selection, not just for this feature.

- Felix

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


Re: [RFC PATCH v2 0/1] Introduce UCI support for configuring DSA VLAN filter rules

2020-07-15 Thread Felix Fietkau
On 2020-07-15 09:56, Jo-Philipp Wich wrote:
> Hi,
> 
>> Changes:
>>   - The device is created as a netifd bridge
>>   - Bridge vlan sections should always refer to the bridge instead of
>> automatically be applied to the first one
>>   - Use = instead of . to mark tagging modifiers. "." is already used
>> for vlan interface names and reusing it here creates ambiguity.
>>   - Allow specifing pvid as a modifier instead of needing to create a
>> separate port section.
>> 
>> The section names are optional, I chose to put them in to make it easier
>> to modify the sections from scripts
>> 
>> What do you think?
> 
> First of all, I was reworking dsaconfig into a very similar direction for a v3
> so overall I do like your proposal and I think it makes sense.
> 
> Specific points from my side:
> 
> - I dislike the equal sign syntax, colon would fit better imho
I didn't pick it, because it is already used for linux alias netdevices.
Since we don't use those, I guess it might not be a problem in practice.

> - For denoting pvid I used a trailing asterisk, like the old roboswitch config
Fine with me as well.

> - The bridge-vlan sections should be anonymous
Sure. As I pointed out, the section name should be optional.

> - Bridge vlan filtering will be implicitely enabled as soon as at least one
>   bridge-vlan section references a bridge device?
Yes. I just discussed with John that I'm going to start working on a
netifd prototype implementation this week, and I'll include your
proposed modifications.

- Felix

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


Re: [RFC PATCH v2 0/1] Introduce UCI support for configuring DSA VLAN filter rules

2020-07-15 Thread Felix Fietkau
Hi,

I would like to propose an alternative configuration format that is
structurally close to your proposal, but more generic in that it is not
tied to DSA directly but configures bridge vlans instead.

Here's the converted form of your example:

config device switch0
option name switch0
option type bridge

config bridge-vlan vlan1
option device switch0
option vlan 1
option ports 'lan1 lan2=t'

config bridge-vlan vlan2
option device switch0
option vlan 2
option ports wan

config bridge-vlan vlan5
option device switch0
option vlan 5
option ports 'lan2=pt lan3'

config bridge-vlan vlan8
option device switch0
option vlan 8
option ports 'lan2=t lan4'

config bridge-vlan vlan11
option device switch0
option vlan 11
option ports 'lan2=t lan4=t'


Changes:
  - The device is created as a netifd bridge
  - Bridge vlan sections should always refer to the bridge instead of
automatically be applied to the first one
  - Use = instead of . to mark tagging modifiers. "." is already used
for vlan interface names and reusing it here creates ambiguity.
  - Allow specifing pvid as a modifier instead of needing to create a
separate port section.

The section names are optional, I chose to put them in to make it easier
to modify the sections from scripts

What do you think?

- Felix

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


Re: [PATCH] wireless-regdb: add country selection

2020-07-14 Thread Felix Fietkau
On 2020-07-14 13:57, Florian Eckert wrote:
> 
>> What's the use case for this?
> As much as I remember there was the problem in 2017 that you can't 
> change the country code or have to restrict it.
> https://ec.europa.eu/growth/sectors/electrical-engineering/red-directive_de
> Of course this change can't fulfill with this patch, but it wouldn't be 
> bad if you can put together regdb yourself and not always provide all 
> country codes.
Makes sense, thanks.

>> Couldn't we simply add add two options to blacklist or whitelist 
>> country
>> codes using a comma separated string?
> 
> That doesn't sound like a bad idea.
> If the option CONFIG_WIRELESS_REGDB_WHITELIST or 
> CONFIG_WIRELESS_REGDB_BLACKLIST is configured then a regdb is created at 
> compile time?
> Otherwise the package uses the regdb as it is.
Yes, exactly.

- Felix

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


Re: [PATCH] wireless-regdb: add country selection

2020-07-13 Thread Felix Fietkau
On 2020-07-13 11:09, Florian Eckert wrote:
> Until now it has not been possible to remove countries from the regdb.
> The packages has been extended, so that it is now possible to add or
> omit individual countries in the regdb during package compile time.
> If this option is not activated, then all countries in the regdb are taken
> over as before.
> 
> If the package is updated to a new version. Then the script
> `build_menuconfig` must be executed with the argument of the new db.txt to
> update the country specification.
> 
> Signed-off-by: Florian Eckert 
What's the use case for this?
Couldn't we simply add add two options to blacklist or whitelist country
codes using a comma separated string?

- Felix

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


Re: [OpenWrt-Devel] [PATCH] build: improve ccache support

2020-06-01 Thread Felix Fietkau
On 2020-06-01 19:11, Michael Jones wrote:
> 
> 
> On Mon, Jun 1, 2020 at 10:33 AM Roman Yeryomin  > wrote:
> 
> Set CCACHE_DIR to $(TOPDIR)/.ccache and CCACHE_BASEDIR to $(TOPDIR).
> This allows to do clean and dirclean. Cache hit rate for test build
> after dirclean is ~65%.
> If CCACHE is enabled stats are printed out at the end of building
> process.
> 
> Signed-off-by: Roman Yeryomin mailto:ro...@advem.lv>>
> 
> 
> This certainly looks like an improvement.
> 
> However, there is an important usage case that this change doesn't address.
> 
> Frequently when I am working on OpenWRT related things, I have many
> different workspaces all tied to the same git repository hosted
> externally. The reason for this is to allow multiple builds to live and
> run independently.
> 
> Having the CCACHE_DIR be located *inside* the repository doesn't share
> the cache between multiple workspaces.
> 
> So can the CCACHE_DIR be made configurable at build time based on the
> .config file? Perhaps it can default to this, and only be set to the
> value in .config if provided? For my purposes, I would always set the
> CCACHE_DIR to a path that all of my workspaces use.
I don't think there's a need for that config option. You could simply
add a .ccache symlink in your source dir and point it to your shared
cache. I do the same with dl on my trees.

- Felix

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


[OpenWrt-Devel] [PATCH libubox 2/3] blobmsg: simplify and fix name length checks in blobmsg_check_name

2020-05-25 Thread Felix Fietkau
blobmsg_hdr_valid_namelen was omitted when name==false
The blob_len vs blobmsg_namelen changes were not taking into account
potential padding between name and data

Signed-off-by: Felix Fietkau 
---
 blobmsg.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/blobmsg.c b/blobmsg.c
index daaa9fc8444b..308bef7bc6b0 100644
--- a/blobmsg.c
+++ b/blobmsg.c
@@ -48,8 +48,8 @@ static bool blobmsg_hdr_valid_namelen(const struct 
blobmsg_hdr *hdr, size_t len)
 
 static bool blobmsg_check_name(const struct blob_attr *attr, size_t len, bool 
name)
 {
-   char *limit = (char *) attr + len;
const struct blobmsg_hdr *hdr;
+   uint16_t namelen;
 
hdr = blobmsg_hdr_from_blob(attr, len);
if (!hdr)
@@ -58,16 +58,11 @@ static bool blobmsg_check_name(const struct blob_attr 
*attr, size_t len, bool na
if (name && !hdr->namelen)
return false;
 
-   if (name && !blobmsg_hdr_valid_namelen(hdr, len))
-   return false;
-
-   if ((char *) hdr->name + blobmsg_namelen(hdr) + 1 > limit)
-   return false;
-
-   if (blobmsg_namelen(hdr) > (blob_len(attr) - sizeof(struct 
blobmsg_hdr)))
+   namelen = blobmsg_namelen(hdr);
+   if (blob_len(attr) < (size_t)blobmsg_hdrlen(namelen))
return false;
 
-   if (hdr->name[blobmsg_namelen(hdr)] != 0)
+   if (hdr->name[namelen] != 0)
return false;
 
return true;
-- 
2.24.0


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


[OpenWrt-Devel] [PATCH libubox 3/3] blobmsg: fix missing length checks

2020-05-25 Thread Felix Fietkau
blobmsg_check_attr_len was calling blobmsg_check_data for some, but not all
attribute types. These checks was missing for arrays and tables.

Additionally, the length check in blobmsg_check_data was a bit off, since
it was comparing the blobmsg data length against the raw blob attr length.

Fix this by checking the raw blob length against the buffer length in
blobmsg_hdr_from_blob

Signed-off-by: Felix Fietkau 
---
 blobmsg.c | 66 +--
 1 file changed, 20 insertions(+), 46 deletions(-)

diff --git a/blobmsg.c b/blobmsg.c
index 308bef7bc6b0..7da418380299 100644
--- a/blobmsg.c
+++ b/blobmsg.c
@@ -30,31 +30,18 @@ bool blobmsg_check_attr(const struct blob_attr *attr, bool 
name)
return blobmsg_check_attr_len(attr, name, blob_raw_len(attr));
 }
 
-static const struct blobmsg_hdr* blobmsg_hdr_from_blob(const struct blob_attr 
*attr, size_t len)
-{
-   if (len < sizeof(struct blob_attr) + sizeof(struct blobmsg_hdr))
-   return NULL;
-
-   return blob_data(attr);
-}
-
-static bool blobmsg_hdr_valid_namelen(const struct blobmsg_hdr *hdr, size_t 
len)
-{
-   if (len < sizeof(struct blob_attr) + sizeof(struct blobmsg_hdr) + 
blobmsg_namelen(hdr) + 1)
-   return false;
-
-   return true;
-}
-
-static bool blobmsg_check_name(const struct blob_attr *attr, size_t len, bool 
name)
+static bool blobmsg_check_name(const struct blob_attr *attr, bool name)
 {
const struct blobmsg_hdr *hdr;
uint16_t namelen;
 
-   hdr = blobmsg_hdr_from_blob(attr, len);
-   if (!hdr)
+   if (!blob_is_extended(attr))
+   return !name;
+
+   if (blob_len(attr) < sizeof(struct blobmsg_hdr))
return false;
 
+   hdr = (const struct blobmsg_hdr *)blob_data(attr);
if (name && !hdr->namelen)
return false;
 
@@ -68,29 +55,20 @@ static bool blobmsg_check_name(const struct blob_attr 
*attr, size_t len, bool na
return true;
 }
 
-static const char* blobmsg_check_data(const struct blob_attr *attr, size_t 
len, size_t *data_len)
-{
-   char *limit = (char *) attr + len;
-   const char *data;
-
-   *data_len = blobmsg_data_len(attr);
-   if (*data_len > blob_raw_len(attr))
-   return NULL;
-
-   data = blobmsg_data(attr);
-   if (data + *data_len > limit)
-   return NULL;
-
-   return data;
-}
-
 bool blobmsg_check_attr_len(const struct blob_attr *attr, bool name, size_t 
len)
 {
const char *data;
size_t data_len;
int id;
 
-   if (!blobmsg_check_name(attr, len, name))
+   if (len < sizeof(struct blob_attr))
+   return false;
+
+   data_len = blob_raw_len(attr);
+   if (data_len < sizeof(struct blob_attr) || data_len > len)
+   return false;
+
+   if (!blobmsg_check_name(attr, name))
return false;
 
id = blob_id(attr);
@@ -100,9 +78,8 @@ bool blobmsg_check_attr_len(const struct blob_attr *attr, 
bool name, size_t len)
if (!blob_type[id])
return true;
 
-   data = blobmsg_check_data(attr, len, _len);
-   if (!data)
-   return false;
+   data = blobmsg_data(attr);
+   data_len = blobmsg_data_len(attr);
 
return blob_check_type(data, data_len, blob_type[id]);
 }
@@ -206,13 +183,13 @@ int blobmsg_parse(const struct blobmsg_policy *policy, 
int policy_len,
}
 
__blob_for_each_attr(attr, data, len) {
-   hdr = blobmsg_hdr_from_blob(attr, len);
-   if (!hdr)
+   if (!blobmsg_check_attr_len(attr, false, len))
return -1;
 
-   if (!blobmsg_hdr_valid_namelen(hdr, len))
-   return -1;
+   if (!blob_is_extended(attr))
+   continue;
 
+   hdr = blob_data(attr);
for (i = 0; i < policy_len; i++) {
if (!policy[i].name)
continue;
@@ -224,9 +201,6 @@ int blobmsg_parse(const struct blobmsg_policy *policy, int 
policy_len,
if (blobmsg_namelen(hdr) != pslen[i])
continue;
 
-   if (!blobmsg_check_attr_len(attr, true, len))
-   return -1;
-
if (tb[i])
continue;
 
-- 
2.24.0


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


[OpenWrt-Devel] [PATCH libubox 1/3] blobmsg: fix length in blobmsg_check_array

2020-05-25 Thread Felix Fietkau
blobmsg_check_array_len expects the length of the full attribute buffer,
not just the data length.
Due to other missing length checks (fixed in the next commit), this did
not show up as a test failure

Signed-off-by: Felix Fietkau 
---
 blobmsg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/blobmsg.c b/blobmsg.c
index 59045e1672c8..daaa9fc8444b 100644
--- a/blobmsg.c
+++ b/blobmsg.c
@@ -114,7 +114,7 @@ bool blobmsg_check_attr_len(const struct blob_attr *attr, 
bool name, size_t len)
 
 int blobmsg_check_array(const struct blob_attr *attr, int type)
 {
-   return blobmsg_check_array_len(attr, type, blob_len(attr));
+   return blobmsg_check_array_len(attr, type, blob_raw_len(attr));
 }
 
 int blobmsg_check_array_len(const struct blob_attr *attr, int type,
-- 
2.24.0


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


Re: [OpenWrt-Devel] [PATCH RFC libubox] blobmsg: another attrs iteration fix for blobmsg_check_array_len()

2020-05-25 Thread Felix Fietkau
On 2020-05-25 10:31, Rafał Miłecki wrote:
> From: Rafał Miłecki 
> 
> After more reviews is seems that blobmsg_for_each_attr() should not be
> used when dealing with untrusted data as it reads length from blob data
> itself. It means it can't be used in the blobmsg_check_array_len().
> 
> Switch back to using __blobmsg_for_each_attr() BUT pass correct length
> to it. Calculate it by subtracting header length from blob length.
This should not be necessary, because the length is validated in the
blobmsg_check_attr_len call earlier in the same function.
I think your previous fix is completely fine as-is.

- Felix

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


Re: [OpenWrt-Devel] [PATCH] build: always use -minterlink-mips16 if USE_MIPS16

2020-05-25 Thread Felix Fietkau
On 2020-05-25 02:19, Eneas U de Queiroz wrote:
> Individual packages may turn off MIPS16 ISA individually with
> PKG_USE_MIPS16.  However, they may link to a library compiled with
> MIPS16.  In such cases, the -minterlink-mips16 is needed to ensure there
> are no direct jumps to code compiled with a different ISA.
> 
> Instead of adding -minterlink-mips16 only when PKG_USE_MIPS16 is on, add
> it when global USE_MIPS16 is on.
> 
> Signed-off-by: Eneas U de Queiroz 
> ---
> Tested by compiling all packages in base, packages, routing and
> telephony feeds for mips_74kc, with MIPS16 enabled.
> 
> This was discovered while working on lxc fixes 
> (https://github.com/openwrt/packages/pull/12241), where compilation with
> mips16 would fail because of '-fstack-check=specific not implemented for
> MIPS16', and it would fail with PKG_USE_MIPS16=0 because of jumping to a
> different ISA mode:
> 
> lxc-4.0.2/src/lxc/caps.c:24:(.text+0xa4): unsupported jump between ISA
> modes; consider recompiling with interlinking enabled
> 
> In theory this could happen in more places, so set interlinking on
> whenever MIPS16 is turned on globally.
I think there needs be a way to opt-out of this behavior.
The -minterlink-mips16 flag affects the performance and code size of
generated code, so libraries that disable MIPS16 for performance reasons
and don't depend on other MIPS16 enabled libraries should not be
compiled with this flag.

- Felix

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


Re: [OpenWrt-Devel] [PATCH 0/3] mac80211: Update to version 5.7-rc2

2020-04-22 Thread Felix Fietkau
On 2020-04-21 23:22, Hauke Mehrtens wrote:
> This updates mac80211 in OpenWrt to version 5.7-rc2.
> This update contains ath11k and many other ieee80211ax updates.
> ath11k only works on the ipq807x devices.
> 
> I tried to start a discussion how we want to go forward with the 
> wireless subsystem we ship in the next OpenWrt release:
> https://lists.infradead.org/pipermail/openwrt-devel/2020-March/022198.html
> 
> I would prefer if we apply this to master and then continuously update 
> to match more recent kernel versions till we are at an LTS kernel 
> version. I assume that the kernel 5.9 or 5.10 will be the next LTS 
> version. Using a normal kernel release as a base will make providing 
> (security) updates much harder.
> 
> You can also find these patches in my staging tree:
> https://git.openwrt.org/?p=openwrt/staging/hauke.git;a=shortlog;h=refs/heads/mac80211-5.6
> 
> Please test this and report any regressions you see compared to the 
> version currently shipped in OpenWrt master.
I tested this and found that the debugfs phy rate control settings
directory was not in the right place. It is supposed to be in
/sys/kernel/debug/ieee80211/phy0/rc but it somehow ended up in
/sys/kernel/debug/rc instead.

- Felix

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


Re: [OpenWrt-Devel] [PATCH 1/1] netifd: add pre-up/down post-up/down callback handling

2020-03-20 Thread Felix Fietkau
On 2020-03-20 15:21, Florian Eckert wrote:
> network
>>> With this change we can decide if this is a user interaction with 
>>> CLI/LuCI,
>>> because with the new callback mechanism I can set/delete a uci config 
>>> flag so
>>> that the connection should really disconnected. And so does not 
>>> restart on a
>>> failed connetion tracking again because the uci config flag is not 
>>> set.
>>> 
>>> Signed-off-by: Florian Eckert 
>> netifd already tracks for every interface if the user requested it to 
>> be
>> enabled or not via the 'autostart' flag, which you can query via ubus.
> 
> I know this is done wit the uci option auto for this interface.
> But if I disable this flag, then on the next boot this interface does 
> not start
> on boot anymore. I have to start this manual. So I think this is not an 
> option.
No, I'm talking about the internal per-interface 'autostart' variable,
which gets set to false if the user does a manual ifdown of an interface
(but not if it just failed to start up).
It's not backed by configuration and you can query it via ubus.
(e.g. ifstatus wan)

- Felix


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


Re: [OpenWrt-Devel] [PATCH 1/1] netifd: add pre-up/down post-up/down callback handling

2020-03-20 Thread Felix Fietkau
On 2020-03-19 13:03, Florian Eckert wrote:
> In some situations it is useful to execute callback scripts when
> manually triggering an ifdown/ifup event via the CLI/LuCI.
> 
> In my case I have a monitoring for a cellular connection.
> If this fails the command `ubus call network.interface up` is execute to
> restart the connection if a config option in uci of the interface
> (keep_connected) flag is set.
> 
> But for me to disconnect correctly I need to know about a user
> interaction with performed with CLI/LuCI.
> 
> With this change we can decide if this is a user interaction with CLI/LuCI,
> because with the new callback mechanism I can set/delete a uci config flag so
> that the connection should really disconnected. And so does not restart on a
> failed connetion tracking again because the uci config flag is not set.
> 
> Signed-off-by: Florian Eckert 
netifd already tracks for every interface if the user requested it to be
enabled or not via the 'autostart' flag, which you can query via ubus.
Is it enough for your use case to track that flag?
If not, please go into more detail, because I don't think hacking
hotplug-call invocations into the ifup script is a good solution.

- Felix

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


Re: [OpenWrt-Devel] [PATCH] ppp: activate PIE ASLR by default

2020-02-22 Thread Felix Fietkau
On 2020-02-22 10:22, Stijn Tintel wrote:
> On 22/02/2020 11:10, Felix Fietkau wrote:
>> On 2020-02-22 09:54, Stijn Tintel wrote:
>>> On 20/02/2020 11:56, Petr Štetiar wrote:
>>>> This activates PIE ASLR support by default when the regular option is
>>>> selected.
>>>>
>>> Unfortunately this seems to break build on x86/64:
>>>
>>> x86_64-openwrt-linux-musl-gcc -O2 -pipe -fno-caller-saves -fno-plt
>>> -fhonour-copts -Wno-error=unused-but-set-variable
>>> -Wno-error=unused-result
>>> -ffile-prefix-map=/home/stijn/Development/LEDE/source/build_dir/target-x86_64_musl/linux-x86_64/ppp-default/ppp-2.4.8=ppp-2.4.8
>>> -Wformat -Werror=format-security -fpic -fstack-protector-strong
>>> -D_FORTIFY_SOURCE=2 -Wl,-z,now -Wl,-z,relro -ffunction-sections
>>> -fdata-sections -flto -DHAVE_PATHS_H -DHAVE_MMAP -I../include
>>> '-DDESTDIR="/usr"' -DCHAPMS=1 -DMPPE=1 -DHAS_SHADOW -DHAVE_CRYPT_H=1
>>> -DUSE_CRYPT=1 -DPLUGIN -DPPP_FILTER -DPPP_PRECOMPILED_FILTER
>>> -I/home/stijn/Development/LEDE/source/staging_dir/target-x86_64_musl/usr/include
>>> -DINET6=1 -DMAXOCTETS
>>> -L/home/stijn/Development/LEDE/source/staging_dir/target-x86_64_musl/usr/lib
>>> -L/home/stijn/Development/LEDE/source/staging_dir/target-x86_64_musl/lib
>>> -L/home/stijn/Development/LEDE/source/staging_dir/toolchain-x86_64_gcc-8.3.0_musl/usr/lib
>>> -L/home/stijn/Development/LEDE/source/staging_dir/toolchain-x86_64_gcc-8.3.0_musl/lib
>>> -fpic
>>> -specs=/home/stijn/Development/LEDE/source/include/hardened-ld-pie.specs
>>> -znow -zrelro -Wl,--gc-sections -flto -fuse-linker-plugin  -Wl,-E -o
>>> pppd main.o magic.o fsm.o lcp.o ipcp.o upap.o chap-new.o md5.o ccp.o
>>> ecp.o auth.o options.o demand.o utils.o sys-linux.o ipxcp.o tty.o eap.o
>>> chap-md5.o session.o md4.o chap_ms.o sha1.o pppcrypt.o pcap_pcc.o
>>> ipv6cp.o eui64.o  -lcrypt -ldl
>>> /home/stijn/Development/LEDE/source/staging_dir/target-x86_64_musl/usr/lib/libpcap.a
>>> /home/build/openwrt/staging_dir/toolchain-x86_64_gcc-8.3.0_musl/bin/../lib/gcc/x86_64-openwrt-linux-musl/8.3.0/../../../../x86_64-openwrt-linux-musl/bin/ld:
>>> /home/stijn/Development/LEDE/source/staging_dir/target-x86_64_musl/usr/lib/libpcap.a(bpf_filter.c.o):
>>> relocation R_X86_64_32S against `.rodata' can not be used when making a
>>> PIE object; recompile with -fPIC
>>> /home/build/openwrt/staging_dir/toolchain-x86_64_gcc-8.3.0_musl/bin/../lib/gcc/x86_64-openwrt-linux-musl/8.3.0/../../../../x86_64-openwrt-linux-musl/bin/ld:
>>> final link failed: nonrepresentable section on output
>>> collect2: error: ld returned 1 exit status
>>>
>>> So NACK from me until this is fixed.
>> This one can most likely be fixed by setting PKG_ASLR_PIE_REGULAR:=1 in
>> libpcap as well. That way -fPIC gets passed for the static library build.
>>
> Interesting. I've added this in the libpcap Makefile and that seems to
> fix it. But I am actually building with CONFIG_PKG_ASLR_PIE_ALL=y, so
> would assume it would enable PIE even if PKG_ASLR_PIE_REGULAR is not set
> in the Makefile. Anyway, I'll send a patch for libpcap, thanks for the
> suggestion.
Maybe it was built before you made that config change? It seems to me
that the ALSR_PIE stuff is missing some PKG_CONFIG_DEPENDS handling.

- Felix

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


Re: [OpenWrt-Devel] [PATCH] ppp: activate PIE ASLR by default

2020-02-22 Thread Felix Fietkau
On 2020-02-22 09:54, Stijn Tintel wrote:
> On 20/02/2020 11:56, Petr Štetiar wrote:
>> This activates PIE ASLR support by default when the regular option is
>> selected.
>>
> Unfortunately this seems to break build on x86/64:
> 
> x86_64-openwrt-linux-musl-gcc -O2 -pipe -fno-caller-saves -fno-plt
> -fhonour-copts -Wno-error=unused-but-set-variable
> -Wno-error=unused-result
> -ffile-prefix-map=/home/stijn/Development/LEDE/source/build_dir/target-x86_64_musl/linux-x86_64/ppp-default/ppp-2.4.8=ppp-2.4.8
> -Wformat -Werror=format-security -fpic -fstack-protector-strong
> -D_FORTIFY_SOURCE=2 -Wl,-z,now -Wl,-z,relro -ffunction-sections
> -fdata-sections -flto -DHAVE_PATHS_H -DHAVE_MMAP -I../include
> '-DDESTDIR="/usr"' -DCHAPMS=1 -DMPPE=1 -DHAS_SHADOW -DHAVE_CRYPT_H=1
> -DUSE_CRYPT=1 -DPLUGIN -DPPP_FILTER -DPPP_PRECOMPILED_FILTER
> -I/home/stijn/Development/LEDE/source/staging_dir/target-x86_64_musl/usr/include
> -DINET6=1 -DMAXOCTETS
> -L/home/stijn/Development/LEDE/source/staging_dir/target-x86_64_musl/usr/lib
> -L/home/stijn/Development/LEDE/source/staging_dir/target-x86_64_musl/lib
> -L/home/stijn/Development/LEDE/source/staging_dir/toolchain-x86_64_gcc-8.3.0_musl/usr/lib
> -L/home/stijn/Development/LEDE/source/staging_dir/toolchain-x86_64_gcc-8.3.0_musl/lib
> -fpic
> -specs=/home/stijn/Development/LEDE/source/include/hardened-ld-pie.specs
> -znow -zrelro -Wl,--gc-sections -flto -fuse-linker-plugin  -Wl,-E -o
> pppd main.o magic.o fsm.o lcp.o ipcp.o upap.o chap-new.o md5.o ccp.o
> ecp.o auth.o options.o demand.o utils.o sys-linux.o ipxcp.o tty.o eap.o
> chap-md5.o session.o md4.o chap_ms.o sha1.o pppcrypt.o pcap_pcc.o
> ipv6cp.o eui64.o  -lcrypt -ldl
> /home/stijn/Development/LEDE/source/staging_dir/target-x86_64_musl/usr/lib/libpcap.a
> /home/build/openwrt/staging_dir/toolchain-x86_64_gcc-8.3.0_musl/bin/../lib/gcc/x86_64-openwrt-linux-musl/8.3.0/../../../../x86_64-openwrt-linux-musl/bin/ld:
> /home/stijn/Development/LEDE/source/staging_dir/target-x86_64_musl/usr/lib/libpcap.a(bpf_filter.c.o):
> relocation R_X86_64_32S against `.rodata' can not be used when making a
> PIE object; recompile with -fPIC
> /home/build/openwrt/staging_dir/toolchain-x86_64_gcc-8.3.0_musl/bin/../lib/gcc/x86_64-openwrt-linux-musl/8.3.0/../../../../x86_64-openwrt-linux-musl/bin/ld:
> final link failed: nonrepresentable section on output
> collect2: error: ld returned 1 exit status
> 
> So NACK from me until this is fixed.
This one can most likely be fixed by setting PKG_ASLR_PIE_REGULAR:=1 in
libpcap as well. That way -fPIC gets passed for the static library build.

- Felix

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


Re: [OpenWrt-Devel] Wireless monitor interface causes device to run out of memory

2020-02-11 Thread Felix Fietkau
On 2020-02-11 17:03, Rafał Miłecki wrote:
> On Wed, 5 Feb 2020 at 11:14, Rafał Miłecki  wrote:
>> I noticed that my bcm53xx devices run out of memory after running
>> wireless monitor interface for about 2 days.
>>
>> This problem occurs since 0694d08c84fd ("bcm53xx: switch to kernel
>> 4.9") which bumped kernel from 4.4.53 to 4.9.14 and is still present
>> when using 4.14.
>>
>> Interestingly this memory drain requires at least one of:
>> net.ipv6.conf.default.forwarding=1
>> net.ipv6.conf.all.forwarding=1
>> to be set. OpenWrt happens to use both by default.
>>
>> This is *not* a memory leak. Restarting wireless interfaces frees all
>> consumed memory.
>>
>> Any idea what may be causing this?
> 
> This regression is caused by 1666d49e1d41 ("mld: do not remove mld
> souce list info when set link down") which was later backported as
> 53a76d633b86 to the linux-4.9.y branch.
> 
> Reverting that commit from 4.9.14 or 4.14.169 /fixes/ device running
> out of memory problem.
Looks like it's missing a backport of

commit a84d016479896b5526a2cc54784e6ffc41c9d6f6
"[PATCH net] mld: fix memory leak in mld_del_delrec()"

- Felix

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


Re: [OpenWrt-Devel] [RFC/RFT PATCH] ath9k: implement kthread entropy collection for AR5008 and AR9002 PHYs.

2020-02-10 Thread Felix Fietkau
On 2020-02-10 10:44, Rui Salvaterra wrote:
> On Fri, 31 Jan 2020 at 08:12, Felix Fietkau  wrote:
>>
>> For at least AR5008 and AR9002, but probably also for AR9003 I would
>> like to keep the behavior of collecting entropy only once at driver
>> initialization.
>> Last time I worked on this I noticed that on several chips, sampling
>> entropy during normal operation caused stability issues that were hard
>> to pin down but quite noticeable.
>> I think the benefit of continuous entropy collection is simply not worth
>> the extra cost of potential stability issues and debugging time.
>>
>> - Felix
> 
> Hi again, Felix,
> 
> FWIW, this patch has just survived a whole weekend of rngtest <
> /dev/random on a Buffalo WZR-HP-AG300H (dual AR9002). Output follows:
> 
> root@AP157427:~# rngtest < /dev/random
> rngtest 6.6
> Copyright (c) 2004 by Henrique de Moraes Holschuh
> This is free software; see the source for copying conditions.  There
> is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
> PARTICULAR PURPOSE.
> 
> rngtest: starting FIPS tests...
> ^Crngtest: bits received from input: 224241980032
> rngtest: FIPS 140-2 successes: 11203058
> rngtest: FIPS 140-2 failures: 9041
> rngtest: FIPS 140-2(2001-10-10) Monobit: 1165
> rngtest: FIPS 140-2(2001-10-10) Poker: 1175
> rngtest: FIPS 140-2(2001-10-10) Runs: 3340
> rngtest: FIPS 140-2(2001-10-10) Long run: 3405
> rngtest: FIPS 140-2(2001-10-10) Continuous run: 0
> rngtest: input channel speed: (min=165.459; avg=1025.932; 
> max=1093.147)Kibits/s
> rngtest: FIPS tests speed: (min=503.707; avg=14584.165; max=14920.741)Kibits/s
> rngtest: Program run time: 228508694816 microseconds
The issues that were reported years ago were mainly stuck beacons,
increase in hardware resets, connection stability issues.

- Felix

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


Re: [OpenWrt-Devel] [PATCH] ramips: ethernet: turn off flow control

2020-01-31 Thread Felix Fietkau
On 2020-01-29 17:22, Petr Štetiar wrote:
> Rosen Penev  [2020-01-25 15:04:03]:
> 
> Hi Bjørn and Rosen,
> 
>> On Thu, Jan 23, 2020 at 12:25 AM Bjørn Mork  wrote:
>> > >
>> > > - if ((rt_sysc_r32(SYSC_REG_CHIP_REV_ID) & 0x) == 0x0101) {
>> > > - /* (GE1, Force 1000M/FD, FC ON, MAX_RX_LENGTH 1536) */
>> > > + if ((rt_sysc_r32(SYSC_REG_CHIP_REV_ID) & 0x) >= 0x0101) {
>> > > + /* (GE1, Force 1000M/FD, FC OFF, MAX_RX_LENGTH 1536) */
>> > >   mtk_switch_w32(gsw, 0x2305e30b, GSW_REG_MAC_P0_MCR);
>> > >   mt7530_mdio_w32(gsw, 0x3600, 0x5e30b);
>> > >   } else {
>> >
>> > If that's really helping then I believe it would be useful to add some
>> > sort of chip_rev_id printk here.  Would be good to know who hits this
>> > and why...
> 
> I just quickly skimmed through the code and it seems like we've that
> information already in dmesg:
> 
>  SoC Type: MediaTek MT7688 ver:1 eco:2= 0x0102
>  SoC Type: MediaTek MT7628AN ver:1 eco:2  = 0x0102
>  SoC Type: MediaTek MT7621 ver:1 eco:3= 0x0103
>  SoC Type: MediaTek MT7620A ver:2 eco:3   = 0x0203
>  SoC Type: MediaTek MT7620N ver:2 eco:6   = 0x0206
> 
>> That's just cosmetic.
> 
>  current: == 0x0101
>  new: >= 0x0101
> 
> Doesn't look like a cosmetic change nor correct, see above list. What about
> following?
> 
>   if (ralink_soc == MT762X_SOC_MT7621AT) {
>/* (GE1, Force 1000M/FD, FC OFF, MAX_RX_LENGTH 1536) */
>mtk_switch_w32(gsw, 0x2305e30b, GSW_REG_MAC_P0_MCR);
>mt7530_mdio_w32(gsw, 0x3600, 0x5e30b);
>   } else {
>...
>   }
> 
> I just don't know which MediaTek SoC is 0x0101 (ver:1 eco:1), but I assume,
> that this condition is just bogus from the beginning.
In my opinion, there really is no need to check the SoC here, because
all of this code only gets used on MT7621.

- Felix

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


Re: [OpenWrt-Devel] [RFC/RFT PATCH] ath9k: implement kthread entropy collection for AR5008 and AR9002 PHYs.

2020-01-31 Thread Felix Fietkau
On 2020-01-30 21:03, Rui Salvaterra wrote:
> The mainline ath9k driver is able to provide a hardware random number
> generator by collecting radio noise from the PHY ADC (using a kthread
> to fill up the entropy pool as needed). Nevertheless, this feature has
> only been implemented for the more recent AR9003 PHYs.
> 
> Meanwhile, OpenWrt has been carrying a patch to provide entropy from the
> ADC for the three supported PHYs for a long time, but this patch only
> collects entropy once per existing PHY, at the driver initialisation
> time.
> 
> This patch enables kconfig support for this feature and updates the
> OpenWrt patch, in order to add ADC entropy collection support to both
> AR5008 and AR9002 PHYs.
> 
> Signed-off-by: Rui Salvaterra 
For at least AR5008 and AR9002, but probably also for AR9003 I would
like to keep the behavior of collecting entropy only once at driver
initialization.
Last time I worked on this I noticed that on several chips, sampling
entropy during normal operation caused stability issues that were hard
to pin down but quite noticeable.
I think the benefit of continuous entropy collection is simply not worth
the extra cost of potential stability issues and debugging time.

- Felix

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


Re: [OpenWrt-Devel] [PATCH 3/7] base-files/functions.sh: use command -v instead of $(which)

2020-01-17 Thread Felix Fietkau
On 2020-01-17 05:43, Rosen Penev wrote:
> $(which) must be executed. command -v is a shell builtin.
> 
> https://github.com/koalaman/shellcheck/wiki/SC2230
> 
> Signed-off-by: Rosen Penev 
> ---
>  package/base-files/files/lib/functions.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/package/base-files/files/lib/functions.sh 
> b/package/base-files/files/lib/functions.sh
> index 0e94162a1a..a8a4734413 100755
> --- a/package/base-files/files/lib/functions.sh
> +++ b/package/base-files/files/lib/functions.sh
> @@ -176,7 +176,7 @@ default_prerm() {
>   ret=$?
>   fi
>  
> - local shell="$(which bash)"
> + local shell="command -v bash"
You dropped the $() there, that looks wrong to me.

- Felix

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


  1   2   3   4   5   6   7   8   9   10   >