[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <106e0d31-c520-4ef6-994e-df1a4c9a986e@linux.intel.com>
Date: Wed, 26 Jun 2024 09:19:12 +0800
From: "Abdul Rahim, Faizal" <faizal.abdul.rahim@...ux.intel.com>
To: "David S . Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Jesse Brandeburg <jesse.brandeburg@...el.com>,
Tony Nguyen <anthony.l.nguyen@...el.com>,
Vinicius Costa Gomes <vinicius.gomes@...el.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, stable@...r.kernel.org,
Vladimir Oltean <vladimir.oltean@....com>,
muhammad.husaini.zulkifli@...el.com
Subject: Re: [PATCH net 1/1] igc: Fix double reset adapter triggered from a
single taprio cmd
Added Vladimir and Husaini in CC.
On 25/6/2024 4:26 pm, Faizal Rahim wrote:
> Following the implementation of "igc: Add TransmissionOverrun counter"
> patch, when a taprio command is triggered by user, igc processes two
> commands: TAPRIO_CMD_REPLACE followed by TAPRIO_CMD_STATS. However, both
> commands unconditionally pass through igc_tsn_offload_apply() which
> evaluates and triggers reset adapter. The double reset causes issues in
> the calculation of adapter->qbv_count in igc.
>
> TAPRIO_CMD_REPLACE command is expected to reset the adapter since it
> activates qbv. It's unexpected for TAPRIO_CMD_STATS to do the same
> because it doesn't configure any driver-specific TSN settings. So, the
> evaluation in igc_tsn_offload_apply() isn't needed for TAPRIO_CMD_STATS.
>
> To address this, commands parsing are relocated to
> igc_tsn_enable_qbv_scheduling(). Commands that don't require an adapter
> reset will exit after processing, thus avoiding igc_tsn_offload_apply().
>
> Fixes: d3750076d464 ("igc: Add TransmissionOverrun counter")
> Signed-off-by: Faizal Rahim <faizal.abdul.rahim@...ux.intel.com>
> ---
> drivers/net/ethernet/intel/igc/igc_main.c | 33 ++++++++++++-----------
> 1 file changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 87b655b839c1..33069880c86c 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -6310,21 +6310,6 @@ static int igc_save_qbv_schedule(struct igc_adapter *adapter,
> size_t n;
> int i;
>
> - switch (qopt->cmd) {
> - case TAPRIO_CMD_REPLACE:
> - break;
> - case TAPRIO_CMD_DESTROY:
> - return igc_tsn_clear_schedule(adapter);
> - case TAPRIO_CMD_STATS:
> - igc_taprio_stats(adapter->netdev, &qopt->stats);
> - return 0;
> - case TAPRIO_CMD_QUEUE_STATS:
> - igc_taprio_queue_stats(adapter->netdev, &qopt->queue_stats);
> - return 0;
> - default:
> - return -EOPNOTSUPP;
> - }
> -
> if (qopt->base_time < 0)
> return -ERANGE;
>
> @@ -6433,7 +6418,23 @@ static int igc_tsn_enable_qbv_scheduling(struct igc_adapter *adapter,
> if (hw->mac.type != igc_i225)
> return -EOPNOTSUPP;
>
> - err = igc_save_qbv_schedule(adapter, qopt);
> + switch (qopt->cmd) {
> + case TAPRIO_CMD_REPLACE:
> + err = igc_save_qbv_schedule(adapter, qopt);
> + break;
> + case TAPRIO_CMD_DESTROY:
> + err = igc_tsn_clear_schedule(adapter);
> + break;
> + case TAPRIO_CMD_STATS:
> + igc_taprio_stats(adapter->netdev, &qopt->stats);
> + return 0;
> + case TAPRIO_CMD_QUEUE_STATS:
> + igc_taprio_queue_stats(adapter->netdev, &qopt->queue_stats);
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> if (err)
> return err;
>
> --
> 2.25.1
>
Powered by blists - more mailing lists