Re: [GROW] Comment on draft-ietf-grow-bmp-tlv

[Paolo Lucente <[e-mail]>]

Hi Dhananjay,

Thanks very much for your feedback, let me address it in-line:

On 10/1/24 12:05, Dhananjay Patki (dhpatki) wrote:
> 
> 1. It would help to introduce the format of Group TLV in Section 4.2.1; 
> just like how the format of the indexed TLV is introduced in Section 3. 
> Currently, group_index is learnt only from the example in section 4.2.1.1.

Ack, will do!


> 2. In a group TLV, ‘index’ and ‘group_index’ are 2 different fields. 
> ‘index’ is redundant (always set to 0). Index=0 means that the TLV 
> applies to all NLRIs which is not true in case of Group TLV.
> 
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>        |            type=TBD2            |         length=0x000a       |
> 
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> *      |             index=0             |      group_index=0x000c     |*
> 
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>        |                  value={0x0004,   0x0005,                     |
> 
>        |                         0x0006} |
> 
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> This could be avoided by using the index space itself for group_index, 
> but differentiated using a bit called ‘G bit’. This bit, when set, would 
> mean that the index is a group index.
> 
> Format:
> 
>        0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> 
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>        |        Type (2 octets)        |     Length (2 octets)         |
> 
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> *      |G|      Index (15 bits)        |*
> 
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>        ~                      Value (variable)                         ~
> 
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> Example:
> 
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>        |            type=TBD2            |         length=0x000a       |
> 
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
> *      |1|         index=0x000c          |*
> 
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 
>        |                  value={0x0004,   0x0005,                     |
> 
>        |                         0x0006} |
> 
>        +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

At the last meeting in Prague, I did end up discussing with a couple of 
people in the hallway precisely this! Without a Group Bit / G-bit one 
would need to know upfront how many items are going to be packed in the 
message before assigning the index to the Group; it would be much more 
helpful to have a separate count for groups, precisely using a G-bit.

So consider the above being on my todo list and coming up in the next 
revision of the draft; i will work on the text for the new revision next 
week.

Agree also that the first index set to zero is redundant. Defining the 
Group Index satisfies the constraint expressed before in the document 
where every TLV in a Route Monitoring message should be indexed.


> 3. Section 4.2.1 says “One or more 2 bytes indexes ..”. Shouldn’t this 
> be “Two or more 2 byte indexes ..”? If the TLV pertains to a single 
> NLRI, Group TLV is not necessary.

Valid point, indeed, i will fix that.

Paolo