Messages in this thread | | | Date | Wed, 11 Oct 2023 12:40:17 +0530 | Subject | Re: [PATCH net-next v4] net: ti: icssg_prueth: add TAPRIO offload support | From | MD Danish Anwar <> |
| |
On 07/10/23 04:01, Vinicius Costa Gomes wrote: > MD Danish Anwar <danishanwar@ti.com> writes: > >> From: Roger Quadros <rogerq@ti.com> >> >> ICSSG dual-emac f/w supports Enhanced Scheduled Traffic (EST – defined >> in P802.1Qbv/D2.2 that later got included in IEEE 802.1Q-2018) >> configuration. EST allows express queue traffic to be scheduled >> (placed) on the wire at specific repeatable time intervals. In >> Linux kernel, EST configuration is done through tc command and >> the taprio scheduler in the net core implements a software only >> scheduler (SCH_TAPRIO). If the NIC is capable of EST configuration, >> user indicate "flag 2" in the command which is then parsed by >> taprio scheduler in net core and indicate that the command is to >> be offloaded to h/w. taprio then offloads the command to the >> driver by calling ndo_setup_tc() ndo ops. This patch implements >> ndo_setup_tc() to offload EST configuration to ICSSG. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com> >> Signed-off-by: MD Danish Anwar <danishanwar@ti.com> >> --- >> Cc: Roger Quadros <rogerq@ti.com> >> Cc: Andrew Lunn <andrew@lunn.ch> >> Cc: Vinicius Costa Gomes <vinicius.gomes@intel.com> >> Cc: Jakub Kicinski <kuba@kernel.org> >>
[ ... ]
>> + >> +static int emac_taprio_replace(struct net_device *ndev, >> + struct tc_taprio_qopt_offload *taprio) >> +{ >> + struct prueth_emac *emac = netdev_priv(ndev); >> + struct tc_taprio_qopt_offload *est_new; >> + int ret, idx; >> + >> + if (taprio->cycle_time_extension) { >> + netdev_err(ndev, "Failed to set cycle time extension"); >> + return -EOPNOTSUPP; >> + } >> + >> + if (taprio->num_entries == 0 || >> + taprio->num_entries > TAS_MAX_CMD_LISTS) { >> + NL_SET_ERR_MSG_FMT_MOD(taprio->extack, "unsupported num_entries %ld in taprio config\n", >> + taprio->num_entries); >> + return -EINVAL; >> + } >> + >> + /* If any time_interval is 0 in between the list, then exit */ >> + for (idx = 0; idx < taprio->num_entries; idx++) { >> + if (taprio->entries[idx].interval == 0) { >> + NL_SET_ERR_MSG_MOD(taprio->extack, "0 interval in taprio config not supported\n"); >> + return -EINVAL; >> + } >> + } >> + >> + if (emac->qos.tas.taprio_admin) >> + devm_kfree(&ndev->dev, emac->qos.tas.taprio_admin); >> + >> + est_new = devm_kzalloc(&ndev->dev, >> + struct_size(est_new, entries, taprio->num_entries), >> + GFP_KERNEL); >> + if (!est_new) >> + return -ENOMEM; >> + >> + emac_cp_taprio(taprio, est_new); >> + emac->qos.tas.taprio_admin = est_new; >> + ret = tas_update_oper_list(emac); >> + if (ret) >> + return ret; >> + >> + ret = tas_set_state(emac, TAS_STATE_ENABLE); > > The double space is still here... >
Sorry my bad. I'll fix this in next revision.
>> + if (ret) >> + devm_kfree(&ndev->dev, est_new); > > ... as is the free'ing of 'est_new' while 'taprio_admin' still points to it. >
I will add below line here to assign 'taprio_admin' to NULL once 'est_new' is freed.
emac->qos.tas.taprio_admin = NULL;
I will send the next revision with these two fixes. Please let me know if this is OK or if something else is also needed.
>> + >> + return ret; >> +} >> +
[ ... ]
>> + >> +void icssg_qos_tas_init(struct net_device *ndev); >> +int icssg_qos_ndo_setup_tc(struct net_device *ndev, enum tc_setup_type type, >> + void *type_data); >> +#endif /* __NET_TI_ICSSG_QOS_H */ >> -- >> 2.34.1 >> >
-- Thanks and Regards, Danish
| |