Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Ian Campbell <Ian.Campbell@eu.citrix.com>
To: David Miller <davem@davemloft.net>
Cc: Jeremy Fitzhardinge <Jeremy.Fitzhardinge@citrix.com>,
	"konrad.wilk@oracle.com" <konrad.wilk@oracle.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"virtualization@lists.linux-foundation.org"
	<virtualization@lists.linux-foundation.org>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	"mirq-linux@rere.qmqm.pl" <mirq-linux@rere.qmqm.pl>
Subject: Re: [PATCH RESEND] net: convert xen-netfront to hw_features
Date: Mon, 4 Apr 2011 13:29:19 +0100
Message-ID: <1301920159.23887.96.camel@zakaz.uk.xensource.com> (raw)
In-Reply-To: <20110401.205455.70198735.davem@davemloft.net>

On Sat, 2011-04-02 at 04:54 +0100, David Miller wrote:
> From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Date: Thu, 31 Mar 2011 13:01:35 +0200 (CEST)
> 
> > Not tested in any way. The original code for offload setting seems broken
> > as it resets the features on every netback reconnect.
> > 
> > This will set GSO_ROBUST at device creation time (earlier than connect time).
> > 
> > RX checksum offload is forced on - so advertise as it is.
> > 
> > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> 
> Applied.

Thanks, but unfortunately the patch results in the features all being
disabled by default, since they are not set in the initial dev->features
and the initial dev->wanted_features is based on features & hw_features.
The ndo_fix_features hook only clears features and doesn't add new
features (nor should it AFAICT).

Features cannot be negotiated with the backend until xennet_connect().
The carrier is not enabled until the end of that function, therefore I
think it is safe to start with a full set of features in dev->features
and rely on the call to netdev_update_features() in xennet_connect() to
clear those which turn out to be unavailable.

The following works for me, I guess the alternative is for
xennet_connect() to expand dev->features based on what it detects? Or is
there a mechanism for a driver to inform the core that a new hardware
feature has become available (I doubt that really happens on physical
h/w so I guess not).

Ian.

8<-----------------

>From 0b56469abe56efae415b4603ef508ce9aec0e4c1 Mon Sep 17 00:00:00 2001
From: Ian Campbell <ian.campbell@citrix.com>
Date: Mon, 4 Apr 2011 10:58:50 +0100
Subject: [PATCH] xen: netfront: assume all hw features are available until backend connection setup

We need to assume that all features will be available when registering the
netdev otherwise they are ommitted from the initial set of
dev->wanted_features. When we connect to the backed we reduce the set as
necessary due to the call to netdev_update_features() in xennet_connect().

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: mirq-linux@rere.qmqm.pl
Cc: netdev@vger.kernel.org <netdev@vger.kernel.org>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: konrad.wilk@oracle.com
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: xen-devel@lists.xensource.com
---
 drivers/net/xen-netfront.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 0cfe4cc..db9a763 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1251,6 +1251,14 @@ static struct net_device * __devinit xennet_create_dev(struct xenbus_device *dev
 				  NETIF_F_GSO_ROBUST;
 	netdev->hw_features	= NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
 
+	/*
+         * Assume that all hw features are available for now. This set
+         * will be adjusted by the call to netdev_update_features() in
+         * xennet_connect() which is the earliest point where we can
+         * negotiate with the backend regarding supported features.
+         */
+	netdev->features |= netdev->hw_features;
+
 	SET_ETHTOOL_OPS(netdev, &xennet_ethtool_ops);
 	SET_NETDEV_DEV(netdev, &dev->dev);
 
-- 
1.7.2.5

  parent reply index

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-31 11:01 Michał Mirosław
2011-03-31 11:13 ` Ian Campbell
2011-03-31 11:37   ` Michał Mirosław
2011-04-02  3:54 ` David Miller
2011-04-03 11:07   ` [PATCH] xen: netfront: fix declaration order Eric Dumazet
2011-04-03 17:35     ` Michał Mirosław
2011-04-04  0:24     ` David Miller
2011-04-04  9:55       ` [PATCH] xen: drop anti-dependency on X86_VISWS (Was: Re: [PATCH] xen: netfront: fix declaration order) Ian Campbell
2011-04-06 21:45         ` [PATCH] xen: drop anti-dependency on X86_VISWS David Miller
2011-04-07  6:58           ` Ian Campbell
2011-04-07 17:00             ` H. Peter Anvin
2011-04-08  6:46               ` Ian Campbell
2011-04-08 20:15                 ` H. Peter Anvin
2011-04-07 18:07             ` Jeremy Fitzhardinge
2011-04-08  6:38               ` Ian Campbell
2011-04-08 15:25                 ` Jeremy Fitzhardinge
2011-04-08 15:42                   ` Jan Beulich
2011-04-08 18:24                     ` Jeremy Fitzhardinge
2011-04-09 10:34                       ` Ian Campbell
2011-04-14  8:20                         ` Jeremy Fitzhardinge
2011-04-04 12:29   ` Ian Campbell [this message]
2011-04-04 18:08     ` [PATCH RESEND] net: convert xen-netfront to hw_features 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=1301920159.23887.96.camel@zakaz.uk.xensource.com \
    --to=ian.campbell@eu.citrix.com \
    --cc=Jeremy.Fitzhardinge@citrix.com \
    --cc=davem@davemloft.net \
    --cc=konrad.wilk@oracle.com \
    --cc=mirq-linux@rere.qmqm.pl \
    --cc=netdev@vger.kernel.org \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=xen-devel@lists.xensource.com \
    /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

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git