Re: Ethdev tracepoints optimization
On Thu, Aug 15, 2024 at 03:32:50PM -0400, Adel Belkhiri wrote: >Hi DPDK Community, >I am currently working on developing performance analyses for >applications using the ethdev library. These analyses are being >implemented in Trace Compass, an open-source performance analyzer. One >of the views I’ve implemented shows the rate of traffic received or >sent by an ethernet port, measured in packets per second. However, I've >encountered an issue with the lib.ethdev.rx.burst event, which triggers >even when no packets are polled, leading to a significant number of >irrelevant events in the trace. This becomes problematic as these >"empty" events can overwhelm the tracer buffer, potentially causing the >loss of more critical events due to their high frequency. >To address this, I've modified the DPDK code in lib/ethdev/rte_ethdev.h >to add a conditional statement that only triggers the event when nb_rx >> 0. My question to the community is whether there are use cases where >an "empty" lib.ethdev.rx.burst event could be useful. If not, would >there be interest in submitting a patch with this modification? Yes, there probably would be, but also likely not in such a simple form. I think there is value in tracing around empty polls too, but only in such a way not to overflow the trace buffer. My suggestion would be to trace the first empty poll only, but suppress any polls thereafter. That way we can know that there are empty polls rather than no calls. An even better solution - though I don't know enough about how the tracing works to know if it's possible - is to match that initial empty poll trace with some sort of trace output when the empty polls stop, i.e. we get traffic, recording the number of empty polls received as a count value. My 2c. /Bruce
Re: [PATCH v1 1/2] usertools/cpu_layout: update coding style
Anatoly Burakov, Aug 14, 2024 at 13:19:
Update coding style:
- make it PEP-484 compliant
- address all flake8, mypy etc. warnings
- use f-strings in place of old-style string interpolation
- refactor printing to make the code more readable
Signed-off-by: Anatoly Burakov
---
Hi Anatoly,
thanks for the patch. Did you format the code using black/ruff? I'd like
to start keeping python code formatting consistent across user tools.
usertools/cpu_layout.py | 162 ++--
1 file changed, 104 insertions(+), 58 deletions(-)
diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index 891b9238fa..843b29a134 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -3,62 +3,108 @@
# Copyright(c) 2010-2014 Intel Corporation
# Copyright(c) 2017 Cavium, Inc. All rights reserved.
-sockets = []
-cores = []
-core_map = {}
-base_path = "/sys/devices/system/cpu"
-fd = open("{}/kernel_max".format(base_path))
-max_cpus = int(fd.read())
-fd.close()
-for cpu in range(max_cpus + 1):
-try:
-fd = open("{}/cpu{}/topology/core_id".format(base_path, cpu))
-except IOError:
-continue
-core = int(fd.read())
-fd.close()
-fd = open("{}/cpu{}/topology/physical_package_id".format(base_path, cpu))
-socket = int(fd.read())
-fd.close()
-if core not in cores:
-cores.append(core)
-if socket not in sockets:
-sockets.append(socket)
-key = (socket, core)
-if key not in core_map:
-core_map[key] = []
-core_map[key].append(cpu)
-
-print(format("=" * (47 + len(base_path
-print("Core and Socket Information (as reported by '{}')".format(base_path))
-print("{}\n".format("=" * (47 + len(base_path
-print("cores = ", cores)
-print("sockets = ", sockets)
-print("")
-
-max_processor_len = len(str(len(cores) * len(sockets) * 2 - 1))
-max_thread_count = len(list(core_map.values())[0])
-max_core_map_len = (max_processor_len * max_thread_count) \
- + len(", ") * (max_thread_count - 1) \
- + len('[]') + len('Socket ')
-max_core_id_len = len(str(max(cores)))
-
-output = " ".ljust(max_core_id_len + len('Core '))
-for s in sockets:
-output += " Socket %s" % str(s).ljust(max_core_map_len - len('Socket '))
-print(output)
-
-output = " ".ljust(max_core_id_len + len('Core '))
-for s in sockets:
-output += " ".ljust(max_core_map_len)
-output += " "
-print(output)
-
-for c in cores:
-output = "Core %s" % str(c).ljust(max_core_id_len)
-for s in sockets:
-if (s, c) in core_map:
-output += " " + str(core_map[(s, c)]).ljust(max_core_map_len)
+from typing import List, Set, Dict, Tuple
+
+
+def _range_expand(rstr: str) -> List[int]:
I don't see any reason to prefix the symbols with an underscore in this
script. It will not be used as an importable library.
+"""Expand a range string into a list of integers."""
+# 0,1-3 => [0, 1-3]
+ranges = rstr.split(",")
+valset: List[int] = []
+for r in ranges:
+# 1-3 => [1, 2, 3]
+if "-" in r:
+start, end = r.split("-")
+valset.extend(range(int(start), int(end) + 1))
else:
-output += " " * (max_core_map_len + 1)
-print(output)
+valset.append(int(r))
+return valset
+
+
+def _read_sysfs(path: str) -> str:
+with open(path) as fd:
+return fd.read().strip()
+
+
+def _print_row(row: Tuple[str, ...], col_widths: List[int]) -> None:
+first, *rest = row
+w_first, *w_rest = col_widths
+first_end = " " * 4
+rest_end = " " * 10
+
+print(first.ljust(w_first), end=first_end)
+for cell, width in zip(rest, w_rest):
+print(cell.rjust(width), end=rest_end)
+print()
+
+
+def _print_section(heading: str) -> None:
+sep = "=" * len(heading)
+print(sep)
+print(heading)
+print(sep)
+print()
+
+
+def _main() -> None:
+sockets_s: Set[int] = set()
+cores_s: Set[int] = set()
+core_map: Dict[Tuple[int, int], List[int]] = {}
+base_path = "/sys/devices/system/cpu"
+
+cpus = _range_expand(_read_sysfs(f"{base_path}/online"))
+
+for cpu in cpus:
+lcore_base = f"{base_path}/cpu{cpu}"
+core = int(_read_sysfs(f"{lcore_base}/topology/core_id"))
+socket = int(_read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+
+cores_s.add(core)
+sockets_s.add(socket)
+key = (socket, core)
+core_map.setdefault(key, [])
+core_map[key].append(cpu)
+
+cores = sorted(cores_s)
+sockets = sorted(sockets_s)
+
+_print_section("Core and Socket Information "
+ f"(as reported by '{base_path}')")
+
+print("cores = ", cores)
+print("sockets = ", sockets)
+print()
+
+# Core, [Socket, Socket, ...]
+heading_strs = "", *[f"Socket {s}" for s in sockets]
+sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
+rows: List[Tuple[str, ...]]
Re: [PATCH v1] dts: correct typos in conf.yaml
Nice one. Reviewed-by: Luca Vizzarro
Re: [PATCH v1 1/2] usertools/cpu_layout: update coding style
On 8/19/2024 11:26 AM, Robin Jarry wrote:
Anatoly Burakov, Aug 14, 2024 at 13:19:
Update coding style:
- make it PEP-484 compliant
- address all flake8, mypy etc. warnings
- use f-strings in place of old-style string interpolation
- refactor printing to make the code more readable
Signed-off-by: Anatoly Burakov
---
Hi Anatoly,
thanks for the patch. Did you format the code using black/ruff? I'd like
to start keeping python code formatting consistent across user tools.
I don't think I did any formatting with a specific tool. Rather, my IDE
had flake8 set up to give me warnings if there are issues with
formatting, and it also does formatting, so this would effectively be
the same thing.
Judging by description of Ruff, it sounds like something I should try
out, so thanks for the suggestion.
usertools/cpu_layout.py | 162 ++--
1 file changed, 104 insertions(+), 58 deletions(-)
diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index 891b9238fa..843b29a134 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -3,62 +3,108 @@
# Copyright(c) 2010-2014 Intel Corporation
# Copyright(c) 2017 Cavium, Inc. All rights reserved.
-output = " ".ljust(max_core_id_len + len('Core '))
-for s in sockets:
- output += " ".ljust(max_core_map_len)
- output += " "
-print(output)
-
-for c in cores:
- output = "Core %s" % str(c).ljust(max_core_id_len)
- for s in sockets:
- if (s, c) in core_map:
- output += " " + str(core_map[(s,
c)]).ljust(max_core_map_len)
+from typing import List, Set, Dict, Tuple
+
+
+def _range_expand(rstr: str) -> List[int]:
I don't see any reason to prefix the symbols with an underscore in this
script. It will not be used as an importable library.
In the general case I prefer not to make such assumptions, but in this
particular case I think it's a safe assumption to make.
--
Thanks,
Anatoly
Re: [PATCH v3] net/gve: add support for TSO in DQO RDA
On Sat, Aug 10, 2024 at 12:19 AM Tathagat Priyadarshi wrote: > > The patch intends on adding support for TSO in DQO RDA format. > > Signed-off-by: Tathagat Priyadarshi > Signed-off-by: Varun Lakkur Ambaji Rao > --- > drivers/net/gve/gve_tx_dqo.c | 26 +++--- > 1 file changed, 23 insertions(+), 3 deletions(-) Hi @Joshua Washington / @Rushil Gupta , Could you please review this patch?
Re: Ethdev tracepoints optimization
On 8/15/2024 8:32 PM, Adel Belkhiri wrote: > Hi DPDK Community, > > I am currently working on developing performance analyses for > applications using the ethdev library. These analyses are being > implemented in Trace Compass, an open-source performance analyzer. One > of the views I’ve implemented shows the rate of traffic received or sent > by an ethernet port, measured in packets per second. However, I've > encountered an issue with the lib.ethdev.rx.burst event, which triggers > even when no packets are polled, leading to a significant number of > irrelevant events in the trace. This becomes problematic as these > "empty" events can overwhelm the tracer buffer, potentially causing the > loss of more critical events due to their high frequency. > > To address this, I've modified the DPDK code in lib/ethdev/rte_ethdev.h > to add a conditional statement that only triggers the event when nb_rx > > 0. My question to the community is whether there are use cases where an > "empty" lib.ethdev.rx.burst event could be useful. If not, would there > be interest in submitting a patch with this modification? > Tracepoint is good way to get frequency of the calls, so I believe there can be value of getting empty burst calls too. But your usecase also a valid one. I wonder if it works to have separate trace calls, for empty and non-empty ones, and how this additional branching impacts the performance, at least branch should be wrapped with 'RTE_ENABLE_TRACE_FP' macro to not impact non-tracing usage.
Re: [PATCH] net/tap: avoid memcpy with NULL arg
On 8/14/2024 3:34 AM, Stephen Hemminger wrote: > Calling memcpy with a null pointer even if zero length is > undefined, so check if data_length is zero. > Problem reported by Gcc analyzer. > > Signed-off-by: Stephen Hemminger > Acked-by: Ferruh Yigit Applied to dpdk-next-net/main, thanks.
Re: [PATCH] net/tap: avoid memcpy with NULL arg
On 8/19/2024 12:03 PM, Ferruh Yigit wrote: > On 8/14/2024 3:34 AM, Stephen Hemminger wrote: >> Calling memcpy with a null pointer even if zero length is >> undefined, so check if data_length is zero. >> Problem reported by Gcc analyzer. >> Btw, how do you run the GCC analyzer? Is this something can we add to our CI checks? >> >> Signed-off-by: Stephen Hemminger >> > > Acked-by: Ferruh Yigit > Applied to dpdk-next-net/main, thanks. >
Re: [PATCH v2 1/4] usertools/cpu_layout: update coding style
Anatoly Burakov, Aug 16, 2024 at 14:16:
Update coding style:
- make it PEP-484 compliant
- address all flake8, mypy etc. warnings
- use f-strings in place of old-style string interpolation
- refactor printing to make the code more readable
Signed-off-by: Anatoly Burakov
---
usertools/cpu_layout.py | 162 ++--
1 file changed, 104 insertions(+), 58 deletions(-)
diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index 891b9238fa..be86f06938 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -3,62 +3,108 @@
# Copyright(c) 2010-2014 Intel Corporation
# Copyright(c) 2017 Cavium, Inc. All rights reserved.
-sockets = []
-cores = []
-core_map = {}
-base_path = "/sys/devices/system/cpu"
-fd = open("{}/kernel_max".format(base_path))
-max_cpus = int(fd.read())
-fd.close()
-for cpu in range(max_cpus + 1):
-try:
-fd = open("{}/cpu{}/topology/core_id".format(base_path, cpu))
-except IOError:
-continue
-core = int(fd.read())
-fd.close()
-fd = open("{}/cpu{}/topology/physical_package_id".format(base_path, cpu))
-socket = int(fd.read())
-fd.close()
-if core not in cores:
-cores.append(core)
-if socket not in sockets:
-sockets.append(socket)
-key = (socket, core)
-if key not in core_map:
-core_map[key] = []
-core_map[key].append(cpu)
-
-print(format("=" * (47 + len(base_path
-print("Core and Socket Information (as reported by '{}')".format(base_path))
-print("{}\n".format("=" * (47 + len(base_path
-print("cores = ", cores)
-print("sockets = ", sockets)
-print("")
-
-max_processor_len = len(str(len(cores) * len(sockets) * 2 - 1))
-max_thread_count = len(list(core_map.values())[0])
-max_core_map_len = (max_processor_len * max_thread_count) \
- + len(", ") * (max_thread_count - 1) \
- + len('[]') + len('Socket ')
-max_core_id_len = len(str(max(cores)))
-
-output = " ".ljust(max_core_id_len + len('Core '))
-for s in sockets:
-output += " Socket %s" % str(s).ljust(max_core_map_len - len('Socket '))
-print(output)
-
-output = " ".ljust(max_core_id_len + len('Core '))
-for s in sockets:
-output += " ".ljust(max_core_map_len)
-output += " "
-print(output)
-
-for c in cores:
-output = "Core %s" % str(c).ljust(max_core_id_len)
-for s in sockets:
-if (s, c) in core_map:
-output += " " + str(core_map[(s, c)]).ljust(max_core_map_len)
+from typing import List, Set, Dict, Tuple
Minor nit pick: recently I started using `import typing as T` and then
referencing T.List, T.Tuple, etc.
This avoids clobbering the patches when new symbols from the typing
module are required and using a short alias keeps the code readable.
+
+
+def _range_expand(rstr: str) -> List[int]:
def range_expand(rstr: str) -> T.List[int]:
+"""Expand a range string into a list of integers."""
+# 0,1-3 => [0, 1-3]
+ranges = rstr.split(",")
+valset: List[int] = []
+for r in ranges:
+# 1-3 => [1, 2, 3]
+if "-" in r:
+start, end = r.split("-")
+valset.extend(range(int(start), int(end) + 1))
else:
-output += " " * (max_core_map_len + 1)
-print(output)
+valset.append(int(r))
+return valset
+
+
+def _read_sysfs(path: str) -> str:
+with open(path, encoding="utf-8") as fd:
+return fd.read().strip()
+
+
+def _print_row(row: Tuple[str, ...], col_widths: List[int]) -> None:
def print_row(row: T.Tuple[str, ...], col_widths: T.List[int]) -> None:
+first, *rest = row
+w_first, *w_rest = col_widths
+first_end = " " * 4
+rest_end = " " * 10
+
+print(first.ljust(w_first), end=first_end)
+for cell, width in zip(rest, w_rest):
+print(cell.rjust(width), end=rest_end)
+print()
+
+
+def _print_section(heading: str) -> None:
+sep = "=" * len(heading)
+print(sep)
+print(heading)
+print(sep)
+print()
+
+
+def _main() -> None:
+sockets_s: Set[int] = set()
+cores_s: Set[int] = set()
+core_map: Dict[Tuple[int, int], List[int]] = {}
sockets_s: T.Set[int] = set()
cores_s: T.Set[int] = set()
core_map: T.Dict[Tuple[int, int], T.List[int]] = {}
+base_path = "/sys/devices/system/cpu"
+
+cpus = _range_expand(_read_sysfs(f"{base_path}/online"))
+
+for cpu in cpus:
+lcore_base = f"{base_path}/cpu{cpu}"
+core = int(_read_sysfs(f"{lcore_base}/topology/core_id"))
+socket = int(_read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+
+cores_s.add(core)
+sockets_s.add(socket)
+key = (socket, core)
+core_map.setdefault(key, [])
+core_map[key].append(cpu)
+
+cores = sorted(cores_s)
+sockets = sorted(sockets_s)
+
+_print_section("Core and Socket Information "
+ f"(as reported by '{base_path}')")
+
+print("cores = ", cores)
+print("sock
Re: [PATCH v2 2/4] usertools/cpu_layout: print out NUMA nodes
Anatoly Burakov, Aug 16, 2024 at 14:16:
In traditional NUMA case, NUMA nodes and physical sockets were used
interchangeably, but there are cases where there can be multiple NUMA
nodes per socket, as well as all CPU's being assigned NUMA node 0 even in
cases of multiple sockets. Use sysfs to print out NUMA information.
Signed-off-by: Anatoly Burakov
---
usertools/cpu_layout.py | 35 ++-
1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/usertools/cpu_layout.py b/usertools/cpu_layout.py
index be86f06938..e43bdbf343 100755
--- a/usertools/cpu_layout.py
+++ b/usertools/cpu_layout.py
@@ -4,6 +4,7 @@
# Copyright(c) 2017 Cavium, Inc. All rights reserved.
from typing import List, Set, Dict, Tuple
+import glob
Can you keep the import sorted alphabetically?
def _range_expand(rstr: str) -> List[int]:
@@ -26,11 +27,19 @@ def _read_sysfs(path: str) -> str:
return fd.read().strip()
+def _read_numa_node(base: str) -> int:
+node_glob = f"{base}/node*"
+node_dirs = glob.glob(node_glob)
+if not node_dirs:
+return 0 # default to node 0
+return int(node_dirs[0].split("node")[1])
Maybe you could make this safer and more "python"-ic as follows:
def read_numa_node(base: str) -> int:
for node in glob.iglob(f"{base}/node*"):
match = re.match(r"^.*/node(\d+)$", node)
if match:
return int(match.group(1))
return 0 # default to node 0
+
+
def _print_row(row: Tuple[str, ...], col_widths: List[int]) -> None:
first, *rest = row
w_first, *w_rest = col_widths
first_end = " " * 4
-rest_end = " " * 10
+rest_end = " " * 4
print(first.ljust(w_first), end=first_end)
for cell, width in zip(rest, w_rest):
@@ -50,6 +59,7 @@ def _main() -> None:
sockets_s: Set[int] = set()
cores_s: Set[int] = set()
core_map: Dict[Tuple[int, int], List[int]] = {}
+numa_map: Dict[int, int] = {}
base_path = "/sys/devices/system/cpu"
cpus = _range_expand(_read_sysfs(f"{base_path}/online"))
@@ -58,12 +68,14 @@ def _main() -> None:
lcore_base = f"{base_path}/cpu{cpu}"
core = int(_read_sysfs(f"{lcore_base}/topology/core_id"))
socket = int(_read_sysfs(f"{lcore_base}/topology/physical_package_id"))
+node = _read_numa_node(lcore_base)
cores_s.add(core)
sockets_s.add(socket)
key = (socket, core)
core_map.setdefault(key, [])
core_map[key].append(cpu)
+numa_map[cpu] = node
cores = sorted(cores_s)
sockets = sorted(sockets_s)
@@ -73,24 +85,37 @@ def _main() -> None:
print("cores = ", cores)
print("sockets = ", sockets)
+print("numa = ", sorted(set(numa_map.values(
print()
-# Core, [Socket, Socket, ...]
-heading_strs = "", *[f"Socket {s}" for s in sockets]
+# Core, [NUMA, Socket, NUMA, Socket, ...]
+heading_strs = "", *[v for s in sockets for v in ("", f"Socket {s}")]
sep_strs = tuple("-" * len(hstr) for hstr in heading_strs)
rows: List[Tuple[str, ...]] = []
+prev_numa = None
for c in cores:
# Core,
row: Tuple[str, ...] = (f"Core {c}",)
-# [lcores, lcores, ...]
+# assume NUMA changes symmetrically
+first_lcore = core_map[(0, c)][0]
+cur_numa = numa_map[first_lcore]
+numa_changed = prev_numa != cur_numa
+prev_numa = cur_numa
+
+# [NUMA, lcores, NUMA, lcores, ...]
for s in sockets:
try:
lcores = core_map[(s, c)]
+numa = numa_map[lcores[0]]
+if numa_changed:
+row += (f"NUMA {numa}",)
+else:
+row += ("",)
row += (str(lcores),)
except KeyError:
-row += ("",)
+row += ("", "")
rows += [row]
# find max widths for each column, including header and rows
--
2.43.5
Re: [PATCH v2 3/4] usertools/dpdk-hugepages.py: sort by NUMA node
Anatoly Burakov, Aug 16, 2024 at 14:16:
Currently, the list of per-NUMA node hugepages is displayed in glob order,
which can be arbitrary. Fix it to sort the glob order.
Signed-off-by: Anatoly Burakov
Hey Anatoly,
I mean no offense to anyone but dpdk-hugepages.py is really ugly :(
Is this script really needed? If it is, maybe it would be a good
opportunity to rewrite it. Is this something you'd be willing to work
on?
---
usertools/dpdk-hugepages.py | 40 ++---
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py
index bf2575ba36..54232ddf22 100755
--- a/usertools/dpdk-hugepages.py
+++ b/usertools/dpdk-hugepages.py
@@ -74,21 +74,37 @@ def set_hugepages(path, reqpages):
gotpages, reqpages, filename))
+def get_numa_pages_node(node):
+'''Read list of hugepage reservations on specific NUMA node'''
+hp_path = f'/sys/devices/system/node/node{node}/hugepages'
+if not os.path.exists(hp_path):
+return
+res = []
+for pg_sz_dir in os.listdir(hp_path):
+pg_sz = int(pg_sz_dir[10:-2])
+nr_pages = get_hugepages(f'{hp_path}/{pg_sz_dir}')
+if nr_pages > 0:
+pg_sz_str = fmt_memsize(pg_sz)
+total_sz_str = fmt_memsize(nr_pages * pg_sz)
+res += [(nr_pages, pg_sz_str, total_sz_str)]
+else:
+res += [(0, None, None)]
+return res
+
+
def show_numa_pages():
'''Show huge page reservations on Numa system'''
+# get list of NUMA nodes and sort them by integer order
print('Node Pages Size Total')
-for numa_path in glob.glob('/sys/devices/system/node/node*'):
-node = numa_path[29:] # slice after /sys/devices/system/node/node
-path = numa_path + '/hugepages'
-if not os.path.exists(path):
-continue
-for hdir in os.listdir(path):
-pages = get_hugepages(path + '/' + hdir)
-if pages > 0:
-kb = int(hdir[10:-2]) # slice out of hugepages-NNNkB
-print('{:<4} {:<5} {:<6} {}'.format(node, pages,
-fmt_memsize(kb),
-fmt_memsize(pages * kb)))
+nodes = sorted(int(node[29:])
+ for node in glob.glob('/sys/devices/system/node/node*'))
+for node in nodes:
+pg_sz_data = get_numa_pages_node(node)
+for nr_pages, pg_sz, total_sz in pg_sz_data:
+if not nr_pages:
+continue
+print('{:<4} {:<5} {:<6} {}'
+ .format(node, nr_pages, pg_sz, total_sz))
def show_non_numa_pages():
--
2.43.5
Re: [PATCH v2 4/4] usertools/dpdk-devbind: print NUMA node
Anatoly Burakov, Aug 16, 2024 at 14:16:
Currently, devbind does not print out any NUMA information, which makes
figuring out which NUMA node device belongs to not trivial. Add printouts
for NUMA information if NUMA support is enabled on the system.
Signed-off-by: Anatoly Burakov
---
Acked-by: Robin Jarry
NB: Although it is better than dpdk-hugepages.py, this script could also
benefit from a major cleanup as you did for cpu_layout.py.
usertools/dpdk-devbind.py | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py
index b276e8efc8..c0611a501d 100755
--- a/usertools/dpdk-devbind.py
+++ b/usertools/dpdk-devbind.py
@@ -110,6 +110,11 @@
args = []
+# check if this system has NUMA support
+def is_numa():
+return os.path.exists('/sys/devices/system/node')
+
+
# check if a specific kernel module is loaded
def module_is_loaded(module):
global loaded_modules
@@ -579,18 +584,24 @@ def show_device_status(devices_type, device_name,
if_field=False):
# print each category separately, so we can clearly see what's used by DPDK
if dpdk_drv:
+extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
+if is_numa():
+extra_param = "numa_node=%(NUMANode)s " + extra_param
display_devices("%s devices using DPDK-compatible driver" %
device_name,
-dpdk_drv, "drv=%(Driver_str)s unused=%(Module_str)s")
+dpdk_drv, extra_param)
if kernel_drv:
-if_text = ""
+extra_param = "drv=%(Driver_str)s unused=%(Module_str)s"
if if_field:
-if_text = "if=%(Interface)s "
-display_devices("%s devices using kernel driver" % device_name,
kernel_drv,
-if_text + "drv=%(Driver_str)s "
-"unused=%(Module_str)s %(Active)s")
+extra_param = "if=%(Interface)s " + extra_param
+if is_numa():
+extra_param = "numa_node=%(NUMANode)s " + extra_param
+display_devices("%s devices using kernel driver" % device_name,
+kernel_drv, extra_param)
if no_drv:
-display_devices("Other %s devices" % device_name, no_drv,
-"unused=%(Module_str)s")
+extra_param = "unused=%(Module_str)s"
+if is_numa():
+extra_param = "numa_node=%(NUMANode)s " + extra_param
+display_devices("Other %s devices" % device_name, no_drv, extra_param)
def show_status():
--
2.43.5
Re: Ethdev tracepoints optimization
On Mon, Aug 19, 2024 at 4:13 PM Ferruh Yigit wrote: > > On 8/15/2024 8:32 PM, Adel Belkhiri wrote: > > Hi DPDK Community, > > > > I am currently working on developing performance analyses for > > applications using the ethdev library. These analyses are being > > implemented in Trace Compass, an open-source performance analyzer. One > > of the views I’ve implemented shows the rate of traffic received or sent > > by an ethernet port, measured in packets per second. However, I've > > encountered an issue with the lib.ethdev.rx.burst event, which triggers > > even when no packets are polled, leading to a significant number of > > irrelevant events in the trace. This becomes problematic as these > > "empty" events can overwhelm the tracer buffer, potentially causing the > > loss of more critical events due to their high frequency. > > > > To address this, I've modified the DPDK code in lib/ethdev/rte_ethdev.h > > to add a conditional statement that only triggers the event when nb_rx > > > 0. My question to the community is whether there are use cases where an > > "empty" lib.ethdev.rx.burst event could be useful. If not, would there > > be interest in submitting a patch with this modification? > > > > Tracepoint is good way to get frequency of the calls, so I believe there > can be value of getting empty burst calls too. > > But your usecase also a valid one. I wonder if it works to have separate > trace calls, for empty and non-empty ones, and how this additional > branching impacts the performance, at least branch should be wrapped > with 'RTE_ENABLE_TRACE_FP' macro to not impact non-tracing usage. CTF(Common trace format) post processing tools can check the value for each field and timestap. So it will easy to skip the ones with no packet for this case. >
Re: Ethdev tracepoints optimization
On Mon, Aug 19, 2024 at 05:07:18PM +0530, Jerin Jacob wrote: > On Mon, Aug 19, 2024 at 4:13 PM Ferruh Yigit wrote: > > > > On 8/15/2024 8:32 PM, Adel Belkhiri wrote: > > > Hi DPDK Community, > > > > > > I am currently working on developing performance analyses for > > > applications using the ethdev library. These analyses are being > > > implemented in Trace Compass, an open-source performance analyzer. One > > > of the views I’ve implemented shows the rate of traffic received or sent > > > by an ethernet port, measured in packets per second. However, I've > > > encountered an issue with the lib.ethdev.rx.burst event, which triggers > > > even when no packets are polled, leading to a significant number of > > > irrelevant events in the trace. This becomes problematic as these > > > "empty" events can overwhelm the tracer buffer, potentially causing the > > > loss of more critical events due to their high frequency. > > > > > > To address this, I've modified the DPDK code in lib/ethdev/rte_ethdev.h > > > to add a conditional statement that only triggers the event when nb_rx > > > > 0. My question to the community is whether there are use cases where an > > > "empty" lib.ethdev.rx.burst event could be useful. If not, would there > > > be interest in submitting a patch with this modification? > > > > > > > Tracepoint is good way to get frequency of the calls, so I believe there > > can be value of getting empty burst calls too. > > > > But your usecase also a valid one. I wonder if it works to have separate > > trace calls, for empty and non-empty ones, and how this additional > > branching impacts the performance, at least branch should be wrapped > > with 'RTE_ENABLE_TRACE_FP' macro to not impact non-tracing usage. > > > CTF(Common trace format) post processing tools can check the value for > each field and timestap. > So it will easy to skip the ones with no packet for this case. > The trouble with empty polls is that they are very, very fast, so if no traffic is arriving you can quickly fill your entire trace buffer with nothing but empty polls. I believe that is the situation the original poster wanted to avoid. /Bruce
RE: Ethdev tracepoints optimization
> From: Jerin Jacob [mailto:jerinjac...@gmail.com] > > On Mon, Aug 19, 2024 at 4:13 PM Ferruh Yigit wrote: > > > > On 8/15/2024 8:32 PM, Adel Belkhiri wrote: > > > Hi DPDK Community, > > > > > > I am currently working on developing performance analyses for > > > applications using the ethdev library. These analyses are being > > > implemented in Trace Compass, an open-source performance analyzer. One > > > of the views I’ve implemented shows the rate of traffic received or sent > > > by an ethernet port, measured in packets per second. However, I've > > > encountered an issue with the lib.ethdev.rx.burst event, which triggers > > > even when no packets are polled, leading to a significant number of > > > irrelevant events in the trace. This becomes problematic as these > > > "empty" events can overwhelm the tracer buffer, potentially causing the > > > loss of more critical events due to their high frequency. > > > > > > To address this, I've modified the DPDK code in lib/ethdev/rte_ethdev.h > > > to add a conditional statement that only triggers the event when nb_rx > > > > 0. My question to the community is whether there are use cases where an > > > "empty" lib.ethdev.rx.burst event could be useful. If not, would there > > > be interest in submitting a patch with this modification? > > > > > > > Tracepoint is good way to get frequency of the calls, so I believe there > > can be value of getting empty burst calls too. Agree. The trace cannot generally omit empty burst calls. > > > > But your usecase also a valid one. I wonder if it works to have separate > > trace calls, for empty and non-empty ones, and how this additional > > branching impacts the performance, at least branch should be wrapped > > with 'RTE_ENABLE_TRACE_FP' macro to not impact non-tracing usage. > > > CTF(Common trace format) post processing tools can check the value for > each field and timestap. > So it will easy to skip the ones with no packet for this case. Doesn't solve the issue of the tracer itself being overwhelmed. Although I like Bruce's suggestion, which somewhat resembles Linux Kernel logging, I fear that it might be incompatible with automated tools processing the trace files afterwards. I think Ferruh's suggestion should work: Replace the current trace point with two separate trace points for respectively empty and non-empty calls: RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_rx_burst_empty, lib.ethdev.rx.burst.empty) RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_rx_burst_nonempty, lib.ethdev.rx.burst.nonempty) And update the code in lib/ethdev/rte_ethdev.h to call the appropriate of the two functions depending on nb_rx. PS: Something similar could be added to the eventdev library's rte_event_dequeue_burst() function. But that's a task for another day. :-)
Re: [PATCH v1] dts: correct typos in conf.yaml
Took me a few reads to even find the differences, haha. On Tue, Aug 13, 2024 at 1:17 PM Dean Marx wrote: > > correct docstring error in conf.yaml showing incorrect > example pci address for TG nodes > > Signed-off-by: Dean Marx Reviewed-by: Jeremy Spewock
Re: [PATCH v1 0/2] dts: port over checksum offload suite
Hi Dean, I went over the test suite and it looks really solid, but noticed you didn't update the `conf_yaml_schema.json`, is there any reason for this? On 8/16/24 15:20, Dean Marx wrote: Port over checksum hardware offload testing suite from old DTS. The suite verifies the ability of the PMD to recognize whether an incoming packet has valid or invalid L4/IP checksum values. v1: In the original test plan, there were two Tx checksum test cases. I removed them due to the lack of consistency in testpmd with Tx checksum flags, either not being displayed during packet transmission or showing values that did not align with the original test plan. Dean Marx (2): dts: add csum HW offload to testpmd shell dts: checksum offload test suite dts/framework/remote_session/testpmd_shell.py | 124 + dts/tests/TestSuite_checksum_offload.py | 255 ++ 2 files changed, 379 insertions(+) create mode 100644 dts/tests/TestSuite_checksum_offload.py Best regards, Alex
Re: [PATCH v1 2/2] dts: checksum offload test suite
Hi Dean, Just though I would point out a few things. On 8/16/24 15:20, Dean Marx wrote: +def send_packet_and_verify( Should this not be `send_packets_and_verify(` as the argument is `packet_list`. +isL4 = any( +VerboseOLFlag.RTE_MBUF_F_RX_L4_CKSUM_GOOD in packet.ol_flags +for index in verbose_output +for packet in index.packets +) How does this filter out noise packets with valid checksums? +isIP = any( +VerboseOLFlag.RTE_MBUF_F_RX_IP_CKSUM_GOOD in packet.ol_flags +for index in verbose_output +for packet in index.packets +) As above I also noticed there was no implementation of IPv6/SCTP. Although the old test suite did not include IPv6/SCTP, would it be worth adding it to the new test suite? It was included in the test plan, but never implemented for some reason. Best regards, Alex
Re: [PATCH v17 5/5] dts: add API doc generation
I ran into some dependency issues while testing that I figured I'd mention here. My build failed while running meson setup with the -Denable_docs=true option since I didn't have the sphinx-build module installed, then my compilation failed while running ninja -C because I didn't have a package called tomli installed. I ran a final time where compilation failed again because my system couldn't find the yaml package. It seems like nobody else ran into this so I'm a little confused if it's something on my end, but I tried running the optional poetry install --with docs mentioned in the cover letter and that didn't seem to work either. I was also able to build and compile without -Denable_docs. Thought I'd bring it up because compilation takes a fairly long time, and if a user runs into this I could see it being frustrating. Reviewed-by: Dean Marx
Re: [PATCH v1 1/2] usertools/cpu_layout: update coding style
On Mon, 19 Aug 2024 11:36:44 +0200 "Burakov, Anatoly" wrote: > On 8/19/2024 11:26 AM, Robin Jarry wrote: > > Anatoly Burakov, Aug 14, 2024 at 13:19: > >> Update coding style: > >> > >> - make it PEP-484 compliant > >> - address all flake8, mypy etc. warnings > >> - use f-strings in place of old-style string interpolation > >> - refactor printing to make the code more readable > >> > >> Signed-off-by: Anatoly Burakov > >> --- > > > > Hi Anatoly, > > > > thanks for the patch. Did you format the code using black/ruff? I'd like > > to start keeping python code formatting consistent across user tools. > > I don't think I did any formatting with a specific tool. Rather, my IDE > had flake8 set up to give me warnings if there are issues with > formatting, and it also does formatting, so this would effectively be > the same thing. I tend to use yapf3
Re: [PATCH v1 2/2] dts: checksum offload test suite
> > > > +def send_packet_and_verify( > > Should this not be `send_packets_and_verify(` as the argument is > `packet_list`. > Yeah that would definitely make more sense, I'll change that in the next version. > > > +isL4 = any( > > +VerboseOLFlag.RTE_MBUF_F_RX_L4_CKSUM_GOOD in packet.ol_flags > > +for index in verbose_output > > +for packet in index.packets > > +) > > How does this filter out noise packets with valid checksums? > It really doesn't right now, I was considering using another verbose output parameter to verify the packet matches the one that is sent, but I hadn't included it yet since I wasn't sure which would be best. I feel as though either length or src_mac would be the easiest to use, but if you or anyone else has a better idea let me know and I'll incorporate it. > > > +isIP = any( > > +VerboseOLFlag.RTE_MBUF_F_RX_IP_CKSUM_GOOD in packet.ol_flags > > +for index in verbose_output > > +for packet in index.packets > > +) > > > I also noticed there was no implementation of IPv6/SCTP. Although the > old test suite did not include IPv6/SCTP, would it be worth adding it to > the new test suite? > It was included in the test plan, but never implemented for some reason. > > I'm assuming you mean SCTP/IPv6 checksums, which I didn't see in any verbose output on any NIC I had access to so I just left it out. I was assuming that was the same reason they left it out of the old suite, as this included the i40e driver, and testpmd only ever displayed IP, L4, and outer checksums.
Re: [PATCH v1 0/2] dts: port over checksum offload suite
On Mon, Aug 19, 2024 at 10:28 AM Alex Chapman wrote: > Hi Dean, > > I went over the test suite and it looks really solid, but noticed you > didn't update the `conf_yaml_schema.json`, is there any reason for this? > Good catch, I must've messed up my git commits earlier. Thanks
Re: [PATCH v17 5/5] dts: add API doc generation
On Wed, Aug 14, 2024 at 11:05 AM Juraj Linkeš wrote: > The tool used to generate DTS API docs is Sphinx, which is already in > use in DPDK. The same configuration is used to preserve style with one > DTS-specific configuration (so that the DPDK docs are unchanged) that > modifies how the sidebar displays the content. There's other Sphinx > configuration related to Python docstrings which doesn't affect DPDK doc > build. All new configuration is in a conditional block, applied only > when DTS API docs are built to not interfere with DPDK doc build. > > Sphinx generates the documentation from Python docstrings. The docstring > format is the Google format [0] which requires the sphinx.ext.napoleon > extension. The other extension, sphinx.ext.intersphinx, enables linking > to objects in external documentations, such as the Python documentation. > > There is one requirement for building DTS docs - the same Python version > as DTS or higher, because Sphinx's autodoc extension imports the code. > > The dependencies needed to import the code don't have to be satisfied, > as the autodoc extension allows us to mock the imports. The missing > packages are taken from the DTS pyproject.toml file. > > And finally, the DTS API docs can be accessed from the DPDK API doxygen > page. > > [0] > https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings > > Signed-off-by: Juraj Linkeš > Tested-by: Dean Marx
Re: [PATCH v17 5/5] dts: add API doc generation
On Mon, Aug 19, 2024 at 10:37 AM Dean Marx wrote: > I ran into some dependency issues while testing that I figured I'd mention > here. My build failed while running meson setup with the -Denable_docs=true > option since I didn't have the sphinx-build module installed, then my > compilation failed while running ninja -C because I didn't have a package > called tomli installed. I ran a final time where compilation failed again > because my system couldn't find the yaml package. It seems like nobody else > ran into this so I'm a little confused if it's something on my end, but I > tried running the optional poetry install --with docs mentioned in the > cover letter and that didn't seem to work either. I was also able to > build and compile without -Denable_docs. Thought I'd bring it up because > compilation takes a fairly long time, and if a user runs into this I could > see it being frustrating. > > Reviewed-by: Dean Marx > Just worked this out with Jeremy, I was running the poetry install --with docs in the DPDK directory instead of the DTS subdirectory. However, while that fixes almost everything, the yaml module is never imported and was throwing errors for me until I installed pyyaml manually, so this might have been missed in the dependency list
