Re: [net-next 04/14] i40e: dump VF information in debugfs
From: "Williams, Mitch A" Date: Thu, 20 Apr 2017 22:25:03 + > I'm not adding backdoor hooks to do naughty things, I'm just (very > slightly) enhancing what's already there. I'm not going to drop this series because of this. I'm just sounding the warning that I probably have to start pushing back harder in the future because seriously.. debugfs is not deployed production and in fact a lot of people disable it for security reasons. Even tracing got out of debugfs for this reason.
Re: [net-next 04/14] i40e: dump VF information in debugfs
On Thu, Apr 20, 2017 at 7:58 PM, David Miller wrote: > From: "Mintz, Yuval" > Date: Thu, 20 Apr 2017 16:24:51 + > >> the HW pipeline itself can't be abstracted in this case. > > I've heard that argument before, and I'm glad Jiri didn't drink the > koolaide and instead wrote dpipe. donno re drinking but re writing s/jiri/arkadi/
RE: [net-next 04/14] i40e: dump VF information in debugfs
> -Original Message- > From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] On > Behalf Of David Miller > Sent: Thursday, April 20, 2017 8:20 AM > To: yuval.mi...@cavium.com > Cc: gerlitz...@gmail.com; Kirsher, Jeffrey T ; > Williams, Mitch A ; netdev@vger.kernel.org; > nhor...@redhat.com; sassm...@redhat.com; jogre...@redhat.com > Subject: Re: [net-next 04/14] i40e: dump VF information in debugfs > > From: "Mintz, Yuval" > Date: Thu, 20 Apr 2017 10:09:28 + > > >> > Dump some internal state about VFs through debugfs. This provides > >> > information not available with 'ip link show'. > >> > >> such as? > >> > >> donnwantobethedebugfspolice, but still, in the 2-3 times we tried to push > >> debugfs to MLNX NIC drivers, Dave disallowed that, and lately the switch > >> team even went further and deleted that portion of the mlxsw driver -- > all to > >> all, I don't see much point for these type of changes, thoughts? > > > > Don't want to hikjack your thread, but continuing this topic - > > Is there some flat-out disapproval for debugfs in net-next now? > > > > We're currently internally engaged with adding qed support for register > dumps > > [~equivalents for `ethtool -d' outputs] through debugfs, on behalf of > storage > > drivers [qedi/qedf] lacking the API for doing that. > > I really hate to see debugfs things in networking drivers. It's a > complete cop out for doing things properly. > > I push back, but I can only fight too much. If people want to keep > adding stupid poorly designed crap endlessly to their drivers there > is only so much I can do... Honestly, Dave, I try very hard not to put random crap into debugfs. I felt that there was enough utility here to send this one up. We already have debugfs hooks upstream that allow us to dump information about a specific VSI. This just adds a tiny bit to that to allow us to link a VF with its specific VSI. It's the missing link, if you will. I'm not adding backdoor hooks to do naughty things, I'm just (very slightly) enhancing what's already there. This particular piece has been very useful for my debugging, and we have customers that have asked for it as well. If you still want to reject this, I won't fight and cry and stomp my little feet. But I honestly think that this a) has genuine utility, b) is fairly device specific, and c) isn't crap. -Mitch
Re: [net-next 04/14] i40e: dump VF information in debugfs
From: "Mintz, Yuval" Date: Thu, 20 Apr 2017 20:50:55 + > Nope, I don't mind doing the work. What I would appreciate is some > constructive suggestions on a seemingly feasible approach that might work. > I feel like I've been bashed enough for one day. :-) Well, it seems like the thing that you want to do is pull items out of an object. Start with that as an abstraction and see where it goes.
RE: [net-next 04/14] i40e: dump VF information in debugfs
> > I surely wouldn't want to write a million lines of code just to > > provide such a detailed abstraction. > > So now the argument has gone from "it's not possible" to "it's going to be a > HUGE amount of code." Nothing here's is strictly impossible - you can always create a simulated HW model in SW, and have a one-to-one mapping with the HW entities. But "unreasonable amount of code" comes very close to it. > I wonder what the next argument will be when I shoot this one down. > > If you simply don't want to do the hard work necessary to implement this > properly, which is fine, just say so. Nope, I don't mind doing the work. What I would appreciate is some constructive suggestions on a seemingly feasible approach that might work. I feel like I've been bashed enough for one day. :-)
Re: [net-next 04/14] i40e: dump VF information in debugfs
From: "Mintz, Yuval" Date: Thu, 20 Apr 2017 20:12:59 + > I surely wouldn't want to write a million lines of code just to > provide such a detailed abstraction. So now the argument has gone from "it's not possible" to "it's going to be a HUGE amount of code." I wonder what the next argument will be when I shoot this one down. If you simply don't want to do the hard work necessary to implement this properly, which is fine, just say so.
RE: [net-next 04/14] i40e: dump VF information in debugfs
> > the HW pipeline itself can't be abstracted in this case. > > I've heard that argument before, and I'm glad Jiri didn't drink the koolaide > and instead wrote dpipe. Perhaps "can't" was the wrong term to use, but I still believe there's an inherent problem with applying the dpipe approach here. dpipe abstraction is meant [And I admit that I'm not that familiar with its fine-grain details, so feel free to correct me] to give an abstract representation of functional things, I.e., bits in the HW pipeline the user might care about and can gain something from sharing information about it - decision-based statistics [such as in Jiri's initial submission] being an immediate candidate for benefiting from such. For the case of debugging you *could* do the same, but at a huge implementation and code-bloat costs - Let's assume user doesn't use 'magic values' for debug gathering, Instead having some HW abstraction it can manipulate intelligibly. What it means is that level of configurability is limited by the complexity of the abstraction, and you have to recall the vast majority of the abstraction would be in-depth about HW-specific bridges and signals the user cares nothing about [with the exception of debug-data collection]. I surely wouldn't want to write a million lines of code just to provide such a detailed abstraction. .
Re: [net-next 04/14] i40e: dump VF information in debugfs
From: "Mintz, Yuval" Date: Thu, 20 Apr 2017 16:24:51 + > the HW pipeline itself can't be abstracted in this case. I've heard that argument before, and I'm glad Jiri didn't drink the koolaide and instead wrote dpipe.
RE: [net-next 04/14] i40e: dump VF information in debugfs
> > I agree this surely isn't *our* convention, but for scsi using > > debugfs [or sysfs] is a common practice for debug purposes. > > > > Also, our HW debug capabilities are highly-customable, and I want to > > have the ability to configure those on the fly [E.g., dynamically > > configuring various HW events to be recorded]. > > Each such configuration involves multiple register writes and reads > > according to user provided inputs. > > I don't really see how to generalize the information collection in a > > way that would benefit anyone else. > > That's basically what everyone says who slaps random crap into debugfs. True; But assuming that means the suggestion to use debugfs means the content is random crap is a logical fallacy. > >> For your inhouse debugging, you should have oot patch to expose > >> whatever you need. > > > > I don't want in-house debugging capabilities - I want field debug > > capabilities. > > Then it has to use a portable, well defined, set of interfaces. How would you be able to describe something that is completely tied to a specific HW this way. E.g., let's assume you'd want to record some messages that pass between 2 internal processors that match some criteria? While you can probably standardize some format from such a matching [although that too would be very tied to different HW capabilities], the HW pipeline itself can't be abstracted in this case.
Re: [net-next 04/14] i40e: dump VF information in debugfs
From: "Mintz, Yuval" Date: Thu, 20 Apr 2017 13:57:08 + > I agree this surely isn't *our* convention, but for scsi using debugfs > [or sysfs] is a common practice for debug purposes. > > Also, our HW debug capabilities are highly-customable, and I want to > have the ability to configure those on the fly [E.g., dynamically > configuring various HW events to be recorded]. > Each such configuration involves multiple register writes and reads > according to user provided inputs. > I don't really see how to generalize the information collection in a way > that would benefit anyone else. That's basically what everyone says who slaps random crap into debugfs. >> For your inhouse debugging, you should have oot >> patch to expose whatever you need. > > I don't want in-house debugging capabilities - > I want field debug capabilities. Then it has to use a portable, well defined, set of interfaces.
Re: [net-next 04/14] i40e: dump VF information in debugfs
From: "Mintz, Yuval" Date: Thu, 20 Apr 2017 10:09:28 + >> > Dump some internal state about VFs through debugfs. This provides >> > information not available with 'ip link show'. >> >> such as? >> >> donnwantobethedebugfspolice, but still, in the 2-3 times we tried to push >> debugfs to MLNX NIC drivers, Dave disallowed that, and lately the switch >> team even went further and deleted that portion of the mlxsw driver -- all to >> all, I don't see much point for these type of changes, thoughts? > > Don't want to hikjack your thread, but continuing this topic - > Is there some flat-out disapproval for debugfs in net-next now? > > We're currently internally engaged with adding qed support for register dumps > [~equivalents for `ethtool -d' outputs] through debugfs, on behalf of storage > drivers [qedi/qedf] lacking the API for doing that. I really hate to see debugfs things in networking drivers. It's a complete cop out for doing things properly. I push back, but I can only fight too much. If people want to keep adding stupid poorly designed crap endlessly to their drivers there is only so much I can do...
RE: [net-next 04/14] i40e: dump VF information in debugfs
> >> > Dump some internal state about VFs through debugfs. This provides > >> > information not available with 'ip link show'. > >> > >> such as? > >> > >> donnwantobethedebugfspolice, but still, in the 2-3 times we tried to > >> push debugfs to MLNX NIC drivers, Dave disallowed that, and lately > >> the switch team even went further and deleted that portion of the > >> mlxsw driver -- all to all, I don't see much point for these type of > >> changes, > thoughts? > > > >Don't want to hikjack your thread, but continuing this topic - Is there > >some flat-out disapproval for debugfs in net-next now? > > It was mentioned by DaveM on the list multiple times. > > > >We're currently internally engaged with adding qed support for register > >dumps [~equivalents for `ethtool -d' outputs] through debugfs, on > >behalf of storage drivers [qedi/qedf] lacking the API for doing that. > > That sounds wrong. Either introduce a generic infra to expose the info you > need or you don't do that. I agree this surely isn't *our* convention, but for scsi using debugfs [or sysfs] is a common practice for debug purposes. Also, our HW debug capabilities are highly-customable, and I want to have the ability to configure those on the fly [E.g., dynamically configuring various HW events to be recorded]. Each such configuration involves multiple register writes and reads according to user provided inputs. I don't really see how to generalize the information collection in a way that would benefit anyone else. > For your inhouse debugging, you should have oot > patch to expose whatever you need. I don't want in-house debugging capabilities - I want field debug capabilities.
Re: [net-next 04/14] i40e: dump VF information in debugfs
Thu, Apr 20, 2017 at 12:09:28PM CEST, yuval.mi...@cavium.com wrote: >> > Dump some internal state about VFs through debugfs. This provides >> > information not available with 'ip link show'. >> >> such as? >> >> donnwantobethedebugfspolice, but still, in the 2-3 times we tried to push >> debugfs to MLNX NIC drivers, Dave disallowed that, and lately the switch >> team even went further and deleted that portion of the mlxsw driver -- all to >> all, I don't see much point for these type of changes, thoughts? > >Don't want to hikjack your thread, but continuing this topic - >Is there some flat-out disapproval for debugfs in net-next now? It was mentioned by DaveM on the list multiple times. > >We're currently internally engaged with adding qed support for register dumps >[~equivalents for `ethtool -d' outputs] through debugfs, on behalf of storage >drivers [qedi/qedf] lacking the API for doing that. That sounds wrong. Either introduce a generic infra to expose the info you need or you don't do that. For your inhouse debugging, you should have oot patch to expose whatever you need.
RE: [net-next 04/14] i40e: dump VF information in debugfs
> > Dump some internal state about VFs through debugfs. This provides > > information not available with 'ip link show'. > > such as? > > donnwantobethedebugfspolice, but still, in the 2-3 times we tried to push > debugfs to MLNX NIC drivers, Dave disallowed that, and lately the switch > team even went further and deleted that portion of the mlxsw driver -- all to > all, I don't see much point for these type of changes, thoughts? Don't want to hikjack your thread, but continuing this topic - Is there some flat-out disapproval for debugfs in net-next now? We're currently internally engaged with adding qed support for register dumps [~equivalents for `ethtool -d' outputs] through debugfs, on behalf of storage drivers [qedi/qedf] lacking the API for doing that.
Re: [net-next 04/14] i40e: dump VF information in debugfs
On Thu, Apr 20, 2017 at 4:57 AM, Jeff Kirsher wrote: > From: Mitch Williams > > Dump some internal state about VFs through debugfs. This provides > information not available with 'ip link show'. such as? donnwantobethedebugfspolice, but still, in the 2-3 times we tried to push debugfs to MLNX NIC drivers, Dave disallowed that, and lately the switch team even went further and deleted that portion of the mlxsw driver -- all to all, I don't see much point for these type of changes, thoughts?