Subject: Re: [rfc 1/3] ipvs: handle PARTIAL_CHECKSUM

From: Simon Horman <[email-masked]>
Date: Mon, 8 Sep 2008 19:05:27 +1000

On Mon, Sep 08, 2008 at 05:24:40PM +1000, Herbert Xu wrote:
> On Mon, Sep 08, 2008 at 12:04:21PM +1000, Simon Horman wrote:
> >
> >  	/* Adjust TCP checksums */
> > -	if (!cp->app && (tcph->check != 0)) {
> > +	if (skb->ip_summed == CHECKSUM_PARTIAL) {
> > +		tcp_partial_csum_update(cp->af, tcph, &cp->daddr, &cp->vaddr,
> > +					htonl(oldlen),
> > +					htonl(skb->len - tcphoff));
> 
> I don't know what cp->app is but should we be updating the checksum
> when it's set? The previous code seems to want to compute a full
> checksum instead.

Hi Herbert,

If cp->app is not present, then only the destiantion IP address
and possibly port will have been altered. If it is present,
then other parts of the packet may also have been altered.

In the case where (skb->ip_summed == CHECKSUM_PARTIAL)
as we are only concerned with checkumming the pseudo header,
the only changes that nat could make that we care about are the
address or the length. The latter may change if cp->app is set,
but I think that my code handles this in tcp_partial_csum_update().

So I think that is safe to use tcp_partial_csum_update() if
(skb->ip_summed == CHECKSUM_PARTIAL), regarless of if cp->app is set or not.