Subject: Re: [i2rs] Benjamin Kaduk's No Objection on draft-ietf-i2rs-yang-l2-network-topology-14: (with COMMENT)

From: Qin Wu <[email-masked]>
Date: Thu, 9 Jul 2020 03:14:18 +0000

Hi, Sue:
-----邮件原件-----
发件人: Susan Hares [mailto:[email-masked]] 
发送时间: 2020年7月9日 1:25
收件人: 'Benjamin Kaduk' <[email-masked]>; 'The IESG' <[email-masked]>
抄送: [email-masked]; [email-masked]; [email-masked]
主题: RE: [i2rs] Benjamin Kaduk's No Objection on draft-ietf-i2rs-yang-l2-network-topology-14: (with COMMENT)

Ben: 

Following up on my earlier mail regarding the investigation comments: 

1)  System management MAC port - 

The 802.1Q-2018 is the latest full specification 

https://1.ieee802.org/yang-modules/
http://ieee802.org/1/files/public/YANGs/ieee802-dot1q-bridge.yang - provides the yang modules

in this module there is a container Bridge with the bridge configuration.
In this container there is a MAC address, but I do not find a clear specification of system-mac or the management mac.  I will ask the IEEE-IETF liaison group to provide me with the appropriate specification to validate the current and future handling of system macs. 

It may be an vendor specific augmentation to the IEEE model.  If so, these additions would still be needed by the L2 topology model which is looking to have an interoperable L2 view of the network. 

2) "other state" in the termination point 

The states of the termination port of "in  use", "blocking",  "down", and "others" concepts 802.1Q Yang models to a simplistic view of the port.  

The concepts of "other" as a mentioned below come from anticipation in 2016 of work for time sensitive network (TSN) task group.  The TSN work could cause the port as a logical termination point to have something besides "in use", "block frames", and "down".  Therefore, we added a state "other" to
allow the model to indicate one of the other concepts.   

The TSN specifications that lead us to this conclusion are the following:
P802F (yang module for Ether Types),  802.1Qcw (yang data models for schedule traffic, frame preemption, and per-stream filtering/policing), 802.1Qdj (configuration enhancements for TSN),  802.1ABcu (LLDP Yang Data model), and 802.1CBcv (Frame Replication and Elimination) model

In the future after the Yang models of the IEEE for TSN are specified and approved, a revision of this I2RS model could specify a simplification of
the TSN functioning into 1 or more specific states.   

Summary: 
Item 1 - management-address  for process that is 

    leaf-list management-address {
           type inet:ip-address;
           description
             "System management address.";
         }
         leaf sys-mac-address {
           type yang:mac-address;
           description
             "System MAC address.";
         }

I'm still trying to correlate this with the Bridge model in 802.1Q or 

[Qin]: Regarding sys-mac-address, it is device level mac address, the device is represented as l2 node in the L2 network topology model,
We also has port level mac address, port is represented as l2 termination point in the L2 network topology model,
Each device could have multiple port or multiple termination points, each port and termination will be assigned with one MAC address.

Item 2 - completed research and "other" is placeholder for Time sensitive network (TSN) work in 802.1. 
This correlates to detnet work. 

Cheerily, Sue 

-----Original Message-----
From: Susan Hares [mailto:[email-masked]]
Sent: Wednesday, July 8, 2020 11:14 AM
To: 'Benjamin Kaduk'; 'The IESG'
Cc: [email-masked]; [email-masked]; [email-masked]
Subject: RE: [i2rs] Benjamin Kaduk's No Objection on
draft-ietf-i2rs-yang-l2-network-topology-14: (with COMMENT)

Benjamin: 

I'm sorry I missed this email until today.    I do not think Qin has
answered your questions.   

Issues we believe are settled
==========================
1) VXLAN - value of zero.
   We believe Yang Doctors have indicated our method is correct. 
   Rob Wilton is the best judge among IESG for this issue. 

2) Network topology models and links - please see RFC 8345 

Issues which need investigation with IEEE/IETF liaison and industry use ===================================================
1) System management  IP address -  We believe that a single system management is the correct basic functionality. 
  
Additional system management ports would be an augmentation of a model by a vendor.  If this does not match the current hardware technology, the authors can change to indicate a list of multiple system management ports.
Advice is needed from switch vendors on the basic.   I have sent a request
to the yang doctors.   Rob Wilton can provide feedback for the IESG if I
do not get Yang Doctors response quickly. 

