Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Kristian Larsson]

Gentlemen,

... snip snip...
On 2018-03-09 01:40, Mahesh Jethanandani wrote:
Choice of source port definition using range/operator or referring to a 
group of source ports, to be added as a future 'case' statement.



 - ditto for "or referring to a group of destination ports."
 - ditto on both of the above for the "udp" container
 - is it possible for both "egress-interface" and "ingress-interface" 
leafs to
   be specified at the same time?  - if not, should there a 'must' 
statement to
   prevent that possibility? - or an explanation for what happens if 
it occurs?


Let me discuss this with my co-authors.


It is possible to match both egress-interface and ingress-interface in 
the same ACL. Different devices support different type of attachment 
points for ACLs. Most routers, like an ASR9k or Juniper MX, supports 
attaching ACLs on interfaces in either ingress or egress direction. If 
we apply ACL FOO ingress on interface BAR then it would perhaps be weird 
to use the ingress-interface match in the FOO ACL since 
ingress-interface will always be BAR for every packet evaluated. Using 
egress-interface would make a lot more sense (and we will know the 
egress-interface if the platform performs the route lookup before 
evluating the ACL which is an implementation issue). We could introduce 
a must constraint to avoid a silly situation but I don't think that's a 
real improvement on the model.


Above all, considering the other type of attachment, which we find among 
others on Linux with iptables or nftables, which is a sort of "global" 
attachment point, it makes sense to be able to specify both. nftables 
rules are not written and attached to a particular interface but rather 
end up in a central list of rules and so it makes sense being able to 
write individual rules that match on ingress-interface, egress-interface 
or both (or none). A must constraint would make that impossible, so 
please don't add a must.


Kind regards,
   Kristian.

___
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod

Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Kent Watsen]

Hi Mahesh,

Two instances of <<>> below.

Kent // shepherd


On 3/14/18, 3:27 PM, "Mahesh Jethanandani" 
> wrote:




On Mar 14, 2018, at 10:42 AM, Kent Watsen 
> wrote:

Hi Mahesh,  please look for <> below.

All, please take a look at the question around renaming the "access-lists" 
container.

Thanks,
Kent



On 3/13/18, 9:46 PM, "Mahesh Jethanandani" 
> wrote:





On Mar 13, 2018, at 3:23 PM, Kent Watsen 
> wrote:

Hi Mahesh,

Please look for  below.

Thanks,
Kent


On 3/8/18, 7:40 PM, "Mahesh Jethanandani" 
> wrote:

Kent,




On Mar 7, 2018, at 1:55 PM, Kent Watsen 
> wrote:

[To all those that said this draft was ready, really?]


Hi Mahesh,

Thanks for the update.  I found some more issues.  Some must be fixed,
others are nits, and might be caught by the RFC Editor.  But I think
that it's embarrassing to receive comments for such things from the
IESG, as is recently the case for the syslog draft, so please see
what you can do.

Thanks,
Kent


From Idnits:

 ** There are 6 instances of too long lines in the document, the longest one
being 7 characters in excess of 72.

Hmm. The idnits at submission time did not complain. Will apply the new script 
that you provided to make sure I wrap them around.





 You wrote before that it was "Fixed", but it's still here?  Note: "**" is
 an error (idnits label)

 -- The document has examples using IPv4 documentation addresses according
to RFC6890, but does not use any IPv6 documentation addresses.  Maybe
there should be IPv6 examples, too?

 I don't feel strongly about this, but if it's easy enough to do...

In the Abstract:
 - I think the word "an" is missing (e.g., an ACL)

Added.





In the Introduction:
 - should "ordered-by-user" be "ordered-by user" to avoid confusion, or perhaps 
say it another way?

How about this in both the Abstract and the Introduction.

OLD:

ACL is a ordered-by-user set of rules

NEW:
An ACL is a set of rules, in an order set by the user

or how about "An ACL is a user-ordered set of rules”?

Ok.







 - what does "a tuple of" mean?  Can this be restated?

How about this?

OLD:

The match criteria consist of a tuple of packet header match criteria and can 
have metadata match criteria as well.

NEW:

The match criteria consist of packet header matches, and or or metadata as 
described below:

or how about "The match criteria can be a multiplicity of criteria, all 
of which must be true for the match to occur.   The match criteria may match 
against values in the packet header or against vendor-specific metadata about 
the packet."?   - or something in between?

Or simply as:

“The match criteria allows for definition of packet headers and metadata, all 
of which must be true for the match to occur."

<> okay



 - s/In case vendor supports it/In case a vendor supports it/ ?

Ok.




 - "The list of X is endless depending on...".  Is "endless" the right word, 
perhaps restate?
OLD:

The list of potential actions is endless


NEW:

The list of potential actions is limitless

 or maybe "unbounded”?

Ok.





 - same sentence as above, should "networked devices" be "network" or 
"networking" devices?

Will change “networked devices” to “networking devices”.





In Section 3:
 - "A network system usually have a list of ACLs"  (s/system/systems/ or 
s/have/has/?)

