Hi Renato,
> On Aug 11, 2023, at 9:41 PM, Renato Westphal <[email protected]> wrote: > > Em dom., 16 de jul. de 2023 às 22:37, Christian Hopps > <[email protected]> escreveu: >>> On Jul 13, 2023, at 17:23, Acee Lindem <[email protected]> wrote: >>> >>> Hi Renato, >>>> >>>> Lastly, this might just be a small nitpick of mine, but I don't think >>>> having a "length" leaf for all TLVs and Sub-TLVs adds much value. In >>>> my opinion, it's only relevant for unknown TLVs that couldn't be >>>> decoded; otherwise, it just adds unnecessary noise. If we take a look >>>> at the IS-IS model, for instance, we can see that it doesn't have a >>>> "length" leaf for the LSP TLVs and Sub-TLVs. >>> >>> I’ve removed the three "length" leaves that were fixed length. I left the >>> ones that were variable due to contained sub-TLVS. >>> Are you saying that these TLVs would be malformed if the length weren’t >>> correct? >> >> I wonder what one would do with these length values? Only "unknown" things >> have a value leaf >> >> `+--ro value? yang:hex-string` >> >> And in that case the length is given by the length of the hex-string. > > Exactly. I'd probably keep the "length" leaf for unknown TLVs/Sub-TLVs > to be consistent with other modules (including the OSPF base module). > But I don't see much point for known TLVs/Sub-TLVs. > >> I definitely think there could be value with having the length field when >> things are malformed i.e., where the `length` field in the [sub-]TLV >> disagrees with the actual length of the `yang:hex-string` value. In this >> case though we might want to call it out as `malformed-length` or something. > > That's an interesting suggestion. In practice, however, I think most > implementations will reject LSAs containing malformed TLVs/Sub-TLVs as > a security measure. Ok - I’ve relented and removed the TLV lengths for the TLVs/Sub-TLVs with the potential to have nested Sub-TLVs. I agree that including it isn’t the right way to debug LSA encoding issues. I also noted that we didn’t include lengths for TLVs with nested Sub-TLVs in RFC 9129. > >> On Jul 13, 2023, at 10:32, Renato Westphal <[email protected]> wrote: >> [snip] >> I think the "ospfv3-lsa-prefix" grouping from the base >> module could be reused since it's identical, except it doesn't include >> the "prefix-length" leaf. > > So, it turns out it wasn't a good idea to reuse that grouping from the > base module. The reason is that its "prefix-options" leaf-list uses an > identityref to "ospfv3-prefix-option", whereas the > "ietf-ospfv3-extended-lsa" module should use an identityref to > "ospfv3-e-prefix-option" instead (notice the "e-" in the middle). My > apologies for the bad suggestion ^^ I’ve restored the prior grouping only without prefix-length is already include in the ip-prefix type. These changes are in the -22 version that I just posted. Thanks, Acee > > Regards, > -- > Renato Westphal _______________________________________________ Lsr mailing list [email protected] https://www.ietf.org/mailman/listinfo/lsr
