From: David Vrabel <david.vrabel@citrix.com>
To: Ian Campbell <ian.campbell@citrix.com>,
David Vrabel <david.vrabel@citrix.com>
Cc: <netdev@vger.kernel.org>, Wei Liu <wei.liu2@citrix.com>,
<xen-devel@lists.xenproject.org>
Subject: Re: [Xen-devel] [PATCHv1 2/2] xen-netback: unref frags when handling a from-guest skb with a frag list
Date: Wed, 4 Mar 2015 09:56:59 +0000 [thread overview]
Message-ID: <54F6D6EB.5020104@citrix.com> (raw)
In-Reply-To: <1425462482.25940.93.camel@citrix.com>
On 04/03/15 09:48, Ian Campbell wrote:
> On Tue, 2015-03-03 at 16:26 +0000, David Vrabel wrote:
>> Every time a VIF is destroyed up-to 256 pages may be leaked if packets
>> with more than MAX_SKB_FRAGS frags where transmitted from the guest.
>> Even worse, if another user of ballooned pages allocated one of these
>> ballooned pages it would not handle the the unexpectedly non-zero page
>> count (e.g., gntdev would deadlock when unmapping a grant because the
>> page count would never reach 1).
>>
>> When handling a from-guest skb with a frag list, unref the frags
>> before releasing them so they are freed correctly when the VIF is
>> destroyed.
>
> Am I right that the majority of the first 2 hunks (and various bits of
> the 3rd) are just switching the outer loop to nr_frags instead of i, to
> free up i for use in the new code below? And also to switch j to the now
> available i in the inner loop.
Yes. If you prefer I can split this into one patch that adds the
skb_frag_unref() calls and one that reorders/refactors.
>> Also swap over to the new (local) frags /after/ calling the skb
>> destructor. This isn't strictly necessary but it's less confusing.
>
> My only concern would be that this now means there is a period where the
> frags list is invalid. However I think the calling context is such that
> nobody else can have a reference to an skb which has the same shinfo as
> the one in our hand. Is that right?
That is correct. This skb is allocated by netback and has not yet been
passed up to the network stack.
David
next prev parent reply other threads:[~2015-03-04 9:57 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-03 16:26 [PATCHv1 0/2 net] xen-netback: fix ethtool stats and memory David Vrabel
2015-03-03 16:26 ` [PATCHv1 1/2] xen-netback: return correct ethtool stats David Vrabel
2015-03-04 12:50 ` Sergei Shtylyov
2015-03-03 16:26 ` [PATCHv1 2/2] xen-netback: unref frags when handling a from-guest skb with a frag list David Vrabel
2015-03-03 16:56 ` [Xen-devel] " Zoltan Kiss
2015-03-04 9:48 ` Ian Campbell
2015-03-04 9:56 ` David Vrabel [this message]
2015-03-04 10:02 ` [Xen-devel] " Ian Campbell
2015-03-04 5:22 ` [PATCHv1 0/2 net] xen-netback: fix ethtool stats and memory David Miller
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=54F6D6EB.5020104@citrix.com \
--to=david.vrabel@citrix.com \
--cc=ian.campbell@citrix.com \
--cc=netdev@vger.kernel.org \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xenproject.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).