s/have/has/.




 - "The match criteria consist of packet header matching" - is consist the 
right word?

How would you restate it? (After I have s/consist/consists/)

 see above (my comment before last, it is the same sentence, right?)

Once we agree on the above comment, I will replicate it.

<> okay.




 - "It as also possible for ACE to match on metadata"  s/as/is/ and s/ACE/an 
ACE/

Ok




 - "When applied to interfaces of a networked device, the ACL is applied in a 
direction
which indicates if it should be applied to packet entering (input) or 
leaving the
device (output)."  - restate to talk about "ingress" and "egress”?

How about:

When applied to interfaces of a networked device, the ACL is applied in a 
direction which indicates if it should be applied to ingress interface (input) 
or egress interface (output).

or maybe "When applied to interfaces of a networked device, distinct ACLs 
are defined for the ingress (input) and egress (output) directions.”

Ok.





 - "An example in the appendix shows how to express it in YANG model." - either 
this
   is not true, or the sentence should not be at the end of this paragraph

Removed.





In Section 3.1:
 - s/and must statements/and 'must' statements/

Done and s/if-feature/‘if-feature’/




 - s/define new "matches" choice/define a new "matches" choice/ ?

Done.





In Section 4.1:
 

Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Mahesh Jethanandani]

> On Mar 14, 2018, at 10:42 AM, Kent Watsen  wrote:
> 
> Hi Mahesh,  please look for <> below.
>  
> All, please take a look at the question around renaming the "access-lists" 
> container.
>  
> Thanks,
> Kent
>  
>  
>  
> On 3/13/18, 9:46 PM, "Mahesh Jethanandani"  > wrote:
>  
>  
> 
> 
>> On Mar 13, 2018, at 3:23 PM, Kent Watsen > > wrote:
>>  
>> Hi Mahesh,
>>  
>> Please look for  below.
>>  
>> Thanks,
>> Kent
>>  
>>  
>> On 3/8/18, 7:40 PM, "Mahesh Jethanandani" > > wrote:
>>  
>> Kent,
>> 
>> 
>> 
>>> On Mar 7, 2018, at 1:55 PM, Kent Watsen >> > wrote:
>>>  
>>> [To all those that said this draft was ready, really?]
>>> 
>>> 
>>> Hi Mahesh,
>>> 
>>> Thanks for the update.  I found some more issues.  Some must be fixed, 
>>> others are nits, and might be caught by the RFC Editor.  But I think
>>> that it's embarrassing to receive comments for such things from the 
>>> IESG, as is recently the case for the syslog draft, so please see 
>>> what you can do.
>>> 
>>> Thanks,
>>> Kent
>>> 
>>> 
>>> From Idnits:
>>> 
>>>  ** There are 6 instances of too long lines in the document, the longest one
>>> being 7 characters in excess of 72.
>>  
>> Hmm. The idnits at submission time did not complain. Will apply the new 
>> script that you provided to make sure I wrap them around.
>> 
>> 
>> 
>>> 
>>>  You wrote before that it was "Fixed", but it's still here?  Note: "**" is
>>>  an error (idnits label)
>>> 
>>>  -- The document has examples using IPv4 documentation addresses according
>>> to RFC6890, but does not use any IPv6 documentation addresses.  Maybe
>>> there should be IPv6 examples, too?
>>> 
>>>  I don't feel strongly about this, but if it's easy enough to do...
>>> 
>>> In the Abstract:
>>>  - I think the word "an" is missing (e.g., an ACL)
>>  
>> Added.
>> 
>> 
>> 
>>> 
>>> In the Introduction:
>>>  - should "ordered-by-user" be "ordered-by user" to avoid confusion, or 
>>> perhaps say it another way?
>>  
>> How about this in both the Abstract and the Introduction.
>>  
>> OLD:
>> ACL is a ordered-by-user set of rules
>>  
>> NEW:
>> An ACL is a set of rules, in an order set by the user
>>  
>> or how about "An ACL is a user-ordered set of rules”?
>  
> Ok.
> 
> 
>> 
>> 
>>  
>>>  - what does "a tuple of" mean?  Can this be restated?
>>  
>> How about this?
>>  
>> OLD:
>> The match criteria consist of a tuple of packet header match criteria and 
>> can have metadata match criteria as well.
>>  
>> NEW:
>> The match criteria consist of packet header matches, and or or metadata as 
>> described below:
>>  
>> or how about "The match criteria can be a multiplicity of criteria, 
>> all of which must be true for the match to occur.   The match criteria may 
>> match against values in the packet header or against vendor-specific 
>> metadata about the packet."?   - or something in between?
>  
> Or simply as:
>  
> “The match criteria allows for definition of packet headers and metadata, all 
> of which must be true for the match to occur."
> 
> <> okay
>>  
>>  
>>  
>>>  - s/In case vendor supports it/In case a vendor supports it/ ?
>>  
>> Ok.
>> 
>> 
>> 
>>>  - "The list of X is endless depending on...".  Is "endless" the right 
>>> word, perhaps restate?
>> OLD:
>> The list of potential actions is endless
>>  
>> NEW:
>> The list of potential actions is limitless
>>  
>>  or maybe "unbounded”?
>  
> Ok.
> 
> 
>>  
>>  
>>>  - same sentence as above, should "networked devices" be "network" or 
>>> "networking" devices?
>>  
>> Will change “networked devices” to “networking devices”.
>> 
>> 
>> 
>>> 
>>> In Section 3:
>>>  - "A network system usually have a list of ACLs"  (s/system/systems/ or 
>>> s/have/has/?)
>>  
>> s/have/has/.
>> 
>> 
>> 
>>>  - "The match criteria consist of packet header matching" - is consist the 
>>> right word?
>>  
>> How would you restate it? (After I have s/consist/consists/)
>>  
>>  see above (my comment before last, it is the same sentence, right?)
>  
> Once we agree on the above comment, I will replicate it.
> 
> <> okay.
>> 
>> 
>> 
>>>  - "It as also possible for ACE to match on metadata"  s/as/is/ and 
>>> s/ACE/an ACE/
>>  
>> Ok
>> 
>> 
>> 
>>>  - "When applied to interfaces of a networked device, the ACL is applied in 
>>> a direction
>>> which indicates if it should be applied to packet entering (input) or 
>>> leaving the
>>> device (output)."  - restate to talk about "ingress" and "egress”?
>>  
>> How about:
>>  
>> When applied to interfaces of a networked device, the ACL is applied in a 
>> direction which indicates if it should be applied to ingress interface 
>> (input) or egress interface (output).
>> 
>> or maybe "When applied to interfaces of a networked device, distinct 
>> ACLs are 

Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Eliot Lear]

