[PATCH] patch fix-PKT_PROT_LEN

[PATCH] patch fix-PKT_PROT_LEN

[Paul Durrant]

---
 drivers/xen/netback/netback.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index c448675..1a4a20e 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -128,7 +128,7 @@ static inline int netif_get_page_ext(struct page *pg, unsigned int *_group, unsi
  * packet processing on them (netfilter, routing, etc). 72 is enough
  * to cover TCP+IP headers including options.
  */
-#define PKT_PROT_LEN 72
+#define PKT_PROT_LEN    (ETH_HLEN + 4 + (15 * 4) + (15 * 4))
 
 static inline pending_ring_idx_t pending_index(unsigned i)
 {
-- 
1.5.6.5

[PATCH] patch fix-rx-packets-accounting

[Paul Durrant]

---
 drivers/xen/netback/netback.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index 1a4a20e..066d140 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -1519,9 +1519,6 @@ static void net_tx_submit(struct xen_netbk *netbk)
 		skb->dev      = netif->dev;
 		skb->protocol = eth_type_trans(skb, skb->dev);
 
-		netif->stats.rx_bytes += skb->len;
-		netif->stats.rx_packets++;
-
 		if (skb->ip_summed == CHECKSUM_PARTIAL) {
 			if (skb_checksum_setup(skb)) {
 				DPRINTK("Can't setup checksum in net_tx_action\n");
@@ -1537,6 +1534,9 @@ static void net_tx_submit(struct xen_netbk *netbk)
 			continue;
 		}
 
+		netif->stats.rx_bytes += skb->len;
+		netif->stats.rx_packets++;
+
 		netif_rx_ni(skb);
 		netif->dev->last_rx = jiffies;
 	}
-- 
1.5.6.5

[PATCH] patch remove-timeout

[Paul Durrant]

---
 drivers/xen/netback/netback.c |   20 +-------------------
 1 files changed, 1 insertions(+), 19 deletions(-)

diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index 066d140..87a2cd4 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -271,13 +271,6 @@ static inline int netbk_queue_full(struct xen_netif *netif)
 	       ((netif->rx.rsp_prod_pvt + NET_RX_RING_SIZE - peek) < needed);
 }
 
-static void tx_queue_callback(unsigned long data)
-{
-	struct xen_netif *netif = (struct xen_netif *)data;
-	if (netif_schedulable(netif))
-		netif_wake_queue(netif->dev);
-}
-
 /* Figure out how many ring slots we're going to need to send @skb to
    the guest. */
 static unsigned count_skb_slots(struct sk_buff *skb, struct xen_netif *netif)
@@ -360,19 +353,8 @@ int netif_be_start_xmit(struct sk_buff *skb, struct net_device *dev)
 		netif->rx.sring->req_event = netif->rx_req_cons_peek +
 			netbk_max_required_rx_slots(netif);
 		mb(); /* request notification /then/ check & stop the queue */
-		if (netbk_queue_full(netif)) {
+		if (netbk_queue_full(netif))
 			netif_stop_queue(dev);
-			/*
-			 * Schedule 500ms timeout to restart the queue, thus
-			 * ensuring that an inactive queue will be drained.
-			 * Packets will be immediately be dropped until more
-			 * receive buffers become available (see
-			 * netbk_queue_full() check above).
-			 */
-			netif->tx_queue_timeout.data = (unsigned long)netif;
-			netif->tx_queue_timeout.function = tx_queue_callback;
-			mod_timer(&netif->tx_queue_timeout, jiffies + HZ/2);
-		}
 	}
 	skb_queue_tail(&netbk->rx_queue, skb);
 
-- 
1.5.6.5

[PATCH] patch fix-tx-work-todo

[Paul Durrant]

---
 drivers/xen/netback/netback.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index 87a2cd4..eca61a9 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -1712,6 +1712,10 @@ static inline int tx_work_todo(struct xen_netbk *netbk)
 	if (netbk->dealloc_cons != netbk->dealloc_prod)
 		return 1;
 
+	if (netbk_copy_skb_mode == NETBK_DELAYED_COPY_SKB &&
+	    !list_empty(&netbk->pending_inuse_head))
+		return 1;
+
 	if (((nr_pending_reqs(netbk) + MAX_SKB_FRAGS) < MAX_PENDING_REQS) &&
 			!list_empty(&netbk->net_schedule_list))
 		return 1;
-- 
1.5.6.5

[PATCH] patch move-pending-tx-copy

[Paul Durrant]

---
 drivers/xen/netback/netback.c |   26 ++++++++++++++------------
 1 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
index eca61a9..25adbf4 100644
--- a/drivers/xen/netback/netback.c
+++ b/drivers/xen/netback/netback.c
@@ -913,11 +913,20 @@ static inline void net_tx_action_dealloc(struct xen_netbk *netbk)
 			gop++;
 		}
 
