Em dom., 13 de ago. de 2023 às 18:16, Acee Lindem
<[email protected]> escreveu:
>
> 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.

Hi Acee,

I'm back from a brief vacation. Thank you for the updated draft, it's
looking great!

Cheers,
-- 
Renato Westphal

_______________________________________________
Lsr mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/lsr

Reply via email to