netdev - Re: [PATCH v6 net-next 2/7] net: ethernet: am65-cpsw: cleanup TAPRIO handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <af23bd5d-2daf-487b-858c-9e3ad684864d@kernel.org>
Date: Tue, 21 Nov 2023 11:23:09 +0200
From: Roger Quadros <rogerq@...nel.org>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, s-vadapalli@...com, r-gunasekaran@...com,
 vigneshr@...com, srk@...com, horms@...nel.org, p-varis@...com,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 net-next 2/7] net: ethernet: am65-cpsw: cleanup TAPRIO
 handling



On 21/11/2023 00:56, Vladimir Oltean wrote:
> On Mon, Nov 20, 2023 at 04:01:42PM +0200, Roger Quadros wrote:
>> -static int am65_cpsw_configure_taprio(struct net_device *ndev,
>> -				      struct am65_cpsw_est *est_new)
>> +static void am65_cpsw_cp_taprio(struct tc_taprio_qopt_offload *from,
>> +				struct tc_taprio_qopt_offload *to)
>> +{
>> +	int i;
>> +
>> +	*to = *from;
>> +	for (i = 0; i < from->num_entries; i++)
>> +		to->entries[i] = from->entries[i];
>> +}
> 
> I think I mentioned this before: have you looked at taprio_offload_get()
> and taprio_offload_put()?

I'm sorry that I missed this. I'll take a look.

> 
>> +
>> +static int am65_cpsw_taprio_replace(struct net_device *ndev,
>> +				    struct tc_taprio_qopt_offload *taprio)
>>  {
>>  	struct am65_cpsw_common *common = am65_ndev_to_common(ndev);
>> +	struct netlink_ext_ack *extack = taprio->mqprio.extack;
>> +	struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
>>  	struct am65_cpts *cpts = common->cpts;
>>  	int ret = 0, tact = TACT_PROG;
>> +	struct am65_cpsw_est *est_new;
>>  
>> -	am65_cpsw_est_update_state(ndev);
>> +	if (!netif_running(ndev)) {
>> +		NL_SET_ERR_MSG_MOD(extack, "interface is down, link speed unknown\n");
> 
> The extack message doesn't need a \n.

OK.

> 
>> +		return -ENETDOWN;
>> +	}
>>  
>> -	if (est_new->taprio.cmd == TAPRIO_CMD_DESTROY) {
>> -		am65_cpsw_stop_est(ndev);
>> -		return ret;
>> +	if (common->pf_p0_rx_ptype_rrobin) {
>> +		NL_SET_ERR_MSG_MOD(extack,
>> +				   "p0-rx-ptype-rrobin flag conflicts with taprio qdisc\n");
> 
> Also here.
> 
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (port->qos.link_speed == SPEED_UNKNOWN)
>> +		return -ENOLINK;
>> +
>> +	if (taprio->cycle_time_extension) {
>> +		NL_SET_ERR_MSG_MOD(extack,
>> +				   "cycle time extension not supported");
> 
> Here it's ok.
> 
>> +		return -EOPNOTSUPP;
>>  	}

Thanks for the detailed review!

-- 
cheers,
-roger