Hi,
"Abdul Rahim, Faizal" faizal.abdul.rahim@linux.intel.com writes:
On 12/7/2024 1:10 am, Vinicius Costa Gomes wrote:
"Abdul Rahim, Faizal" faizal.abdul.rahim@linux.intel.com writes:
Hi Vinicius,
On 11/7/2024 6:44 am, Vinicius Costa Gomes wrote:
Faizal Rahim faizal.abdul.rahim@linux.intel.com writes:
Following the "igc: Fix TX Hang issue when QBV Gate is close" changes, remaining issues with the reset adapter logic in igc_tsn_offload_apply() have been observed:
- The reset adapter logics for i225 and i226 differ, although they should be the same according to the guidelines in I225/6 HW Design Section 7.5.2.1 on software initialization during tx mode changes.
- The i225 resets adapter every time, even though tx mode doesn't change. This occurs solely based on the condition igc_is_device_id_i225() when calling schedule_work().
- i226 doesn't reset adapter for tsn->legacy tx mode changes. It only resets adapter for legacy->tsn tx mode transitions.
- qbv_count introduced in the patch is actually not needed; in this context, a non-zero value of qbv_count is used to indicate if tx mode was unconditionally set to tsn in igc_tsn_enable_offload(). This could be replaced by checking the existing register IGC_TQAVCTRL_TRANSMIT_MODE_TSN bit.
This patch resolves all issues and enters schedule_work() to reset the adapter only when changing tx mode. It also removes reliance on qbv_count.
qbv_count field will be removed in a future patch.
Test ran:
Verify reset adapter behaviour in i225/6: a) Enrol a new GCL Reset adapter observed (tx mode change legacy->tsn) b) Enrol a new GCL without deleting qdisc No reset adapter observed (tx mode remain tsn->tsn) c) Delete qdisc Reset adapter observed (tx mode change tsn->legacy)
Tested scenario from "igc: Fix TX Hang issue when QBV Gate is closed" to confirm it remains resolved.
Fixes: 175c241288c0 ("igc: Fix TX Hang issue when QBV Gate is closed") Signed-off-by: Faizal Rahim faizal.abdul.rahim@linux.intel.com Reviewed-by: Simon Horman horms@kernel.org
There were a quite a few bugs, some of them my fault, on this part of the code, changing between the modes in the hardware.
So I would like some confirmation that ETF offloading/LaunchTime was also tested with this change. Just to be sure.
But code-wise, looks good:
Acked-by: Vinicius Costa Gomes vinicius.gomes@intel.com
Cheers,
Tested etf with offload, looks like working correctly.
- mqprio
tc qdisc add dev enp1s0 handle 100: parent root mqprio num_tc 3 \ map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \ queues 1@0 1@1 2@2 \ hw 0
No reset adapter observed.
- etf with offload
tc qdisc replace dev enp1s0 parent 100:1 etf \ clockid CLOCK_TAI delta 300000 offload
Reset adapter observed (tx mode legacy -> tsn).
- delete qdisc
tc qdisc delete dev enp1s0 parent root handle 100
Reset adapter observed (tx mode tsn -> legacy).
That no unexpected resets are happening, is good.
But what I had in mind was some functional tests that ETF is working. I guess that's the only way of knowing that it's still working. Sorry that I wasn't clear about that.
Cheers,
My bad.
Just tested ETF functionality and it is working.
Awesome. Thanks for the confirmation:
Acked-by: Vinicius Costa Gomes vinicius.gomes@intel.com
Cheers,