Subject: [Gen-art] Gen-ART LC Review of draft-ietf-forces-interop-07

From: Ben Campbell <[email-masked]>
Date: Mon, 13 May 2013 14:28:04 -0500

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at

<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-forces-interop-07
Reviewer: Ben Campbell
Review Date: 2013-05-13
IETF LC End Date: 2013-05-13
IESG Telechat date: 

Summary: This draft is almost ready for publication as an informational RFC. I have a few minor questions and editorial comments that may be worth considering prior to publication.

*** Major issues:

None.

*** Minor issues:

-- The draft mentions a couple of instances of tests that failed because of an incorrect implementation or differing encapsulation formats. Does this suggest that the specifications should be clarified? In particular, in the case of encapsulation format mismatch, should the specs include stronger requirements to be able to receive all encapsulation formats? Or should the number of options be reduced?

-- I have a mild concern that the use of origin country names for each implementation could confuse readers into thinking that the countries themselves officially participated, rather than organizations from each country.

-- section 4.4, last paragraph:

The text says that since the mentioned failures were likely the result of bugs, it doesn't indicate an interoperability problem in the specs. I have to point out that, it also doesn't prove interoperability in both directions for the particular test. It would also be worth commenting on whether the probably bugs were programming errors rather than misunderstandings of the specification.

*** Nits/editorial comments:

-- The draft uses inconsistent verb tense throughout. I found this a bit confusing, as I assume the draft talks entirely about tests that have already occurred.

-- IDNits points out that you have several references without explicit citations. I see that you called the references out by name in the text, but it would be better to include the citations.

-- Section 1, paragraph 6:

Please expand abbreviations on first mention.

-- Section 1.1:

Please expand FE on first mention.

-- section 2.2.2, paragraph 1: "... from China and Japan implementations..."

Missing "the".

Is it possible to add a reference for details on the Smartbits testing machine?

-- Figure 2:

Do you really want to publish the IP addresses used in an RFC? RFCs live forever...

-- Section 2.2.2, paragraph after figure 2: 

First sentence does not parse.

-- Figure 3:

The figure has some formatting issues, at least in the PDF version. Also, is it possible to avoid splitting the figure across the page break?

-- section 3.2, paragraph 3: "Because of system deficiency to deploy IPSec over TML in Greece,..."

Phrase doesn't parse.

-- section 3.2, paragraph 4: "... over IPSec channel."

Missing "the".

"...to have established..."

to establish.

-- section 4 and subsections:

It seems like some of the test descriptions in 4.X may be redundant to the previous scenario descriptions.

-- section 4.1, notes on 28 and 29:

Sentence does not parse.

... notes on 30 and 31:

Missing articles.

-- section 5.1, last paragraph in list item "2.": "The interoperability test witnessed that..."

The test _showed_...   [or the _testers_ witnessed...]

-- section 9:

It would be worth mentioning that you explicitly tested for IPSec support.

---

From: "Wang,Weiming" <[email-masked]>
Date: Fri, 17 May 2013 11:19:11 +0800

Hi Ben, 

Thank you very much for the review comments. Please see inline responses from authors of the document on the comments.

Hi Sherpherd and AD, 

we will update a version very soon. 

thanks a lot.
Weiming
 
----- Original Message ----- 
From: "Ben Campbell" <[email-masked]>

*** Major issues:

None.

*** Minor issues:

-- The draft mentions a couple of instances of tests that failed because of an incorrect implementation or differing encapsulation formats. Does this suggest that the specifications should be clarified? In particular, in the case of encapsulation format mismatch, should the specs include stronger requirements to be able to receive all encapsulation formats? Or should the number of options be reduced?
[Re. by ΕΗ] The protocol provides a number of different approaches 
[Re. by Weiming] The key issue is still from the deep understanding of the protocol from implementations. I still have not seen need for any urgent change for the protocol. 

[Re by EH]Makes sense. We should possibly change this. 
In the beginning of section 2.2.1 where it says:
"The University of Patras implementation joined remotely from Greece"
Change to:
"The University of Patras (UoP) implementation joined remotely from Greece"
And then use UoP after that for us.

[Re. by Weiming] I agree with Evangelos on this. If possible (key issue might be the text space for some figures) we may change the G to UoP, the J to NTT, the C to ZJSU ? 
Other authors, please show your views. Thanks. 

The text says that since the mentioned failures were likely the result of bugs, it doesn't indicate an interoperability problem in the specs. I have to point out that, it also doesn't prove interoperability in both directions for the particular test. It would also be worth commenting on whether the probably bugs were programming errors rather than misunderstandings of the specification.
[Re. by Weiming] to change the whole paragraph to:
  <t> The two test items failed. Note that Test #7 and #8 were identical to the tests, only with CE and FE implementers were exchanged. Moreover, test #12 and #13 showed that the redirect channel worked well. Therefore, it can be reasonably inferred that the problem caused the failure was from the implementations, rather than from the ForCES protocol itself or from misunderstanding of implementations on the protocol specification. Although the failure made the OSPF interoperability test incomplete, it did not show interoperability problem. More test work is needed to verify the OSPF interoperability.</t>

-- The draft uses inconsistent verb tense throughout. I found this a bit confusing, as I assume the draft talks entirely about tests that have already occurred.
[Re. by ΕΗ] We will stick with the past tense.

