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: <PH0PR11MB5830008CD236D81B7224A05FD8B22@PH0PR11MB5830.namprd11.prod.outlook.com>
Date: Thu, 1 Aug 2024 07:45:36 +0000
From: "Song, Yoong Siang" <yoong.siang.song@...el.com>
To: "Drewek, Wojciech" <wojciech.drewek@...el.com>, "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 6:09 PM, Drewek, Wojciech <wojciech.drewek@...el.com> wrote:

[...]

>> +	if (mrqe != IGC_MRQC_ENABLE_MQ && mrqe !=
>IGC_MRQC_ENABLE_RSS_MQ)
>> +		return false;
>
>just:
>return mrqe != IGC_MRQC_ENABLE_MQ && mrqe != IGC_MRQC_ENABLE_RSS_MQ
>

Hi Drewek Wojciech,
Thanks for your review comments.
I will improve the code accordingly in v2.
[...]

>> +	if (!igc_is_default_queue_supported(mrqc)) {
>> +		netdev_warn(adapter->netdev,
>> +			    "MRQ disabled: default RxQ is ignored.\n");
>
>Should we return an error here?

Yes, we should. I plan to refactor this patch to use
ethtool ntuple method, instead of sysfs.
I will consider your suggestion in v2.

>> +	}
>> +
>> +	return (mrqc & IGC_MRQC_DEFAULT_QUEUE_MASK) >>
>> +		IGC_MRQC_DEFAULT_QUEUE_SHIFT;
>
>use FIELD_GET here
>

Noted.
[...]

>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	struct net_device *netdev = pci_get_drvdata(pdev);
>> +	struct igc_adapter *adapter = netdev_priv(netdev);
>> +	u32 default_rx_queue = igc_get_default_rx_queue(adapter);
>
>Use RCT rule
>

Noted.
[...]

>> +	struct pci_dev *pdev = to_pci_dev(dev);
>> +	struct net_device *netdev = pci_get_drvdata(pdev);
>> +	struct igc_adapter *adapter = netdev_priv(netdev);
>> +	u32 default_rx_queue;
>> +	int err;
>
>RCT
>

Noted.
[...]

>> +	err = sysfs_create_group(&adapter->pdev->dev.kobj, &attr_group);
>> +	if (err) {
>
>no need for brackets
>

Noted.
[...]

Thanks & Regards
Siang