Subject: [babel] Document Shepherd review of draft-ietf-babel-source-specific-04

From: Donald Eastlake <[email-masked]>
Date: Thu, 27 Dec 2018 23:34:38 -0500

I read draft-ietf-babel-source-specific-04. It seems to be in good shape
but and I have the following thought:

I believe babel-source-specific does not have routing loops even with a
mixture of BABEL routers supporting and not supporting source-specific if
routing messages are well formed. However, if a routing message is
interpreted differently by different routers, then it seems to me that you
might get loops. In Section 7, 2nd paragraph, the flexibility that
      "a node
   receiving more than one Source Prefix sub-TLV in a single TLV SHOULD
   ignore this TLV.  It MAY ignore the whole packet."
seems like it might cause problems. Assuming a malformed message with two
Source Prefix sub-TLVs where one router accepts it and uses the first while
the other accepts it and uses the second, or the other possibilities where
they end up with persistent different routing states. Perhaps it would be
better to say "a node receiving more than one Source Prefix sub-TLV in a
single TLV MUST ignore this TLV." (or only use the first or last sub-TLV or
any other precisely deterministic rule). Similarly, in Section 7.2, it says
      "A
   wildcard retraction (Update with AE equals to 0) MUST NOT carry a
   Source Prefix sub-TLV."
So what happens if it does? I would feel more confident if it said
something deterministic so either (1) it MUST NOT carry a Source Prefix
sub-TLV and MUST be ignored if it does or (2) it MUST Not carry a Source
Prefix sub-TLV and if it does, the Source Prefix sub-TLV is ignored.

Thanks,
Donald
===============================
 Donald E. Eastlake 3rd   +1-508-333-2270 (cell)
 1424 Pro Shop Court, Davenport, FL 33896 USA
 [email-masked]

---

From: Juliusz Chroboczek <[email-masked]>
Date: Sun, 30 Dec 2018 01:08:01 +0100

Thanks a lot for the review, Donald.

We're slightly incoherent here:

  - Section 7 says that multiple sub-TLVs in a single Update MUST NOT be
    sent and the whole TLV SHOULD be ignored;
  - Section 7 also says that a sub-TLV in a retraction MUST NOT be sent,
    while Section 5 says that such a sub-TLV MUST be ignored on reception.

(You appear to have missed the bit in Section 5, which I cannot blame you
for -- it needs repeating in Section 7).

The reason for the inconsistence is that an implementation must check for
the presence of the sub-TLV in any case, so the requirement to discard
retractions with the sub-TLV puts no extra onus on implementations.  On
the other hand, checking for the case of multiple occurrences of the
sub-TLV puts a small cost on implementations, but one that's probably
negligible.

I'm definitely going to update Section 7 to repeat the requirement in
Section 5.

I think you're right, that the SHOULD about multiple sub-TLVs should be
a MUST, but I want to sleep over it and check the implementation.  Toke,
do you have an opinion?

-- Juliusz

---

From: Toke Høiland-Jørgensen <[email-masked]>
Date: Mon, 31 Dec 2018 13:35:34 +0100

> a MUST, but I want to sleep over it and check the implementation.  Toke,

Bird already ignores the whole TLV if it has more than one SS sub-tlv in
it. No objection to making this mandatory :)

-Toke