| Message ID | 20191211144459.13235-1-fercerpav@gmail.com |
|---|---|
| State | Superseded |
| Delegated to: | Petr Štetiar |
| Headers | show |
| Series | [OpenWrt-Devel] kernel: ath10k-ct: provide a build variant for small RAM devices | expand |
On Wed, Dec 11, 2019 at 05:44:59PM +0300, Paul Fertser wrote: > +define Build/Patch > + $(if $(QUILT),rm -rf $(PKG_BUILD_DIR)/patches; mkdir -p $(PKG_BUILD_DIR)/patches) > + $(call PatchDir,$(PKG_BUILD_DIR),$(PATCH_DIR),) > +ifeq ($(BUILD_VARIANT),smallbuffers) > + $(call PatchDir,$(PKG_BUILD_DIR),$(PATCH_DIR)-smallbuffers,patches-smallbuffers) > +endif > + $(if $(QUILT),touch $(PKG_BUILD_DIR)/.quilt_used) > +endef This is not correctly creating the patches-smallbuffers directory, I'll fix it for v2 along with other review points (if this approach is considered worthy at all).
On 12/11/19 6:44 AM, Paul Fertser wrote: > According to many bugreports [0][1][2] the default ath10k-ct kernel > module is unusable on devices with just 64 MiB RAM or with 128 MiB and > dual ath10k cards. The target boards boot but eventually oom-killer > starts to interfere with normal operation, so the current state is > effectively broken. > > Since the two patches in question might have a performance impact (and > possibly some other unexpected side-effects) a dedicated build variant > is added so that users of the low RAM devices can still benefit from all > the ath10k-ct advantages. > > [0] http://lists.infradead.org/pipermail/openwrt-devel/2019-December/020573.html > [1] https://github.com/openwrt/openwrt/pull/1077 > [2] https://bugs.openwrt.org/index.php?do=details&task_id=2664 I am fine with this approach. And also if you want to just have the makefile pass a -DBUILD_ATH10K_SMALL or something like that and #ifdef code in the ath10k-ct driver, then I'd apply that patch to ath10k-ct so that you don't need the patches. Thanks, Ben > > Signed-off-by: Paul Fertser <fercerpav@gmail.com> > --- > package/kernel/ath10k-ct/Makefile | 30 +++++++- > ...0-0010-ath10k-limit-htt-rx-ring-size.patch | 22 ++++++ > ...60-0011-ath10k-limit-pci-buffer-size.patch | 76 +++++++++++++++++++ > 3 files changed, 127 insertions(+), 1 deletion(-) > create mode 100644 package/kernel/ath10k-ct/patches-smallbuffers/960-0010-ath10k-limit-htt-rx-ring-size.patch > create mode 100644 package/kernel/ath10k-ct/patches-smallbuffers/960-0011-ath10k-limit-pci-buffer-size.patch > > diff --git a/package/kernel/ath10k-ct/Makefile b/package/kernel/ath10k-ct/Makefile > index dbf75fe174..d5726a1c88 100644 > --- a/package/kernel/ath10k-ct/Makefile > +++ b/package/kernel/ath10k-ct/Makefile > @@ -35,6 +35,7 @@ define KernelPackage/ath10k-ct > $(PKG_BUILD_DIR)/ath10k$(CT_KVER)/ath10k_core.ko > AUTOLOAD:=$(call AutoProbe,ath10k_pci) > PROVIDES:=kmod-ath10k > + VARIANT:=regular > endef > > define KernelPackage/ath10k-ct/config > @@ -42,7 +43,17 @@ define KernelPackage/ath10k-ct/config > config ATH10K-CT_LEDS > bool "Enable LED support" > default y > - depends on PACKAGE_kmod-ath10k-ct > + depends on PACKAGE_kmod-ath10k-ct || PACKAGE_kmod-ath10k-ct-smallbuffers > +endef > + > +define KernelPackage/ath10k-ct-smallbuffers > +$(call KernelPackage/ath10k-ct) > + TITLE+= (small buffers to work on low-RAM devices) > + VARIANT:=smallbuffers > +endef > + > +define KernelPackage/ath10k-ct-smallbuffers/config > +$(call KernelPackage/ath10k-ct/config) > endef > > NOSTDINC_FLAGS = \ > @@ -90,6 +101,22 @@ ifeq ($(CONFIG_ATH10K-CT_LEDS),y) > NOSTDINC_FLAGS += -DCONFIG_ATH10K_LEDS > endif > > +define Build/Patch > + $(if $(QUILT),rm -rf $(PKG_BUILD_DIR)/patches; mkdir -p $(PKG_BUILD_DIR)/patches) > + $(call PatchDir,$(PKG_BUILD_DIR),$(PATCH_DIR),) > +ifeq ($(BUILD_VARIANT),smallbuffers) > + $(call PatchDir,$(PKG_BUILD_DIR),$(PATCH_DIR)-smallbuffers,patches-smallbuffers) > +endif > + $(if $(QUILT),touch $(PKG_BUILD_DIR)/.quilt_used) > +endef > + > +define Quilt/Refresh/Package > + $(call Quilt/RefreshDir,$(PKG_BUILD_DIR),$(PATCH_DIR),) > +ifeq ($(BUILD_VARIANT),smallbuffers) > + $(call Quilt/RefreshDir,$(PKG_BUILD_DIR),$(PATCH_DIR)-smallbuffers,patches-smallbuffers) > +endif > +endef > + > define Build/Configure > cp $(STAGING_DIR)/usr/include/mac80211/ath/*.h $(PKG_BUILD_DIR) > endef > @@ -107,3 +134,4 @@ define Build/Compile > endef > > $(eval $(call KernelPackage,ath10k-ct)) > +$(eval $(call KernelPackage,ath10k-ct-smallbuffers)) > diff --git a/package/kernel/ath10k-ct/patches-smallbuffers/960-0010-ath10k-limit-htt-rx-ring-size.patch b/package/kernel/ath10k-ct/patches-smallbuffers/960-0010-ath10k-limit-htt-rx-ring-size.patch > new file mode 100644 > index 0000000000..f73b02e5ef > --- /dev/null > +++ b/package/kernel/ath10k-ct/patches-smallbuffers/960-0010-ath10k-limit-htt-rx-ring-size.patch > @@ -0,0 +1,22 @@ > +--- a/ath10k-4.19/htt.h > ++++ b/ath10k-4.19/htt.h > +@@ -226,7 +226,7 @@ enum htt_rx_ring_flags { > + }; > + > + #define HTT_RX_RING_SIZE_MIN 128 > +-#define HTT_RX_RING_SIZE_MAX 2048 > ++#define HTT_RX_RING_SIZE_MAX 512 > + #define HTT_RX_RING_SIZE HTT_RX_RING_SIZE_MAX > + #define HTT_RX_RING_FILL_LEVEL (((HTT_RX_RING_SIZE) / 2) - 1) > + #define HTT_RX_RING_FILL_LEVEL_DUAL_MAC (HTT_RX_RING_SIZE - 1) > +--- a/ath10k-5.2/htt.h > ++++ b/ath10k-5.2/htt.h > +@@ -226,7 +226,7 @@ enum htt_rx_ring_flags { > + }; > + > + #define HTT_RX_RING_SIZE_MIN 128 > +-#define HTT_RX_RING_SIZE_MAX 2048 > ++#define HTT_RX_RING_SIZE_MAX 512 > + #define HTT_RX_RING_SIZE HTT_RX_RING_SIZE_MAX > + #define HTT_RX_RING_FILL_LEVEL (((HTT_RX_RING_SIZE) / 2) - 1) > + #define HTT_RX_RING_FILL_LEVEL_DUAL_MAC (HTT_RX_RING_SIZE - 1) > diff --git a/package/kernel/ath10k-ct/patches-smallbuffers/960-0011-ath10k-limit-pci-buffer-size.patch b/package/kernel/ath10k-ct/patches-smallbuffers/960-0011-ath10k-limit-pci-buffer-size.patch > new file mode 100644 > index 0000000000..27c0032bfb > --- /dev/null > +++ b/package/kernel/ath10k-ct/patches-smallbuffers/960-0011-ath10k-limit-pci-buffer-size.patch > @@ -0,0 +1,76 @@ > +--- a/ath10k-4.19/pci.c > ++++ b/ath10k-4.19/pci.c > +@@ -131,7 +131,7 @@ static struct ce_attr host_ce_config_wla > + .flags = CE_ATTR_FLAGS, > + .src_nentries = 0, > + .src_sz_max = 2048, > +- .dest_nentries = 512, > ++ .dest_nentries = 128, > + .recv_cb = ath10k_pci_htt_htc_rx_cb, > + }, > + > +@@ -140,7 +140,7 @@ static struct ce_attr host_ce_config_wla > + .flags = CE_ATTR_FLAGS, > + .src_nentries = 0, > + .src_sz_max = 2048, > +- .dest_nentries = 128, > ++ .dest_nentries = 64, > + .recv_cb = ath10k_pci_htc_rx_cb, > + }, > + > +@@ -167,7 +167,7 @@ static struct ce_attr host_ce_config_wla > + .flags = CE_ATTR_FLAGS, > + .src_nentries = 0, > + .src_sz_max = 512, > +- .dest_nentries = 512, > ++ .dest_nentries = 128, > + .recv_cb = ath10k_pci_htt_rx_cb, > + }, > + > +@@ -192,7 +192,7 @@ static struct ce_attr host_ce_config_wla > + .flags = CE_ATTR_FLAGS, > + .src_nentries = 0, > + .src_sz_max = 2048, > +- .dest_nentries = 128, > ++ .dest_nentries = 96, > + .recv_cb = ath10k_pci_pktlog_rx_cb, > + }, > + > +--- a/ath10k-5.2/pci.c > ++++ b/ath10k-5.2/pci.c > +@@ -131,7 +131,7 @@ static struct ce_attr host_ce_config_wla > + .flags = CE_ATTR_FLAGS, > + .src_nentries = 0, > + .src_sz_max = 2048, > +- .dest_nentries = 512, > ++ .dest_nentries = 128, > + .recv_cb = ath10k_pci_htt_htc_rx_cb, > + }, > + > +@@ -140,7 +140,7 @@ static struct ce_attr host_ce_config_wla > + .flags = CE_ATTR_FLAGS, > + .src_nentries = 0, > + .src_sz_max = 2048, > +- .dest_nentries = 128, > ++ .dest_nentries = 64, > + .recv_cb = ath10k_pci_htc_rx_cb, > + }, > + > +@@ -167,7 +167,7 @@ static struct ce_attr host_ce_config_wla > + .flags = CE_ATTR_FLAGS, > + .src_nentries = 0, > + .src_sz_max = 512, > +- .dest_nentries = 512, > ++ .dest_nentries = 128, > + .recv_cb = ath10k_pci_htt_rx_cb, > + }, > + > +@@ -192,7 +192,7 @@ static struct ce_attr host_ce_config_wla > + .flags = CE_ATTR_FLAGS, > + .src_nentries = 0, > + .src_sz_max = 2048, > +- .dest_nentries = 128, > ++ .dest_nentries = 96, > + .recv_cb = ath10k_pci_pktlog_rx_cb, > + }, > + >
Hey Ben, On Wed, Dec 11, 2019 at 10:06:26AM -0800, Ben Greear wrote: > On 12/11/19 6:44 AM, Paul Fertser wrote: > > According to many bugreports [0][1][2] the default ath10k-ct kernel ... > And also if you want to just have the makefile pass a -DBUILD_ATH10K_SMALL or something > like that and #ifdef code in the ath10k-ct driver, then I'd apply that patch to ath10k-ct > so that you don't need the patches. I am offering my patch to the OpenWrt maintainers as kind of a stop-gap measure to get ath10k-ct working for the release (or in any way they think is appropriate). Another approach they can choose is to select the upstream ath10k for those devices. Otherwise some previously supported boards will require manual intervention to get WiFi working after an upgrade. Regarding your fwcfg idea, I am not sure it will work as it seems the PCI initialisation is happening before fwcfg is parsed and applied. Adding a Kconfig option is another possibility. But what do you think about an additional module parameter, wouldn't it be the cleanest solution in the long run? BTW, according to the git logs the patches were initially added by Christian Lamparter, so I hope he can clarify the situation a bit. Probably there were some performance tests executed back than to measure the impact.
On 12/11/19 11:16 AM, Paul Fertser wrote: > Hey Ben, > > On Wed, Dec 11, 2019 at 10:06:26AM -0800, Ben Greear wrote: >> On 12/11/19 6:44 AM, Paul Fertser wrote: >>> According to many bugreports [0][1][2] the default ath10k-ct kernel > ... >> And also if you want to just have the makefile pass a -DBUILD_ATH10K_SMALL or something >> like that and #ifdef code in the ath10k-ct driver, then I'd apply that patch to ath10k-ct >> so that you don't need the patches. > > I am offering my patch to the OpenWrt maintainers as kind of a > stop-gap measure to get ath10k-ct working for the release (or in any > way they think is appropriate). Another approach they can choose is to > select the upstream ath10k for those devices. Otherwise some > previously supported boards will require manual intervention to get > WiFi working after an upgrade. > > Regarding your fwcfg idea, I am not sure it will work as it seems the > PCI initialisation is happening before fwcfg is parsed and applied. > > Adding a Kconfig option is another possibility. > > But what do you think about an additional module parameter, wouldn't > it be the cleanest solution in the long run? If fwcfg will not work, and maybe it just will not due to the reasons you suggest, then I'm fine with adding a module parameter to ath10k-ct. You may want to conditionally compile the default value of that module parameter so that on the small platforms the user does not actually have to set the module param if they want the default (small) values? Thanks, Ben > > BTW, according to the git logs the patches were initially added by > Christian Lamparter, so I hope he can clarify the situation a > bit. Probably there were some performance tests executed back than to > measure the impact. >
On Wednesday, 11 December 2019 20:16:52 CET Paul Fertser wrote: > Hey Ben, > > On Wed, Dec 11, 2019 at 10:06:26AM -0800, Ben Greear wrote: > > On 12/11/19 6:44 AM, Paul Fertser wrote: > > > According to many bugreports [0][1][2] the default ath10k-ct kernel > ... > > And also if you want to just have the makefile pass a -DBUILD_ATH10K_SMALL or something > > like that and #ifdef code in the ath10k-ct driver, then I'd apply that patch to ath10k-ct > > so that you don't need the patches. > > I am offering my patch to the OpenWrt maintainers as kind of a > stop-gap measure to get ath10k-ct working for the release (or in any > way they think is appropriate). Another approach they can choose is to > select the upstream ath10k for those devices. Otherwise some > previously supported boards will require manual intervention to get > WiFi working after an upgrade. > > Regarding your fwcfg idea, I am not sure it will work as it seems the > PCI initialisation is happening before fwcfg is parsed and applied. > > Adding a Kconfig option is another possibility. > > But what do you think about an additional module parameter, wouldn't > it be the cleanest solution in the long run? > > BTW, according to the git logs the patches were initially added by > Christian Lamparter, so I hope he can clarify the situation a > bit. Probably there were some performance tests executed back than to > measure the impact. > Heh no. These patches come up in discussions time and time again. And I would rather see them being removed alltogether. What I can tell you is that the Idea of limiting ath10k memory thirst came from Qualcomm itself. If you look on the ML you can find the old posts like: <https://www.mail-archive.com/lede-dev@lists.infradead.org/msg04738.html> And for reference: Here's a independent bootlog (from pepe2k/Piotr Dymacz no less) with the "Low Memory System" messages for the RT-AC58U: <https://gist.github.com/pepe2k/eba2766f05ccf4e089347c531c49848b> From what I remember Sven Eckelmann also measured the impact from the patches on the performance and posted his results to the OpenWrt ML (google will find them). I think for this to have any chance of moving forward you'll need to pressure your ODMs and if that doesn't work: Go with a different WIFI chip vendor that supports low memory devices, or add more RAM. From what I can tell, Qualcomm nowadays gets what they want "for free" and for the past four-five years, they certainly didn't feel pressured to add "low memory" support to ath10k. Cheers, Christian
Thank you for the answer Christian, On Sun, Dec 15, 2019 at 12:00:48PM +0100, Christian Lamparter wrote: > I think for this to have any chance of moving forward you'll need to > pressure your ODMs and if that doesn't work: Go with a different WIFI > chip vendor that supports low memory devices, or add more RAM. > From what I can tell, Qualcomm nowadays gets what they want "for free" > and for the past four-five years, they certainly didn't feel pressured > to add "low memory" support to ath10k. FWIW, OpenWrt's ath10k vendor is CT now, not QCA, so it's not much relevant what do ODMs and (whatever is left from) QCA say, I guess. It would be kind of weird to force OpenWrt users of certain devices to downgrade to upstream ath10k (or to abandon hardware that is working fine for them with previous OpenWrt release) just because Atheros no longer exist and Qualcomm can't care less about free software community, don't you think so? I'll try to find the mailing list posts you're talking about to help Ben decide if he is still OK with those patches getting used on low-RAM devices in OpenWrt or not.
On Sun, Dec 15, 2019 at 12:00:48PM +0100, Christian Lamparter wrote: > From what I remember Sven Eckelmann also measured the impact from the > patches on the performance and posted his results to the OpenWrt ML > (google will find them). https://github.com/freifunk-gluon/gluon/pull/1440 is what contains all the relevant data. So Ben is well aware of the performance implications. "Even a 256MB device with three radios can go OOM" It looks like having a version of kmod-ath10k-ct with reduced buffers would be beneficial to the OpenWrt users.
On Sunday, 15 December 2019 13:01:14 CET Paul Fertser wrote: > Thank you for the answer Christian, > > On Sun, Dec 15, 2019 at 12:00:48PM +0100, Christian Lamparter wrote: > > I think for this to have any chance of moving forward you'll need to > > pressure your ODMs and if that doesn't work: Go with a different WIFI > > chip vendor that supports low memory devices, or add more RAM. > > From what I can tell, Qualcomm nowadays gets what they want "for free" > > and for the past four-five years, they certainly didn't feel pressured > > to add "low memory" support to ath10k. > > FWIW, OpenWrt's ath10k vendor is CT now, not QCA, so it's not much > relevant what do ODMs and (whatever is left from) QCA say, I guess. Well, not sure what you are trying to say here. But I think Ben is free to do what he wants as well. For example see the: "ath10k: add LED and GPIO controlling support for various chipsets" patch that OpenWrt has been carrying because neither upstream (linux-wireless) nor CT wants to integrate it. <https://github.com/openwrt/openwrt/blob/master/package/kernel/ath10k-ct/patches/201-ath10k-4.16_add-LED-and-GPIO-controlling-support-for-various-chipsets.patch> The situation with the "low memory" support wasn't much better. Because from what I remember, there was a discussion about this very topic between Ben an Hauke in the past (and you can see how it played out, since you wouldn't have posted this series if it was integrated back then). But it seems that Ben had a change of heart in this regard. I don't know the details or why, But it makes sense because it would enable his company to save some money for the systems his company sells: <https://www.candelatech.com/lf_systems.php> so there is some value in supporting these devices, especially if someone else does all the work for it. > It would be kind of weird to force OpenWrt users of certain devices to > downgrade to upstream ath10k (or to abandon hardware that is working > fine for them with previous OpenWrt release) just because Atheros no > longer exist and Qualcomm can't care less about free software > community, don't you think so? This is something like "another" 32/4 situation, right? Well, can you tell me what was the result of that? > I'll try to find the mailing list posts you're talking about to help > Ben decide if he is still OK with those patches getting used on > low-RAM devices in OpenWrt or not. Well, if you look at ath10k-ct (<https://github.com/greearb/ath10k-ct>), you see that Ben takes upstream ath10k, adds his patches and pulls upstream fixes. So if you are willing to work for it anyways, you might as well go with upstream Linux-wireless and see what they want. After all the QSDK has the "Low Memory" mode. Cheers, Christian
On 12/15/2019 05:09 AM, Christian Lamparter wrote: > On Sunday, 15 December 2019 13:01:14 CET Paul Fertser wrote: >> Thank you for the answer Christian, >> >> On Sun, Dec 15, 2019 at 12:00:48PM +0100, Christian Lamparter wrote: >>> I think for this to have any chance of moving forward you'll need to >>> pressure your ODMs and if that doesn't work: Go with a different WIFI >>> chip vendor that supports low memory devices, or add more RAM. >>> From what I can tell, Qualcomm nowadays gets what they want "for free" >>> and for the past four-five years, they certainly didn't feel pressured >>> to add "low memory" support to ath10k. >> >> FWIW, OpenWrt's ath10k vendor is CT now, not QCA, so it's not much >> relevant what do ODMs and (whatever is left from) QCA say, I guess. > Well, not sure what you are trying to say here. But I think Ben is free > to do what he wants as well. For example see the: > "ath10k: add LED and GPIO controlling support for various chipsets" > patch that OpenWrt has been carrying because neither upstream (linux-wireless) > nor CT wants to integrate it. > <https://github.com/openwrt/openwrt/blob/master/package/kernel/ath10k-ct/patches/201-ath10k-4.16_add-LED-and-GPIO-controlling-support-for-various-chipsets.patch> > > The situation with the "low memory" support wasn't much better. Because > from what I remember, there was a discussion about this very topic between > Ben an Hauke in the past (and you can see how it played out, since you wouldn't > have posted this series if it was integrated back then). > But it seems that Ben had a change of heart in this regard. I don't know the > details or why, But it makes sense because it would enable his company to save > some money for the systems his company sells: > <https://www.candelatech.com/lf_systems.php> so there is some value > in supporting these devices, especially if someone else does all the work > for it. My goal is to have stable and fully featured ath10k that works well for higher powered systems by default (our test rigs are typically powerful x86 with gigs of RAM running Fedora or similar). OpenWRT is all about adding patches on top of upstream projects for specific platforms, so that would easily work with ath10k-ct too. And, if someone wants to write a patch that either modifies ath10k-ct for OpenWRT to work better on certain systems, then by all means, go ahead. This should be just as easy as doing the same thing for upstream ath10k. If someone writes a patch that conditionally compiles for OpenWRT and/or allows OpenWRT to configure smaller memory usage, then I will apply that to my ath10k-ct driver (with caveat that defaults for non openwrt systems needs to remain as is). Thanks, Ben
On 15/12/19 14:09, Christian Lamparter wrote: > > But it seems that Ben had a change of heart in this regard. I don't know the > details or why, But it makes sense because it would enable his company to save > some money for the systems his company sells: > <https://www.candelatech.com/lf_systems.php> so there is some value > in supporting these devices, especially if someone else does all the work > for it. These are wifi/network testing equipment, not network devices. Also I don't see the value in "saving some money" by using a bit less RAM when the cheaper system is sold for 3k, and most stuff is above 10k. You could use standard whitebox x86 stuff at that price point. -Alberto
On 12/16/2019 03:26 AM, Alberto Bursi wrote: > > On 15/12/19 14:09, Christian Lamparter wrote: >> >> But it seems that Ben had a change of heart in this regard. I don't know the >> details or why, But it makes sense because it would enable his company to save >> some money for the systems his company sells: >> <https://www.candelatech.com/lf_systems.php> so there is some value >> in supporting these devices, especially if someone else does all the work >> for it. > > These are wifi/network testing equipment, not network devices. > > Also I don't see the value in "saving some money" by using a bit less RAM > > when the cheaper system is sold for 3k, and most stuff is above 10k. > > You could use standard whitebox x86 stuff at that price point. The low-ram thing is for people using OpenWRT on low-powered AP boards. We don't need it for our test equipment. Hopefully someone can test Paul Fertser's patches, and if they work well, then can be applied to OpenWRT. Maybe later we can conditionally compile it directly into ath10k-ct instead of having the extra patch in OpenWRT. Thanks, Ben
Hello, On Mon, Dec 16, 2019 at 12:27 PM Alberto Bursi <bobafetthotmail@gmail.com> wrote: > > > On 15/12/19 14:09, Christian Lamparter wrote: > > > > But it seems that Ben had a change of heart in this regard. I don't know the > > details or why, But it makes sense because it would enable his company to save > > some money for the systems his company sells: > > <https://www.candelatech.com/lf_systems.php> so there is some value > > in supporting these devices, especially if someone else does all the work > > for it. > > These are wifi/network testing equipment, not network devices. > > Also I don't see the value in "saving some money" by using a bit less RAM > > when the cheaper system is sold for 3k, and most stuff is above 10k. > > You could use standard whitebox x86 stuff at that price point. I'm glad this is getting some attention and others are chiming in. But let me tell you first, that I'm not an opponent of the "American way", I'm trying to make sense of it though and also what would happen to the ath10k GPIO patches that got quietly dropped from your reply there... As for the "These are wifi/network testing equipment, not network devices." True and If you are interested you can buy cheaper devices like <https://www.candelatech.com/ct314_product.php> from the company as well: "The CT314 is a low-power and affordable applicance with a single 10/100 Ethernet port and one Broadcome 802.11b/g/n Wireless interface. It is targeted at users who wish to have an inexpensive appliance that can be left at remote sites for network monitoring and lower speed testing. The maximum throughput is about 90Mbps bi-directional wired. Wireless throughput is steady at 38Mbps and can peak at 48Mpbs. The CT314 is based on the Raspberry PI B version 3 platform, running the Ubuntu Server OS. [...]". I know these have not much to do with the issue at hand ("low-memory system" support in ath10k(-ct) with OpenWrt). But as Ben explained in the follow-up that he has a keen interest for supporting the ath10k-ct driver+firmware and he's doing a great job with the ath10k-ct issue tracker. Cheers, Christian
On 16/12/19 21:04, Christian Lamparter wrote: > Hello, > > On Mon, Dec 16, 2019 at 12:27 PM Alberto Bursi > <bobafetthotmail@gmail.com> wrote: >> >> >> On 15/12/19 14:09, Christian Lamparter wrote: >>> >>> But it seems that Ben had a change of heart in this regard. I don't know the >>> details or why, But it makes sense because it would enable his company to save >>> some money for the systems his company sells: >>> <https://www.candelatech.com/lf_systems.php> so there is some value >>> in supporting these devices, especially if someone else does all the work >>> for it. >> >> These are wifi/network testing equipment, not network devices. >> >> Also I don't see the value in "saving some money" by using a bit less RAM >> >> when the cheaper system is sold for 3k, and most stuff is above 10k. >> >> You could use standard whitebox x86 stuff at that price point. > > I'm glad this is getting some attention and others are chiming in. But > let me tell > you first, that I'm not an opponent of the "American way", I'm trying > to make sense > of it though and also what would happen to the ath10k GPIO patches that got > quietly dropped from your reply there... I was just commenting about the fact that they clearly don't care about RAM consumption for their own hardware, I found it strange that you pulled that up as a "potential way to save money". Saving 10-20$ (RAM prices) on a low-volume high-price item costing thousands of dollars is mostly irrelevant. > > As for the "These are wifi/network testing equipment, not network devices." > True and If you are interested you can buy cheaper devices like > <https://www.candelatech.com/ct314_product.php> from the company as well: > When I said "expensive devices" I was talking of their devices that could mount a ath10k-supported card. A Raspi really does not. > > I know these have not much to do with the issue at hand ("low-memory system" > support in ath10k(-ct) with OpenWrt). But as Ben explained in the follow-up that > he has a keen interest for supporting the ath10k-ct driver+firmware > and he's doing > a great job with the ath10k-ct issue tracker. > I fully agree with merging and possibly upstreaming the current patches about a build option to reduce buffer sizes so that this thing does not OOM on devices that OpenWrt supports. My remarks about RAM usage being irrelevant was specifc to their own usecase in their "expensive test equipment". -Alberto
diff --git a/package/kernel/ath10k-ct/Makefile b/package/kernel/ath10k-ct/Makefile index dbf75fe174..d5726a1c88 100644 --- a/package/kernel/ath10k-ct/Makefile +++ b/package/kernel/ath10k-ct/Makefile @@ -35,6 +35,7 @@ define KernelPackage/ath10k-ct $(PKG_BUILD_DIR)/ath10k$(CT_KVER)/ath10k_core.ko AUTOLOAD:=$(call AutoProbe,ath10k_pci) PROVIDES:=kmod-ath10k + VARIANT:=regular endef define KernelPackage/ath10k-ct/config @@ -42,7 +43,17 @@ define KernelPackage/ath10k-ct/config config ATH10K-CT_LEDS bool "Enable LED support" default y - depends on PACKAGE_kmod-ath10k-ct + depends on PACKAGE_kmod-ath10k-ct || PACKAGE_kmod-ath10k-ct-smallbuffers +endef + +define KernelPackage/ath10k-ct-smallbuffers +$(call KernelPackage/ath10k-ct) + TITLE+= (small buffers to work on low-RAM devices) + VARIANT:=smallbuffers +endef + +define KernelPackage/ath10k-ct-smallbuffers/config +$(call KernelPackage/ath10k-ct/config) endef NOSTDINC_FLAGS = \ @@ -90,6 +101,22 @@ ifeq ($(CONFIG_ATH10K-CT_LEDS),y) NOSTDINC_FLAGS += -DCONFIG_ATH10K_LEDS endif +define Build/Patch + $(if $(QUILT),rm -rf $(PKG_BUILD_DIR)/patches; mkdir -p $(PKG_BUILD_DIR)/patches) + $(call PatchDir,$(PKG_BUILD_DIR),$(PATCH_DIR),) +ifeq ($(BUILD_VARIANT),smallbuffers) + $(call PatchDir,$(PKG_BUILD_DIR),$(PATCH_DIR)-smallbuffers,patches-smallbuffers) +endif + $(if $(QUILT),touch $(PKG_BUILD_DIR)/.quilt_used) +endef + +define Quilt/Refresh/Package + $(call Quilt/RefreshDir,$(PKG_BUILD_DIR),$(PATCH_DIR),) +ifeq ($(BUILD_VARIANT),smallbuffers) + $(call Quilt/RefreshDir,$(PKG_BUILD_DIR),$(PATCH_DIR)-smallbuffers,patches-smallbuffers) +endif +endef + define Build/Configure cp $(STAGING_DIR)/usr/include/mac80211/ath/*.h $(PKG_BUILD_DIR) endef @@ -107,3 +134,4 @@ define Build/Compile endef $(eval $(call KernelPackage,ath10k-ct)) +$(eval $(call KernelPackage,ath10k-ct-smallbuffers)) diff --git a/package/kernel/ath10k-ct/patches-smallbuffers/960-0010-ath10k-limit-htt-rx-ring-size.patch b/package/kernel/ath10k-ct/patches-smallbuffers/960-0010-ath10k-limit-htt-rx-ring-size.patch new file mode 100644 index 0000000000..f73b02e5ef --- /dev/null +++ b/package/kernel/ath10k-ct/patches-smallbuffers/960-0010-ath10k-limit-htt-rx-ring-size.patch @@ -0,0 +1,22 @@ +--- a/ath10k-4.19/htt.h ++++ b/ath10k-4.19/htt.h +@@ -226,7 +226,7 @@ enum htt_rx_ring_flags { + }; + + #define HTT_RX_RING_SIZE_MIN 128 +-#define HTT_RX_RING_SIZE_MAX 2048 ++#define HTT_RX_RING_SIZE_MAX 512 + #define HTT_RX_RING_SIZE HTT_RX_RING_SIZE_MAX + #define HTT_RX_RING_FILL_LEVEL (((HTT_RX_RING_SIZE) / 2) - 1) + #define HTT_RX_RING_FILL_LEVEL_DUAL_MAC (HTT_RX_RING_SIZE - 1) +--- a/ath10k-5.2/htt.h ++++ b/ath10k-5.2/htt.h +@@ -226,7 +226,7 @@ enum htt_rx_ring_flags { + }; + + #define HTT_RX_RING_SIZE_MIN 128 +-#define HTT_RX_RING_SIZE_MAX 2048 ++#define HTT_RX_RING_SIZE_MAX 512 + #define HTT_RX_RING_SIZE HTT_RX_RING_SIZE_MAX + #define HTT_RX_RING_FILL_LEVEL (((HTT_RX_RING_SIZE) / 2) - 1) + #define HTT_RX_RING_FILL_LEVEL_DUAL_MAC (HTT_RX_RING_SIZE - 1) diff --git a/package/kernel/ath10k-ct/patches-smallbuffers/960-0011-ath10k-limit-pci-buffer-size.patch b/package/kernel/ath10k-ct/patches-smallbuffers/960-0011-ath10k-limit-pci-buffer-size.patch new file mode 100644 index 0000000000..27c0032bfb --- /dev/null +++ b/package/kernel/ath10k-ct/patches-smallbuffers/960-0011-ath10k-limit-pci-buffer-size.patch @@ -0,0 +1,76 @@ +--- a/ath10k-4.19/pci.c ++++ b/ath10k-4.19/pci.c +@@ -131,7 +131,7 @@ static struct ce_attr host_ce_config_wla + .flags = CE_ATTR_FLAGS, + .src_nentries = 0, + .src_sz_max = 2048, +- .dest_nentries = 512, ++ .dest_nentries = 128, + .recv_cb = ath10k_pci_htt_htc_rx_cb, + }, + +@@ -140,7 +140,7 @@ static struct ce_attr host_ce_config_wla + .flags = CE_ATTR_FLAGS, + .src_nentries = 0, + .src_sz_max = 2048, +- .dest_nentries = 128, ++ .dest_nentries = 64, + .recv_cb = ath10k_pci_htc_rx_cb, + }, + +@@ -167,7 +167,7 @@ static struct ce_attr host_ce_config_wla + .flags = CE_ATTR_FLAGS, + .src_nentries = 0, + .src_sz_max = 512, +- .dest_nentries = 512, ++ .dest_nentries = 128, + .recv_cb = ath10k_pci_htt_rx_cb, + }, + +@@ -192,7 +192,7 @@ static struct ce_attr host_ce_config_wla + .flags = CE_ATTR_FLAGS, + .src_nentries = 0, + .src_sz_max = 2048, +- .dest_nentries = 128, ++ .dest_nentries = 96, + .recv_cb = ath10k_pci_pktlog_rx_cb, + }, + +--- a/ath10k-5.2/pci.c ++++ b/ath10k-5.2/pci.c +@@ -131,7 +131,7 @@ static struct ce_attr host_ce_config_wla + .flags = CE_ATTR_FLAGS, + .src_nentries = 0, + .src_sz_max = 2048, +- .dest_nentries = 512, ++ .dest_nentries = 128, + .recv_cb = ath10k_pci_htt_htc_rx_cb, + }, + +@@ -140,7 +140,7 @@ static struct ce_attr host_ce_config_wla + .flags = CE_ATTR_FLAGS, + .src_nentries = 0, + .src_sz_max = 2048, +- .dest_nentries = 128, ++ .dest_nentries = 64, + .recv_cb = ath10k_pci_htc_rx_cb, + }, + +@@ -167,7 +167,7 @@ static struct ce_attr host_ce_config_wla + .flags = CE_ATTR_FLAGS, + .src_nentries = 0, + .src_sz_max = 512, +- .dest_nentries = 512, ++ .dest_nentries = 128, + .recv_cb = ath10k_pci_htt_rx_cb, + }, + +@@ -192,7 +192,7 @@ static struct ce_attr host_ce_config_wla + .flags = CE_ATTR_FLAGS, + .src_nentries = 0, + .src_sz_max = 2048, +- .dest_nentries = 128, ++ .dest_nentries = 96, + .recv_cb = ath10k_pci_pktlog_rx_cb, + }, +
According to many bugreports [0][1][2] the default ath10k-ct kernel module is unusable on devices with just 64 MiB RAM or with 128 MiB and dual ath10k cards. The target boards boot but eventually oom-killer starts to interfere with normal operation, so the current state is effectively broken. Since the two patches in question might have a performance impact (and possibly some other unexpected side-effects) a dedicated build variant is added so that users of the low RAM devices can still benefit from all the ath10k-ct advantages. [0] http://lists.infradead.org/pipermail/openwrt-devel/2019-December/020573.html [1] https://github.com/openwrt/openwrt/pull/1077 [2] https://bugs.openwrt.org/index.php?do=details&task_id=2664 Signed-off-by: Paul Fertser <fercerpav@gmail.com> --- package/kernel/ath10k-ct/Makefile | 30 +++++++- ...0-0010-ath10k-limit-htt-rx-ring-size.patch | 22 ++++++ ...60-0011-ath10k-limit-pci-buffer-size.patch | 76 +++++++++++++++++++ 3 files changed, 127 insertions(+), 1 deletion(-) create mode 100644 package/kernel/ath10k-ct/patches-smallbuffers/960-0010-ath10k-limit-htt-rx-ring-size.patch create mode 100644 package/kernel/ath10k-ct/patches-smallbuffers/960-0011-ath10k-limit-pci-buffer-size.patch