On 14.03.18 10:42, Kent Watsen wrote:
>
> BTW, why is the container called "access-lists" and not
> e.g., "acls".  I thought that there was a node-naming idiom along
> the lines of "/widgets/widget" for when a list is a descendent of
> a container.
>
>  
>
> History. When we inherited the draft, it was named access-lists. I can
> change it to “acls”.
>
>
> <> I think it should be "acls", but I wouldn't want to make this
> change unilaterally. 
>
> Does anybody else have an issue with the container being called
> "access-lists" ?
>

I would be opposed to a change at this point.  It doesn't change
functionality and we're not saving that many characters.  What is there
is clear.  Please can we just leave this alone.


Eliot



signature.asc
Description: OpenPGP digital signature
___
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod

Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Kent Watsen]

Hi Mahesh,  please look for <> below.

All, please take a look at the question around renaming the "access-lists" 
container.

Thanks,
Kent



On 3/13/18, 9:46 PM, "Mahesh Jethanandani" 
> wrote:




On Mar 13, 2018, at 3:23 PM, Kent Watsen 
> wrote:

Hi Mahesh,

Please look for  below.

Thanks,
Kent


On 3/8/18, 7:40 PM, "Mahesh Jethanandani" 
> wrote:

Kent,



On Mar 7, 2018, at 1:55 PM, Kent Watsen 
> wrote:

[To all those that said this draft was ready, really?]


Hi Mahesh,

Thanks for the update.  I found some more issues.  Some must be fixed,
others are nits, and might be caught by the RFC Editor.  But I think
that it's embarrassing to receive comments for such things from the
IESG, as is recently the case for the syslog draft, so please see
what you can do.

Thanks,
Kent


From Idnits:

 ** There are 6 instances of too long lines in the document, the longest one
being 7 characters in excess of 72.

Hmm. The idnits at submission time did not complain. Will apply the new script 
that you provided to make sure I wrap them around.




 You wrote before that it was "Fixed", but it's still here?  Note: "**" is
 an error (idnits label)

 -- The document has examples using IPv4 documentation addresses according
to RFC6890, but does not use any IPv6 documentation addresses.  Maybe
there should be IPv6 examples, too?

 I don't feel strongly about this, but if it's easy enough to do...

In the Abstract:
 - I think the word "an" is missing (e.g., an ACL)

Added.




In the Introduction:
 - should "ordered-by-user" be "ordered-by user" to avoid confusion, or perhaps 
say it another way?

How about this in both the Abstract and the Introduction.

OLD:

ACL is a ordered-by-user set of rules

NEW:
An ACL is a set of rules, in an order set by the user

or how about "An ACL is a user-ordered set of rules”?

Ok.





 - what does "a tuple of" mean?  Can this be restated?

How about this?

OLD:

The match criteria consist of a tuple of packet header match criteria and can 
have metadata match criteria as well.

NEW:

The match criteria consist of packet header matches, and or or metadata as 
described below:

or how about "The match criteria can be a multiplicity of criteria, all 
of which must be true for the match to occur.   The match criteria may match 
against values in the packet header or against vendor-specific metadata about 
the packet."?   - or something in between?

Or simply as:

“The match criteria allows for definition of packet headers and metadata, all 
of which must be true for the match to occur."

<> okay



 - s/In case vendor supports it/In case a vendor supports it/ ?

