mbox series

[v2,net-next,00/12] Add tc-mqprio and tc-taprio support for preemptible traffic classes

Message ID 20230219135309.594188-1-vladimir.oltean@nxp.com
Headers
Series Add tc-mqprio and tc-taprio support for preemptible traffic classes |

Message

Vladimir Oltean Feb. 19, 2023, 1:52 p.m. UTC 
  The last RFC in August 2022 contained a proposal for the UAPI of both
TSN standards which together form Frame Preemption (802.1Q and 802.3):
https://patchwork.kernel.org/project/netdevbpf/cover/20220816222920.1952936-1-vladimir.oltean@nxp.com/

It wasn't clear at the time whether the 802.1Q portion of Frame Preemption
should be exposed via the tc qdisc (mqprio, taprio) or via some other
layer (perhaps also ethtool like the 802.3 portion, or dcbnl), even
though the options were discussed extensively, with pros and cons:
https://patchwork.kernel.org/project/netdevbpf/patch/20220816222920.1952936-3-vladimir.oltean@nxp.com/

So the 802.3 portion got submitted separately and finally was accepted:
https://patchwork.kernel.org/project/netdevbpf/cover/20230119122705.73054-1-vladimir.oltean@nxp.com/

leaving the only remaining question: how do we expose the 802.1Q bits?

This series proposes that we use the Qdisc layer, through separate
(albeit very similar) UAPI in mqprio and taprio, and that both these
Qdiscs pass the information down to the offloading device driver through
the common mqprio offload structure (which taprio also passes).

Implementations are provided for the NXP LS1028A on-board Ethernet
(enetc, felix).

Some patches should have maybe belonged to separate series, leaving here
only patches 09/12 - 12/12, for ease of review. That may be true,
however due to a perceived lack of time to wait for the prerequisite
cleanup to be merged, here they are all together.

Changes in v2:
- add missing EXPORT_SYMBOL_GPL(ethtool_dev_mm_supported)
- slightly reword some commit messages
- move #include <linux/ethtool_netlink.h> to the respective patch in
  mqprio
- remove self-evident comment "only for dump and offloading" in mqprio

v1 at:
https://patchwork.kernel.org/project/netdevbpf/cover/20230216232126.3402975-1-vladimir.oltean@nxp.com/

Vladimir Oltean (12):
  net: enetc: rename "mqprio" to "qopt"
  net: mscc: ocelot: add support for mqprio offload
  net: dsa: felix: act upon the mqprio qopt in taprio offload
  net: ethtool: fix __ethtool_dev_mm_supported() implementation
  net: ethtool: create and export ethtool_dev_mm_supported()
  net/sched: mqprio: simplify handling of nlattr portion of TCA_OPTIONS
  net/sched: mqprio: add extack to mqprio_parse_nlattr()
  net/sched: mqprio: add an extack message to mqprio_parse_opt()
  net/sched: mqprio: allow per-TC user input of FP adminStatus
  net/sched: taprio: allow per-TC user input of FP adminStatus
  net: mscc: ocelot: add support for preemptible traffic classes
  net: enetc: add support for preemptible traffic classes

 drivers/net/dsa/ocelot/felix_vsc9959.c        |  44 ++++-
 drivers/net/ethernet/freescale/enetc/enetc.c  |  31 ++-
 drivers/net/ethernet/freescale/enetc/enetc.h  |   1 +
 .../net/ethernet/freescale/enetc/enetc_hw.h   |   4 +
 drivers/net/ethernet/mscc/ocelot.c            |  51 +++++
 drivers/net/ethernet/mscc/ocelot.h            |   2 +
 drivers/net/ethernet/mscc/ocelot_mm.c         |  56 ++++++
 include/linux/ethtool_netlink.h               |   6 +
 include/net/pkt_sched.h                       |   1 +
 include/soc/mscc/ocelot.h                     |   6 +
 include/uapi/linux/pkt_sched.h                |  17 ++
 net/ethtool/mm.c                              |  25 ++-
 net/sched/sch_mqprio.c                        | 182 +++++++++++++++---
 net/sched/sch_mqprio_lib.c                    |  14 ++
 net/sched/sch_mqprio_lib.h                    |   2 +
 net/sched/sch_taprio.c                        |  65 +++++--
 16 files changed, 460 insertions(+), 47 deletions(-)

