Hi Gowrishankar, Thanks for your comments & suggestions
Replies inline .. Regards & Thanks Anju -----Original Message----- From: Gowrishankar Muthukrishnan [mailto:[email protected]] Sent: Thursday, September 27, 2018 4:28 PM To: Keshav Gupta <[email protected]> Cc: [email protected]; Anju Thomas <[email protected]> Subject: Re: [ovs-dev] [PATCH v5] Improved Packet Drop Statistics in OVS Hi Keshav, Thank you for the patches. They certainly help us debugging packet drops more meaningfully. Following your changes, some of the places where additional reasons for drop could also be added as I hope helpful. Could you please check below items as well. __netdev_dpdk_vhost_send() AND netdev_dpdk_send__(): drop from netdev_dpdk_filter_packet_len(), when packet length exceeds wrt max based on mtu (DPIF_DROP_TX_INVALID_PACKET_DROP ?) drop from netdev_dpdk_qos_run(), when egress policer drops the packets (DPIF_DROP_TX_QOS_DROP ?) [Anju] These are accounted as part of the Interface and policer drops. The idea of this feature was to account for the earlier unaccounted drops so We haven’t actually looked into dividing these into further categories. But I think this is something we can consider to do as an enhancement bundle_send_learning_packets(): Seems like it is used when bond needs mac learning. However, there is no differentiation wrt error/drop when xlate_send_packet() would fail down the line. Also any plan to cover drops in dummy dpif for testing aspects ?. Eg. eth_from_flow() could return NULL packet. [Anju] We had planned to account for drops in the dpdk datapath. Hence we have not considered drops in bundle_run and others which happens in the main thread. For similar reasone we have not planning for dummy dpif for now. Again I feel this is a very good idea for our future enhancements. I have one comment which is inline below. On 09/15/2018 01:33 AM, Keshav Gupta 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. Also OVS trouble-shooters face difficulties in > analysing packet drops. > > With this patch we implement following changes to address the issues > mentioned above. <cut> > +static void > dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, > const struct nlattr *a, bool should_steal) > OVS_NO_THREAD_SAFETY_ANALYSIS > @@ -6449,6 +6520,8 @@ dp_execute_cb(void *aux_, struct dp_packet_batch > *packets_, > struct dp_netdev *dp = pmd->dp; > int type = nl_attr_type(a); > struct tx_port *p; > + uint32_t packet_count, packet_dropped; > + struct dpif_drop_counter cntr; > > switch ((enum ovs_action_attr)type) { > case OVS_ACTION_ATTR_OUTPUT: > @@ -6490,6 +6563,11 @@ dp_execute_cb(void *aux_, struct dp_packet_batch > *packets_, > dp_packet_batch_add(&p->output_pkts, packet); dp_packet_batch_add__ would drop packet if dp_packet_batch has reached NETDEV_MAX_BURST. Could we add another counter (and map its reason) for it. DPIF_RX_PKT_BATCH_FULL ? [Anju]. Thanks for this . I will include this as well Thanks, Gowrishankar > } > return; > + } else { > + cntr.type = DPIF_DROP_TYPE_DP; > + cntr.counter.dp = DPIF_DP_INVALID_PORT_DROP; > + pmd_perf_update_drop_counter(&pmd->perf_stats, &cntr, > + packets_->count); > } > break; > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
