Re: [netmod] AD review of draft-ietf-netmod-rfc6991-bis-15
Andy Bierman <andy@yumaworks.com> Thu, 23 March 2023 18:29 UTC
Return-Path: <andy@yumaworks.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 ED891C1526E9 for <netmod@ietfa.amsl.com>; Thu, 23 Mar 2023 11:29:24 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.095
X-Spam-Level:
X-Spam-Status: No, score=-2.095 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001, URIBL_DBL_BLOCKED_OPENDNS=0.001, URIBL_ZEN_BLOCKED_OPENDNS=0.001] autolearn=unavailable autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=yumaworks.com
Received: from mail.ietf.org ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id suD9njCIgaI4 for <netmod@ietfa.amsl.com>; Thu, 23 Mar 2023 11:29:20 -0700 (PDT)
Received: from mail-lf1-x12d.google.com (mail-lf1-x12d.google.com [IPv6:2a00:1450:4864:20::12d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 76512C152567 for <netmod@ietf.org>; Thu, 23 Mar 2023 11:29:20 -0700 (PDT)
Received: by mail-lf1-x12d.google.com with SMTP id c29so11555369lfv.3 for <netmod@ietf.org>; Thu, 23 Mar 2023 11:29:20 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yumaworks.com; s=google; t=1679596158; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=ptvjk6cvnF53XqJLikj2c5OWlhM/wR0p47Lyx3zk9VA=; b=AVvQXTbzTdBHwQS6028zXky3ilBjQN7ON43vIlcNocieGB2115wEO58+UAOkDPFayS z/GBv5SEICPRayT+pkbeU9m5yU8jzOtOlZSykne0SbieE+QYYmMbTNB2uH/Jp38SSz+8 veBiVay20YOHh0lly1d9R+meR5T8aZSXVyoIeIdhXSYtlKEtsfRHcRrphflNmTQvV+RJ ojkf7bFeG4A7jCLLj/qqtIz42qUJ8if/0s/q2C2tSOCLXtlQ7RT7TYKbkcwvFcbZZ5kP 8Psl9W6md40ltXYk/wWk1GqxOzCKf+TXBIBepNnjI01KmeLj62/zL6vB/5TgiurXSC7C RH9A==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679596158; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=ptvjk6cvnF53XqJLikj2c5OWlhM/wR0p47Lyx3zk9VA=; b=xz7OPQuQAposkpmMgP0d58ibPY8dkA/l3oRrv3M3vJT8XNNhy/a59pCkDy/cLYp2DM pL+0a+PUIOkKe9sgqkHTXPeYZA0PWGmjZqt47/TSXppB2lUvx2cVCR9UivYaFzjbvD83 AFH3OHT7gYBEV+lLLhk9eyiFJV00Ci7SVSzqa0v1qPE1q2U57nyonvw8iHqHu2hlNYkV wS1J0kUqwJvRiIGrI99JA3bQM1W6+QMqx3jxC9hpZoz3MqTdv2ykbMfE9kLAGOdd0zYJ OtvBvy2/uR3Qdx+DcvDM2NgptMoa4mhcBuZBjHUwPfQIowdzQwLYZrCtHmKXg7ybVnor Sv1Q==
X-Gm-Message-State: AO0yUKVKe8Uno26oBK3kulEVqyp3Y7XloL8vgTImYd7ly3vTabyXVxMp Ld89/5U7yS7lBfKbuqJ3lFB9QfsBpAMiGPHRTPf0Ew==
X-Google-Smtp-Source: AK7set9mc2pLPlCJNOupXFmp29qSQQiKl8wB3jjr5uQIzrdxmipxf3xdOuSueNjVnae9+MQ8YZwqEltD9eAHgS7XpSQ=
X-Received: by 2002:ac2:5182:0:b0:4d5:ca42:e43e with SMTP id u2-20020ac25182000000b004d5ca42e43emr3361882lfi.7.1679596158219; Thu, 23 Mar 2023 11:29:18 -0700 (PDT)
MIME-Version: 1.0
References: <BY5PR11MB4196AF276BC24AB7BEC310A6B5869@BY5PR11MB4196.namprd11.prod.outlook.com> <20230323111314.gd5xis346eyylygt@anna> <AM7PR07MB624840744B6F925E96C6F526A0879@AM7PR07MB6248.eurprd07.prod.outlook.com>
In-Reply-To: <AM7PR07MB624840744B6F925E96C6F526A0879@AM7PR07MB6248.eurprd07.prod.outlook.com>
From: Andy Bierman <andy@yumaworks.com>
Date: Thu, 23 Mar 2023 11:29:06 -0700
Message-ID: <CABCOCHRBPrHk8Wk0xJtYzhB8aTiS+bwUPxi70LtAPEL5Qy87kw@mail.gmail.com>
To: tom petch <ietfc@btconnect.com>
Cc: "Rob Wilton (rwilton)" <rwilton@cisco.com>, Jürgen Schönwälder <jschoenwaelder@constructor.university>, "netmod@ietf.org" <netmod@ietf.org>, "draft-ietf-netmod-rfc6991-bis.all@ietf.org" <draft-ietf-netmod-rfc6991-bis.all@ietf.org>
Content-Type: multipart/alternative; boundary="000000000000608cd805f79574f6"
Archived-At: <https://mailarchive.ietf.org/arch/msg/netmod/Ue0_cDRKBG4tVteEiMKFzI60uwI>
Subject: Re: [netmod] AD review of draft-ietf-netmod-rfc6991-bis-15
X-BeenThere: netmod@ietf.org
X-Mailman-Version: 2.1.39
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, 23 Mar 2023 18:29:25 -0000
On Thu, Mar 23, 2023 at 5:11 AM tom petch <ietfc@btconnect.com> wrote: > From: netmod <netmod-bounces@ietf.org> on behalf of Jürgen Schönwälder > <jschoenwaelder@constructor.university> > Sent: 23 March 2023 11:13 > > On Wed, Mar 22, 2023 at 01:31:43PM +0000, Rob Wilton (rwilton) wrote: > > Hi Jürgen, > > > > Thanks for the draft. Please see my AD review comments below, except > for a couple of comments related to the change to ipv6-address definition > that I've spun into a separate thread so that I can include the interested > parties of draft-ietf-6man-rfc6874bis into the discussion. > > Thanks for your review. See responses inline. > > > Moderate level comments: > > > > (1) p 13, sec 3. Core YANG Types > > > > typedef date-with-zone-offset { > > > > Why don't we just call this 'date' rather than 'date-with-zone-offset', > particularly because the zone information is optional? Intuitively, from > the name of this type, I would have expected that zone information as being > required rather than being optional. > > > > I also note that the current naming convention of this type seems > somewhat inconsistent from "date-no-zone", since one of them includes > "offset" and the other does not. > > > > This same comment also applies to 'time-with-zone-offset'. > > Earlier versions had just 'date' and 'time' and both included a zone > offset. We then also got 'date-no-zone' and 'time-no-zone' and all was > kind of nice and consistent. Then the IP address debate kicked in and > finally some people made the point that we should be always explicit > (but then you can't encode all semantics in a name anyway). So this is > how we got to the names we have now. For me personally, 'date' and > 'date-no-zone' and 'time' and 'time-no-zone' was just fine. The > '-offset' came in as a way to future proof definitions since in some > contexts you may want to indicate the timezone not with a fixed offset > but with a timezone name like 'Europe/Berlin' that is then resolved > using more complex rules to a specific offset. > > I personally would be happy to change date-with-zone-offset back to > date and time-with-zone-offset back to time and to deal with the > future in the future... > > <tp> > > Me too. I think that the fashion for incorporating ever more semantics > into an identifier is a misunderstanding of what an identifier is for ie to > identify. > > Me three. IMO simple names like 'date' and 'time' and 'date-and-time' are easier to understand. Optional fields are different than mandatory fields. If a data type has some mandatory field, then it may need to have its own typedef and name. Tom Petch > Andy > > > (2) p 27, sec 4. Internet Protocol Suite Types > > > > I've moved this comment to a separate thread. > > > > > > (3) p 28, sec 4. Internet Protocol Suite Types > > > > I've moved this comment to a separate thread. > > > > > > Minor level comments: > > > > (4) p 13, sec 3. Core YANG Types > > > > description > > "The date type represents a time-interval of the length > > of a day, i.e., 24 hours. > > > > I think that it might be helpful if the first part of the description > stated that the type optionally includes the zone offset, particularly to > differentiate from the type that excludes it. > > I am happy to add. "It includes and optional time zone offset." so > that it says: > > "The date type represents a time-interval of the length > of a day, i.e., 24 hours. It includes and optional time > zone offset. > > With the current naming scheme, we would have to > s/date/date-with-zone-offset/ everywhere in the description. > That will look pretty ugly. [sarcasm on] Perhaps we should > call it 'date-with-optional-zone-offset' and then the additional > sentence is not needed anymore. [sarcasm off] > > > This same comment also applies to 'time-with-zone-offset'. > > Yes. > > > (5) p 14, sec 3. Core YANG Types > > > > type date-with-zone-offset { > > pattern '[0-9]{4}-(1[0-2]|0[1-9])-(0[1-9]|[1-2][0-9]|3[0-1])'; > > } > > > > Although I can understand why it is modelled this way, i.e., to make the > relationship between the types clear, there is likely to be a small > performance overhead of modelling it this way, where this regex for this > type is a strict subset of date-with-zone-offset. I wonder whether it > would be cleaner to just define this type as an equivalent top-level type > to date-with-zone-offset, both in the definition and description rather > than as a derived type? > > > > This same comment also applies to 'time-no-zone'. > > This would require to copy quite some text and then this cloned text > needs to be kept consistent in the future. I do not think this is a > good idea. Implementations can take shortcuts if this is found to be > time critical (but that might also be very implementation specific). > My preference is to not change this. > > > (6) p 15, sec 3. Core YANG Types > > > > The maximum time period that can be expressed is in the > > range [-89478485 days 08:00:00 to 89478485 days 07:00:00]. > > > > I found this notation slightly confusing. When I originally saw it, I > assumed that it is talking about time zones, and it only really made sense > when comparing it to the other periods. > > > > I wasn't sure whether the specific details are that important, and > whether defining it as -89478485 days to 89478485 days, might be both > sufficient and easier to read. > > > > E.g., > > The maximum time period that can be expressed is in the > > range [-89478485to 89478485] days . > > > > If changed, this same comment applies to the other period types as well. > > For time periods with lower resolution, the details start to matter, > (see microseconds32 on the extreme end) and so I ended up using the > same notation and precision for all types. I think this is generally > the right thing to do, being always precise is better than arbitrarily > dropping precision. If someone has ideas for a better notation, I am > open for that. Perhaps adding ", where hh:mm:ss stands for hours, > minutes and seconds" would already do it?: > > The maximum time period that can be expressed is in the > range [-89478485 days 08:00:00 to 89478485 days 07:00:00], > where hh:mm:ss stands for hours, minutes and seconds." > > > (7) p 15, sec 3. Core YANG Types > > > > This type should be range restricted in situations > > where only non-negative time periods are desirable, > > (i.e., range '0..max')."; > > > > Isn't this going to be the common mainline case for network > configuration? I.e., I presume that most cases where periods are intervals > are going to be reported will be positive. Hence, it might be helpful to > have a separate set of types defined for the positive only cases. > > > > This same comment applies to the other period types. > > I ones had unsigned versions of these types. If we also add unsigned > types, we end up with nine additional types, we would get something > like: > > hours-int32 > hours-uint32 > minutes-int32 > minutes-uint32 > seconds-int32 > seconds-uint32 > ... > nanoseconds-int64 > nanoseconds-uint64 > > Well, perhaps this is the right thing to do, people can then pick what > is most appropriate for their use case. > > > (8) p 16, sec 3. Core YANG Types > > > > typedef milliseconds32 { > > > > I was slightly surprised that we don't have a milliseconds64, e.g., the > default timestamp in Java is given as an int64 in milliseconds. > > > > So far nobody asked for it. On POSIX systems (I think POSIX.1-2001 and > later), you usually have system APIs that can go into microsecond > resolution: > > struct timeval { > time_t tv_sec; /* seconds */ > suseconds_t tv_usec; /* microseconds */ > }; > > But if there is a use case for milliseconds64, I can easily add it. > well milliseconds-int64 and milliseconds-uint64, depending on the > resolution of your previous point. > > > Nit level comments: > > > > (9) p 21, sec 3. Core YANG Types > > > > 7950. An earlier version of this definition did exclude > > > > I suggest 'did exclude' -> 'excluded' > > Changed. > > /js > > -- > Jürgen Schönwälder Constructor University Bremen gGmbH > Phone: +49 421 200 3587 Campus Ring 1 | 28759 Bremen | Germany > Fax: +49 421 200 3103 <https://constructor.university/> > > _______________________________________________ > 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 >
- [netmod] AD review of draft-ietf-netmod-rfc6991-b… Rob Wilton (rwilton)
- Re: [netmod] AD review of draft-ietf-netmod-rfc69… Jürgen Schönwälder
- Re: [netmod] AD review of draft-ietf-netmod-rfc69… tom petch
- Re: [netmod] AD review of draft-ietf-netmod-rfc69… Andy Bierman
- Re: [netmod] AD review of draft-ietf-netmod-rfc69… Mahesh Jethanandani
- Re: [netmod] AD review of draft-ietf-netmod-rfc69… Jürgen Schönwälder
- Re: [netmod] AD review of draft-ietf-netmod-rfc69… Mahesh Jethanandani
- Re: [netmod] AD review of draft-ietf-netmod-rfc69… Jürgen Schönwälder
- Re: [netmod] AD review of draft-ietf-netmod-rfc69… Carsten Bormann
- Re: [netmod] AD review of draft-ietf-netmod-rfc69… mohamed.boucadair