From: Johannes Berg <johannes@sipsolutions.net>
To: Oleksij Rempel <o.rempel@pengutronix.de>,
mkl@pengutronix.de, "David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Oliver Hartkopp <socketcan@hartkopp.net>,
Robin van der Gracht <robin@protonic.nl>
Cc: kernel@pengutronix.de, linux-can@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Eric Dumazet <edumazet@google.com>,
linux-wireless@vger.kernel.org
Subject: Re: [PATCH net v1 3/3] [RFC] mac80211: ieee80211_store_ack_skb(): make use of skb_clone_sk_optional()
Date: Mon, 22 Feb 2021 17:30:59 +0100 [thread overview]
Message-ID: <3823be537c3c138de90154835573113c6577188e.camel@sipsolutions.net> (raw)
In-Reply-To: <20210222151247.24534-4-o.rempel@pengutronix.de>
On Mon, 2021-02-22 at 16:12 +0100, Oleksij Rempel wrote:
> This code is trying to clone the skb with optional skb->sk. But this
> will fail to clone the skb if socket was closed just after the skb was
> pushed into the networking stack.
Which IMHO is completely fine. If we then still clone the SKB we can't
do anything with it, since the point would be to ... send it back to the
socket, but it's gone.
Nothing to fix here, I'd think. If you wanted to get a copy back that
gives you the status of the SKB, it should not come as a huge surprise
that you have to keep the socket open for that :)
Having the ACK skb will just make us do more work by handing it back
to skb_complete_wifi_ack() at TX status time, which is supposed to put
it into the socket's error queue, but if the socket is closed ... no
point in that.
johannes
next prev parent reply other threads:[~2021-02-22 16:35 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-22 15:12 [PATCH net v1 0/3] add support for skb with sk ref cloning Oleksij Rempel
2021-02-22 15:12 ` [PATCH net v1 1/3] skbuff: skb_clone_sk_optional(): add function to always clone a skb and increase refcount on sk if valid Oleksij Rempel
2021-02-22 15:12 ` [PATCH net v1 2/3] can: fix ref count warning if socket was closed before skb was cloned Oleksij Rempel
2021-02-22 15:12 ` [PATCH net v1 3/3] [RFC] mac80211: ieee80211_store_ack_skb(): make use of skb_clone_sk_optional() Oleksij Rempel
2021-02-22 16:30 ` Johannes Berg [this message]
2021-02-22 18:51 ` Marc Kleine-Budde
2021-02-23 9:47 ` Johannes Berg
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=3823be537c3c138de90154835573113c6577188e.camel@sipsolutions.net \
--to=johannes@sipsolutions.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kernel@pengutronix.de \
--cc=kuba@kernel.org \
--cc=linux-can@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=mkl@pengutronix.de \
--cc=netdev@vger.kernel.org \
--cc=o.rempel@pengutronix.de \
--cc=robin@protonic.nl \
--cc=socketcan@hartkopp.net \
/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).