Re: RFR: 8281236: (D)TLS key exchange named groups [v3]

2022-11-21 Thread Sean Mullan
On Tue, 8 Nov 2022 22:07:35 GMT, Sean Mullan  wrote:

>>> Unfortunately, I only have author status and can only comment.
>> 
>> I think OpenJDK Author can approve as well.  I just need to get another 
>> Reviewer approval before integration.
>
>> > Unfortunately, I only have author status and can only comment.
>> 
>> I think OpenJDK Author can approve as well. I just need to get another 
>> Reviewer approval before integration.
> 
> For a CSR, I believe that is true. But you will need a Reviewer for the PR, 
> and they may have comments on parts that are covered by the CSR.
> 
> I would suggest moving the CSR to Proposed (which doesn't require a Reviewer 
> AFAIK). That will move it forward a bit.

> @seanjmullan Did you have cycle to review this PR and CSR? This CSR is 
> similar to the one we did for signature algorithms. I was wondering if it is 
> possible to have it in JDK 20 so that more specific TLS benchmarks could be 
> introduced sooner. Thanks!

I added my name as Reviewer to the CSR, so you can finalize that, as that will 
give you a better chance of it being approved and getting this into JDK 20. It 
will take me a little while longer to review the code changes, though I don't 
expect it will impact the API.

-

PR: https://git.openjdk.org/jdk/pull/9776


Re: RFR: 8281236: (D)TLS key exchange named groups [v3]

2022-11-20 Thread Xue-Lei Andrew Fan
On Tue, 8 Nov 2022 22:07:35 GMT, Sean Mullan  wrote:

>>> Unfortunately, I only have author status and can only comment.
>> 
>> I think OpenJDK Author can approve as well.  I just need to get another 
>> Reviewer approval before integration.
>
>> > Unfortunately, I only have author status and can only comment.
>> 
>> I think OpenJDK Author can approve as well. I just need to get another 
>> Reviewer approval before integration.
> 
> For a CSR, I believe that is true. But you will need a Reviewer for the PR, 
> and they may have comments on parts that are covered by the CSR.
> 
> I would suggest moving the CSR to Proposed (which doesn't require a Reviewer 
> AFAIK). That will move it forward a bit.

@seanjmullan Did you have cycle to review this PR and CSR?  This CSR is similar 
to the one we did for signature algorithms.  I was wondering if it is possible 
to have it in JDK 20 so that more specific TLS benchmarks could be introduced 
sooner.  Thanks!

-

PR: https://git.openjdk.org/jdk/pull/9776


Re: RFR: 8281236: (D)TLS key exchange named groups [v3]

2022-11-08 Thread Xue-Lei Andrew Fan
On Tue, 8 Nov 2022 22:07:35 GMT, Sean Mullan  wrote:

> > > Unfortunately, I only have author status and can only comment.
> > 
> > 
> > I think OpenJDK Author can approve as well. I just need to get another 
> > Reviewer approval before integration.
> 
> For a CSR, I believe that is true. But you will need a Reviewer for the PR, 
> and they may have comments on parts that are covered by the CSR.
> 

+1.

> I would suggest moving the CSR to Proposed (which doesn't require a Reviewer 
> AFAIK). That will move it forward a bit.

I changed the CSR to Proposed.  Thanks for the suggestion.

-

PR: https://git.openjdk.org/jdk/pull/9776


Re: RFR: 8281236: (D)TLS key exchange named groups [v3]

2022-11-08 Thread Sean Mullan
On Tue, 8 Nov 2022 21:52:51 GMT, Xue-Lei Andrew Fan  wrote:

> > Unfortunately, I only have author status and can only comment.
> 
> I think OpenJDK Author can approve as well. I just need to get another 
> Reviewer approval before integration.

For a CSR, I believe that is true. But you will need a Reviewer for the PR, and 
they may have comments on parts that are covered by the CSR.

I would suggest moving the CSR to Proposed (which doesn't require a Reviewer 
AFAIK). That will move it forward a bit.

-

PR: https://git.openjdk.org/jdk/pull/9776


Re: RFR: 8281236: (D)TLS key exchange named groups [v3]

2022-11-08 Thread Xue-Lei Andrew Fan
On Tue, 8 Nov 2022 21:18:20 GMT, Mark Powers  wrote:

> Unfortunately, I only have author status and can only comment.

I think OpenJDK Author can approve as well.  I just need to get another 
Reviewer approval before integration.

-

PR: https://git.openjdk.org/jdk/pull/9776


Re: RFR: 8281236: (D)TLS key exchange named groups [v3]

2022-11-08 Thread Mark Powers
On Mon, 7 Nov 2022 18:48:27 GMT, Xue-Lei Andrew Fan  wrote:

