[ovs-dev] [rfc 3/6] ovs-datapath: Remove set_skb_csum_bits()

Jesse Gross jesse at nicira.com
Thu Aug 26 04:33:05 UTC 2010


On Wed, Aug 25, 2010 at 12:08 AM, Simon Horman <horms at verge.net.au> wrote:
> set_skb_csum_bits() is empty in the current kernel tree
> so just remove it.

There are actually a lot of checksum related compatibility bits mixed
into the main code.  It's definitely the single most invasive
compatibility issue that we have.

The issue is that on older kernels both the modern CHECKSUM_COMPLETE
and CHECKSUM_PARTIAL were defined as CHECKSUM_HW.  This normally is OK
because you can tell whether you are on the transmit or receive path
and behave appropriately.  However, as a switch we are right in the
middle and can't divine the correct meaning (and the meanings are
essentially opposite).  This is made worse by Xen which defines a few
extra bits in the skb to disambiguate these values, which we have to
handle as well.  It's a real mess and requires littering things
throughout the code to track and check the actual state.  Throw in a
few bugs (like what this function is compensating for) and it gets
even more exciting.

Thankfully the current kernel tree has resolved all of these issues
and we can delete all of this crap when merging upstream.  However, we
should probably do it all at the same time.  In addition, all of the
other #ifdefs based on the kernel version should be removed at that
time as well.  There's also at least one function copied from the
kernel source because it isn't exported, which we will be able to
avoid.

I'm definitely happy to see this type of thing go but I wonder what
the best time is to do that.  Doing it now will significantly clean
the code up and will be appreciated by anyone reviewing the code.  On
the other hand, since it will be a big patch it will be more work to
integrate changes as we incorporate feedback.




More information about the dev mailing list