linux-kernel - RE: [PATCH iwl-next 1/1] igc: Improve XDP_SETUP

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] [day] [month] [year] [list]

Message-ID: <PH0PR11MB5830F1658AB3F74BBA2C9634D8302@PH0PR11MB5830.namprd11.prod.outlook.com>
Date: Thu, 5 Dec 2024 02:45:01 +0000
From: "Song, Yoong Siang" <yoong.siang.song@...el.com>
To: "Gomes, Vinicius" <vinicius.gomes@...el.com>, Alexei Starovoitov
	<ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, "David S . Miller"
	<davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, "Jesper Dangaard
 Brouer" <hawk@...nel.org>, John Fastabend <john.fastabend@...il.com>,
	"Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>, Andrew Lunn <andrew+netdev@...n.ch>, "Eric
 Dumazet" <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, "Richard
 Cochran" <richardcochran@...il.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>
Subject: RE: [PATCH iwl-next 1/1] igc: Improve XDP_SETUP_PROG process

On Thursday, December 5, 2024 8:38 AM, Gomes, Vinicius <vinicius.gomes@...el.com> wrote:
>Song Yoong Siang <yoong.siang.song@...el.com> writes:
>
>> Improve XDP_SETUP_PROG process by avoiding unnecessary link down/up event
>> and hardware device reset.
>>
>
>Some examples of problems that these hardware resets are causing would
>be good.
>

Sure, maybe I can give an example of "before" and "after" behavior
on ptp4l clock sync log in commit msg in next submission.

>> Signed-off-by: Song Yoong Siang <yoong.siang.song@...el.com>
>> ---
>
>The duplication of code doesn't look that good. Initialization is
>tricky, it seems to make it easy to update one place and forget to
>update the other.
>

The original idea of this patch is to avoid huge changes on
igc_open/igc_close functions. But I agree with you that
duplication of code is challenging to maintain. I will create
common function whenever necessary in next submission.

>A couple of ideas:
> - separate the code into functions that can be used from the "usual"
> igc_open()/igc_close() flow;
> - it seems that igc_close()/igc_open() are too big a hammer for
> installing a new XDP program: what do we really need? (my mental model
> is: 1. stop new traffic from going into any queue; 2. wait for any
> packets "in progress"; 3. install the program; 4. resume operations;
> what else?)

I think will need to reallocate DMA resource, if user enable XDP pool first,
then only setup XDP prog. Maybe I can reuse the reset sequence in
igc_xdp_enable_pool(). I will study further.

[...]
>
>Cheers,
>--
>Vinicius

Thanks & Regards
Siang