On Thu, 25 May 2017, Nachi Prachanda wrote:
> > From: Shrikrishna Khare [mailto:skh...@shri-linux.eng.vmware.com] > > Sent: Thursday, May 25, 2017 1:27 PM > > > > On Thu, 25 May 2017, Nachi Prachanda wrote: > > > > > > From: Shrikrishna Khare [mailto:skh...@shri-linux.eng.vmware.com] > > > > Sent: Wednesday, May 24, 2017 2:10 PM > > > > > > > > On Fri, 19 May 2017, Charles (Chas) Williams wrote: > > > > > > > > > From: Nachiketa Prachanda <nprac...@brocade.com> > > > > > > > > > > Most nics like virtio, igb/ixgbe etc. don't reset counters on > > > > > dev_start and arguably this helps in monitoring the counters > > > > > across a longer time span with multiple device start/stops. > > > > > vmxnet3 behavior is opposite to that and counters are reset by the > > > > > host side implementation each time the device is restarted. > > > > > > > > > > Change the driver to save the counters in its private context > > > > > before it is reset by writing CMD_ACTIVATE to REG_CMD. > > > > > > > > > > Signed-off-by: Nachiketa Prachanda <nprac...@brocade.com> > > > > > > > > This won't be able to deal with vMotion or suspend/resume? > > > > > > Correct - this can't deal with the VM suspend/resume unless hypervisor > > maintains the counter. But this patch doesn't make that behavior any worse > > than what it was before. > > > > The current code always resets stats, but am concerned that this patch will > > make the behavior inconsistent for cases like suspend/resume. > > > > Wondering if this will be better handled by the device emulation instead of > > the > > driver (for igb/ixgbe, is this handled by the hardware?). > > A little more nuanced.. - see below. > > > If we were to handle this in the device emulation, what would be the > > goals/requirements: > > - device start/stop should not reset stats? > > - any other operations where we would like to maintain/reset stats? > > - what might be the expectation around how accurate the stats need to be? > > - any other requirement on the device? > > I haven't thought about dealing it at the emulation layer - but the > expectation > would be not clear counters not cleared for the lifetime of the device - and > have > a way to clear them from the driver when needed. > > don't know if there is a standard behavior about resetting counters. But > for igb/ixgb the counters are read/clear registers and they are maintained > at driver. May not be always accurate if the hardware is reset without > updating > the driver's counter - but at least ensures that it is monotonically > increasing since > on each read driver only gets the delta. > > virtio emulation doesn't provide the counters - mostly the receive/send > functions > updates the counters. > > > > > Also, note that if we proceed with this patch, and later extend device > > support > > to not reset stats, driver with this patch running on the extended device > > will > > report incorrect stats. > > Agree - it will need some work if the emulation changes. I spent some time looking at the emulation code. It turns out that these stats are already retained across vMotion, suspend/resume. And we have no immediate plan to change the device behavior to retain these stats across device start/stop. So am fine with this patch. Thank you for being patient with this. Thanks, Shri > > Regards, > Nachi > > > Thanks, > > Shri >