diff mbox series

#package #poky #yocto connman: Bug in connman causing endless write to file

Message ID gwtb.1736773617531475991.iGrZ@lists.yoctoproject.org (mailing list archive)
State New
Headers show
Series #package #poky #yocto connman: Bug in connman causing endless write to file | expand

Commit Message

Dieterich, Peter-Simon Jan. 13, 2025, 1:06 p.m. UTC
Dear maintainers of poky,

we found a bug in connman (https://lore.kernel.org/connman/AM0PR09MB4500BF2A13FBACE6E7046E23881F2@AM0PR09MB4500.eurprd09.prod.outlook.com/) that can cause high-cpu load and an endless write to a file in /var/lib/connman/ on systems that have been updated from a yocto version with 32-bit time_t to yocto versions that have 64-bit time_t support enabled (https://patchwork.yoctoproject.org/project/oe-core/patch/20230426095036.2632847-9-alex@linutronix.de/). In our case, we went from Dunfell to Scarthgap.

Our solution was to disable the stats module. We think it would be best to have the stats module disabled by default as it is a functionality that most users won't need.

Should I send the patch in a separate message?

Best,
Peter

Signed-off-by: Peter-Simon Dieterich <peter-simon.dieterich@vaillant-group.com>
---
meta/recipes-connectivity/connman/connman.inc | 1 +
1 file changed, 1 insertion(+)

Comments

Alexander Kanavin Jan. 13, 2025, 1:17 p.m. UTC | #1
On Mon, 13 Jan 2025 at 14:06, Dieterich, Peter-Simon via
lists.yoctoproject.org
<peter-simon.dieterich=vaillant-group.com@lists.yoctoproject.org>
wrote:
> Our solution was to disable the stats module. We think it would be best to have the stats module disabled by default as it is a functionality that most users won't need.
>
> Should I send the patch in a separate message?

First please find out whether it's possible to solve the data format
issue with an upstream fix. Disabling something because of a bug
triggered by a very corner case isn't a great idea, particularly when
justified by broad, difficult to verify statements like 'most users
don't need this'.

Alex
Dieterich, Peter-Simon Jan. 13, 2025, 2:15 p.m. UTC | #2
On Mon, Jan 13, 2025 at 05:17 AM, Alexander Kanavin wrote:

> 
> First please find out whether it's possible to solve the data format
> issue with an upstream fix. Disabling something because of a bug
> triggered by a very corner case isn't a great idea, particularly when
> justified by broad, difficult to verify statements like 'most users
> don't need this'.

Will do. I agree that the statement is very broad. To be honest, I don't know the exact use case of this feature. From what I've seen it saves connection statistics that look very much like debug information, for which you also need special binaries to analyze.

Concerning the very corner case, I'd like to point out that this issue affects all users using connman and migrating existing systems from previous yocto versions without 64-bit support to a current version with 64-bit support.
Alexander Kanavin Jan. 13, 2025, 3:52 p.m. UTC | #3
On Mon, 13 Jan 2025 at 15:15, Dieterich, Peter-Simon via
lists.yoctoproject.org
<peter-simon.dieterich=vaillant-group.com@lists.yoctoproject.org>
wrote:
> Will do. I agree that the statement is very broad. To be honest, I don't know the exact use case of this feature. From what I've seen it saves connection statistics that look very much like debug information, for which you also need special binaries to analyze.

Right, I'd suggest that
- you try to work out a solution with upstream; if you can propose a
proper source patch that's better than just reporting an issue and
leaving them to figure out what to do about it.
- you get a full understanding of the difference between
--enable-stats and --disable-stats by studying the connman source

If upstream shows no interest, or otherwise there's no progress, we
can revisit the disabling, but even then it should be a recipe option
that can be adjusted:
PACKAGECONFIG[stats] = "--enable-stats,--disable-stats"
At that point the "full understanding" would become useful to write a
good commit message for the disabling-by-default.

Alex
diff mbox series

Patch

diff --git a/meta/recipes-connectivity/connman/connman.inc b/meta/recipes-connectivity/connman/connman.inc
index 073061eeda..45f1621088 100644
--- a/meta/recipes-connectivity/connman/connman.inc
+++ b/meta/recipes-connectivity/connman/connman.inc
@@ -28,6 +28,7 @@  EXTRA_OECONF += "\
--enable-tools \
--disable-polkit \
--runstatedir=/run \
+    --disable-stats \
"
# For smooth operation it would be best to start only one wireless daemon at a time.
# If wpa-supplicant is running, connman will use it preferentially.