Subject: [netmod] RtgDir review: draft-ietf-netmod-ip-cfg-12

From: Alia Atlas <[email-masked]>
Date: Wed, 22 Jan 2014 18:29:26 -0500

Hello,

I have been selected as the Routing Directorate reviewer for this draft.
The Routing Directorate seeks to review all routing or routing-related
drafts as they pass through IETF last call and IESG review. The purpose of
the review is to provide assistance to the Routing ADs. For more
information about the Routing Directorate, please see
http://www.ietf.org/iesg/directorate/routing.html

Although these comments are primarily for the use of the Routing ADs, it
would be helpful if you could consider them along with any other IETF Last
Call comments that you receive, and strive to resolve them through
discussion or by updating the draft.

Document: draft-ietf-netmod-ip-cfg-12.txt
Reviewer: Alia Atlas
Review Date: 22 January 2014
IETF LC End Date: 23 January 2014
Intended Status: Proposed Standard

*Summary:*
I have a few minor concerns about this document (clarifying a few aspects)
that I think should be resolved before publication.

*Comments:*
This document is very clearly written, but I did find a few details that
could use a bit more clarity.  I was able to figure out what was intended
by wandering through the referenced RFCs.

*Major Issues:*
No major issues found.

*Minor Issues:*

   1. ipv4/forwarding and ipv6/forwarding:  First, I wasn't sure what this
   meant or how it was different from ipv4/enable or ipv6/enable.   I went
   back to look at RFC 4293 to learn that ipv4/forwarding means “acting as a
   router and forwarding traffic not destined to the device”.  Could you
   clarify the text to add those key details into the description for
   ipv4/forwarding and ipv6/forwarding?  Second, I was surprised that the
   default is false; I think about this from the router perspective, of
   course, but a bit of explanation as to why that decision was made would be
   useful.
   2. ipv4/enabled and ipv6/enabled:  Similarly, what is enabled by
   ipv4/enabled isn’t clear (unless I look at RFC 4293).   Could you clarify
   that it means connected to an IPv4 stack for sending out IPv4 packets and
   for receiving to-me packets and do the same clarification for ipv6/enabled?
   3. ipv4/mtu and ipv6/mtu:  This is discussing sending and receiving
   packets – a mention of the MRU would be useful and even quoting  RFC2460 in
   Sec 5 which says “From each link to which a node is directly attached,
   the node must be  able to accept packets as large as that link's MTU. “
   so it’s clear.
   4.  For the ARP cache (ipv4/neighbor) and ND cache (ipv6/neighbor), is
   there a reason that lastUpdated isn’t included?  This doesn’t seem to
   provide quite enough information to see whenr mappings were last updated?
   Is the assumption that there will be a ARP specific and ND specific YANG
   model for getting the details?

*Nits:*

I've put these as nits because I think that they are probably textual
errors rather than intentional...

   1.  ipv6/forwarding:   why is the reference RFC 4861 – neighbor
   discovery???  That seems like it applies to the neighbor info, unless I’ve
   misunderstood what ipv6/forwarding is doing…
   2. In the second table in Sec 3, ipv4/neighbor/interface and
   ipv6/neighbor/interface are listed, but I don’t see them in state tree in
   Sec 2.  I think that’s because they aren’t necessary??

Regards,
Alia

---

From: Martin Bjorklund <[email-masked]>
Date: Sat, 25 Jan 2014 15:18:47 +0100 (CET)

Hi,

Alia Atlas <[email-masked]> wrote:
>    ipv4/forwarding and ipv6/forwarding?

I think this probably becomes more clear if we clarify the "enabled"
leaf as you suggest below in (2).  With the suggested new text below
maybe this "forwarding" leaf is ok?  Otherwise, we could write in the
style of 4293:

OLD:

          "Controls if IPv4 packet forwarding is enabled or disabled
           on this interface.";

NEW:

          "Controls IPv4 packet forwarding of datagrams received by,
           but not addressed to, this interface.  IPv4 routers forward
           datagrams.  IPv4 hosts do not (except those source-routed
           via the host)";

>    Second, I was surprised that the

The conceptual variable IsRouter, defined in RFC 4861, has FALSE as
its default value.  (see more below).

>    for receiving to-me packets and do the same clarification for ipv6/enabled?

Would this work?

OLD:

          "Controls if IPv6 is enabled or disabled on this
           interface.";

NEW:

          "Controls if IPv6 is enabled or disabled on this
           interface.  When IPv6 is enabled, this interface is
           connected to an IPv6 stack, and the interface can send
           and receive IPv6 packets.";

>    the node must be  able to accept packets as large as that link's MTU. “

Hmm, the current text says:

        "The size, in octets, of the largest IPv6 packet that the
         interface will send and receive.

Isn't it clear that it applies also to the size of received packets?
Or maybe I misunderstood your concern?

>    model for getting the details?

No, the intention is that this model should be sufficient.

There is not such "last updated" object defined in RFC 4861.  Also, it
seems this information is not available in some implementations (not
in linux).   Do other implementation typcially store this info?  If
so, in what form?

>    misunderstood what ipv6/forwarding is doing…

Actually, the object IsRouter in 6.2.1 in RFC 4861 applies to the
interface, not to the neighbor.  This object is defined as:

                     A flag indicating whether routing is enabled on
                     this interface.  Enabling routing on the interface
                     would imply that a router can forward packets to or
                     from the interface.

