On 18.12.2019 05:48, Anju Thomas wrote:
> Currently OVS maintains explicit packet drop/error counters only on port
> level.  Packets that are dropped as part of normal OpenFlow processing
> are counted in flow stats of “drop” flows or as table misses in table
> stats. These can only be interpreted by controllers that know the
> semantics of the configured OpenFlow pipeline.  Without that knowledge,
> it is impossible for an OVS user to obtain e.g. the total number of
> packets dropped due to OpenFlow rules.
> 
> Furthermore, there are numerous other reasons for which packets can be
> dropped by OVS slow path that are not related to the OpenFlow pipeline.
> The generated datapath flow entries include a drop action to avoid
> further expensive upcalls to the slow path, but subsequent packets
> dropped by the datapath are not accounted anywhere.
> 
> Finally, the datapath itself drops packets in certain error situations.
> Also, these drops are today not accounted for.This makes it difficult
> for OVS users to monitor packet drop in an OVS instance and to alert a
> management system in case of a unexpected increase of such drops.
> AlsoOVS trouble-shooters face difficulties in analysing packet drops.
> 
> With this patch we implement following changes to address the issues
> mentioned above.
> 
> 1. Identify and account all the silent packet drop scenarios
> 
> 2. Display these drops in ovs-appctl coverage/show
> 
> Co-authored-by: Rohith Basavaraja <[email protected]>
> Co-authored-by: Keshav Gupta <[email protected]>
> Signed-off-by: Anju Thomas <[email protected]>
> Signed-off-by: Rohith Basavaraja <[email protected]>
> Signed-off-by: Keshav Gupta <[email protected]>
> Acked-by: Eelco Chaudron <[email protected]
> ---

Thanks.  This version looks OK to me beside the fact that I'd rename
the patch to something like "userspace: Improved packet drop statistics.",
but this is minor.  One more thing is that we might want to rename
'enum xlate_error' to 'enum ovs_xlate_error' and prefix all its members
with 'OVS_' to keep the style along with kernel definitions, but this is
a mechanical change that could be done later in a separate patch as it
will touch a lot of unrelated code.

Acked-by: Ilya Maximets <[email protected]>

Hi Ben,
could you, please, take a look at this patch one more time?
There were a couple of changes as we moved 'enum xlate_error' to
openvswitch.h header to avoid inclusion of 'ofproto/ofproto-dpif-xlate.h'
from the 'lib' code, new datapath capability was documented and we
cleaned the patch up a little bit.

BTW, I will be traveling starting from tomorrow until the end of next
week, so feel free to apply this patch if it looks good to you.  I could
apply it myself today (if you could reply on it today) or after my trip.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to