Re: [MPTCP] [PATCH MPTCP 0/2] mptcp: token api refactoring/cleanup

Re: [MPTCP] [PATCH MPTCP 0/2] mptcp: token api refactoring/cleanup

[Matthieu Baerts]

[-- Attachment #1: Type: text/plain, Size: 1336 bytes --]

Hi Florian, Peter,

On 05/09/2019 19:25, Peter Krystad wrote:
> 
> On Mon, 2019-09-02 at 13:09 +0200, Florian Westphal wrote:
>> I tried to fold the earlier 'RFC v2' in a way so that it allows
>> easy squashing.
>>
>> Patch #1 is the only one where this is painless.
>> For all others, things are not easy because conflicts exist all
>> over the place.
>>
>> So I see two ways forward:
>> 1. I rebase this myself and offer a pull request for the resulting series
>> 2. Only squash the first patch and keep the rest as extra patch.
>>
>> I suggest we go with #2.  The export tree already has a few patches
>> that need to be squashed anyway before sending a formal series.
>>
> This looks good and I'm fine with the extra patch for now, would just be too
> hard to split this up and squash to all kinds of different places.

Again, thank you for the patches and reviews!

- d02941b33b1a: first patch "squashed" into "mptcp: Add key generation
and token tree"
- 4b56d876cb96: signed-off
- f491c702f9c3..02bf3cf7f6d2: result of the first patch

The second patch has been added at the end.

Tests are still OK!

Cheers,
Matt
-- 
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium

Re: [MPTCP] [PATCH MPTCP 0/2] mptcp: token api refactoring/cleanup

[Peter Krystad]

[-- Attachment #1: Type: text/plain, Size: 1413 bytes --]


On Mon, 2019-09-02 at 13:09 +0200, Florian Westphal wrote:
> I tried to fold the earlier 'RFC v2' in a way so that it allows
> easy squashing.
> 
> Patch #1 is the only one where this is painless.
> For all others, things are not easy because conflicts exist all
> over the place.
> 
> So I see two ways forward:
> 1. I rebase this myself and offer a pull request for the resulting series
> 2. Only squash the first patch and keep the rest as extra patch.
> 
> I suggest we go with #2.  The export tree already has a few patches
> that need to be squashed anyway before sending a formal series.
> 
This looks good and I'm fine with the extra patch for now, would just be too
hard to split this up and squash to all kinds of different places.

Peter.

> Also, the export branch is currently not bisectable, I get build
> errors mid-series.
> 
> I don't think this is a problem for now, I think it makes sense to
> not squash the larger patches for the time being.
> 
> Once another patch series is to be submitted to netdev, the export
> branch can be renamed/archived and a reorganisation can be done at that
> point (and the result diff'd versus the old export branch to make sure
> there is no difference in resulting code).
> 
> 
> _______________________________________________
> mptcp mailing list
> mptcp(a)lists.01.org
> https://lists.01.org/mailman/listinfo/mptcp

Re: [MPTCP] [PATCH MPTCP 0/2] mptcp: token api refactoring/cleanup

[Florian Westphal]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

Matthieu Baerts <matthieu.baerts(a)tessares.net> wrote:
> >> That has to happen :/
> > 
> > Why?
> 
> I was suspecting that with the all the squashes and rebase we did and
> continue to do. If we do not monitor this, we will have more issues :)
> On my side, I don't compile each commit we export.

Argh, ok, sorry -- I misunderstood what you said :-)
Until we go upstream with this the intermediate breakage is fine of
course, no need to do anything right now.

Thanks for the pad update!

Re: [MPTCP] [PATCH MPTCP 0/2] mptcp: token api refactoring/cleanup

[Matthieu Baerts]

