Subject: [netconf] Review comments on draft-ietf-netconf-udp-notif

From: Mahesh Jethanandani <[email-masked]>
Date: Sun, 7 Nov 2021 17:21:01 -0500

Hi authors of draft-ietf-netconf-udp-notif,

Here are some of my comments on the -04 version of the draft.

Nits:

Please give the draft a more descriptive description for the short title rather than unyte-udp-notif. Something like "UDP-based Subscription”.

S/DTLS is layered next tot/DTLS is layered on top of/

Comments (major and minor):

In Section 1 there is an inherent assumption that the network processors/line cards have network access to the receiver. I think that requirement needs to be made explicit. Otherwise, routing all messages through an internal consolidator might be the only option. BTW, I believe the option to route notification messages through a internal consolidator should be kept as an option for UDP transport in case the line cards cannot route out of the box.

In Section 2, it is not clear who is responsible for sending the notification to the publisher that the receiver is ready to receive notifications.

In Section 3.1, TLS is specified as a transport protocol. I think you mean TCP or HTTP2 over TCP.

There is use of term ’notification message” and “Notification Messages”. What is the difference? If you are using a pre-defined term, can you add a definition and nomenclature section that brings in the reference. If you are defining a new term, can you add a definition in the document.

In Section 3.2, it is better for the Ver field to have an initial version of 1 instead of 0 to distinguish from a value that my not have been set.

Section 6.1. Not clear on the statement “Since UDP is an unreliable transport, with DTLS, an originator or a relay may not realize ..”. What has DTLS got to do with the unreliability of the transport protocol?

Section 6.2. Is the draft asking for assignment of a well known port for UDP based transport?

Section 8.

Has the model been compiled with pyang and yanglint? See compilation outputs below.

File name has a date that is different from the revision date in the model.

Please add references to the models that are imported into this model in the draft itself. They are there in the model, but not in the draft itself.

How about support for retransmission timeout value in the YANG model?

Change WG reference to  <https://datatracker.ietf.org/wg/netconf>

Remove the prefix “mailto:” It is great in a HTML document, but does not help in a YANG model.

Update Copyright year to 2021.

Indentations are off the model. Please fix.

If the grouping “target-receiver” is not going to reused anywhere else, please remove it and inline it within the augment statement.

$ pyang -f tree [email-masked]

[email-masked]:120: warning: node "ietf-subscribed-notifications::transport" is not found in "ietf-subscribed-notifications::subscriptions”

$ yanglint [email-masked]

libyang warn: Schema node "transport" for parent "/ietf-subscribed-notifications:subscriptions" not found; in expr "derived-from(../../../transport" with context node "/ietf-subscribed-notifications:subscriptions/subscription/receivers/receiver”.

Thanks

Mahesh Jethanandani

[email-masked]

---

From: Tianran Zhou <[email-masked]>
Date: Mon, 8 Nov 2021 12:36:50 +0000

Hi Mahesh,

Thanks very much for your detailed review and comments.
Please see in line for my reply.

Cheers,
Tianran

From: Mahesh Jethanandani [mailto:[email-masked]]
Sent: Monday, November 8, 2021 6:21 AM
To: [email-masked]
Cc: Netconf <[email-masked]>
Subject: Review comments on draft-ietf-netconf-udp-notif

Nits:

  *   Please give the draft a more descriptive description for the short title rather than unyte-udp-notif. Something like "UDP-based Subscription”.
  *   S/DTLS is layered next tot/DTLS is layered on top of/
ZTR> I agree with your suggestions.

  *   In Section 1 there is an inherent assumption that the network processors/line cards have network access to the receiver. I think that requirement needs to be made explicit. Otherwise, routing all messages through an internal consolidator might be the only option. BTW, I believe the option to route notification messages through a internal consolidator should be kept as an option for UDP transport in case the line cards cannot route out of the box.
ZTR>Yes, it’s a good suggestion. I will add this to the new revision.

  *   In Section 2, it is not clear who is responsible for sending the notification to the publisher that the receiver is ready to receive notifications.
ZTR> I am sorry, I do not remember the configure subscription clear. What’s defined generic in RFC8639 about how the publisher know the receiver? Or the publisher does not need to deal with this anyway?

  *   In Section 3.1, TLS is specified as a transport protocol. I think you mean TCP or HTTP2 over TCP.
ZTR> This I would like to leave it to Pierre.

  *   There is use of term ’notification message” and “Notification Messages”. What is the difference? If you are using a pre-defined term, can you add a definition and nomenclature section that brings in the reference. If you are defining a new term, can you add a definition in the document.
ZTR> In my opinion, no difference. I would like to add a ref from existing definition.

  *   In Section 3.2, it is better for the Ver field to have an initial version of 1 instead of 0 to distinguish from a value that my not have been set.
ZTR> Is this normal? I do not mind to do this.

  *   Section 6.1. Not clear on the statement “Since UDP is an unreliable transport, with DTLS, an originator or a relay may not realize ..”. What has DTLS got to do with the unreliability of the transport protocol?
ZTR> This I would like to leave it to Pierre.

  *   Section 6.2. Is the draft asking for assignment of a well known port for UDP based transport?
ZTR> Yes. This is what we request from IANA.

  *   Section 8.

     *   Has the model been compiled with pyang and yanglint? See compilation outputs below.
ZTR> I noticed this. But it seems from “ietf-subscribed-notifications”. Do you know what’s happened?
I also noticed the datatracker shows 4 errors. It seems the same errors exist in draft-ietf-netconf-https-notif. Could anyone help on this?

     *   File name has a date that is different from the revision date in the model.
     *   Please add references to the models that are imported into this model in the draft itself. They are there in the model, but not in the draft itself.
     *   How about support for retransmission timeout value in the YANG model?
