All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Westphal <fw at strlen.de>
To: mptcp at lists.01.org
Subject: Re: [MPTCP] [PRE-RFC 0/6] mptcp: implement retransmit infrastructure
Date: Mon, 19 Aug 2019 13:06:54 +0200	[thread overview]
Message-ID: <20190819110654.GC2588@breakpoint.cc> (raw)
In-Reply-To: cover.1565970525.git.pabeni@redhat.com

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

Paolo Abeni <pabeni(a)redhat.com> wrote:
> As suggested by the subj prefix, this is a very early draft, mainly to
> discuss design decisions and/or architectural issues. This is only
> compile-tested so far.
> 
> In the current status:
> - updates the MPTCP unacked sequence number (patches 1-3) on sendmsg()
>   - also try to cope with 32 bit sequence number
> - queue data (page frags) in MPTCP retransmission queue (patch 4)
> - cleanup acked data from retransmission queue and schedule a timeout to
>    update MPTCP unacked sequence number (patch 5)
> - do memory accounting for the MPTCP rtx queue (patch 6)

Can you explain how this rtx queue is expected to be used?

At this time, we only use one tcp connection for sending data, so
we can rely on tcp level retransmits.

Is this for future failover case?

> Open points:
> - what about the current seq_una update schema (patch 2-3)? too much complex?
>    too much loose? too much generic?
> - is it too early to plug any heuristic inside the code? (as done in patch 3)

As long as they don't expose a particular behaviour i think its fine, it
can be tweaked/refined later.

> - do we really need a timer to update the unacked sequence?
>   - we can avoid it with an additional atomic operation in
>     mptcp_incoming_options() - either using an atomic type for 'snd_una' or
>     use an additional spin lock to protect it (and possibly other fields)[2]

I think that using a timer is very strange.  I will comment in the patch
itself.

>   - still we will likely need a timer to detect that a subflow has become
>     unusable e.g. due to link down event on the peer side. Should we rely on
>     explicit MPTCP notification only (e.g. DEL_ADDR sub-option)?

Ah, that answers my question above.  I don't think we can rely on
on-wire notifications only.  Client can e.g. move outside of wifi range
for example, so we would only notice that the tcp connection is
"blocked" (e.g. outstanding data with no further acks coming in).

> - retransmission code needs to run in process context, as we will need to
>   acquire msk socket lock and ssh socket lock in order.
>   - As MPTCP-level retransmission should be considerably less frequent than
>     TCP-level retransmission-timeout, we could use/schedule a workqueue for
>     that.

Agree, workqueue seems right for this.

             reply	other threads:[~2019-08-19 11:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-19 11:06 Florian Westphal [this message]
  -- strict thread matches above, loose matches on Subject: below --
2019-08-19 14:35 [MPTCP] [PRE-RFC 0/6] mptcp: implement retransmit infrastructure Florian Westphal
2019-08-19 13:02 Paolo Abeni
2019-08-16 16:47 Paolo Abeni

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190819110654.GC2588@breakpoint.cc \
    --to=unknown@example.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.