Hi Luuk,
Inline:
On 10/3/23 15:28, Luuk Hendriks wrote:
Sec 4.2
- Do we really want the BGP PDU TLV to be possibly preceded by other
TLVs? Are there any examples of when this would be useful, other than
TLVs containing info needed to correctly parse the BGP PDU (which we
should not want anyway in my opinion)?
For use cases where one just wants the BGP PDU, being able to bail out
as early as possible sounds useful.
Or another way to think about this: the BGP PDU TLV is a mandatory TLV
for the RouteMonitoring message. To me it makes sense to first list the
mandatory TLVs, then the optional ones.
This was the original thinking behind Route Mirroring in RFC7854.
Hence i wanted to get this new proposal to be consistent to something
that had already been (successfully) proposed.
Not sure I follow: what do you mean with 'This' here?
I was referring to the fact that BGP PDU TLV being preceded by other
TLVs was proposed in RFC7854 for the Route Mirroring message, 4.7:
"Type = 0: BGP Message. A BGP PDU. This PDU may or may not be an
Update message. If the BGP Message TLV occurs in the Route
Mirroring message, it MUST occur last in the list of TLVs."
Another thing is: true now it is all TLV'd but some comments to the
previous way of structuring the Route Monitoring message with TLVs, ie.
BGP Update first then TLVs, was criticized because TLVs may contain info
that may be good to have upfront when parsing the BGP PDU.
Finally, since I was seeing no point for the BGP PDU TLV not to be
followed by other TLVs, i proposed "A Route Monitoring message MUST
contain one BGP Message TLV which may be preceeded and followed by other
optional TLV data.".
- do the Group TLVs appear before or after the normal TLVs? If we do
enforce sorting by code points, we could steer this behavior by
picking a certain code point.
I was not thinking to make Group TLVs somehow special compared to any other
TLVs that we may define later such that ordering would matter.
Group TLVs introduce new indices that other ('normal') TLVs can point
to, so having the Group TLVs come before the other ones feels like a
'define before use' case to me.
I see, let's ponder about this part a bit further (and let's see if
anybody else would have input too). On one hand, you do have a point
with the 'define before use' part; on the other i wonder how we could
enforce 'define before use' with a spec that could be potentially
updated by further documents.
- can multiple TLVs point to a Group TLV index? (I assume yes)
Yes. Do you believe text would be needed for this?
It'd be nice to leave as little room for interpretation as possible.
Currently in sec 3 the text states:
Multiple TLVs of the same type and with the same index can be repeated
as part of the same message.
Maybe generalize this to:
Multiple TLVs of the same type and/or with the same (Group) index
can be repeated as part of the same message.
(Though at that point the Group TLV is not yet introduced, so maybe this
is more confusing than it is actually helpful.)
I still struggle with this a bit - can i ask you an example? Because a
Group TLV with the same Group Index would be like re-defining the Group,
wouldn't it? And actually i was now thinking it would be good to add
some text to say we don't want that.
- can Group TLV indices appear in the pointed-to indices in another
Group TLV, i.e. nested grouping? (I'm not sure this makes things
easier for anyone in the end, so if there is no convincing use-case
for it, perhaps it should be explicitly forbidden?)
Good one, i agree we could start with explicitely forbidding this and then
re-visiting if anybody comes with a use-case around it. Added text.
Should we also state the pointed-to-indices can not be 0x0000, simply
because it would not make sense?
Sure, added text.
https://github.com/paololucente/draft-ietf-grow-bmp-tlv/commit/9201ef559a1c951f5c26c4330a90aaffbbade2d6
For my own understanding I created this example RouteMonitoring message
(in sort of pseudo-wireformat if you will) comprising:
- the usual CommonHeader and PerPeerHeader
- the BGP PDU in the first TLV, containing 10 NLRI
- two Group TLVs, pointing to four and three actual TLVs, respectively
- one TLV pointing to a single NLRI (NLRI number 7)
- one TLV pointing to the first Group TLV
- one TLV pointing to the second Group TLV
RouteMonitoring
CommonHeader
PerPeerHeader
TLVs
[type=TBD1, length=x, index=0, value=$BGP_PDU{ NLRI1, .. NLRI10 } ]
[type=TBD2, length=x, index=0, value={0x0001, 0x0002, 0x0003, 0x000a}]
[type=TBD2, length=x, index=0, value={0x0004, 0x0005, 0x0006}]
[type=TBDw, length=x, index=7, value=something_that_applies_to_NLRI7]
[type=TBDx, length=x, index=0, value=something_that_applies_to_all]
[type=TBDy, length=x, index=0x000b, value=applies_to_four_nlri]
[type=TBDz, length=x, index=0x000c, value=applies_to_three_nlri]
If this is correctly interpreted and somehow useful for the draft, I'm
happy to make this into a proper diagram.
Please, that contribution would be very much appreciated!
Please see
https://github.com/paololucente/draft-ietf-grow-bmp-tlv/pull/3
for a first shot at this. Might need some more love but I wanted to
provide something before the cut-off coming Monday.
Absolutely, thank you! In the same commit referenced above in this email
i did couple minor edits to it, mainly spelling and review to length
calculations.
Paolo
_______________________________________________
GROW mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/grow
