From: Ian Campbell <ian.campbell@citrix•com>
To: 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 10:02:01 +0000 [thread overview]
Message-ID: <1425463321.25940.103.camel@citrix.com> (raw)
In-Reply-To: <54F6D6EB.5020104@citrix.com>
On Wed, 2015-03-04 at 09:56 +0000, David Vrabel wrote:
> 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.
If you can be bothered that might make things easier to read, thanks.
> >> 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.
Phew! ;-)
next prev parent reply other threads:[~2015-03-04 10:02 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 ` [Xen-devel] " David Vrabel
2015-03-04 10:02 ` Ian Campbell [this message]
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=1425463321.25940.103.camel@citrix.com \
--to=ian.campbell@citrix$(echo .)com \
--cc=david.vrabel@citrix$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=wei.liu2@citrix$(echo .)com \
--cc=xen-devel@lists$(echo .)xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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