From: Paolo Abeni <pabeni@redhat.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
mptcp@lists.01.org
Subject: Re: [PATCH net 0/2] mptcp: some fallback fixes
Date: Mon, 12 Oct 2020 10:01:10 +0200 [thread overview]
Message-ID: <c5ea3f4a906c0dcd7b53b0dd70fc13eb3d2386e2.camel@redhat.com> (raw)
In-Reply-To: <20201010111332.3ac29a5f@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Sat, 2020-10-10 at 11:13 -0700, Jakub Kicinski wrote:
> On Fri, 9 Oct 2020 18:59:59 +0200 Paolo Abeni wrote:
> > pktdrill pointed-out we currently don't handle properly some
> > fallback scenario for MP_JOIN subflows
> >
> > The first patch addresses such issue.
> >
> > Patch 2/2 fixes a related pre-existing issue that is more
> > evident after 1/2: we could keep using for MPTCP signaling
> > closed subflows.
>
> Applied, thanks Paolo.
>
> You already have a few of those in the code, but:
>
> + if (... &&
> + schedule_work(&mptcp_sk(sk)->work))
> + sock_hold(sk);
>
> isn't this a fairly questionable construct?
>
> You take a reference for the async work to release _after_ you
> scheduled the async work?
Thank you for reviewing! Indeed we need to add some comments there:
IIRC that chunk already raised a question in the past.
Afaics, that is safe because the caller (a subflow) held a reference to
sk and sk can't be freed in between the scheduling and the next
sock_hold().
We have a pending refactor, targeting the next development cycle, that
will consolidate the workqueue scheduling into an helper. We will add
some comments there to clarify the above.
Thanks,
Paolo
prev parent reply other threads:[~2020-10-12 8:01 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-09 16:59 [PATCH net 0/2] mptcp: some fallback fixes Paolo Abeni
2020-10-09 17:00 ` [PATCH net 1/2] mptcp: fix fallback for MP_JOIN subflows Paolo Abeni
2020-10-09 17:00 ` [PATCH net 2/2] mptcp: subflows garbage collection Paolo Abeni
2020-10-10 18:13 ` [PATCH net 0/2] mptcp: some fallback fixes Jakub Kicinski
2020-10-12 8:01 ` Paolo Abeni [this message]
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=c5ea3f4a906c0dcd7b53b0dd70fc13eb3d2386e2.camel@redhat.com \
--to=pabeni@redhat.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=mptcp@lists.01.org \
--cc=netdev@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).