Comments

Ferenc Fejes Feb. 20, 2023, 11:15 a.m. UTC | #1 
On Sun, 2023-02-19 at 15:52 +0200, Vladimir Oltean wrote:
> The last RFC in August 2022 contained a proposal for the UAPI of both
> TSN standards which together form Frame Preemption (802.1Q and
> 802.3):
> https://patchwork.kernel.org/project/netdevbpf/cover/20220816222920.1952936-1-vladimir.oltean@nxp.com/
> 
> It wasn't clear at the time whether the 802.1Q portion of Frame
> Preemption
> should be exposed via the tc qdisc (mqprio, taprio) or via some other
> layer (perhaps also ethtool like the 802.3 portion, or dcbnl), even
> though the options were discussed extensively, with pros and cons:
> https://patchwork.kernel.org/project/netdevbpf/patch/20220816222920.1952936-3-vladimir.oltean@nxp.com/
> 
> So the 802.3 portion got submitted separately and finally was
> accepted:
> https://patchwork.kernel.org/project/netdevbpf/cover/20230119122705.73054-1-vladimir.oltean@nxp.com/
> 
> leaving the only remaining question: how do we expose the 802.1Q
> bits?
> 
> This series proposes that we use the Qdisc layer, through separate
> (albeit very similar) UAPI in mqprio and taprio, and that both these
> Qdiscs pass the information down to the offloading device driver
> through
> the common mqprio offload structure (which taprio also passes).
> 
> Implementations are provided for the NXP LS1028A on-board Ethernet
> (enetc, felix).
> 
> Some patches should have maybe belonged to separate series, leaving
> here
> only patches 09/12 - 12/12, for ease of review. That may be true,
> however due to a perceived lack of time to wait for the prerequisite
> cleanup to be merged, here they are all together.
> 
> Changes in v2:
> - add missing EXPORT_SYMBOL_GPL(ethtool_dev_mm_supported)
> - slightly reword some commit messages
> - move #include <linux/ethtool_netlink.h> to the respective patch in
>   mqprio
> - remove self-evident comment "only for dump and offloading" in
> mqprio
> 
> v1 at:
> https://patchwork.kernel.org/project/netdevbpf/cover/20230216232126.3402975-1-vladimir.oltean@nxp.com/
> 
> Vladimir Oltean (12):
>   net: enetc: rename "mqprio" to "qopt"
>   net: mscc: ocelot: add support for mqprio offload
>   net: dsa: felix: act upon the mqprio qopt in taprio offload
>   net: ethtool: fix __ethtool_dev_mm_supported() implementation
>   net: ethtool: create and export ethtool_dev_mm_supported()
>   net/sched: mqprio: simplify handling of nlattr portion of
> TCA_OPTIONS
>   net/sched: mqprio: add extack to mqprio_parse_nlattr()
>   net/sched: mqprio: add an extack message to mqprio_parse_opt()
>   net/sched: mqprio: allow per-TC user input of FP adminStatus
>   net/sched: taprio: allow per-TC user input of FP adminStatus
>   net: mscc: ocelot: add support for preemptible traffic classes
>   net: enetc: add support for preemptible traffic classes
> 
>  drivers/net/dsa/ocelot/felix_vsc9959.c        |  44 ++++-
>  drivers/net/ethernet/freescale/enetc/enetc.c  |  31 ++-
>  drivers/net/ethernet/freescale/enetc/enetc.h  |   1 +
>  .../net/ethernet/freescale/enetc/enetc_hw.h   |   4 +
>  drivers/net/ethernet/mscc/ocelot.c            |  51 +++++
>  drivers/net/ethernet/mscc/ocelot.h            |   2 +
>  drivers/net/ethernet/mscc/ocelot_mm.c         |  56 ++++++
>  include/linux/ethtool_netlink.h               |   6 +
>  include/net/pkt_sched.h                       |   1 +
>  include/soc/mscc/ocelot.h                     |   6 +
>  include/uapi/linux/pkt_sched.h                |  17 ++
>  net/ethtool/mm.c                              |  25 ++-
>  net/sched/sch_mqprio.c                        | 182 +++++++++++++++-
> --
>  net/sched/sch_mqprio_lib.c                    |  14 ++
>  net/sched/sch_mqprio_lib.h                    |   2 +
>  net/sched/sch_taprio.c                        |  65 +++++--
>  16 files changed, 460 insertions(+), 47 deletions(-)
> 

