netdev - Re: [PATCH v2 net-next 1/1] net: dsa: hellcreek: Add TAPRIO offloading support

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]

Date:   Wed, 20 Jan 2021 08:18:15 +0100
From:   Kurt Kanzenbach <kurt@...utronix.de>
To:     Jakub Kicinski <kuba@...nel.org>,
        Vladimir Oltean <olteanv@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Vinicius Costa Gomes <vinicius.gomes@...el.com>,
        netdev@...r.kernel.org
Subject: Re: [PATCH v2 net-next 1/1] net: dsa: hellcreek: Add TAPRIO offloading support

On Tue Jan 19 2021, Jakub Kicinski wrote:
> On Sat, 16 Jan 2021 13:49:22 +0100 Kurt Kanzenbach wrote:
>> +	if (base_time_ns - current_ns < (s64)8 * NSEC_PER_SEC)
>> +		return true;
>> +
>> +	return false;
>
> nit:
> 	return base_time_ns - current_ns < (s64)8 * NSEC_PER_SEC;

Sure.

>> +	/* Schedule periodic schedule check */
>> +	schedule_delayed_work(&hellcreek_port->schedule_work,
>> +			      HELLCREEK_SCHEDULE_PERIOD);
>
> Why schedule this work every 2 seconds rather than scheduling it
> $start_time - 8 sec + epsilon?

The two seconds are taken from the programming guide. That's why I used
it.

The PTP frequency starts to matter for large deltas. In theory the
rescheduling period can be increased [1]. Should I adjust it? 

>
>> +static bool hellcreek_validate_schedule(struct hellcreek *hellcreek,
>> +					struct tc_taprio_qopt_offload *schedule)
>> +{
>> +	/* Does this hellcreek version support Qbv in hardware? */
>> +	if (!hellcreek->pdata->qbv_support)
>> +		return false;
>> +
>> +	/* cycle time can only be 32bit */
>> +	if (schedule->cycle_time > (u32)-1)
>> +		return false;
>> +
>> +	/* cycle time extension is not supported */
>> +	if (schedule->cycle_time_extension)
>> +		return false;
>
> What's the story with entries[i].command? I see most drivers validate
> the command is what they expect.
>

Good catch! I'll add the validation.

Thanks,
Kurt

[1] - https://lkml.kernel.org/netdev/20200901144755.jd2wnmweywwvkwvl@skbuf/

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)