2) What "others" states might be added in the future?
  
 802.1 has been adding some other states to ports for ACTIVE-ACTIVE fail-over and for highly deterministic L2 paths for video (~IETF detnet).
I need to check when these "other states" have been defined by IEEE 802.1, and when these new "other" states might be deployed in new switch devices.
If the new states are in devices, we will need to add them to this model
following the 802.1 definitions.   As a shepherd, I had not check upon
these in the last 2 years.  I have concern about putting  these states in
a model unless we have implementations that will use these states.   The
normal way to add these states is with an augmentation for the model.   

Good catch - I should have re-examined the IEEE work and current technology in the last few weeks. 

3) Normative and informative choices - Authors will abide by IESG and IEEE recommendations The authors will abide by IESG and IEEE recommendations for the status of RFCs.  The following should be discussed by IESG and the IEEE

  a) Normative --> informative   RFC3688 and RFC7951 
  b) Informative--> normative: [IEEE802.1Qcp], RFC7348
  
Issues which need editorial changes:  (Qin SHOULD change/add] ================
1) QinQ definitions

The svlan-id and cvlan-id can should augment the structures with longer descriptions.  These descriptions should point to the IEEE longer
descriptions.   This change will need to be validated by yang doctors and
IETF-IEEE liaison person. 

2) change of text in security sections
[(a) "intended" to "intended" datastore and b) NETCONF access control model to NACM)]

Thanks for catching the nits in the security text.  

Thank you for your careful and thoughtful review,  Susan Hares 

-----Original Message-----
From: i2rs [mailto:[email-masked]] On Behalf Of Benjamin Kaduk via Datatracker
Sent: Monday, July 6, 2020 9:35 PM
To: The IESG
Cc: [email-masked]; [email-masked]; [email-masked]
Subject: [i2rs] Benjamin Kaduk's No Objection on
draft-ietf-i2rs-yang-l2-network-topology-14: (with COMMENT)

Benjamin Kaduk has entered the following ballot position for
draft-ietf-i2rs-yang-l2-network-topology-14: No Objection

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/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-i2rs-yang-l2-network-topology/

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

Why is the "management-address" for a l2-node an IP address?  Does that exclude any potential use of this data model for non-IP networks?

Section 3

   o  Links in the "ietf-network-topology" module are augmented as well
      with a set of Layer 2 parameters, allowing to associate a link
      with a name, a set of Layer 2 link attributes, and flags.

Interesting that names for links were not part of the core network-topology module.  Are there any potential issues if other ntework types also specify a link name and there is disagreement between them?

[Sue:  The links were part of the core network topology 

>From RFC 8345: 

   module: ietf-network-topology
     augment /nw:networks/nw:network:
       +--rw link* [link-id]
          +--rw link-id            link-id
          +--rw source
          |  +--rw source-node?   -> ../../../nw:node/node-id
          |  +--rw source-tp?     leafref
          +--rw destination
          |  +--rw dest-node?   -> ../../../nw:node/node-id
          |  +--rw dest-tp?     leafref
          +--rw supporting-link* [network-ref link-ref]
             +--rw network-ref
             |       -> ../../../nw:supporting-network/network-ref
             +--rw link-ref       leafref

End of Sue-2] 

Section 4

It reads a little oddly to use the flag-identity as the base type of a typedef before the identity itself is declared, but I am way out of my league here and defer to the YANG experts.

       description
         "VXLAN Network Identifier or VXLAN Segment ID.
          It allows up to 16 M VXLAN segments to coexist
          within the same administrative domain.

          The use of value '0' is implementation-specific.";

Is this intended as a nod to the use of 0 for the management VLAN?/ (I seem to recall this topic raising to some level of controversy in the discussions around draft-ietf-bfd-vxlan.)

[Sue2:  You are correct this raised controversy. 
  The value of "0" for VXLAN Segment ID  = 0 is valid for some implementations,
  And invalid for other implementations.  
  Our best understanding from Yang doctors and implementations 
   Is that this is the correct way to denote this to yang modules. 
   Implementations will need to handle a value of zero per box.] 

     /*

      * Features
      */

