Hi, On Thu, Dec 15, 2022 at 5:15 AM Antonio Quartulli <a...@unstable.cc> wrote:
> Hi, > > On 15/12/2022 10:31, Lev Stipakov wrote: > > Hi, > > > >> In non-dco use, the stats as persisted by the management interface are > not reset throughout the lifetime of the process. With dco, what the driver > provides is "Peer Stats" which is reset in OvpnPeerNew() (linux appears to > do the same). A quick option > would be to move the zero-ing to > OvpnEvtFileCleanup() which, I guess, would work at least when persist-tun > is in use. > > > > Yeah, without persist-tun client closes the handle, which triggers > > OvpnEvtFileCleanup(), which would reset stats. > Currently its worse than that -- stats is reset in OvpnNewPeer() which would happen (I guess) even if the tun handle is not closed. My suggestion was to move it to OvpnEvtFileCleanup() which will help those who use persist-tun. But all that is moot based on Antonio's explanation below -- this is Peer Stats, not adapter stats, so it has to reset as it is done now... I was treating it like the stats in a network interface which would be reset only on rare events lke adapter reset or driver update or reboot. > > > >> Or in a new callback that gets called on tun-open. But then it wont be > strictly "Peer Stats". > > > > Yeah that might be an option, to have RESET_STATS ioctl. > > > > Also I wonder if the driver could remember, say, the last pid, and > > then reset stats on NEW_PEER if pid != last pid. Since pids are > > recycled, we might "leak" stats in unfortunate cases, but that's okay > > I guess? > > > > Sorry for jumping in the discussion a bit late. > > IMHO the stats belong to the Peer object. > Let's not forget that, apart from windows, DCO supports multi-peer mode, > therefore we have to consider that case as well. > > Due to the above, I'd "reset" the stats when a new peer object is > instantiated. Basically when a new peer object is instantiated it should > come with empty stats, as expected. > > If the desired behvaiour in userspace is to keep the stats persistent > across a number of events, it should simply be userspace to keep those > stats. > > This way we don't mix up the logic of counting the bytes per peer, and > keeping a general picture of the VPN process. > Good point. I was late to realize that Peer Stats has to remain as it is to make sense in multi-peer scenarios. In that case we'll have to find a way to persist the stats across NEW_PEER controls within the client process. Possibly we can merge the current patch for openvpn.exe and then improve it later in bug-fix releases of 2.6? Selva
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
