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