Messages in this thread Patch in this message | | | From | Daniel Colascione <> | Date | Sun, 29 Jul 2018 13:46:44 -0700 | Subject | Re: [PATCH v2] Add BPF_SYNCHRONIZE_MAPS bpf(2) command |
| |
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sun, Jul 29, 2018 at 8:51 AM, Alexei Starovoitov <span dir="ltr"><<a href="mailto:alexei.starovoitov@gmail.com" target="_blank">alexei.starovoitov@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Thu, Jul 26, 2018 at 7:51 PM, Daniel Colascione <<a href="mailto:dancol@google.com">dancol@google.com</a>> wrote:<br> > BPF_SYNCHRONIZE_MAPS waits for the release of any references to a BPF<br> > map made by a BPF program that is running at the time the<br> > BPF_SYNCHRONIZE_MAPS command is issued. The purpose of this command is<br> > to provide a means for userspace to replace a BPF map with another,<br> > newer version, then ensure that no component is still using the "old"<br> > map before manipulating the "old" map in some way.<br> ><br> > Signed-off-by: Daniel Colascione <<a href="mailto:dancol@google.com">dancol@google.com</a>><br> > ---<br> > include/uapi/linux/bpf.h | 9 +++++++++<br> > kernel/bpf/syscall.c | 13 +++++++++++++<br> > 2 files changed, 22 insertions(+)<br> ><br> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h<br> > index b7db3261c62d..5b27e9117d3e 100644<br> > --- a/include/uapi/linux/bpf.h<br> > +++ b/include/uapi/linux/bpf.h<br> > @@ -75,6 +75,14 @@ struct bpf_lpm_trie_key {<br> > __u8 data[0]; /* Arbitrary size */<br> > };<br> ><br> > +/* BPF_SYNCHRONIZE_MAPS waits for the release of any references to a<br> > + * BPF map made by a BPF program that is running at the time the<br> > + * BPF_SYNCHRONIZE_MAPS command is issued. The purpose of this command<br> <br> </span>that doesn't sound right to me.<br> such command won't wait for the release of the references.<br> in case of map-in-map the program does not hold<br> the references to inner map (only to outer map).<br></blockquote><div><br></div><div>Well, today that's the guarantee it provides. :-) I figured it'd be simpler to explain the guarantee this way.</div><div><br></div><div>How about "waits for the release of any reference to a map obtained from another map"?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <span class=""><br> > + * is to provide a means for userspace to replace a BPF map with<br> > + * another, newer version, then ensure that no component is still<br> > + * using the "old" map before manipulating the "old" map in some way.<br> > + */<br> <br> </span>that's correct, but the whole paragraph still reads too<br> 'generic' which will lead to confusion,<br> whereas the use case is map-in-map only.<br> I think bpf_sync_inner_map or<br> bpf_sync_map_in_map would be better<br> choices for the name.</blockquote><div><br></div><div>I worry that a name like that would be too specific.</div></div></div></div>
| |