Subject: [OPSAWG] Francesca Palombini's Discuss on draft-ietf-opsawg-l3sm-l3nm-16: (with DISCUSS and COMMENT)

From: Francesca Palombini via Datatracker <[email-masked]>
Date: Mon, 04 Oct 2021 03:51:30 -0700

Francesca Palombini has entered the following ballot position for
draft-ietf-opsawg-l3sm-l3nm-16: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/blog/handling-iesg-ballot-positions/
for more information about how to handle DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-opsawg-l3sm-l3nm/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Thank you for the work on this document, and apologies for the delayed review.
I have one DISCUSS point, a couple of comments.

As noted in https://www.ietf.org/blog/handling-iesg-ballot-positions/, a
DISCUSS ballot is a request to have a discussion; I really think that the
document would be improved with a change here, but can be convinced otherwise.

I have divided comments into "minor" (including the questions) and "nits".
Neither require replies strictly speaking, please feel free to address as you
see fit. I will appreciate answers to my questions, to improve my
understanding. If any clarification comes out of it, I hope it will help
improve the document.

Francesca

1. -----

                       leaf holdtime {
                         type uint32;
                         units "msec";

FP: This might be me not finding the right reference (or little knowledge of
YANG), but I was wondering if "msec" was defined somewhere as a unit (note that
the description does not mention that the unit is milliseconds either).

While doing my due diligence to see if I missed or misunderstood something, I
researched the RFCs mentioned in the beginning of the YANG module:

   This module uses types defined in [RFC6991] and [RFC8343].  It also
   uses groupings defined in [RFC8519], [RFC8177], and [RFC8294].

And found no use of the "msec" unit. A quick google search shows that RFC 8299
uses it, so there is precedence for it, but I couldn't find its definition from
that document either. All the other leaves use "milliseconds" (which is defined
in RFC 8294), so my preference would be to have consistency, if "msec" was
defined and I just missed it.

(Note that a similar remark could be made for "bps" used, which does not appear
in the description text, and is also used in RFC 8466, however there is no
issue about consistency there).

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

## Minor

2. -----

   'status':  Indicates the status of the OSPF routing instance.

FP: Most likely a copy-paste leftover in section 7.6.3.4 should be IS-IS
instead of OSPF.

3. -----

Section 7.6.3.5, re timers

FP: shouldn't the units be explicitly stated in the timers description text, or
are they defined somewhere else? Actually, I can see the unit is specified
later on in the YANG module - so my suggestion is to add some simple text in
7.6.3.5 to explicitly say that the timers are in seconds.

4. -----

                       leaf required-min-rx-interval {

FP: I see that RFC 5880 does not specify a default value for this; is there
really no default that can be specified here?

## Nits

5. -----

                             "It is included only when enryption
                              is enabled.";
                         }

FP: typo s/enryption/encryption

---

From: <[email-masked]>
Date: Mon, 4 Oct 2021 11:34:05 +0000

Hi Francesca, 

Thank you for the review. 

Please see inline.

Cheers,
Med

> -----Message d'origine-----
> De : Francesca Palombini via Datatracker <[email-masked]>
> Envoyé : lundi 4 octobre 2021 12:52
> À : The IESG <[email-masked]>
> Cc : [email-masked]; [email-masked];
> [email-masked]; [email-masked]; [email-masked]
> Objet : Francesca Palombini's Discuss on draft-ietf-opsawg-l3sm-l3nm-16:
> (with DISCUSS and COMMENT)
> 

[Med] Cool to have this reminder in the preamble :-)  

> Neither require replies strictly speaking, please feel free to address as
> you see fit. I will appreciate answers to my questions, to improve my
> FP: This might be me not finding the right reference (or little knowledge
> of YANG), but I was wondering if "msec" was defined somewhere as a unit

[Med] There is no formal list of units. RFC7520 does not make any constraint on the string used in the "units" statement:

   The "units" statement, which is optional, takes as an argument a
   string that contains a textual definition of the units associated
   with the type. 

