ssn_offset field is u32 and is placed into the netlink response with
nla_put_u32(), but only 2 bytes are reserved for the attribute payload
in subflow_get_info_size() (even though it makes no difference in the end,
as it is aligned up to 4 bytes).  Supply the correct argument to the relevant
nla_total_size() call to make it less confusing.

Fixes: 5147dfb5083204d6 ("mptcp: allow dumping subflow context to userspace")
Signed-off-by: Eugene Syromiatnikov <esyr@redhat.com>
---
 net/mptcp/diag.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/diag.c b/net/mptcp/diag.c
index 3ae46b545d2c..2d3efb405437 100644
--- a/net/mptcp/diag.c
+++ b/net/mptcp/diag.c
@@ -94,7 +94,7 @@ static size_t subflow_get_info_size(const struct sock *sk)
 		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_RELWRITE_SEQ */
 		nla_total_size_64bit(8) +	/* MPTCP_SUBFLOW_ATTR_MAP_SEQ */
 		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_MAP_SFSEQ */
-		nla_total_size(2) +	/* MPTCP_SUBFLOW_ATTR_SSN_OFFSET */
+		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_SSN_OFFSET */
 		nla_total_size(2) +	/* MPTCP_SUBFLOW_ATTR_MAP_DATALEN */
 		nla_total_size(4) +	/* MPTCP_SUBFLOW_ATTR_FLAGS */
 		nla_total_size(1) +	/* MPTCP_SUBFLOW_ATTR_ID_REM */
-- 
2.28.0

Hi Eugene,

On 09/08/2024 11:43, Eugene Syromiatnikov wrote:
> ssn_offset field is u32 and is placed into the netlink response with
> nla_put_u32(), but only 2 bytes are reserved for the attribute payload
> in subflow_get_info_size() (even though it makes no difference in the end,
> as it is aligned up to 4 bytes).  Supply the correct argument to the relevant
> nla_total_size() call to make it less confusing.

Good catch, thank you for sharing this patch!

The modification in the code and the description look good to me!


Please note that when you submit a patch to the Netdev mailing list, the
subject should have the 'PATCH net' prefix (or net-next) as mentioned in
the Netdev doc:

  https://docs.kernel.org/process/maintainer-netdev.html

> Fixes: 5147dfb5083204d6 ("mptcp: allow dumping subflow context to userspace")

Checkpatch is complaining about this line, because the commit ID should
have 12 chars:


https://docs.kernel.org/process/submitting-patches.html#describe-your-changes

Note that checkpatch.pl also points out the fact that the lines in the
commit description should have max 75 chars -- ideally 72 -- which is
not the case above with the line ending with 'relevant'.

Do you mind sending a v2 with these small fixes, so your patch can be
directly applied in the net tree, please?

(Do not forget to wait 24h between two versions as mentioned in the doc)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

On Fri, Aug 09, 2024 at 12:42:19PM +0200, Matthieu Baerts wrote:
> Do you mind sending a v2 with these small fixes, so your patch can be
> directly applied in the net tree, please?

Sure, will do.

Hi Eugene,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/10317353849

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/f842e648d7b6
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=878144


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)