Subject: Re: [babel] (Juliusz) I-D Action: draft-ietf-babel-information-model-02.txt

From: "STARK, BARBARA H" <[email-masked]>
Date: Tue, 5 Jun 2018 17:37:44 +0000

Response in-line. I've noted a couple of items with "***" where I'd appreciate more info.
Barbara

> -----Original Message-----
> From: Juliusz Chroboczek <[email-masked]>
> 
> Thanks, Barbara.  I haven't checked the security bits, but the rest looks fine
> to me.  A few minor nits.
> 
>    Due to the simplicity of the Babel protocol and the fact that it is
>    designed to be used in non-professionally administered environments
>    (such as home networks)
> 
> Please don't say that -- Babel is a general-purpose routing protocol that is
> designed to survive where other protocols die.  In particular, it can survive in
> the hostile Home Network environment, but I'd like to avoid restricting it to
> that case.  I suggest removing the bit from "and the fact" up to the closing
> paranthesis.

Done

>       babel-supported-link-types: set of values of supported link types
>       where the following enumeration values MUST be supported: 1 =
>       wireless, 2 = physical-layer ethernet, 99 = other
> 
> I'd prefer this to be a list of strings, with possible values "wired", "wireless",
> "tunnel" and "other".

Changed to string (and removed sentence under "int" data type that said it was used for enumerations. But I used "ethernet" instead of "wired". I really think at some point it may be useful to see if it makes sense to treat technologies over coax and powerline (which are also technically "wired") different from (physical layer) ethernet.
 
> > 3.2.  Definition of babel-constants-obj
> 
> I don't see a way to find out if this Babel instance is speaking IPv6, IPv4, or
> both.  (Toke suggested we dump the IPv4 support, I'm half inclined to agree.)

Deleted  [ip-address   babel-mcast-group-ipv4;]

> > 3.3.  Definition of babel-interfaces-obj
> 
>       babel-link-type: indicates the type of link; integer values
> 
> Same as above -- I'd prefer this to be a string.

Same as above.
 
>       babel-external-cost: external input to cost of link of this
>       interface (need to determine how to express this);MUST be
>       configurable if implemented
> 
> I suggest "if supported, this is a value that is added to the metrics of routes
> learned over this interface".  There's a space missing after the semicolon.

Done. Also added "how an implementation uses the value is up to the implementation, which means the use may not be consistent across implementations"
 
>       babel-neighbors: a set of babel-neighbors objects
> 
> There's a "u" missing ;-)

Comment rejected. U can't make me (because, when u is missing there's not much u can do).
 
> > 3.4.  Definition of babel-neighbors-obj
> 
>       babel-hello-mcast-history: the multicast Hello history of whether
>       or not each of the 16 multicast Hello messages prior to babel-
>       hello-seqno was received; represented as a 16 bit (4 hex digits)
>       value where 1 = Hello received and 0 = Hello not received; see
>       draft-ietf-babel-rfc6126bis [rfc6126bis] section A.1
> 
> You need to specify the endiannness of this bitstring.  The reference
> implementation puts the most recent bits at the big-endian end, and shifts
> right. 
> On a related note, shouldn't this be a bit string of an implementation-defined
> length?

Thx. I had actually tried to figure out the endianness from the appendix text, but wasn't quite sure after reading it. That's why I just referred to the text; I thought maybe it was just me being dense. I'm adding a statement that the most recent received Hello is placed in the most significant bit and previous Hello bits are shifted right; and making the string length undefined. Since endianness is often thought of in the context of how the bits are internally stored (as opposed to how they are visually represented to humans), I'm avoiding that word and using "most significant bit" instead.
 
>      babel-hello-seqno: expected Hello sequence number
> 
> This needs to be split into "babel-expected-ucast-hello-seqno" and "babel-
> expected-mcast-hello-seqno".

OK. I used "exp" instead of "expected". I also made both mandatory but said "if [multi|uni]cast Hello messages are not expected, or processing of [multi|uni]icast messages is not enabled, this MUST be 0. I could have made them conditionally mandatory but thought it would be easy enough to implement and set to 0 even if the type of casting isn't supported. Changed the history parameter descriptions to reference these new names.
 
> > 3.6.  Definition of babel-routes-obj
> 
> I think there's a field missing -- the interface over which this route has been
> learned.  Alternatively, you could put a reference to the neighbour from
> which this route has been learned.

Ok. Included neighbor reference. It had been in the previous iteration and for some reason I can't quite remember was deleted. I think I got confused because the definition was bad. 
***question***: It's possible for the same route to be advertised by multiple neighbors, right? -- so is this entry the one that has been selected to actually use (and other advertisements were discarded)?
 
>       babel-route-next-hop: the next-hop address of this route; this
>       will be empty if this route has no next-hop address
> 
> I suggest "if this is empty, the next hop is the address of the neighbour from
> which the route has been learnt."

***opinion needed***: Hmm. If the next hop is the address of the neighbor, then that's what should be in this parameter and it shouldn't be empty. The purpose of "empty" (according to my notes of our discussion) was for the case where this device itself is the "next hop". I'm not making changes to this until we discuss.
 
>       babel-route-metric: the metric with which this route was
>       advertised by the neighbor, or maximum value (infinity) to
>       [...]
>       babel-route-announced-metric: a calculated metric for this route;
>       how the metric is calculated is implementation-specific; either
> 
> Have you inverted the two?
>
> Please add "if this is the value 65535, then this route has been retracted and
> is temporarily unreachable (see Section 3.5.5 of [RFC6126bis])".

To avoid ambiguity, I've renamed to babel-route-received-metric and babel-route-calculated-metric.
***opinion need***: The current definition (for the what is now received-metric) says "maximum value (infinity)" will indicate retraction, which is consistent with language of section 3.5.5. And calculated-metric has nothing similar. Are you suggesting (a) received-metric should replace "maximum value (infinity)" with 65535 and calculated-metric should have this statement added, (b) received-metric should continue to use "maximum value (infinity)" but calculated-metric should have the statement you suggest (with 65535), or (c) would it be better for both to use "maximum value (infinity)" in such a statement? I'm going with choice (c) in absence of additional advice.
 
>       babel-route-feasible: a boolean flag indicating whether this route
>       is known to work; a route that is not feasible will never be
>       selected 
> A boolean flag indicating whether this route is feasible, as defined in Section
> 3.5.1 of [RFC6126bis].

OK. Changed.
 
>    5.   Do we need a registry for the supported security mechanisms?
> 
> No.

OK. Issue moved to Closed.

---

From: Juliusz Chroboczek <[email-masked]>
Date: Thu, 07 Jun 2018 03:58:19 +0200

Good work, Barbara, thanks a lot.

> Changed to string (and removed sentence under "int" data type that said
> it was used for enumerations. But I used "ethernet" instead of
> "wired". I really think at some point it may be useful to see if it
> makes sense to treat technologies over coax and powerline (which are
> also technically "wired") different from (physical layer) ethernet.

Ok, I'll add an alias to the configuration language of babeld.

>> I don't see a way to find out if this Babel instance is speaking IPv6,
>> IPv4, or both.  (Toke suggested we dump the IPv4 support, I'm half
>> inclined to agree.)

> Deleted  [ip-address   babel-mcast-group-ipv4;]

Suggest renaming babel-mcast-group-ipv6 to babel-mcast-group.

> Comment rejected. U can't make me (because, when u is missing there's

One of the major hurdles in first year Java programming is to learn to
spell "length".  (And then they claim that Polish has too many consonants.)
 
>> babel-hello-mcast-history:

[...]

>> You need to specify the endiannness of this bitstring.

Hmm, now it's a string but you speak about most-significant bits?  I'm
confused.

> ***question***: It's possible for the same route to be advertised by
> multiple neighbors, right?

No.  Multiple routes to the same destination can be announced by multiple
neighbours, but Babel considers them as distinct routes.  See 6126bis
Section 3.2.6 -- two routes are the same if they have the same triple
(prefix, plen, neighbour).

> -- so is this entry the one that has been selected to actually use (and
> -- other advertisements were discarded)?

No.  Of the multiple routes to a given destination, at most one is selected.

>> I suggest "if this is empty, the next hop is the address of the
>> neighbour from which the route has been learnt."

> ***opinion needed***: Hmm. If the next hop is the address of the
> neighbor, then that's what should be in this parameter and it shouldn't
> be empty. The purpose of "empty" (according to my notes of our
> discussion) was for the case where this device itself is the "next
> hop". I'm not making changes to this until we discuss.

You're right.  Artefact of my implementation.

> ***opinion need***: The current definition (for the what is now
> received-metric) says "maximum value (infinity)" will indicate
> retraction, which is consistent with language of section 3.5.5. And
> calculated-metric has nothing similar. Are you suggesting (a)
> received-metric should replace "maximum value (infinity)" with 65535 and
> calculated-metric should have this statement added, (b) received-metric
> should continue to use "maximum value (infinity)" but calculated-metric
> should have the statement you suggest (with 65535), or (c) would it be
> better for both to use "maximum value (infinity)" in such a statement?
> I'm going with choice (c) in absence of additional advice.

Agreed.
 
Thanks again,

-- Juliusz