[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7456cbc2-7ea1-b68c-ce75-fa4b5392ec8c@ti.com>
Date: Wed, 11 Oct 2023 12:40:17 +0530
From: MD Danish Anwar <danishanwar@...com>
To: Vinicius Costa Gomes <vinicius.gomes@...el.com>,
Andrew Lunn
<andrew@...n.ch>, Roger Quadros <rogerq@...nel.org>,
Vignesh Raghavendra
<vigneshr@...com>,
Richard Cochran <richardcochran@...il.com>,
Paolo Abeni
<pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>,
Eric Dumazet
<edumazet@...gle.com>,
"David S. Miller" <davem@...emloft.net>, <vladimir.oltean@....com>,
Simon Horman <horms@...nel.org>
CC: <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>, <srk@...com>,
<r-gunasekaran@...com>, <linux-arm-kernel@...ts.infradead.org>,
Roger Quadros
<rogerq@...com>
Subject: Re: [PATCH net-next v4] net: ti: icssg_prueth: add TAPRIO offload
support
On 07/10/23 04:01, Vinicius Costa Gomes wrote:
> MD Danish Anwar <danishanwar@...com> writes:
>
>> From: Roger Quadros <rogerq@...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@...com>
>> Signed-off-by: Vignesh Raghavendra <vigneshr@...com>
>> Signed-off-by: MD Danish Anwar <danishanwar@...com>
>> ---
>> Cc: Roger Quadros <rogerq@...com>
>> Cc: Andrew Lunn <andrew@...n.ch>
>> Cc: Vinicius Costa Gomes <vinicius.gomes@...el.com>
>> Cc: Jakub Kicinski <kuba@...nel.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
Powered by blists - more mailing lists