[MPTCP] Re: [PATCH v3 0/6] mptcp: update mptcp ack sequence from work queue

[MPTCP] Re: [PATCH v3 0/6] mptcp: update mptcp ack sequence from work queue

[Paolo Abeni]

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

On Tue, 2020-02-18 at 13:21 +0100, Florian Westphal wrote:
> This is v3 of the attempt to allow mptcp to update its ack-sequence
> number even if userspace doesn't call recv() for some time.
> This includes a change to patch 2/6 to address a theoretical race when
> two subflows deliver same data: previous version may have discarded the
> additional data correctly, but would leave EPOLLIN indication enabled,
> even though all data has to be tossed as duplicate.
> 
> Rest of patches is unchanged.

The series LGTM.

My understanding is that the patches are not going to be squashed, just
sent directly U/S, right?

If so I think the commit message in patch 2 and 3 should be cleaned-up
a bit.

Thanks,

Paolo

[MPTCP] Re: [PATCH v3 0/6] mptcp: update mptcp ack sequence from work queue

[Florian Westphal]

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

Paolo Abeni <pabeni(a)redhat.com> wrote:
> On Tue, 2020-02-18 at 13:21 +0100, Florian Westphal wrote:
> > This is v3 of the attempt to allow mptcp to update its ack-sequence
> > number even if userspace doesn't call recv() for some time.
> > This includes a change to patch 2/6 to address a theoretical race when
> > two subflows deliver same data: previous version may have discarded the
> > additional data correctly, but would leave EPOLLIN indication enabled,
> > even though all data has to be tossed as duplicate.
> > 
> > Rest of patches is unchanged.
> 
> The series LGTM.
> 
> My understanding is that the patches are not going to be squashed, just
> sent directly U/S, right?

I don't mind, whatever seems more appropriate when we roll this
out to netdev.

3 and 6 could be squashed into #2.

But for now I think they can be applied as-is.

> If so I think the commit message in patch 2 and 3 should be cleaned-up
> a bit.

3 is too brief, yes, but whats missing in #2?

[MPTCP] Re: [PATCH v3 0/6] mptcp: update mptcp ack sequence from work queue

[Paolo Abeni]

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

On Wed, 2020-02-19 at 13:43 +0100, Florian Westphal wrote:
> Paolo Abeni <pabeni(a)redhat.com> wrote:
> > If so I think the commit message in patch 2 and 3 should be cleaned-up
> > a bit.
> 
> 3 is too brief, yes, but whats missing in #2?

Nothing, but I think the changelog so-far should be removed ;)

Cheers,

Paolo

[MPTCP] Re: [PATCH v3 0/6] mptcp: update mptcp ack sequence from work queue

[Florian Westphal]

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

Florian Westphal <fw(a)strlen.de> wrote:
> V3.  No changes since v2, I only updated commit messages in
> patches 1, 2, 3 and 6.
> 
> I think we should try to avoid needless squashing and keep
> these as-is for now.  We can always condense some of this later
> on in case we need to reduce number of individual patches for some
> reason.

As per yesterdays meeting, I've rebased this on top of net-next:

https://git.breakpoint.cc/cgit/fw/net-next.git/log/?h=mptcp_ssk_drain_01

Paolo, I extracted the parts that add the work queue from your retrans
patch, I renamed the work queue because 'rtx_work' doesn't makse
sense if its not related to retransmits.

The patch is rather tiny so I wonder if it should be merged with the
main patch (mptcp: update mptcp ack sequence from work queue).

Let me know.

Also, I would propose to drop
"mptcp: protocol: re-check dsn before reading from subflow" before
sending this to netdev since that patch isn't related to the rest
of the patches.

It should probably be part of the batch after this, at the latest
it needs to go in when MP_JOIN support is added.

WDYT?

I can't post it to netdev yet anyway because I need to wait for
net -> net-next merge to avoid a small conflict in protocol.h, so
you can take your time with reviewing this.