netdev - RE: [patch net-next 3/5] net: sch: prio: Add offload ability to PRIO qdisc

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:   Fri, 12 Jan 2018 09:20:46 +0000
From:   Nogah Frankel <nogahf@...lanox.com>
To:     Yuval Mintz <yuvalm@...lanox.com>, Jakub Kicinski <kubakici@...pl>
CC:     Jiri Pirko <jiri@...nulli.us>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "Ido Schimmel" <idosch@...lanox.com>, mlxsw <mlxsw@...lanox.com>,
        "jhs@...atatu.com" <jhs@...atatu.com>,
        "xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>
Subject: RE: [patch net-next 3/5] net: sch: prio: Add offload ability to PRIO
 qdisc



> -----Original Message-----
> From: Yuval Mintz
> Sent: Friday, January 12, 2018 2:05 AM
> To: Jakub Kicinski <kubakici@...pl>
> Cc: Jiri Pirko <jiri@...nulli.us>; netdev@...r.kernel.org; Nogah Frankel
> <nogahf@...lanox.com>; davem@...emloft.net; Ido Schimmel
> <idosch@...lanox.com>; mlxsw <mlxsw@...lanox.com>; jhs@...atatu.com;
> xiyou.wangcong@...il.com
> Subject: RE: [patch net-next 3/5] net: sch: prio: Add offload ability to PRIO qdisc
> 
> > > > > +struct tc_prio_qopt_offload_params {
> > > > > +	int bands;
> > > > > +	u8 priomap[TC_PRIO_MAX + 1];
> > > > > +	/* In case that a prio qdisc is offloaded and now is changed to a
> > > > > +	 * non-offloadedable config, it needs to update the backlog value
> > > > > +	 * to negate the HW backlog value.
> > > > > +	 */
> > > > > +	u32 *backlog;
> > > > > +};
> > > >
> > > > Could we please pass the full qstats on replace and destroy.  This
> > > > simplifies the driver code and allows handling the qlen as well as
> > > > backlog.  Please see the 2 patch series I sent earlier yesterday.

On replace - no problem.
On destroy, I think it is redundant because the qdisc is going to be
deleted anyway.
(To make sure that we are on the same page, if replace in the HW fails, we
"rollback" only the backlog and qlen. The rest of the counters like TX
count should reflect the number of the packets that passed in the HW while
this qdisc was offloaded)

> > >
> > > That might give the false impression that offloading driver is expected
> > > to correct all the qstats fields during destruction, whereas for most of
> > > them it doesn't seem appropriate.
> >
> > The driver is supposed to return the momentary stats to their
> > original/SW-only value.  And the driver knows exactly which stats
> > those are, just look at your patch 5, you handle backlog completely
> > differently than other stats already.
> 
> *we* surely understand that now. I'm just mentioning it might
> confuse future offloaders; No strong objection here.
> And I agree the alternative [passing pointers to each momentary stat]
> is quite ugly.