Re: [apps-discuss] Review of draft-melnikov-smtp-priority-04

Alexey Melnikov <alexey.melnikov@isode.com> Wed, 11 January 2012 12:48 UTC

Return-Path: <alexey.melnikov@isode.com>
X-Original-To: apps-discuss@ietfa.amsl.com
Delivered-To: apps-discuss@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 32C0821F8667; Wed, 11 Jan 2012 04:48:19 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -102.466
X-Spam-Level:
X-Spam-Status: No, score=-102.466 tagged_above=-999 required=5 tests=[AWL=0.132, BAYES_00=-2.599, HTML_MESSAGE=0.001, USER_IN_WHITELIST=-100]
Received: from mail.ietf.org ([12.22.58.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id OJJ-A+5WIEOg; Wed, 11 Jan 2012 04:48:16 -0800 (PST)
Received: from rufus.isode.com (cl-125.lon-03.gb.sixxs.net [IPv6:2a00:14f0:e000:7c::2]) by ietfa.amsl.com (Postfix) with ESMTP id 0387A21F864A; Wed, 11 Jan 2012 04:48:15 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1326286093; d=isode.com; s=selector; i=@isode.com; bh=fd9d9+FfPlg99OnnWMDpq/dlEW6ny+gqS4EFYatjVDI=; h=From:Sender:Reply-To:Subject:Date:Message-ID:To:Cc:MIME-Version: In-Reply-To:References:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description; b=E81nlLmJnCPxYIqU1U7DMD840Z0OPFzlhOXYMReSOZAp9h7RYrzlCCAdSE25KEFdwtQbkt PUUCX4wDwTyC8fcw6gI2HI9lbqqVxeLMLoFPh/ree3BWn0rcI8E0F0W9XVZKbAvuZ/JsgQ m8SmlrVXi1fMLiHqpvIN93dEMYTYoNI=;
Received: from [172.16.1.29] (shiny.isode.com [62.3.217.250]) by rufus.isode.com (submission channel) via TCP with ESMTPSA id <Tw2FCwBOhaNK@rufus.isode.com>; Wed, 11 Jan 2012 12:48:13 +0000
Message-ID: <4F0D8520.1080206@isode.com>
Date: Wed, 11 Jan 2012 12:48:32 +0000
From: Alexey Melnikov <alexey.melnikov@isode.com>
User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20111105 Thunderbird/8.0
To: Glenn Parsons <glenn.parsons@ericsson.com>
References: <D9DBDA6E6E3A9F438D9F76F0AF9D7AE33B3EEA96F2@EUSAACMS0714.eamcs.ericsson.se>
In-Reply-To: <D9DBDA6E6E3A9F438D9F76F0AF9D7AE33B3EEA96F2@EUSAACMS0714.eamcs.ericsson.se>
MIME-Version: 1.0
Content-Type: multipart/alternative; boundary="------------000203060407060504080107"
Cc: "draft-melnikov-smtp-priority.all@tools.ietf.org" <draft-melnikov-smtp-priority.all@tools.ietf.org>, "iesg@ietf.org" <iesg@ietf.org>, "apps-discuss@ietf.org" <apps-discuss@ietf.org>
Subject: Re: [apps-discuss] Review of draft-melnikov-smtp-priority-04
X-BeenThere: apps-discuss@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: General discussion of application-layer protocols <apps-discuss.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/apps-discuss>, <mailto:apps-discuss-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/apps-discuss>
List-Post: <mailto:apps-discuss@ietf.org>
List-Help: <mailto:apps-discuss-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/apps-discuss>, <mailto:apps-discuss-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 11 Jan 2012 12:48:19 -0000

Hi Glenn,
Thank you for your review.

On 10/01/2012 21:10, Glenn Parsons wrote:
> I have been selected as the Applications Area Directorate reviewer for 
> this draft (for background on appsdir, please see 
> http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirectorate). 
>
> Please resolve these comments along with any other Last Call comments 
> you may receive. Please wait for direction from your document shepherd 
> or AD before posting a new version of the draft.
> Document: draft-melnikov-smtp-priority-04
> Title: Simple Mail Transfer Protocol extension for Message Priorities
> Reviewer: Glenn Parsons
> Review Date:  January 10, 2012
> Summary:
> This draft is not ready for publication as a Proposed Standard and 
> should be revised before publication
> Major Issues:
> 3.  in item #7 I presume this should be LMTP, but my concern is that 
> it says this is being defined for LMTP.  Since LMTP is informational, 
> I don't think that is appropriate.  Instead, I would suggest that it 
> is defined for Message Submission and as a result may also be used for 
> LMTP.   I would prefer this to be noted in an Appendix (along with the 
> LMTP advice in section 7) -- but if you would prefer it to remain in 
> the main body, it needs to be reworded.  In either case, I would move 
> LMTP to the informative references.
I know that LMTP (RFC 2033) being Informational is annoying, but really, 
it is a perfectly valid DownRef and can be called out during IETF LC.
(At some point LMTP should be promoted to Proposed Standard, but that is 
largely independent of this work.)
> 3.  the too long example in 3.1 is apparently justified by item #6 
> here ... but you can't have a line longer than 72 chars in an RFC...
I think I've fixed that in my copy.
> but more importantly, how do you guarantee that this does not 
> accidentally blow something up.  This is a HUGE change that is 
> mentioned in passing without justification.  This seems like a large 
> hurdle for an implementation to be able to add support for this.
Why? Advertisement of supported levels is entirely optional.
> The use for this appears to be to fit in message size parameters for 
> each priority (and I presume the message size is in bytes as it does 
> not say).
Yes, in bytes. Clarified in my copy.
> So why not use Kb instead of bytes?
I was trying to be consistent with the SIZE SMTP extension which also 
uses bytes. But as you are the second person to complain about that, 
maybe I will give up and switch to Kb ;-).
> Or render the numbers in hex?  Or split it over two lines?
The last doesn't work well with SMTP. At least no SMTP extension ever 
listed more than 1 capability in EHLO response.
> I think any of these would be better than increasing the line length.
> 5. The priority level definition is not clear.  There are 200 possible 
> values.  But there MUST be at least 6 supported.  OK that seems like 
> overkill -- why can't it be from -9 to 9 then?  Anyway, is that 6 
> distinct numbers, 6 distinct numbers from the range shown, or any 
> number for the range shown?  Further I would  suggest that a default 
> value should be specified for these 6 levels and the default mapping 
> ranges -- all in a table.
As other people made the same comment, I will reply to this with my 
reasoning in a separate email.
> 6 & 8. You need to pick something and be consistent.  I would suggest 
> it is not necessary for them to be the same.  So the EHLO one could be 
> short "PRI" and the header long "Message Transfer Priority".  In any 
> case, the terminology then needs alignment throughout the document.
> A definition of the Priority message header field is missing.  It 
> should be after section 3.
I think a forward reference in section 4.2 (where the MT-Priority header 
field is specified) would be sufficient. Unless you meant something else.
> Minor Issues:
> Title:  I would change "Message Priorities " to "Message Transfer 
> Priority"
> 6. The units for "message size" need to be indicated at some point in 
> the document, at least in the syntax of section 6.
This is already fixed in my copy (will be in -05).
> 3. I would suggest to replace "The following service extension is 
> defined:" with "The Priorty SMTP service extension is defined as 
> follows:"
> 5.1.x I am somewhat concerned about informative implementation details 
> -- I presume that is why this is informative?  Or is it because this 
> is not exhaustive?
Mostly the latter.
> In any case, I think this should be in the appendix.
Ok.
> 6. It is confusing to have both specifications for the ESMTP and 
> Header field in the same place.   I would suggest separate 
> appropriatley titled sub-sections of section 6
> 7.  I would suggest adding an example fo the message header field in 
> this clause.
> 9.  What are the "anchor" placeholders for?
This is just a hint to RFC Editor to insert the final RFC number, once 
known.
> 9. Presumably the TBD items need to be filled in...
The 2 values will be assigned by IANA before publication.
> Nits:
> 3.1 the example is too long
> ** There is 1 instance of too long lines in the document, the longest 
> one being 12 characters in excess of 72.
> ...but I could not find this one:
> == There are 1 instance of lines with non-RFC2606-compliant FQDNs in 
> the document.
Ok, I will check.