On 1/9/23 10:31, Maxime Coquelin wrote:
>
>
> On 1/6/23 16:58, David Marchand wrote:
>> The DPDK vhost-user library maintains more granular per queue stats
>> which can replace what OVS was providing for vhost-user ports.
>>
>> The benefits for OVS:
>> - OVS can skip parsing packet sizes on the rx side,
>> - dev->stats_lock won't be taken in rx/tx code unless some packet is
>> dropped,
>> - vhost-user is aware of which packets are transmitted to the guest,
>> so per *transmitted* packet size stats can be reported,
>> - more internal stats from vhost-user may be exposed, without OVS
>> needing to understand them,
>>
>> Note: the vhost-user library does not provide global stats for a port.
>> The proposed implementation is to have the global stats (exposed via
>> netdev_get_stats()) computed by querying and aggregating all per queue
>> stats.
>> Since per queue stats are exposed via another netdev ops
>> (netdev_get_custom_stats()), this may lead to some race and small
>> discrepancies.
>> This issue might already affect other netdev classes.
>>
>> Example:
>> $ ovs-vsctl get interface vhost4 statistics |
>> sed -e 's#[{}]##g' -e 's#, #\n#g' |
>> grep -v =0$
>> rx_1_to_64_packets=12
>> rx_256_to_511_packets=15
>> rx_65_to_127_packets=21
>> rx_broadcast_packets=15
>> rx_bytes=7497
>> rx_multicast_packets=33
>> rx_packets=48
>> rx_q0_good_bytes=242
>> rx_q0_good_packets=3
>> rx_q0_guest_notifications=3
>> rx_q0_multicast_packets=3
>> rx_q0_size_65_127_packets=2
>> rx_q0_undersize_packets=1
>> rx_q1_broadcast_packets=15
>> rx_q1_good_bytes=7255
>> rx_q1_good_packets=45
>> rx_q1_guest_notifications=45
>> rx_q1_multicast_packets=30
>> rx_q1_size_256_511_packets=15
>> rx_q1_size_65_127_packets=19
>> rx_q1_undersize_packets=11
>> tx_1_to_64_packets=36
>> tx_256_to_511_packets=45
>> tx_65_to_127_packets=63
>> tx_broadcast_packets=45
>> tx_bytes=22491
>> tx_multicast_packets=99
>> tx_packets=144
>> tx_q0_broadcast_packets=30
>> tx_q0_good_bytes=14994
>> tx_q0_good_packets=96
>> tx_q0_guest_notifications=96
>> tx_q0_multicast_packets=66
>> tx_q0_size_256_511_packets=30
>> tx_q0_size_65_127_packets=42
>> tx_q0_undersize_packets=24
>> tx_q1_broadcast_packets=15
>> tx_q1_good_bytes=7497
>> tx_q1_good_packets=48
>> tx_q1_guest_notifications=48
>> tx_q1_multicast_packets=33
>> tx_q1_size_256_511_packets=15
>> tx_q1_size_65_127_packets=21
>> tx_q1_undersize_packets=12
>>
>> Reviewed-by: Maxime Coquelin <maxime.coque...@redhat.com>
>> Signed-off-by: David Marchand <david.march...@redhat.com>
>> ---
>> Changes since v6:
>> - fixed tx_retries,
>> - dropped netdev_dpdk_vhost_update_[rt]x_counters helpers. This makes
>> it clearer that a lock can be taken in the vhost rx/tx code and this
>> aligns with the dpdk rx/tx code too,
>>
>> Changes since v5:
>> - added missing dev->stats_lock acquire in netdev_dpdk_vhost_get_stats,
>> - changed netdev_dpdk_vhost_update_[rt]x_counters to take dev->stats_lock
>> only when some packets got dropped in OVS. Since the rx side won't
>> take the lock unless some QoS configuration is in place, this change
>> will likely have the same effect as separating stats_lock into rx/tx
>> dedicated locks. Testing shows a slight (around 1%) improvement of
>> performance for some V2V setup,
>>
>> Changes since v3:
>> - rebased to master now that v22.11 landed,
>> - fixed error code in stats helper when vhost port is not "running",
>> - shortened rx/tx stats macro names,
>>
>> Changes since RFC v2:
>> - dropped the experimental api check (now that the feature is marked
>> stable in DPDK),
>> - moved netdev_dpdk_get_carrier() forward declaration next to the
>> function needing it,
>> - used per q stats for netdev_get_stats() and removed OVS per packet
>> size accounting logic,
>> - fixed small packets counter (see rx_undersized_errors hack),
>> - added more Tx stats,
>> - added unit tests,
>>
>> ---
>> lib/netdev-dpdk.c | 447 ++++++++++++++++++++++++++++++-------------
>> tests/system-dpdk.at | 33 +++-
>> 2 files changed, 348 insertions(+), 132 deletions(-)
>>
>
> My R-by was already set from previous revision, but just to confirm this
> new version is good to me:
>
> Reviewed-by: Maxime Coquelin <maxime.coque...@redhat.com>
Thanks, Maxime and David! Applied.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
