| Message ID | 36fe14dbd444daf86a22cfa5fb50d590324e7b58.1697190267.git.dcaratti@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | [mptcp-next] Squash to: "net: mptcp: use policy generated by YAML spec" | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/build | success | Build and static analysis OK |
| matttbe/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
| matttbe/KVM_Validation__normal__except_selftest_mptcp_join_ | success | Success! ✅ |
| matttbe/KVM_Validation__debug__except_selftest_mptcp_join_ | fail | Script error! ❓ |
| matttbe/KVM_Validation__normal__only_selftest_mptcp_join_ | success | Success! ✅ |
| matttbe/KVM_Validation__debug__only_selftest_mptcp_join_ | warning | Unstable: 1 failed test(s): selftest_mptcp_join |
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
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
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,
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
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,
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
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, },
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(-)