Ok.



 - "The list of X is endless depending on...".  Is "endless" the right word, 
perhaps restate?
OLD:

The list of potential actions is endless


NEW:

The list of potential actions is limitless

 or maybe "unbounded”?

Ok.




 - same sentence as above, should "networked devices" be "network" or 
"networking" devices?

Will change “networked devices” to “networking devices”.




In Section 3:
 - "A network system usually have a list of ACLs"  (s/system/systems/ or 
s/have/has/?)

s/have/has/.



 - "The match criteria consist of packet header matching" - is consist the 
right word?

How would you restate it? (After I have s/consist/consists/)

 see above (my comment before last, it is the same sentence, right?)

Once we agree on the above comment, I will replicate it.

<> okay.



 - "It as also possible for ACE to match on metadata"  s/as/is/ and s/ACE/an 
ACE/

Ok



 - "When applied to interfaces of a networked device, the ACL is applied in a 
direction
which indicates if it should be applied to packet entering (input) or 
leaving the
device (output)."  - restate to talk about "ingress" and "egress”?

How about:

When applied to interfaces of a networked device, the ACL is applied in a 
direction which indicates if it should be applied to ingress interface (input) 
or egress interface (output).

or maybe "When applied to interfaces of a networked device, distinct ACLs 
are defined for the ingress (input) and egress (output) directions.”

Ok.




 - "An example in the appendix shows how to express it in YANG model." - either 
this
   is not true, or the sentence should not be at the end of this paragraph

Removed.




In Section 3.1:
 - s/and must statements/and 'must' statements/

Done and s/if-feature/‘if-feature’/



 - s/define new "matches" choice/define a new "matches" choice/ ?

Done.




In Section 4.1:
 - "ietf-access-control-list" is the standard top level module for access lists
 - what does this mean?

OLD:
"ietf-access-control-list" is the standard top level module for access lists

NEW:
"ietf-access-control-list" is the top level module for access lists

 it's more than the word 

Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Eliot Lear]

On 14.03.18 10:14, Mahesh Jethanandani wrote:
> The description under the ‘matches’ container says:
>
> If no matches are defined in a particular container,
> then any packet will match that container. If no
> matches are specified at all in an ACE, then any
> packet will match the ACE.
>

Right.  And that's good.

Eliot
>
>>
>> Eliot
>>
>
> Mahesh Jethanandani
> mjethanand...@gmail.com 
>



signature.asc
Description: OpenPGP digital signature
___
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod

Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Mahesh Jethanandani]

> On Mar 14, 2018, at 8:28 AM, Eliot Lear  wrote:
> 
> Hi Mahesh,
> 
> Just one point.
> 
> On 13.03.18 18:46, Mahesh Jethanandani wrote:
>>> or how about "The match criteria can be a multiplicity of criteria, 
>>> all of which must be true for the match to occur.   The match criteria may 
>>> match against values in the packet header or against vendor-specific 
>>> metadata about the packet."?   - or something in between?
>> 
>> Or simply as:
>> 
>> “The match criteria allows for definition of packet headers and metadata, 
>> all of which must be true for the match to occur."
> 
> So long as we make clear what the null set means.  To me, that's "match 
> everything”.

The description under the ‘matches’ container says:

If no matches are defined in a particular container,
then any packet will match that container. If no
matches are specified at all in an ACE, then any
packet will match the ACE.


> 
> Eliot
> 

Mahesh Jethanandani
mjethanand...@gmail.com

___
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod

Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Eliot Lear]

Hi Mahesh,

Just one point.


On 13.03.18 18:46, Mahesh Jethanandani wrote:
>> or how about "The match criteria can be a multiplicity of
>> criteria, all of which must be true for the match to occur.   The
>> match criteria may match against values in the packet header or
>> against vendor-specific metadata about the packet."?   - or something
>> in between?
>
> Or simply as:
>
> “The match criteria allows for definition of packet headers and
> metadata, all of which must be true for the match to occur."

So long as we make clear what the null set means.  To me, that's "match
everything".

Eliot



signature.asc
Description: OpenPGP digital signature
___
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod

Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Mahesh Jethanandani]

