search

ietf-ccamp-wg / draft-ietf-ccamp-mw-topo-yang

Other
3 stars 1 forks source link

AD Comments #32

Open samans opened 6 months ago

samans commented 6 months ago

Éric's review (Thanks!) Responses included below marked with "response"

NITS (no need to reply or act)

Leafs vs. leaves

I am non-English speaker, but I think that the plural form of ‘leaf’ is ‘leaves’.

Response

Leafs is what is used in RFC 7950 and RFC 8407, so will keep using leafs (agree it is a bit weird)

Section 4 and configurtion

s/configurtion/configuration/

Response

agreed

S 6.3.2 json

Please use JSON in uppercase

Response

agreed

Behaviour vs. behavior

In US English, I think that the correct writing is ‘behavior’.

Response

agreed

E.g. comma

“E.g.” should be followed by a comma.

Response

Will discuss with the RFC editor. Agree in principle.

SVG diagrams

Several figures are quite complex and would benefit of adding a SVG rendering in addition to the ASCII art. This could be time consuming but there are tools to assist you, notable aasvg (which is really impressive and worth a try), see also https://authors.ietf.org/en/diagrams.

Response

SVG diagrams have been produced, will look to add the SVG to markdown or XML directly for non-txt based generation. Will look into Martin's aasvg tool.

COMMENTS (expecting either reply or a document update)

S 1 L0/L1

Please expand L0/L1at first use. Same for L2 (even if obvious)

Response

Layer 0 / Layer 1 (L0/L1) Layer 2 (L2) on first use

S 1 L2 Ethernet

Isn’t L2 Ethernet a pleonasm ?

Response

No it is not, Ethernet covers Layer 1 and Layer 2, so in this case we are specially addressing Layer 2 Ethernet.

S 1 where it e.g. could be used

I do not think that where it e.g. could be used is correct English, suggest using “could be used, e.g., ...;”

Response

In the numbered list in section 1, the text explains that they are examples. Therefore suggest the following edits:

  1.  Correlation between microwave radio links and the supported links
       on higher topology layers, (e.g., an L2 Ethernet topology).  This
       information can be used to understand how changes in the
       performance/status of a microwave radio link affect traffic on
       higher layers.

   2.  Propagation of relevant characteristics of a microwave radio
       link, such as bandwidth, to higher topology layers, where it
       could be used as a criterion when configuring and optimizing a
       path for a connection/service through the network end to end.

   3.  Optimization of the microwave radio link configurations on a
       network level, with the purpose to minimize overall
       interference and/or maximize the overall capacity provided by the
       links.

removing the e.g. from 2 and 3, and fixing the formatting in 1.

S 1.1

While this section title includes ‘definition’, it actually contains only acronyms expansions. Suggest to either define the terms or change the section title.

Response

Change the title of the section to Abbreviations and fix the wording in the body of the section (change acronym to abbreviation)

S 2

If the normative BCP14 uppercase words are not used in the I-D (which is the case AFAICT), then there is no need to include the BCP14 template (and the associated references). Suggest removing this section.

Response

We will remove the section. Clean up usage of normative language to remove confusion.

Prefixes in Data Node Names

Should there be a section/table like often found in other YANG models RFCs ? E.g., https://datatracker.ietf.org/doc/html/rfc8795#section-1.3

Response

add the following (with a table of imported module prefixes)

1.3 Prefix in Data Node Names
In this document, names of data nodes and other data model objects are prefixed using the standard prefix associated with the corresponding YANG imported modules, as shown in the following table.

S 3.1 is dry

Most of the RFC explain the YANG tree notably with basic explanations and relationships among other leaves.

Response

It is common practice to show the tree, then have the explanation after. So no change is suggested.

S 3.1 leaves names

It seems that a recommended practice in YANG modules is to avoid repeating the module/parent name in the leaves names. E.g., s/mw-bandwidth/bandwidth/

Response

In this and other technology-specific internet drafts that augment te-topology, the "technology" is included on the containers and attributes related to the augmentation. See the Ethernet Client Topology and Packet Topology drafts. This is for easy of readability when there are several technologies included in one topology. Looking for a consistent approach for all augmentations of te-topology.

Repetitive text about L1 not being L3

The I-D contains some repetitive text such as cannot be used to perform cross-connection or switching of the traffic to create network connectivity across multiple microwave, which is also somehow obvious. Suggest to at least avoid the repetition.

Response

Change to...

   Since microwave is a point-to-point radio technology, a majority of the leafs in the Data Model for Traffic
   Engineering (TE) Topologies augmented by the microwave topology model are not applicable.

S 3.4 2nd paragraph

Perhaps due to my lack of expertise in radio links, but here are a list of points causing my eyebrows to raise:

Alternative: (This one is preferred by the authors) More specifically in the context of the microwave-specific augmentations of te-topology, admin-status and oper-status leafs (from te-topology) are only applicable to microwave carriers (in the mw-link tree) and not microwave radio links. Enable and disable of a radio link is instead done in the constituent carriers. Furthermore the status leafs related to mw-tp can be used when links are inter-domain and when the status of only one side of link is known, but since microwave is a point-to-point technology where both ends normally belong to the same domain it is not expected to be applicable in normal cases. 



- `are recommended`: am I correct that this is not the normative “RECOMMENDED” ?

### Response
remove normative language

- `recommended to be reported for links only`: why not for carriers as well? It may well be the case for currently deployed products, but 1/ this may change 2/ this justification could be added in the text.
### Response
See rewrite of paragraph above.

- `Enable and disable of a radio link is instead done in the constituent carriers`: same comment as above. I tend to see a radio link as a ‘LACP LAG group’ with the constituent ‘Ethernet links’ that can be individually monitored/configured.
### Response
See rewrite of paragraph above.

## S 3.5 missing revision date in the module

Normally, in the YANG module name, the revision date is included.

### Response
Will fix

## S 3.5 missing references

Normally, a YANG module add text about references before the actual YANG module(s).

### Response
Will fix

## S3.5 microwave-bandwidth

Should the unit be “bits/seconds” rather than `bits/seconds`?
### Response
Will fix

## S A.1

What about using an IPv6 address rather than `192.0.2.3` for `ietf-te-topology:te-tp-id`? (I am a big fan of IPv6, feel free to ignore this suggestion)
### Response
Prefer to keep the IPv4 addresses for readability.

## S B

Are there informative references to the models cited in `interface reference topology (ifref) and bandwidth-availability-topology (bwa) models` ?
### Response
Will add informative references.
evyncke commented 6 months ago

When can we expect a revised I-D with all the changes ?

Once done, and as I agree with all responses (thank you), then I am requesting the IETF last call (i.e., the next step).

Thank you !