* [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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ messages in thread
end of thread, other threads:[~2010-07-12 9:07 UTC | newest] Thread overview: 9+ 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
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).