Hi Michal,
Thanks - one question below hopefully someone can help with.
On 17-12-05 02:29 PM, Michal Kubecek wrote:
On Tue, Dec 05, 2017 at 02:06:09PM -0800, Scott Branden wrote:
On 17-12-05 01:30 PM, Michal Kubecek wrote:
On Tue, Dec 05, 2017 at 12:53:23PM -0800, Scott Branden wrote:
Add ETHTOOL_RESET support via --reset command.
ie. ethtool --reset DEVNAME <flagname(s)>
flagnames currently match the ETH_RESET_xxx names:
mgmt,irq,dma,filter,offload,mac,phy,ram,dedicated,all
Alternatively, you can specific component bitfield directly using
ethtool --reset DEVNAME flags %x
IMHO it would be more consistent with e.g. msglvl without the keyword
"flags".
I don't see the consistency in ethtool of specifying a number without a
keyword in front of it.
I can only find --set-dump specify a number?
Others have keyword and number. msglvl is the keyword after specifying -s -
same as flags is the keyword I use after specifying --reset.
What I meant is that you can write
ethtool -s eth0 msglvl drv on probe off
ethtool -s eth0 msglvl 0x7
i.e. either number or names (with on/off in this case) while your patch
has
ethtool --reset eth0 mgmg,irq
ethtool --reset eth0 flags 0x3
i.e. an extra keyword if a number is used.
But it's not really important, it doesn't seem I would be able to share
a parser for this with any other subcommand or parameter anyway.
It would be also nice to provide a symbolic way to specify the
shared flags.
I'll change to allow -shared to be added to the end of each component
specified to use the shared bit.
IE. mgmt-shared, irq-shared, dma-shared ?
Sounds good to me.
+ resetinfo.cmd = ETHTOOL_RESET;
+
+ if (send_ioctl(ctx, &resetinfo)) {
+ perror("Cannot issue RESET");
+ return 1;
+ }
+ fprintf(stdout, "RESET 0x%x issued\n", resetinfo.data);
According to documentation, driver is supposed to clear the flags
corresponding to components which were reset so that what is left are
those which were _not_ reset.
I'll move the print above the send_ioctl.
It might be even more useful if ethtool informed user what actually
happened, i.e. either change the message to saying these are bits for
components not reset (if resetinfo.data is not zero) or save the
original value of resetinfo.data and show saved_data & ~resetinfo.data
In making the improvement I found a bug in the bnxt_en kernel driver.
The bnxt_en driver currently doesn't clear any of the component flags on
success so I need to send in a fix for that.
Although in one case (RESET_ALL) in the driver it doesn't actually
execute the reset until all necessary drivers are unloaded to prevent
the PCIe bus from hanging.
So question: should the flags be cleared if the reset is "pending" but
hasn't actually happened yet, but will reset once all the drivers are
all properly unloaded?
Michal Kubecek
