Date:   Mon, 29 Jan 2018 15:36:16 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
Cc:     davem@...emloft.net, jiri@...nulli.us, dsahern@...il.com,
        daniel@...earbox.net, john.fastabend@...il.com,
        netdev@...r.kernel.org, oss-drivers@...ronome.com,
        aring@...atatu.com, Simon Horman <simon.horman@...ronome.com>,
        Eelco Chaudron <echaudro@...hat.com>,
        Or Gerlitz <gerlitz.or@...il.com>
Subject: Re: [PATCH net-next v2 00/12] net: sched: propagate extack to cls
 offloads on destroy and only with skip_sw

On Sun, 28 Jan 2018 20:39:02 -0200, Marcelo Ricardo Leitner wrote:
> On Thu, Jan 25, 2018 at 02:57:17PM -0800, Jakub Kicinski wrote:
> > On Thu, 25 Jan 2018 13:11:57 -0200, Marcelo Ricardo Leitner wrote:  
> > > On Wed, Jan 24, 2018 at 12:54:12PM -0800, Jakub Kicinski wrote:  
> > > > Hi!
> > > > 
> > > > This series some of Jiri's comments and the fact that today drivers
> > > > may produce extack even if there is no skip_sw flag (meaning the
> > > > driver failure is not really a problem), and warning messages will
> > > > only confuse the users.    
> > > 
> > > It's a fair point, but I think this is not the best solution. How will
> > > the user then know why it failed to install in hw? Will have to
> > > install a new rule, just with skip_sw, and hope that it fails with the
> > > same reason?
> > >
> > > Maybe it's better to let tc/ovs/etc only exhibit this information
> > > under a certain log/debug level.  
> > 
> > What you say does make sense in case of classifiers which are basically
> > HW offload vehicles.  But for cls_bpf, which people are actually using
> > heavily as a software solution, I don't want any warning to be produced
> > just because someone happened to run the command on a Netronome
> > card :(  Say someone swaps an old NIC for a NFP, and runs their old
> > cls_bpf scripts and suddenly there are warnings they don't care about
> > and have no way of silencing.  
> 
> (Sorry for the delay on replying, btw. I'm still traveling.)
> 
> Makes sense. I agree that at least it shouldn't be displayed in a way
> that may lead the user to think it was a big/fatal error.
> 
> > I do think skip_sw will fail for the same reason, unless someone adds
> > extacks for IO or memory allocation problems or other transient things.  
> 
> I don't really follow this one. Fail you mean, fail to report the
> actual reason? If so, ok, but that's something easily fixable I think,
> especially because with skip_sw, if such an error happens, it's fatal
> for the operation so the error reporting is consistent.

I was referring to your question: "Will have to install a new rule,
just with skip_sw, and hope that it fails with the same reason?"
So yes, currently the only way to get the extack would be to retry the
command with skip_sw.

> > add a "verbose offload" flag to the API or filter the bad extacks at
> > the user space level.  Although, again, my preference would be not to
> > filter at the user space level, because user space can't differentiate
> > between a probably-doesn't-matter-but-HW-offload-failed warning or legit
> > something-is-not-right-in-the-software-but-command-succeeded warning :S  
> 
> But userspace is the original requester. It should know what the rule
> is intended for and how to act upon it. For ovs, for example, it could
> just log a message and move on, while tc could report "hey, ok, but
> please note that the rule couldn't be offloaded".
> 
> > So if there is a major use for non-forced offload failure warnings I
> > would lean towards a new flag.  
> 
> I'm thinking about this, still undecided. In the end maybe a counter
> somewhere could be enough and such reporting is not needed. Thinking..

Hm, special counters for failure reasons or just for all failures to
offload?  FWIW user space can dump the filters and if the filter is
offloaded there will be an in_hw flag Or added a few releases back.  
Let us know what your thoughts are :)

Powered by blists - more mailing lists