On Thu, 2013-05-02 at 13:04 +0100, Jan Beulich wrote:

quoted

quoted

quoted

On 02.05.13 at 12:43, Wei Liu [off-list ref] wrote:

@@ -934,11 +934,14 @@ static int netbk_count_requests(struct xenvif *vif,
 	RING_IDX cons = vif->tx.req_cons;
 	int slots = 0;
 	int drop_err = 0;
+	int more_data;
 
 	if (!(first->flags & XEN_NETTXF_more_data))
 		return 0;
 
 	do {
+		struct xen_netif_tx_request dropped_tx = { 0 };
+

No need for an initializer here.

quoted

 		if (slots >= work_to_do) {
 			netdev_err(vif->dev,
 				   "Asked for %d slots but exceeds this limit\n",

@@ -972,6 +975,9 @@ static int netbk_count_requests(struct xenvif *vif,
 			drop_err = -E2BIG;
 		}
 
+		if (drop_err)
+			txp = &dropped_tx;
+
 		memcpy(txp, RING_GET_REQUEST(&vif->tx, cons + slots),
 		       sizeof(*txp));
 

@@ -1001,7 +1007,13 @@ static int netbk_count_requests(struct xenvif *vif,
 			netbk_fatal_tx_err(vif);
 			return -EINVAL;
 		}
-	} while ((txp++)->flags & XEN_NETTXF_more_data);
+
+		more_data = txp->flags & XEN_NETTXF_more_data;
+
+		if (!drop_err)
+			txp++;

And no need for the conditional here afaict.

I think it is needed, in the case where you've assigned txp =
&dropped_tx you don't want to increment txp.

Or perhaps it just gets reassigned back to &dropped_tx at the top of the
next loop, so the increment doesn't matter. Subtle! I'm happy with
whichever way Wei prefers, but it is probably worthy of a comment

Jan

quoted

+
+	} while (more_data);
 
 	if (drop_err) {
 		netbk_tx_err(vif, first, cons + slots);