Hi Paolo, Yunan, all,
Some thoughts/questions on the -10 version. First a few general points
that also have been touched upon on the list before, then more detailed
comments per section.
General:
- For stateless parsing, I think all the necessary information (Four
Octet Capable, AddPath, Multiple Labels) should come before the BGP
PDU starts. I am strongly in favour of using the peer flags in the
Per-Peer Header for that, and not TLVs. And as such, I think we
should introduce a bit for AddPath, and one for Multiple Labels.
- Regarding bumping the version: I think this draft describes changes
that do not require a version bump (i.e. the new flags as described
above), while others perhaps do (i.e. changing the RouteMonitoring
wireformat). A version bump should have maximum visibility and as such
either a -bis or a new document might be the better place. That would
unfortunately mean the ebit and path marking drafts will be stalled as
they depend on the TLVs, if I recall things correctly. But it might
prevent a lot of confusion later on, especially for new implementers.
Per section:
Sec 3:
- For indexed TLVs, the text is not explicit about whether the length
field includes the 2 bytes for the index field, or describes the
length of only the value.
I don't see a benefit of one over the other, as long as the text is
clear about it.
- If an indexed TLV for a specific NLRI (index > 0) exists alongside a
TLV with index 0, does the specific TLV trump the generic one for that
NLRI, or does it compliment the generic one?
I'd assume the latter.
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.
4.2.1
- I had hard time figuring out how the Group TLV was supposed to work,
until I looked back at some older mails on the list. If my
understanding below is correct, I'm happy to propose some text.
The first two bytes in the value of the Group TLV are a new, made up
index, representing a group of NLRIs. The remainder of the value are
the indices of the actual NLRI in the BGP PDU that belong to the
group. Then, other, 'normal' TLVs can point to the newly made up
index, and carry a value that applies to all the NLRI listed in the
Group TLV.
If that is the correct interpretation, some points/questions I think
should be answered in the text:
- the text might need an additional MUST, stating the newly made up
index MUST be unique for that RouteMonitoring message.
- can one NLRI appear in more than one Group TLV? (I assume yes)
- 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.
- do the indices in the Group TLV have to be sorted? This might be
convenient for parsing/processing at the BMP station.
- can multiple TLVs point to a Group TLV index? (I assume yes)
- 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?)
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.
4.2.2
As mentioned, I think these TLVs should go in favour of flags in the
Per-Peer header.
4.3
Wouldn't it make sense to wrap the BGP PDU in a TLV (for reason codes 1
and 3), for the same reasons we want to wrap the PDU in RouteMonitoring
messages?
Hope this helps. Again, happy to provide text, just want to make sure I
interpreted things correctly first.
Thanks,
luuk
On Tue 8 Nov 2022, 10:11, [email protected] wrote:
>
> A New Internet-Draft is available from the on-line Internet-Drafts
> directories.
> This draft is a work item of the Global Routing Operations WG of the IETF.
>
> Title : TLV support for BMP Route Monitoring and Peer Down
> Messages
> Authors : Paolo Lucente
> Yunan Gu
> Filename : draft-ietf-grow-bmp-tlv-10.txt
> Pages : 8
> Date : 2022-11-08
>
> Abstract:
> Most of the message types defined by the BGP Monitoring Protocol
> (BMP) make provision for data in TLV format. However, Route
> Monitoring messages (which provide a snapshot of the monitored
> Routing Information Base) and Peer Down messages (which indicate that
> a peering session was terminated) do not. Supporting (optional) data
> in TLV format across all BMP message types allows for a homogeneous
> and extensible surface that would be useful for the most different
> use-cases that need to convey additional data to a BMP station.
> While it is not intended for this document to cover any specific
> utilization scenario, it defines a simple way to support TLV data in
> all message types.
>
>
> The IETF datatracker status page for this draft is:
> https://datatracker.ietf.org/doc/draft-ietf-grow-bmp-tlv/
>
> There is also an htmlized version available at:
> https://datatracker.ietf.org/doc/html/draft-ietf-grow-bmp-tlv-10
>
> A diff from the previous version is available at:
> https://www.ietf.org/rfcdiff?url2=draft-ietf-grow-bmp-tlv-10
>
>
> Internet-Drafts are also available by rsync at rsync.ietf.org::internet-drafts
>
>
> _______________________________________________
> GROW mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/grow
_______________________________________________
GROW mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/grow
