[PATCH net] netlink: specs: mptcp: fix port endianness

Asbjørn Sloth Tønnesen posted 1 patch 3 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20240911091003.1112179-1-ast@fiberby.net
Documentation/netlink/specs/mptcp_pm.yaml | 1 -
1 file changed, 1 deletion(-)
[PATCH net] netlink: specs: mptcp: fix port endianness
Posted by Asbjørn Sloth Tønnesen 3 months, 1 week ago
The MPTCP port attribute is in host endianness, but was documented
as big-endian in the ynl specification.

Below are two examples from net/mptcp/pm_netlink.c showing that the
attribute is converted to/from host endianness for use with netlink.

Import from netlink:
  addr->port = htons(nla_get_u16(tb[MPTCP_PM_ADDR_ATTR_PORT]))

Export to netlink:
  nla_put_u16(skb, MPTCP_PM_ADDR_ATTR_PORT, ntohs(addr->port))

Where addr->port is defined as __be16.

No functional change intended.

Fixes: bc8aeb2045e2 ("Documentation: netlink: add a YAML spec for mptcp")
Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
---
 Documentation/netlink/specs/mptcp_pm.yaml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml
index af525ed29792..30d8342cacc8 100644
--- a/Documentation/netlink/specs/mptcp_pm.yaml
+++ b/Documentation/netlink/specs/mptcp_pm.yaml
@@ -109,7 +109,6 @@ attribute-sets:
       -
         name: port
         type: u16
-        byte-order: big-endian
       -
         name: flags
         type: u32
-- 
2.45.2

Re: [PATCH net] netlink: specs: mptcp: fix port endianness
Posted by Matthieu Baerts 3 months, 1 week ago
Hi Asbjørn,

On 11/09/2024 11:10, Asbjørn Sloth Tønnesen wrote:
> The MPTCP port attribute is in host endianness, but was documented
> as big-endian in the ynl specification.
> 
> Below are two examples from net/mptcp/pm_netlink.c showing that the
> attribute is converted to/from host endianness for use with netlink.
> 
> Import from netlink:
>   addr->port = htons(nla_get_u16(tb[MPTCP_PM_ADDR_ATTR_PORT]))
> 
> Export to netlink:
>   nla_put_u16(skb, MPTCP_PM_ADDR_ATTR_PORT, ntohs(addr->port))
> 
> Where addr->port is defined as __be16.
> 
> No functional change intended.

Good catch. Indeed, this port attribute is supposed to be in host
endianness.

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

This patch can be applied in netdev directly.

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

Re: [PATCH net] netlink: specs: mptcp: fix port endianness
Posted by Davide Caratti 3 months, 1 week ago
hello Asbjørn,

On Wed, Sep 11, 2024 at 09:10:02AM +0000, Asbjørn Sloth Tønnesen wrote:
> The MPTCP port attribute is in host endianness, but was documented
> as big-endian in the ynl specification.
> 
> Below are two examples from net/mptcp/pm_netlink.c showing that the
> attribute is converted to/from host endianness for use with netlink.
> 
> Import from netlink:
>   addr->port = htons(nla_get_u16(tb[MPTCP_PM_ADDR_ATTR_PORT]))
> 
> Export to netlink:
>   nla_put_u16(skb, MPTCP_PM_ADDR_ATTR_PORT, ntohs(addr->port))
> 
> Where addr->port is defined as __be16.
> 
> No functional change intended.
> 
> Fixes: bc8aeb2045e2 ("Documentation: netlink: add a YAML spec for mptcp")
> Signed-off-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>

not sure why we are doing addresses in network byte order
and ports in host byte order for endpoints. But it's like this since the
very beginning, so this host ordering in spec is correct.

Reviewed-by: Davide Caratti <dcaratti@redhat.com>
Re: [PATCH net] netlink: specs: mptcp: fix port endianness
Posted by MPTCP CI 3 months, 1 week ago
Hi Asbjørn,

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/10808837330

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


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)