RE: [PATCH net-next v2 4/9] nfp: extend flower add flow offload

From: David Laight <David.Laight@ACULAB.COM>
To: 'Jakub Kicinski' <jakub.kicinski@netronome.com>,
	Yunsheng Lin <linyunsheng@huawei.com>
Cc: Simon Horman <simon.horman@netronome.com>,
	David Miller <davem@davemloft.net>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"oss-drivers@netronome.com" <oss-drivers@netronome.com>,
	"Pieter Jansen van Vuuren" <pieter.jansenvanvuuren@netronome.com>
Subject: RE: [PATCH net-next v2 4/9] nfp: extend flower add flow offload
Date: Thu, 29 Jun 2017 13:47:33 +0000	[thread overview]
Message-ID: <063D6719AE5E284EB5DD2968C1650D6DD002DBD7@AcuExch.aculab.com> (raw)
In-Reply-To: <20170628234829.76098a18@cakuba.netronome.com>

From: Jakub Kicinski
> Sent: 29 June 2017 07:48
> On Thu, 29 Jun 2017 14:18:07 +0800, Yunsheng Lin wrote:
> > > +	if (mask_basic->n_proto) {
> > cpu_to_be16(mask_basic->n_proto)

Should be be16_to_cpu()

> > remove cpu_to_be16 in case.
> 
> Thanks, but this is incorrect.  Byte swapping constants is done at
> compilation time - therefore it's preferred.

Except that the 'cpu' values are likely to be dense so the compiler
is likely to generate a jump table for the switch statement instead
of sequence of conditionals.

OTOH the jump table is almost certainly a data cache miss, whereas
the conditionals might be predicted correctly.

The best code might come from an explicitly ordered sequence of
conditionals.

	David