First, I would like to thank the BMP authors and contributors for this very valuable work.
I have some feedback on BMP draft 07 from an implementation perspective. I've summarized my issues at top and go into a little more detail later. 1# general, Maximum and Inferred Lengths 2# Pg8 Peer Distinguisher .. ambiguity 3# PPH Peer Address field length ambiguity 4# 2038 problem 5# (Ed) Information Length on Pg10 6# (Ed) Type 1 Reason "not optional" vs "MUST" Pg11 7# Stats Count but no similar for Initiation 8# (Ed) Peer Down Notif should mention PPH inclusion Pg13 9# PD Notif inferred vs explicit length 10# Peer Up length of Local Address ambiguity 11# (Ed) Peer Up Remote Port field, s/fixed header/PPH/ Pg15 12# lengths not encoded, relies on BGP PDU This may appear to be a laundry list but I wanted to organize my remarks so they can be addressed easily. Here is some more detail on the above items. *1# general, Maximum and Inferred Lengths* No maximum length is defined for a BMP message. This is easy to perceive as a feature until you write a program to actually implement a BMP collector. Then you wish the spec provided a limit so you allocate the right size buffer for message assembly and parsing. The longest message that should be practical today is: sizeof(common_header) + sizeof(peer_peer_header) + sizeof(peer_up_notification_header) + (2*4096) Unless a lot of very big strings are arriving in Initiation messages, the longest practical message might contain two encapsulated BGP OPEN messages, plus the headers above. BGP might extend its maximum message size to 64KB in the future. Setting a maximum limit of around headers+(2*64K) may be wise. Any value would be better than none at all, requiring the implementation to choose an arbitrary buffer size or be prepared for a 2^32-1 length message. Inferred or calculated lengths are pervasive in BMP. This should be addressed by explicitly encoding the lengths of, for example, each encapsulated BGP PDU in the BMP protocol fields. The implementation will of course know how to parse the BGP messages, but it shouldn't rely on them for outer-protocol framing IMO. Again the Peer Up Notification is a good example. *2# Pg8 Peer Distinguisher .. ambiguity* It is unclear whether or not the Peer Distinguisher field is always allocated space in the PPH. The text says "this field is zero filled" if the peer is a non-L3VPN-CE peer, but the diagram says "present based on peer-type." The later language, in the diagram's description of that field, should be adjusted for clarity. *3# PPH Peer Address field length ambiguity* On Pg3 in Peer Address description, the text says "It is 4 bytes long if an IPv4 address is carried in this field (with most significant bytes zero filled) ..." This could be more clear. *4# 2038 problem* The timestamp encoding should allocate additional space for a 64-bit seconds value to avoid a 2038 problem. See http://en.wikipedia.org/wiki/2038_problem *5# (Ed) Information Length on Pg10* Information Length field description says "The length of *the following*Information field" and the description of Information (variable) does indeed follow it. However, this positional reference struck me as odd at first. *6# (Ed) Type 1 Reason "not optional" vs "MUST" Pg11* Replace "Inclusion of this TLV is not optional" with something that says MUST. For example, "This TLV MUST be included." *7# Stats Count but no similar for Initiation* The statistics message has a nice Stats Count field. It would be nice if there was a similar field for Initiation messages. *8# (Ed) Peer Down Notif should mention PPH inclusion Pg13* The Peer Down Notification message introduction should mention that the per-peer header is included. Peer Up Notification mentions this, as do other message types, but Peer Down Notification forgets to state this. *9# PD Notif inferred vs explicit length* The length of Data in the Peer Down Notification should be encoded explicitly following the Reason field. *10# Peer Up length of Local Address ambiguity* Again I think the "4 bytes long if an IPv4 address is carried" is a little ambiguous. Also the structure of that sentence has room for improvement! *11# (Ed) Peer Up Remote Port field, s/fixed header/PPH/ Pg15* Change mention of "fixed header" to "per-peer header." *12# lengths not encoded, relies on BGP PDU* I could gripe about this all day. Avoiding encoding data-field-lengths directly and explicitly saves a few bytes on the wire but often makes implementation or DEBUGGING more challenging. -- Jeff S Wheeler <[email protected]> Sr Network Operator / Innovative Network Concepts
_______________________________________________ GROW mailing list [email protected] https://www.ietf.org/mailman/listinfo/grow
