[dpdk-dev] [PATCH 1/6] net/vmxnet3: retain counters on restart

[Shrikrishna Khare]

On Thu, 25 May 2017, Nachi Prachanda wrote:

> > From: Shrikrishna Khare [mailto:skhare at 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:skhare at 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 <nprachan at 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 <nprachan at 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
>