Re: [Xen-devel] [NET] back: Copy tx_ring data before verification

[Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>]

On Thu, Oct 12, 2006 at 02:54:14PM +0100, Keir Fraser wrote:
>
> I would very much like to take this patch, but right now it does not work!

How embarrassing! This version should work better:

[NET] back: Copy tx_ring data before verification

We need to make a copy of data from tx_ring before
verifying them as otherwise what we end up using may
be different from what was verified.

Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff -r f7d65fb7299b linux-2.6-xen-sparse/drivers/xen/netback/netback.c
--- a/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Tue Oct 10 
22:04:01 2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Thu Oct 12 
23:00:24 2006 +0800
@@ -875,20 +875,28 @@ static void netbk_tx_err(netif_t *netif,
        netif_put(netif);
 }
 
-static int netbk_count_requests(netif_t *netif, netif_tx_request_t *txp,
-                               int work_to_do)
-{
-       netif_tx_request_t *first = txp;
+static int netbk_count_requests(netif_t *netif, netif_tx_request_t *first,
+                               netif_tx_request_t *txp, int work_to_do)
+{
        RING_IDX cons = netif->tx.req_cons;
        int frags = 0;
 
-       while (txp->flags & NETTXF_more_data) {
+       if (!(first->flags & NETTXF_more_data))
+               return 0;
+
+       do {
                if (frags >= work_to_do) {
                        DPRINTK("Need more frags\n");
                        return -frags;
                }
 
-               txp = RING_GET_REQUEST(&netif->tx, cons + frags);
+               if (unlikely(frags >= MAX_SKB_FRAGS)) {
+                       DPRINTK("Too many frags\n");
+                       return -frags;
+               }
+
+               memcpy(txp, RING_GET_REQUEST(&netif->tx, cons + frags),
+                      sizeof(*txp));
                if (txp->size > first->size) {
                        DPRINTK("Frags galore\n");
                        return -frags;
@@ -902,27 +910,25 @@ static int netbk_count_requests(netif_t 
                                txp->offset, txp->size);
                        return -frags;
                }
-       }
+       } while ((txp++)->flags & NETTXF_more_data);
 
        return frags;
 }
 
 static gnttab_map_grant_ref_t *netbk_get_requests(netif_t *netif,
                                                  struct sk_buff *skb,
+                                                 netif_tx_request_t *txp,
                                                  gnttab_map_grant_ref_t *mop)
 {
        struct skb_shared_info *shinfo = skb_shinfo(skb);
        skb_frag_t *frags = shinfo->frags;
-       netif_tx_request_t *txp;
        unsigned long pending_idx = *((u16 *)skb->data);
-       RING_IDX cons = netif->tx.req_cons;
        int i, start;
 
        /* Skip first skb fragment if it is on same page as header fragment. */
        start = ((unsigned long)shinfo->frags[0].page == pending_idx);
 
-       for (i = start; i < shinfo->nr_frags; i++) {
-               txp = RING_GET_REQUEST(&netif->tx, cons++);
+       for (i = start; i < shinfo->nr_frags; i++, txp++) {
                pending_idx = pending_ring[MASK_PEND_IDX(pending_cons++)];
 
                gnttab_set_map_op(mop++, idx_to_kaddr(pending_idx),
@@ -1036,7 +1042,7 @@ int netbk_get_extras(netif_t *netif, str
 int netbk_get_extras(netif_t *netif, struct netif_extra_info *extras,
                     int work_to_do)
 {
-       struct netif_extra_info *extra;
+       struct netif_extra_info extra;
        RING_IDX cons = netif->tx.req_cons;
 
        do {
@@ -1045,18 +1051,18 @@ int netbk_get_extras(netif_t *netif, str
                        return -EBADR;
                }
 
-               extra = (struct netif_extra_info *)
-                       RING_GET_REQUEST(&netif->tx, cons);
-               if (unlikely(!extra->type ||
-                            extra->type >= XEN_NETIF_EXTRA_TYPE_MAX)) {
+               memcpy(&extra, RING_GET_REQUEST(&netif->tx, cons),
+                      sizeof(extra));
+               if (unlikely(!extra.type ||
+                            extra.type >= XEN_NETIF_EXTRA_TYPE_MAX)) {
                        netif->tx.req_cons = ++cons;
-                       DPRINTK("Invalid extra type: %d\n", extra->type);
+                       DPRINTK("Invalid extra type: %d\n", extra.type);
                        return -EINVAL;
                }
 
-               memcpy(&extras[extra->type - 1], extra, sizeof(*extra));
+               memcpy(&extras[extra.type - 1], &extra, sizeof(extra));
                netif->tx.req_cons = ++cons;
-       } while (extra->flags & XEN_NETIF_EXTRA_FLAG_MORE);
+       } while (extra.flags & XEN_NETIF_EXTRA_FLAG_MORE);
 
        return work_to_do;
 }
@@ -1091,6 +1097,7 @@ static void net_tx_action(unsigned long 
        struct sk_buff *skb;
        netif_t *netif;
        netif_tx_request_t txreq;
+       netif_tx_request_t txfrags[MAX_SKB_FRAGS];
        struct netif_extra_info extras[XEN_NETIF_EXTRA_TYPE_MAX - 1];
        u16 pending_idx;
        RING_IDX i;
@@ -1167,18 +1174,12 @@ static void net_tx_action(unsigned long 
                        }
                }
 
-               ret = netbk_count_requests(netif, &txreq, work_to_do);
+               ret = netbk_count_requests(netif, &txreq, txfrags, work_to_do);
                if (unlikely(ret < 0)) {
                        netbk_tx_err(netif, &txreq, i - ret);
                        continue;
                }
                i += ret;
-
-               if (unlikely(ret > MAX_SKB_FRAGS)) {
-                       DPRINTK("Too many frags\n");
-                       netbk_tx_err(netif, &txreq, i);
-                       continue;
-               }
 
                if (unlikely(txreq.size < ETH_HLEN)) {
                        DPRINTK("Bad packet size: %d\n", txreq.size);
@@ -1248,7 +1249,7 @@ static void net_tx_action(unsigned long 
 
                pending_cons++;
 
-               mop = netbk_get_requests(netif, skb, mop);
+               mop = netbk_get_requests(netif, skb, txfrags, mop);
 
                netif->tx.req_cons = i;
                netif_schedule_work(netif);

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