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:

quoted

On Thu, 25 Jan 2018 13:11:57 -0200, Marcelo Ricardo Leitner wrote:  

quoted

On Wed, Jan 24, 2018 at 12:54:12PM -0800, Jakub Kicinski wrote:  

quoted

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.

quoted

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.

quoted

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".

quoted

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 :)