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

[Mahesh Jethanandani <[e-mail]>]

Hi Pierre,

See inline with [mj]

> On Nov 9, 2021, at 4:11 AM, Pierre Francois <[e-mail]> wrote:
> 
> Hello Tianran, Mahesh, 
> 
> Thank you for the comprehensive review. 
> 
> Inline PFR>
> 
> ----- Mail original -----
>> De: "Tianran Zhou" <[e-mail]>
>> À: "Mahesh Jethanandani" <[e-mail]>, [e-mail]
>> Cc: "Netconf" <[e-mail]>
>> Envoyé: Lundi 8 Novembre 2021 13:36:50
>> Objet: RE: Review comments on draft-ietf-netconf-udp-notif
> 
>> Hi Mahesh,
>> 
>> Thanks very much for your detailed review and comments.
>> Please see in line for my reply.
>> 
>> Cheers,
>> Tianran
>> 
>> From: Mahesh Jethanandani [mailto:[e-mail]]
>> Sent: Monday, November 8, 2021 6:21 AM
>> To: [e-mail]
>> Cc: Netconf <[e-mail]>
>> Subject: Review comments on draft-ietf-netconf-udp-notif
>> 
>> 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/
>> ZTR> I agree with your suggestions.
>> 
>> 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.
>> 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.
> 
> PFR> I'm afraid I don't understand the comment.

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

> 
>> 
>> *   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.
> 
> 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.

Ok.

Thanks

> 
> Cheers, 
> 
> 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
>> [e-mail] <mailto:[e-mail]><mailto:[e-mail] <mailto:[e-mail]>>
>> [e-mail] <mailto:[e-mail]><mailto:[e-mail] <mailto:[e-mail]>>:120:
>> warning: node "ietf-subscribed-notifications::transport" is not found in
>> "ietf-subscribed-notifications::subscriptions”
>> $ yanglint [e-mail] <mailto:[e-mail]><mailto:[e-mail] <mailto:[e-mail]>>
>> 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
>> [e-mail] <mailto:[e-mail]><mailto:[e-mail] <mailto:[e-mail]>>

Mahesh Jethanandani
[e-mail]