netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Vrabel <david.vrabel@citrix.com>
To: Wei Liu <wei.liu2@citrix.com>
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Ian Campbell <Ian.Campbell@citrix.com>,
	"annie.li@oracle.com" <annie.li@oracle.com>,
	"konrad.wilk@oracle.com" <konrad.wilk@oracle.com>
Subject: Re: [PATCH 5/6] xen-netback: coalesce slots before copying
Date: Tue, 26 Mar 2013 18:02:34 +0000	[thread overview]
Message-ID: <5151E2BA.6070703@citrix.com> (raw)
In-Reply-To: <1364209702-12437-6-git-send-email-wei.liu2@citrix.com>

On 25/03/13 11:08, Wei Liu wrote:
> This patch tries to coalesce tx requests when constructing grant copy
> structures. It enables netback to deal with situation when frontend's
> MAX_SKB_FRAGS is larger than backend's MAX_SKB_FRAGS.
> 
> It defines max_skb_slots, which is a estimation of the maximum number of slots
> a guest can send, anything bigger than that is considered malicious. Now it is
> set to 20, which should be enough to accommodate Linux (16 to 19).
> 
> Also change variable name from "frags" to "slots" in netbk_count_requests.

It it worth summarizing an (off-line) discussion I had with Wei on this
patch.

There are two regression that need to be addressed here.

1. The reduction of the number of supported ring entries (slots) per
packet (from 18 to 17).

2. The XSA-39 security fix turning "too many frags" errors from just
dropping the packet to a fatal error and disabling the VIF.

The key root cause of the problem is that the protocol is poorly
specified using a property external to the netback/netfront drivers
(i.e., MAX_SKB_FRAGS).

A secondary problem is some frontends have used a max slots per packet
value that is larger than netback as supported. e.g., the Windows GPLPV
drivers use up to 19.  These packets have always been dropped.

The first step is to properly specify the maximum slots per-packet as
part of the interface.  This should be specified as 18 (the historical
value).

The second step is to define a threshold for slots per packet, above
which the guest is considered to be malicious and the error is fatal.
20 seems a sensible value here.

The behavior of netback for packet is thus:

    1-18 slots: valid
   19-20 slots: drop and respond with an error
   21+   slots: fatal error

Note that we do not make 19-20 valid as this is a change to the protocol
and guests may end up relying on this which would then break the guests
if they migrate or start on host with a limit of 18.

A third (and future) step would be to investigate whether increasing the
slots per-packet limit is sensible.  There would then need to be a
mechanism to negotiate this limit between the front and back ends.

David

  parent reply	other threads:[~2013-03-26 18:02 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-25 11:08 [PATCH 0/6 V2] Bundle fixes for Xen netfront / netback Wei Liu
2013-03-25 11:08 ` [PATCH 1/6] xen-netfront: remove unused variable `extra' Wei Liu
2013-03-25 14:21   ` [Xen-devel] " David Vrabel
2013-03-25 16:20   ` David Miller
2013-03-25 11:08 ` [PATCH 2/6] xen-netfront: reduce gso_max_size to account for ethernet header Wei Liu
2013-03-25 14:23   ` [Xen-devel] " David Vrabel
2013-03-25 14:39     ` Jan Beulich
2013-03-25 15:50   ` Sergei Shtylyov
2013-03-25 16:18   ` David Miller
2013-03-25 16:54     ` Eric Dumazet
2013-03-25 16:59       ` David Miller
2013-03-25 17:24         ` Eric Dumazet
2013-03-25 20:49         ` Ben Hutchings
2013-03-25 18:32     ` Wei Liu
2013-03-25 18:39       ` David Miller
2013-03-25 16:56   ` Konrad Rzeszutek Wilk
2013-03-25 11:08 ` [PATCH 3/6] xen-netfront: frags -> slots in xennet_get_responses Wei Liu
2013-03-25 14:25   ` [Xen-devel] " David Vrabel
2013-03-25 16:20   ` David Miller
2013-03-25 11:08 ` [PATCH 4/6] xen-netback: remove skb in xen_netbk_alloc_page Wei Liu
2013-03-25 16:20   ` David Miller
2013-03-25 11:08 ` [PATCH 5/6] xen-netback: coalesce slots before copying Wei Liu
2013-03-25 15:13   ` David Vrabel
2013-03-25 15:47     ` [Xen-devel] " Wei Liu
2013-03-25 16:34       ` David Vrabel
2013-03-25 16:58         ` Wei Liu
2013-03-25 17:06           ` [Xen-devel] " Wei Liu
2013-03-25 18:29           ` David Vrabel
2013-03-25 19:09             ` Wei Liu
2013-03-26  9:16               ` Paul Durrant
2013-03-26  9:51                 ` Wei Liu
2013-03-26 11:00                 ` James Harper
2013-03-26 11:24                   ` Paul Durrant
2013-03-26 11:29                     ` James Harper
2013-03-26 11:38                       ` Paul Durrant
2013-03-26 11:27                   ` David Laight
2013-03-26 10:52               ` James Harper
2013-03-26 11:06                 ` David Vrabel
2013-03-26 11:15                   ` James Harper
2013-03-26 11:13               ` David Vrabel
2013-03-26 11:29                 ` Wei Liu
2013-03-25 16:20   ` David Miller
2013-03-25 16:57   ` [Xen-devel] " Konrad Rzeszutek Wilk
2013-04-09 15:10     ` Ian Campbell
2013-03-26 18:02   ` David Vrabel [this message]
2013-03-25 11:08 ` [PATCH 6/6] xen-netback: don't disconnect frontend when seeing oversize frame Wei Liu
2013-03-25 11:47   ` David Vrabel
2013-03-25 12:00     ` Wei Liu
2013-03-25 12:53   ` [Xen-devel] " Jan Beulich
2013-03-25 13:52     ` Wei Liu
2013-03-25 16:21   ` David Miller
2013-03-25 16:58   ` [Xen-devel] " Konrad Rzeszutek Wilk
2013-03-27 10:03     ` William Dauchy

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=5151E2BA.6070703@citrix.com \
    --to=david.vrabel@citrix.com \
    --cc=Ian.Campbell@citrix.com \
    --cc=annie.li@oracle.com \
    --cc=konrad.wilk@oracle.com \
    --cc=netdev@vger.kernel.org \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.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).