Subject: IESG feedback on draft-ietf-ipngwg-rfc2553bis-06.txt

From: Thomas Narten <[email-masked]>
Date: Tue, 03 Sep 2002 13:46:07 -0400

The IESG recently reviewed this document. No major issues, I think. However
the following issues were raised and warrant investigation. Once these
comments are addressed, I expect the IESG to approve  the document.

Comments:

I only have draft 7 of 1003.1-2001 with me, so maybe some of these
issues are bugs in draft 7.

note: the above was then followed up with the following:

I looked at my CD-ROM copy of The Open Group's SUSv3 (their version of
the Austin standard, also 1003.1-2001) and didn't find any differences
from draft 7 on these issues.]

In the middle of section 3.3, this sentence doesn't make any sense to
me:

  The sin6_flowinfo
  field SHOULD be set to zero by an implementation prior to using the
  sockaddr_in6 structure by an application on receive operations.

There's no reference to this in 1003.1-2001, as far as I can tell, and
I don't even really understand what it means - does it mean that the
previously "result only" sockaddr arg to accept() or recvfrom() is now
"value-result", and that a non-zero sin6_flowinfo passed in as the value
has some undesirable semantic?  Does it mean that an application should
always say "sin6->sin6_flowinfo = 0" after receiving it from an accept()
or recvfrom()?

In section 3.10, the text was updated to remove the extra prefixed
underscore in the sockaddr_storage definition, but the struct was not.
e.g. the struct has elements like "__ss_pad1" and the text refers to
"_ss_pad1".  This is true of all of the struct elements and the
references to them in the comment.

There's a similar problem with the definition for systems with an
sa_len field, but there's also a second problem here: the definition of
_SS_PAD2SIZE is incorrect.  It needs to add back the sizeof(uint8_t)
as well.  (I actually checked this by compiling test programs).
It would seem much simpler to just define _SS_PAD2SIZE as
_SS_MAXSIZE - 2*_SS_ALIGNSIZE, but that would break parallelism with
1003.1-2001 so it's probably best to just add in the sizeof(uint8_t), i.e.

#define _SS_PAD2SIZE (_SS_MAXSIZE - (sizeof (uint8_t) + sizeof (sa_family_t) +
                              _SS_PAD1SIZE + _SS_ALIGNSIZE))

(This bug isn't in 1003.1-2001 since it doesn't mention sa_len at all)

The last paragraph in section 4.4 seems odd; e.g. there's no parallel
wording for <netdb.h> or any of the other headers that gain new prototypes
and structs and constants.  Is it necessary?

Section 6.1 doesn't describe EAI_OVERFLOW as a possible return value
from getaddrinfo(), while 1003.1-2001 does.  Conversely, 6.2 describes
EAI_OVERFLOW as a possible return value from getnameinfo(), while
1003.1-2001 doesn't.

The first paragraph of the Acks section reads oddly, switching from
Richard to Rich and back again.  Maybe just replace "Rich's" with "His"
to avoid it?

--------------------------------------------------------------------
IETF IPng Working Group Mailing List
IPng Home Page:                      http://playground.sun.com/ipng
FTP archive:                      ftp://playground.sun.com/pub/ipng
Direct all administrative requests to [email-masked]
--------------------------------------------------------------------

---

From: Jack McCann <[email-masked]>
Date: Mon, 16 Sep 2002 14:44:19 -0400 (EDT)

Thomas,

Comments inline below.  I will incorporate this feedback (and 
feedback from one other reviewer email) and rev the spec one 
more time.

- Jack

>From [email-masked] Tue Sep  3 13:49:39 2002
>To: [email-masked]
>Cc: [email-masked]
>Subject: IESG feedback on draft-ietf-ipngwg-rfc2553bis-06.txt
>Date: Tue, 03 Sep 2002 13:46:07 -0400
>From: Thomas Narten <[email-masked]>
>

Good point.  After staring at it a bit, I don't think it means either 
of those things.  I think this is intended as a recommendation to the
*implementation* (e.g. the kernel) to make sure it does not leave 
uninitialized garbage in the sin6_flowinfo field of a sockaddr_in6
it is going to give back to the application on a receive operation.
I think the word "provided" is missing, that is the sentence should
read "...sockaddr_in6 structure provided by an application...".

That said:

a. I could be wrong about that, I don't know the original intent.

b. I think the sentence in question can be safely deleted.

c. The use of sin6_flowinfo is rather underspecified.  Of the 32
   bits, which ones contain the traffic class and which ones contain 
   the flow label?  What's in the other 4 bits?  Can I use it to set
   the traffic class and/or flow label for outbound packets (e.g.
   on a connect(), sendto() or sendmsg()?  What is behavior of
   this field on accept/recvfrom/recvmsg?  What is behavior with TCP?
   What is interaction with IPV6_TCLASS option from rfc2292bis?

>references to them in the comment.

Will fix.

>(This bug isn't in 1003.1-2001 since it doesn't mention sa_len at all)

Will fix.

>and structs and constants.  Is it necessary?

Not any more, I will delete it.

>from getaddrinfo(), while 1003.1-2001 does.  

That was a mistake in 1003.1-2001, getaddrinfo() has no buffer
arguments to overflow.  I've filed a bug report to try to have it 
removed from the getaddrinfo() definition in 1003.1-2001.

>Conversely, 6.2 describes

That was a mistake in 1003.1-2001 and is fixed in the pending
Technical Corrigenda 1 to that document.

>Richard to Rich and back again.  Maybe just replace "Rich's" with "His"

OK.