From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> This patch fixes a typo introduced in the last backport. Fixed a wrong check. Upstream code does not have this issue. Bugzilla ID: 977 Fixes: 942eb8e842fc ("net/bnxt: fix xstats names query overrun") Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> --- drivers/net/bnxt/bnxt_stats.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c index 39fd100..bc181db 100644 --- a/drivers/net/bnxt/bnxt_stats.c +++ b/drivers/net/bnxt/bnxt_stats.c @@ -612,7 +612,7 @@ int bnxt_dev_xstats_get_names_op(__rte_unused struct rte_eth_dev *eth_dev, if (rc) return rc; - if (xstats_names != NULL || size < stat_cnt) + if (xstats_names == NULL || size < stat_cnt) return stat_cnt; for (i = 0; i < RTE_DIM(bnxt_rx_stats_strings); i++) { -- 2.10.1
On Fri, Mar 25, 2022 at 9:15 AM Kalesh A P <kalesh-anakkur.purayil@broadcom.com> wrote: > > From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> > > This patch fixes a typo introduced in the last backport. > Fixed a wrong check. Hi Kalesh, thanks for the fix. Since we already have entered -rc1 and testing I wanted to ask how sever this issue is. The options we have are a) not really an important issue, hold it back not and make it part of 19.11.13 later this year b) breaking bnxt too much, needs to get into 19.11.12 - but we would not reset testing (no new RC), you'd cover some bnxt related tests c) breaking bnxt too much and having potential to influence all things, taking into 19.11.12 and casting an -rc2 resetting tests for everyone I'm tempted to consider this a case for (b), but wanted to know if you agree and if you could make the related bnxt based re-test happen? > Upstream code does not have this issue. > > Bugzilla ID: 977 > Fixes: 942eb8e842fc ("net/bnxt: fix xstats names query overrun") > > Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> > --- > drivers/net/bnxt/bnxt_stats.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c > index 39fd100..bc181db 100644 > --- a/drivers/net/bnxt/bnxt_stats.c > +++ b/drivers/net/bnxt/bnxt_stats.c > @@ -612,7 +612,7 @@ int bnxt_dev_xstats_get_names_op(__rte_unused struct rte_eth_dev *eth_dev, > if (rc) > return rc; > > - if (xstats_names != NULL || size < stat_cnt) > + if (xstats_names == NULL || size < stat_cnt) > return stat_cnt; > > for (i = 0; i < RTE_DIM(bnxt_rx_stats_strings); i++) { > -- > 2.10.1 > -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd
[-- Attachment #1: Type: text/plain, Size: 2098 bytes --] On Sun, Mar 27, 2022 at 11:29 PM Christian Ehrhardt <christian.ehrhardt@canonical.com> wrote: > > On Fri, Mar 25, 2022 at 9:15 AM Kalesh A P > <kalesh-anakkur.purayil@broadcom.com> wrote: > > > > From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> > > > > This patch fixes a typo introduced in the last backport. > > Fixed a wrong check. > > Hi Kalesh, > thanks for the fix. > > Since we already have entered -rc1 and testing I wanted to ask how > sever this issue is. > The options we have are > a) not really an important issue, hold it back not and make it part of > 19.11.13 later this year > b) breaking bnxt too much, needs to get into 19.11.12 - but we would > not reset testing (no new RC), you'd cover some bnxt related tests > c) breaking bnxt too much and having potential to influence all > things, taking into 19.11.12 and casting an -rc2 resetting tests for > everyone > > I'm tempted to consider this a case for (b), but wanted to know if you > agree and if you could make the related bnxt based re-test happen? Christian, option (b) is fine. We can take care of bnxt related tests. Thanks Ajit > > > Upstream code does not have this issue. > > > > Bugzilla ID: 977 > > Fixes: 942eb8e842fc ("net/bnxt: fix xstats names query overrun") > > > > Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> > > --- > > drivers/net/bnxt/bnxt_stats.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/bnxt/bnxt_stats.c b/drivers/net/bnxt/bnxt_stats.c > > index 39fd100..bc181db 100644 > > --- a/drivers/net/bnxt/bnxt_stats.c > > +++ b/drivers/net/bnxt/bnxt_stats.c > > @@ -612,7 +612,7 @@ int bnxt_dev_xstats_get_names_op(__rte_unused struct rte_eth_dev *eth_dev, > > if (rc) > > return rc; > > > > - if (xstats_names != NULL || size < stat_cnt) > > + if (xstats_names == NULL || size < stat_cnt) > > return stat_cnt; > > > > for (i = 0; i < RTE_DIM(bnxt_rx_stats_strings); i++) { > > -- > > 2.10.1 > > > > > -- > Christian Ehrhardt > Staff Engineer, Ubuntu Server > Canonical Ltd [-- Attachment #2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 4218 bytes --]
[-- Attachment #1: Type: text/plain, Size: 2512 bytes --] On Mon, Mar 28, 2022 at 3:46 PM Ajit Khaparde <ajit.khaparde@broadcom.com> wrote: > On Sun, Mar 27, 2022 at 11:29 PM Christian Ehrhardt > <christian.ehrhardt@canonical.com> wrote: > > > > On Fri, Mar 25, 2022 at 9:15 AM Kalesh A P > > <kalesh-anakkur.purayil@broadcom.com> wrote: > > > > > > From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> > > > > > > This patch fixes a typo introduced in the last backport. > > > Fixed a wrong check. > > > > Hi Kalesh, > > thanks for the fix. > > > > Since we already have entered -rc1 and testing I wanted to ask how > > sever this issue is. > > The options we have are > > a) not really an important issue, hold it back not and make it part of > > 19.11.13 later this year > > b) breaking bnxt too much, needs to get into 19.11.12 - but we would > > not reset testing (no new RC), you'd cover some bnxt related tests > > c) breaking bnxt too much and having potential to influence all > > things, taking into 19.11.12 and casting an -rc2 resetting tests for > > everyone > > > > I'm tempted to consider this a case for (b), but wanted to know if you > > agree and if you could make the related bnxt based re-test happen? > Christian, option (b) is fine. We can take care of bnxt related tests. > Thank you, applied without tagging -rc2 as agreed. For your testing it is here https://git.dpdk.org/dpdk-stable/log/?h=19.11 > Thanks > Ajit > > > > > Upstream code does not have this issue. > > > > > > Bugzilla ID: 977 > > > Fixes: 942eb8e842fc ("net/bnxt: fix xstats names query overrun") > > > > > > Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com> > > > --- > > > drivers/net/bnxt/bnxt_stats.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/bnxt/bnxt_stats.c > b/drivers/net/bnxt/bnxt_stats.c > > > index 39fd100..bc181db 100644 > > > --- a/drivers/net/bnxt/bnxt_stats.c > > > +++ b/drivers/net/bnxt/bnxt_stats.c > > > @@ -612,7 +612,7 @@ int bnxt_dev_xstats_get_names_op(__rte_unused > struct rte_eth_dev *eth_dev, > > > if (rc) > > > return rc; > > > > > > - if (xstats_names != NULL || size < stat_cnt) > > > + if (xstats_names == NULL || size < stat_cnt) > > > return stat_cnt; > > > > > > for (i = 0; i < RTE_DIM(bnxt_rx_stats_strings); i++) { > > > -- > > > 2.10.1 > > > > > > > > > -- > > Christian Ehrhardt > > Staff Engineer, Ubuntu Server > > Canonical Ltd > -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd [-- Attachment #2: Type: text/html, Size: 4000 bytes --]