YANG Modules for IPv4-in-IPv6 Address plus Port (A+P) Softwires
draft-ietf-softwire-yang-16

I support Alissa's DISCUSS, as well as Benjamin's "timestamp" issue.

Unfortunately I ran low on time and so wasn't able to do as full a review as I would have liked, so I'm trusting the sponsoring AD.

I support Alissa's discuss.

Further to Benjamin's discuss, I note that the YANG modules in this document
indicate two editors (I. Farrer and M. Bocadair) and five authors. Assuming
this distinction is intentional, RFC 7322 points to a clear resolution:

   If there is a
   request for more than five authors, the stream-approving body needs
   to consider if one or two editors should have primary responsibility
   for this document, with the other individuals listed in the
   Contributors or Acknowledgements section.

Based on the RFC 7322 text, I suspect that the best approach is moving the
five non-editors to a "Contributors" section.

I am agreeing with Alissa's and Benjamin's DISCUSSes.

Thanks for addressing my DISCUSS. I would suggest s/privacy data/private data/ in the security considerations section.

I support Alissa's DISCUSS.

I share Benjamin's question about the number of authors.

Thank you for quickly resolving my Discuss points!  Original COMMENT section preserved below.

Please expand CE on first usage.

Section 4.1

It feels a little strange to put something as generic as
/if:interfaces/if:interface/if:statistics:sent-ipv4-packets in the
ietf-softwire-ce module.  Are these counters likely to be useful for other
(non-softwire?) tunneling techniques?

Section 5.2

   o  softwire-num-max: used to set the maximum number of softwire
      binding rules that can be created on the lw4o6 element
      simultaneously.  This paramter must not be set to zero because
      this is equivalent to disabling the BR instance.

This seems to leave it ambiguous whether a server should reject an attempt
to set it to zero, or accept it but diable the BR instance.

Section 7

            leaf enable-hairpinning {
              type boolean;
              default "true";
              description
                "Enables/disables support for locally forwarding
                 (hairpinning) traffic between two CEs.";
              reference "Section 6.2 of RFC7596";

Is a global toggle sufficient or would there be cases where more
fine-grained control would be needed?

Section 8

    container algo-versioning {
[...] 
      leaf date {

        type yang:date-and-time;
        description
          "Timestamp when the algorithm instance was activated.

           An algorithm instance may be provided with mapping
           rules that may change in time (for example, increase
           the size of the port set). When an abuse party
           presents an external IP address/port, the version
           of the algorithm is important because depending on
           the version, a distinct customer may be identified.

nit: "abuse party" is probably not a term that everyone is familiar with.
(similarly in br-instances)

Section 9

Is there any possibility of a situation where the
invalid-/added/modified-entry notifications cause a substantial amount of
notification traffic (i.e., a DoS level of traffic)?

Is the derived-from() complexity for statistics counters really required given that the module itself is for softwires, and any tunnel using it would be covered?

Some minor comments:

1) Sec 4.2:
"softwire-path-mru: optionally used to set the maximum IPv6
      softwire packet size that can be received, including the
      encapsulation/translation overhead.  Needed if the softwire
      implementation is unable to correctly calculate the correct IPv4
      Maximum Receive Unit (MRU) size automatically [RFC4213]."
I guess this should both be IPv6...?

2) Why does the description of "rcvd-ipv4-bytes" say
"IPv4 traffic received for processing, in bytes"..?
Does the "for processing" have any special meaning or why is it only phrased like this for that one entry?

3) Also the description for "sent-ipv4-bytes" and "sent-ipv4-packets" could be unified.

I would have thought putting in a prioritization mechanism (like RFC8026 does) for ordering the different mechanisms would have been useful in this YANG module in order to configure the CE. Was this something that was considered?