On 7/9/19 11:48 PM, Jiri Pirko wrote:
Tue, Jul 09, 2019 at 09:13:53PM CEST, [email protected] wrote:
On 7/8/19 11:56 PM, Jiri Pirko wrote:
Tue, Jul 09, 2019 at 12:58:00AM CEST, [email protected] wrote:
On 7/8/19 1:03 PM, Jiri Pirko wrote:
Mon, Jul 08, 2019 at 09:58:09PM CEST, [email protected] wrote:
If I'm not mistaken, the alloc is only allocating enough for a pointer, not
the whole per device struct, and a few lines down from here the pointer to
the new devlink struct is assigned to ionic->dl. This was based on what I
found in the qed driver's qed_devlink_register(), and it all seems to work.
I'm not saying your code won't work. What I say is that you should have
a struct for device that would be allocated by devlink_alloc()
Is there a particular reason why? I appreciate that devlink_alloc() can give
you this device specific space, just as alloc_etherdev_mq() can, but is there
Yes. Devlink manipulates with the whole device. However,
alloc_etherdev_mq() allocates only net_device. These are 2 different
things. devlink port relates 1:1 to net_device. However, devlink
instance can have multiple ports. What I say is do it correctly.
So what you are saying is that anyone who wants to add even the smallest
devlink feature to their driver needs to rework their basic device memory
setup to do it the devlink way. I can see where some folks may have a
problem with this.
It's just about having a structure to hold device data. You don't have
to rework anything, just add this small one.
Well, there's a bit of logic rework to and a little data twiddling - not
too bad in our case. Others may not be thrilled depending on how
they've already implemented their drivers.
The ionic struct should be associated with devlink_port. That you are
missing too.
We don't support any of devlink_port features at this point, just the simple
device information.
No problem, you can still register devlink_port. You don't have to do
much in order to do so.
Is there any write-up to help guide developers new to devlink in using the
interface correctly? I haven't found much yet, but perhaps I've missed
something. The manpages are somewhat useful in showing what the user might
do, but they really don't help much in guiding the developer through these
details.
That is not job of a manpage. See the rest of the code to get inspired.
Sure, we should all be able to poke through the code and figure out the
basics - "use the Force, read the source" - but as software engineers we
should be including some bits of documentation to help those new to the
feature to steer away from pitfalls and use the feature correctly.
We're all busy with our own projects and only have limited time to dig
into and understand someone else's code; if there's not a guide, we'll
do what we can to get it working and then move on, with no guarantee
that we followed the original intent.
There's a Documentation page on the devlink-health feature, and a brief
bit on devlink-params, but I haven't seen anything yet that spells out
the "proper" way to use the devlink framework. Of course, the
open-source spirit is for me to scratch my own itch and take care of the
need myself: I'd be happy to get a brief doc started, but if the
original developers can take a few minutes to at least sketch some notes
down about important bits like "the device struct should be associated
with devlink_port" and why it should, then we have a chance at saving a
lot of other people's time, and perhaps we can fill out the details
correctly and not miss something important.
sln
