netdev - Re: [PATCH net-next 2/8] igc: remove I226 Qbv BaseTime restriction

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: <87pmcu13mi.fsf@kurt>
Date:   Thu, 08 Dec 2022 13:28:21 +0100
From:   Kurt Kanzenbach <kurt@...utronix.de>
To:     Tony Nguyen <anthony.l.nguyen@...el.com>, davem@...emloft.net,
        kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com
Cc:     Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@...el.com>,
        netdev@...r.kernel.org, anthony.l.nguyen@...el.com,
        sasha.neftin@...el.com, Tan Tee Min <tee.min.tan@...ux.intel.com>,
        Naama Meir <naamax.meir@...ux.intel.com>
Subject: Re: [PATCH net-next 2/8] igc: remove I226 Qbv BaseTime restriction

On Mon Dec 05 2022, Tony Nguyen wrote:
> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@...el.com>
>
> Remove the Qbv BaseTime restriction for I226 so that the BaseTime can be
> scheduled to the future time. A new register bit of Tx Qav Control
> (Bit-7: FutScdDis) was introduced to allow I226 scheduling future time as
> Qbv BaseTime and not having the Tx hang timeout issue.
>
> Besides, according to datasheet section 7.5.2.9.3.3, FutScdDis bit has to
> be configured first before the cycle time and base time.
>
> Indeed the FutScdDis bit is only active on re-configuration, thus we have
> to set the BASET_L to zero and then only set it to the desired value.
>
> Please also note that the Qbv configuration flow is moved around based on
> the Qbv programming guideline that is documented in the latest datasheet.
>
> Co-developed-by: Tan Tee Min <tee.min.tan@...ux.intel.com>
> Signed-off-by: Tan Tee Min <tee.min.tan@...ux.intel.com>
> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@...el.com>
> Tested-by: Naama Meir <naamax.meir@...ux.intel.com>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>

[snip]

> @@ -5852,8 +5853,10 @@ static bool validate_schedule(struct igc_adapter *adapter,
>  	 * in the future, it will hold all the packets until that
>  	 * time, causing a lot of TX Hangs, so to avoid that, we
>  	 * reject schedules that would start in the future.
> +	 * Note: Limitation above is no longer in i226.
>  	 */
> -	if (!is_base_time_past(qopt->base_time, &now))
> +	if (!is_base_time_past(qopt->base_time, &now) &&
> +	    igc_is_device_id_i225(hw))
>  		return false;

Nothing against this patch per se, but you should lift the base time
restriction for i225 as well. Even if it's hardware limitation, the
driver should deal with that e.g., using a timer, workqueue, ... The
TAPRIO interface allows the user to set an arbitrary base time, which
can and most likely will be in the future. IMHO the driver should handle
that. For instance, the hellcreek TSN switch has a similar limitation
(base time can only be applied up to 8 seconds in the future) and I've
worked around it in the driver.

Thanks,
Kurt

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