[PATCH] Make frontend features distinct from netback feature flags.

xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed

* [PATCH] Make frontend features distinct from netback feature flags.
@ 2010-07-02 14:55 Paul Durrant
  0 siblings, 0 replies; 6+ messages in thread
From: Paul Durrant @ 2010-07-02 14:55 UTC (permalink / raw)
  To: xen-devel, jeremy; +Cc: Paul Durrant, Ian Campbell

Make sure that if a feature flag is disabled by ethtool on netback
that we do not gratuitously re-enabled it when we check the frontend
features during ring connection.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/netback/common.h    |   15 ++++++---
 drivers/xen/netback/interface.c |   68 ++++++++++++++++++++++++++++++--------
 drivers/xen/netback/netback.c   |    2 +-
 drivers/xen/netback/xenbus.c    |   51 +++++++++++------------------
 4 files changed, 83 insertions(+), 53 deletions(-)

diff --git a/drivers/xen/netback/common.h b/drivers/xen/netback/common.h
index 1cbc4ff..a673331 100644
--- a/drivers/xen/netback/common.h
+++ b/drivers/xen/netback/common.h
@@ -76,14 +76,18 @@ struct xen_netif {
 	struct vm_struct *tx_comms_area;
 	struct vm_struct *rx_comms_area;
 
-	/* Set of features that can be turned on in dev->features. */
-	int features;
+    /* Flags that must not be set in dev->features */
+    int features_disabled;
 
-	int smart_poll;
+	/* Frontend feature information. */
+    u8 can_sg:1;
+    u8 gso:1;
+    u8 gso_prefix:1;
+    u8 csum:1;
+    u8 smart_poll:1;
 
 	/* Internal feature information. */
 	u8 can_queue:1;	    /* can queue packets for receiver? */
-	u8 gso_prefix:1;    /* use a prefix segment for GSO information */
 
 	/* Allow netif_be_start_xmit() to peek ahead in the rx request
 	 * ring.  This is a prediction of what rx_req_cons will be once
@@ -189,6 +193,7 @@ void netif_accel_init(void);
 
 void netif_disconnect(struct xen_netif *netif);
 
+void netif_set_features(struct xen_netif *netif);
 struct xen_netif *netif_alloc(struct device *parent, domid_t domid, unsigned int handle);
 int netif_map(struct xen_netif *netif, unsigned long tx_ring_ref,
 	      unsigned long rx_ring_ref, unsigned int evtchn);
@@ -225,7 +230,7 @@ static inline int netbk_can_queue(struct net_device *dev)
 static inline int netbk_can_sg(struct net_device *dev)
 {
 	struct xen_netif *netif = netdev_priv(dev);
-	return netif->features & NETIF_F_SG;
+	return netif->can_sg;
 }
 
 struct pending_tx_info {
diff --git a/drivers/xen/netback/interface.c b/drivers/xen/netback/interface.c
index 172ef4c..2e8508a 100644
--- a/drivers/xen/netback/interface.c
+++ b/drivers/xen/netback/interface.c
@@ -121,31 +121,69 @@ static int netbk_change_mtu(struct net_device *dev, int mtu)
 	return 0;
 }
 
-static int netbk_set_sg(struct net_device *dev, u32 data)
+void netif_set_features(struct xen_netif *netif)
 {
-	if (data) {
-		struct xen_netif *netif = netdev_priv(dev);
+	struct net_device *dev = netif->dev;
+	int features = dev->features;
+
+	if (netif->can_sg)
+		features |= NETIF_F_SG;
+	if (netif->gso || netif->gso_prefix)
+		features |= NETIF_F_TSO;
+	if (netif->csum)
+		features |= NETIF_F_IP_CSUM;
+
+	features &= ~(netif->features_disabled);
 
-		if (!(netif->features & NETIF_F_SG))
+	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN)
+		dev->mtu = ETH_DATA_LEN;
+
+	dev->features = features;
+}
+
+static int netbk_set_tx_csum(struct net_device *dev, u32 data)
+{
+	struct xen_netif *netif = netdev_priv(dev);
+	if (data) {
+		if (!netif->csum)
 			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_IP_CSUM;
+	} else {
+		netif->features_disabled |= NETIF_F_IP_CSUM;
 	}
 
-	if (dev->mtu > ETH_DATA_LEN)
-		dev->mtu = ETH_DATA_LEN;
+	netif_set_features(netif);
+	return 0;
+}
 
-	return ethtool_op_set_sg(dev, data);
+static int netbk_set_sg(struct net_device *dev, u32 data)
+{
+	struct xen_netif *netif = netdev_priv(dev);
+	if (data) {
+		if (!netif->can_sg)
+			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_SG;
+	} else {
+		netif->features_disabled |= NETIF_F_SG;
+	}
+
+	netif_set_features(netif);
+	return 0;
 }
 
 static int netbk_set_tso(struct net_device *dev, u32 data)
 {
+	struct xen_netif *netif = netdev_priv(dev);
 	if (data) {
-		struct xen_netif *netif = netdev_priv(dev);
-
-		if (!(netif->features & NETIF_F_TSO))
+		if (!netif->gso && !netif->gso_prefix)
 			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_TSO;
+	} else {
+		netif->features_disabled |= NETIF_F_TSO;
 	}
 
-	return ethtool_op_set_tso(dev, data);
+	netif_set_features(netif);
+	return 0;
 }
 
 static void netbk_get_drvinfo(struct net_device *dev,
@@ -200,7 +238,7 @@ static struct ethtool_ops network_ethtool_ops =
 	.get_drvinfo = netbk_get_drvinfo,
 
 	.get_tx_csum = ethtool_op_get_tx_csum,
-	.set_tx_csum = ethtool_op_set_tx_csum,
+	.set_tx_csum = netbk_set_tx_csum,
 	.get_sg = ethtool_op_get_sg,
 	.set_sg = netbk_set_sg,
 	.get_tso = ethtool_op_get_tso,
@@ -242,7 +280,8 @@ struct xen_netif *netif_alloc(struct device *parent, domid_t domid, unsigned int
 	netif->domid  = domid;
 	netif->group  = -1;
 	netif->handle = handle;
-	netif->features = NETIF_F_SG;
+	netif->can_sg = 1;
+	netif->csum = 1;
 	atomic_set(&netif->refcnt, 1);
 	init_waitqueue_head(&netif->waiting_to_free);
 	netif->dev = dev;
@@ -259,8 +298,7 @@ struct xen_netif *netif_alloc(struct device *parent, domid_t domid, unsigned int
 	init_timer(&netif->tx_queue_timeout);
 
 	dev->netdev_ops	= &netback_ops;
-	dev->features   = NETIF_F_IP_CSUM|NETIF_F_SG;
-
+	netif_set_features(netif);
 	SET_ETHTOOL_OPS(dev, &network_ethtool_ops);
 
 	dev->tx_queue_len = netbk_queue_length;
diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index 93f0686..426aada 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -238,7 +238,7 @@ static struct sk_buff *netbk_copy_skb(struct sk_buff *skb)
 
 static inline int netbk_max_required_rx_slots(struct xen_netif *netif)
 {
-	if (netif->features & (NETIF_F_SG|NETIF_F_TSO))
+	if (netif->can_sg || netif->gso || netif->gso_prefix)
 		return MAX_SKB_FRAGS + 2; /* header + extra_info + frags */
 	return 1; /* all in one */
 }
diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
index 74c035b..99831c7 100644
--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -412,6 +412,7 @@ static void connect(struct backend_info *be)
 
 static int connect_rings(struct backend_info *be)
 {
+	struct xen_netif *netif = be->netif;
 	struct xenbus_device *dev = be->dev;
 	unsigned long tx_ring_ref, rx_ring_ref;
 	unsigned int evtchn, rx_copy;
@@ -445,61 +446,47 @@ static int connect_rings(struct backend_info *be)
 	if (!rx_copy)
 		return -EOPNOTSUPP;
 
-	if (be->netif->dev->tx_queue_len != 0) {
+	if (netif->dev->tx_queue_len != 0) {
 		if (xenbus_scanf(XBT_NIL, dev->otherend,
 				 "feature-rx-notify", "%d", &val) < 0)
 			val = 0;
 		if (val)
-			be->netif->can_queue = 1;
+			netif->can_queue = 1;
 		else
 			/* Must be non-zero for pfifo_fast to work. */
-			be->netif->dev->tx_queue_len = 1;
+			netif->dev->tx_queue_len = 1;
 	}
 
-	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg", "%d", &val) < 0)
+	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg",
+			 "%d", &val) < 0)
 		val = 0;
-	if (!val) {
-		be->netif->features &= ~NETIF_F_SG;
-		be->netif->dev->features &= ~NETIF_F_SG;
-		if (be->netif->dev->mtu > ETH_DATA_LEN)
-			be->netif->dev->mtu = ETH_DATA_LEN;
-	}
+	netif->can_sg = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4",
-			"%d", &val) < 0)
+			 "%d", &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features |= NETIF_F_TSO;
-		be->netif->dev->features |= NETIF_F_TSO;
-	}
+	netif->gso = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4-prefix",
-			"%d", &val) < 0)
+			 "%d", &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features |= NETIF_F_TSO;
-		be->netif->dev->features |= NETIF_F_TSO;
-		be->netif->gso_prefix = 1;
-	}
+	netif->gso_prefix = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-offload",
-			"%d", &val) < 0)
+			 "%d", &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features &= ~NETIF_F_IP_CSUM;
-		be->netif->dev->features &= ~NETIF_F_IP_CSUM;
-	}
+	netif->csum = !val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-smart-poll",
-			"%d", &val) < 0)
+			 "%d", &val) < 0)
 		val = 0;
-	if (val)
-		be->netif->smart_poll = 1;
-	else
-		be->netif->smart_poll = 0;
+	netif->smart_poll = !!val;
+
+	/* Set dev->features */
+	netif_set_features(netif);
 
 	/* Map the shared frame, irq etc. */
