Re: Comments on the latest IPoIB-CM draft

On Tue, 18 Oct 2005, H.K. Jerry Chu wrote:

> Vivek,

Jerry,

Thanks for the detailed comments. I'll incorporate and resend the draft.
Some responses below.

>
> The following is a list of my comments on
> draft-ietf-ipoib-connected-mode-01.txt, many are editorial.
>
> 1) Line 27
> "Internet- Drafts" =>
> "Internet-Drafts".

yep

>
> 2) Line 48
> "This document specifies a method" =>
> "This document specifies an optional method"
>
> Suggest to spell out the "optional" part up front.

ok

>
> 3) Line 92
> Remove the non-ASCII character.

Puzzling. Am using text editor on Linux (vi) and
nroff. Will remove it.

>
> 4) Various places in the draft has paragraphs describing IPoIB-CM
> as if it were an independent transport mechansim for IPoIB that
> is separate from IPoIB-UD. If the draft simply states upfront that
> IPoIB-CM is an extension (or add-on) to IPoIB-UD, many paragraphs
> become redudant and can be removed. E.g. paragraphs beginning at
> line 161, section 2.1, most of sec 2.2. Line 313 to 319, 516-518,
> ...etc.

The idea is to delineate the differences between the option -CM and
the required -UD mode. However, I'll relook at the wording.

>
> 5) <nit> The first five lines of 2.2 may give an impression that
> one QP may be used to connect to all remote nodes.

ok..will make that clearer.

>
> 6) Continue from 4) and 5) above, sections 2.2 and 2.3 may be
> combined as follows.
>
> Every IPoIB-CM interface MUST have two sets of QPs associated with it:
>
>        1) An unreliable datagram mode QP
>        2) one or more connected mode QPs
>
> [IPoIB_UD] describes how a node can obtain 1) and, through the address
> resolution procedure, a remote node's link-layer address. Once the
> latter is known, an IB connection must be setup between the nodes
> before any IP communication may occur over an IB connected transport.
>
> [continue from line 226.]

agreed..though one reason for going into more detail is to make sure
the whole process is understood w/o any ambiguity.

>
> 7) Line 291, why not simply say the QPN is the same as the one from
> [IPoIB-UD] to avoid any confusion?

ok..

>
> 8) Line 350, change
> "The IB connection is setup using the Service-ID as defined above."
> to
> "The IB connection is setup using the Service-ID as defined in 3.5
> below."

ok

>
> 9) Line 317 contains garbled characters (looks like apostrophe), so
> does line 331, 373, 375, 377, 386.

must be character set problem...need to verify and fix.

>
> 10) Line 377, 378
> QP => QPN

ok

>
> 11) Line 383 change
> "To ensure that two IB connections are not setup between the peers"
> =>
> "To ensure that two IB connections are not setup between the peers
> due to REQ crossing"

ok

>
> 12) [nit] Section 3.3 should spell out how to numerically compare 20
> byte link-layer address.

ok

>
> 13) Section 3.4, add a stronger statement
> "IB connections created though IPoIB-CM are considered part of
> an IPoIB-CM interface. As such, they SHOULD be torn down when an
> IPoIB-CM interface is torn down.
>

yes, good suggestion.

> 14) Section 3.5 the paragraph
> "The Reserved fields MUST be transmitted as zeroes. It is
> dependent on the CM to ignore or check for zeroes in the
> Reserved fields. This is because some implementations of CMs
> require all ServiceIDs to be explicitly specified and cannot
> listen to a range of values."
> sounds awkward. If we allow implementations to either check for
> zeros or ignore on the listening side, we might as well not to say
> nothing.

ok..

>
> 15) Section 4.0
> Do we expect ARP/RARP to be sent through IB connections ever?
> If not it should say so in section 7.0 and remove ARP/RARP from
> the table to avoid any confusion.

ok..will remove.

>
> 16) Section 7.0
> Need to clarify ARP/RARP over IB connections. E.g., ARP reply
> is unicast. Can it be sent back through an IB connection? (Suggest
> no.)

will do. yes, agree on the 'no'.

Vivek
>
>
> _______________________________________________
> IPoverIB mailing list
> [email protected]
> https://www1.ietf.org/mailman/listinfo/ipoverib
>
>