[-- Attachment #1: Type: text/plain, Size: 3786 bytes --]

Hi Florian,

Thank you for your quick answer!

On 04/09/2019 13:43, Florian Westphal wrote:
> Matthieu Baerts <matthieu.baerts(a)tessares.net> wrote:
>> I only propose to squash commits to ease the work for later and still
>> have a patch-set that can be easy to read, not ending with a lot of "to
>> squash" patches. But here I understand that there are conflicts everywhere.
> 
> I have no problem with squashing the 'obvious' patches.
> 
>> Another possibility is to do the "rebase" directly in the TopGit tree if
>> you prefer. I have no issue to give you the "push" permissions and you
>> have experience with TopGit. But I can also understand that you prefer
>> to postpone the rebase for later (or you don't like TopGit :) )
> 
> I would have to take a look at topgit again.  I used it for a short
> period but that was more than 6 years ago.

I guess the logic is still the same but they improved the workflow, more
tools and better speed: https://github.com/mackyle/topgit

>>> The export tree already has a few patches
>>> that need to be squashed anyway before sending a formal series.
>>
>> That's true. If we want to change that before, simply tell me which
>> patches have to be squashed and I can already do the work :)
>>
>>> Also, the export branch is currently not bisectable, I get build
>>> errors mid-series.
>>
>> That has to happen :/
> 
> Why?

I was suspecting that with the all the squashes and rebase we did and
continue to do. If we do not monitor this, we will have more issues :)
On my side, I don't compile each commit we export.

>> Is it something very important if we send the whole patch-set upstream?
> 
> Yes, each step on the way needs to result in a compileable tree.
> Otherwise, other developers doing 'git bisect' on the kernel later will
> may hit this.

It makes sense, thank you!

>> I guess it is better to have "consistent" patches and we will certainly
>> fix that but the goal is to merge the whole series.
> 
> Of course.  Note that its not a problem to add a function in patch n
> and then only add a user in patch n+1 later in the series.

Thank you for your answer!

>>> Once another patch series is to be submitted to netdev, the export
>>> branch can be renamed/archived and a reorganisation can be done at that
>>> point (and the result diff'd versus the old export branch to make sure
>>> there is no difference in resulting code).
>>
>> Sounds good to me!
>> We can even re-create another TopGit tree in parallel after the rebase
>> if we think that it will take some times before being accepted upstream.
> 
> Thats good to know.
> 
>> Another question linked to that: do you think it could be possible and a
>> good idea to push our code upstream ASAP with known limitations and
>> continue our work directly in a dedicated repo from git.kernel.org with
>> "regular" merge request for net-next?
> 
> I think its good to merge early, but it needs to be well
> understood/described where the limits are.
> 
> At the moment I don't think we're there yet.
> 
> Once mptcp_poll is a bit saner (I added a note to the pad and will be
> working on this), we've sorted out token refcounting (also working on
> this), Paolos retrans patches are in *and* we can deal with data
> arriving on multiple subflows I think we'd be good for a non-rfc
> patchset.
> 
> IPv6 would be nice but I don't think its a deal-breaker if we'd
> place it on the immediate post-merge todo list.

Excellent, thank you for sharing this with us! I also added this to the pad!

Cheers,
Matt
-- 
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium

Re: [MPTCP] [PATCH MPTCP 0/2] mptcp: token api refactoring/cleanup

[Florian Westphal]

[-- Attachment #1: Type: text/plain, Size: 2934 bytes --]

Matthieu Baerts <matthieu.baerts(a)tessares.net> wrote:
> I only propose to squash commits to ease the work for later and still
> have a patch-set that can be easy to read, not ending with a lot of "to
> squash" patches. But here I understand that there are conflicts everywhere.

I have no problem with squashing the 'obvious' patches.

> Another possibility is to do the "rebase" directly in the TopGit tree if
> you prefer. I have no issue to give you the "push" permissions and you
> have experience with TopGit. But I can also understand that you prefer
> to postpone the rebase for later (or you don't like TopGit :) )

I would have to take a look at topgit again.  I used it for a short
period but that was more than 6 years ago.

> > The export tree already has a few patches
> > that need to be squashed anyway before sending a formal series.
> 
> That's true. If we want to change that before, simply tell me which
> patches have to be squashed and I can already do the work :)
> 
> > Also, the export branch is currently not bisectable, I get build
> > errors mid-series.
> 
> That has to happen :/

Why?

> Is it something very important if we send the whole patch-set upstream?

Yes, each step on the way needs to result in a compileable tree.
Otherwise, other developers doing 'git bisect' on the kernel later will
may hit this.

> I guess it is better to have "consistent" patches and we will certainly
> fix that but the goal is to merge the whole series.

Of course.  Note that its not a problem to add a function in patch n
and then only add a user in patch n+1 later in the series.

> > Once another patch series is to be submitted to netdev, the export
> > branch can be renamed/archived and a reorganisation can be done at that
> > point (and the result diff'd versus the old export branch to make sure
> > there is no difference in resulting code).
> 
> Sounds good to me!
> We can even re-create another TopGit tree in parallel after the rebase
> if we think that it will take some times before being accepted upstream.

Thats good to know.

> Another question linked to that: do you think it could be possible and a
> good idea to push our code upstream ASAP with known limitations and
> continue our work directly in a dedicated repo from git.kernel.org with
> "regular" merge request for net-next?

I think its good to merge early, but it needs to be well
understood/described where the limits are.

At the moment I don't think we're there yet.

Once mptcp_poll is a bit saner (I added a note to the pad and will be
working on this), we've sorted out token refcounting (also working on
this), Paolos retrans patches are in *and* we can deal with data
arriving on multiple subflows I think we'd be good for a non-rfc
patchset.

