Encapsulate block metadata better
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 10 Nov 2017 17:44:52 +0000 (12:44 -0500)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 16 Nov 2017 15:46:56 +0000 (10:46 -0500)
Everytime I add something, like network_offset, I had to update a few
places.  Well, two places.  This way, we're slightly more clear about what
we're doing, instead of adding minor hacks each time.

A couple points:
- We had been copying the metadata over only if certain flags were set.  We
  should be able to copy that data over no matter what.
- It was probably buggy to only copy network_offset and transport_offset
  based on a flag.
- If you muck with a block, like in padblock(), you still need to adjust
  any offsets.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
kern/include/ns.h
kern/src/net/icmp.c
kern/src/ns/allocb.c
kern/src/ns/qio.c

index 28b1763..21a8984 100644 (file)
@@ -656,6 +656,8 @@ struct block *block_alloc(size_t, int);
 int block_add_extd(struct block *b, unsigned int nr_bufs, int mem_flags);
 int block_append_extra(struct block *b, uintptr_t base, uint32_t off,
                        uint32_t len, int mem_flags);
+void block_copy_metadata(struct block *new_b, struct block *old_b);
+void block_reset_metadata(struct block *b);
 int anyhigher(void);
 int anyready(void);
 void _assert(char *unused_char_p_t);
index fb08a48..e63d4ab 100644 (file)
@@ -320,8 +320,9 @@ static struct block *mkechoreply(struct Proto *icmp, struct block *bp)
        uint8_t ip[4];
 
        /* we're repurposing bp to send it back out.  we need to remove any inbound
-        * checksum flags (which were saying the HW did the checksum) */
-       bp->flag &= ~BCKSUM_FLAGS;
+        * checksum flags (which were saying the HW did the checksum) and any other
+        * metadata.  We might need to fill in some of the metadata too. */
+       block_reset_metadata(bp);
        q = (Icmp *) bp->rp;
        q->vihl = IP_VER4;
        memmove(ip, q->src, sizeof(q->dst));
index 092c529..57c4bd9 100644 (file)
@@ -176,6 +176,28 @@ int block_append_extra(struct block *b, uintptr_t base, uint32_t off,
        return 0;
 }
 
+/* There's metadata in each block related to the data payload.  For instance,
+ * the TSO mss, the offsets to various headers, whether csums are needed, etc.
+ * When you create a new block, like in copyblock, this will copy those bits
+ * over. */
+void block_copy_metadata(struct block *new_b, struct block *old_b)
+{
+       new_b->flag |= (old_b->flag & BCKSUM_FLAGS);
+       new_b->tx_csum_offset = old_b->tx_csum_offset;
+       new_b->mss = old_b->mss;
+       new_b->network_offset = old_b->network_offset;
+       new_b->transport_offset = old_b->transport_offset;
+}
+
+void block_reset_metadata(struct block *b)
+{
+       b->flag &= ~BCKSUM_FLAGS;
+       b->tx_csum_offset = 0;
+       b->mss = 0;
+       b->network_offset = 0;
+       b->transport_offset = 0;
+}
+
 void free_block_extra(struct block *b)
 {
        struct extra_bdata *ebd;
index 2998f84..b9d5d9e 100644 (file)
@@ -131,11 +131,6 @@ struct block *padblock(struct block *bp, int size)
 {
        int n;
        struct block *nbp;
-       uint8_t bcksum = bp->flag & BCKSUM_FLAGS;
-       uint16_t tx_csum_offset = bp->tx_csum_offset;
-       uint16_t mss = bp->mss;
-       uint16_t network_offset = bp->network_offset;
-       uint16_t transport_offset = bp->transport_offset;
 
        QDEBUG checkb(bp, "padblock 1");
        if (size >= 0) {
@@ -152,6 +147,7 @@ struct block *padblock(struct block *bp, int size)
                n = BLEN(bp);
                padblockcnt++;
                nbp = block_alloc(size + n, MEM_WAIT);
+               block_copy_metadata(nbp, bp);
                nbp->rp += size;
                nbp->wp = nbp->rp;
                memmove(nbp->wp, bp->rp, n);
@@ -172,17 +168,11 @@ struct block *padblock(struct block *bp, int size)
                n = BLEN(bp);
                padblockcnt++;
                nbp = block_alloc(size + n, MEM_WAIT);
+               block_copy_metadata(nbp, bp);
                memmove(nbp->wp, bp->rp, n);
                nbp->wp += n;
                freeb(bp);
        }
-       if (bcksum) {
-               nbp->flag |= bcksum;
-               nbp->tx_csum_offset = tx_csum_offset;
-               nbp->mss = mss;
-               nbp->network_offset = network_offset;
-               nbp->transport_offset = transport_offset;
-       }
        QDEBUG checkb(nbp, "padblock 1");
        return nbp;
 }
@@ -266,14 +256,7 @@ struct block *copyblock(struct block *bp, int mem_flags)
                amt = copy_to_block_body(newb, (void*)ebd->base + ebd->off, ebd->len);
                assert(amt == ebd->len);
        }
-       /* TODO: any other flags that need copied over? */
-       if (bp->flag & BCKSUM_FLAGS) {
-               newb->flag |= (bp->flag & BCKSUM_FLAGS);
-               newb->tx_csum_offset = bp->tx_csum_offset;
-               newb->mss = bp->mss;
-               newb->network_offset = bp->network_offset;
-               newb->transport_offset = bp->transport_offset;
-       }
+       block_copy_metadata(newb, bp);
        copyblockcnt++;
        QDEBUG checkb(newb, "copyblock 1");
        return newb;