-	err = netif_map(be->netif, tx_ring_ref, rx_ring_ref, evtchn);
+	err = netif_map(netif, tx_ring_ref, rx_ring_ref, evtchn);
 	if (err) {
 		xenbus_dev_fatal(dev, err,
 				 "mapping shared-frames %lu/%lu port %u",
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] Fix basic indentation issue
@ 2010-07-02  9:28 Paul Durrant
  2010-07-02  9:28 ` [PATCH] Add a new style of passing GSO packets to frontends Paul Durrant
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Durrant @ 2010-07-02  9:28 UTC (permalink / raw)
  To: xen-devel, jeremy; +Cc: Paul Durrant

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
 drivers/xen/netback/netback.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index aacb286..c8f5c1b 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -445,10 +445,13 @@ static void netbk_gop_frag_copy(struct xen_netif *netif,
 		copy_gop = npo->copy + npo->copy_prod++;
 		copy_gop->flags = GNTCOPY_dest_gref;
 		if (PageForeign(page)) {
-		struct xen_netbk *netbk = &xen_netbk[group];
-		struct pending_tx_info *src_pend = &netbk->pending_tx_info[idx];
-		copy_gop->source.domid = src_pend->netif->domid;
-		copy_gop->source.u.ref = src_pend->req.gref;
+			struct xen_netbk *netbk = &xen_netbk[group];
+			struct pending_tx_info *src_pend;
+
+			src_pend = &netbk->pending_tx_info[idx];
+
+			copy_gop->source.domid = src_pend->netif->domid;
+			copy_gop->source.u.ref = src_pend->req.gref;
 			copy_gop->flags |= GNTCOPY_source_gref;
 		} else {
 			copy_gop->source.domid = DOMID_SELF;
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] Add a new style of passing GSO packets to frontends.
  2010-07-02  9:28 [PATCH] Fix basic indentation issue Paul Durrant
@ 2010-07-02  9:28 ` Paul Durrant
  2010-07-02  9:28   ` [PATCH] Make frontend features distinct from netback feature flags Paul Durrant
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Durrant @ 2010-07-02  9:28 UTC (permalink / raw)
  To: xen-devel, jeremy; +Cc: Paul Durrant, Ian Campbell

feature-gso-tcpv4-prefix uses precedes the packet data passed to
the frontend with a ring entry that contains the necessary
metadata. This style of GSO passing is required for Citrix
Windows PV Drivers.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/netback/common.h     |    3 +-
 drivers/xen/netback/netback.c    |   43 ++++++++++++++++++++++++++++++++++---
 drivers/xen/netback/xenbus.c     |   17 +++++++++++---
 include/xen/interface/io/netif.h |    4 +++
 4 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/drivers/xen/netback/common.h b/drivers/xen/netback/common.h
index 857778c..1cbc4ff 100644
--- a/drivers/xen/netback/common.h
+++ b/drivers/xen/netback/common.h
@@ -82,7 +82,8 @@ struct xen_netif {
 	int smart_poll;
 
 	/* Internal feature information. */
-	u8 can_queue:1;	/* can queue packets for receiver? */
+	u8 can_queue:1;	    /* can queue packets for receiver? */
+	u8 gso_prefix:1;    /* use a prefix segment for GSO information */
 
 	/* Allow netif_be_start_xmit() to peek ahead in the rx request
 	 * ring.  This is a prediction of what rx_req_cons will be once
diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index c8f5c1b..93f0686 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -313,8 +313,12 @@ int netif_be_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	netbk = &xen_netbk[netif->group];
 
+	/* Drop the packet if the netif is not up or there is no carrier. */
+	if (unlikely(!netif_schedulable(netif)))
+		goto drop;
+
 	/* Drop the packet if the target domain has no receive buffers. */
-	if (unlikely(!netif_schedulable(netif) || netbk_queue_full(netif)))
+	if (unlikely(netbk_queue_full(netif)))
 		goto drop;
 
 	/*
@@ -432,6 +436,7 @@ static void netbk_gop_frag_copy(struct xen_netif *netif,
 			/* Overflowed this request, go to the next one */
 			req = RING_GET_REQUEST(&netif->rx, netif->rx.req_cons++);
 			meta = npo->meta + npo->meta_prod++;
+			meta->gso_size = 0;
 			meta->size = 0;
 			meta->id = req->id;
 			npo->copy_off = 0;
@@ -492,9 +497,23 @@ static int netbk_gop_skb(struct sk_buff *skb,
 
 	old_meta_prod = npo->meta_prod;
 
+	/* Set up a GSO prefix descriptor, if necessary */
+	if (skb_shinfo(skb)->gso_size && netif->gso_prefix) {
+		req = RING_GET_REQUEST(&netif->rx, netif->rx.req_cons++);
+		meta = npo->meta + npo->meta_prod++;
+		meta->gso_size = skb_shinfo(skb)->gso_size;
+		meta->size = 0;
+		meta->id = req->id;
+	}
+
 	req = RING_GET_REQUEST(&netif->rx, netif->rx.req_cons++);
 	meta = npo->meta + npo->meta_prod++;
-	meta->gso_size = skb_shinfo(skb)->gso_size;
+
+	if (!netif->gso_prefix)
+		meta->gso_size = skb_shinfo(skb)->gso_size;
+	else
+		meta->gso_size = 0;
+
 	meta->size = 0;
 	meta->id = req->id;
 	npo->copy_off = 0;
@@ -506,7 +525,7 @@ static int netbk_gop_skb(struct sk_buff *skb,
 			    offset_in_page(skb->data), 1);
 
 	/* Leave a gap for the GSO descriptor. */
-	if (skb_shinfo(skb)->gso_size)
+	if (skb_shinfo(skb)->gso_size && !netif->gso_prefix)
 		netif->rx.req_cons++;
 
 	for (i = 0; i < nr_frags; i++) {
@@ -623,6 +642,21 @@ static void net_rx_action(unsigned long data)
 
 		netif = netdev_priv(skb->dev);
 
+		if (netbk->meta[npo.meta_cons].gso_size && netif->gso_prefix) {
+			resp = RING_GET_RESPONSE(&netif->rx,
+						netif->rx.rsp_prod_pvt++);
+
+			resp->flags = NETRXF_gso_prefix | NETRXF_more_data;
+
+			resp->offset = netbk->meta[npo.meta_cons].gso_size;
+			resp->id = netbk->meta[npo.meta_cons].id;
+			resp->status = sco->meta_slots_used;
+
+			npo.meta_cons++;
+			sco->meta_slots_used--;
+		}
+
+
 		netif->stats.tx_bytes += skb->len;
 		netif->stats.tx_packets++;
 
@@ -633,6 +667,7 @@ static void net_rx_action(unsigned long data)
 			flags = 0;
 		else
 			flags = NETRXF_more_data;
+
 		if (skb->ip_summed == CHECKSUM_PARTIAL) /* local packet? */
 			flags |= NETRXF_csum_blank | NETRXF_data_validated;
 		else if (skb->ip_summed == CHECKSUM_UNNECESSARY)
@@ -645,7 +680,7 @@ static void net_rx_action(unsigned long data)
 					netbk->meta[npo.meta_cons].size,
 					flags);
 
-		if (netbk->meta[npo.meta_cons].gso_size) {
+		if (netbk->meta[npo.meta_cons].gso_size && !netif->gso_prefix) {
 			struct xen_netif_extra_info *gso =
 				(struct xen_netif_extra_info *)
 				RING_GET_RESPONSE(&netif->rx,
diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
index ba7b1de..74c035b 100644
--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -465,16 +465,25 @@ static int connect_rings(struct backend_info *be)
 			be->netif->dev->mtu = ETH_DATA_LEN;
 	}
 
-	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4", "%d",
-			 &val) < 0)
+	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4",
+			"%d", &val) < 0)
 		val = 0;
 	if (val) {
 		be->netif->features |= NETIF_F_TSO;
 		be->netif->dev->features |= NETIF_F_TSO;
 	}
 
+	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4-prefix",
+			"%d", &val) < 0)
+		val = 0;
+	if (val) {
+		be->netif->features |= NETIF_F_TSO;
+		be->netif->dev->features |= NETIF_F_TSO;
+		be->netif->gso_prefix = 1;
+	}
+
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-offload",
-			 "%d", &val) < 0)
+			"%d", &val) < 0)
 		val = 0;
 	if (val) {
 		be->netif->features &= ~NETIF_F_IP_CSUM;
@@ -482,7 +491,7 @@ static int connect_rings(struct backend_info *be)
 	}
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-smart-poll",
-			 "%d", &val) < 0)
+			"%d", &val) < 0)
 		val = 0;
 	if (val)
 		be->netif->smart_poll = 1;
diff --git a/include/xen/interface/io/netif.h b/include/xen/interface/io/netif.h
index 518481c..8309344 100644
--- a/include/xen/interface/io/netif.h
+++ b/include/xen/interface/io/netif.h
@@ -131,6 +131,10 @@ struct xen_netif_rx_request {
 #define _NETRXF_extra_info     (3)
 #define  NETRXF_extra_info     (1U<<_NETRXF_extra_info)
 
+/* GSO Prefix descriptor. */
+#define _NETRXF_gso_prefix     (4)
+#define  NETRXF_gso_prefix     (1U<<_NETRXF_gso_prefix)
+
 struct xen_netif_rx_response {
     uint16_t id;
     uint16_t offset;       /* Offset in page of start of received packet  */
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] Make frontend features distinct from netback feature flags.
  2010-07-02  9:28 ` [PATCH] Add a new style of passing GSO packets to frontends Paul Durrant
@ 2010-07-02  9:28   ` Paul Durrant
  2010-07-02 14:54     ` Paul Durrant
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Durrant @ 2010-07-02  9:28 UTC (permalink / raw)
  To: xen-devel, jeremy; +Cc: Paul Durrant, Ian Campbell

Make sure that if a feature flag is disabled by ethtool on netback
that we do not gratuitously re-enabled it when we check the frontend
features during ring connection.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/netback/common.h    |   15 ++++++---
 drivers/xen/netback/interface.c |   68 ++++++++++++++++++++++++++++++--------
 drivers/xen/netback/netback.c   |    2 +-
 drivers/xen/netback/xenbus.c    |   51 +++++++++++------------------
 4 files changed, 83 insertions(+), 53 deletions(-)

diff --git a/drivers/xen/netback/common.h b/drivers/xen/netback/common.h
index 1cbc4ff..a673331 100644
--- a/drivers/xen/netback/common.h
+++ b/drivers/xen/netback/common.h
@@ -76,14 +76,18 @@ struct xen_netif {
 	struct vm_struct *tx_comms_area;
 	struct vm_struct *rx_comms_area;
 
-	/* Set of features that can be turned on in dev->features. */
-	int features;
+    /* Flags that must not be set in dev->features */
+    int features_disabled;
 
-	int smart_poll;
+	/* Frontend feature information. */
+    u8 can_sg:1;
+    u8 gso:1;
+    u8 gso_prefix:1;
+    u8 csum:1;
+    u8 smart_poll:1;
 
 	/* Internal feature information. */
 	u8 can_queue:1;	    /* can queue packets for receiver? */
-	u8 gso_prefix:1;    /* use a prefix segment for GSO information */
 
 	/* Allow netif_be_start_xmit() to peek ahead in the rx request
 	 * ring.  This is a prediction of what rx_req_cons will be once
@@ -189,6 +193,7 @@ void netif_accel_init(void);
 
 void netif_disconnect(struct xen_netif *netif);
 
+void netif_set_features(struct xen_netif *netif);
 struct xen_netif *netif_alloc(struct device *parent, domid_t domid, unsigned int handle);
 int netif_map(struct xen_netif *netif, unsigned long tx_ring_ref,
 	      unsigned long rx_ring_ref, unsigned int evtchn);
@@ -225,7 +230,7 @@ static inline int netbk_can_queue(struct net_device *dev)
 static inline int netbk_can_sg(struct net_device *dev)
 {
 	struct xen_netif *netif = netdev_priv(dev);
-	return netif->features & NETIF_F_SG;
+	return netif->can_sg;
 }
 
 struct pending_tx_info {
diff --git a/drivers/xen/netback/interface.c b/drivers/xen/netback/interface.c
index 172ef4c..2e8508a 100644
--- a/drivers/xen/netback/interface.c
+++ b/drivers/xen/netback/interface.c
@@ -121,31 +121,69 @@ static int netbk_change_mtu(struct net_device *dev, int mtu)
 	return 0;
 }
 
-static int netbk_set_sg(struct net_device *dev, u32 data)
+void netif_set_features(struct xen_netif *netif)
 {
-	if (data) {
-		struct xen_netif *netif = netdev_priv(dev);
+	struct net_device *dev = netif->dev;
+	int features = dev->features;
+
+	if (netif->can_sg)
+		features |= NETIF_F_SG;
+	if (netif->gso || netif->gso_prefix)
+		features |= NETIF_F_TSO;
+	if (netif->csum)
+		features |= NETIF_F_IP_CSUM;
+
+	features &= ~(netif->features_disabled);
 
-		if (!(netif->features & NETIF_F_SG))
+	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN)
+		dev->mtu = ETH_DATA_LEN;
+
+	dev->features = features;
+}
+
+static int netbk_set_tx_csum(struct net_device *dev, u32 data)
+{
+	struct xen_netif *netif = netdev_priv(dev);
+	if (data) {
+		if (!netif->csum)
 			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_IP_CSUM;
+	} else {
+		netif->features_disabled |= NETIF_F_IP_CSUM;
 	}
 
-	if (dev->mtu > ETH_DATA_LEN)
-		dev->mtu = ETH_DATA_LEN;
+	netif_set_features(netif);
+	return 0;
+}
 
-	return ethtool_op_set_sg(dev, data);
+static int netbk_set_sg(struct net_device *dev, u32 data)
+{
+	struct xen_netif *netif = netdev_priv(dev);
+	if (data) {
+		if (!netif->can_sg)
+			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_SG;
+	} else {
+		netif->features_disabled |= NETIF_F_SG;
+	}
+
+	netif_set_features(netif);
+	return 0;
 }
 
 static int netbk_set_tso(struct net_device *dev, u32 data)
 {
+	struct xen_netif *netif = netdev_priv(dev);
 	if (data) {
-		struct xen_netif *netif = netdev_priv(dev);
-
-		if (!(netif->features & NETIF_F_TSO))
+		if (!netif->gso && !netif->gso_prefix)
 			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_TSO;
+	} else {
+		netif->features_disabled |= NETIF_F_TSO;
 	}
 
-	return ethtool_op_set_tso(dev, data);
+	netif_set_features(netif);
+	return 0;
 }
 
 static void netbk_get_drvinfo(struct net_device *dev,
@@ -200,7 +238,7 @@ static struct ethtool_ops network_ethtool_ops =
 	.get_drvinfo = netbk_get_drvinfo,
 
 	.get_tx_csum = ethtool_op_get_tx_csum,
-	.set_tx_csum = ethtool_op_set_tx_csum,
+	.set_tx_csum = netbk_set_tx_csum,
 	.get_sg = ethtool_op_get_sg,
 	.set_sg = netbk_set_sg,
 	.get_tso = ethtool_op_get_tso,
@@ -242,7 +280,8 @@ struct xen_netif *netif_alloc(struct device *parent, domid_t domid, unsigned int
 	netif->domid  = domid;
 	netif->group  = -1;
 	netif->handle = handle;
-	netif->features = NETIF_F_SG;
+	netif->can_sg = 1;
+	netif->csum = 1;
 	atomic_set(&netif->refcnt, 1);
 	init_waitqueue_head(&netif->waiting_to_free);
 	netif->dev = dev;
@@ -259,8 +298,7 @@ struct xen_netif *netif_alloc(struct device *parent, domid_t domid, unsigned int
 	init_timer(&netif->tx_queue_timeout);
 
 	dev->netdev_ops	= &netback_ops;
-	dev->features   = NETIF_F_IP_CSUM|NETIF_F_SG;
-
+	netif_set_features(netif);
 	SET_ETHTOOL_OPS(dev, &network_ethtool_ops);
 
 	dev->tx_queue_len = netbk_queue_length;
diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index 93f0686..426aada 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -238,7 +238,7 @@ static struct sk_buff *netbk_copy_skb(struct sk_buff *skb)
 
 static inline int netbk_max_required_rx_slots(struct xen_netif *netif)
 {
-	if (netif->features & (NETIF_F_SG|NETIF_F_TSO))
+	if (netif->can_sg || netif->gso || netif->gso_prefix)
 		return MAX_SKB_FRAGS + 2; /* header + extra_info + frags */
 	return 1; /* all in one */
 }
diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
index 74c035b..81b3a96 100644
--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -412,6 +412,7 @@ static void connect(struct backend_info *be)
 
 static int connect_rings(struct backend_info *be)
 {
+	struct xen_netif *netif = be->netif;
 	struct xenbus_device *dev = be->dev;
 	unsigned long tx_ring_ref, rx_ring_ref;
 	unsigned int evtchn, rx_copy;
@@ -445,61 +446,47 @@ static int connect_rings(struct backend_info *be)
 	if (!rx_copy)
 		return -EOPNOTSUPP;
 
-	if (be->netif->dev->tx_queue_len != 0) {
+	if (netif->dev->tx_queue_len != 0) {
 		if (xenbus_scanf(XBT_NIL, dev->otherend,
 				 "feature-rx-notify", "%d", &val) < 0)
 			val = 0;
 		if (val)
-			be->netif->can_queue = 1;
+			netif->can_queue = 1;
 		else
 			/* Must be non-zero for pfifo_fast to work. */
-			be->netif->dev->tx_queue_len = 1;
+			netif->dev->tx_queue_len = 1;
 	}
 
-	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg", "%d", &val) < 0)
+	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg",
+			 "%d", &val) < 0)
 		val = 0;
-	if (!val) {
-		be->netif->features &= ~NETIF_F_SG;
-		be->netif->dev->features &= ~NETIF_F_SG;
-		if (be->netif->dev->mtu > ETH_DATA_LEN)
-			be->netif->dev->mtu = ETH_DATA_LEN;
-	}
+	netif->can_sg = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4",
-			"%d", &val) < 0)
+			 "%d", &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features |= NETIF_F_TSO;
-		be->netif->dev->features |= NETIF_F_TSO;
-	}
+	netif->gso = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4-prefix",
-			"%d", &val) < 0)
+			 "%d", &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features |= NETIF_F_TSO;
-		be->netif->dev->features |= NETIF_F_TSO;
-		be->netif->gso_prefix = 1;
-	}
+	netif->gso_prefix = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-offload",
-			"%d", &val) < 0)
+			 "%d", &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features &= ~NETIF_F_IP_CSUM;
-		be->netif->dev->features &= ~NETIF_F_IP_CSUM;
-	}
+	netif->csum = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-smart-poll",
-			"%d", &val) < 0)
+			 "%d", &val) < 0)
 		val = 0;