LGTM.

Reviewed-by: Ferenc Fejes <fejes@inf.elte.hu>
Vladimir Oltean Feb. 20, 2023, 11:48 a.m. UTC | #2 
On Mon, Feb 20, 2023 at 12:15:57PM +0100, Ferenc Fejes wrote:
> LGTM.
> 
> Reviewed-by: Ferenc Fejes <fejes@inf.elte.hu>

Thanks a lot for the review!

Unfortunately I need to send a v3, because the C language apparently
doesn't like "default" switch cases with no code, and I need to make
this change (which surprises me, since the code did compile fine with
my gcc-arm-11.2-2022.02-x86_64-aarch64-none-linux-gnu toolchain):

diff --git a/drivers/net/ethernet/mscc/ocelot_mm.c b/drivers/net/ethernet/mscc/ocelot_mm.c
index 21d5656dfc70..f7766927bdd2 100644
--- a/drivers/net/ethernet/mscc/ocelot_mm.c
+++ b/drivers/net/ethernet/mscc/ocelot_mm.c
@@ -57,20 +57,16 @@ void ocelot_port_update_preemptible_tcs(struct ocelot *ocelot, int port)
 
 	lockdep_assert_held(&mm->lock);
 	
-	/* On NXP LS1028A, when using QSGMII, the port hangs if transmitting
-	 * preemptible frames at any other link speed than gigabit
+	/* Only commit preemptible TCs when MAC Merge is active.
+	 * On NXP LS1028A, when using QSGMII, the port hangs if transmitting
+	 * preemptible frames at any other link speed than gigabit, so avoid
+	 * preemption at lower speeds in this PHY mode.
 	 */
-	if (ocelot_port->phy_mode != PHY_INTERFACE_MODE_QSGMII ||
-	    ocelot_port->speed == SPEED_1000) {
-		/* Only commit preemptible TCs when MAC Merge is active */
-		switch (mm->verify_status) {
-		case ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED:
-		case ETHTOOL_MM_VERIFY_STATUS_DISABLED:
-			val = mm->preemptible_tcs;
-			break;
-		default:
-		}
-	}
+	if ((ocelot_port->phy_mode != PHY_INTERFACE_MODE_QSGMII ||
+	     ocelot_port->speed == SPEED_1000) &&
+	    (mm->verify_status == ETHTOOL_MM_VERIFY_STATUS_SUCCEEDED ||
+	     mm->verify_status == ETHTOOL_MM_VERIFY_STATUS_DISABLED))
+		val = mm->preemptible_tcs;
 	
 	ocelot_rmw_rix(ocelot, QSYS_PREEMPTION_CFG_P_QUEUES(val),
 		       QSYS_PREEMPTION_CFG_P_QUEUES_M,

Besides, I'm also taking the opportunity to make one more change, and
really do a thorough job with the netlink extack: I will be passing it
down to the device driver in v3, via struct tc_mqprio_qopt_offload and
struct tc_taprio_qopt_offload.

I'll replicate your review tag for all patches from v2 that will be
present in an unchanged form in v3, ok?
Ferenc Fejes Feb. 20, 2023, 12:05 p.m. UTC | #3 
Hi!

On Mon, 2023-02-20 at 13:48 +0200, Vladimir Oltean wrote:
> On Mon, Feb 20, 2023 at 12:15:57PM +0100, Ferenc Fejes wrote:
> > LGTM.
> > 
> > Reviewed-by: Ferenc Fejes <fejes@inf.elte.hu>
> 
> Thanks a lot for the review!
> 
> Unfortunately I need to send a v3, because the C language apparently
> doesn't like "default" switch cases with no code, and I need to make
> this change (which surprises me, since the code did compile fine with
> my gcc-arm-11.2-2022.02-x86_64-aarch64-none-linux-gnu toolchain):
> 
> 

[...]

> 
> Besides, I'm also taking the opportunity to make one more change, and
> really do a thorough job with the netlink extack: I will be passing
> it
> down to the device driver in v3, via struct tc_mqprio_qopt_offload
> and
> struct tc_taprio_qopt_offload.
> 
> I'll replicate your review tag for all patches from v2 that will be
> present in an unchanged form in v3, ok?

Sure!

Best,
Ferenc