| Message ID | 20231222122146.65519-3-lulie@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | bpf: introduce BPF_MAP_TYPE_RELAY | expand |
On Fri, Dec 22, 2023 at 08:21:45PM +0800, Philo Lu wrote: > map_update_elem is used to create relay files and bind them with the > relay channel, which is created with BPF_MAP_CREATE. This allows users > to set a custom directory name. It must be used with key=NULL and > flag=0. > > Here is an example: > ``` > struct { > __uint(type, BPF_MAP_TYPE_RELAY); > __uint(max_entries, 4096); > } my_relay SEC(".maps"); > ... > char dir_name[] = "relay_test"; > bpf_map_update_elem(map_fd, NULL, dir_name, 0); > ``` > > Then, directory `/sys/kerenl/debug/relay_test` will be created, which > includes files of my_relay0...my_relay[#cpu]. Each represents a per-cpu > buffer with size 8 * 4096 B (there are 8 subbufs by default, each with > size 4096B). > > Signed-off-by: Philo Lu <lulie@linux.alibaba.com> > --- > kernel/bpf/relaymap.c | 32 +++++++++++++++++++++++++++++++- > 1 file changed, 31 insertions(+), 1 deletion(-) > > diff --git a/kernel/bpf/relaymap.c b/kernel/bpf/relaymap.c > index d0adc7f67758..588c8de0a4bd 100644 > --- a/kernel/bpf/relaymap.c > +++ b/kernel/bpf/relaymap.c > @@ -117,7 +117,37 @@ static void *relay_map_lookup_elem(struct bpf_map *map, void *key) > static long relay_map_update_elem(struct bpf_map *map, void *key, void *value, > u64 flags) > { > - return -EOPNOTSUPP; > + struct bpf_relay_map *rmap; > + struct dentry *parent; > + int err; > + > + if (unlikely(flags)) > + return -EINVAL; > + > + if (unlikely(key)) > + return -EINVAL; > + > + rmap = container_of(map, struct bpf_relay_map, map); > + > + /* The directory already exists */ > + if (rmap->relay_chan->has_base_filename) > + return -EEXIST; > + > + /* Setup relay files. Note that the directory name passed as value should > + * not be longer than map->value_size, including the '\0' at the end. > + */ > + ((char *)value)[map->value_size - 1] = '\0'; > + parent = debugfs_create_dir(value, NULL); > + if (IS_ERR_OR_NULL(parent)) > + return PTR_ERR(parent); > + > + err = relay_late_setup_files(rmap->relay_chan, map->name, parent); > + if (err) { > + debugfs_remove_recursive(parent); > + return err; > + } looks like this patch could be moved to the previous one? jirka > + > + return 0; > } > > static long relay_map_delete_elem(struct bpf_map *map, void *key) > -- > 2.32.0.3.g01195cf9f > >
On 2023/12/22 22:45, Jiri Olsa wrote: > On Fri, Dec 22, 2023 at 08:21:45PM +0800, Philo Lu wrote: >> map_update_elem is used to create relay files and bind them with the >> relay channel, which is created with BPF_MAP_CREATE. This allows users >> to set a custom directory name. It must be used with key=NULL and >> flag=0. >> >> Here is an example: >> ``` >> struct { >> __uint(type, BPF_MAP_TYPE_RELAY); >> __uint(max_entries, 4096); >> } my_relay SEC(".maps"); >> ... >> char dir_name[] = "relay_test"; >> bpf_map_update_elem(map_fd, NULL, dir_name, 0); >> ``` >> >> Then, directory `/sys/kerenl/debug/relay_test` will be created, which >> includes files of my_relay0...my_relay[#cpu]. Each represents a per-cpu >> buffer with size 8 * 4096 B (there are 8 subbufs by default, each with >> size 4096B). >> >> Signed-off-by: Philo Lu <lulie@linux.alibaba.com> >> --- >> kernel/bpf/relaymap.c | 32 +++++++++++++++++++++++++++++++- >> 1 file changed, 31 insertions(+), 1 deletion(-) >> >> diff --git a/kernel/bpf/relaymap.c b/kernel/bpf/relaymap.c >> index d0adc7f67758..588c8de0a4bd 100644 >> --- a/kernel/bpf/relaymap.c >> +++ b/kernel/bpf/relaymap.c >> @@ -117,7 +117,37 @@ static void *relay_map_lookup_elem(struct bpf_map *map, void *key) >> static long relay_map_update_elem(struct bpf_map *map, void *key, void *value, >> u64 flags) >> { >> - return -EOPNOTSUPP; >> + struct bpf_relay_map *rmap; >> + struct dentry *parent; >> + int err; >> + >> + if (unlikely(flags)) >> + return -EINVAL; >> + >> + if (unlikely(key)) >> + return -EINVAL; >> + >> + rmap = container_of(map, struct bpf_relay_map, map); >> + >> + /* The directory already exists */ >> + if (rmap->relay_chan->has_base_filename) >> + return -EEXIST; >> + >> + /* Setup relay files. Note that the directory name passed as value should >> + * not be longer than map->value_size, including the '\0' at the end. >> + */ >> + ((char *)value)[map->value_size - 1] = '\0'; >> + parent = debugfs_create_dir(value, NULL); >> + if (IS_ERR_OR_NULL(parent)) >> + return PTR_ERR(parent); >> + >> + err = relay_late_setup_files(rmap->relay_chan, map->name, parent); >> + if (err) { >> + debugfs_remove_recursive(parent); >> + return err; >> + } > > looks like this patch could be moved to the previous one? > OK, will do it. > jirka > >> + >> + return 0; >> } >> >> static long relay_map_delete_elem(struct bpf_map *map, void *key) >> -- >> 2.32.0.3.g01195cf9f >> >>
Hi, On 12/22/2023 8:21 PM, Philo Lu wrote: > map_update_elem is used to create relay files and bind them with the > relay channel, which is created with BPF_MAP_CREATE. This allows users > to set a custom directory name. It must be used with key=NULL and > flag=0. > > Here is an example: > ``` > struct { > __uint(type, BPF_MAP_TYPE_RELAY); > __uint(max_entries, 4096); > } my_relay SEC(".maps"); > ... > char dir_name[] = "relay_test"; > bpf_map_update_elem(map_fd, NULL, dir_name, 0); > ``` > > Then, directory `/sys/kerenl/debug/relay_test` will be created, which > includes files of my_relay0...my_relay[#cpu]. Each represents a per-cpu > buffer with size 8 * 4096 B (there are 8 subbufs by default, each with > size 4096B). It is a little weird. Because the name of the relay file is $debug_fs_root/$value_name/${map_name}xxx. Could we update it to $debug_fs_root/$map_name/$value_name/xxx instead ? > Signed-off-by: Philo Lu <lulie@linux.alibaba.com> > --- > kernel/bpf/relaymap.c | 32 +++++++++++++++++++++++++++++++- > 1 file changed, 31 insertions(+), 1 deletion(-) > > diff --git a/kernel/bpf/relaymap.c b/kernel/bpf/relaymap.c > index d0adc7f67758..588c8de0a4bd 100644 > --- a/kernel/bpf/relaymap.c > +++ b/kernel/bpf/relaymap.c > @@ -117,7 +117,37 @@ static void *relay_map_lookup_elem(struct bpf_map *map, void *key) > static long relay_map_update_elem(struct bpf_map *map, void *key, void *value, > u64 flags) > { > - return -EOPNOTSUPP; > + struct bpf_relay_map *rmap; > + struct dentry *parent; > + int err; > + > + if (unlikely(flags)) > + return -EINVAL; > + > + if (unlikely(key)) > + return -EINVAL; > + > + rmap = container_of(map, struct bpf_relay_map, map); > + Lock is needed here, because .map_update_elem can be invoked concurrently. > + /* The directory already exists */ > + if (rmap->relay_chan->has_base_filename) > + return -EEXIST; > + > + /* Setup relay files. Note that the directory name passed as value should > + * not be longer than map->value_size, including the '\0' at the end. > + */ > + ((char *)value)[map->value_size - 1] = '\0'; > + parent = debugfs_create_dir(value, NULL); > + if (IS_ERR_OR_NULL(parent)) > + return PTR_ERR(parent); > + > + err = relay_late_setup_files(rmap->relay_chan, map->name, parent); > + if (err) { > + debugfs_remove_recursive(parent); > + return err; > + } > + > + return 0; > } > > static long relay_map_delete_elem(struct bpf_map *map, void *key)
On 2023/12/23 19:28, Hou Tao wrote: > Hi, > > On 12/22/2023 8:21 PM, Philo Lu wrote: >> map_update_elem is used to create relay files and bind them with the >> relay channel, which is created with BPF_MAP_CREATE. This allows users >> to set a custom directory name. It must be used with key=NULL and >> flag=0. >> >> Here is an example: >> ``` >> struct { >> __uint(type, BPF_MAP_TYPE_RELAY); >> __uint(max_entries, 4096); >> } my_relay SEC(".maps"); >> ... >> char dir_name[] = "relay_test"; >> bpf_map_update_elem(map_fd, NULL, dir_name, 0); >> ``` >> >> Then, directory `/sys/kerenl/debug/relay_test` will be created, which >> includes files of my_relay0...my_relay[#cpu]. Each represents a per-cpu >> buffer with size 8 * 4096 B (there are 8 subbufs by default, each with >> size 4096B). > > It is a little weird. Because the name of the relay file is > $debug_fs_root/$value_name/${map_name}xxx. Could we update it to > $debug_fs_root/$map_name/$value_name/xxx instead ? I think a unique directory is enough for a relay map, so currently users can use map_update_elem to set the directory name. Thus $map_name/${value_name}xxx may be better than $map_name/$value_name/xxx. As for whether map_name or value_name is better to be used as the directory name, I think it more likely that different bpf programs share a same map_name. So value_name is currently used. >> Signed-off-by: Philo Lu <lulie@linux.alibaba.com> >> --- >> kernel/bpf/relaymap.c | 32 +++++++++++++++++++++++++++++++- >> 1 file changed, 31 insertions(+), 1 deletion(-) >> >> diff --git a/kernel/bpf/relaymap.c b/kernel/bpf/relaymap.c >> index d0adc7f67758..588c8de0a4bd 100644 >> --- a/kernel/bpf/relaymap.c >> +++ b/kernel/bpf/relaymap.c >> @@ -117,7 +117,37 @@ static void *relay_map_lookup_elem(struct bpf_map *map, void *key) >> static long relay_map_update_elem(struct bpf_map *map, void *key, void *value, >> u64 flags) >> { >> - return -EOPNOTSUPP; >> + struct bpf_relay_map *rmap; >> + struct dentry *parent; >> + int err; >> + >> + if (unlikely(flags)) >> + return -EINVAL; >> + >> + if (unlikely(key)) >> + return -EINVAL; >> + >> + rmap = container_of(map, struct bpf_relay_map, map); >> + > > Lock is needed here, because .map_update_elem can be invoked concurrently. Got it. I will fix it in the next version. Thanks. >> + /* The directory already exists */ >> + if (rmap->relay_chan->has_base_filename) >> + return -EEXIST; >> + >> + /* Setup relay files. Note that the directory name passed as value should >> + * not be longer than map->value_size, including the '\0' at the end. >> + */ >> + ((char *)value)[map->value_size - 1] = '\0'; >> + parent = debugfs_create_dir(value, NULL); >> + if (IS_ERR_OR_NULL(parent)) >> + return PTR_ERR(parent); >> + >> + err = relay_late_setup_files(rmap->relay_chan, map->name, parent); >> + if (err) { >> + debugfs_remove_recursive(parent); >> + return err; >> + } >> + >> + return 0; >> } >> >> static long relay_map_delete_elem(struct bpf_map *map, void *key)
diff --git a/kernel/bpf/relaymap.c b/kernel/bpf/relaymap.c index d0adc7f67758..588c8de0a4bd 100644 --- a/kernel/bpf/relaymap.c +++ b/kernel/bpf/relaymap.c @@ -117,7 +117,37 @@ static void *relay_map_lookup_elem(struct bpf_map *map, void *key) static long relay_map_update_elem(struct bpf_map *map, void *key, void *value, u64 flags) { - return -EOPNOTSUPP; + struct bpf_relay_map *rmap; + struct dentry *parent; + int err; + + if (unlikely(flags)) + return -EINVAL; + + if (unlikely(key)) + return -EINVAL; + + rmap = container_of(map, struct bpf_relay_map, map); + + /* The directory already exists */ + if (rmap->relay_chan->has_base_filename) + return -EEXIST; + + /* Setup relay files. Note that the directory name passed as value should + * not be longer than map->value_size, including the '\0' at the end. + */ + ((char *)value)[map->value_size - 1] = '\0'; + parent = debugfs_create_dir(value, NULL); + if (IS_ERR_OR_NULL(parent)) + return PTR_ERR(parent); + + err = relay_late_setup_files(rmap->relay_chan, map->name, parent); + if (err) { + debugfs_remove_recursive(parent); + return err; + } + + return 0; } static long relay_map_delete_elem(struct bpf_map *map, void *key)
map_update_elem is used to create relay files and bind them with the relay channel, which is created with BPF_MAP_CREATE. This allows users to set a custom directory name. It must be used with key=NULL and flag=0. Here is an example: ``` struct { __uint(type, BPF_MAP_TYPE_RELAY); __uint(max_entries, 4096); } my_relay SEC(".maps"); ... char dir_name[] = "relay_test"; bpf_map_update_elem(map_fd, NULL, dir_name, 0); ``` Then, directory `/sys/kerenl/debug/relay_test` will be created, which includes files of my_relay0...my_relay[#cpu]. Each represents a per-cpu buffer with size 8 * 4096 B (there are 8 subbufs by default, each with size 4096B). Signed-off-by: Philo Lu <lulie@linux.alibaba.com> --- kernel/bpf/relaymap.c | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-)