>> This update is to support key exchange named groups customization for 
>> individual (D)TLS connection. Please review the CSR as well:
>> CSR: https://bugs.openjdk.org/browse/JDK-8291950
>> RFE: https://bugs.openjdk.org/browse/JDK-8281236
>> Release-note: https://bugs.openjdk.org/browse/JDK-8291975
>> 
>> This is an effort similar to [JDK-8280494: "(D)TLS signature 
>> schemes"](https://bugs.openjdk.org/browse/JDK-8280494)
>
> Xue-Lei Andrew Fan has updated the pull request with a new target base due to 
> a merge or a rebase. The pull request now contains four commits:
> 
>  - Merge
>  - Merge
>  - add test cases
>  - 8281236: (D)TLS key exchange algorithms

Unfortunately, I only have author status and can only comment.

-

PR: https://git.openjdk.org/jdk/pull/9776


Re: RFR: 8281236: (D)TLS key exchange named groups [v3]

2022-11-08 Thread Xue-Lei Andrew Fan
On Tue, 8 Nov 2022 16:51:24 GMT, Mark Powers  wrote:

> I'll look at your CSR this morning. Make sure your SSLParameters additions 
> look correct with JavaDoc. I always forget that step.

Thank you for the suggestion.  I double checked the JavaDoc.  It looks good to 
me.

Please feel free to add your name as reviewer in the CSR and approve this PR 
when you are ready.

-

PR: https://git.openjdk.org/jdk/pull/9776


Re: RFR: 8281236: (D)TLS key exchange named groups [v3]

2022-11-08 Thread Mark Powers
On Mon, 7 Nov 2022 18:48:27 GMT, Xue-Lei Andrew Fan  wrote:

>> This update is to support key exchange named groups customization for 
>> individual (D)TLS connection. Please review the CSR as well:
>> CSR: https://bugs.openjdk.org/browse/JDK-8291950
>> RFE: https://bugs.openjdk.org/browse/JDK-8281236
>> Release-note: https://bugs.openjdk.org/browse/JDK-8291975
>> 
>> This is an effort similar to [JDK-8280494: "(D)TLS signature 
>> schemes"](https://bugs.openjdk.org/browse/JDK-8280494)
>
> Xue-Lei Andrew Fan has updated the pull request with a new target base due to 
> a merge or a rebase. The pull request now contains four commits:
> 
>  - Merge
>  - Merge
>  - add test cases
>  - 8281236: (D)TLS key exchange algorithms

I'll look at your CSR this morning. Make sure your SSLParameters additions look 
correct with JavaDoc. I always forget that step.

-

PR: https://git.openjdk.org/jdk/pull/9776


Re: RFR: 8281236: (D)TLS key exchange named groups [v3]

2022-11-07 Thread Xue-Lei Andrew Fan
On Mon, 7 Nov 2022 23:24:39 GMT, Mark Powers  wrote:

>> Xue-Lei Andrew Fan has updated the pull request with a new target base due 
>> to a merge or a rebase. The pull request now contains four commits:
>> 
>>  - Merge
>>  - Merge
>>  - add test cases
>>  - 8281236: (D)TLS key exchange algorithms
>
> Well crafted. LGTM.

Thank you very much, @mcpowers!

May I have the CSR reviewed? https://bugs.openjdk.org/browse/JDK-8291950. The 
new feature deadline is approaching.  I appreciate if the CSR could be reviewed 
in 2 weeks.

-

PR: https://git.openjdk.org/jdk/pull/9776


Re: RFR: 8281236: (D)TLS key exchange named groups [v3]

2022-11-07 Thread Mark Powers
On Mon, 7 Nov 2022 18:48:27 GMT, Xue-Lei Andrew Fan  wrote:

>> This update is to support key exchange named groups customization for 
>> individual (D)TLS connection. Please review the CSR as well:
>> CSR: https://bugs.openjdk.org/browse/JDK-8291950
>> RFE: https://bugs.openjdk.org/browse/JDK-8281236
>> Release-note: https://bugs.openjdk.org/browse/JDK-8291975
>> 
>> This is an effort similar to [JDK-8280494: "(D)TLS signature 
>> schemes"](https://bugs.openjdk.org/browse/JDK-8280494)
>
> Xue-Lei Andrew Fan has updated the pull request with a new target base due to 
> a merge or a rebase. The pull request now contains four commits:
> 
>  - Merge
>  - Merge
>  - add test cases
>  - 8281236: (D)TLS key exchange algorithms

Well crafted. LGTM.

-

PR: https://git.openjdk.org/jdk/pull/9776


Re: RFR: 8281236: (D)TLS key exchange named groups [v3]

2022-11-07 Thread Xue-Lei Andrew Fan
> This update is to support key exchange named groups customization for 
> individual (D)TLS connection. Please review the CSR as well:
> CSR: https://bugs.openjdk.org/browse/JDK-8291950
> RFE: https://bugs.openjdk.org/browse/JDK-8281236
> Release-note: https://bugs.openjdk.org/browse/JDK-8291975
> 
> This is an effort similar to [JDK-8280494: "(D)TLS signature 
> schemes"](https://bugs.openjdk.org/browse/JDK-8280494)

Xue-Lei Andrew Fan has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains four commits:

 - Merge
 - Merge
 - add test cases
 - 8281236: (D)TLS key exchange algorithms

-

Changes: https://git.openjdk.org/jdk/pull/9776/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9776&range=02
  Stats: 893 lines in 16 files changed: 657 ins; 195 del; 41 mod
  Patch: https://git.openjdk.org/jdk/pull/9776.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9776/head:pull/9776

PR: https://git.openjdk.org/jdk/pull/9776