Hello, >Do you know how much it is going to impact the vhost throughput? Other than >this the change looks good to me.
I compared the results based on (OVS head) vs (OVS head + patch) and seems there is very small performance drop: 4,763,016 (HEAD) vs 4,699,571(Patch). -----Original Message----- From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di Proietto Sent: Monday, July 6, 2015 6:59 PM To: Puha, TimoX Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH] netdev-dpdk: Add some missing statistics. On 01/07/2015 11:49, "Timo Puha" <timox.p...@intel.com> wrote: >New stats for vhost ports are rx_bytes, tx_bytes, multicast, rx_errors >and rx_length_errors. New stats for PMD ports are rx_dropped, >rx_length_errors, rx_crc_errors and rx_missed_errors. DPDK imissed >packets are now classified as dropped instead of errors. > >Signed-off-by: Timo Puha <timox.p...@intel.com> Thanks for the patch. I tried it and I can see the packets properly counted as "dropped" instead of "errors". Do you know how much it is going to impact the vhost throughput? Other than this the change looks good to me. >--- > lib/netdev-dpdk.c | 86 >++++++++++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 76 insertions(+), 10 deletions(-) > >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index >8b843db..9fe625d 100644 >--- a/lib/netdev-dpdk.c >+++ b/lib/netdev-dpdk.c >@@ -884,6 +884,35 @@ is_vhost_running(struct virtio_net *dev) > return (dev != NULL && (dev->flags & VIRTIO_DEV_RUNNING)); } > >+static inline void >+netdev_dpdk_vhost_update_rx_counters(struct netdev_stats *stats, >+ struct dp_packet **packets, int >count) >+{ >+ int i; >+ struct dp_packet *packet; >+ >+ stats->rx_packets += count; >+ for (i = 0; i < count; i++) { >+ packet = packets[i]; >+ >+ if (OVS_UNLIKELY(dp_packet_size(packet) < ETH_HEADER_LEN)) { >+ /* This only protects the following multicast counting from >+ * too short packets, but it does not stop the packet from >+ * further processing. */ >+ stats->rx_errors++; >+ stats->rx_length_errors++; >+ continue; >+ } >+ >+ struct eth_header *eh = (struct eth_header *) >dp_packet_data(packet); >+ if (OVS_UNLIKELY(eth_addr_is_multicast(eh->eth_dst))) { >+ stats->multicast++; >+ } >+ >+ stats->rx_bytes += dp_packet_size(packet); >+ } >+} >+ > /* > * The receive path for the vhost port is the TX path out from guest. > */ >@@ -911,7 +940,7 @@ netdev_dpdk_vhost_rxq_recv(struct netdev_rxq *rxq_, > } > > rte_spinlock_lock(&vhost_dev->stats_lock); >- vhost_dev->stats.rx_packets += (uint64_t)nb_rx; >+ netdev_dpdk_vhost_update_rx_counters(&vhost_dev->stats, packets, >nb_rx); > rte_spinlock_unlock(&vhost_dev->stats_lock); > > *c = (int) nb_rx; >@@ -948,6 +977,23 @@ netdev_dpdk_rxq_recv(struct netdev_rxq *rxq_, >struct dp_packet **packets, > return 0; > } > >+static inline void >+netdev_dpdk_vhost_update_tx_counters(struct netdev_stats *stats, >+ struct dp_packet **packets, >+ int attempted, >+ int dropped) { >+ int i; >+ int sent = attempted - dropped; >+ >+ stats->tx_packets += sent; >+ stats->tx_dropped += dropped; >+ >+ for (i = 0; i < sent; i++) { >+ stats->tx_bytes += dp_packet_size(packets[i]); >+ } >+} >+ > static void > __netdev_dpdk_vhost_send(struct netdev *netdev, struct dp_packet **pkts, > int cnt, bool may_steal) @@ -1005,8 +1051,8 >@@ __netdev_dpdk_vhost_send(struct netdev *netdev, struct dp_packet >**pkts, > rte_spinlock_unlock(&vhost_dev->vhost_tx_lock); > > rte_spinlock_lock(&vhost_dev->stats_lock); >- vhost_dev->stats.tx_packets += (total_pkts - cnt); >- vhost_dev->stats.tx_dropped += cnt; >+ netdev_dpdk_vhost_update_tx_counters(&vhost_dev->stats, pkts, >total_pkts, >+ cnt); > rte_spinlock_unlock(&vhost_dev->stats_lock); > > out: >@@ -1309,14 +1355,10 @@ netdev_dpdk_vhost_get_stats(const struct netdev >*netdev, > ovs_mutex_lock(&dev->mutex); > memset(stats, 0, sizeof(*stats)); > /* Unsupported Stats */ >- stats->rx_errors = UINT64_MAX; >- stats->tx_errors = UINT64_MAX; >- stats->multicast = UINT64_MAX; > stats->collisions = UINT64_MAX; > stats->rx_crc_errors = UINT64_MAX; > stats->rx_fifo_errors = UINT64_MAX; > stats->rx_frame_errors = UINT64_MAX; >- stats->rx_length_errors = UINT64_MAX; > stats->rx_missed_errors = UINT64_MAX; > stats->rx_over_errors = UINT64_MAX; > stats->tx_aborted_errors = UINT64_MAX; @@ -1325,16 +1367,20 @@ >netdev_dpdk_vhost_get_stats(const struct netdev *netdev, > stats->tx_fifo_errors = UINT64_MAX; > stats->tx_heartbeat_errors = UINT64_MAX; > stats->tx_window_errors = UINT64_MAX; >- stats->rx_bytes += UINT64_MAX; > stats->rx_dropped += UINT64_MAX; >- stats->tx_bytes += UINT64_MAX; > > rte_spinlock_lock(&dev->stats_lock); > /* Supported Stats */ > stats->rx_packets += dev->stats.rx_packets; > stats->tx_packets += dev->stats.tx_packets; > stats->tx_dropped += dev->stats.tx_dropped; >+ stats->multicast = dev->stats.multicast; >+ stats->rx_bytes = dev->stats.rx_bytes; >+ stats->tx_bytes = dev->stats.tx_bytes; >+ stats->rx_errors = dev->stats.rx_errors; >+ stats->rx_length_errors = dev->stats.rx_length_errors; > rte_spinlock_unlock(&dev->stats_lock); >+ > ovs_mutex_unlock(&dev->mutex); > > return 0; >@@ -1357,13 +1403,33 @@ netdev_dpdk_get_stats(const struct netdev >*netdev, struct netdev_stats *stats) > stats->tx_packets = rte_stats.opackets; > stats->rx_bytes = rte_stats.ibytes; > stats->tx_bytes = rte_stats.obytes; >- stats->rx_errors = rte_stats.ierrors; >+ /* DPDK counts imissed as errors, but count them here as dropped >instead */ >+ stats->rx_errors = rte_stats.ierrors - rte_stats.imissed; > stats->tx_errors = rte_stats.oerrors; > stats->multicast = rte_stats.imcasts; > > rte_spinlock_lock(&dev->stats_lock); > stats->tx_dropped = dev->stats.tx_dropped; > rte_spinlock_unlock(&dev->stats_lock); >+ >+ /* These are the available DPDK counters for packets not received >due to >+ * local resource constraints in DPDK and NIC respectively. */ >+ stats->rx_dropped = rte_stats.rx_nombuf + rte_stats.imissed; >+ stats->collisions = UINT64_MAX; >+ >+ stats->rx_length_errors = rte_stats.ibadlen; >+ stats->rx_over_errors = UINT64_MAX; >+ stats->rx_crc_errors = rte_stats.ibadcrc; >+ stats->rx_frame_errors = UINT64_MAX; >+ stats->rx_fifo_errors = UINT64_MAX; >+ stats->rx_missed_errors = rte_stats.imissed; >+ >+ stats->tx_aborted_errors = UINT64_MAX; >+ stats->tx_carrier_errors = UINT64_MAX; >+ stats->tx_fifo_errors = UINT64_MAX; >+ stats->tx_heartbeat_errors = UINT64_MAX; >+ stats->tx_window_errors = UINT64_MAX; >+ > ovs_mutex_unlock(&dev->mutex); > > return 0; >-- >1.8.3.1 > >_______________________________________________ >dev mailing list >dev@openvswitch.org >https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mai >lma >n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r >=Sm >B5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=YB1NS4G9tC50cTYnobqmD0-Oxw_ >5cs nIinlyjloTzas&s=8B7QnC-P6OT6v7VMlWAi_PFd-A6v2O_V3RYxEo4OQpY&e= _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev -------------------------------------------------------------- Intel Shannon Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 Business address: Dromore House, East Park, Shannon, Co. Clare This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev