[PATCH mptcp-net v2 3/3] netlink: specs: mptcp: fix missing doc

Matthieu Baerts (NGI0) posted 3 patches 5 days, 16 hours ago
[PATCH mptcp-net v2 3/3] netlink: specs: mptcp: fix missing doc
Posted by Matthieu Baerts (NGI0) 5 days, 16 hours ago
Two operations didn't have a small description. It looks like something
that has been missed in the original commit introducing this file.

Replace the two 'todo' by a small and simple description: Create/Destroy
subflow.

Fixes: bc8aeb2045e2 ("Documentation: netlink: add a YAML spec for mptcp")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 Documentation/netlink/specs/mptcp_pm.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml
index 59087a23056510dfb939b702e231b6e97ae042c7..a9519ae77d38ce2af58de77c059ef22cd2e3f563 100644
--- a/Documentation/netlink/specs/mptcp_pm.yaml
+++ b/Documentation/netlink/specs/mptcp_pm.yaml
@@ -375,7 +375,7 @@ operations:
            - loc-id
     -
       name: subflow-create
-      doc: todo
+      doc: Create subflow
       attribute-set: attr
       dont-validate: [ strict ]
       flags: [ uns-admin-perm ]
@@ -387,7 +387,7 @@ operations:
             - addr-remote
     -
       name: subflow-destroy
-      doc: todo
+      doc: Destroy subflow
       attribute-set: attr
       dont-validate: [ strict ]
       flags: [ uns-admin-perm ]

-- 
2.45.2
Re: [PATCH mptcp-net v2 3/3] netlink: specs: mptcp: fix missing doc
Posted by Geliang Tang 4 days, 20 hours ago
Hi Matt,