(The neighbor cache also tags each neighbor as being a router or not).

>    Sec 2.  I think that’s because they aren’t necessary??

No this is a bug - I forgot to remove these when we changed the data
model at some time.  Thanks for spotting it!

/martin

---

From: Benoit Claise <[email-masked]>
Date: Fri, 31 Jan 2014 15:29:19 +0100

Alia,

Please let us know if your concerns are addressed.
We would like to progress the draft.

Regards, Benoit
> _______________________________________________
> netmod mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netmod

---

From: Alia Atlas <[email-masked]>
Date: Fri, 31 Jan 2014 10:12:43 -0500

Responses in-line.

On Sat, Jan 25, 2014 at 9:18 AM, Martin Bjorklund <[email-masked]> wrote:

> >    back to look at RFC 4293 to learn that ipv4/forwarding means "acting
> as a
> >    router and forwarding traffic not destined to the device".  Could you
[Alia] Perfect

> [Alia] That's fine.

> >    ipv4/enabled isn't clear (unless I look at RFC 4293).   Could you
> clarify
[Alia] Sounds great - same for IPv4?

> >    packets - a mention of the MRU would be useful and even quoting
>  RFC2460 in
> >    Sec 5 which says "From each link to which a node is directly attached,
> >    the node must be  able to accept packets as large as that link's MTU.
> "
> >    so it's clear.
>

[Alia] MRU and MTU do not need to be the same thing.  The current text
assumes that it is - and I agree that is a common way of configuring both
in a single move.  I was just looking for a bit more clarity that you
really do mean MRU as well.  It's not something that I feel strongly about.

> >    there a reason that lastUpdated isn't included?  This doesn't seem to

[Alia] I was looking at RFC 4293 and your table of comparison of objects
between this YANG model and that MIB.  If you look at the MIB, it has the
following:

IpNetToPhysicalEntry ::= SEQUENCE {
        ipNetToPhysicalIfIndex         InterfaceIndex,
        ipNetToPhysicalNetAddressType  InetAddressType,
        ipNetToPhysicalNetAddress      InetAddress,
        ipNetToPhysicalPhysAddress     PhysAddress,
        ipNetToPhysicalLastUpdated     TimeStamp,
        ipNetToPhysicalType            INTEGER,
        ipNetToPhysicalState           INTEGER,
        ipNetToPhysicalRowStatus       RowStatus
    }

ipNetToPhysicalLastUpdated OBJECT-TYPE
    SYNTAX     TimeStamp
    MAX-ACCESS read-only
    STATUS     current
    DESCRIPTION
           "The value of sysUpTime at the time this entry was last
            updated.  If this entry was updated prior to the last re-
            initialization of the local network management subsystem,
            then this object contains a zero value."
    ::= { ipNetToPhysicalEntry 5 }

The only piece of information not captured in the YANG model that I noticed
is the ipNetToPhysicalLastUpdated.

That is why I asked about it being intentionally left out versus
accidentally.

> >    1.  ipv6/forwarding:   why is the reference RFC 4861 - neighbor
> >    discovery???  That seems like it applies to the neighbor info, unless
> I've
> >    misunderstood what ipv6/forwarding is doing...
>
[Alia] Ok - but a reference to the RFC for neighbor discovery is really not
clear when all you are talking about is whether the interface routes
packets or not.  PLEASE either add a more specific reference to the section
or otherwise clarify.  I do think that the text changes we discussed above
will help also.

> >    ipv6/neighbor/interface are listed, but I don't see them in state
> tree in
> >    Sec 2.  I think that's because they aren't necessary??
>

[Alia] Another pair of eyes :-)  Glad to help.

Sorry for the delayed response,
Alia

---

From: Ladislav Lhotka <[email-masked]>
Date: Fri, 31 Jan 2014 16:41:07 +0100

>            datagrams.  IPv4 hosts do not (except those source-routed

This description then might also mention that this leaf has specific implications for the “ietf-routing” module, see sec. 6.2 in draft-ietf-netmod-routing-cfg-13.

Lada

--
Ladislav Lhotka, CZ.NIC Labs
PGP Key ID: E74E8C0C

---

From: Martin Bjorklund <[email-masked]>
Date: Fri, 31 Jan 2014 17:40:41 +0100 (CET)

> > Alia Atlas <[email-masked]> wrote:

[...]

> [Alia] Sounds great - same for IPv4?

Yes.

> or otherwise clarify.  I do think that the text changes we discussed above

The current reference is:

    container ipv6 {
      ...
      leaf forwarding {
        ...
        reference
          "RFC 4861: Neighbor Discovery for IP version 6 (IPv6)
                     Section 6.2.1, IsRouter";
      }
    }

I hope this is specific enough?

/martin

---

From: Alia Atlas <[email-masked]>
Date: Fri, 31 Jan 2014 11:50:08 -0500

On Fri, Jan 31, 2014 at 11:40 AM, Martin Bjorklund <[email-masked]> wrote:

> > > >    that it means connected to an IPv4 stack for sending out IPv4
> packets
> > > and
[Alia] Yes - sorry - should have double-checked the draft again..

Alia

---

From: Ladislav Lhotka <[email-masked]>
Date: Fri, 31 Jan 2014 17:54:38 +0100

> I hope this is specific enough?

I think it is. The other router configuration parameters that are specified in that 4861 section are dealt with in the “ietf-ipv6-unicast-routing” module - perhaps it would be useful to mention this in the description, too.

Lada