qio: clean up block_.*_metadata()
authorBarret Rhoden <brho@cs.berkeley.edu>
Thu, 6 Jun 2019 13:36:01 +0000 (09:36 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Thu, 6 Jun 2019 13:44:10 +0000 (09:44 -0400)
We were missing the free() function pointer.  No one was using that, but
if you padblocked a block from udp, where they override free(), the
block wouldn't have the same method.  Same goes for linearizeblock().

The 'next' pointer was a subject of concern, especially given the
occasional confusion between blocks and blocklists.  If you are copying
the metadata to a new block, that's often because you are either freeing
the old block, in which case a lost next pointer is a leak, or you are
'teeing' traffic, in which case you have two blocks with the same next
pointer (disaster).

The other fields seem to be taken care of by their callers: extra data,
rp/wp/lim/etc.  'list' is used by callers like ARP to have their own
lists.  Not sure if they can double-up on 'next' or not.

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

index 47e4200..cd352a5 100644 (file)
@@ -185,6 +185,13 @@ void block_copy_metadata(struct block *new_b, struct block *old_b)
        new_b->mss = old_b->mss;
        new_b->network_offset = old_b->network_offset;
        new_b->transport_offset = old_b->transport_offset;
+       new_b->free = old_b->free;
+
+       /* This is probably OK.  Right now, no one calls us with a blocklist.
+        * Any callers that do would need to manage 'next', either to avoid
+        * leaking memory (of old_b is freed) or to have multiple pointers to
+        * the same block (if new_b is a copy for e.g. snoop). */
+       warn_on(old_b->next);
 }
 
 void block_reset_metadata(struct block *b)
@@ -194,6 +201,7 @@ void block_reset_metadata(struct block *b)
        b->mss = 0;
        b->network_offset = 0;
        b->transport_offset = 0;
+       b->free = NULL;
 }
 
 /* Adds delta (which may be negative) to the block metadata offsets that are