nit: there seems to be a spurious blank line here.

[Sue 2:   Thank you for noting the spurious line. 
                Qin - will catch that in the next revision. 

     grouping l2-node-attributes {
         [...]
         leaf sys-mac-address {
           type yang:mac-address;
           description
             "System MAC address.";
         }

Is there only one "System MAC address" per node?

[Sue: In my work (around 2008-2010)  with developing level 2 system support
    there was usually  one system MAC for input/output of   Management
issues.     
     If there is more than one, it is usually an augment for 
      A secondary network management processor. 
     Most devices (and Chip sets) indicate this is a different processor. 
    Qin may be able to check with hardware people to determine 
     If it is common to have more than one system MAC. 
    Do you have specific knowledge that changes this fact?
   If so, please propose it.
 End of sue:]
0

         leaf delay {
           type uint32;
           units "microseconds";
           description
             "Link delay in microseconds.";

I guess we don't expect to use this model for deep-space links :)
[Sue:  Yes - it is usually for local earth links.    Deep space links
would be an augmentation to the model. 
   (smile.) end Sue] 

       container l2-termination-point-attributes {
         description
           "Containing L2 termination point attributes.";
         leaf description {
           type string;
           description
             "Port description.";

Is a termination point always a "port", or should the description be modified?

[Sue:  According to the base model [RFC8345 the supporting termination point is considered a logical port.  Whether the physical configuration is a physical port, depends on the mapping for the model. ]

             list qinq {
               [...]
               leaf svlan-id {
                 type dot1q-types:vlanid;
                 description
                   "SVLAN ID.";
               }
               leaf cvlan-id {
                 type dot1q-types:vlanid;
                 description
                   "CVLAN ID.";

Could we perhaps expand "service" and "customer"?
[Sue:  We have tried to stay aligned with the 
   802.1Q  Qin Q specifications.  We'll add service and customer VLAN ID information, but  aligns with the IEEE use.  
  The brevity was to point to the 802.1 QinQ defintions. 
   Thank you for catching the fact the brief information was too brief.]

           }
           //case ethernet

RFC 6020 suggests that YANG comments are "C++-style", which would seem to indicate that the single-line comment could start on the same line as the closing brace.  This, in turn, would save some confusion since the block comments apply to the content after the comment, but these comments apply to the content before the comment.
(Also later on as well.)

[Sue:  If you use the recommended style of RFC6020, the tools chain does not handle it well. 
   The RFC processors for xml and nits blow up.    The style you have
below seems to make the 
    RFC processors and yang tools have less problems.   We'll be glad to
implement RFC6020 if the 
    Tools and RFC processing would not blow up. Sigh - long standing fight with bugs in tool change.
    Benoit Claise was correct to place a great deal of emphasis on the tool chain for yang.
  End-Sue comment]  

         leaf tp-state {
           [...]
             enum others {
               value 4;
               description
                 "The termination point is in other state.";
             }

Is there a plan for how substructure of these "others" states might be added in the future?
[Ben:  Other states are being developed in 802.1 for fast fail-over of
ports and deterministic failure over (~IETF detnet group.   Good catch.  I
had only check on the level of deployment 24 months ago. ]

Section 6

Thank you for updating the privacy considerations in response to the secdir review!

   In the case of a topology that is configured, i.e. whose origin is
   "intended", the undesired configuration could become effective and be

Perhaps toss the word "datastore" in here somewhere to remind the less-clueful reader (i.e., me) that origin is an NMDA concept?  Though if it's sufficiently obvious, no need to do it just for me.
[Sue: Good point.   I think it should be added.] 

   reflected in the operational state datastore, leading to disruption
   of services provided via this topology might be disrupted.  For those

nit: deduplicate "disruption"/"disrupted".
[Sue: Godo point. 
   Final text: 
    New/ In the case of a topology that is configured,  i.e. whose origin is

            the "intended" datastore, the undesired configuration could become effective and be 
           reflected in the operational datastore, leading to a disruption

           of services provided via this topology./ 
     ]  

   reasons, it is important that the NETCONF access control model is
   vigorously applied to prevent topology misconfiguration by
   unauthorized clients.

Should we condense "NACM" here since we provided the acronym at the start of the paragraph?
[Sue:  optional to authors.   I'm fine with condensation. ] 

   o  l2-node-attributes: A malicious client could attempt to sabotage
      the configuration of important node attributes, such as the name
      or the management-address.

I don't feel a need for a text change here (since "such as" suffices), but would a change to the node's MAC address be similarly impactful?

[Sue:  Each Node has many MAC addresses.  I assume that you are implying the management port MAC address. 
   I need a bit more clarity on your question to try to address this point.
] 

   Some of the readable data nodes in this YANG module may be considered
   sensitive or vulnerable in some network environments.  It is thus
   important to control read access (e.g., via get, get-config, or
   notification) to these data nodes.  In particular, the YANG model for
   layer 2 topology may expose sensitive information, for example the
   MAC addresses of devices.  Unrestricted use of such information can

I think I've been told that in some environments the topology itself (especially VLAN/VXLAN identifiers) can be considered sensitive.  What's written here is consistent with that, and I don't insist on a change to the text, but wondered if that was seen as a common enough thing to be worth mentioning.

[Sue: The VLAN/VXLAN identifiers are considered part of the same sensitive information. 
   However, in my mind the text above covers both ports and VLAN/VXLAN identifiers. 
    If you have additional text, please suggest it. ] 

Section 8.1

RFC 3688 could arguably be demoted to informative, as could RFC 7951.

[Sue:  We had followed current Yang specifications.   Demoting either
RFC3688 and RFC7951 can be done if the Yang Doctors, NM/OPS ADs, and IESG
agree.    Authors and WG only tried to follow Best common practices.] 

Section 8.2

If we use types defined in [IEEE802.1Qcp], that seems like a normative reference to me.

[Sue: By importing the IEEE 802.1Qcp model, it seemed like a normative reference.  However, my best understanding of the IEEE review was that it was not normative.  Again, all the authors need is a clear reading of the correct status from IEEE liaison. ] 

Noting the discussion at
https://www.ietf.org/about/groups/iesg/statements/normative-informative-re
ferences/
about even optional features still being normative references, I think RFC
7348 would be better placed as a normative reference.  Note that there is not a process/downref issue in doing so, since it is already listed at https://datatracker.ietf.org/doc/downref/
[Sue: I've added this RFC7348 as one of the things that must be resolved with IESG + IEEE-IETF liaison] 

I could go either way (informative or normative) for RFC 8342; presumably there's a convention to stick to.

[Sue: RFC8342 is considered informative as it sets the design but does not specify a model that must be followed. 
We'll leave it informative for this point since I think this is the convention.  If not, hopefully Rob Wilton will provide a comment about it.
] 

Appendix B

I was going to reference
https://www.iab.org/2016/11/07/iab-statement-on-ipv6/ and suggest IPv6 addresses as example management-addresses, but I have a lingering memory that the IPv4 form is still used to identify nodes even in v6-only environments.  Do the right thing, of course.

[Note that I did not do an extensive consistency/sanity check on the example topology or try to reconstruct it from the JSON.]

[Sue:  I'm confused on the Appendix B.  All I see in MAC Addresses and
logical identifiers.   Could you let me know what I missed? ] 

_______________________________________________
i2rs mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/i2rs

---

From: Qin Wu <[email-masked]>
Date: Sun, 12 Jul 2020 07:54:52 +0000

Thanks Ben, I have incorporate your comment in v-15
https://tools.ietf.org/html/draft-ietf-i2rs-yang-l2-network-topology-15
Regarding the extensibility to support future tp state, here is the proposed changes:
OLD TEXT:
"
         leaf tp-state {
           type enumeration {
             enum in-use {
               value 1;
               description
                 "The termination point is in forwarding state.";
             }
             enum blocking {
               value 2;
               description
                 "The termination point is in blocking state.";
             }
             enum down {
               value 3;
               description
                 "The termination point is in down state.";
             }
             enum others {
               value 4;
               description
                 "The termination point is in other state.";
             }
           }
           config false;
           description
             "State of the termination point.";
         }
       }
     } "
NEW TEXT:
"
  identity tp-state-type {
    description
      "Base type for termination point state.";
  }

  identity inuse {
    base tp-state-type;
    description
      "The termination point is in forwarding state.";
  }

  identity blocking {
    base tp-state-type;
    description
      "The termination point is in blocking state.";
  }

  identity down {
    base tp-state-type;
    description
      "The termination point is in down state.";
  }

  identity unknown {
    base tp-state-type;
    description
      "The termination point is in unknown state.";
  }
  leaf tp-state {
    type identityref {
    base tp-state-type;
    }
   config false;
   description
    "State of the termination point.";
 }
"
-Qin
-----邮件原件-----
发件人: Benjamin Kaduk [mailto:[email-masked]] 
发送时间: 2020年7月10日 6:54
收件人: Qin Wu <[email-masked]>
抄送: The IESG <[email-masked]>; [email-masked]; [email-masked]; [email-masked]
主题: Re: Benjamin Kaduk's No Objection on draft-ietf-i2rs-yang-l2-network-topology-14: (with COMMENT)

Hi Qin,

On Tue, Jul 07, 2020 at 01:40:23PM +0000, Qin Wu wrote:
> Hi, Ben:
> -----邮件原件-----
> 发件人: Benjamin Kaduk via Datatracker [mailto:[email-masked]]
> 发送时间: 2020年7月7日 9:35
> 收件人: The IESG <[email-masked]>
> 抄送: [email-masked]; 
> [email-masked]; [email-masked]
> 主题: Benjamin Kaduk's No Objection on 
> 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/iesg/statement/discuss-criteria.html
> [Qin]: No, it doesn't. management address is used to setup management channel and provide management interface for the administrator.

Ah, I think I see.
So it would only be a problem if the device didn't speak IP on its management channel, which can be different from what L2 data-plane technology it switches.  And if the device doesn't speak IP on that channel, it's probably not going to have much use for this YANG module...

> [Qin]: core network-topology module defined in RFC8345 doesn't specify 
> name but specify link-id, link-id should be unique among different network types, For name defined in L2 topo, we follow the same design style for L3 topo, i.e., specify name for each L2 link which is optional attribute.

Okay, so if everything is using the link-id as the primary key then there's no real issues with having name collisions.

> [Qin]: pyang tool doesn't complain about this. If needed, we can change the order.

I saw the same ordering in draft-ietf-pim-igmp-mld-snooping-yang (also on this week's telecha), so I expect that this is fine to leave as-is.

> Is this intended as a nod to the use of 0 for the management VLAN?/ (I 
> seem to recall this topic raising to some level of controversy in the 
> discussions around draft-ietf-bfd-vxlan.)
> [Qin]: See Martin's reply, thanks Martin.
>      /*
> 
>       * Features
>       */
> 
> [Qin]:Yes, your are correct.
>          leaf delay {
>            type uint32;
>            units "microseconds";
>            description
> [Qin] Correct.
> [Qin]: I think port and l2 termination point is equivalent, we could change to layer 2 termination point description.

It's probably good to have the descriptions be consistent if there's not an actual difference to emphasize.

> [Qin]: Sure, will do as you suggested.
>            }
>            //case ethernet
> 
> [Qin]: Okay, will move single line comment to the end of closing brace.

Thanks.  (draft-ietf-pim-igmp-mld-snooping-yang already used the style with the comment on the same line as the closing brace.)

> [Qin]: Others means unknown termination point state.

Right.  Suppose in 10 years there have accumulated five (picking an arbitrary number) specific possible termination point states that have been identified in addition to the four already listed here.  If we wanted to differentiate between those (or some other, yet-unknown state), is there a way to use or extend this model to make such a distinction possible?

> [Qin] Correct, will add.
> [Qin]:Good catch.
> [Qin]: Okay.
> [Qin]: Yes, we could add if you think it needed.

It's probably okay to leave this as-is.

> [Qin]: I think we could mention VLAN and VXLAN identifiers as sensitive information example.
> Section 8.1
> 
> https://www.ietf.org/about/groups/iesg/statements/normative-informativ
> e-references/ about even optional features still being normative 
> references, I think RFC 7348 would be better placed as a normative 
> reference.  Note that there is not a process/downref issue in doing 
> so, since it is already listed at 
> https://datatracker.ietf.org/doc/downref/
> 
> [Qin]: Thanks, will see how to fix these references, thanks.
> Appendix B
> 
> I was going to reference
> [Qin]: Okay, will add IPv6 support in the JSON example.

Sounds good, thanks for the explanations and updates!

-Ben