Re: [netmod] WG Last Call: draft-ietf-netmod-acl-model-14

Mahesh Jethanandani <mjethanandani@gmail.com> Thu, 02 November 2017 12:40 UTC

Return-Path: <mjethanandani@gmail.com>
X-Original-To: netmod@ietfa.amsl.com
Delivered-To: netmod@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 3EC7213F45D for <netmod@ietfa.amsl.com>; Thu, 2 Nov 2017 05:40:38 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2
X-Spam-Level:
X-Spam-Status: No, score=-2 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 2SSYatVyxwKE for <netmod@ietfa.amsl.com>; Thu, 2 Nov 2017 05:40:35 -0700 (PDT)
Received: from mail-pg0-x241.google.com (mail-pg0-x241.google.com [IPv6:2607:f8b0:400e:c05::241]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 3796B138103 for <netmod@ietf.org>; Thu, 2 Nov 2017 05:40:34 -0700 (PDT)
Received: by mail-pg0-x241.google.com with SMTP id s75so5016463pgs.0 for <netmod@ietf.org>; Thu, 02 Nov 2017 05:40:34 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=references:mime-version:in-reply-to:content-transfer-encoding :message-id:cc:from:subject:date:to; bh=XmrXz4ouSzlECnIMEtal33kqMuJqgzdsqmP5r7yoquM=; b=Qw61WfAv9zWQwOWHXVYp42jF3yfFtsh0OsI4ZmPMcQmc1iM63KOEkm51oho1qaWN9K Sss0wHYUvQP2h5kVfoX5o6rUec9HVhH8W3s1hqcOw3Dbsd4WUInuHxAGqzTPc1gYaVop 0+vGIFdqo9PDa2WwwuacC/gVAHl5Auq4AFBESB9zFRWm2pOB8SnYlVlvpK5MzX/eNP68 JHbsF/pXvInv+noVgIVa1HZCwZQR9lUrNnBCstGV2vvd0f9KDDYKtXNjr69XlR90BVum eEKYnS6OSHmGMx59otk3pAic8ZI6Ol9d/lm/5CNcqDRFk2dGtSl8xeXN0RMagRWAqZmM V9JA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:references:mime-version:in-reply-to :content-transfer-encoding:message-id:cc:from:subject:date:to; bh=XmrXz4ouSzlECnIMEtal33kqMuJqgzdsqmP5r7yoquM=; b=DO7M5FJ58bu05DXzpAvSpEn4GkGBAy3QDurmuMJxCl+BjYADOjSvPlBkBw0ax4PZFY E6mnO9pVDg649NscJOeHCwGcY3/HdjXYrb8p1ofb3P/RuKmz46TAwBtUsaBkrGIzDhiU acxicJSCq/mgaKRpehX6vJZGzbBM6M9XNjw43LXo1kXL1Upa6LLjs0sNfOfx2IaCXSAZ CMaBBywF7X3+rUHCTeSl5mvo0wVuT31bweu94G/cBeuLk2Rp+zL3wC+F+yQbz4XuL3+r mWQgy9Hb3bGqpS2Bez2Kwy2lebA4DDh7wNJGbfktnf7yTvK43alqClnoqRGC0BqSkCxi p5qg==
X-Gm-Message-State: AMCzsaVkvqugV/dmIQ1TVyKFbm4GWVJeH9XpoAjLllaUy9Auzd7ObSbC nFwGNrZlyJQ19w4DwkFHZOg=
X-Google-Smtp-Source: ABhQp+TOPi3A1DLuo6eLDx2MhPOFHc9xNj7kOR9B3J4TYarinnobBlTFXeG2vYcsZpVR5UWLfyDf7A==
X-Received: by 10.98.15.22 with SMTP id x22mr3629784pfi.13.1509626433554; Thu, 02 Nov 2017 05:40:33 -0700 (PDT)
Received: from [10.116.192.241] ([115.132.156.192]) by smtp.gmail.com with ESMTPSA id c22sm6677988pfe.177.2017.11.02.05.40.32 (version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 02 Nov 2017 05:40:32 -0700 (PDT)
References: <20171102074318.GC12688@spritelink.se> <6359CD50-0F0D-4315-A58B-1D4CF0583475@gmail.com> <ac9fc676-80f7-723d-9a85-c99fbb122476@cisco.com> <20171102.132634.1363976895007772742.mbj@tail-f.com>
Mime-Version: 1.0 (1.0)
In-Reply-To: <20171102.132634.1363976895007772742.mbj@tail-f.com>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: quoted-printable
Message-Id: <51CEDFCB-88CF-4066-8428-55BF7521D1F0@gmail.com>
Cc: rwilton@cisco.com, kristian@spritelink.net, netmod@ietf.org
X-Mailer: iPad Mail (13G36)
From: Mahesh Jethanandani <mjethanandani@gmail.com>
Date: Thu, 2 Nov 2017 19:10:30 +0630
To: Martin Bjorklund <mbj@tail-f.com>
Archived-At: <https://mailarchive.ietf.org/arch/msg/netmod/EwPxGYRzhlGnVTZKpdOunU13DQM>
Subject: Re: [netmod] WG Last Call: draft-ietf-netmod-acl-model-14
X-BeenThere: netmod@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: NETMOD WG list <netmod.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netmod>, <mailto:netmod-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netmod/>
List-Post: <mailto:netmod@ietf.org>
List-Help: <mailto:netmod-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netmod>, <mailto:netmod-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 02 Nov 2017 12:40:38 -0000

Ok. Will update the model to reflect the discussion on this thread.

Mahesh Jethanandani
mjethanandani@gmail.com

> On Nov 2, 2017, at 6:56 PM, Martin Bjorklund <mbj@tail-f.com>; wrote:
> 
> Robert Wilton <rwilton@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 <kristian@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.schoenwaelder@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
>>> mjethanandani@gmail.com
>>> _______________________________________________
>>> netmod mailing list
>>> netmod@ietf.org
>>> https://www.ietf.org/mailman/listinfo/netmod
>>> .
>>