[PATCH mptcp-next] Squash to: "net: mptcp: use policy generated by YAML spec"

[PATCH mptcp-next] Squash to: "net: mptcp: use policy generated by YAML spec"

[Davide Caratti]

as per Jakub's review, a newer YAML spec is going to validate addr6
with NLA_POLICY_EXACT_LEN(16)

Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 net/mptcp/mptcp_pm_gen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/mptcp_pm_gen.c b/net/mptcp/mptcp_pm_gen.c
index 673b5167af6b..a2325e70ddab 100644
--- a/net/mptcp/mptcp_pm_gen.c
+++ b/net/mptcp/mptcp_pm_gen.c
@@ -15,7 +15,7 @@ const struct nla_policy mptcp_pm_address_nl_policy[MPTCP_PM_ADDR_ATTR_IF_IDX + 1
 	[MPTCP_PM_ADDR_ATTR_FAMILY] = { .type = NLA_U16, },
 	[MPTCP_PM_ADDR_ATTR_ID] = { .type = NLA_U8, },
 	[MPTCP_PM_ADDR_ATTR_ADDR4] = { .type = NLA_U32, },
-	[MPTCP_PM_ADDR_ATTR_ADDR6] = { .len = 16, },
+	[MPTCP_PM_ADDR_ATTR_ADDR6] = NLA_POLICY_EXACT_LEN(16),
 	[MPTCP_PM_ADDR_ATTR_PORT] = { .type = NLA_U16, },
 	[MPTCP_PM_ADDR_ATTR_FLAGS] = { .type = NLA_U32, },
 	[MPTCP_PM_ADDR_ATTR_IF_IDX] = { .type = NLA_S32, },
-- 
2.41.0

Re: Squash to: "net: mptcp: use policy generated by YAML spec": Tests Results

[MPTCP CI]

Hi Davide,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5677417939861504
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5677417939861504/summary/summary.txt

- KVM Validation: debug (except selftest_mptcp_join):
  - Script error! ❓:
  - Task: https://cirrus-ci.com/task/5141953440907264
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5141953440907264/summary/summary.txt

- KVM Validation: normal (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5194619055505408
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5194619055505408/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
  - Task: https://cirrus-ci.com/task/5743465154215936
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5743465154215936/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/6a5c7957b315


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-debug

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 (Tessares)

Re: [PATCH mptcp-next] Squash to: "net: mptcp: use policy generated by YAML spec"

[Matthieu Baerts]

Hi Davide,

On 13/10/2023 11:45, Davide Caratti wrote:
> as per Jakub's review, a newer YAML spec is going to validate addr6
> with NLA_POLICY_EXACT_LEN(16)

I'm not sure to understand your modification: is it just a test patch to
check that the CI is OK with the modifications, or do you want to have
this patch included in the tree?

Because I guess we cannot just accept this patch, we also need a
modification of the YAML spec and the YNL tool. Then this file you
modified here will be re-generated, and we will get this new result, no?

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

Re: [PATCH mptcp-next] Squash to: "net: mptcp: use policy generated by YAML spec"

[Davide Caratti]

hello Matthieu,

On Fri, Oct 13, 2023 at 5:07 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>
> Hi Davide,
>

>
> I'm not sure to understand your modification: is it just a test patch to
> check that the CI is OK with the modifications, or do you want to have
> this patch included in the tree?

yes the goal was to check NLA_POLICY_EXACT_LEN() with CI (we already
have it, but better to double check on top of the whole series).
By the way , I see a failure but it doesn't seem real (I hope :) )

> Because I guess we cannot just accept this patch, we also need a
> modification of the YAML spec and the YNL tool. Then this file you
> modified here will be re-generated, and we will get this new result, no?

right, I have a series ready that also includes the parser of
'exact-len:' key in specfiles (and a de-uglified yaml spec, as per
Jakub's comment). If you are ok with it, I will send this series
directly to netdev - then, 'export' and 'net-next' will slightly
differ until the next merge. Let me know if that is ok for you,

thanks,
-- 
davide

Re: [PATCH mptcp-next] Squash to: "net: mptcp: use policy generated by YAML spec"

[Matthieu Baerts]

Hi Davide,

On 13/10/2023 17:53, Davide Caratti wrote:
> hello Matthieu,
> 
> On Fri, Oct 13, 2023 at 5:07 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>
>> Hi Davide,
>>
> 
>>
>> I'm not sure to understand your modification: is it just a test patch to
>> check that the CI is OK with the modifications, or do you want to have
>> this patch included in the tree?
> 
> yes the goal was to check NLA_POLICY_EXACT_LEN() with CI (we already
> have it, but better to double check on top of the whole series).
> By the way , I see a failure but it doesn't seem real (I hope :) )

Good!

>> Because I guess we cannot just accept this patch, we also need a
>> modification of the YAML spec and the YNL tool. Then this file you
>> modified here will be re-generated, and we will get this new result, no?
> 
> right, I have a series ready that also includes the parser of
> 'exact-len:' key in specfiles (and a de-uglified yaml spec, as per
> Jakub's comment). If you are ok with it, I will send this series
> directly to netdev - then, 'export' and 'net-next' will slightly
> differ until the next merge. Let me know if that is ok for you,

I would prefer to have squash-to patches, so we can easily review the
modifications and apply them in our tree before sending them to
'net-next' (or this last step in parallel). Also, by doing that, the
MPTCP CI will be able to validate the modifications.

If a different version is applied in net-next, the MPTCP tree will no
longer be in sync: it means the CI might not be able to automatically
sync with net-next and manual steps might be required.

But if the series had a lot of modifications and having squash-to
patches no longer make sense, we can work around that: reverting the
different patches one by one, resolving manually the conflicts, etc. I
can also try to apply a v2, but again, with manually steps :)

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

Re: [PATCH mptcp-next] Squash to: "net: mptcp: use policy generated by YAML spec"

[Davide Caratti]

hi  Matthieu,

On Mon, Oct 16, 2023 at 1:34 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>
[...]
>
> I would prefer to have squash-to patches, so we can easily review the
> modifications and apply them in our tree before sending them to
> 'net-next' (or this last step in parallel). Also, by doing that, the
> MPTCP CI will be able to validate the modifications.

that means generating 4 small squash-to patches:
1) the conversion to small-ops ( 1 line)
2) the spec file (~ 10 lines)
3) the uAPI header generation ( 1 line)
4) the generated nl_policy C file ( 1 line)

what about the additional patch for ynl tool? theoretically it should
be inserted at least before the last patch in the series (in practice,
the build is not broken even if you add it at the top. But I'm not
sure if the resync with net-next will have troubles, then).

thanks,
-- 
davide

Re: [PATCH mptcp-next] Squash to: "net: mptcp: use policy generated by YAML spec"

[Matthieu Baerts]

Hi Davide,

On 16/10/2023 14:25, Davide Caratti wrote:
> hi  Matthieu,
> 
> On Mon, Oct 16, 2023 at 1:34 PM Matthieu Baerts <matttbe@kernel.org> wrote:
>>
> [...]
>>
>> I would prefer to have squash-to patches, so we can easily review the
>> modifications and apply them in our tree before sending them to
>> 'net-next' (or this last step in parallel). Also, by doing that, the
>> MPTCP CI will be able to validate the modifications.
> 
> that means generating 4 small squash-to patches:
> 1) the conversion to small-ops ( 1 line)
> 2) the spec file (~ 10 lines)
> 3) the uAPI header generation ( 1 line)
> 4) the generated nl_policy C file ( 1 line)

Sounds good to me, except if it is difficult to generate.

> what about the additional patch for ynl tool? theoretically it should
> be inserted at least before the last patch in the series (in practice,
> the build is not broken even if you add it at the top. But I'm not
> sure if the resync with net-next will have troubles, then).

I can easily apply 'squash-to' patches and insert patches anywhere in
the tree.

So if we have a series with 4 'squash-to' patches + 1 to include in a
specific place, that's fine for me and the CI can validate them :)

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