All of lore.kernel.org
 help / color / mirror / Atom feed
* [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; 10+ 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] 10+ 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
  2010-07-03  7:22   ` [PATCH] Add a new style of passing GSO packets to frontends Jeremy Fitzhardinge
  0 siblings, 2 replies; 10+ 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] 10+ 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
  2010-07-03  7:22   ` [PATCH] Add a new style of passing GSO packets to frontends Jeremy Fitzhardinge
  1 sibling, 1 reply; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread
* [PATCH] Make frontend features distinct from netback feature flags.
@ 2010-07-02 14:55 Paul Durrant
  0 siblings, 0 replies; 10+ 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] 10+ messages in thread
* Re: [PATCH] Add a new style of passing GSO packets to frontends.
  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-03  7:22   ` Jeremy Fitzhardinge
  2010-07-09 14:59     ` Ian Campbell
  1 sibling, 1 reply; 10+ messages in thread
From: Jeremy Fitzhardinge @ 2010-07-03  7:22 UTC (permalink / raw)
  To: Paul Durrant; +Cc: xen-devel, Ian Campbell

On 07/02/2010 10:28 AM, Paul Durrant wrote:
> 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;
>   

Are these related to the gso negotiation or a separate fix?  If they're
separate, could I have it as a separate patch with its own description
of the change (and if not, perhaps some comment about how this relates
to the rest of the patch)?

Thanks,
    J

>  
>  	/*
> @@ -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  */
>   

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH] Add a new style of passing GSO packets to frontends.
  2010-07-03  7:22   ` [PATCH] Add a new style of passing GSO packets to frontends Jeremy Fitzhardinge
@ 2010-07-09 14:59     ` Ian Campbell
  0 siblings, 0 replies; 10+ messages in thread
From: Ian Campbell @ 2010-07-09 14:59 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: Paul Durrant, xen-devel

On Sat, 2010-07-03 at 08:22 +0100, Jeremy Fitzhardinge wrote:
> On 07/02/2010 10:28 AM, Paul Durrant wrote:
> > 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;
> >   
> 
> Are these related to the gso negotiation or a separate fix?  If they're
> separate, could I have it as a separate patch with its own description
> of the change (and if not, perhaps some comment about how this relates
> to the rest of the patch)?

I think it is just splitting the existing || clause into two separate if
statements with their own descriptive comment? IOW it's an unrelated
cleanup I guess?

Ian.

^ permalink raw reply	[flat|nested] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread
end of thread, other threads:[~2010-07-12  9:07 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2010-07-03  7:22   ` [PATCH] Add a new style of passing GSO packets to frontends Jeremy Fitzhardinge
2010-07-09 14:59     ` Ian Campbell
  -- strict thread matches above, loose matches on Subject: below --
2010-07-02 14:55 [PATCH] Make frontend features distinct from netback feature flags Paul Durrant

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.