-	if (val)
-		be->netif->smart_poll = 1;
-	else
-		be->netif->smart_poll = 0;
+	netif->smart_poll = !!val;
+
+	/* Set dev->features */
+	netif_set_features(netif);
 
 	/* Map the shared frame, irq etc. */
-	err = netif_map(be->netif, tx_ring_ref, rx_ring_ref, evtchn);
+	err = netif_map(netif, tx_ring_ref, rx_ring_ref, evtchn);
 	if (err) {
 		xenbus_dev_fatal(dev, err,
 				 "mapping shared-frames %lu/%lu port %u",
-- 
1.5.6.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH] Make frontend features distinct from netback feature flags.
  2010-07-02  9:28   ` [PATCH] Make frontend features distinct from netback feature flags Paul Durrant
@ 2010-07-02 14:54     ` Paul Durrant
  2010-07-09 16:17       ` Paul Durrant
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Durrant @ 2010-07-02 14:54 UTC (permalink / raw)
  To: Paul Durrant, xen-devel@lists.xensource.com, jeremy@goop.org; +Cc: Ian Campbell

I notice there is a 1 char typo in this patch. See below for details. I'll send a corrected patch shortly.

  Paul

> -----Original Message-----
> From: Paul Durrant
> Sent: 02 July 2010 10:28
> To: xen-devel@lists.xensource.com; jeremy@goop.org
> Cc: Paul Durrant; Ian Campbell
> Subject: [PATCH] Make frontend features distinct from netback
> feature flags.
> 
> Make sure that if a feature flag is disabled by ethtool on netback
> that we do not gratuitously re-enabled it when we check the frontend
> features during ring connection.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> ---
>  drivers/xen/netback/common.h    |   15 ++++++---
>  drivers/xen/netback/interface.c |   68
> ++++++++++++++++++++++++++++++--------
>  drivers/xen/netback/netback.c   |    2 +-
>  drivers/xen/netback/xenbus.c    |   51 +++++++++++-----------------
> -
>  4 files changed, 83 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/xen/netback/common.h
> b/drivers/xen/netback/common.h
> index 1cbc4ff..a673331 100644
> --- a/drivers/xen/netback/common.h
> +++ b/drivers/xen/netback/common.h
> @@ -76,14 +76,18 @@ struct xen_netif {
>  	struct vm_struct *tx_comms_area;
>  	struct vm_struct *rx_comms_area;
> 
> -	/* Set of features that can be turned on in dev->features. */
> -	int features;
> +    /* Flags that must not be set in dev->features */
> +    int features_disabled;
> 
> -	int smart_poll;
> +	/* Frontend feature information. */
> +    u8 can_sg:1;
> +    u8 gso:1;
> +    u8 gso_prefix:1;
> +    u8 csum:1;
> +    u8 smart_poll:1;
> 
>  	/* Internal feature information. */
>  	u8 can_queue:1;	    /* can queue packets for receiver?
> */
> -	u8 gso_prefix:1;    /* use a prefix segment for GSO
> information */
> 
>  	/* Allow netif_be_start_xmit() to peek ahead in the rx
> request
>  	 * ring.  This is a prediction of what rx_req_cons will be
> once
> @@ -189,6 +193,7 @@ void netif_accel_init(void);
> 
>  void netif_disconnect(struct xen_netif *netif);
> 
> +void netif_set_features(struct xen_netif *netif);
>  struct xen_netif *netif_alloc(struct device *parent, domid_t domid,
> unsigned int handle);
>  int netif_map(struct xen_netif *netif, unsigned long tx_ring_ref,
>  	      unsigned long rx_ring_ref, unsigned int evtchn);
> @@ -225,7 +230,7 @@ static inline int netbk_can_queue(struct
> net_device *dev)
>  static inline int netbk_can_sg(struct net_device *dev)
>  {
>  	struct xen_netif *netif = netdev_priv(dev);
> -	return netif->features & NETIF_F_SG;
> +	return netif->can_sg;
>  }
> 
>  struct pending_tx_info {
> diff --git a/drivers/xen/netback/interface.c
> b/drivers/xen/netback/interface.c
> index 172ef4c..2e8508a 100644
> --- a/drivers/xen/netback/interface.c
> +++ b/drivers/xen/netback/interface.c
> @@ -121,31 +121,69 @@ static int netbk_change_mtu(struct net_device
> *dev, int mtu)
>  	return 0;
>  }
> 
> -static int netbk_set_sg(struct net_device *dev, u32 data)
> +void netif_set_features(struct xen_netif *netif)
>  {
> -	if (data) {
> -		struct xen_netif *netif = netdev_priv(dev);
> +	struct net_device *dev = netif->dev;
> +	int features = dev->features;
> +
> +	if (netif->can_sg)
> +		features |= NETIF_F_SG;
> +	if (netif->gso || netif->gso_prefix)
> +		features |= NETIF_F_TSO;
> +	if (netif->csum)
> +		features |= NETIF_F_IP_CSUM;
> +
> +	features &= ~(netif->features_disabled);
> 
> -		if (!(netif->features & NETIF_F_SG))
> +	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN)
> +		dev->mtu = ETH_DATA_LEN;
> +
> +	dev->features = features;
> +}
> +
> +static int netbk_set_tx_csum(struct net_device *dev, u32 data)
> +{
> +	struct xen_netif *netif = netdev_priv(dev);
> +	if (data) {
> +		if (!netif->csum)
>  			return -ENOSYS;
> +		netif->features_disabled &= ~NETIF_F_IP_CSUM;
> +	} else {
> +		netif->features_disabled |= NETIF_F_IP_CSUM;
>  	}
> 
> -	if (dev->mtu > ETH_DATA_LEN)
> -		dev->mtu = ETH_DATA_LEN;
> +	netif_set_features(netif);
> +	return 0;
> +}
> 
> -	return ethtool_op_set_sg(dev, data);
> +static int netbk_set_sg(struct net_device *dev, u32 data)
> +{
> +	struct xen_netif *netif = netdev_priv(dev);
> +	if (data) {
> +		if (!netif->can_sg)
> +			return -ENOSYS;
> +		netif->features_disabled &= ~NETIF_F_SG;
> +	} else {
> +		netif->features_disabled |= NETIF_F_SG;
> +	}
> +
> +	netif_set_features(netif);
> +	return 0;
>  }
> 
>  static int netbk_set_tso(struct net_device *dev, u32 data)
>  {
> +	struct xen_netif *netif = netdev_priv(dev);
>  	if (data) {
> -		struct xen_netif *netif = netdev_priv(dev);
> -
> -		if (!(netif->features & NETIF_F_TSO))
> +		if (!netif->gso && !netif->gso_prefix)
>  			return -ENOSYS;
> +		netif->features_disabled &= ~NETIF_F_TSO;
> +	} else {
> +		netif->features_disabled |= NETIF_F_TSO;
>  	}
> 
> -	return ethtool_op_set_tso(dev, data);
> +	netif_set_features(netif);
> +	return 0;
>  }
> 
>  static void netbk_get_drvinfo(struct net_device *dev,
> @@ -200,7 +238,7 @@ static struct ethtool_ops network_ethtool_ops =
>  	.get_drvinfo = netbk_get_drvinfo,
> 
>  	.get_tx_csum = ethtool_op_get_tx_csum,
> -	.set_tx_csum = ethtool_op_set_tx_csum,
> +	.set_tx_csum = netbk_set_tx_csum,
>  	.get_sg = ethtool_op_get_sg,
>  	.set_sg = netbk_set_sg,
>  	.get_tso = ethtool_op_get_tso,
> @@ -242,7 +280,8 @@ struct xen_netif *netif_alloc(struct device
> *parent, domid_t domid, unsigned int
>  	netif->domid  = domid;
>  	netif->group  = -1;
>  	netif->handle = handle;
> -	netif->features = NETIF_F_SG;
> +	netif->can_sg = 1;
> +	netif->csum = 1;
>  	atomic_set(&netif->refcnt, 1);
>  	init_waitqueue_head(&netif->waiting_to_free);
>  	netif->dev = dev;
> @@ -259,8 +298,7 @@ struct xen_netif *netif_alloc(struct device
> *parent, domid_t domid, unsigned int
>  	init_timer(&netif->tx_queue_timeout);
> 
>  	dev->netdev_ops	= &netback_ops;
> -	dev->features   = NETIF_F_IP_CSUM|NETIF_F_SG;
> -
> +	netif_set_features(netif);
>  	SET_ETHTOOL_OPS(dev, &network_ethtool_ops);
> 
>  	dev->tx_queue_len = netbk_queue_length;
> diff --git a/drivers/xen/netback/netback.c
> b/drivers/xen/netback/netback.c
> index 93f0686..426aada 100644
> --- a/drivers/xen/netback/netback.c
> +++ b/drivers/xen/netback/netback.c
> @@ -238,7 +238,7 @@ static struct sk_buff *netbk_copy_skb(struct
> sk_buff *skb)
> 
>  static inline int netbk_max_required_rx_slots(struct xen_netif
> *netif)
>  {
> -	if (netif->features & (NETIF_F_SG|NETIF_F_TSO))
> +	if (netif->can_sg || netif->gso || netif->gso_prefix)
>  		return MAX_SKB_FRAGS + 2; /* header + extra_info +
> frags */
>  	return 1; /* all in one */
>  }
> diff --git a/drivers/xen/netback/xenbus.c
> b/drivers/xen/netback/xenbus.c
> index 74c035b..81b3a96 100644
> --- a/drivers/xen/netback/xenbus.c
> +++ b/drivers/xen/netback/xenbus.c
> @@ -412,6 +412,7 @@ static void connect(struct backend_info *be)
> 
>  static int connect_rings(struct backend_info *be)
>  {
> +	struct xen_netif *netif = be->netif;
>  	struct xenbus_device *dev = be->dev;
>  	unsigned long tx_ring_ref, rx_ring_ref;
>  	unsigned int evtchn, rx_copy;
> @@ -445,61 +446,47 @@ static int connect_rings(struct backend_info
> *be)
>  	if (!rx_copy)
>  		return -EOPNOTSUPP;
> 
> -	if (be->netif->dev->tx_queue_len != 0) {
> +	if (netif->dev->tx_queue_len != 0) {
>  		if (xenbus_scanf(XBT_NIL, dev->otherend,
>  				 "feature-rx-notify", "%d", &val) < 0)
>  			val = 0;
>  		if (val)
> -			be->netif->can_queue = 1;
> +			netif->can_queue = 1;
>  		else
>  			/* Must be non-zero for pfifo_fast to work. */
> -			be->netif->dev->tx_queue_len = 1;
> +			netif->dev->tx_queue_len = 1;
>  	}
> 
> -	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg", "%d",
> &val) < 0)
> +	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg",
> +			 "%d", &val) < 0)
>  		val = 0;
> -	if (!val) {
> -		be->netif->features &= ~NETIF_F_SG;
> -		be->netif->dev->features &= ~NETIF_F_SG;
> -		if (be->netif->dev->mtu > ETH_DATA_LEN)
> -			be->netif->dev->mtu = ETH_DATA_LEN;
> -	}
> +	netif->can_sg = !!val;
> 
>  	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4",
> -			"%d", &val) < 0)
> +			 "%d", &val) < 0)
>  		val = 0;
> -	if (val) {
> -		be->netif->features |= NETIF_F_TSO;
> -		be->netif->dev->features |= NETIF_F_TSO;
> -	}
> +	netif->gso = !!val;
> 
>  	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4-
> prefix",
> -			"%d", &val) < 0)
> +			 "%d", &val) < 0)
>  		val = 0;
> -	if (val) {
> -		be->netif->features |= NETIF_F_TSO;
> -		be->netif->dev->features |= NETIF_F_TSO;
> -		be->netif->gso_prefix = 1;
> -	}
> +	netif->gso_prefix = !!val;
> 
>  	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-
> offload",
> -			"%d", &val) < 0)
> +			 "%d", &val) < 0)
>  		val = 0;
> -	if (val) {
> -		be->netif->features &= ~NETIF_F_IP_CSUM;
> -		be->netif->dev->features &= ~NETIF_F_IP_CSUM;
> -	}
> +	netif->csum = !!val;

This should be !val.

> 
>  	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-smart-
> poll",
> -			"%d", &val) < 0)
> +			 "%d", &val) < 0)
>  		val = 0;
> -	if (val)
> -		be->netif->smart_poll = 1;
> -	else
> -		be->netif->smart_poll = 0;
> +	netif->smart_poll = !!val;
> +
> +	/* Set dev->features */
> +	netif_set_features(netif);
> 
>  	/* Map the shared frame, irq etc. */
> -	err = netif_map(be->netif, tx_ring_ref, rx_ring_ref, evtchn);
> +	err = netif_map(netif, tx_ring_ref, rx_ring_ref, evtchn);
>  	if (err) {
>  		xenbus_dev_fatal(dev, err,
>  				 "mapping shared-frames %lu/%lu port
> %u",
> --
> 1.5.6.5

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] Make frontend features distinct from netback feature flags.
  2010-07-02 14:54     ` Paul Durrant
@ 2010-07-09 16:17       ` Paul Durrant
  2010-07-09 17:53         ` Jeremy Fitzhardinge
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Durrant @ 2010-07-09 16:17 UTC (permalink / raw)
  To: Ian Campbell; +Cc: jeremy@goop.org, xen-devel@lists.xensource.com

[-- Attachment #1: Type: text/plain, Size: 457 bytes --]

Ian,

  I regenerated the patch with git-format-patches and re-checked with checkpatch.pl:

[pauldu@cosworth:/misc/scratch/pauldu/git/linux-2.6-xen]./scripts/checkpatch.pl 0001-Make-frontend-features-distinct-from-netback-feature.patch
total: 0 errors, 0 warnings, 237 lines checked

0001-Make-frontend-features-distinct-from-netback-feature.patch has no obvious style problems and is ready for submission.

  Can you see this ok now?

    Paul

[-- Attachment #2: 0001-Make-frontend-features-distinct-from-netback-feature.patch --]
[-- Type: application/octet-stream, Size: 8715 bytes --]

From 37ee8da32a3f9449530299e101549d0fd76c31e0 Mon Sep 17 00:00:00 2001
From: Paul Durrant <paul.durrant@citrix.com>
Date: Fri, 2 Jul 2010 15:51:52 +0100
Subject: [PATCH] Make frontend features distinct from netback feature flags.

Make sure that if a feature flag is disabled by ethtool on netback
that we do not gratuitously re-enable it when we check the frontend
features during ring connection.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
---
 drivers/xen/netback/common.h    |   15 ++++++---
 drivers/xen/netback/interface.c |   68 ++++++++++++++++++++++++++++++--------
 drivers/xen/netback/netback.c   |    2 +-
 drivers/xen/netback/xenbus.c    |   51 +++++++++++------------------
 4 files changed, 83 insertions(+), 53 deletions(-)

diff --git a/drivers/xen/netback/common.h b/drivers/xen/netback/common.h
index 1cbc4ff..a673331 100644
--- a/drivers/xen/netback/common.h
+++ b/drivers/xen/netback/common.h
@@ -76,14 +76,18 @@ struct xen_netif {
 	struct vm_struct *tx_comms_area;
 	struct vm_struct *rx_comms_area;
 
-	/* Set of features that can be turned on in dev->features. */
-	int features;
+    /* Flags that must not be set in dev->features */
+    int features_disabled;
 
-	int smart_poll;
+	/* Frontend feature information. */
+    u8 can_sg:1;
+    u8 gso:1;
+    u8 gso_prefix:1;
+    u8 csum:1;
+    u8 smart_poll:1;
 
 	/* Internal feature information. */
 	u8 can_queue:1;	    /* can queue packets for receiver? */
-	u8 gso_prefix:1;    /* use a prefix segment for GSO information */
 
 	/* Allow netif_be_start_xmit() to peek ahead in the rx request
 	 * ring.  This is a prediction of what rx_req_cons will be once
@@ -189,6 +193,7 @@ void netif_accel_init(void);
 
 void netif_disconnect(struct xen_netif *netif);
 
+void netif_set_features(struct xen_netif *netif);
 struct xen_netif *netif_alloc(struct device *parent, domid_t domid, unsigned int handle);
 int netif_map(struct xen_netif *netif, unsigned long tx_ring_ref,
 	      unsigned long rx_ring_ref, unsigned int evtchn);
@@ -225,7 +230,7 @@ static inline int netbk_can_queue(struct net_device *dev)
 static inline int netbk_can_sg(struct net_device *dev)
 {
 	struct xen_netif *netif = netdev_priv(dev);
-	return netif->features & NETIF_F_SG;
+	return netif->can_sg;
 }
 
 struct pending_tx_info {
diff --git a/drivers/xen/netback/interface.c b/drivers/xen/netback/interface.c
index 172ef4c..2e8508a 100644
--- a/drivers/xen/netback/interface.c
+++ b/drivers/xen/netback/interface.c
@@ -121,31 +121,69 @@ static int netbk_change_mtu(struct net_device *dev, int mtu)
 	return 0;
 }
 
-static int netbk_set_sg(struct net_device *dev, u32 data)
+void netif_set_features(struct xen_netif *netif)
 {
-	if (data) {
-		struct xen_netif *netif = netdev_priv(dev);
+	struct net_device *dev = netif->dev;
+	int features = dev->features;
+
+	if (netif->can_sg)
+		features |= NETIF_F_SG;
+	if (netif->gso || netif->gso_prefix)
+		features |= NETIF_F_TSO;
+	if (netif->csum)
+		features |= NETIF_F_IP_CSUM;
+
+	features &= ~(netif->features_disabled);
 
-		if (!(netif->features & NETIF_F_SG))
+	if (!(features & NETIF_F_SG) && dev->mtu > ETH_DATA_LEN)
+		dev->mtu = ETH_DATA_LEN;
+
+	dev->features = features;
+}
+
+static int netbk_set_tx_csum(struct net_device *dev, u32 data)
+{
+	struct xen_netif *netif = netdev_priv(dev);
+	if (data) {
+		if (!netif->csum)
 			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_IP_CSUM;
+	} else {
+		netif->features_disabled |= NETIF_F_IP_CSUM;
 	}
 
-	if (dev->mtu > ETH_DATA_LEN)
-		dev->mtu = ETH_DATA_LEN;
+	netif_set_features(netif);
+	return 0;
+}
 
-	return ethtool_op_set_sg(dev, data);
+static int netbk_set_sg(struct net_device *dev, u32 data)
+{
+	struct xen_netif *netif = netdev_priv(dev);
+	if (data) {
+		if (!netif->can_sg)
+			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_SG;
+	} else {
+		netif->features_disabled |= NETIF_F_SG;
+	}
+
+	netif_set_features(netif);
+	return 0;
 }
 
 static int netbk_set_tso(struct net_device *dev, u32 data)
 {
+	struct xen_netif *netif = netdev_priv(dev);
 	if (data) {
-		struct xen_netif *netif = netdev_priv(dev);
-
-		if (!(netif->features & NETIF_F_TSO))
+		if (!netif->gso && !netif->gso_prefix)
 			return -ENOSYS;
+		netif->features_disabled &= ~NETIF_F_TSO;
+	} else {
+		netif->features_disabled |= NETIF_F_TSO;
 	}
 
-	return ethtool_op_set_tso(dev, data);
+	netif_set_features(netif);
+	return 0;
 }
 
 static void netbk_get_drvinfo(struct net_device *dev,
@@ -200,7 +238,7 @@ static struct ethtool_ops network_ethtool_ops =
 	.get_drvinfo = netbk_get_drvinfo,
 
 	.get_tx_csum = ethtool_op_get_tx_csum,
-	.set_tx_csum = ethtool_op_set_tx_csum,
+	.set_tx_csum = netbk_set_tx_csum,
 	.get_sg = ethtool_op_get_sg,
 	.set_sg = netbk_set_sg,
 	.get_tso = ethtool_op_get_tso,
@@ -242,7 +280,8 @@ struct xen_netif *netif_alloc(struct device *parent, domid_t domid, unsigned int
 	netif->domid  = domid;
 	netif->group  = -1;
 	netif->handle = handle;
-	netif->features = NETIF_F_SG;
+	netif->can_sg = 1;
+	netif->csum = 1;
 	atomic_set(&netif->refcnt, 1);
 	init_waitqueue_head(&netif->waiting_to_free);
 	netif->dev = dev;
@@ -259,8 +298,7 @@ struct xen_netif *netif_alloc(struct device *parent, domid_t domid, unsigned int
 	init_timer(&netif->tx_queue_timeout);
 
 	dev->netdev_ops	= &netback_ops;
-	dev->features   = NETIF_F_IP_CSUM|NETIF_F_SG;
-
+	netif_set_features(netif);
 	SET_ETHTOOL_OPS(dev, &network_ethtool_ops);
 
 	dev->tx_queue_len = netbk_queue_length;
diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index c7ca2e7..50666fe 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -238,7 +238,7 @@ static struct sk_buff *netbk_copy_skb(struct sk_buff *skb)
 
 static inline int netbk_max_required_rx_slots(struct xen_netif *netif)
 {
-	if (netif->features & (NETIF_F_SG|NETIF_F_TSO))
+	if (netif->can_sg || netif->gso || netif->gso_prefix)
 		return MAX_SKB_FRAGS + 2; /* header + extra_info + frags */
 	return 1; /* all in one */
 }
diff --git a/drivers/xen/netback/xenbus.c b/drivers/xen/netback/xenbus.c
index 74c035b..99831c7 100644
--- a/drivers/xen/netback/xenbus.c
+++ b/drivers/xen/netback/xenbus.c
@@ -412,6 +412,7 @@ static void connect(struct backend_info *be)
 
 static int connect_rings(struct backend_info *be)
 {
+	struct xen_netif *netif = be->netif;
 	struct xenbus_device *dev = be->dev;
 	unsigned long tx_ring_ref, rx_ring_ref;
 	unsigned int evtchn, rx_copy;
@@ -445,61 +446,47 @@ static int connect_rings(struct backend_info *be)
 	if (!rx_copy)
 		return -EOPNOTSUPP;
 
-	if (be->netif->dev->tx_queue_len != 0) {
+	if (netif->dev->tx_queue_len != 0) {
 		if (xenbus_scanf(XBT_NIL, dev->otherend,
 				 "feature-rx-notify", "%d", &val) < 0)
 			val = 0;
 		if (val)
-			be->netif->can_queue = 1;
+			netif->can_queue = 1;
 		else
 			/* Must be non-zero for pfifo_fast to work. */
-			be->netif->dev->tx_queue_len = 1;
+			netif->dev->tx_queue_len = 1;
 	}
 
-	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg", "%d", &val) < 0)
+	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-sg",
+			 "%d", &val) < 0)
 		val = 0;
-	if (!val) {
-		be->netif->features &= ~NETIF_F_SG;
-		be->netif->dev->features &= ~NETIF_F_SG;
-		if (be->netif->dev->mtu > ETH_DATA_LEN)
-			be->netif->dev->mtu = ETH_DATA_LEN;
-	}
+	netif->can_sg = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4",
-			"%d", &val) < 0)
+			 "%d", &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features |= NETIF_F_TSO;
-		be->netif->dev->features |= NETIF_F_TSO;
-	}
+	netif->gso = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-gso-tcpv4-prefix",
-			"%d", &val) < 0)
+			 "%d", &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features |= NETIF_F_TSO;
-		be->netif->dev->features |= NETIF_F_TSO;
-		be->netif->gso_prefix = 1;
-	}
+	netif->gso_prefix = !!val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-no-csum-offload",
-			"%d", &val) < 0)
+			 "%d", &val) < 0)
 		val = 0;