-		if (netbk_copy_skb_mode != NETBK_DELAYED_COPY_SKB ||
-		    list_empty(&netbk->pending_inuse_head))
-			break;
+	} while (dp != netbk->dealloc_prod);
+
+	netbk->dealloc_cons = dc;
 
-		/* Copy any entries that have been pending for too long. */
+	ret = HYPERVISOR_grant_table_op(
+		GNTTABOP_unmap_grant_ref, netbk->tx_unmap_ops,
+		gop - netbk->tx_unmap_ops);
+	BUG_ON(ret);
+
+	/*
+	 * Copy any entries that have been pending for too long
+	 */
+	if (netbk_copy_skb_mode == NETBK_DELAYED_COPY_SKB &&
+	    !list_empty(&netbk->pending_inuse_head)) {
 		list_for_each_entry_safe(inuse, n,
 				&netbk->pending_inuse_head, list) {
 			struct pending_tx_info *pending_tx_info;
@@ -943,14 +952,7 @@ static inline void net_tx_action_dealloc(struct xen_netbk *netbk)
 
 			break;
 		}
-	} while (dp != netbk->dealloc_prod);
-
-	netbk->dealloc_cons = dc;
-
-	ret = HYPERVISOR_grant_table_op(
-		GNTTABOP_unmap_grant_ref, netbk->tx_unmap_ops,
-		gop - netbk->tx_unmap_ops);
-	BUG_ON(ret);
+	}
 
 	list_for_each_entry_safe(inuse, n, &list, list) {
 		struct pending_tx_info *pending_tx_info;
-- 
1.5.6.5

Re: [PATCH] patch remove-timeout

[Konrad Rzeszutek Wilk]

On Tue, Dec 14, 2010 at 05:38:01PM +0000, Paul Durrant wrote:

Why? ooh wait your next email gives a truncated answer..

I will wait until you get your git send-mail-fu sorted out.

> ---
>  drivers/xen/netback/netback.c |   20 +-------------------
>  1 files changed, 1 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
> index 066d140..87a2cd4 100644
> --- a/drivers/xen/netback/netback.c
> +++ b/drivers/xen/netback/netback.c
> @@ -271,13 +271,6 @@ static inline int netbk_queue_full(struct xen_netif *netif)
>  	       ((netif->rx.rsp_prod_pvt + NET_RX_RING_SIZE - peek) < needed);
>  }
>  
> -static void tx_queue_callback(unsigned long data)
> -{
> -	struct xen_netif *netif = (struct xen_netif *)data;
> -	if (netif_schedulable(netif))
> -		netif_wake_queue(netif->dev);
> -}
> -
>  /* Figure out how many ring slots we're going to need to send @skb to
>     the guest. */
>  static unsigned count_skb_slots(struct sk_buff *skb, struct xen_netif *netif)
> @@ -360,19 +353,8 @@ int netif_be_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  		netif->rx.sring->req_event = netif->rx_req_cons_peek +
>  			netbk_max_required_rx_slots(netif);
>  		mb(); /* request notification /then/ check & stop the queue */
> -		if (netbk_queue_full(netif)) {
> +		if (netbk_queue_full(netif))
>  			netif_stop_queue(dev);
> -			/*
> -			 * Schedule 500ms timeout to restart the queue, thus
> -			 * ensuring that an inactive queue will be drained.
> -			 * Packets will be immediately be dropped until more
> -			 * receive buffers become available (see
> -			 * netbk_queue_full() check above).
> -			 */
> -			netif->tx_queue_timeout.data = (unsigned long)netif;
> -			netif->tx_queue_timeout.function = tx_queue_callback;
> -			mod_timer(&netif->tx_queue_timeout, jiffies + HZ/2);
> -		}
>  	}
>  	skb_queue_tail(&netbk->rx_queue, skb);
>  
> -- 
> 1.5.6.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] patch fix-rx-packets-accounting

[Konrad Rzeszutek Wilk]

On Tue, Dec 14, 2010 at 05:38:00PM +0000, Paul Durrant wrote:

Shouldn't there also be a patch to move the tx_bytes?

> ---
>  drivers/xen/netback/netback.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/xen/netback/netback.c b/drivers/xen/netback/netback.c
> index 1a4a20e..066d140 100644
> --- a/drivers/xen/netback/netback.c
> +++ b/drivers/xen/netback/netback.c
> @@ -1519,9 +1519,6 @@ static void net_tx_submit(struct xen_netbk *netbk)
>  		skb->dev      = netif->dev;
>  		skb->protocol = eth_type_trans(skb, skb->dev);
>  
> -		netif->stats.rx_bytes += skb->len;
> -		netif->stats.rx_packets++;
> -
>  		if (skb->ip_summed == CHECKSUM_PARTIAL) {
>  			if (skb_checksum_setup(skb)) {
>  				DPRINTK("Can't setup checksum in net_tx_action\n");
> @@ -1537,6 +1534,9 @@ static void net_tx_submit(struct xen_netbk *netbk)
>  			continue;
>  		}
>  
> +		netif->stats.rx_bytes += skb->len;
> +		netif->stats.rx_packets++;
> +
>  		netif_rx_ni(skb);
>  		netif->dev->last_rx = jiffies;
>  	}
> -- 
> 1.5.6.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

