From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Paul Durrant <paul.durrant@citrix.com>
Cc: xen-devel@lists.xensource.com, Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH] Add a new style of passing GSO packets to frontends.
Date: Sat, 03 Jul 2010 08:22:11 +0100 [thread overview]
Message-ID: <4C2EE523.7060409@goop.org> (raw)
In-Reply-To: <1278062893-970-2-git-send-email-paul.durrant@citrix.com>
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 */
>
next prev parent reply other threads:[~2010-07-03 7:22 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` Jeremy Fitzhardinge [this message]
2010-07-09 14:59 ` [PATCH] Add a new style of passing GSO packets to frontends Ian Campbell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4C2EE523.7060409@goop.org \
--to=jeremy@goop.org \
--cc=ian.campbell@citrix.com \
--cc=paul.durrant@citrix.com \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).