-- IDNits points out that you have several references without explicit citations. I see that you called the references out by name in the text, but it would be better to include the citations.
[Re. by Weiming]We will try to fix this as much as possible.

[Re. by Weiming] Yes.

-- Section 1.1:

[Re. by Weiming] Yes.

Missing "the".

-- Figure 2:

[Re. by Weiming] In Figure 2, changed the '124.90.146.218 (ADSL)' to ' (ADSL)'.  In Figure 3, changed the '150.140.254.110(VPN)' to '(VPN)' to avoid personal IP address in public.

Section 2.2.2, paragraph after figure 2, first sentence is changed to: 
<t>CE and FE from the Greece implementation were located within the
   University of Patras, Greece, and were connected together using LAN as 
   shown in <xref target="Figure-3"/>. Connetion to the Internet was via a VPN channel.
</t>
[/Re]

-- Figure 3:

[Re. by Weiming] ok, we'll try.
[/Re]

Phrase doesn't parse.

Missing "the".

to establish.
[Re. by Weiming] The section 3.2 para.3 has been changed to: 
<t>... Because there came unfortunately a problem with the test system in Greece to deploy IPSec over TML during the test process, this test only took place between test systems in China and Japan.</t>
Other nits are also corrected.
thanks.
[/Re]
-- section 4 and subsections:

[Re. by Weiming] previous section 3.x is the scenario description, and the 4.x related section is the test results.

Sentence does not parse.

... notes on 30 and 31:

Missing articles.
[Re. by Weiming]The two notes (Note on test #28 and #29 and note on test #30 and #31 are vaguae.  After considerations, we decide to remove thess two notes. 
Thanks.
[/Re.]

[Re. by Weiming] modified to 'The interoperability testers witnessed that...".  Thanks.
[/Re.]

-- section 9:

[Re. by Weiming] As a result, the section has been updated as:

 9.  Security Considerations

   Developers of ForCES FEs and CEs must take the security
   considerations of the ForCES Framework [RFC3746]  and the ForCES
   Protocol [RFC5810]  into account.  Also, as specified in the security
   considerations section of the SCTP-Based TML for the ForCES Protocol
   [RFC5811], the transport-level security has to be ensured by IPsec.Test 
   results of TML with IPsec supported have been shown in Section 4.2 of the document. 
[/Re.]

---

From: Kentaro Ogawa <[email-masked]>
Date: Tue, 21 May 2013 14:19:16 +0900

I agree with the modification points of the draft from Evangelos and 
Weiming.

Regards,
Kentaro Ogawa

---

From: Ben Campbell <[email-masked]>
Date: Tue, 21 May 2013 15:33:47 -0500

Thanks for the response. Comments inline. I removed sections for which I have no further comment.

Thanks!

Ben.

On May 16, 2013, at 10:19 PM, "Wang,Weiming" <[email-masked]> wrote:

[...]

> [Re. by Weiming] The key issue is still from the deep understanding of the protocol from implementations. I still have not seen need for any urgent change for the protocol. 

I don't have enough knowledge of the protocol to form a specific opinion, but it's been my experience in other areas that when implementors interpret things differently, there's often room for clarification, even if the text is formally correct. I agree it doesn't imply an urgent need, but would it be worth one or more errata?

[...]

>  <t> The two test items failed. Note that Test #7 and #8 were identical to the tests, only with CE and FE implementers were exchanged. Moreover, test #12 and #13 showed that the redirect channel wo…

Works for me, thanks!

[...]

> <t>... Because there came unfortunately a problem with the test system in Greece to deploy IPSec over TML during the test process, this test only took place between test systems in China and Japan.…

The sentence is still hard to parse. Do you mean the following?

"Because an unfortunate problem with the test system in Greece prevented the deployment of IPSec over TML, this test only took place between the test systems in China and Japan."

[...]

---

From: "Wang,Weiming" <[email-masked]>
Date: Wed, 22 May 2013 23:38:15 +0800

Hi Ben and all,

Some more replies are inline. 

Attached is the up to date sub-version(08v1) of the document and the associated diff file with last version 07.  

I have to metion I still have not yet finished to change last pages of the document to past tense. I'l continue doing it.

Hope to see more comments and then with more modification, I hope to update to IETF with that version. 

thanks a lot.

Weiming

----- Original Message ----- 

Thanks!

Ben.

On May 16, 2013, at 10:19 PM, "Wang,Weiming" <[email-masked]> wrote:

[...]

[Re:] We have actually submitted a suggestion to errata of RFC 5810, but was denied with the reason that mutual understanding of different encapsulation formats are essential to protocol implementations, and need not a specific notification.
 
[...]

>  <t> The two test items failed. Note that Test #7 and #8 were identical to the tests, only with CE and FE implementers were exchanged. Moreover, test #12 and #13 showed that the redirect channel wo…

Works for me, thanks!
[Re: ] Thanks too.

[...]

"Because an unfortunate problem with the test system in Greece prevented the deployment of IPSec over TML, this test only took place between the test systems in China and Japan."
[Re.] Thanks again. I'v just incorparated it. 

[...]

---

From: Jari Arkko <[email-masked]>
Date: Mon, 27 May 2013 23:48:30 +0300

Thank you Ben and others for the review and updates. I have balloted no objection for this document, based on the Gen-ART review and my own quick review.

Jari