Subject: [secdir] Secdir early review of draft-ietf-idr-bgp-ls-app-specific-attr-05

From: Paul Wouters via Datatracker <[email-masked]>
Date: Thu, 20 May 2021 19:35:32 -0700

Reviewer: Paul Wouters
Review result: Has Nits

I have reviewed this document as part of the security directorate's  ongoing
effort to review all IETF documents being processed by the  IESG.  These
comments were written primarily for the benefit of the  security area
directors.  Document editors and WG chairs should treat  these comments just
like any other last call comments.

The summary of the review is Has Nits

In section 3 it lists 1092 as "TE Metric" but RFC 7522 lists it as "TE Default
Metric". And 1116 is listed as "Unidirectional link delay variation" but RFC
8571 lists it as "Unidirectional Delay Variation". And 1117 shows "packet loss"
vs "link loss". There are more subtle differences.  Maybe ensure these terms
are synced up better, unless there is a reason these terms are
different/updated ?

      Link attributes that do not have application-specific semantics SHOULD
      NOT be advertised within the ASLA TLV.

Is there a reason why this is SHOULD NOT and not MUST NOT? In other words, do
you have an example of where the SHOULD NOT would not apply? And if so, should
that be mentioned here? Same for the following SHOULD case,

In section 4, "[RFC8920] " appears without an actual link. It's either missing
the xref target, or there is a bug in rendering the xml to html?

grammar nit:
CURRENT:  They were originally defined
PROPOSED: These were originally defined

---

From: "Ketan Talaulikar (ketant)" <[email-masked]>
Date: Fri, 21 May 2021 12:17:07 +0000

Hi Paul,

Thanks for your review and please check inline below for responses.

-----Original Message-----
From: Paul Wouters via Datatracker <[email-masked]> 
Sent: 21 May 2021 08:06
To: [email-masked]
Cc: [email-masked]; [email-masked]
Subject: Secdir early review of draft-ietf-idr-bgp-ls-app-specific-attr-05

Reviewer: Paul Wouters
Review result: Has Nits

I have reviewed this document as part of the security directorate's  ongoing effort to review all IETF documents being processed by the  IESG.  These comments were written primarily for the benefit of the  security area directors.  Document editors and WG chairs should treat  these comments just like any other last call comments.

In section 3 it lists 1092 as "TE Metric" but RFC 7522 lists it as "TE Default Metric". And 1116 is listed as "Unidirectional link delay variation" but RFC
8571 lists it as "Unidirectional Delay Variation". And 1117 shows "packet loss"
vs "link loss". There are more subtle differences.  Maybe ensure these terms are synced up better, unless there is a reason these terms are different/updated ?
[KT] Good catch! Fixed them all.

Is there a reason why this is SHOULD NOT and not MUST NOT? In other words, do you have an example of where the SHOULD NOT would not apply? And if so, should that be mentioned here? 
Same for the following SHOULD case,
[KT] Thinking over it now, I do not see a reason for the SHOULD. Will change it to MUST unless someone believes it needs to stay as it is.

In section 4, "[RFC8920] " appears without an actual link. It's either missing the xref target, or there is a bug in rendering the xml to html?
[KT] Looks like an xml2rfc rendering issue.

grammar nit:
[KT] Ack.

Thanks,
Ketan