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

[Glenn Parsons <[e-mail]>]

Alexey,

You're welcome :-)

Despite your assertion that LMTP would be a valid DownRef ... I do not think this extension should be defined for it.  That is I would suggest adding text along the lines of ...it is defined for Message Submission and as a result may also be used for LMTP.

On lines longer than 72 -- it just feels bad.  And further, you have no justification for this here.  Is it for the message length?  If it is won't moving to Kb remove the need for a longer line?

On the naming you need to be consistent.  For example, is it "PRI" or "PRIORITY"

On docuemnt organization, there are two things defined here:  Priority SMTP extension & Priority message header.  These need to be in distinct sections with appropriate titles.  It is the second that is currently hidden.

Cheers,
Glenn.

________________________________
From: Alexey Melnikov [mailto:[e-mail]]
Sent: January-11-12 7:49 AM
To: Glenn Parsons
Cc: [e-mail]; [e-mail]; [e-mail]
Subject: Re: [apps-discuss] Review of draft-melnikov-smtp-priority-04

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.