Re: [PATCH 1/6] net/vmxnet3: retain counters on restart

[Nachi Prachanda <nprachan@Brocade.com>]

> From: Shrikrishna Khare [mailto:skhare@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@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@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@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.

Regards,
Nachi

> Thanks,
> Shri