> On Mar 13, 2018, at 3:23 PM, Kent Watsen  wrote:
> 
> Hi Mahesh,
>  
> Please look for  below.
>  
> Thanks,
> Kent
>  
>  
> On 3/8/18, 7:40 PM, "Mahesh Jethanandani"  > wrote:
>  
> Kent,
> 
> 
>> On Mar 7, 2018, at 1:55 PM, Kent Watsen > > wrote:
>>  
>> [To all those that said this draft was ready, really?]
>> 
>> 
>> Hi Mahesh,
>> 
>> Thanks for the update.  I found some more issues.  Some must be fixed, 
>> others are nits, and might be caught by the RFC Editor.  But I think
>> that it's embarrassing to receive comments for such things from the 
>> IESG, as is recently the case for the syslog draft, so please see 
>> what you can do.
>> 
>> Thanks,
>> Kent
>> 
>> 
>> From Idnits:
>> 
>>  ** There are 6 instances of too long lines in the document, the longest one
>> being 7 characters in excess of 72.
>  
> Hmm. The idnits at submission time did not complain. Will apply the new 
> script that you provided to make sure I wrap them around.
> 
> 
>> 
>>  You wrote before that it was "Fixed", but it's still here?  Note: "**" is
>>  an error (idnits label)
>> 
>>  -- The document has examples using IPv4 documentation addresses according
>> to RFC6890, but does not use any IPv6 documentation addresses.  Maybe
>> there should be IPv6 examples, too?
>> 
>>  I don't feel strongly about this, but if it's easy enough to do...
>> 
>> In the Abstract:
>>  - I think the word "an" is missing (e.g., an ACL)
>  
> Added.
> 
> 
>> 
>> In the Introduction:
>>  - should "ordered-by-user" be "ordered-by user" to avoid confusion, or 
>> perhaps say it another way?
>  
> How about this in both the Abstract and the Introduction.
>  
> OLD:
> ACL is a ordered-by-user set of rules
>  
> NEW:
> An ACL is a set of rules, in an order set by the user
>  
> or how about "An ACL is a user-ordered set of rules”?

Ok.

> 
>  
>>  - what does "a tuple of" mean?  Can this be restated?
>  
> How about this?
>  
> OLD:
> The match criteria consist of a tuple of packet header match criteria and can 
> have metadata match criteria as well.
>  
> NEW:
> The match criteria consist of packet header matches, and or or metadata as 
> described below:
>  
> or how about "The match criteria can be a multiplicity of criteria, all 
> of which must be true for the match to occur.   The match criteria may match 
> against values in the packet header or against vendor-specific metadata about 
> the packet."?   - or something in between?

Or simply as:

“The match criteria allows for definition of packet headers and metadata, all 
of which must be true for the match to occur."

>  
>  
>  
>>  - s/In case vendor supports it/In case a vendor supports it/ ?
>  
> Ok.
> 
> 
>>  - "The list of X is endless depending on...".  Is "endless" the right word, 
>> perhaps restate?
> OLD:
> The list of potential actions is endless
>  
> NEW:
> The list of potential actions is limitless
>  
>  or maybe "unbounded”?

Ok.

>  
>  
>>  - same sentence as above, should "networked devices" be "network" or 
>> "networking" devices?
>  
> Will change “networked devices” to “networking devices”.
> 
> 
>> 
>> In Section 3:
>>  - "A network system usually have a list of ACLs"  (s/system/systems/ or 
>> s/have/has/?)
>  
> s/have/has/.
> 
> 
>>  - "The match criteria consist of packet header matching" - is consist the 
>> right word?
>  
> How would you restate it? (After I have s/consist/consists/)
>  
>  see above (my comment before last, it is the same sentence, right?)

Once we agree on the above comment, I will replicate it.

> 
> 
>>  - "It as also possible for ACE to match on metadata"  s/as/is/ and s/ACE/an 
>> ACE/
>  
> Ok
> 
> 
>>  - "When applied to interfaces of a networked device, the ACL is applied in 
>> a direction
>> which indicates if it should be applied to packet entering (input) or 
>> leaving the
>> device (output)."  - restate to talk about "ingress" and "egress”?
>  
> How about:
>  
> When applied to interfaces of a networked device, the ACL is applied in a 
> direction which indicates if it should be applied to ingress interface 
> (input) or egress interface (output).
> 
> or maybe "When applied to interfaces of a networked device, distinct 
> ACLs are defined for the ingress (input) and egress (output) directions.”

Ok.

>  
>  
>>  - "An example in the appendix shows how to express it in YANG model." - 
>> either this
>>is not true, or the sentence should not be at the end of this paragraph
>  
> Removed.
> 
> 
>> 
>> In Section 3.1:
>>  - s/and must statements/and 'must' statements/
>  
> Done and s/if-feature/‘if-feature’/
> 
> 
>>  - s/define new "matches" choice/define a new "matches" choice/ ?
>  
> Done.
> 
> 
>> 
>> In Section 4.1:
>>  - "ietf-access-control-list" is the standard top level module for access 
>> lists
>>  - what does this mean?
>  
> OLD:
> 

Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Kent Watsen]

Hi Mahesh,

Please look for  below.

Thanks,
Kent


On 3/8/18, 7:40 PM, "Mahesh Jethanandani" 
> wrote:

Kent,


On Mar 7, 2018, at 1:55 PM, Kent Watsen 
> wrote:

[To all those that said this draft was ready, really?]


Hi Mahesh,

Thanks for the update.  I found some more issues.  Some must be fixed,
others are nits, and might be caught by the RFC Editor.  But I think
that it's embarrassing to receive comments for such things from the
IESG, as is recently the case for the syslog draft, so please see
what you can do.

Thanks,
Kent


From Idnits:

 ** There are 6 instances of too long lines in the document, the longest one
being 7 characters in excess of 72.