On Mon, 2024-12-16 at 11:39 +0100, Matthieu Baerts (NGI0) wrote:
> Two operations didn't have a small description. It looks like
> something
> that has been missed in the original commit introducing this file.
> 
> Replace the two 'todo' by a small and simple description:
> Create/Destroy
> subflow.
> 
> Fixes: bc8aeb2045e2 ("Documentation: netlink: add a YAML spec for
> mptcp")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  Documentation/netlink/specs/mptcp_pm.yaml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/netlink/specs/mptcp_pm.yaml
> b/Documentation/netlink/specs/mptcp_pm.yaml
> index
> 59087a23056510dfb939b702e231b6e97ae042c7..a9519ae77d38ce2af58de77c059
> ef22cd2e3f563 100644
> --- a/Documentation/netlink/specs/mptcp_pm.yaml
> +++ b/Documentation/netlink/specs/mptcp_pm.yaml
> @@ -375,7 +375,7 @@ operations:
>             - loc-id
>      -
>        name: subflow-create
> -      doc: todo
> +      doc: Create subflow
>        attribute-set: attr
>        dont-validate: [ strict ]
>        flags: [ uns-admin-perm ]
> @@ -387,7 +387,7 @@ operations:
>              - addr-remote
>      -
>        name: subflow-destroy
> -      doc: todo
> +      doc: Destroy subflow
>        attribute-set: attr
>        dont-validate: [ strict ]
>        flags: [ uns-admin-perm ]

This patch looks good! Thanks.

I think other three doc needs to be updated too:

"flush addresses", "announce new sf", "announce removal"

->

"Flush addresses", "Announce new address (?)", "Remove address (?)"

WDYT?

-Geliang

Re: [PATCH mptcp-net v2 3/3] netlink: specs: mptcp: fix missing doc
Posted by Matthieu Baerts 4 days, 17 hours ago
Hi Geliang,

On 17/12/2024 07:37, Geliang Tang wrote:
> Hi Matt,
> 
> On Mon, 2024-12-16 at 11:39 +0100, Matthieu Baerts (NGI0) wrote:
>> Two operations didn't have a small description. It looks like
>> something
>> that has been missed in the original commit introducing this file.
>>
>> Replace the two 'todo' by a small and simple description:
>> Create/Destroy
>> subflow.
>>
>> Fixes: bc8aeb2045e2 ("Documentation: netlink: add a YAML spec for
>> mptcp")
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  Documentation/netlink/specs/mptcp_pm.yaml | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/netlink/specs/mptcp_pm.yaml
>> b/Documentation/netlink/specs/mptcp_pm.yaml
>> index
>> 59087a23056510dfb939b702e231b6e97ae042c7..a9519ae77d38ce2af58de77c059
>> ef22cd2e3f563 100644
>> --- a/Documentation/netlink/specs/mptcp_pm.yaml
>> +++ b/Documentation/netlink/specs/mptcp_pm.yaml
>> @@ -375,7 +375,7 @@ operations:
>>             - loc-id
>>      -
>>        name: subflow-create
>> -      doc: todo
>> +      doc: Create subflow
>>        attribute-set: attr
>>        dont-validate: [ strict ]
>>        flags: [ uns-admin-perm ]
>> @@ -387,7 +387,7 @@ operations:
>>              - addr-remote
>>      -
>>        name: subflow-destroy
>> -      doc: todo
>> +      doc: Destroy subflow
>>        attribute-set: attr
>>        dont-validate: [ strict ]
>>        flags: [ uns-admin-perm ]
> 
> This patch looks good! Thanks.
> 
> I think other three doc needs to be updated too:
> 
> "flush addresses", "announce new sf", "announce removal"
> 
> ->
> 
> "Flush addresses", "Announce new address (?)", "Remove address (?)"

I initially thought that it was not really a fix, but yes, why not,
while at it, I guess it can pass.

Regarding "Remove address", I think "Announce removal" is better: the
goal is to "announce" the address is no longer available. That's also
the term that is used in the RFC:

  https://datatracker.ietf.org/doc/html/rfc8684#name-remove-address

Is it OK if I do the modifications when applying the patches?

> diff --git a/Documentation/netlink/specs/mptcp_pm.yaml b/Documentation/netlink/specs/mptcp_pm.yaml
> index dc190bf838fe..c260a972a394 100644
> --- a/Documentation/netlink/specs/mptcp_pm.yaml
> +++ b/Documentation/netlink/specs/mptcp_pm.yaml
> @@ -306,8 +306,8 @@ operations:
>           attributes:
>             - addr
>      -
> -      name:  flush-addrs
> -      doc: flush addresses
> +      name: flush-addrs
> +      doc: Flush addresses
>        attribute-set: endpoint
>        dont-validate: [ strict ]
>        flags: [ uns-admin-perm ]
> @@ -351,7 +351,7 @@ operations:
>              - addr-remote
>      -
>        name: announce
> -      doc: announce new sf
> +      doc: Announce new address
>        attribute-set: attr
>        dont-validate: [ strict ]
>        flags: [ uns-admin-perm ]
> @@ -362,7 +362,7 @@ operations:
>              - token
>      -
>        name: remove
> -      doc: announce removal
> +      doc: Announce removal
>        attribute-set: attr
>        dont-validate: [ strict ]
>        flags: [ uns-admin-perm ]

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

Re: [PATCH mptcp-net v2 3/3] netlink: specs: mptcp: fix missing doc
Posted by Geliang Tang 4 days, 2 hours ago
On Tue, 2024-12-17 at 11:22 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 17/12/2024 07:37, Geliang Tang wrote:
> > Hi Matt,
> > 
> > On Mon, 2024-12-16 at 11:39 +0100, Matthieu Baerts (NGI0) wrote:
> > > Two operations didn't have a small description. It looks like
> > > something
> > > that has been missed in the original commit introducing this
> > > file.
> > > 
> > > Replace the two 'todo' by a small and simple description:
> > > Create/Destroy
> > > subflow.
> > > 
> > > Fixes: bc8aeb2045e2 ("Documentation: netlink: add a YAML spec for
> > > mptcp")
> > > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> > > ---
> > >  Documentation/netlink/specs/mptcp_pm.yaml | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/Documentation/netlink/specs/mptcp_pm.yaml
> > > b/Documentation/netlink/specs/mptcp_pm.yaml
> > > index
> > > 59087a23056510dfb939b702e231b6e97ae042c7..a9519ae77d38ce2af58de77
> > > c059
> > > ef22cd2e3f563 100644
> > > --- a/Documentation/netlink/specs/mptcp_pm.yaml
> > > +++ b/Documentation/netlink/specs/mptcp_pm.yaml
> > > @@ -375,7 +375,7 @@ operations:
> > >             - loc-id
> > >      -
> > >        name: subflow-create
> > > -      doc: todo
> > > +      doc: Create subflow
> > >        attribute-set: attr
> > >        dont-validate: [ strict ]
> > >        flags: [ uns-admin-perm ]
> > > @@ -387,7 +387,7 @@ operations:
> > >              - addr-remote
> > >      -
> > >        name: subflow-destroy
> > > -      doc: todo
> > > +      doc: Destroy subflow
> > >        attribute-set: attr
> > >        dont-validate: [ strict ]
> > >        flags: [ uns-admin-perm ]
> > 
> > This patch looks good! Thanks.
> > 
> > I think other three doc needs to be updated too:
> > 
> > "flush addresses", "announce new sf", "announce removal"
> > 
> > ->
> > 
> > "Flush addresses", "Announce new address (?)", "Remove address (?)"
> 
> I initially thought that it was not really a fix, but yes, why not,
> while at it, I guess it can pass.
> 
> Regarding "Remove address", I think "Announce removal" is better: the
> goal is to "announce" the address is no longer available. That's also
> the term that is used in the RFC:
> 
>   https://datatracker.ietf.org/doc/html/rfc8684#name-remove-address
> 
> Is it OK if I do the modifications when applying the patches?

Sure, I'll add my reviewed-by tag in the cover letter.

> 
> > diff --git a/Documentation/netlink/specs/mptcp_pm.yaml
> > b/Documentation/netlink/specs/mptcp_pm.yaml
> > index dc190bf838fe..c260a972a394 100644
> > --- a/Documentation/netlink/specs/mptcp_pm.yaml
> > +++ b/Documentation/netlink/specs/mptcp_pm.yaml
> > @@ -306,8 +306,8 @@ operations:
> >           attributes:
> >             - addr
> >      -
> > -      name:  flush-addrs
> > -      doc: flush addresses
> > +      name: flush-addrs
> > +      doc: Flush addresses
> >        attribute-set: endpoint
> >        dont-validate: [ strict ]
> >        flags: [ uns-admin-perm ]
> > @@ -351,7 +351,7 @@ operations:
> >              - addr-remote
> >      -
> >        name: announce
> > -      doc: announce new sf
> > +      doc: Announce new address
> >        attribute-set: attr
> >        dont-validate: [ strict ]
> >        flags: [ uns-admin-perm ]
> > @@ -362,7 +362,7 @@ operations:
> >              - token
> >      -
> >        name: remove
> > -      doc: announce removal
> > +      doc: Announce removal
> >        attribute-set: attr
> >        dont-validate: [ strict ]
> >        flags: [ uns-admin-perm ]
> 
> Cheers,
> Matt