RE: [PATCH] patch fix-rx-packets-accounting

[Paul Durrant]

I think tx_packets and tx_bytes accounting is ok. Do you think they are wrong?

  Paul

> -----Original Message-----
> From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@oracle.com]
> Sent: 15 December 2010 15:45
> To: Paul Durrant
> Cc: xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] patch fix-rx-packets-accounting
> 
> On Tue, Dec 14, 2010 at 05:38:00PM +0000, Paul Durrant wrote:
> 
> Shouldn't there also be a patch to move the tx_bytes?
> 
> > ---
> >  drivers/xen/netback/netback.c |    6 +++---
> >  1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/xen/netback/netback.c
> b/drivers/xen/netback/netback.c
> > index 1a4a20e..066d140 100644
> > --- a/drivers/xen/netback/netback.c
> > +++ b/drivers/xen/netback/netback.c
> > @@ -1519,9 +1519,6 @@ static void net_tx_submit(struct xen_netbk
> *netbk)
> >  		skb->dev      = netif->dev;
> >  		skb->protocol = eth_type_trans(skb, skb->dev);
> >
> > -		netif->stats.rx_bytes += skb->len;
> > -		netif->stats.rx_packets++;
> > -
> >  		if (skb->ip_summed == CHECKSUM_PARTIAL) {
> >  			if (skb_checksum_setup(skb)) {
> >  				DPRINTK("Can't setup checksum in
> net_tx_action\n");
> > @@ -1537,6 +1534,9 @@ static void net_tx_submit(struct xen_netbk
> *netbk)
> >  			continue;
> >  		}
> >
> > +		netif->stats.rx_bytes += skb->len;
> > +		netif->stats.rx_packets++;
> > +
> >  		netif_rx_ni(skb);
> >  		netif->dev->last_rx = jiffies;
> >  	}
> > --
> > 1.5.6.5
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel

RE: [PATCH] patch fix-rx-packets-accounting

[Ian Campbell]

On Thu, 2010-12-16 at 11:37 +0000, Paul Durrant wrote:
> I think tx_packets and tx_bytes accounting is ok. Do you think they are wrong?

I wonder if it might be more correct to only increment them if status ==
NETIF_RSP_OK?

Ian.

RE: [PATCH] patch fix-rx-packets-accounting

[Paul Durrant]

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

No, I don't think so. The stats should be bumped on data ingress to the shared ring. After all, if this were ethernet, you generally have no way of knowing whether the packet you send ever makes it to its destination.

  Paul

> -----Original Message-----
> From: Ian Campbell
> Sent: 16 December 2010 11:45
> To: Paul Durrant
> Cc: Konrad Rzeszutek Wilk; xen-devel@lists.xensource.com
> Subject: RE: [Xen-devel] [PATCH] patch fix-rx-packets-accounting
> 
> On Thu, 2010-12-16 at 11:37 +0000, Paul Durrant wrote:
> > I think tx_packets and tx_bytes accounting is ok. Do you think
> they are wrong?
> 
> I wonder if it might be more correct to only increment them if
> status ==
> NETIF_RSP_OK?
> 
> Ian.
> 


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

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

Re: [PATCH] patch fix-rx-packets-accounting

[Konrad Rzeszutek Wilk]

On Thu, Dec 16, 2010 at 11:49:04AM +0000, Paul Durrant wrote:
> No, I don't think so. The stats should be bumped on data ingress to the shared ring. After all, if this were ethernet, you generally have no way of knowing whether the packet you send ever makes it to its destination.
> 

OK. Thanks.

>   Paul
> 
> > -----Original Message-----
> > From: Ian Campbell
> > Sent: 16 December 2010 11:45
> > To: Paul Durrant
> > Cc: Konrad Rzeszutek Wilk; xen-devel@lists.xensource.com
> > Subject: RE: [Xen-devel] [PATCH] patch fix-rx-packets-accounting
> > 
> > On Thu, 2010-12-16 at 11:37 +0000, Paul Durrant wrote:
> > > I think tx_packets and tx_bytes accounting is ok. Do you think
> > they are wrong?
> > 
> > I wonder if it might be more correct to only increment them if
> > status ==
> > NETIF_RSP_OK?
> > 
> > Ian.
> > 
>