On 11/9/2023 2:08 PM, shaib...@amazon.com wrote:
> From: Shai Brandes <shaib...@amazon.com>
>
> Changed the rte_memcpy call to use the precomputed buf_size.
> Rearranged the ena adapter structure and removed redundant
> '&' operators as a precaution.
>
What is the reason of the structure rearrange?
> Coverity issue: 405363
> Coverity issue: 405357
> Coverity issue: 405359
>
Can you please split the patch per each fix if they are not logically
related or caused from same code.
> Fixes: 92401abfbcb9 ("net/ena: support connection tracking stats")
> Signed-off-by: Shai Brandes <shaib...@amazon.com>
> ---
> drivers/net/ena/ena_ethdev.c | 21 ++++++++++-----------
> drivers/net/ena/ena_ethdev.h | 4 ++--
> 2 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
> index dc846d2e84..53e7251874 100644
> --- a/drivers/net/ena/ena_ethdev.c
> +++ b/drivers/net/ena/ena_ethdev.c
> @@ -531,8 +531,8 @@ ENA_PROXY_DESC(ena_com_get_eni_stats,
> ENA_MP_ENI_STATS_GET,
> ({
> ENA_TOUCH(rsp);
> ENA_TOUCH(ena_dev);
> - if (stats != (struct ena_admin_eni_stats *)&adapter->metrics_stats)
> - rte_memcpy(stats, &adapter->metrics_stats, sizeof(*stats));
> + if (stats != (struct ena_admin_eni_stats *)adapter->metrics_stats)
> + rte_memcpy(stats, adapter->metrics_stats, sizeof(*stats));
> }),
> struct ena_com_dev *ena_dev, struct ena_admin_eni_stats *stats);
>
> @@ -590,9 +590,8 @@ ENA_PROXY_DESC(ena_com_get_customer_metrics,
> ENA_MP_CUSTOMER_METRICS_GET,
> ({
> ENA_TOUCH(rsp);
> ENA_TOUCH(ena_dev);
> - ENA_TOUCH(buf_size);
> - if (buf != (char *)&adapter->metrics_stats)
> - rte_memcpy(buf, &adapter->metrics_stats, adapter->metrics_num *
> sizeof(uint64_t));
> + if (buf != (char *)adapter->metrics_stats)
> + rte_memcpy(buf, adapter->metrics_stats, buf_size);
> }),
> struct ena_com_dev *ena_dev, char *buf, size_t buf_size);
>
> @@ -3240,7 +3239,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue,
> struct rte_mbuf **tx_pkts,
> }
>
> static void ena_copy_customer_metrics(struct ena_adapter *adapter, uint64_t
> *buf,
> - size_t num_metrics)
> + size_t num_metrics)
>
Please drop unrelated changed from the set.
> {
> struct ena_com_dev *ena_dev = &adapter->ena_dev;
> int rc;
> @@ -3252,10 +3251,10 @@ static void ena_copy_customer_metrics(struct
> ena_adapter *adapter, uint64_t *buf
> }
> rte_spinlock_lock(&adapter->admin_lock);
> rc = ENA_PROXY(adapter,
> - ena_com_get_customer_metrics,
> - &adapter->ena_dev,
> - (char *)buf,
> - num_metrics * sizeof(uint64_t));
> + ena_com_get_customer_metrics,
> + &adapter->ena_dev,
> + (char *)buf,
> + num_metrics * sizeof(uint64_t));
>
ditto
> rte_spinlock_unlock(&adapter->admin_lock);
> if (rc != 0) {
> PMD_DRV_LOG(WARNING, "Failed to get customer metrics,
> rc: %d\n", rc);
> @@ -4088,7 +4087,7 @@ ena_mp_primary_handle(const struct rte_mp_msg *mp_msg,
> const void *peer)
> case ENA_MP_CUSTOMER_METRICS_GET:
> res = ena_com_get_customer_metrics(ena_dev,
> (char *)adapter->metrics_stats,
> - sizeof(uint64_t) * adapter->metrics_num);
> + adapter->metrics_num * sizeof(uint64_t));
>
Does above change makes any difference? What is the motivation?
> break;
> case ENA_MP_SRD_STATS_GET:
> res = ena_com_get_ena_srd_info(ena_dev,
> diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
> index 4988fbffb5..17d292101c 100644
> --- a/drivers/net/ena/ena_ethdev.h
> +++ b/drivers/net/ena/ena_ethdev.h
> @@ -344,8 +344,8 @@ struct ena_adapter {
> * Helper variables for holding the information about the supported
> * metrics.
> */
> - uint64_t metrics_stats[ENA_MAX_CUSTOMER_METRICS] __rte_cache_aligned;
> - uint16_t metrics_num;
> + uint16_t metrics_num __rte_cache_aligned;
> + uint64_t metrics_stats[ENA_MAX_CUSTOMER_METRICS];
> struct ena_stats_srd srd_stats __rte_cache_aligned;
> };
>
