Ok. Will update the model to reflect the discussion on this thread.
Mahesh Jethanandani
mjethanand...@gmail.com
> On Nov 2, 2017, at 6:56 PM, Martin Bjorklund <m...@tail-f.com> wrote:
>
> Robert Wilton <rwil...@cisco.com> wrote:
>> Hi Mahesh,
>>
>> I also think that the model would be cleaner if you don't have
>> separate containers for each "type of ACL". In particular, I think
>> that the model is easier for clients, and perhaps easier to implement,
>> if a given ACE field is always on the same path rather that being on a
>> different path based on ACL type.
>
> +1
>
>> I've suggested a minor change to
>> the model that may retain the same benefits, but keep consistent paths
>> for the ACE fields.
>>
>> Hence, would the following help improve the model?
>>
>> 1) Move the L2, IPv4, IPv6 match fields under their own containers, as
>> Juergen/Kristian have suggested.
>> 2) Under each of those containers remove the "if-feature", and change
>> the "must" statement to a "when statement".
>> 3) Add if-feature statements under the acl-type identity definitions,
>> allowing a device to control what acl-types mixes they support.
>>
>> E.g.
>>
>> // ACL features defined as below.
>>
>> // ACL type identities updated to:
>> identity ipv4-acl {
>> base acl:acl-base;
>> if-feature "ipv4-acl";
>> }
>> identity ipv6-acl {
>> base acl:acl-base;
>> if-feature "ipv6-acl";
>> }
>> identity eth-acl {
>> base acl:acl-base;
>> if-feature "l2-acl";
>> }
>> identity mixed-l2-l3-ipv4-acl {
>> base "acl:ipv4-acl";
>> base "acl:eth-acl";
>> if-feature "mixed-ipv4-acl";
>> }
>
> There seem to be some inconsistencies in these names. For example,
> why is the identity called "eth-acl" but the feature "l2-acl"? And
> when the identity for eth is used with "mixed-" it is called "l2".
>
> Also, why is "mixed-l2-l3-ipv4-acl" not called "mixed-l2-ipv4-acl"?
> And why is the corresponding feature just called "mixed-ipv4-acl"?
>
>
>
> /martin
>
>
>
>
>
>> identity mixed-l2-l3-ipv6-acl {
>> base "acl:ipv6-acl";
>> base "acl:eth-acl";
>> if-feature "mixed-ipv6-acl";
>> }
>>
>> ...
>>
>> ...
>> leaf acl-type {
>> type acl-type;
>> }
>> container aces {
>> list ace {
>> key "rule-name";
>> ordered-by user;
>> leaf rule-name ...
>>
>> container matches {
>> container l2 {
>> when "derived-from(../../../../acl-type, 'acl:eth-acl')";
>> uses packet-fields:acl-eth-header-fields;
>> description
>> "Rule set for L2 ACL.";
>> }
>> container ipv4 {
>> when "derived-from(../../../../acl-type, acl:ipv4-acl')";
>> uses packet-fields:acl-ip-header-fields;
>> uses packet-fields:acl-ipv4-header-fields;
>> description
>> "Rule set that supports IPv4 headers.";
>> }
>> container ipv6 {
>> ...
>> }
>>
>>
>> Thanks,
>> Rob
>>
>>
>>> On 02/11/2017 08:50, Mahesh Jethanandani wrote:
>>> Kristian,
>>>
>>> I hear you. What I am providing is the rational for the current
>>> design.
>>>
>>> I would like to hear from others in the WG. We have been reviewing
>>> this draft for the last couple of years, and we are now at the tail
>>> end of the LC. I would really like to see this draft move forward,
>>> particularly since it is not broken.
>>>
>>> Thanks.
>>>
>>>> On Nov 2, 2017, at 2:13 PM, Kristian Larsson <krist...@spritelink.net>
>>>> wrote:
>>>>
>>>>> On Thu, Nov 02, 2017 at 06:13:04AM +0630, Mahesh Jethanandani wrote:
>>>>> On Nov 1, 2017, at 5:52 PM, Juergen Schoenwaelder <
>>>>> j.schoenwael...@jacobs-university.de> wrote:
>>>>>
>>>>> Mahesh,
>>>>>
>>>>> I think the question is why we need to have different match containers
>>>>> for each possible feature set combination instead of having a single
>>>>> match container with groups of leafs in it marked as features. This
>>>>> would seem to cut down the size of the module and the tree diagram
>>>>> significantly. I think this will also make clients simpler sicne they
>>>>> do not have to select a certain container based on the feature set
>>>>> announced.
>>>>>
>>>>>
>>>>> The current design of match containers was chosen to allow platforms
>>>>> to select
>>>>> one container that matched what the hardware supported from a l2, l3
>>>>> and ipv
>>>>> {4,6} perspective.
>>>> Sure, but you are conflating the structure of the model with the
>>>> feature-wraps. Without changing the features of the model, we can
>>>> structure it in a different way where there is not a 1:1 mapping
>>>> between features and containers under the matches container.
>>>>
>>>>
>>>>> I would argue that even though the overall diagram is bigger
>>>>> with this design, once the platform selects the container of choice,
>>>>> the tree
>>>>> and the configuration itself would be a little simpler/smaller.
>>>> I am arguing the opposite. It's really awkward to place the same
>>>> type of data, like IPv4 match conditions, under different paths
>>>> based on a feature.
>>>>
>>>> If the system model had done the same we would have:
>>>> /system
>>>> /system-with-ntp
>>>> /system-with-ntp-and-radius
>>>>
>>>> How do you augment in new leaves? While you are using groupings
>>>> which allows for a reuse of data across all these containers,
>>>> someone who is augmenting can't, since you can't augment a
>>>> container, only where it is used. Someone wishing to add a leaf
>>>> to the model needs to augment in three different locations to
>>>> support a new match condition for IPv4 (let's say some meta-data
>>>> attribute).
>>>>
>>>>
>>>>> Take the case where the desired selection is l2,-l3, ipv4 and
>>>>> ipv6. The current
>>>>> tree looks like this:
>>>>>
>>>>>
>>>>> | | +--rw l2-l3-ipv4-ipv6-acl {l2-l3-ipv4-ipv6-acl}?
>>>>> | | | +--rw destination-mac-address? yang:mac-address
>>>>> | | | +--rw destination-mac-address-mask?
>>>>> | | | yang:mac-address
>>>>> | | | +--rw source-mac-address? yang:mac-address
>>>>> | | | +--rw source-mac-address-mask? yang:mac-address
>>>>> | | | +--rw ethertype? eth:ethertype
>>>>> | | | +--rw dscp? inet:dscp
>>>>> | | | +--rw ecn? uint8
>>>>> | | | +--rw length? uint16
>>>>> | | | +--rw ttl? uint8
>>>>> | | | +--rw protocol? uint8
>>>>> | | | +--rw source-port-range!
>>>>> | | | | +--rw lower-port inet:port-number
>>>>> | | | | +--rw upper-port? inet:port-number
>>>>> | | | | +--rw operation? operator
>>>>> | | | +--rw destination-port-range!
>>>>> | | | | +--rw lower-port inet:port-number
>>>>> | | | | +--rw upper-port? inet:port-number
>>>>> | | | | +--rw operations? operator
>>>>> | | | +--rw ihl? uint8
>>>>> | | | +--rw flags? bits
>>>>> | | | +--rw offset? uint16
>>>>> | | | +--rw identification? uint16
>>>>> | | | +--rw destination-ipv4-network? inet:ipv4-prefix
>>>>> | | | +--rw source-ipv4-network? inet:ipv4-prefix
>>>>> | | | +--rw next-header? uint8
>>>>> | | | +--rw destination-ipv6-network? inet:ipv6-prefix
>>>>> | | | +--rw source-ipv6-network? inet:ipv6-prefix
>>>>>
>>>>> | | | +--rw flow-label?
>>>>> | | | inet:ipv6-flow-label
>>>>>
>>>>>
>>>>>
>>>>> whereas, if the design went with one match container with each group
>>>>> of leafs
>>>>> in their own container (to support the if-feature statement for that
>>>>> container), the tree would look like this:
>>>>>
>>>>>
>>>>> | | +--rw l2-acl {l2-acl}?
>>>>> | | | +--rw destination-mac-address? yang:mac-address
>>>>> | | | +--rw destination-mac-address-mask?
>>>>> | | | yang:mac-address
>>>>> | | | +--rw source-mac-address? yang:mac-address
>>>>> | | | +--rw source-mac-address-mask? yang:mac-address
>>>>> | | | +--rw ethertype? eth:ethertype
>>>>>
>>>>> | | +--rw ipv4-acl {ipv4-acl}?
>>>>> | | | +--rw dscp? inet:dscp
>>>>> | | | +--rw ecn? uint8
>>>>> | | | +--rw length? uint16
>>>>> | | | +--rw ttl? uint8
>>>>> | | | +--rw protocol? uint8
>>>>> | | | +--rw source-port-range!
>>>>> | | | | +--rw lower-port inet:port-number
>>>>> | | | | +--rw upper-port? inet:port-number
>>>>> | | | | +--rw operation? operator
>>>>> | | | +--rw destination-port-range!
>>>>> | | | | +--rw lower-port inet:port-number
>>>>> | | | | +--rw upper-port? inet:port-number
>>>>> | | | | +--rw operations? operator
>>>>> | | | +--rw ihl? uint8
>>>>> | | | +--rw flags? bits
>>>>> | | | +--rw offset? uint16
>>>>> | | | +--rw identification? uint16
>>>>> | | | +--rw destination-ipv4-network? inet:ipv4-prefix
>>>>> | | | +--rw source-ipv4-network? inet:ipv4-prefix
>>>>> | | +--rw ipv6-acl {ipv6-acl}?
>>>>> | | | +--rw dscp? inet:dscp
>>>>> | | | +--rw ecn? uint8
>>>>> | | | +--rw length? uint16
>>>>> | | | +--rw ttl? uint8
>>>>> | | | +--rw protocol? uint8
>>>>> | | | +--rw source-port-range!
>>>>> | | | | +--rw lower-port inet:port-number
>>>>> | | | | +--rw upper-port? inet:port-number
>>>>> | | | | +--rw operation? operator
>>>>> | | | +--rw destination-port-range!
>>>>> | | | | +--rw lower-port inet:port-number
>>>>> | | | | +--rw upper-port? inet:port-number
>>>>> | | | | +--rw operations? operator
>>>>> | | | +--rw next-header? uint8
>>>>> | | | +--rw destination-ipv6-network? inet:ipv6-prefix
>>>>> | | | +--rw source-ipv6-network? inet:ipv6-prefix
>>>>> | | | +--rw flow-label? inet:ipv6-flow-label
>>>>>
>>>>>
>>>>> The difference though is small and comes down to a preference. Select
>>>>> one
>>>>> feature statement and get one container with everything in it, or
>>>>> define
>>>>> multiple feature statements and assemble together the pieces to define
>>>>> the ACE
>>>>> entry.
>>>> Again, I think you mix up the features available vs the structure
>>>> of the data.
>>>>
>>>> This is the current list of features (which again, I want to
>>>> change but that's separate):
>>>> * l2-acl
>>>> * ipv4-acl
>>>> * ipv6-acl
>>>> * mixed-ipv4-acl
>>>> * mixed-ipv6-acl
>>>> * l2-l3-ipv4-ipv6-acl
>>>> * tcp-acl
>>>> * udp-acl
>>>> * icmp-acl
>>>> * any-acl
>>>>
>>>> As per your second example above we have an ipv4-acl container
>>>> under matches, i.e. /access-list/acl/aces/ace/matches/ipv4-acl.
>>>> It is only possible to define ipv4 matches if one of the
>>>> following features are present:
>>>> * ipv4-acl
>>>> * mixed-ipv4-acl
>>>> * l2-l3-ipv4-ipv6-acl
>>>>
>>>> Thus you write an if-feature statement to reflect that, like
>>>> this:
>>>>
>>>> if-feature "ipv4-acl mixed-ipv4-acl l2-l3-ipv4-ipv6-acl";
>>>>
>>>> So you see, the tight coupling you have between the data
>>>> structure and the if-features isn't necessary.
>>>>
>>>> I think the structure should first be established without
>>>> features and then features can be inserted where they make sense
>>>> to make part of the model optional. Starting with the list of
>>>> features and then designing the structure around them makes for a
>>>> much less natural data structure IMHO.
>>>>
>>>> Kind regards,
>>>> Kristian.
>>> Mahesh Jethanandani
>>> mjethanand...@gmail.com
>>> _______________________________________________
>>> netmod mailing list
>>> netmod@ietf.org
>>> https://www.ietf.org/mailman/listinfo/netmod
>>> .
>>
_______________________________________________
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod
