Re: [RTG-DIR] Rtgdir telechat review of draft-ietf-babel-rfc6126bis-11

[Juliusz Chroboczek <[e-mail]>]

Dear Ms. Qu,

Sorry for the delay, and thank you for your reminder.

>> The description about packet trailer is in RFC 7557 section 2.5, but
>> not included in the bis version.

>     The packet trailer is described in Section 4.2.

> [YQ]: In RFC 7557 section 2.5, there is a clear description about how the
> packet trailer length is calculated etc. Personally I feel it's more
> straightforward, but if you think section 4.2 is enough, I'm not going to
> insist.

I see.  Yes, I'll clarify that.
 
>> There are multiple places in the document about TLV/Sub-TLV being
>> “self-terminating”, but I didn’t find what it means. Maybe I’m missing
>> something here?

>     This is defined in Section 4.4.  I have checked that all uses of
>     self-terminating occur after its definition.

> [YQ]: in section 4.4, it says self-terminating can be used to determine the
> length of the TLV body without using the length field. This is clear, however
> what does it look like? how to detect it? maybe I'm missing something here.

For example, the router-id TLV is always 8 octets long.  It is
self-terminating.

The Pad1 TLV can have arbitrary length, and it is impossible to determine
its length without the explicit Length field.  It is not self-terminating.
 
>> Section 1.1
>> “unmanaged and wireless environment”, what does “unmanaged” mean here?

>     Not being actively managed by a human administrator.  This is a
>     non-normative
>     section.  Please let me know if I'm using this term badly.

> [YQ]: my understanding is unmanaged can mean anything, like no management at
> all. I don't think it's a good term, especially put together with wireless. 
> maybe try to add a bit more explanation?

This is a conceptual overview, unmanaged is used in a non-technical sense.

>> Section 3.2.3 The interface Hello seqno is changed to outing multicast
>> hello seqno, however there is no description about unicast hello at
>> all.

>     Unicast Hellos are per-neighbour, so they appear in Section 3.2.4 (Section
>     3.2.3 describes per-interface data structure while 3.2.4 describes
>     per-neighbour structures).  Both kinds of Hellos are described in more
>     detail in Section 3.4.1.

> [YQ]: there is nothing wrong with current text. I was thinking of adding
> unicast just for easy readability. same for the following three related
> questions.

The approach taken here is to define the data structures before explaining
the protocol.  The advantage is to have a clear reference for the
implementer; but of course it leads to a number of things that are not
entirely clear at this point.  It's a difficult tradeoff, I'm going to
leave it as is.

>> Section 3.8.1.1

>> After a node receives a route request, if the given prefix doesn’t
>> exist in its route table, it MUST send a retraction for that prefix. So
>> my question is whether a node is allowed to send multiple explicit
>> requests for a given prefix?

[...]

> [YQ] thanks for the explanation. I'm just wondering whether we should
> document it somewhere?

No, I don't think so.  This case is not specific -- for example, sending
multiple copies of updates is also not a good idea.
 
> [YQ]: My understanding is Babel uses a stateful parser, and TLVs in the packet
> trailer are not allowed to modify the state. So the text here is suggesting to
> implement a stateless parser for packet trailer. so the question is: will
> packet trailer to able to use/read the state? is the stateless parser suggested
> just for implementation simplicity?

None of the TLVs allowed in the packet trailer use or modify the state.
See also the last paragraph of the appendix "Considerations for protocol
extensions".
  
>> Section 4.5
>> 1674       parsing a TLV MUST update the parser state even if the TLV is
>> 1675       otherwise ignored due to an unknown mandatory sub-TLV.

>     I believe you may have forgotten to write your comment.

> [YQ]: is it correct that the parser state is updated by a TLV even if the TLV
> is ignored due to unknown sub-tlv? what about a TLV ignored due to other
> reasons?

You're right, I'll add a comment.

>> Section 4.6.5
>> 1805                 send a new scheduled Hello TLV with the same setting
>     of the
>> 1806                 Unicast flag.  If this is 0, then this Hello
>     represents an
>> I’d suggest changing the text to “the same setting of flags” instead of
>     “the
>> same setting of the Unicast flag”, considering the flags will be extended
>     later.

>     I disagree.  This paragraph is about the multicast/unicast dichotomy, not
>     about future flags of an informative nature.

> [YQ]: then I misunderstood it. I thought it was meant to copy the entire flag
> field.

>> Nits:

>> 1049       metric) from a neighbour neigh with a link cost value equal to
>     cost,
>> 1050       it checks whether it already has a route table entry indexed
>     by
>> [neighbour neigh] => [neighbour]

>     I disagree.  We're defining the variable neigh, which we use in the next
>     sequence.

> [YQ]: it seems that this is the only place using "neigh", so I thought it was a
> typo. I would suggest some clarification.

This has been removed.

-- Juliusz