'_filters' is compared twice, second one will be always false, removing it using the message more relevant to the '_filters'. Fixes: 2deb6b5246d7 ("app/procinfo: add collectd format and host id") Cc: stable@dpdk.org Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- Cc: romanx.korynkevych@intel.com --- app/proc-info/main.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/proc-info/main.c b/app/proc-info/main.c index d743209f0d..35e5b596eb 100644 --- a/app/proc-info/main.c +++ b/app/proc-info/main.c @@ -420,11 +420,9 @@ static void collectd_resolve_cnt_type(char *cnt_type, size_t cnt_type_len, } else if ((type_end != NULL) && (strncmp(cnt_name, "flow_", strlen("flow_"))) == 0) { if (strncmp(type_end, "_filters", strlen("_filters")) == 0) - strlcpy(cnt_type, "operations", cnt_type_len); + strlcpy(cnt_type, "filter_result", cnt_type_len); else if (strncmp(type_end, "_errors", strlen("_errors")) == 0) strlcpy(cnt_type, "errors", cnt_type_len); - else if (strncmp(type_end, "_filters", strlen("_filters")) == 0) - strlcpy(cnt_type, "filter_result", cnt_type_len); } else if ((type_end != NULL) && (strncmp(cnt_name, "mac_", strlen("mac_"))) == 0) { if (strncmp(type_end, "_errors", strlen("_errors")) == 0) -- 2.26.2
'parse_xstats_ids()' return 'int'. The return value is assigned to 'nb_xstats_ids' unsigned value, later negative check on this variable is wrong. Adding interim 'int' variable for negative check. Fixes: 7ac16a3660c0 ("app/proc-info: support xstats by ID and by name") Cc: stable@dpdk.org Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- Cc: kubax.kozak@intel.com --- app/proc-info/main.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/proc-info/main.c b/app/proc-info/main.c index 35e5b596eb..c11fe25af4 100644 --- a/app/proc-info/main.c +++ b/app/proc-info/main.c @@ -301,14 +301,13 @@ proc_info_parse_args(int argc, char **argv) } else if (!strncmp(long_option[option_index].name, "xstats-ids", MAX_LONG_OPT_SZ)) { - nb_xstats_ids = parse_xstats_ids(optarg, + int ret = parse_xstats_ids(optarg, xstats_ids, MAX_NB_XSTATS_IDS); - - if (nb_xstats_ids <= 0) { + if (ret <= 0) { printf("xstats-id list parse error.\n"); return -1; } - + nb_xstats_ids = (uint32_t)ret; } break; default: -- 2.26.2
The intention with the "sizeof(0)" usage is not clear, but the 'stats' already 'memset' by 'rte_cryptodev_stats_get()' API, removing 'memset' in application. Fixes: fe773600fe3e ("app/procinfo: add --show-crypto") Cc: stable@dpdk.org Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- Cc: vipin.varghese@intel.com --- app/proc-info/main.c | 1 - 1 file changed, 1 deletion(-) diff --git a/app/proc-info/main.c b/app/proc-info/main.c index c11fe25af4..dc5cc92209 100644 --- a/app/proc-info/main.c +++ b/app/proc-info/main.c @@ -1207,7 +1207,6 @@ show_crypto(void) display_crypto_feature_info(dev_info.feature_flags); - memset(&stats, 0, sizeof(0)); if (rte_cryptodev_stats_get(i, &stats) == 0) { printf("\t -- stats\n"); printf("\t\t + enqueue count (%"PRIu64")" -- 2.26.2
'flag' initialized to '0' but it is overwritten later, initial assignment can be removed. Fixes: 0101a0ec6217 ("app/procinfo: add --show-mempool") Cc: stable@dpdk.org Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- Cc: vipin.varghese@intel.com --- app/proc-info/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/proc-info/main.c b/app/proc-info/main.c index dc5cc92209..52975842fa 100644 --- a/app/proc-info/main.c +++ b/app/proc-info/main.c @@ -1264,7 +1264,7 @@ show_ring(char *name) static void show_mempool(char *name) { - uint64_t flags = 0; + uint64_t flags; snprintf(bdr_str, MAX_STRING_LEN, " show - MEMPOOL "); STATS_BDR_STR(10, bdr_str); -- 2.26.2
Hi Ferruh,
Thanks for the update
snipped
> show_mempool(char *name) {
> - uint64_t flags = 0;
> + uint64_t flags;
>
Checking the current code base it makes more sense to move the code inside `if` condition check. Sample code shared below
```
- uint64_t flags = 0;
snprintf(bdr_str, MAX_STRING_LEN, " show - MEMPOOL ");
STATS_BDR_STR(10, bdr_str);
if (name != NULL) {
struct rte_mempool *ptr = rte_mempool_lookup(name);
if (ptr != NULL) {
struct rte_mempool_ops *ops;
+ unsigned int flags = ptr->flags;
ops = rte_mempool_get_ops(ptr->ops_index);
```
But it is ok
Thanks Ferruh
> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Tuesday, November 17, 2020 10:45 PM
> To: Tahhan, Maryam <maryam.tahhan@intel.com>; Pattan, Reshma
> <reshma.pattan@intel.com>; Mcnamara, John <john.mcnamara@intel.com>;
> Varghese, Vipin <vipin.varghese@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH 3/4] app/procinfo: remove suspicious sizeof
>
> The intention with the "sizeof(0)" usage is not clear, but the 'stats'
> already 'memset' by 'rte_cryptodev_stats_get()' API, removing 'memset'
> in application.
>
> Fixes: fe773600fe3e ("app/procinfo: add --show-crypto")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: vipin.varghese@intel.com
> ---
> app/proc-info/main.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/app/proc-info/main.c b/app/proc-info/main.c index
> c11fe25af4..dc5cc92209 100644
> --- a/app/proc-info/main.c
> +++ b/app/proc-info/main.c
> @@ -1207,7 +1207,6 @@ show_crypto(void)
>
> display_crypto_feature_info(dev_info.feature_flags);
>
> - memset(&stats, 0, sizeof(0));
> if (rte_cryptodev_stats_get(i, &stats) == 0) {
> printf("\t -- stats\n");
> printf("\t\t + enqueue count (%"PRIu64")"
> --
> 2.26.2
On 11/17/2020 6:04 PM, Varghese, Vipin wrote:
> Hi Ferruh,
>
> Thanks for the update
>
> snipped
>> show_mempool(char *name) {
>> -uint64_t flags = 0;
>> +uint64_t flags;
>>
>
> Checking the current code base it makes more sense to move the code inside `if` condition check. Sample code shared below
>
> ```
> -uint64_t flags = 0;
>
> snprintf(bdr_str, MAX_STRING_LEN, " show - MEMPOOL ");
> STATS_BDR_STR(10, bdr_str);
>
> if (name != NULL) {
> struct rte_mempool *ptr = rte_mempool_lookup(name);
> if (ptr != NULL) {
> struct rte_mempool_ops *ops;
>
> +unsigned int flags = ptr->flags;
> ops = rte_mempool_get_ops(ptr->ops_index);
> ```
>
> But it is ok
>
I think both are OK, this is trivial, let me send a quick v2.
'_filters' is compared twice, second one will be always false, removing it using the message more relevant to the '_filters'. Fixes: 2deb6b5246d7 ("app/procinfo: add collectd format and host id") Cc: stable@dpdk.org Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- app/proc-info/main.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/proc-info/main.c b/app/proc-info/main.c index d743209f0d..35e5b596eb 100644 --- a/app/proc-info/main.c +++ b/app/proc-info/main.c @@ -420,11 +420,9 @@ static void collectd_resolve_cnt_type(char *cnt_type, size_t cnt_type_len, } else if ((type_end != NULL) && (strncmp(cnt_name, "flow_", strlen("flow_"))) == 0) { if (strncmp(type_end, "_filters", strlen("_filters")) == 0) - strlcpy(cnt_type, "operations", cnt_type_len); + strlcpy(cnt_type, "filter_result", cnt_type_len); else if (strncmp(type_end, "_errors", strlen("_errors")) == 0) strlcpy(cnt_type, "errors", cnt_type_len); - else if (strncmp(type_end, "_filters", strlen("_filters")) == 0) - strlcpy(cnt_type, "filter_result", cnt_type_len); } else if ((type_end != NULL) && (strncmp(cnt_name, "mac_", strlen("mac_"))) == 0) { if (strncmp(type_end, "_errors", strlen("_errors")) == 0) -- 2.26.2
'parse_xstats_ids()' return 'int'. The return value is assigned to 'nb_xstats_ids' unsigned value, later negative check on this variable is wrong. Adding interim 'int' variable for negative check. Fixes: 7ac16a3660c0 ("app/proc-info: support xstats by ID and by name") Cc: stable@dpdk.org Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- app/proc-info/main.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/proc-info/main.c b/app/proc-info/main.c index 35e5b596eb..c11fe25af4 100644 --- a/app/proc-info/main.c +++ b/app/proc-info/main.c @@ -301,14 +301,13 @@ proc_info_parse_args(int argc, char **argv) } else if (!strncmp(long_option[option_index].name, "xstats-ids", MAX_LONG_OPT_SZ)) { - nb_xstats_ids = parse_xstats_ids(optarg, + int ret = parse_xstats_ids(optarg, xstats_ids, MAX_NB_XSTATS_IDS); - - if (nb_xstats_ids <= 0) { + if (ret <= 0) { printf("xstats-id list parse error.\n"); return -1; } - + nb_xstats_ids = (uint32_t)ret; } break; default: -- 2.26.2
The intention with the "sizeof(0)" usage is not clear, but the 'stats' already 'memset' by 'rte_cryptodev_stats_get()' API, removing 'memset' in application. Fixes: fe773600fe3e ("app/procinfo: add --show-crypto") Cc: stable@dpdk.org Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- app/proc-info/main.c | 1 - 1 file changed, 1 deletion(-) diff --git a/app/proc-info/main.c b/app/proc-info/main.c index c11fe25af4..dc5cc92209 100644 --- a/app/proc-info/main.c +++ b/app/proc-info/main.c @@ -1207,7 +1207,6 @@ show_crypto(void) display_crypto_feature_info(dev_info.feature_flags); - memset(&stats, 0, sizeof(0)); if (rte_cryptodev_stats_get(i, &stats) == 0) { printf("\t -- stats\n"); printf("\t\t + enqueue count (%"PRIu64")" -- 2.26.2
'flag' is initialized to '0' but it is overwritten later, moving the declaration where it is used and initialize with actual value. Fixes: 0101a0ec6217 ("app/procinfo: add --show-mempool") Cc: stable@dpdk.org Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- app/proc-info/main.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/proc-info/main.c b/app/proc-info/main.c index dc5cc92209..b891622ccb 100644 --- a/app/proc-info/main.c +++ b/app/proc-info/main.c @@ -1264,8 +1264,6 @@ show_ring(char *name) static void show_mempool(char *name) { - uint64_t flags = 0; - snprintf(bdr_str, MAX_STRING_LEN, " show - MEMPOOL "); STATS_BDR_STR(10, bdr_str); @@ -1273,8 +1271,8 @@ show_mempool(char *name) struct rte_mempool *ptr = rte_mempool_lookup(name); if (ptr != NULL) { struct rte_mempool_ops *ops; + uint64_t flags = ptr->flags; - flags = ptr->flags; ops = rte_mempool_get_ops(ptr->ops_index); printf(" - Name: %s on socket %d\n" " - flags:\n" -- 2.26.2
'ret' is already defined in the function scope, removing the 'ret' in the block scope. Fixes: c9507cd0cada ("net/pcap: support physical interface MAC address") Cc: stable@dpdk.org Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- Cc: juhamatti.kuusisaari@coriant.com --- drivers/net/pcap/rte_eth_pcap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c index 4930d7d382..dd9ef33c85 100644 --- a/drivers/net/pcap/rte_eth_pcap.c +++ b/drivers/net/pcap/rte_eth_pcap.c @@ -1324,7 +1324,7 @@ eth_from_pcaps(struct rte_vdev_device *vdev, /* phy_mac arg is applied only only if "iface" devarg is provided */ if (rx_queues->phy_mac) { - int ret = eth_pcap_update_mac(rx_queues->queue[0].name, + ret = eth_pcap_update_mac(rx_queues->queue[0].name, eth_dev, vdev->device.numa_node); if (ret == 0) internals->phy_mac = 1; -- 2.26.2
Assignment of function parameter 'umem' removed. Fixes: f0ce7af0e182 ("net/af_xdp: remove resources when port is closed") Cc: stable@dpdk.org Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- drivers/net/af_xdp/rte_eth_af_xdp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c index 2c7892bd7e..7fc70df713 100644 --- a/drivers/net/af_xdp/rte_eth_af_xdp.c +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c @@ -840,7 +840,6 @@ xdp_umem_destroy(struct xsk_umem_info *umem) #endif rte_free(umem); - umem = NULL; } static int -- 2.26.2
Removing useless 'return' statement. Fixes: b2da02480cb7 ("net/bnxt: support EEM system memory") Cc: stable@dpdk.org Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- Cc: peter.spreadborough@broadcom.com --- drivers/net/bnxt/tf_core/tf_em_common.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/net/bnxt/tf_core/tf_em_common.c b/drivers/net/bnxt/tf_core/tf_em_common.c index ad92cbdc75..c96c21c2e9 100644 --- a/drivers/net/bnxt/tf_core/tf_em_common.c +++ b/drivers/net/bnxt/tf_core/tf_em_common.c @@ -307,7 +307,6 @@ tf_em_page_tbl_pgcnt(uint32_t num_pages, { return roundup(num_pages, MAX_PAGE_PTRS(page_size)) / MAX_PAGE_PTRS(page_size); - return 0; } /** -- 2.26.2
On Wed, Nov 18, 2020 at 12:46 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> '_filters' is compared twice, second one will be always false, removing
> it using the message more relevant to the '_filters'.
>
> Fixes: 2deb6b5246d7 ("app/procinfo: add collectd format and host id")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> app/proc-info/main.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
> index d743209f0d..35e5b596eb 100644
> --- a/app/proc-info/main.c
> +++ b/app/proc-info/main.c
> @@ -420,11 +420,9 @@ static void collectd_resolve_cnt_type(char *cnt_type, size_t cnt_type_len,
> } else if ((type_end != NULL) &&
> (strncmp(cnt_name, "flow_", strlen("flow_"))) == 0) {
> if (strncmp(type_end, "_filters", strlen("_filters")) == 0)
> - strlcpy(cnt_type, "operations", cnt_type_len);
> + strlcpy(cnt_type, "filter_result", cnt_type_len);
Do you know what impact this change of type could have?
--
David Marchand
On Wed, Nov 18, 2020 at 12:46 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> 'parse_xstats_ids()' return 'int'. The return value is assigned to
> 'nb_xstats_ids' unsigned value, later negative check on this variable is
> wrong.
>
> Adding interim 'int' variable for negative check.
>
> Fixes: 7ac16a3660c0 ("app/proc-info: support xstats by ID and by name")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> app/proc-info/main.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
> index 35e5b596eb..c11fe25af4 100644
> --- a/app/proc-info/main.c
> +++ b/app/proc-info/main.c
> @@ -301,14 +301,13 @@ proc_info_parse_args(int argc, char **argv)
> } else if (!strncmp(long_option[option_index].name,
> "xstats-ids",
> MAX_LONG_OPT_SZ)) {
> - nb_xstats_ids = parse_xstats_ids(optarg,
> + int ret = parse_xstats_ids(optarg,
> xstats_ids, MAX_NB_XSTATS_IDS);
> -
> - if (nb_xstats_ids <= 0) {
> + if (ret <= 0) {
> printf("xstats-id list parse error.\n");
> return -1;
> }
> -
> + nb_xstats_ids = (uint32_t)ret;
The cast is unneeded.
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
David Marchand
On Wed, Nov 18, 2020 at 12:46 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> The intention with the "sizeof(0)" usage is not clear, but the 'stats'
> already 'memset' by 'rte_cryptodev_stats_get()' API, removing 'memset'
> in application.
>
> Fixes: fe773600fe3e ("app/procinfo: add --show-crypto")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> app/proc-info/main.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
> index c11fe25af4..dc5cc92209 100644
> --- a/app/proc-info/main.c
> +++ b/app/proc-info/main.c
> @@ -1207,7 +1207,6 @@ show_crypto(void)
>
> display_crypto_feature_info(dev_info.feature_flags);
>
> - memset(&stats, 0, sizeof(0));
> if (rte_cryptodev_stats_get(i, &stats) == 0) {
> printf("\t -- stats\n");
> printf("\t\t + enqueue count (%"PRIu64")"
> --
> 2.26.2
>
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
David Marchand
On Wed, Nov 18, 2020 at 12:47 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote: > > 'flag' is initialized to '0' but it is overwritten later, moving the > declaration where it is used and initialize with actual value. > > Fixes: 0101a0ec6217 ("app/procinfo: add --show-mempool") > Cc: stable@dpdk.org > > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> > --- > app/proc-info/main.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/app/proc-info/main.c b/app/proc-info/main.c > index dc5cc92209..b891622ccb 100644 > --- a/app/proc-info/main.c > +++ b/app/proc-info/main.c > @@ -1264,8 +1264,6 @@ show_ring(char *name) > static void > show_mempool(char *name) > { > - uint64_t flags = 0; > - > snprintf(bdr_str, MAX_STRING_LEN, " show - MEMPOOL "); > STATS_BDR_STR(10, bdr_str); > > @@ -1273,8 +1271,8 @@ show_mempool(char *name) > struct rte_mempool *ptr = rte_mempool_lookup(name); > if (ptr != NULL) { > struct rte_mempool_ops *ops; > + uint64_t flags = ptr->flags; Do we really need a temp storage? But otherwise, Reviewed-by: David Marchand <david.marchand@redhat.com> > > - flags = ptr->flags; > ops = rte_mempool_get_ops(ptr->ops_index); > printf(" - Name: %s on socket %d\n" > " - flags:\n" > -- > 2.26.2 > -- David Marchand
On Wed, Nov 18, 2020 at 12:47 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> 'ret' is already defined in the function scope, removing the 'ret' in
> the block scope.
>
> Fixes: c9507cd0cada ("net/pcap: support physical interface MAC address")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: juhamatti.kuusisaari@coriant.com
> ---
> drivers/net/pcap/rte_eth_pcap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
> index 4930d7d382..dd9ef33c85 100644
> --- a/drivers/net/pcap/rte_eth_pcap.c
> +++ b/drivers/net/pcap/rte_eth_pcap.c
> @@ -1324,7 +1324,7 @@ eth_from_pcaps(struct rte_vdev_device *vdev,
>
> /* phy_mac arg is applied only only if "iface" devarg is provided */
> if (rx_queues->phy_mac) {
> - int ret = eth_pcap_update_mac(rx_queues->queue[0].name,
> + ret = eth_pcap_update_mac(rx_queues->queue[0].name,
> eth_dev, vdev->device.numa_node);
> if (ret == 0)
> internals->phy_mac = 1;
It is not used after, why not simply check eth_pcap_update_mac() == 0 ?
--
David Marchand
On Wed, Nov 18, 2020 at 12:47 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> Assignment of function parameter 'umem' removed.
>
> Fixes: f0ce7af0e182 ("net/af_xdp: remove resources when port is closed")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> drivers/net/af_xdp/rte_eth_af_xdp.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c b/drivers/net/af_xdp/rte_eth_af_xdp.c
> index 2c7892bd7e..7fc70df713 100644
> --- a/drivers/net/af_xdp/rte_eth_af_xdp.c
> +++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
> @@ -840,7 +840,6 @@ xdp_umem_destroy(struct xsk_umem_info *umem)
> #endif
>
> rte_free(umem);
> - umem = NULL;
> }
>
> static int
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
David Marchand
On Wed, Nov 18, 2020 at 12:48 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> Removing useless 'return' statement.
>
> Fixes: b2da02480cb7 ("net/bnxt: support EEM system memory")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: peter.spreadborough@broadcom.com
> ---
> drivers/net/bnxt/tf_core/tf_em_common.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/bnxt/tf_core/tf_em_common.c b/drivers/net/bnxt/tf_core/tf_em_common.c
> index ad92cbdc75..c96c21c2e9 100644
> --- a/drivers/net/bnxt/tf_core/tf_em_common.c
> +++ b/drivers/net/bnxt/tf_core/tf_em_common.c
> @@ -307,7 +307,6 @@ tf_em_page_tbl_pgcnt(uint32_t num_pages,
> {
> return roundup(num_pages, MAX_PAGE_PTRS(page_size)) /
> MAX_PAGE_PTRS(page_size);
> - return 0;
> }
>
> /**
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
David Marchand
On Wed, Nov 18, 2020 at 6:43 AM David Marchand <david.marchand@redhat.com> wrote: > > On Wed, Nov 18, 2020 at 12:48 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote: > > > > Removing useless 'return' statement. > > > > Fixes: b2da02480cb7 ("net/bnxt: support EEM system memory") > > Cc: stable@dpdk.org > > > > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> > > --- > > Cc: peter.spreadborough@broadcom.com > > --- > > drivers/net/bnxt/tf_core/tf_em_common.c | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/drivers/net/bnxt/tf_core/tf_em_common.c b/drivers/net/bnxt/tf_core/tf_em_common.c > > index ad92cbdc75..c96c21c2e9 100644 > > --- a/drivers/net/bnxt/tf_core/tf_em_common.c > > +++ b/drivers/net/bnxt/tf_core/tf_em_common.c > > @@ -307,7 +307,6 @@ tf_em_page_tbl_pgcnt(uint32_t num_pages, > > { > > return roundup(num_pages, MAX_PAGE_PTRS(page_size)) / > > MAX_PAGE_PTRS(page_size); > > - return 0; > > } > > > > /** > > Reviewed-by: David Marchand <david.marchand@redhat.com> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com> > > > -- > David Marchand >
On 11/18/2020 2:10 PM, David Marchand wrote:
> On Wed, Nov 18, 2020 at 12:46 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> '_filters' is compared twice, second one will be always false, removing
>> it using the message more relevant to the '_filters'.
>>
>> Fixes: 2deb6b5246d7 ("app/procinfo: add collectd format and host id")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> app/proc-info/main.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/app/proc-info/main.c b/app/proc-info/main.c
>> index d743209f0d..35e5b596eb 100644
>> --- a/app/proc-info/main.c
>> +++ b/app/proc-info/main.c
>> @@ -420,11 +420,9 @@ static void collectd_resolve_cnt_type(char *cnt_type, size_t cnt_type_len,
>> } else if ((type_end != NULL) &&
>> (strncmp(cnt_name, "flow_", strlen("flow_"))) == 0) {
>> if (strncmp(type_end, "_filters", strlen("_filters")) == 0)
>> - strlcpy(cnt_type, "operations", cnt_type_len);
>> + strlcpy(cnt_type, "filter_result", cnt_type_len);
>
> Do you know what impact this change of type could have?
>
Since the default behavior is changing it has potential to impact it's users but
I don't know.
"_filters" stat was searched twice and type were set "operations" &
"filter_result" respectively, I unified it to one and selected "filter_result"
which seems more relevant.