IPv6 would be nice but I don't think its a deal-breaker if we'd
place it on the immediate post-merge todo list.

Re: [MPTCP] [PATCH MPTCP 0/2] mptcp: token api refactoring/cleanup

[Matthieu Baerts]

[-- Attachment #1: Type: text/plain, Size: 2668 bytes --]

Hi Florian,

On 02/09/2019 13:09, Florian Westphal wrote:
> I tried to fold the earlier 'RFC v2' in a way so that it allows
> easy squashing.
> 
> Patch #1 is the only one where this is painless.
> For all others, things are not easy because conflicts exist all
> over the place.
> 
> So I see two ways forward:
> 1. I rebase this myself and offer a pull request for the resulting series
> 2. Only squash the first patch and keep the rest as extra patch.
> 
> I suggest we go with #2.

Sounds good to me. Is it OK for others if we do that?

I only propose to squash commits to ease the work for later and still
have a patch-set that can be easy to read, not ending with a lot of "to
squash" patches. But here I understand that there are conflicts everywhere.

Another possibility is to do the "rebase" directly in the TopGit tree if
you prefer. I have no issue to give you the "push" permissions and you
have experience with TopGit. But I can also understand that you prefer
to postpone the rebase for later (or you don't like TopGit :) )

> The export tree already has a few patches
> that need to be squashed anyway before sending a formal series.

That's true. If we want to change that before, simply tell me which
patches have to be squashed and I can already do the work :)

> Also, the export branch is currently not bisectable, I get build
> errors mid-series.

That has to happen :/

Is it something very important if we send the whole patch-set upstream?
I guess it is better to have "consistent" patches and we will certainly
fix that but the goal is to merge the whole series.

> I don't think this is a problem for now, I think it makes sense to
> not squash the larger patches for the time being.

I am fine with that.

> Once another patch series is to be submitted to netdev, the export
> branch can be renamed/archived and a reorganisation can be done at that
> point (and the result diff'd versus the old export branch to make sure
> there is no difference in resulting code).

Sounds good to me!
We can even re-create another TopGit tree in parallel after the rebase
if we think that it will take some times before being accepted upstream.

Another question linked to that: do you think it could be possible and a
good idea to push our code upstream ASAP with known limitations and
continue our work directly in a dedicated repo from git.kernel.org with
"regular" merge request for net-next?

Cheers,
Matt
-- 
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium

[MPTCP] [PATCH MPTCP 0/2] mptcp: token api refactoring/cleanup

[Florian Westphal]

[-- Attachment #1: Type: text/plain, Size: 995 bytes --]

I tried to fold the earlier 'RFC v2' in a way so that it allows
easy squashing.

Patch #1 is the only one where this is painless.
For all others, things are not easy because conflicts exist all
over the place.

So I see two ways forward:
1. I rebase this myself and offer a pull request for the resulting series
2. Only squash the first patch and keep the rest as extra patch.

I suggest we go with #2.  The export tree already has a few patches
that need to be squashed anyway before sending a formal series.

Also, the export branch is currently not bisectable, I get build
errors mid-series.

I don't think this is a problem for now, I think it makes sense to
not squash the larger patches for the time being.

Once another patch series is to be submitted to netdev, the export
branch can be renamed/archived and a reorganisation can be done at that
point (and the result diff'd versus the old export branch to make sure
there is no difference in resulting code).