ZTR> I do not understand this. What’s this suggestion?

     *   Change WG reference to  <https://datatracker.ietf.org/wg/netconf>
     *   Remove the prefix “mailto:” It is great in a HTML document, but does not help in a YANG model.
     *   Update Copyright year to 2021.
     *   Indentations are off the model. Please fix.
     *   If the grouping “target-receiver” is not going to reused anywhere else, please remove it and inline it within the augment statement.
ZTR> Others received. I will update. Many thanks!

$ pyang -f tree [email-masked]<mailto:[email-masked]>
[email-masked]<mailto:[email-masked]>:120: warning: node "ietf-subscribed-notifications::transport" is not found in "ietf-subscribed-notifications::subscriptions”
$ yanglint [email-masked]<mailto:[email-masked]>
libyang warn: Schema node "transport" for parent "/ietf-subscribed-notifications:subscriptions" not found; in expr "derived-from(../../../transport" with context node "/ietf-subscribed-notifications:subscriptions/subscription/receivers/receiver”.

Thanks

Mahesh Jethanandani
[email-masked]<mailto:[email-masked]>

---

From: Pierre Francois <[email-masked]>
Date: Tue, 9 Nov 2021 13:11:22 +0100 (CET)

Hello Tianran, Mahesh, 

Thank you for the comprehensive review. 

Inline PFR>

----- Mail original -----
> De: "Tianran Zhou" <[email-masked]>
> À: "Mahesh Jethanandani" <[email-masked]>, [email-masked]
> Cc: "Netconf" <[email-masked]>
> Envoyé: Lundi 8 Novembre 2021 13:36:50
> Objet: RE: Review comments on draft-ietf-netconf-udp-notif

>  *   Please give the draft a more descriptive description for the short title
>  rather than unyte-udp-notif. Something like "UDP-based Subscription”.
>  *   In Section 1 there is an inherent assumption that the network
>  processors/line cards have network access to the receiver. I think that
>  requirement needs to be made explicit. Otherwise, routing all messages through
>  an internal consolidator might be the only option. BTW, I believe the option to
>  route notification messages through a internal consolidator should be kept as
>  an option for UDP transport in case the line cards cannot route out of the box.
>  *   In Section 2, it is not clear who is responsible for sending the
>  notification to the publisher that the receiver is ready to receive
>  notifications.
> ZTR> I am sorry, I do not remember the configure subscription clear. What’s
> defined generic in RFC8639 about how the publisher know the receiver? Or the
> publisher does not need to deal with this anyway?
> 

PFR> I'm afraid I don't understand the comment.

> 
>  *   There is use of term ’notification message” and “Notification Messages”.
>  What is the difference? If you are using a pre-defined term, can you add a
>  definition and nomenclature section that brings in the reference. If you are
>  defining a new term, can you add a definition in the document.
>  *   In Section 3.2, it is better for the Ver field to have an initial version of
>  1 instead of 0 to distinguish from a value that my not have been set.
>  *   Section 6.1. Not clear on the statement “Since UDP is an unreliable
>  transport, with DTLS, an originator or a relay may not realize ..”. What has
>  DTLS got to do with the unreliability of the transport protocol?

PFR> What this means is that you don't get to know whether what you produced as an originator 
got consumed by the receiver, as you would using a connection oriented transport. Actually, 
it's obvious so we may want to remove this paragraph. The intent was to not have the reader think 
dtls changes things compared to basic udp transport.

Cheers, 

Pierre.

> I also noticed the datatracker shows 4 errors. It seems the same errors exist in
> draft-ietf-netconf-https-notif. Could anyone help on this?
> 
>     *   Please add references to the models that are imported into this model in the
>     draft itself. They are there in the model, but not in the draft itself.
>     *   If the grouping “target-receiver” is not going to reused anywhere else,
>     please remove it and inline it within the augment statement.
> $ pyang -f tree
> [email-masked]<mailto:[email-masked]>
> [email-masked]<mailto:[email-masked]>:120:
> warning: node "ietf-subscribed-notifications::transport" is not found in
> "ietf-subscribed-notifications::subscriptions”
> libyang warn: Schema node "transport" for parent
> "/ietf-subscribed-notifications:subscriptions" not found; in expr
> "derived-from(../../../transport" with context node
> "/ietf-subscribed-notifications:subscriptions/subscription/receivers/receiver”.
> 
> Thanks
> 
> Mahesh Jethanandani

---

From: Mahesh Jethanandani <[email-masked]>
Date: Tue, 9 Nov 2021 14:05:10 -0800

Hi Pierre,

See inline with [mj]

> On Nov 9, 2021, at 4:11 AM, Pierre Francois <[email-masked]> wrote:
> 
> Hello Tianran, Mahesh, 
> 

[mj] TLS is a cryptographic protocol, not a transport protocol. 

> dtls changes things compared to basic udp transport.

Ok.

Thanks

>> [email-masked] <mailto:[email-masked]><mailto:[email-masked] <mailto:[email-masked]>>
>> [email-masked] <mailto:[email-masked]><mailto:[email-masked] <mailto:[email-masked]>>:120:
>> $ yanglint [email-masked] <mailto:[email-masked]><mailto:[email-masked] <mailto:[email-masked]>>
>> [email-masked] <mailto:[email-masked]><mailto:[email-masked] <mailto:[email-masked]>>

Mahesh Jethanandani
[email-masked]

---

From: Pierre Francois <[email-masked]>
Date: Wed, 10 Nov 2021 08:33:52 +0100 (CET)

Mahesh, 

> [mj] TLS is a cryptographic protocol, not a transport protocol.

Haha ok right I'll make sure we are precise there. 

> Ok.

Great. 
Thanks again for the review, 

Cheers, 

Pierre.