patches for DPDK stable branches
 help / color / Atom feed
From: Morten Brørup <mb@smartsharesystems.com>
To: "Stephen Hemminger" <stephen@networkplumber.org>,
	"guohongzhi" <guohongzhi1@huawei.com>
Cc: <dev@dpdk.org>, <stable@dpdk.org>, <olivier.matz@6wind.com>,
	<konstantin.ananyev@intel.com>, <jiayu.hu@intel.com>,
	<ferruh.yigit@intel.com>, <nicolas.chautru@intel.com>,
	<cristian.dumitrescu@intel.com>, <zhoujingbin@huawei.com>,
	<chenchanghu@huawei.com>, <jerry.lilijun@huawei.com>,
	<haifeng.lin@huawei.com>
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] bugfix: udptcp_checksum should tread tcp and udp differently
Date: Wed, 27 May 2020 17:36:59 +0200
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35C6100E@smartserver.smartshare.dk> (raw)
In-Reply-To: <20200527080242.16dceeae@hermes.lan>

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Stephen Hemminger
> Sent: Wednesday, May 27, 2020 5:03 PM
> 
> On Wed, 27 May 2020 22:41:27 +0800
> guohongzhi <guohongzhi1@huawei.com> wrote:
> 
> > +	/* For Udp, if the computed checksum is zero,
> > +	 * it is transmitted as all ones.RFC768
> > +	 */
> > +	if (cksum == 0 && ipv4_hdr->next_proto_id == IPPROTO_UDP)
> >  		cksum = 0xffff;
> >
> 
> 
> The comment should be reformatted to be clearer.
> 
> Maybe:
> 	/*
> 	 * Per RFC768:
> 	 * If the computed  checksum  is zero,it is transmitted  as all
> ones
> 	 * (the equivalent  in one's complement  arithmetic).
> 	 */
> 

But without the double spaces. :-)

> There is no special case required here, it is true for TCP as well.

I disagree. I researched this topic when Hongzhi Guo initially submitted the patches, and have only seen the special exception mentioned in RFC 768 (describing UDP), where a transmitted value of 0x0000 means that the checksum has not been generated. None of the RFCs regarding Internet Checksum or TCP checksum mentions this special case.

> In TCP it appears 0 is allowed but not generally used. Most
> implementations
> use common checksum for both TCP and UDP

Then most implementations are wrong.

Jon Postel famously wrote: "Be liberal in what you accept, and conservative in what you send."

This function is in the transmission path, so we should calculate it correctly.

In the receive path, when checking the checksum as described in RFC 1071, any of the two values (0x0000 or 0xFFFF) in the Checksum field will yield the correct result. Which is why most implementations can get away with doing it wrong.



  reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-27 14:41 [dpdk-stable] " guohongzhi
2020-05-27 15:02 ` [dpdk-stable] [dpdk-dev] " Stephen Hemminger
2020-05-27 15:36   ` Morten Brørup [this message]
2020-07-06  7:58     ` Olivier Matz

Reply instructions:

You may reply publically 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=98CBD80474FA8B44BF855DF32C47DC35C6100E@smartserver.smartshare.dk \
    --to=mb@smartsharesystems.com \
    --cc=chenchanghu@huawei.com \
    --cc=cristian.dumitrescu@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=guohongzhi1@huawei.com \
    --cc=haifeng.lin@huawei.com \
    --cc=jerry.lilijun@huawei.com \
    --cc=jiayu.hu@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=nicolas.chautru@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=stable@dpdk.org \
    --cc=stephen@networkplumber.org \
    --cc=zhoujingbin@huawei.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

patches for DPDK stable branches

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ http://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/ public-inbox