Because I interpret your comment as "msec" is confusing as a unit and for consistency with 'microseconds" already in use in the module, I made this change: s/msec/milliseconds

> (note that the description does not mention that the unit is milliseconds
> either).

[Med] The description of the YANG module does not include such mention because this is redundant with the "units" statement. However, I updated Section 7.6.4 to mention that the timer is expressed in milliseconds. Thanks.

> 
> While doing my due diligence to see if I missed or misunderstood
> something, I researched the RFCs mentioned in the beginning of the YANG
> module:
> 
> And found no use of the "msec" unit. A quick google search shows that RFC
> 8299 uses it

[Med] Bingo! We are actually inheriting that leaf (including the unit) from the L3VPN service module. 

, so there is precedence for it, but I couldn't find its
> definition from that document either. All the other leaves use
> "milliseconds" (which is defined in RFC 8294), so my preference would be
> to have consistency, if "msec" was defined and I just missed it.

[Med] I think this is fixed with the proposed change above. 

> 
> (Note that a similar remark could be made for "bps" used, which does not
> appear in the description text, and is also used in RFC 8466, however
> there is no issue about consistency there).

[Med] Indeed. Made this change to avoid any confusion: 

OLD:
 Indicates the inbound bandwidth of the connection

NEW:
 Indicates, in bytes per second (bps), the inbound bandwidth of the connection

And a similar for the outbound bw.

[Med] Fixed. Thank you for catching this. 

> FP: shouldn't the units be explicitly stated in the timers description
> text, or are they defined somewhere else? Actually, I can see the unit is
> specified later on in the YANG module - so my suggestion is to add some
> simple text in

[Med] Agree. Fixed. 

> FP: I see that RFC 5880 does not specify a default value for this; is
> there really no default that can be specified here?

[Med] I thought we had already one! Updated the text to use 1000000 as default to align with the BFD device module (draft-ietf-bfd-yang). Thank you for pointing to this.

> FP: typo s/enryption/encryption

[Med] Fixed. FYI, all the changes to address your review can be tracked at: https://github.com/IETF-OPSAWG-WG/lxnm/commit/b8904b4cfbd545f2128848207898f53dffc9e83d 

Please let me know if any further change is needed. Many thanks again for the review. 

_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

---

From: Francesca Palombini <[email-masked]>
Date: Mon, 4 Oct 2021 11:51:50 +0000

Hi Med!

Amazing reaction time, as always :) Comments inline, but it all looks good! As soon as you submit the new version I’ll remove my discuss.

Thanks,
Francesca

From: [email-masked] <[email-masked]>
Date: Monday, 4 October 2021 at 13:34
To: Francesca Palombini <[email-masked]>, The IESG <[email-masked]>
Cc: [email-masked] <[email-masked]>, [email-masked] <[email-masked]>, [email-masked] <[email-masked]>, [email-masked] <[email-masked]>
Subject: RE: Francesca Palombini's Discuss on draft-ietf-opsawg-l3sm-l3nm-16: (with DISCUSS and COMMENT)
Hi Francesca,

Please see inline.

Cheers,
Med

[Med] Cool to have this reminder in the preamble :-)
FP: Right? All kudos for this text goes to Warren.

FP: Thank you for the explanation, I was missing that there was no formal list of units. And as you saw, given that, consistency is exactly what I was hoping for. Thank you for making this change!

FP: Indeed.

OLD:

NEW:

FP: Thank you! I see that was not strictly necessary, but the addition is appreciated anyway.

[Med] Fixed. Thank you for catching this.
FP: Happy to have helped!

[Med] Agree. Fixed.

FP: Great, thank you.

> FP: typo s/enryption/encryption

[Med] Fixed. FYI, all the changes to address your review can be tracked at: https://protect2.fireeye.com/v1/url?k=4aa8d067-1533e965-4aa890fc-869a14f4b08c-2d8a4280e1e2daab&q=1&e=727a2a92-9a8f-4ac9-a806-37e28ad13c45&u=https%3A%2F%2Fgithub.com%2FIETF-OPSAWG-WG%2Flxnm%2Fcommit%2Fb8904b4cfbd545f2128848207898f53dffc9e83d

