netdev - RE: [PATCH iwl-next,v1 2/3] igc: Add default Rx queue configuration via sysfs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <PH0PR11MB58307F45C9803E808D7F984AD8B22@PH0PR11MB5830.namprd11.prod.outlook.com>
Date: Thu, 1 Aug 2024 07:20:26 +0000
From: "Song, Yoong Siang" <yoong.siang.song@...el.com>
To: Kurt Kanzenbach <kurt@...utronix.de>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "David S . Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Richard Cochran <richardcochran@...il.com>, "Alexei
 Starovoitov" <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Jesper
 Dangaard Brouer <hawk@...nel.org>, John Fastabend <john.fastabend@...il.com>,
	"Gomes, Vinicius" <vinicius.gomes@...el.com>, Jonathan Corbet
	<corbet@....net>, "Kitszel, Przemyslaw" <przemyslaw.kitszel@...el.com>,
	Shinas Rasheed <srasheed@...vell.com>, "Tian, Kevin" <kevin.tian@...el.com>,
	Brett Creeley <brett.creeley@....com>, "Blanco Alcaine, Hector"
	<hector.blanco.alcaine@...el.com>, "Hay, Joshua A" <joshua.a.hay@...el.com>,
	"Neftin, Sasha" <sasha.neftin@...el.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"bpf@...r.kernel.org" <bpf@...r.kernel.org>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>
Subject: RE: [PATCH iwl-next,v1 2/3] igc: Add default Rx queue configuration
 via sysfs

On Tuesday, July 30, 2024 5:59 PM, Kurt Kanzenbach <kurt@...utronix.de> wrote:
>> --- a/drivers/net/ethernet/intel/igc/igc_regs.h
>> +++ b/drivers/net/ethernet/intel/igc/igc_regs.h
>> @@ -63,6 +63,12 @@
>>  /* RSS registers */
>>  #define IGC_MRQC		0x05818 /* Multiple Receive Control - RW */
>>
>> +/* MRQC register bit definitions */
>
>Nit: Now, the MRQC register definitions are scattered over two files:
>igc_regs.h and igc.h. igc.h has
>
>#define IGC_MRQC_ENABLE_RSS_MQ		0x00000002
>#define IGC_MRQC_RSS_FIELD_IPV4_UDP	0x00400000
>#define IGC_MRQC_RSS_FIELD_IPV6_UDP	0x00800000
>
>Maybe combine them into a single location?
>

Hi Kurt Kanzenbach,

Thanks for your review comment.
Sure, I will try to combine them into single location.

>> +#define IGC_MRQC_ENABLE_MQ		0x00000000
>> +#define IGC_MRQC_ENABLE_MASK		GENMASK(2, 0)
>> +#define IGC_MRQC_DEFAULT_QUEUE_MASK	GENMASK(5, 3)
>> +#define IGC_MRQC_DEFAULT_QUEUE_SHIFT	3
>
>Nit: FIELD_GET() and FIELD_PREP() can help to get rid of the manual
>shifting. See below.
>

Noted.
[...]

>> +	return (mrqc & IGC_MRQC_DEFAULT_QUEUE_MASK) >>
>> +		IGC_MRQC_DEFAULT_QUEUE_SHIFT;
>
>Nit: return FIELD_GET(IGC_MRQC_DEFAULT_QUEUE_MASK, mrqc);
>

Noted.
[...]

>> +	mrqc &= ~IGC_MRQC_DEFAULT_QUEUE_MASK;
>> +	mrqc |= queue << IGC_MRQC_DEFAULT_QUEUE_SHIFT;
>
>Nit: mrqc |= FIELD_PREP(IGC_MRQC_DEFAULT_QUEUE_MASK, queue);
>

Noted.
[...]

>Thanks,
>Kurt

Thanks & Regards
Siang