When using pm_nl_ctl to validate userspace path-manager's behaviours, it
was failing on 32-bit architectures ~half of the time.

pm_nl_ctl was not reporting any error but the command was not doing what
it was expected to do. As a result, the expected linked event was not
triggered after and the test failed.

This is due to the fact the token given in argument to the application
was parsed as an integer with atoi(): in a 32-bit arch, if the number
was bigger than INT_MAX, 2147483647 was used instead.

This can simply be fixed by using strtoul() instead of atoi().

The errors have been seen "by change" when manually looking at the
results from LKFT.

Fixes: 9a0b36509df0 ("selftests: mptcp: support MPTCP_PM_CMD_ANNOUNCE")
Fixes: ecd2a77d672f ("selftests: mptcp: support MPTCP_PM_CMD_REMOVE")
Fixes: cf8d0a6dfd64 ("selftests: mptcp: support MPTCP_PM_CMD_SUBFLOW_CREATE")
Fixes: 57cc361b8d38 ("selftests: mptcp: support MPTCP_PM_CMD_SUBFLOW_DESTROY")
Fixes: ca188a25d43f ("selftests: mptcp: userspace PM support for MP_PRIO signals")
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
index abddf4c63e79..1887bd61bd9a 100644
--- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
+++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
@@ -425,7 +425,7 @@ int dsf(int fd, int pm_family, int argc, char *argv[])
 	}
 
 	/* token */
-	token = atoi(params[4]);
+	token = strtoul(params[4], NULL, 10);
 	rta = (void *)(data + off);
 	rta->rta_type = MPTCP_PM_ATTR_TOKEN;
 	rta->rta_len = RTA_LENGTH(4);
@@ -551,7 +551,7 @@ int csf(int fd, int pm_family, int argc, char *argv[])
 	}
 
 	/* token */
-	token = atoi(params[4]);
+	token = strtoul(params[4], NULL, 10);
 	rta = (void *)(data + off);
 	rta->rta_type = MPTCP_PM_ATTR_TOKEN;
 	rta->rta_len = RTA_LENGTH(4);
@@ -598,7 +598,7 @@ int remove_addr(int fd, int pm_family, int argc, char *argv[])
 			if (++arg >= argc)
 				error(1, 0, " missing token value");
 
-			token = atoi(argv[arg]);
+			token = strtoul(argv[arg], NULL, 10);
 			rta = (void *)(data + off);
 			rta->rta_type = MPTCP_PM_ATTR_TOKEN;
 			rta->rta_len = RTA_LENGTH(4);
@@ -710,7 +710,7 @@ int announce_addr(int fd, int pm_family, int argc, char *argv[])
 			if (++arg >= argc)
 				error(1, 0, " missing token value");
 
-			token = atoi(argv[arg]);
+			token = strtoul(argv[arg], NULL, 10);
 		} else
 			error(1, 0, "unknown keyword %s", argv[arg]);
 	}
@@ -1347,7 +1347,7 @@ int set_flags(int fd, int pm_family, int argc, char *argv[])
 				error(1, 0, " missing token value");
 
 			/* token */
-			token = atoi(argv[arg]);
+			token = strtoul(argv[arg], NULL, 10);
 		} else if (!strcmp(argv[arg], "flags")) {
 			char *tok, *str;
 

---
base-commit: 619c3c3e5a9b93d2a5f62298a592919b245db5e3
change-id: 20230704-mptcp-issue-368-selftests-32-bits-cf7cfe2d8632

Best regards,
-- 
Matthieu Baerts <matthieu.baerts@tessares.net>

Hello,

On 04/07/2023 18:21, Matthieu Baerts wrote:
> When using pm_nl_ctl to validate userspace path-manager's behaviours, it
> was failing on 32-bit architectures ~half of the time.
> 
> pm_nl_ctl was not reporting any error but the command was not doing what
> it was expected to do. As a result, the expected linked event was not
> triggered after and the test failed.
> 
> This is due to the fact the token given in argument to the application
> was parsed as an integer with atoi(): in a 32-bit arch, if the number
> was bigger than INT_MAX, 2147483647 was used instead.
> 
> This can simply be fixed by using strtoul() instead of atoi().
> 
> The errors have been seen "by change" when manually looking at the
> results from LKFT.

The fix looks simple enough. I suggest to apply it to be able to send it
with others to netdev, hoping to have it backported "soon" so I don't
forget to check if there are other errors in LKFT results :)

I also added the simple fix for the selftests' config.

So now in our tree (fixes for -net):

New patches for t/upstream-net and t/upstream:
- 1e277517c902: selftests: mptcp: depend on SYN_COOKIES
- ad1a75f2c761: selftests: mptcp: pm_nl_ctl: fix 32-bit support
- Results: 8e89661e0490..cec2abcca31c (export-net)
- Results: a8188147cc02..c44a30e46e17 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20230704T191805
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230704T191805

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Hello,

On 04/07/2023 18:21, Matthieu Baerts wrote:
> When using pm_nl_ctl to validate userspace path-manager's behaviours, it
> was failing on 32-bit architectures ~half of the time.
> 
> pm_nl_ctl was not reporting any error but the command was not doing what
> it was expected to do. As a result, the expected linked event was not
> triggered after and the test failed.
> 
> This is due to the fact the token given in argument to the application
> was parsed as an integer with atoi(): in a 32-bit arch, if the number
> was bigger than INT_MAX, 2147483647 was used instead.
> 
> This can simply be fixed by using strtoul() instead of atoi().
> 
> The errors have been seen "by change" when manually looking at the
> results from LKFT.

This is a fix for -net of course, I forgot to edit the subject, sorry
for that.

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net