How to help with getting KTLS patches merged

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

How to help with getting KTLS patches merged

John Baldwin
At the moment there are 3 open PRs related to Kernel TLS offload
support that I'm aware of:

- 11589 adds TLS1.3 for Linux, has one approval from Matt Caswell
- 10626 adds TLS1.3 for FreeBSD, from which 11589 is derived, but will
  need to be rebased on top of 11589 once it is merged
- 11679 adds TLS1.[012] RX offload for FreeBSD, but would conflict
  a bit with 11589

Matt has already approved 11589, and I think 11589 looks ok from
my vantage point (I'm more familiar with the KTLS infrastructure
in FreeBSD rather than Linux).  Given that 11589 has received the
most review, I think it is probably prudent for it to be merged
first at for me to then rebase the other two PRs on top of that
and resolve conflicts, etc.

Is there anything I can do to help with getting 11589 merged?  I'm
not an OpenSSL committer, so I can't formally add the second
approval it needs.  Similarly once 11589 lands, is there anything
I can do to help get review on the other two PRs once they are
updated?

Ideally I'd really like to have these three merged into 3.0 before
it is released.  I'm already maintaining a side branch of 1.1.1
with KTLS support for use in FreeBSD's ports tree, but it would be
really nice if 3.0 did not require additional patches out of the
box.

Thanks!

--
John Baldwin
Reply | Threaded
Open this post in threaded view
|

Re: How to help with getting KTLS patches merged

Kurt Roeckx
On Thu, Jun 04, 2020 at 09:00:08AM -0700, John Baldwin wrote:

> At the moment there are 3 open PRs related to Kernel TLS offload
> support that I'm aware of:
>
> - 11589 adds TLS1.3 for Linux, has one approval from Matt Caswell
> - 10626 adds TLS1.3 for FreeBSD, from which 11589 is derived, but will
>   need to be rebased on top of 11589 once it is merged
> - 11679 adds TLS1.[012] RX offload for FreeBSD, but would conflict
>   a bit with 11589
>
> Matt has already approved 11589, and I think 11589 looks ok from
> my vantage point (I'm more familiar with the KTLS infrastructure
> in FreeBSD rather than Linux).  Given that 11589 has received the
> most review, I think it is probably prudent for it to be merged
> first at for me to then rebase the other two PRs on top of that
> and resolve conflicts, etc.

11589 has now been merged.


Kurt

Reply | Threaded
Open this post in threaded view
|

Re: How to help with getting KTLS patches merged

John Baldwin
On 6/8/20 4:12 AM, Kurt Roeckx wrote:

> On Thu, Jun 04, 2020 at 09:00:08AM -0700, John Baldwin wrote:
>> At the moment there are 3 open PRs related to Kernel TLS offload
>> support that I'm aware of:
>>
>> - 11589 adds TLS1.3 for Linux, has one approval from Matt Caswell
>> - 10626 adds TLS1.3 for FreeBSD, from which 11589 is derived, but will
>>   need to be rebased on top of 11589 once it is merged
>> - 11679 adds TLS1.[012] RX offload for FreeBSD, but would conflict
>>   a bit with 11589
>>
>> Matt has already approved 11589, and I think 11589 looks ok from
>> my vantage point (I'm more familiar with the KTLS infrastructure
>> in FreeBSD rather than Linux).  Given that 11589 has received the
>> most review, I think it is probably prudent for it to be merged
>> first at for me to then rebase the other two PRs on top of that
>> and resolve conflicts, etc.
>
> 11589 has now been merged.

Thanks for all the folks who jumped in to review that!

--
John Baldwin
Reply | Threaded
Open this post in threaded view
|

Re: How to help with getting KTLS patches merged

John Baldwin
On 6/10/20 3:48 PM, John Baldwin wrote:

> On 6/8/20 4:12 AM, Kurt Roeckx wrote:
>> On Thu, Jun 04, 2020 at 09:00:08AM -0700, John Baldwin wrote:
>>> At the moment there are 3 open PRs related to Kernel TLS offload
>>> support that I'm aware of:
>>>
>>> - 11589 adds TLS1.3 for Linux, has one approval from Matt Caswell
>>> - 10626 adds TLS1.3 for FreeBSD, from which 11589 is derived, but will
>>>   need to be rebased on top of 11589 once it is merged
>>> - 11679 adds TLS1.[012] RX offload for FreeBSD, but would conflict
>>>   a bit with 11589
>>>
>>> Matt has already approved 11589, and I think 11589 looks ok from
>>> my vantage point (I'm more familiar with the KTLS infrastructure
>>> in FreeBSD rather than Linux).  Given that 11589 has received the
>>> most review, I think it is probably prudent for it to be merged
>>> first at for me to then rebase the other two PRs on top of that
>>> and resolve conflicts, etc.
>>
>> 11589 has now been merged.
>
> Thanks for all the folks who jumped in to review that!

I have one more request related to KTLS patches.

After 11589 was merged, I reworked the two FreeBSD changes and ended
up merging them into a new PR that includes some refactoring inspired
by 11589 and the original two patches (which are now much smaller).

The new PR is 12111.  One question I have is if a new PR is the
right way to handle this, and if so should I close 10626 and 11679?
I also have some open questions in 12111 about possibly reducing
OS-specific #ifdef's outside of <internal/ktls.h>.

Thanks for any feedback or comments.

--
John Baldwin
Reply | Threaded
Open this post in threaded view
|

Re: How to help with getting KTLS patches merged

Matt Caswell-2


On 23/07/2020 23:06, John Baldwin wrote:

> On 6/10/20 3:48 PM, John Baldwin wrote:
>> On 6/8/20 4:12 AM, Kurt Roeckx wrote:
>>> On Thu, Jun 04, 2020 at 09:00:08AM -0700, John Baldwin wrote:
>>>> At the moment there are 3 open PRs related to Kernel TLS offload
>>>> support that I'm aware of:
>>>>
>>>> - 11589 adds TLS1.3 for Linux, has one approval from Matt Caswell
>>>> - 10626 adds TLS1.3 for FreeBSD, from which 11589 is derived, but will
>>>>   need to be rebased on top of 11589 once it is merged
>>>> - 11679 adds TLS1.[012] RX offload for FreeBSD, but would conflict
>>>>   a bit with 11589
>>>>
>>>> Matt has already approved 11589, and I think 11589 looks ok from
>>>> my vantage point (I'm more familiar with the KTLS infrastructure
>>>> in FreeBSD rather than Linux).  Given that 11589 has received the
>>>> most review, I think it is probably prudent for it to be merged
>>>> first at for me to then rebase the other two PRs on top of that
>>>> and resolve conflicts, etc.
>>>
>>> 11589 has now been merged.
>>
>> Thanks for all the folks who jumped in to review that!
>
> I have one more request related to KTLS patches.
>
> After 11589 was merged, I reworked the two FreeBSD changes and ended
> up merging them into a new PR that includes some refactoring inspired
> by 11589 and the original two patches (which are now much smaller).
>
> The new PR is 12111.  One question I have is if a new PR is the
> right way to handle this, and if so should I close 10626 and 11679?
> I also have some open questions in 12111 about possibly reducing
> OS-specific #ifdef's outside of <internal/ktls.h>.

If its a significant refactoring of earlier PRs then I think a new PR is
appropriate - and in that case closing the earlier PRs is also fine. It
would be helpful to refer to the new PR when closing the old PR (and
vice versa) so that the history is not lost.

Matt