[net-next,0/6] igc: Fix corner cases for TSN offload

Message ID 20230614140714.14443-1-florian.kauer@linutronix.de
Headers
Series igc: Fix corner cases for TSN offload |

Message

Florian Kauer June 14, 2023, 2:07 p.m. UTC
  The igc driver supports several different offloading capabilities
relevant in the TSN context. Recent patches in this area introduced
regressions for certain corner cases that are fixed in this series.

Each of the patches (except the first one) addresses a different
regression that can be separately reproduced. Still, they have
overlapping code changes so they should not be separately applied.

Especially #4 and #6 address the same observation,
but both need to be applied to avoid TX hang occurrences in
the scenario described in the patches.

Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>

Florian Kauer (6):
  igc: Rename qbv_enable to taprio_offload_enable
  igc: Do not enable taprio offload for invalid arguments
  igc: Handle already enabled taprio offload for basetime 0
  igc: No strict mode in pure launchtime/CBS offload
  igc: Fix launchtime before start of cycle
  igc: Fix inserting of empty frame for launchtime

 drivers/net/ethernet/intel/igc/igc.h      |  2 +-
 drivers/net/ethernet/intel/igc/igc_main.c | 24 ++++++++-------------
 drivers/net/ethernet/intel/igc/igc_tsn.c  | 26 ++++++++++++++++++++---
 3 files changed, 33 insertions(+), 19 deletions(-)
  

Comments

Zulkifli, Muhammad Husaini June 16, 2023, 3:53 p.m. UTC | #1
Dear Florian,

Thanks for the patch series 😊

> -----Original Message-----
> From: Florian Kauer <florian.kauer@linutronix.de>
> Sent: Wednesday, 14 June, 2023 10:07 PM
> To: Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Gomes, Vinicius <vinicius.gomes@intel.com>;
> David S . Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Tan Tee Min <tee.min.tan@linux.intel.com>; Zulkifli,
> Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>; Gunasekaran,
> Aravindhan <aravindhan.gunasekaran@intel.com>; Chilakala, Mallikarjuna
> <mallikarjuna.chilakala@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; kurt@linutronix.de; florian.kauer@linutronix.de
> Subject: [PATCH net-next 0/6] igc: Fix corner cases for TSN offload
> 
> The igc driver supports several different offloading capabilities relevant in the
> TSN context. Recent patches in this area introduced regressions for certain
> corner cases that are fixed in this series.
> 
> Each of the patches (except the first one) addresses a different regression that
> can be separately reproduced. Still, they have overlapping code changes so they
> should not be separately applied.
> 
> Especially #4 and #6 address the same observation, but both need to be applied
> to avoid TX hang occurrences in the scenario described in the patches.
> 
> Signed-off-by: Florian Kauer <florian.kauer@linutronix.de>
> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
> 
> Florian Kauer (6):
>   igc: Rename qbv_enable to taprio_offload_enable
>   igc: Do not enable taprio offload for invalid arguments
>   igc: Handle already enabled taprio offload for basetime 0
>   igc: No strict mode in pure launchtime/CBS offload
>   igc: Fix launchtime before start of cycle
>   igc: Fix inserting of empty frame for launchtime

All six patches, as far as I can see here, have the Fixes tag. Should they go to Net instead of Net-Next?

> 
>  drivers/net/ethernet/intel/igc/igc.h      |  2 +-
>  drivers/net/ethernet/intel/igc/igc_main.c | 24 ++++++++-------------
> drivers/net/ethernet/intel/igc/igc_tsn.c  | 26 ++++++++++++++++++++---
>  3 files changed, 33 insertions(+), 19 deletions(-)
> 
> --
> 2.39.2
  
Florian Kauer June 16, 2023, 4:10 p.m. UTC | #2
On 16.06.23 17:53, Zulkifli, Muhammad Husaini wrote:
>> Florian Kauer (6):
>>   igc: Rename qbv_enable to taprio_offload_enable
>>   igc: Do not enable taprio offload for invalid arguments
>>   igc: Handle already enabled taprio offload for basetime 0
>>   igc: No strict mode in pure launchtime/CBS offload
>>   igc: Fix launchtime before start of cycle
>>   igc: Fix inserting of empty frame for launchtime
> 
> All six patches, as far as I can see here, have the Fixes tag. Should they go to Net instead of Net-Next?

You are correct, these are all fixes and could go to net.
However, in its current form they will not fully apply to net 
(e.g. due to the commit 2d800bc500fb ("net/sched: taprio: replace tc_taprio_qopt_offload :: enable with a "cmd" enum")
that has overlapping code changes) and are also not tested with net. 
If you prefer to have them in net already I could send a second series.
For me personally all options (net, net-next or iwl-next) would be fine.

Thanks,
Florian
  
Zulkifli, Muhammad Husaini June 16, 2023, 4:39 p.m. UTC | #3
> -----Original Message-----
> From: Florian Kauer <florian.kauer@linutronix.de>
> Sent: Saturday, 17 June, 2023 12:11 AM
> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>;
> Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Gomes, Vinicius <vinicius.gomes@intel.com>;
> David S . Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Tan Tee Min <tee.min.tan@linux.intel.com>;
> Gunasekaran, Aravindhan <aravindhan.gunasekaran@intel.com>; Chilakala,
> Mallikarjuna <mallikarjuna.chilakala@intel.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; kurt@linutronix.de
> Subject: Re: [PATCH net-next 0/6] igc: Fix corner cases for TSN offload
> 
> On 16.06.23 17:53, Zulkifli, Muhammad Husaini wrote:
> >> Florian Kauer (6):
> >>   igc: Rename qbv_enable to taprio_offload_enable
> >>   igc: Do not enable taprio offload for invalid arguments
> >>   igc: Handle already enabled taprio offload for basetime 0
> >>   igc: No strict mode in pure launchtime/CBS offload
> >>   igc: Fix launchtime before start of cycle
> >>   igc: Fix inserting of empty frame for launchtime
> >
> > All six patches, as far as I can see here, have the Fixes tag. Should they go to
> Net instead of Net-Next?
> 
> You are correct, these are all fixes and could go to net.
> However, in its current form they will not fully apply to net (e.g. due to the
> commit 2d800bc500fb ("net/sched: taprio: replace tc_taprio_qopt_offload ::
> enable with a "cmd" enum") that has overlapping code changes) and are also
> not tested with net.
> If you prefer to have them in net already I could send a second series.
> For me personally all options (net, net-next or iwl-next) would be fine.

Yeah I would prefer "net" so that it can be available in current development kernel.
"Net-next" will take sometimes to go in....
Looks like only patch no 2 "igc: Do not enable taprio offload for invalid arguments"
will have conflict due to the new "cmd" command introduced by vlamidir. 
But I think should be minor changes.

Thanks

> 
> Thanks,
> Florian