Re: [Sidrops] WGLC = draft-ietf-sidrops-aspa-verification - ENDS 03/22/2023 (Mar 22 2023)

["Sriram, Kotikalapudi (Fed)" <[e-mail]>]

Hi Amir,

Thank you. Thanks also for the off-list discussion earlier to clarify several points between us. 
My comments are inline below.

 >I've reviewed this draft and recommend advancing it, I think its adoption
 >is relatively-easy and can have significant benefits - Kudos!
 >

Great to hear that. Thanks.

 >I have few comments, nothing critical though.
 >
 >1. section 2, change the language and say that an AS  _MAY_ export any
 >announcement, including from non-customers, to sibling AS. The current
 >language may be interpreted as if siblings necessarily export everything.
 >

OK, good. Will make that change in v-13.

 >2. section 4:  clarify that ASes A, B which are siblings MUST either both
 >register the other (in SPAS), or neither register the other in SPAS. I'm
 >concerned that if you don't clarify this condition, then we may have
 >deployments where A registers B in its SPAS but B doesn't register A in its
 >SPAS. Let C be  a provider of  A, and consider an announcement which A
 >receives from C and exports to B, which exports it to a customer D (who
 >adopts ASPA). Then D may (incorrectly)  filter this announcement (since it
 >appears to have been sent `up' from A to B).
 >

Yes, good point. Will clarify that in v-13.

 >3. 2. section 4: you refer to  an ASPA object `showing only AS 0 as a
 >provider AS' as an AS0 ASPA. It leaves the possibility of an ASPA object
 >showing as provider AS both AS 0 and another AS. I think you better forbid
 >this explicitly, i.e., AS 0 should only appear as the only provider AS (if
 >it appears at all).

OK, good catch. Will add appropriate wording for that.

 >
 >4.  section 4: you say that all ASes MUST have an ASPA. I think this should
 >be only SHOULD as we can't really require/force ASes to adopt a standard.
 >Or just say that this requirement MUST be met by ASes compliant with the
 >current specifications.
 >

Yes, will make this substitution:
s/ Any AS, including an RS AS, MUST have an ASPA. / 
A compliant AS, including a compliant RS AS, MUST have an ASPA. /

 >5. section 5: in the hop-check function, the value `Provider' should better
 >be renamed to allow for the case the AS(j) is a RS or sibling of AS(i).
 >Probably best to change the terms in ASPA_Profile too. I realize this
 >change is a bit problematic since what we mean is really something like
 >`effective/extended provider', and people are already used to it too, but
 >maybe something like eProvider or extProvider will work? or UPok? In any
 >case, using exactly the same term for two different concepts - even with
 >explanation - is bound to cause confusion and may result in implementation
 >errors.

As discussed off-list, your other suggestion 'Provider+' seems be the best term to use.

So in the equation for the hop-check function in Section 5:
s/Provider/ Provider+/

Will also add a paragraph before the equation in Section 5 as follows:

The term "Provider+" in the definition of the hop-check function 
is meant to encompass the possibilities of Provider, RS, or Sibling. 
An RS is effectively a Provider to its RS-client. Siblings regard 
each other as Provider and include each other in their 
respective SPAS (see Section 4).

Also, in Section 6, all the instances of "Provider" in association with
the hop-check function, hop(..), will be replaced with "Provider+".    

 >
 >and here are some even more minor comments (nitpicks):
 >
 >- in section 2, clarify that the relationships, except siblings, are
 >defined in RFC9234
 >- in section 4 add reference to definition of non-transparent RS AS (or
 >define)
 >- In 6.1, s/ AS(i)in/ AS(i) in/
 >- also  in 6.1: s/ For 2 <= i <= N, if there is an i/If there is an i s.t.
 >2 <= i <= N and/
 >- in 6.2, step 4, s/find the lowest value of v/find the lowest value of u/

All good catches. Will fix these.

Hopefully, my responses adequately address your concerns.

Sriram