---

From: Carsten Bormann <[email-masked]>
Date: Mon, 4 Oct 2021 17:49:53 +0200

On 2021-10-04, at 13:34, [email-masked] wrote:
> bytes per second (bps),

Whoa.

I know that the IETF doesn’t usually care about precision in these things, but “bps” is kitchen slang for “bit/s”, so this is very confusing.

(Given that we have done the work in RFC 8428 and 8798, I’d rather see that we use it, instead of creating more confusion at each further step.
We do have ms and B/s in RFC 8798, because people using SenML asked for that.)

Grüße, Carsten

---

From: <[email-masked]>
Date: Tue, 5 Oct 2021 06:02:25 +0000

Hi Carsten,

I confirm that what I meant is "bits per second" to align with rfc8299#6.12.1.

I'm actually for more explicit units similar to what we are using in another active spec: 

==
      enum bit-ps {
        value 2;
        description
          "Bits per Second (bit/s).";
      }
      enum byte-ps {
        value 3;
        description
          "Bytes per second (Byte/s).";
      }
==

However, we are in a territory where we are trying to map as much to the above service model and hence use the same labels for the units.

FWIW, RFC8466 used to have the following:

=
                  leaf pbs {
                    type uint64;
                    units "bps";
                    description
                      "Peak Burst Size.  It is measured in bytes per
                       second.";
                  }
=

...which is weird. This is why we don't blindly inherit that draft-ietf-opsawg-l2nm and went for the following: 

                      leaf pbs {
                        type uint64;
                        units "bytes per second";
                        description
                          "Peak Burst Size.";
                      }

Cheers,
Med

