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] [day] [month] [year] [list]

Message-ID: <SJ1PR11MB6180F185A881FD0E48DD2D39B81C9@SJ1PR11MB6180.namprd11.prod.outlook.com>
Date:   Fri, 9 Dec 2022 01:50:23 +0000
From:   "Zulkifli, Muhammad Husaini" <muhammad.husaini.zulkifli@...el.com>
To:     Kurt Kanzenbach <kurt@...utronix.de>,
        "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
        "Neftin, Sasha" <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

Hello,

> -----Original Message-----
> From: Kurt Kanzenbach <kurt@...utronix.de>
> Sent: Thursday, 8 December, 2022 8:28 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@...el.com>;
> davem@...emloft.net; kuba@...nel.org; pabeni@...hat.com;
> edumazet@...gle.com
> Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@...el.com>;
> netdev@...r.kernel.org; Nguyen, Anthony L
> <anthony.l.nguyen@...el.com>; Neftin, Sasha <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 for the comment. I agreed with you. We can revisit this for i225
later. As for now, our side will lift the restriction for i226 first as HW supporting 
it.

Thanks,
Husaini

> Thanks,
> Kurt