On 11.11.2019 16:55, Kevin Traynor wrote: > On 10/11/2019 23:20, Ilya Maximets wrote: >> On 29.10.2019 15:50, Sriram Vatala wrote:> @@ -2388,12 +2412,16 @@ >> __netdev_dpdk_vhost_send(struct netdev *netdev, int qid, >>> } >>> } while (cnt && (retries++ < max_retries)); >>> >>> + tx_failure = cnt; >>> rte_spinlock_unlock(&dev->tx_q[qid].tx_lock); >>> >>> rte_spinlock_lock(&dev->stats_lock); >>> netdev_dpdk_vhost_update_tx_counters(&dev->stats, pkts, total_pkts, >>> cnt + dropped); >>> - dev->tx_retries += MIN(retries, max_retries); >>> + sw_stats->tx_retries += MIN(retries, max_retries); >>> + sw_stats->tx_failure_drops += tx_failure; >>> + sw_stats->tx_mtu_exceeded_drops += mtu_drops; >>> + sw_stats->tx_qos_drops += qos_drops; >> >> Kevin pointed to this part of code in hope that we can move this to >> a separate function and reuse in his review for v9. This code catches >> my eyes too. I don't think that we can reuse this part, at least this >> will be not very efficient in current situation (clearing of the unused >> fields in a stats structure will be probably needed before calling such >> a common function, but ETH tx uses only half of the struct). >> >> But there is another thing here. We already have special functions >> for vhost tx/rx counters. And it looks strange when we're not using >> these functions to update tx/rx failure counters. >> >> Suggesting following incremental. >> Kevin, Sriram, please, share your thoughts. >> > > The incremental patch looks good, thanks. One additional thing is that > OVS_REQUIRES(dev->stats_lock) annotations can be used for the vhost > rx/tx update counter functions now (even if it seems unlikely someone > would miss doing that). > > @Sriram, see below or you can check similar OVS_REQUIRES usage elsewhere > in the file.
I'm not sure if clang annotations will work with rte_spinlock. DPDK doesn't have proper annotations for locking functions. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