> De : Carsten Bormann <[email-masked]>
> Envoyé : lundi 4 octobre 2021 17:50
> À : BOUCADAIR Mohamed INNOV/NET <[email-masked]>
> Cc : Francesca Palombini <[email-masked]>; The IESG
> <[email-masked]>; [email-masked]; opsawg-
> [email-masked]; [email-masked]; [email-masked]
> Objet : Re: [OPSAWG] Francesca Palombini's Discuss on draft-ietf-opsawg-
> l3sm-l3nm-16: (with DISCUSS and COMMENT)
> 
> I know that the IETF doesn’t usually care about precision in these things,
> but “bps” is kitchen slang for “bit/s”, so this is very confusing.
> 
> (Given that we have done the work in RFC 8428 and 8798, I’d rather see
> that we use it, instead of creating more confusion at each further step.

---

From: Carsten Bormann <[email-masked]>
Date: Tue, 5 Oct 2021 09:05:21 +0200

Hi Med,

> I confirm that what I meant is "bits per second" to align with rfc8299#6.12.1.

Ah.

> I'm actually for more explicit units similar to what we are using in another active spec: 

As long as there is this confusion in YANG units, that is the only way that makes sense.

>          "Bytes per second (Byte/s).";

Maybe use the actual ISO/IEC 80000 notation here: B/s.
(For those that don’t know how ISO/IEC 80000 allocates “B” for byte, the legend “Bytes per second” is unambiguous.)

> ...which is weird.

This is really errata land, as “bps” is used as the kitchen slang for “bit/s” in all other cases (along with “mbps” for Mbit/s, shudder).

> This is why we don't blindly inherit that draft-ietf-opsawg-l2nm and went for the following: 
> 
>                      leaf pbs {
>                        type uint64;

I think this would also benefit from “Bytes per Second (B/s)”.

Grüße, Carsten

---

From: Francesca Palombini <[email-masked]>
Date: Tue, 5 Oct 2021 14:11:53 +0000

Thanks Carsten for noticing this! I did overlook completely that bps was being used as bytes per seconds… Thanks Med for clarifying in this document and for opening errata https://www.rfc-editor.org/errata/eid6703 . The OPS ADs are on it, I am sure.

Francesca

From: Carsten Bormann <[email-masked]>
Date: Tuesday, 5 October 2021 at 09:05
To: [email-masked] <[email-masked]>
Cc: Francesca Palombini <[email-masked]>, The IESG <[email-masked]>, [email-masked] <[email-masked]>, [email-masked] <[email-masked]>, [email-masked] <[email-masked]>, [email-masked] <[email-masked]>
Subject: Re: [OPSAWG] Francesca Palombini's Discuss on draft-ietf-opsawg-l3sm-l3nm-16: (with DISCUSS and COMMENT)
Hi Med,

Ah.

Grüße, Carsten

---

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Tue, 5 Oct 2021 14:56:08 +0000

Carsten & Med,

Thanks for raising this.  I agree with the errata, but this will need to be hold for doc update, because we cannot create a different revision of a YANG module through the errata process.

Thanks,
Rob

From: iesg <[email-masked]> On Behalf Of Francesca Palombini
Sent: 05 October 2021 15:12
To: Carsten Bormann <[email-masked]>; [email-masked]
Cc: [email-masked]; [email-masked]; [email-masked]; The IESG <[email-masked]>; [email-masked]
Subject: Re: [OPSAWG] Francesca Palombini's Discuss on draft-ietf-opsawg-l3sm-l3nm-16: (with DISCUSS and COMMENT)

Thanks Carsten for noticing this! I did overlook completely that bps was being used as bytes per seconds... Thanks Med for clarifying in this document and for opening errata https://www.rfc-editor.org/errata/eid6703 . The OPS ADs are on it, I am sure.

Francesca

From: Carsten Bormann <[email-masked]<mailto:[email-masked]>>
Date: Tuesday, 5 October 2021 at 09:05
To: [email-masked]<mailto:[email-masked]> <[email-masked]<mailto:[email-masked]>>
Cc: Francesca Palombini <[email-masked]<mailto:[email-masked]>>, The IESG <[email-masked]<mailto:[email-masked]>>, [email-masked]<mailto:[email-masked]> <[email-masked]<mailto:[email-masked]>>, [email-masked]<mailto:[email-masked]> <[email-masked]<mailto:[email-masked]>>, [email-masked]<mailto:[email-masked]> <[email-masked]<mailto:[email-masked]>>, [email-masked]<mailto:[email-masked]> <[email-masked]<mailto:[email-masked]>>
Subject: Re: [OPSAWG] Francesca Palombini's Discuss on draft-ietf-opsawg-l3sm-l3nm-16: (with DISCUSS and COMMENT)
Hi Med,

Ah.

Maybe use the actual ISO/IEC 80000 notation here: B/s.
(For those that don't know how ISO/IEC 80000 allocates "B" for byte, the legend "Bytes per second" is unambiguous.)

>                      "Peak Burst Size.  It is measured in bytes per

This is really errata land, as "bps" is used as the kitchen slang for "bit/s" in all other cases (along with "mbps" for Mbit/s, shudder).

>                        units "bytes per second";

I think this would also benefit from "Bytes per Second (B/s)".

Grüße, Carsten

>> De : Carsten Bormann <[email-masked]<mailto:[email-masked]>>
>> À : BOUCADAIR Mohamed INNOV/NET <[email-masked]<mailto:[email-masked]>>
>> Cc : Francesca Palombini <[email-masked]<mailto:[email-masked]>>; The IESG
>> <[email-masked]<mailto:[email-masked]>>; [email-masked]<mailto:[email-masked]>; opsawg-
>> [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>
>> On 2021-10-04, at 13:34, [email-masked]<mailto:[email-masked]> wrote:
>>>
>>> bytes per second (bps),
>>
>> Whoa.
>>
>> I know that the IETF doesn't usually care about precision in these things,
>> but "bps" is kitchen slang for "bit/s", so this is very confusing.
>>
>> (Given that we have done the work in RFC 8428 and 8798, I'd rather see