On 28 Aug 2024, at 2:37, Ilya Maximets wrote:
> On 8/20/24 15:55, Mike Pattrick wrote:
>> Clang analyzer will complain about floating point operations conducted
>> with integer types as rounding is undefined. In pmd_info_show_rxq() a
>> percentage was calculated inside uint64 integers instead of a floating
>> pointer variable for a user visible message. There isn't a good reason
>> not to use floating point types here.
>
> I don't see a good reason to use floating point arithmetic here. We're using
> integer division here and we are not interested in fractional parts. So, I'm
> a little confused on why this is a problem.
>
> Is it because the '100' has an ambiguous type? Will it still complain if we
> use '100ULL' or UINT64_C(100) ?
I quickly tried this but does not work. In other parts of the code they do
stuff like this:
ds_put_format(reply,
" avg cycles per packet: %.02f (%"PRIu64"/%"PRIu64")\n",
total_cycles / (double) total_packets,
total_cycles, total_packets);
So we could probably do a similar fix here.
Thoughts?
//Eelco
> Best regards, Ilya Maximets.
>
>>
>> Signed-off-by: Mike Pattrick <m...@redhat.com>
>> ---
>> lib/dpif-netdev.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index f0594e5f5..69d5ddfa1 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -900,8 +900,8 @@ pmd_info_show_rxq(struct ds *reply, struct
>> dp_netdev_pmd_thread *pmd,
>> if (pmd->core_id != NON_PMD_CORE_ID) {
>> struct rxq_poll *list;
>> size_t n_rxq;
>> - uint64_t total_pmd_cycles = 0;
>> - uint64_t busy_pmd_cycles = 0;
>> + double total_pmd_cycles = 0;
>> + double busy_pmd_cycles = 0;
>> uint64_t total_rxq_proc_cycles = 0;
>> unsigned int intervals;
>>
>> @@ -930,7 +930,7 @@ pmd_info_show_rxq(struct ds *reply, struct
>> dp_netdev_pmd_thread *pmd,
>> for (int i = 0; i < n_rxq; i++) {
>> struct dp_netdev_rxq *rxq = list[i].rxq;
>> const char *name = netdev_rxq_get_name(rxq->rx);
>> - uint64_t rxq_proc_cycles = 0;
>> + double rxq_proc_cycles = 0;
>>
>> rxq_proc_cycles = get_interval_values(rxq->cycles_intrvl,
>> &rxq->intrvl_idx,
>> @@ -942,9 +942,8 @@ pmd_info_show_rxq(struct ds *reply, struct
>> dp_netdev_pmd_thread *pmd,
>> ? "(enabled) " : "(disabled)");
>> ds_put_format(reply, " pmd usage: ");
>> if (total_pmd_cycles) {
>> - ds_put_format(reply, "%2"PRIu64"",
>> - rxq_proc_cycles * 100 / total_pmd_cycles);
>> - ds_put_cstr(reply, " %");
>> + ds_put_format(reply, "%2.0f %%",
>> + (rxq_proc_cycles * 100) / total_pmd_cycles);
>> } else {
>> ds_put_format(reply, "%s", "NOT AVAIL");
>> }
>> @@ -954,13 +953,14 @@ pmd_info_show_rxq(struct ds *reply, struct
>> dp_netdev_pmd_thread *pmd,
>> if (n_rxq > 0) {
>> ds_put_cstr(reply, " overhead: ");
>> if (total_pmd_cycles) {
>> - uint64_t overhead_cycles = 0;
>> + double overhead_cycles = 0;
>>
>> if (total_rxq_proc_cycles < busy_pmd_cycles) {
>> overhead_cycles = busy_pmd_cycles -
>> total_rxq_proc_cycles;
>> }
>> - ds_put_format(reply, "%2"PRIu64" %%",
>> - overhead_cycles * 100 / total_pmd_cycles);
>> +
>> + ds_put_format(reply, "%2.0f %%",
>> + (overhead_cycles * 100) / total_pmd_cycles);
>> } else {
>> ds_put_cstr(reply, "NOT AVAIL");
>> }
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
