| Message ID | 20230219135309.594188-1-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add tc-mqprio and tc-taprio support for preemptible traffic classes | expand |
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>
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?
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