From: Simon Horman <horms@verge.net.au>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: lvs-devel@vger.kernel.org, netdev@vger.kernel.org,
"Siim Põder" <siim@p6drad-teel.net>,
"Julian Anastasov" <ja@ssi.bg>,
"Malcolm Turnbull" <malcolm@loadbalancer.org>,
"Julius Volz" <juliusv@google.com>,
"Vince Busam" <vbusam@google.com>
Subject: Re: [rfc 1/3] ipvs: handle PARTIAL_CHECKSUM
Date: Mon, 8 Sep 2008 19:05:27 +1000 [thread overview]
Message-ID: <20080908090525.GA855@verge.net.au> (raw)
In-Reply-To: <20080908072439.GA27257@gondor.apana.org.au>
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.
next prev parent reply other threads:[~2008-09-08 9:05 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-08 2:04 [rfc 0/3] IPVS: checksum updates Simon Horman
2008-09-08 2:04 ` [rfc 1/3] ipvs: handle PARTIAL_CHECKSUM Simon Horman
2008-09-08 7:24 ` Herbert Xu
2008-09-08 9:05 ` Simon Horman [this message]
2008-09-08 9:54 ` Herbert Xu
2008-09-08 2:04 ` [rfc 2/3] ipvs: Use inet_proto_csum_replace*() Simon Horman
2008-09-08 2:04 ` [rfc 3/3] ipvs: Consolidate checksuming code Simon Horman
2008-09-08 10:03 ` [rfc 0/3] IPVS: checksum updates Julius Volz
2008-09-08 10:41 ` Simon Horman
2008-09-08 11:42 ` Julius Volz
2008-09-08 11:57 ` Simon Horman
2008-09-08 12:04 ` Simon Horman
2008-09-08 12:14 ` Julius Volz
2008-09-08 12:34 ` Simon Horman
2008-09-08 13:12 ` Julius Volz
2008-09-08 13:20 ` Simon Horman
2008-09-08 13:42 ` Julius Volz
2008-09-08 15:32 ` Julius Volz
2008-09-08 23:22 ` Simon Horman
2008-09-08 23:40 ` Simon Horman
2008-09-09 9:30 ` Julius Volz
2008-09-09 11:31 ` Simon Horman
2008-09-10 17:30 ` Julius Volz
2008-09-10 23:29 ` Simon Horman
2008-09-11 13:07 ` Wensong Zhang
2008-09-11 13:45 ` Simon Horman
2008-09-11 13:55 ` Julius Volz
2008-09-11 14:43 ` Wensong Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080908090525.GA855@verge.net.au \
--to=horms@verge.net.au \
--cc=herbert@gondor.apana.org.au \
--cc=ja@ssi.bg \
--cc=juliusv@google.com \
--cc=lvs-devel@vger.kernel.org \
--cc=malcolm@loadbalancer.org \
--cc=netdev@vger.kernel.org \
--cc=siim@p6drad-teel.net \
--cc=vbusam@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).