Hmm. The idnits at submission time did not complain. Will apply the new script 
that you provided to make sure I wrap them around.



 You wrote before that it was "Fixed", but it's still here?  Note: "**" is
 an error (idnits label)

 -- The document has examples using IPv4 documentation addresses according
to RFC6890, but does not use any IPv6 documentation addresses.  Maybe
there should be IPv6 examples, too?

 I don't feel strongly about this, but if it's easy enough to do...

In the Abstract:
 - I think the word "an" is missing (e.g., an ACL)

Added.



In the Introduction:
 - should "ordered-by-user" be "ordered-by user" to avoid confusion, or perhaps 
say it another way?

How about this in both the Abstract and the Introduction.

OLD:

ACL is a ordered-by-user set of rules

NEW:
An ACL is a set of rules, in an order set by the user

or how about "An ACL is a user-ordered set of rules"?


 - what does "a tuple of" mean?  Can this be restated?

How about this?

OLD:

The match criteria consist of a tuple of packet header match criteria and can 
have metadata match criteria as well.

NEW:

The match criteria consist of packet header matches, and or or metadata as 
described below:

or how about "The match criteria can be a multiplicity of criteria, all 
of which must be true for the match to occur.   The match criteria may match 
against values in the packet header or against vendor-specific metadata about 
the packet."?   - or something in between?



 - s/In case vendor supports it/In case a vendor supports it/ ?

Ok.


 - "The list of X is endless depending on...".  Is "endless" the right word, 
perhaps restate?
OLD:

The list of potential actions is endless


NEW:

The list of potential actions is limitless

 or maybe "unbounded"?


 - same sentence as above, should "networked devices" be "network" or 
"networking" devices?

Will change “networked devices” to “networking devices”.



In Section 3:
 - "A network system usually have a list of ACLs"  (s/system/systems/ or 
s/have/has/?)

s/have/has/.


 - "The match criteria consist of packet header matching" - is consist the 
right word?

How would you restate it? (After I have s/consist/consists/)

 see above (my comment before last, it is the same sentence, right?)


 - "It as also possible for ACE to match on metadata"  s/as/is/ and s/ACE/an 
ACE/

Ok


 - "When applied to interfaces of a networked device, the ACL is applied in a 
direction
which indicates if it should be applied to packet entering (input) or 
leaving the
device (output)."  - restate to talk about "ingress" and "egress”?

How about:

When applied to interfaces of a networked device, the ACL is applied in a 
direction which indicates if it should be applied to ingress interface (input) 
or egress interface (output).

or maybe "When applied to interfaces of a networked device, distinct ACLs 
are defined for the ingress (input) and egress (output) directions."


 - "An example in the appendix shows how to express it in YANG model." - either 
this
   is not true, or the sentence should not be at the end of this paragraph

Removed.



In Section 3.1:
 - s/and must statements/and 'must' statements/

Done and s/if-feature/‘if-feature’/


 - s/define new "matches" choice/define a new "matches" choice/ ?

Done.



In Section 4.1:
 - "ietf-access-control-list" is the standard top level module for access lists
 - what does this mean?

OLD:
"ietf-access-control-list" is the standard top level module for access lists

NEW:
"ietf-access-control-list" is the top level module for access lists

 it's more than the word "standard".  Maybe something like this:  The 
"ietf-access-control-list"
module defines a container called "access-list"  - what do you think?

BTW, why is the container called "access-lists" and not e.g., "acls".  I 
thought that there was a node-naming idiom along the lines of "/widgets/widget" 
for when a list is a descendent of a container.



 - The "access-lists" container stores a list of "acl". - s/stores/has or 
contains?/

s/stores/has/


 - "...that can be used to determine which rule was matched upon" - not sure if 
this
   part is needed, or maybe better restated ", which can later be 

Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Mahesh Jethanandani]

Kent,

> On Mar 7, 2018, at 1:55 PM, Kent Watsen  wrote:
> 
> [To all those that said this draft was ready, really?]
> 
> 
> Hi Mahesh,
> 
> Thanks for the update.  I found some more issues.  Some must be fixed, 
> others are nits, and might be caught by the RFC Editor.  But I think
> that it's embarrassing to receive comments for such things from the 
> IESG, as is recently the case for the syslog draft, so please see 
> what you can do.
> 
> Thanks,
> Kent
> 
> 
> From Idnits:
> 
>  ** There are 6 instances of too long lines in the document, the longest one
> being 7 characters in excess of 72.

Hmm. The idnits at submission time did not complain. Will apply the new script 
that you provided to make sure I wrap them around.

> 
>  You wrote before that it was "Fixed", but it's still here?  Note: "**" is
>  an error (idnits label)
> 
>  -- The document has examples using IPv4 documentation addresses according
> to RFC6890, but does not use any IPv6 documentation addresses.  Maybe
> there should be IPv6 examples, too?
> 
>  I don't feel strongly about this, but if it's easy enough to do...
> 
> In the Abstract:
>  - I think the word "an" is missing (e.g., an ACL)

Added.

> 
> In the Introduction:
>  - should "ordered-by-user" be "ordered-by user" to avoid confusion, or 
> perhaps say it another way?

How about this in both the Abstract and the Introduction.

OLD:
ACL is a ordered-by-user set of rules

NEW:
An ACL is a set of rules, in an order set by the user

>  - what does "a tuple of" mean?  Can this be restated?

How about this?

OLD:
The match criteria consist of a tuple of packet header match criteria and can 
have metadata match criteria as well.

NEW:
The match criteria consist of packet header matches, and or or metadata as 
described below:

>  - s/In case vendor supports it/In case a vendor supports it/ ?

Ok.

>  - "The list of X is endless depending on...".  Is "endless" the right word, 
> perhaps restate?
OLD:
The list of potential actions is endless

NEW:
The list of potential actions is limitless

>  - same sentence as above, should "networked devices" be "network" or 
> "networking" devices?

Will change “networked devices” to “networking devices”.

> 
> In Section 3:
>  - "A network system usually have a list of ACLs"  (s/system/systems/ or 
> s/have/has/?)

s/have/has/.

>  - "The match criteria consist of packet header matching" - is consist the 
> right word?

How would you restate it? (After I have s/consist/consists/)

>  - "It as also possible for ACE to match on metadata"  s/as/is/ and s/ACE/an 
> ACE/

Ok

>  - "When applied to interfaces of a networked device, the ACL is applied in a 
> direction
> which indicates if it should be applied to packet entering (input) or 
> leaving the
> device (output)."  - restate to talk about "ingress" and "egress”?

How about:

When applied to interfaces of a networked device, the ACL is applied in a 
direction which indicates if it should be applied to ingress interface (input) 
or egress interface (output).

>  - "An example in the appendix shows how to express it in YANG model." - 
> either this
>is not true, or the sentence should not be at the end of this paragraph

Removed.

> 
> In Section 3.1:
>  - s/and must statements/and 'must' statements/

Done and s/if-feature/‘if-feature’/

>  - s/define new "matches" choice/define a new "matches" choice/ ?

Done.

> 
> In Section 4.1:
>  - "ietf-access-control-list" is the standard top level module for access 
> lists
>  - what does this mean?

OLD:
"ietf-access-control-list" is the standard top level module for access lists

NEW:
"ietf-access-control-list" is the top level module for access lists

>  - The "access-lists" container stores a list of "acl". - s/stores/has or 
> contains?/ 

s/stores/has/

>  - "...that can be used to determine which rule was matched upon" - not sure 
> if this
>part is needed, or maybe better restated ", which can later be used to 
> determine…"?

Ok.

>  - s/ability for ACL's to be/ability for ACLs to be/

Ok.

> 
> In Section 4.1 (in the YANG module):
>  - A number of identities read "ACL that primarily matches...".  Is 
> "primarily"
>an accurate word? - if so, then do we need to say anything about when it's
>not the case?

As one of the text says. It primarily matches IPv4, and does not match either 
ethernet or IPv6 headers. Such ACL types are different from the mixed ACL types 
that might match on a combination of ethernet and IPv4 headers etc.

>  Separately, s/ACL/an ACL/?

Ok.

>  - A number of features read "Device can support..." - s/Device/The device/?

Ok.

>  - "It can have one or more Access Control Lists" - lists should be singular.

Really? English grammar says that if a sentence has both a singular and a 
plural, the one nearest to the subject is the one you select.

>  - "An Access Control List(ACL)" - put a space before (ACL)

Ok.

>  - " Indicates the primary intended" - here's that 

Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Kent Watsen]

[To all those that said this draft was ready, really?]


Hi Mahesh,

Thanks for the update.  I found some more issues.  Some must be fixed, 
others are nits, and might be caught by the RFC Editor.  But I think
that it's embarrassing to receive comments for such things from the 
IESG, as is recently the case for the syslog draft, so please see 
what you can do.

Thanks,
Kent


>From Idnits:

  ** There are 6 instances of too long lines in the document, the longest one
 being 7 characters in excess of 72.

  You wrote before that it was "Fixed", but it's still here?  Note: "**" is
  an error (idnits label)

  -- The document has examples using IPv4 documentation addresses according
 to RFC6890, but does not use any IPv6 documentation addresses.  Maybe
 there should be IPv6 examples, too?

  I don't feel strongly about this, but if it's easy enough to do...

In the Abstract:
  - I think the word "an" is missing (e.g., an ACL)

In the Introduction:
  - should "ordered-by-user" be "ordered-by user" to avoid confusion, or 
perhaps say it another way?
  - what does "a tuple of" mean?  Can this be restated?
  - s/In case vendor supports it/In case a vendor supports it/ ?
  - "The list of X is endless depending on...".  Is "endless" the right word, 
perhaps restate?
  - same sentence as above, should "networked devices" be "network" or 
"networking" devices?

In Section 3:
  - "A network system usually have a list of ACLs"  (s/system/systems/ or 
s/have/has/?)
  - "The match criteria consist of packet header matching" - is consist the 
