<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);" class="elementToProof">
Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
<div><br>
</div>
<div>But (and I'll bring this up again on patch 5/5, where it's more important) it feels like this is just the beginning of a much longer patch series, and I'd like to see the whole series before buying the initial patches. It'd be nice to see a more detailed
picture of what one wants to use maps of maps for... how they'll be used, what problems one intends to solve, etc.</div>
<div><br>
</div>
<div>In the commit msg, how about</div>
<div> s/indexed aggregations/aggregation keys/</div>
<div>The documentation, when discussing aggregations, typically (almost always) speaks of aggregation keys. We should be consistent with our documentation.</div>
<div><br>
</div>
<div>How about allowing NULL names?</div>
<br>
</div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Kris Van Hees via DTrace-devel <dtrace-devel@oss.oracle.com><br>
<b>Sent:</b> Friday, August 19, 2022 10:26 AM<br>
<b>To:</b> dtrace-devel@oss.oracle.com <dtrace-devel@oss.oracle.com><br>
<b>Subject:</b> [DTrace-devel] [PATCH 2/5] Implement dt_bpf_map_create_meta() and create_gmap_of_maps()</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">The indexed aggregation support will make use of the map-of-maps<br>
structure that BPF supports. The dt_bpf_map_create_meta() function<br>
provides the low level implementation and create_gmap_of_maps()<br>
provides a function with argument checking and error reporting.<br>
<br>
This patch also cleans up some coding style issues with bpf_*()<br>
functions (and it declares local-use-only functions static).<br>
<br>
Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com><br>
---<br>
libdtrace/dt_bpf.c | 118 ++++++++++++++++++++++++++++++++++++++++++---<br>
1 file changed, 110 insertions(+), 8 deletions(-)<br>
<br>
diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c<br>
index 720fe02d..e92d34ee 100644<br>
--- a/libdtrace/dt_bpf.c<br>
+++ b/libdtrace/dt_bpf.c<br>
@@ -69,7 +69,8 @@ dt_bpf_lockmem_error(dtrace_hdl_t *dtp, const char *msg)<br>
/*<br>
* Load a BPF program into the kernel.<br>
*/<br>
-int dt_bpf_prog_load(enum bpf_prog_type prog_type, const dtrace_difo_t *dp,<br>
+static int<br>
+dt_bpf_prog_load(enum bpf_prog_type prog_type, const dtrace_difo_t *dp,<br>
uint32_t log_level, char *log_buf, size_t log_buf_sz)<br>
{<br>
union bpf_attr attr;<br>
@@ -99,9 +100,10 @@ int dt_bpf_prog_load(enum bpf_prog_type prog_type, const dtrace_difo_t *dp,<br>
/*<br>
* Create a named BPF map.<br>
*/<br>
-int dt_bpf_map_create(enum bpf_map_type map_type, const char *name,<br>
- uint32_t key_size, uint32_t value_size,<br>
- uint32_t max_entries, uint32_t map_flags)<br>
+static int<br>
+dt_bpf_map_create(enum bpf_map_type map_type, const char *name,<br>
+ uint32_t key_size, uint32_t value_size, uint32_t max_entries,<br>
+ uint32_t map_flags)<br>
{<br>
union bpf_attr attr;<br>
<br>
@@ -117,10 +119,55 @@ int dt_bpf_map_create(enum bpf_map_type map_type, const char *name,<br>
return bpf(BPF_MAP_CREATE, &attr);<br>
}<br>
<br>
+/*<br>
+ * Create a named BPF meta map (array of maps or hash of maps).<br>
+ */<br>
+static int<br>
+dt_bpf_map_create_meta(enum bpf_map_type otype, const char *name,<br>
+ uint32_t oksz, uint32_t osize, uint32_t oflags,<br>
+ enum bpf_map_type itype,<br>
+ uint32_t iksz, uint32_t ivsz, uint32_t isize,<br>
+ uint32_t iflags)<br>
+{<br>
+ union bpf_attr attr;<br>
+ int ifd, ofd;<br>
+<br>
+ /* Create (temporary) inner map as template. */<br>
+ memset(&attr, 0, sizeof(attr));<br>
+ attr.map_type = itype;<br>
+ attr.key_size = iksz;<br>
+ attr.value_size = ivsz;<br>
+ attr.max_entries = isize;<br>
+ attr.map_flags = iflags;<br>
+<br>
+ ifd = bpf(BPF_MAP_CREATE, &attr);<br>
+ if (ifd < 0)<br>
+ return -1;<br>
+<br>
+ /* Create the map-of-maps. */<br>
+ memset(&attr, 0, sizeof(attr));<br>
+ attr.map_type = otype;<br>
+ attr.key_size = oksz;<br>
+ attr.value_size = sizeof(uint32_t);<br>
+ attr.inner_map_fd = ifd;<br>
+ attr.max_entries = osize;<br>
+ attr.map_flags = oflags;<br>
+ memcpy(attr.map_name, name, MIN(strlen(name),<br>
+ sizeof(attr.map_name) - 1));<br>
+<br>
+ ofd = bpf(BPF_MAP_CREATE, &attr);<br>
+<br>
+ /* Done with the template map. */<br>
+ close(ifd);<br>
+<br>
+ return ofd;<br>
+}<br>
+<br>
/*<br>
* Load the value for the given key in the map referenced by the given fd.<br>
*/<br>
-int dt_bpf_map_lookup(int fd, const void *key, void *val)<br>
+int<br>
+dt_bpf_map_lookup(int fd, const void *key, void *val)<br>
{<br>
union bpf_attr attr;<br>
<br>
@@ -135,7 +182,8 @@ int dt_bpf_map_lookup(int fd, const void *key, void *val)<br>
/*<br>
* Find the next key after the given key in the map referenced by the given fd.<br>
*/<br>
-int dt_bpf_map_next_key(int fd, const void *key, void *nxt)<br>
+int<br>
+dt_bpf_map_next_key(int fd, const void *key, void *nxt)<br>
{<br>
union bpf_attr attr;<br>
<br>
@@ -150,7 +198,8 @@ int dt_bpf_map_next_key(int fd, const void *key, void *nxt)<br>
/*<br>
* Delete the given key from the map referenced by the given fd.<br>
*/<br>
-int dt_bpf_map_delete(int fd, const void *key)<br>
+int<br>
+dt_bpf_map_delete(int fd, const void *key)<br>
{<br>
union bpf_attr attr;<br>
<br>
@@ -164,7 +213,8 @@ int dt_bpf_map_delete(int fd, const void *key)<br>
/*<br>
* Store the (key, value) pair in the map referenced by the given fd.<br>
*/<br>
-int dt_bpf_map_update(int fd, const void *key, const void *val)<br>
+int<br>
+dt_bpf_map_update(int fd, const void *key, const void *val)<br>
{<br>
union bpf_attr attr;<br>
<br>
@@ -286,6 +336,58 @@ create_gmap(dtrace_hdl_t *dtp, const char *name, enum bpf_map_type type,<br>
return fd;<br>
}<br>
<br>
+static int<br>
+create_gmap_of_maps(dtrace_hdl_t *dtp, const char *name,<br>
+ enum bpf_map_type otype, size_t oksz, size_t osize,<br>
+ enum bpf_map_type itype, size_t iksz, size_t ivsz,<br>
+ size_t isize)<br>
+{<br>
+ int fd, err;<br>
+ dt_ident_t *idp;<br>
+<br>
+ dt_dprintf("Creating BPF meta map '%s' (ksz %lu, sz %lu)\n",<br>
+ name, oksz, osize);<br>
+ dt_dprintf(" storing BPF maps (ksz %lu, vsz %lu, sz %lu)\n",<br>
+ iksz, ivsz, isize);<br>
+<br>
+ if (oksz > UINT_MAX || osize > UINT_MAX ||<br>
+ iksz > UINT_MAX || ivsz > UINT_MAX || isize > UINT_MAX) {<br>
+ fd = -1;<br>
+ err = E2BIG;<br>
+ } else {<br>
+ fd = dt_bpf_map_create_meta(otype, name, oksz, osize, 0,<br>
+ itype, iksz, ivsz, isize, 0);<br>
+ err = errno;<br>
+ }<br>
+<br>
+ if (fd < 0) {<br>
+ char msg[64];<br>
+<br>
+ snprintf(msg, sizeof(msg),<br>
+ "failed to create BPF map '%s'", name);<br>
+ if (err == E2BIG)<br>
+ return dt_bpf_error(dtp, "%s: Too big\n", msg);<br>
+ if (err == EPERM)<br>
+ return dt_bpf_lockmem_error(dtp, msg);<br>
+ return dt_bpf_error(dtp, "%s: %s\n", msg, strerror(err));<br>
+ }<br>
+<br>
+ dt_dprintf("BPF map '%s' is FD %d\n", name, fd);<br>
+<br>
+ /*<br>
+ * Assign the fd as id for the BPF map identifier.<br>
+ */<br>
+ idp = dt_dlib_get_map(dtp, name);<br>
+ if (idp == NULL) {<br>
+ close(fd);<br>
+ return dt_bpf_error(dtp, "cannot find BPF map '%s'\n", name);<br>
+ }<br>
+<br>
+ dt_ident_set_id(idp, fd);<br>
+<br>
+ return fd;<br>
+}<br>
+<br>
/*<br>
* Create the 'state' BPF map.<br>
*<br>
-- <br>
2.34.1<br>
<br>
<br>
_______________________________________________<br>
DTrace-devel mailing list<br>
DTrace-devel@oss.oracle.com<br>
<a href="https://oss.oracle.com/mailman/listinfo/dtrace-devel">https://oss.oracle.com/mailman/listinfo/dtrace-devel</a><br>
</div>
</span></font></div>
</body>
</html>