[ upstream commit 00437823cb80b8fa87dbe61becc07bd42ee98549 ] fgets(3)/fread(3)/fscanf(3) etc. use mmap(2)/munmap(2) which leads to TLB shutdown interrupts to all DPDK app cores including RX cores. This can cause packet drops. Use read(2)/write(2) instead. Bugzilla ID: 440 Signed-off-by: Mohsin Shaikh <mohsinshaikh@niometrics.com> Reviewed-by: Alexander Kozyrev <akozyrev@mellanox.com> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com> --- drivers/net/mlx5/mlx5_stats.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c index 6906dc8..0b1fe3d 100644 --- a/drivers/net/mlx5/mlx5_stats.c +++ b/drivers/net/mlx5/mlx5_stats.c @@ -3,11 +3,13 @@ * Copyright 2015 Mellanox Technologies, Ltd */ +#include <fcntl.h> #include <inttypes.h> #include <linux/sockios.h> #include <linux/ethtool.h> #include <stdint.h> #include <stdio.h> +#include <unistd.h> #include <rte_ethdev_driver.h> #include <rte_common.h> @@ -139,19 +141,24 @@ static inline void mlx5_read_ib_stat(struct mlx5_priv *priv, const char *ctr_name, uint64_t *stat) { - FILE *file; + int fd; + MKSTR(path, "%s/ports/1/hw_counters/%s", priv->ibdev_path, ctr_name); - file = fopen(path, "rb"); - if (file) { - int n = fscanf(file, "%" SCNu64, stat); + fd = open(path, O_RDONLY); + if (fd != -1) { + char buf[21] = {'\0'}; + ssize_t n = read(fd, buf, sizeof(buf)); - fclose(file); - if (n != 1) - stat = 0; + close(fd); + if (n != -1) { + *stat = strtoull(buf, NULL, 10); + return; + } } + *stat = 0; } /** -- 1.8.3.1
From: Mohsin Shaikh <mohsinshaikh@niometrics.com> From: Mohsin Shaikh <mohsinshaikh@niometrics.com> fgets(3)/fread(3)/fscanf(3) etc. use mmap(2)/munmap(2) which leads to TLB shutdown interrupts to all DPDK app cores including RX cores. This can cause packet drops. Use read(2)/write(2) instead. Bugzilla ID: 440 Cc: stable@dpdk.org Signed-off-by: Mohsin Shaikh <mohsinshaikh@niometrics.com> Reviewed-by: Alexander Kozyrev <akozyrev@mellanox.com> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com> --- drivers/net/mlx5/mlx5_stats.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c index 6906dc8..0b1fe3d 100644 --- a/drivers/net/mlx5/mlx5_stats.c +++ b/drivers/net/mlx5/mlx5_stats.c @@ -3,11 +3,13 @@ * Copyright 2015 Mellanox Technologies, Ltd */ +#include <fcntl.h> #include <inttypes.h> #include <linux/sockios.h> #include <linux/ethtool.h> #include <stdint.h> #include <stdio.h> +#include <unistd.h> #include <rte_ethdev_driver.h> #include <rte_common.h> @@ -139,19 +141,24 @@ static inline void mlx5_read_ib_stat(struct mlx5_priv *priv, const char *ctr_name, uint64_t *stat) { - FILE *file; + int fd; + MKSTR(path, "%s/ports/1/hw_counters/%s", priv->ibdev_path, ctr_name); - file = fopen(path, "rb"); - if (file) { - int n = fscanf(file, "%" SCNu64, stat); + fd = open(path, O_RDONLY); + if (fd != -1) { + char buf[21] = {'\0'}; + ssize_t n = read(fd, buf, sizeof(buf)); - fclose(file); - if (n != 1) - stat = 0; + close(fd); + if (n != -1) { + *stat = strtoull(buf, NULL, 10); + return; + } } + *stat = 0; } /** -- 1.8.3.1
On 08/12/2020 11:34, Viacheslav Ovsiienko wrote: > From: Mohsin Shaikh <mohsinshaikh@niometrics.com> > > From: Mohsin Shaikh <mohsinshaikh@niometrics.com> > Hi Slava, your patches are putting in duplicate From: tags ^ and missing upstream commit ids. Please add upstream commit ids so I don't have to find it for every patch. i.e. [ upstream commit 00437823cb80b8fa87dbe61becc07bd42ee98549 ] > fgets(3)/fread(3)/fscanf(3) etc. use mmap(2)/munmap(2) which leads > to TLB shutdown interrupts to all DPDK app cores including RX cores. > This can cause packet drops. Use read(2)/write(2) instead. > > Bugzilla ID: 440 > Cc: stable@dpdk.org > We can drop the stable tag as it's targetted to LTS branch anyway > Signed-off-by: Mohsin Shaikh <mohsinshaikh@niometrics.com> > Reviewed-by: Alexander Kozyrev <akozyrev@mellanox.com> > Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com> > --- > drivers/net/mlx5/mlx5_stats.c | 21 ++++++++++++++------- > 1 file changed, 14 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c > index 6906dc8..0b1fe3d 100644 > --- a/drivers/net/mlx5/mlx5_stats.c > +++ b/drivers/net/mlx5/mlx5_stats.c > @@ -3,11 +3,13 @@ > * Copyright 2015 Mellanox Technologies, Ltd > */ > > +#include <fcntl.h> > #include <inttypes.h> > #include <linux/sockios.h> > #include <linux/ethtool.h> > #include <stdint.h> > #include <stdio.h> > +#include <unistd.h> > > #include <rte_ethdev_driver.h> > #include <rte_common.h> > @@ -139,19 +141,24 @@ > static inline void > mlx5_read_ib_stat(struct mlx5_priv *priv, const char *ctr_name, uint64_t *stat) > { > - FILE *file; > + int fd; > + > MKSTR(path, "%s/ports/1/hw_counters/%s", > priv->ibdev_path, > ctr_name); > > - file = fopen(path, "rb"); > - if (file) { > - int n = fscanf(file, "%" SCNu64, stat); > + fd = open(path, O_RDONLY); > + if (fd != -1) { > + char buf[21] = {'\0'}; > + ssize_t n = read(fd, buf, sizeof(buf)); > > - fclose(file); > - if (n != 1) > - stat = 0; > + close(fd); > + if (n != -1) { > + *stat = strtoull(buf, NULL, 10); > + return; > + } > } > + *stat = 0; > } > > /** >
OK, so, should I resend v2 with:
- no From: tag
- add [ upstream commit 00437823cb80b8fa87dbe61becc07bd42ee98549 ]
With best regards, Slava
> -----Original Message-----
> From: Kevin Traynor <ktraynor@redhat.com>
> Sent: Tuesday, December 8, 2020 14:07
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; stable@dpdk.org
> Cc: mohsinshaikh@niometrics.com
> Subject: Re: [PATCH] [18.11] net/mlx5: use open/read/close for ib stats query
>
> On 08/12/2020 11:34, Viacheslav Ovsiienko wrote:
> > From: Mohsin Shaikh <mohsinshaikh@niometrics.com>
> >
> > From: Mohsin Shaikh <mohsinshaikh@niometrics.com>
> >
>
> Hi Slava, your patches are putting in duplicate From: tags ^ and missing
> upstream commit ids. Please add upstream commit ids so I don't have to find it
> for every patch. i.e.
>
> [ upstream commit 00437823cb80b8fa87dbe61becc07bd42ee98549 ]
>
> > fgets(3)/fread(3)/fscanf(3) etc. use mmap(2)/munmap(2) which leads to
> > TLB shutdown interrupts to all DPDK app cores including RX cores.
> > This can cause packet drops. Use read(2)/write(2) instead.
> >
> > Bugzilla ID: 440
> > Cc: stable@dpdk.org
> >
>
> We can drop the stable tag as it's targetted to LTS branch anyway
>
> > Signed-off-by: Mohsin Shaikh <mohsinshaikh@niometrics.com>
> > Reviewed-by: Alexander Kozyrev <akozyrev@mellanox.com>
> > Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> > ---
> > drivers/net/mlx5/mlx5_stats.c | 21 ++++++++++++++-------
> > 1 file changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_stats.c
> > b/drivers/net/mlx5/mlx5_stats.c index 6906dc8..0b1fe3d 100644
> > --- a/drivers/net/mlx5/mlx5_stats.c
> > +++ b/drivers/net/mlx5/mlx5_stats.c
> > @@ -3,11 +3,13 @@
> > * Copyright 2015 Mellanox Technologies, Ltd
> > */
> >
> > +#include <fcntl.h>
> > #include <inttypes.h>
> > #include <linux/sockios.h>
> > #include <linux/ethtool.h>
> > #include <stdint.h>
> > #include <stdio.h>
> > +#include <unistd.h>
> >
> > #include <rte_ethdev_driver.h>
> > #include <rte_common.h>
> > @@ -139,19 +141,24 @@
> > static inline void
> > mlx5_read_ib_stat(struct mlx5_priv *priv, const char *ctr_name,
> > uint64_t *stat) {
> > - FILE *file;
> > + int fd;
> > +
> > MKSTR(path, "%s/ports/1/hw_counters/%s",
> > priv->ibdev_path,
> > ctr_name);
> >
> > - file = fopen(path, "rb");
> > - if (file) {
> > - int n = fscanf(file, "%" SCNu64, stat);
> > + fd = open(path, O_RDONLY);
> > + if (fd != -1) {
> > + char buf[21] = {'\0'};
> > + ssize_t n = read(fd, buf, sizeof(buf));
> >
> > - fclose(file);
> > - if (n != 1)
> > - stat = 0;
> > + close(fd);
> > + if (n != -1) {
> > + *stat = strtoull(buf, NULL, 10);
> > + return;
> > + }
> > }
> > + *stat = 0;
> > }
> >
> > /**
> >
On 08/12/2020 12:41, Slava Ovsiienko wrote: > OK, so, should I resend v2 with: > - no From: tag > - add [ upstream commit 00437823cb80b8fa87dbe61becc07bd42ee98549 ] > Got it, thanks. > With best regards, Slava > >> -----Original Message----- >> From: Kevin Traynor <ktraynor@redhat.com> >> Sent: Tuesday, December 8, 2020 14:07 >> To: Slava Ovsiienko <viacheslavo@nvidia.com>; stable@dpdk.org >> Cc: mohsinshaikh@niometrics.com >> Subject: Re: [PATCH] [18.11] net/mlx5: use open/read/close for ib stats query >> >> On 08/12/2020 11:34, Viacheslav Ovsiienko wrote: >>> From: Mohsin Shaikh <mohsinshaikh@niometrics.com> >>> >>> From: Mohsin Shaikh <mohsinshaikh@niometrics.com> >>> >> Hi Slava, your patches are putting in duplicate From: tags ^ and missing >> upstream commit ids. Please add upstream commit ids so I don't have to find it >> for every patch. i.e. >> >> [ upstream commit 00437823cb80b8fa87dbe61becc07bd42ee98549 ] >> >>> fgets(3)/fread(3)/fscanf(3) etc. use mmap(2)/munmap(2) which leads to >>> TLB shutdown interrupts to all DPDK app cores including RX cores. >>> This can cause packet drops. Use read(2)/write(2) instead. >>> >>> Bugzilla ID: 440 >>> Cc: stable@dpdk.org >>> >> We can drop the stable tag as it's targetted to LTS branch anyway >> >>> Signed-off-by: Mohsin Shaikh <mohsinshaikh@niometrics.com> >>> Reviewed-by: Alexander Kozyrev <akozyrev@mellanox.com> >>> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com> >>> --- >>> drivers/net/mlx5/mlx5_stats.c | 21 ++++++++++++++------- >>> 1 file changed, 14 insertions(+), 7 deletions(-)