On 7/12/24 19:06, Adrian Moreno wrote:
> Add a cache entry type for local sample objects.
> Store both the dpif_lsample reference and the collector_set_id so we can
> quickly find the particular exporter.
> 
> Using this mechanism, account for packet and byte statistics.
> 
> Acked-by: Eelco Chaudron <[email protected]>
> Signed-off-by: Adrian Moreno <[email protected]>
> ---
>  ofproto/ofproto-dpif-lsample.c     | 18 ++++++++++++++++++
>  ofproto/ofproto-dpif-lsample.h     |  4 ++++
>  ofproto/ofproto-dpif-xlate-cache.c | 11 ++++++++++-
>  ofproto/ofproto-dpif-xlate-cache.h |  6 ++++++
>  ofproto/ofproto-dpif-xlate.c       | 13 +++++++++++++
>  ofproto/ofproto-dpif.c             |  1 +
>  6 files changed, 52 insertions(+), 1 deletion(-)

Hi, Adrian.  The code is fine here, but I don't think the change is logically
correct.  Flow translation cache will accumulate flow statistics and it will
not be equal to the collector statistics unless the sampling rate is 100%.
If it's not 100% packet will go through the flow, stats will be updated, but
they will not be smapled and so should not be reported as lsample statistics.

I actually do not know how we could capture the collector's stats in this case.
In case of sFlow or IPFIX the packet are going through userspace and we can
count them when they arrive to userspace and to the corresponding module.
In case of psample, packets stay in the kenrel and unless we listen on psample
we don't have a way to get the actual number of sampled packets and bytes.

Should we just drop patches 7 and 8 and remove the lsample/show calls from
the tests?

Best regards, Ilya Maximets.

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to