right word?
  - "It as also possible for ACE to match on metadata"  s/as/is/ and s/ACE/an 
ACE/
  - "When applied to interfaces of a networked device, the ACL is applied in a 
direction
 which indicates if it should be applied to packet entering (input) or 
leaving the
 device (output)."  - restate to talk about "ingress" and "egress"?
  - "An example in the appendix shows how to express it in YANG model." - 
either this
is not true, or the sentence should not be at the end of this paragraph

In Section 3.1:
  - s/and must statements/and 'must' statements/
  - s/define new "matches" choice/define a new "matches" choice/ ?

In Section 4.1:
  - "ietf-access-control-list" is the standard top level module for access lists
  - what does this mean?
  - The "access-lists" container stores a list of "acl". - s/stores/has or 
contains?/ 
  - "...that can be used to determine which rule was matched upon" - not sure 
if this
part is needed, or maybe better restated ", which can later be used to 
determine..."?
  - s/ability for ACL's to be/ability for ACLs to be/

In Section 4.1 (in the YANG module):
  - A number of identities read "ACL that primarily matches...".  Is "primarily"
an accurate word? - if so, then do we need to say anything about when it's
not the case?  Separately, s/ACL/an ACL/?
  - A number of features read "Device can support..." - s/Device/The device/?
  - "It can have one or more Access Control Lists" - lists should be singular.
  - "An Access Control List(ACL)" - put a space before (ACL)
  - " Indicates the primary intended" - here's that word "primary" again...
  - s/a list of access-list-entries(ACE)/ a list of access-list-entry nodes 
(ACE)/?
  - s/List of access list entries(ACE)/List of access list entry nodes (ACE)/?
  - there is more than one instance of this in the model
  - "../../../../type" - still some long relative XPaths
  - " or referring to a group of source ports" - this isn't there yet.  I think 
you
want to say something like "this is a choice so as to support future 'case'
statements, such as one enabling a group of source ports to be referenced"
  - ditto for "or referring to a group of destination ports."
  - ditto on both of the above for the "udp" container
  - is it possible for both "egress-interface" and "ingress-interface" leafs to 
be specified at the same time?  - if not, should there a 'must' statement to
prevent that possibility? - or an explanation for what happens if it occurs?
  - s/The ACL's applied/The ACLs applied/   (this happens more than once in 
model)

In Section 4.2:
  - references them by "uses" --> references them by 'uses' statements  ???
  - not all your 'reference' statements have the title of the referenced 
document.
  - "then the datagram must be destroyed" - s/destroyed/dropped/?
  - "or referring to a group of ..."  - same comments as for previous module
  - "ece" is missing a 'reference' statement?  - 
  - "Indicates that the Urgent pointer field is significant" - urgent is
capitalized, but there's no context as for why.  Perhaps missing a
reference statement too?
  - in "window-size" leaf description, remove parentheses

In Section 4.3:
  - the text says that it drops traffic from X to Y, but the example seems to do
the reverse.

In Section 4.4:
  - The "With the follow XML example:"  "This represents..." is 
difficult to read.  How about just 

Re: [netmod] I-D Action: draft-ietf-netmod-acl-model-17.txt

[Mahesh Jethanandani]

This version of the draft addresses comments raised during LC, shepherd review 
and other comments received during that period.

> On Mar 3, 2018, at 2:13 PM, internet-dra...@ietf.org wrote:
> 
> 
> A New Internet-Draft is available from the on-line Internet-Drafts 
> directories.
> This draft is a work item of the Network Modeling WG of the IETF.
> 
>Title   : Network Access Control List (ACL) YANG Data Model
>Authors : Mahesh Jethanandani
>  Lisa Huang
>  Sonal Agarwal
>  Dana Blair
>   Filename: draft-ietf-netmod-acl-model-17.txt
>   Pages   : 57
>   Date: 2018-03-03
> 
> Abstract:
>   This document defines a data model for Access Control List (ACL).
>   ACL is a ordered-by-user set of rules, used to configure the
>   forwarding behavior in device.  Each rule is used to find a match on
>   a packet, and define actions that will be performed on the packet.
> 
> 
> The IETF datatracker status page for this draft is:
> https://datatracker.ietf.org/doc/draft-ietf-netmod-acl-model/
> 
> There are also htmlized versions available at:
> https://tools.ietf.org/html/draft-ietf-netmod-acl-model-17
> https://datatracker.ietf.org/doc/html/draft-ietf-netmod-acl-model-17
> 
> A diff from the previous version is available at:
> https://www.ietf.org/rfcdiff?url2=draft-ietf-netmod-acl-model-17
> 
> 
> Please note that it may take a couple of minutes from the time of submission
> until the htmlized version and diff are available at tools.ietf.org.
> 
> Internet-Drafts are also available by anonymous FTP at:
> ftp://ftp.ietf.org/internet-drafts/
> 
> ___
> netmod mailing list
> netmod@ietf.org
> https://www.ietf.org/mailman/listinfo/netmod

Mahesh Jethanandani
mjethanand...@gmail.com

___
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod