On Wed, Jun 15, 2011 at 11:00:06AM -0700, Jesse Gross wrote: > The current implementation of make_writable() is both overly complex > and unnecessarily aggressive about copying data. We can improve > performance by only making a copy of the data if someone else holds > a reference to the portion of the data that we want to modify. This > means that if a clone is held by the TCP stack for retransmission then > we do not need to make a copy if we are changing the IP header because > it will get regenerated on retransmit anyways. Even when it is necessary > to copy we avoid duplicating struct sk_buff. > > Signed-off-by: Jesse Gross <[email protected]>
This looks like a potentially very useful optimization. Thanks for doing it. I've studied it for a few minutes and I don't see any actual problems. I noticed in set_nw_tos() that the variable "f" doesn't help with anything or make the code easier to read (not a new issue). I think we should just write nh->tos in place of *f. In do_execute_actions(), I think that the scope of the new 'err' variable could be limited to the "for" loop. The very final call to kfree_skb() at the end of do_execute_actions() could possibly be a consume_skb(), since this isn't really an error (not a new issue either). Acked-by: Ben Pfaff <[email protected]> _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
