Re: AD Review of draft-ietf-rtgwg-policy-model-27

[Yingzhen Qu <[e-mail]>]

Hi Alvaro,

Thanks for the review. Considering these are minor comments, if you don’t mind I’ll make the changes in next revision with other comments received later.

Please see my answers inline.

Thanks,
Yingzhen

> On Jun 7, 2021, at 3:43 PM, Alvaro Retana <[e-mail]> wrote:
> 
> On June 7, 2021 at 2:23:50 PM, Yingzhen Qu wrote:
> 
> 
> Yingzhen:
> 
> Hi!
> 
> ...
>> We’ve published a new version to address your comments. Please see inline
>> below for detailed answers.
> 
> I have a couple of minor comments below.   I'm starting the IETF LC.
> 
> Thanks!!
> 
> Alvaro.
> 
> 
> 
> ...
>>> 381 +--rw set-preference? uint16
>>> 
>>> [?] What is the preference? Is this intended to be related to
>>> "administrative distance", or local-pref, or what? Or maybe it is
>>> something completely different.
>>> 
>> [Yingzhen]: added description in the model, consistent with RFC 8349.
>> leaf set-preference {
> 
> Ahhh, that's where it comes from.  Why not call it "route-preference”?
> 
[Yingzhen]: we can change the leaf to “set-route-preference”. 
> 
> ...
>>> 382 +--rw set-tag? tag-type
>>> 383 +--rw set-application-tag? tag-type
>>> 
>>> [?] What is an application tag? What is the difference between that an a
>>> tag?
>>> 
>> [Yingzhen]: added description. Hope this clarifies.
> 
> Hmmm..so, is that a local thing?  If it is not advertised...
> 
> The name "application" is what's throwing me off.
> 
> 
> 
> ...
>>> 420 5. Policy evaluation
>>> 
>>> 422 Evaluation of each policy definition proceeds by evaluating its
>>> 423 corresponding individual policy statements in order. When all the
>>> 424 condition statements in a policy statement are satisfied, the
>>> 425 corresponding action statements are executed. If the actions include
>>> 426 either accept-route or reject-route actions, evaluation of the
>>> 427 current policy definition stops, and no further policy statement is
>>> 428 evaluated. If there are multiple policies in the policy chain,
>>> 429 subsequent policies are not evaluated. Policy chains are sequences
>>> 430 of policy definitions (described in . (Section 4)).
>>> 
>>> [nit] s/described in . (Section 4)/as described in Section 4
>>> 
>> [Yingzhen]: fixed.
> 
> s/in . (Section 4)/in Section 4
[Yingzhen]: will fix this.
> 
> 
> ...
>>> 980 typedef metric-modification-type {
>>> 981 type enumeration {
>>> 982 enum set-metric {
>>> 983 description
>>> 984 "Set the metric to the specified value.";
>>> 985 }
>>> 986 enum add-metric {
>>> 987 description
>>> 988 "Add the specified value to the existing metric.
>>> 989 If the result would overflow the maximum metric
>>> 990 (0xffffffff), set the metric to the maximum.";
>>> 
>>> [major] Not all metrics have the same length! How should shorter
>>> metric fields be handled? Is that expected to be solved by an
>>> augmentation, if/when needed?
>>> 
>> [Yingzhen]: if a metric is shorter, say only 0xff, and a value is set out of
>> range, the implementation should reject it. It’s set to maximum here for
>> simplicity.
> 
> Sure...but that is not what the text says.  The text is only specific
> about 0xffffffff.  I think that if you take "(0xffffffff)" then it
> would be a general statement.
[Yingzhen]: will add some description for shorter range.