-	if (val) {
-		be->netif->features &= ~NETIF_F_IP_CSUM;
-		be->netif->dev->features &= ~NETIF_F_IP_CSUM;
-	}
+	netif->csum = !val;
 
 	if (xenbus_scanf(XBT_NIL, dev->otherend, "feature-smart-poll",
-			"%d", &val) < 0)
+			 "%d", &val) < 0)
 		val = 0;
-	if (val)
-		be->netif->smart_poll = 1;
-	else
-		be->netif->smart_poll = 0;
+	netif->smart_poll = !!val;
+
+	/* Set dev->features */
+	netif_set_features(netif);
 
 	/* Map the shared frame, irq etc. */
-	err = netif_map(be->netif, tx_ring_ref, rx_ring_ref, evtchn);
+	err = netif_map(netif, tx_ring_ref, rx_ring_ref, evtchn);
 	if (err) {
 		xenbus_dev_fatal(dev, err,
 				 "mapping shared-frames %lu/%lu port %u",
-- 
1.5.6.5


[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] Make frontend features distinct from netback feature flags.
  2010-07-09 16:17       ` Paul Durrant
@ 2010-07-09 17:53         ` Jeremy Fitzhardinge
  2010-07-12  9:07           ` Paul Durrant
  0 siblings, 1 reply; 6+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-09 17:53 UTC (permalink / raw)
  To: Paul Durrant; +Cc: Ian Campbell, xen-devel@lists.xensource.com

On 07/09/2010 09:17 AM, Paul Durrant wrote:
> Ian,
>
>   I regenerated the patch with git-format-patches and re-checked with checkpatch.pl:
>
> [pauldu@cosworth:/misc/scratch/pauldu/git/linux-2.6-xen]./scripts/checkpatch.pl 0001-Make-frontend-features-distinct-from-netback-feature.patch
> total: 0 errors, 0 warnings, 237 lines checked
>
> 0001-Make-frontend-features-distinct-from-netback-feature.patch has no obvious style problems and is ready for submission.
>
>   Can you see this ok now?
>   

Actually I had already committed your changes with some fixups to
xen/dom0/backend/netback (excluding the feature watch one).  Could you
double-check your changes in that branch?

Thanks,
    J

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] Make frontend features distinct from netback feature flags.
  2010-07-09 17:53         ` Jeremy Fitzhardinge
@ 2010-07-12  9:07           ` Paul Durrant
  0 siblings, 0 replies; 6+ messages in thread
From: Paul Durrant @ 2010-07-12  9:07 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Ian Campbell, xen-devel@lists.xensource.com

> -----Original Message-----
> From: Jeremy Fitzhardinge [mailto:jeremy@goop.org]
> Sent: 09 July 2010 18:53
> To: Paul Durrant
> Cc: Ian Campbell; xen-devel@lists.xensource.com
> Subject: Re: [PATCH] Make frontend features distinct from netback
> feature flags.
> 
> On 07/09/2010 09:17 AM, Paul Durrant wrote:
> > Ian,
> >
> >   I regenerated the patch with git-format-patches and re-checked
> with checkpatch.pl:
> >
> > [pauldu@cosworth:/misc/scratch/pauldu/git/linux-2.6-
> xen]./scripts/checkpatch.pl 0001-Make-frontend-features-distinct-
> from-netback-feature.patch
> > total: 0 errors, 0 warnings, 237 lines checked
> >
> > 0001-Make-frontend-features-distinct-from-netback-feature.patch
> has no obvious style problems and is ready for submission.
> >
> >   Can you see this ok now?
> >
> 
> Actually I had already committed your changes with some fixups to
> xen/dom0/backend/netback (excluding the feature watch one).  Could
> you
> double-check your changes in that branch?
> 

Looks alright there.

  Paul

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-07-12  9:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-02 14:55 [PATCH] Make frontend features distinct from netback feature flags Paul Durrant
  -- strict thread matches above, loose matches on Subject: below --
2010-07-02  9:28 [PATCH] Fix basic indentation issue Paul Durrant
2010-07-02  9:28 ` [PATCH] Add a new style of passing GSO packets to frontends Paul Durrant
2010-07-02  9:28   ` [PATCH] Make frontend features distinct from netback feature flags Paul Durrant
2010-07-02 14:54     ` Paul Durrant
2010-07-09 16:17       ` Paul Durrant
2010-07-09 17:53         ` Jeremy Fitzhardinge
2010-07-12  9:07           ` Paul Durrant

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).