netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: salil.mehta@huawei.com
Cc: yisen.zhuang@huawei.com, mehta.salil.lnk@gmail.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linuxarm@huawei.com
Subject: Re: [PATCH V2 net-next] net: hns: Fix to conditionally convey RX checksum flag to stack
Date: Wed, 30 Nov 2016 14:25:39 -0500 (EST)	[thread overview]
Message-ID: <20161130.142539.1927956259851457047.davem@davemloft.net> (raw)
In-Reply-To: <20161129130945.919372-1-salil.mehta@huawei.com>

From: Salil Mehta <salil.mehta@huawei.com>
Date: Tue, 29 Nov 2016 13:09:45 +0000

> +	/* We only support checksum for IPv4,UDP(over IPv4 or IPv6), TCP(over
> +	 * IPv4 or IPv6) and SCTP but we support many L3(IPv4, IPv6, MPLS,
> +	 * PPPoE etc) and L4(TCP, UDP, GRE, SCTP, IGMP, ICMP etc.) protocols.
> +	 * We want to filter out L3 and L4 protocols early on for which checksum
> +	 * is not supported.
 ...
> +	 */
> +	l3id = hnae_get_field(flag, HNS_RXD_L3ID_M, HNS_RXD_L3ID_S);
> +	l4id = hnae_get_field(flag, HNS_RXD_L4ID_M, HNS_RXD_L4ID_S);
> +	if ((l3id != HNS_RX_FLAG_L3ID_IPV4) &&
> +	    ((l3id != HNS_RX_FLAG_L3ID_IPV6) ||
> +	     (l4id != HNS_RX_FLAG_L4ID_UDP)) &&
> +	    ((l3id != HNS_RX_FLAG_L3ID_IPV6) ||
> +	     (l4id != HNS_RX_FLAG_L4ID_TCP)) &&
> +	    (l4id != HNS_RX_FLAG_L4ID_SCTP))
> +		return;

I have a hard time understanding this seemingly overcomplicated
check.

It looks like if L3 is IPV4 it will accept any underlying L4 protocol,
but is that what is really intended?  That doesn't match what this new
comment states.

My understanding is that the chip supports checksums for:

	UDP/IPV4
	UDP/IPV6
	TCP/IPV4
	TCP/IPV6
	SCTP/IPV4
	SCTP/IPV6

So the simplest thing is to validate each level one at a time:

	if (l3 != IPV4 && l3 != IPV6)
		return;
	if (l4 != UDP && l4 != TCP && l4 != SCTP)
		return;

  reply	other threads:[~2016-11-30 19:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-29 13:09 [PATCH V2 net-next] net: hns: Fix to conditionally convey RX checksum flag to stack Salil Mehta
2016-11-30 19:25 ` David Miller [this message]
2016-12-01 12:09   ` Salil Mehta
2016-12-03 20:09     ` David Miller
2016-12-05 15:43       ` Salil Mehta
2016-12-01 16:59   ` Salil Mehta
2016-12-03 20:25     ` David Miller
2016-12-05 15:42       ` Salil Mehta

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=20161130.142539.1927956259851457047.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=mehta.salil.lnk@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=salil.mehta@huawei.com \
    --cc=yisen.zhuang@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
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).