RE: [PATCH iwl-next 1/1] igc: Improve XDP_SETUP_PROG process

[Song, Yoong Siang]

On Thursday, December 5, 2024 8:38 AM, Gomes, Vinicius <vinicius.gomes@xxxxxxxxx> wrote:
>Song Yoong Siang <yoong.siang.song@xxxxxxxxx> 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@xxxxxxxxx>
>> ---
>
>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