introduce map.lookup_or_try_init() (iovisor#2577)

Previously, map.lookup_or_init() may cause unexpected
return from the function when lookup finds no element and
init failed e.g. due to unlikely racy update or
sometimes hash table full.
This has caught surprise from many users. So, the commit
  iovisor@ba64f03
attempts to remove the early return in map.lookup_or_init().
But then since NULL pointer could be returned,
user will need to change their bpf program to check return value,
otherwise, verifier will reject the program.

As described in the above, such an API behavior change may cause
verifier failure and reject previously loadable bpf programs.
bcc should try to maintain API stability, esp. to avoid subtle
API behavior change.

This patch propose to restore the behavior of map.lookup_or_init()
and introduce a new one map.lookup_or_try_init(), which will
avoid unexpected return. The name is suggested by Alexei
to reflect that init may fail. map.lookup_or_try_init() will be formally
documented and used in bcc. A warning will be generated if
map.lookup_or_init() is used. Documentation will make it clear
that map.lookup_or_try_init() is preferred over map.lookup_or_init().

```
-bash-4.4$ sudo ./syscount.py
/virtual/main.c:71:11: warning: lookup_or_init() may return from the function, use loopup_or_try_init() instead.
    val = data.lookup_or_init(&key, &zero);
          ^
1 warning generated.
Tracing syscalls, printing top 10... Ctrl+C to quit.
...
```

All uses in examples and tools are converted to use
lookup_or_try_init(). Most tests are converted to use
lookup_or_try_init() too except test_trace_maxactive.py
and test_tracepoint.py to test lookup_or_init()
functionality.

docs/reference_guide.md

@@ -45,7 +45,7 @@ This guide is incomplete. If something feels missing, check the bcc and kernel s
         - [10. BPF_DEVMAP](#10-bpf_devmap)
         - [11. BPF_CPUMAP](#11-bpf_cpumap)
         - [12. map.lookup()](#12-maplookup)
-        - [13. map.lookup_or_init()](#13-maplookup_or_init)
+        - [13. map.lookup_or_try_init()](#13-maplookup_or_try_init)
         - [14. map.delete()](#14-mapdelete)
         - [15. map.update()](#15-mapupdate)
         - [16. map.insert()](#16-mapinsert)
@@ -537,7 +537,7 @@ Syntax: ```BPF_TABLE(_table_type, _key_type, _leaf_type, _name, _max_entries)```
 
 Creates a map named ```_name```. Most of the time this will be used via higher-level macros, like BPF_HASH, BPF_HIST, etc.
 
-Methods (covered later): map.lookup(), map.lookup_or_init(), map.delete(), map.update(), map.insert(), map.increment().
+Methods (covered later): map.lookup(), map.lookup_or_try_init(), map.delete(), map.update(), map.insert(), map.increment().
 
 Examples in situ:
 [search /examples](https://github.com/iovisor/bcc/search?q=BPF_TABLE+path%3Aexamples&type=Code),
@@ -570,7 +570,7 @@ BPF_HASH(start, struct request *);
 
 This creates a hash named ```start``` where the key is a ```struct request *```, and the value defaults to u64. This hash is used by the disksnoop.py example for saving timestamps for each I/O request, where the key is the pointer to struct request, and the value is the timestamp.
 
-Methods (covered later): map.lookup(), map.lookup_or_init(), map.delete(), map.update(), map.insert(), map.increment().
+Methods (covered later): map.lookup(), map.lookup_or_try_init(), map.delete(), map.update(), map.insert(), map.increment().
 
 Examples in situ:
 [search /examples](https://github.com/iovisor/bcc/search?q=BPF_HASH+path%3Aexamples&type=Code),
@@ -705,7 +705,7 @@ BPF_LPM_TRIE(trie, struct key_v6);
 
 This creates an LPM trie map named `trie` where the key is a `struct key_v6`, and the value defaults to u64.
 
-Methods (covered later): map.lookup(), map.lookup_or_init(), map.delete(), map.update(), map.insert(), map.increment().
+Methods (covered later): map.lookup(), map.lookup_or_try_init(), map.delete(), map.update(), map.insert(), map.increment().
 
 Examples in situ:
 [search /examples](https://github.com/iovisor/bcc/search?q=BPF_LPM_TRIE+path%3Aexamples&type=Code),
@@ -766,15 +766,18 @@ Examples in situ:
 [search /examples](https://github.com/iovisor/bcc/search?q=lookup+path%3Aexamples&type=Code),
 [search /tools](https://github.com/iovisor/bcc/search?q=lookup+path%3Atools&type=Code)
 
-### 13. map.lookup_or_init()
+### 13. map.lookup_or_try_init()
 
-Syntax: ```*val map.lookup_or_init(&key, &zero)```
+Syntax: ```*val map.lookup_or_try_init(&key, &zero)```
 
 Lookup the key in the map, and return a pointer to its value if it exists, else initialize the key's value to the second argument. This is often used to initialize values to zero. If the key cannot be inserted (e.g. the map is full) then NULL is returned.
 
 Examples in situ:
-[search /examples](https://github.com/iovisor/bcc/search?q=lookup_or_init+path%3Aexamples&type=Code),
-[search /tools](https://github.com/iovisor/bcc/search?q=lookup_or_init+path%3Atools&type=Code)
+[search /examples](https://github.com/iovisor/bcc/search?q=lookup_or_try_init+path%3Aexamples&type=Code),
+[search /tools](https://github.com/iovisor/bcc/search?q=lookup_or_try_init+path%3Atools&type=Code)
+
+Note: The old map.lookup_or_init() may cause return from the function, so lookup_or_try_init() is recommended as it
+does not have this side effect.
 
 ### 14. map.delete()
 

docs/tutorial_bcc_python_developer.md

@@ -558,7 +558,7 @@ int count(struct pt_regs *ctx) {
 
     bpf_probe_read(&key.c, sizeof(key.c), (void *)PT_REGS_PARM1(ctx));
     // could also use `counts.increment(key)`
-    val = counts.lookup_or_init(&key, &zero);
+    val = counts.lookup_or_try_init(&key, &zero);
     if (val) {
       (*val)++;
     }
@@ -679,7 +679,7 @@ int count_sched(struct pt_regs *ctx, struct task_struct *prev) {
     key.prev_pid = prev->pid;
 
     // could also use `stats.increment(key);`
-    val = stats.lookup_or_init(&key, &zero);
+    val = stats.lookup_or_try_init(&key, &zero);
     if (val) {
       (*val)++;
     }

examples/cpp/LLCStat.cc

@@ -42,7 +42,7 @@ int on_cache_miss(struct bpf_perf_event_data *ctx) {
     get_key(&key);
 
     u64 zero = 0, *val;
-    val = miss_count.lookup_or_init(&key, &zero);
+    val = miss_count.lookup_or_try_init(&key, &zero);
     if (val) {
         (*val) += ctx->sample_period;
     }
@@ -55,7 +55,7 @@ int on_cache_ref(struct bpf_perf_event_data *ctx) {
     get_key(&key);
 
     u64 zero = 0, *val;
-    val = ref_count.lookup_or_init(&key, &zero);
+    val = ref_count.lookup_or_try_init(&key, &zero);
     if (val) {
         (*val) += ctx->sample_period;
     }

examples/cpp/TCPSendStack.cc

@@ -38,7 +38,7 @@ int on_tcp_send(struct pt_regs *ctx) {
   key.user_stack = stack_traces.get_stackid(ctx, BPF_F_USER_STACK);
 
   u64 zero = 0, *val;
-  val = counts.lookup_or_init(&key, &zero);
+  val = counts.lookup_or_try_init(&key, &zero);
   if (val) {
     (*val)++;
   }

examples/cpp/UseExternalMap.cc

@@ -55,7 +55,7 @@ int on_sched_switch(struct tracepoint__sched__sched_switch *args) {
   key.next_pid = args->next_pid;
   __builtin_memcpy(&key.prev_comm, args->prev_comm, 16);
   __builtin_memcpy(&key.next_comm, args->next_comm, 16);
-  val = counts.lookup_or_init(&key, &zero);
+  val = counts.lookup_or_try_init(&key, &zero);
   if (val) {
     (*val)++;
   }

examples/lua/offcputime.lua

@@ -64,7 +64,7 @@ int oncpu(struct pt_regs *ctx, struct task_struct *prev) {
     bpf_get_current_comm(&key.name, sizeof(key.name));
     key.stack_id = stack_traces.get_stackid(ctx, stack_flags);
 
-    val = counts.lookup_or_init(&key, &zero);
+    val = counts.lookup_or_try_init(&key, &zero);
     if (val) {
         (*val) += delta;
     }

examples/lua/task_switch.lua

@@ -32,7 +32,7 @@ int count_sched(struct pt_regs *ctx, struct task_struct *prev) {
   key.curr_pid = bpf_get_current_pid_tgid();
   key.prev_pid = prev->pid;
 
-  val = stats.lookup_or_init(&key, &zero);
+  val = stats.lookup_or_try_init(&key, &zero);
   if (val) {
     (*val)++;
   }

examples/networking/distributed_bridge/tunnel.c

@@ -36,7 +36,7 @@ int handle_ingress(struct __sk_buff *skb) {
   if (ifindex) {
     //bpf_trace_printk("ingress tunnel_id=%d ifindex=%d\n", tkey.tunnel_id, *ifindex);
     struct vni_key vk = {ethernet->src, *ifindex, 0};
-    struct host *src_host = mac2host.lookup_or_init(&vk,
+    struct host *src_host = mac2host.lookup_or_try_init(&vk,
         &(struct host){tkey.tunnel_id, tkey.remote_ipv4, 0, 0});
     if (src_host) {
       lock_xadd(&src_host->rx_pkts, 1);

examples/networking/http_filter/http-parse-complete.c

@@ -141,7 +141,7 @@ int http_filter(struct __sk_buff *skb) {
 	//keep the packet and send it to userspace retruning -1
 	HTTP_MATCH:
 	//if not already present, insert into map <Key, Leaf>
-	sessions.lookup_or_init(&key,&zero);
+	sessions.lookup_or_try_init(&key,&zero);
 
 	//send packet to userspace returning -1
 	KEEP:

examples/networking/tunnel_monitor/monitor.c

@@ -125,7 +125,7 @@ ip: ;
   if (key.outer_dip < key.outer_sip)
     swap_ipkey(&key);
   struct counters zleaf = {0};
-  struct counters *leaf = stats.lookup_or_init(&key, &zleaf);
+  struct counters *leaf = stats.lookup_or_try_init(&key, &zleaf);
   if (leaf) {
     if (is_ingress) {
       lock_xadd(&leaf->rx_pkts, 1);

examples/networking/vlan_learning/vlan_learning.c

@@ -32,7 +32,7 @@ int handle_phys2virt(struct __sk_buff *skb) {
       // auto-program reverse direction table
       int out_ifindex = leaf->out_ifindex;
       struct ifindex_leaf_t zleaf = {0};
-      struct ifindex_leaf_t *out_leaf = egress.lookup_or_init(&out_ifindex, &zleaf);
+      struct ifindex_leaf_t *out_leaf = egress.lookup_or_try_init(&out_ifindex, &zleaf);
       if (out_leaf) {
 	// to capture potential configuration changes
 	out_leaf->out_ifindex = skb->ifindex;

examples/tracing/mallocstacks.py

@@ -46,7 +46,7 @@
 
     // could also use `calls.increment(key, size);`
     u64 zero = 0, *val;
-    val = calls.lookup_or_init(&key, &zero);
+    val = calls.lookup_or_try_init(&key, &zero);
     if (val) {
       (*val) += size;
     }

examples/tracing/strlen_count.py

@@ -33,7 +33,7 @@
 
     bpf_probe_read(&key.c, sizeof(key.c), (void *)PT_REGS_PARM1(ctx));
     // could also use `counts.increment(key)`
-    val = counts.lookup_or_init(&key, &zero);
+    val = counts.lookup_or_try_init(&key, &zero);
     if (val) {
       (*val)++;
     }

examples/tracing/task_switch.c

@@ -15,7 +15,7 @@ int count_sched(struct pt_regs *ctx, struct task_struct *prev) {
     key.prev_pid = prev->pid;
 
     // could also use `stats.increment(key);`
-    val = stats.lookup_or_init(&key, &zero);
+    val = stats.lookup_or_try_init(&key, &zero);
     if (val) {
         (*val)++;
     }

examples/usdt_sample/scripts/lat_avg.py

@@ -78,7 +78,7 @@
     start_hash.delete(&operation_id);
 
     struct hash_leaf_t zero = {};
-    struct hash_leaf_t* hash_leaf = lat_hash.lookup_or_init(&hash_key, &zero);
+    struct hash_leaf_t* hash_leaf = lat_hash.lookup_or_try_init(&hash_key, &zero);
     if (0 == hash_leaf) {
         return 0;
     }

src/cc/export/helpers.h

@@ -78,6 +78,7 @@ struct _name##_table_t { \
   _leaf_type leaf; \
   _leaf_type * (*lookup) (_key_type *); \
   _leaf_type * (*lookup_or_init) (_key_type *, _leaf_type *); \
+  _leaf_type * (*lookup_or_try_init) (_key_type *, _leaf_type *); \
   int (*update) (_key_type *, _leaf_type *); \
   int (*insert) (_key_type *, _leaf_type *); \
   int (*delete) (_key_type *); \

src/cc/frontends/clang/b_frontend_action.cc

@@ -291,7 +291,8 @@ bool ProbeVisitor::assignsExtPtr(Expr *E, int *nbAddrOf) {
           if (!A->getName().startswith("maps"))
             return false;
 
-          if (memb_name == "lookup" || memb_name == "lookup_or_init") {
+          if (memb_name == "lookup" || memb_name == "lookup_or_init" ||
+              memb_name == "lookup_or_try_init") {
             if (m_.find(Ref->getDecl()) != m_.end()) {
               // Retrieved an ext. pointer from a map, mark LHS as ext. pointer.
               // Pointers from maps always need a single dereference to get the
@@ -772,7 +773,7 @@ bool BTypeVisitor::VisitCallExpr(CallExpr *Call) {
         string txt;
         auto rewrite_start = GET_BEGINLOC(Call);
         auto rewrite_end = GET_ENDLOC(Call);
-        if (memb_name == "lookup_or_init") {
+        if (memb_name == "lookup_or_init" || memb_name == "lookup_or_try_init") {
           string name = Ref->getDecl()->getName();
           string arg0 = rewriter_.getRewrittenText(expansionRange(Call->getArg(0)->getSourceRange()));
           string arg1 = rewriter_.getRewrittenText(expansionRange(Call->getArg(1)->getSourceRange()));
@@ -782,6 +783,11 @@ bool BTypeVisitor::VisitCallExpr(CallExpr *Call) {
           txt += "if (!leaf) {";
           txt += " " + update + ", " + arg0 + ", " + arg1 + ", BPF_NOEXIST);";
           txt += " leaf = " + lookup + ", " + arg0 + ");";
+          if (memb_name == "lookup_or_init") {
+            warning(GET_BEGINLOC(Call), "lookup_or_init() may cause return from the function, "
+                                        "use lookup_or_try_init() instead.");
+            txt += " if (!leaf) return 0;";
+          }
           txt += "}";
           txt += "leaf;})";
         } else if (memb_name == "increment") {

tests/cc/test_bpf_table.cc

@@ -181,7 +181,7 @@ TEST_CASE("test bpf stack table", "[bpf_stack_table]") {
     int on_sys_getuid(void *ctx) {
       int stack_id = stack_traces.get_stackid(ctx, BPF_F_REUSE_STACKID);
       int zero = 0, *val;
-      val = id.lookup_or_init(&zero, &stack_id);
+      val = id.lookup_or_try_init(&zero, &stack_id);
       if (val) {
         (*val) = stack_id;
       }
@@ -234,7 +234,7 @@ TEST_CASE("test bpf stack_id table", "[bpf_stack_table]") {
     int on_sys_getuid(void *ctx) {
       int stack_id = stack_traces.get_stackid(ctx, BPF_F_USER_STACK);
       int zero = 0, *val;
-      val = id.lookup_or_init(&zero, &stack_id);
+      val = id.lookup_or_try_init(&zero, &stack_id);
       if (val) {
         (*val) = stack_id;
       }

tests/lua/test_clang.lua

@@ -243,7 +243,7 @@ int kprobe__finish_task_switch(struct pt_regs *ctx, struct task_struct *prev) {
   key.curr_pid = bpf_get_current_pid_tgid();
   key.prev_pid = prev->pid;
 
-  val = stats.lookup_or_init(&key, &zero);
+  val = stats.lookup_or_try_init(&key, &zero);
   if (val) {
     (*val)++;
   }

tests/python/test_clang.py

@@ -396,7 +396,7 @@ def test_task_switch(self):
   key.curr_pid = bpf_get_current_pid_tgid();
   key.prev_pid = prev->pid;
 
-  val = stats.lookup_or_init(&key, &zero);
+  val = stats.lookup_or_try_init(&key, &zero);
   if (val) {
     (*val)++;
   }

tests/python/test_license.py

@@ -42,7 +42,7 @@ class TestLicense(unittest.TestCase):
     key.uid = uid & 0xFFFFFFFF;
     bpf_get_current_comm(&(key.comm), 16);
 
-    val = counts.lookup_or_init(&key, &zero);  // update counter
+    val = counts.lookup_or_try_init(&key, &zero);  // update counter
     if (val) {
         (*val)++;
     }

tests/python/test_lru.py

@@ -26,7 +26,7 @@ def test_lru_percpu_hash(self):
         int hello_world(void *ctx) {
             u32 key=0;
             u32 value = 0, *val;
-            val = stats.lookup_or_init(&key, &value);
+            val = stats.lookup_or_try_init(&key, &value);
             if (val) {
                 *val += 1;
             }

tests/python/test_percpu.py

@@ -29,7 +29,7 @@ def test_u64(self):
         int hello_world(void *ctx) {
             u32 key=0;
             u64 value = 0, *val;
-            val = stats.lookup_or_init(&key, &value);
+            val = stats.lookup_or_try_init(&key, &value);
             if (val) {
                 *val += 1;
             }
@@ -61,7 +61,7 @@ def test_u32(self):
         int hello_world(void *ctx) {
             u32 key=0;
             u32 value = 0, *val;
-            val = stats.lookup_or_init(&key, &value);
+            val = stats.lookup_or_try_init(&key, &value);
             if (val) {
                 *val += 1;
             }
@@ -97,7 +97,7 @@ def test_struct_custom_func(self):
         int hello_world(void *ctx) {
             u32 key=0;
             counter value = {0,0}, *val;
-            val = stats.lookup_or_init(&key, &value);
+            val = stats.lookup_or_try_init(&key, &value);
             if (val) {
                 val->c1 += 1;
                 val->c2 += 1;

tests/python/test_stat1.c

@@ -47,7 +47,7 @@ int on_packet(struct __sk_buff *skb) {
       tx = 1;
     }
     struct IPLeaf zleaf = {0};
-    struct IPLeaf *leaf = stats.lookup_or_init(&key, &zleaf);
+    struct IPLeaf *leaf = stats.lookup_or_try_init(&key, &zleaf);
     if (leaf) {
       lock_xadd(&leaf->rx_pkts, rx);
       lock_xadd(&leaf->tx_pkts, tx);

tests/python/test_trace2.c

@@ -8,7 +8,7 @@ BPF_HASH(stats, struct Ptr, struct Counters, 1024);
 int count_sched(struct pt_regs *ctx) {
   struct Ptr key = {.ptr = PT_REGS_PARM1(ctx)};
   struct Counters zleaf = {0};
-  struct Counters *val = stats.lookup_or_init(&key, &zleaf);
+  struct Counters *val = stats.lookup_or_try_init(&key, &zleaf);
   if (val) {
     val->stat1++;
   }

tests/python/test_trace2.py

@@ -19,7 +19,7 @@
   struct Counters zleaf;
 
   memset(&zleaf, 0, sizeof(zleaf));
-  struct Counters *val = stats.lookup_or_init(&key, &zleaf);
+  struct Counters *val = stats.lookup_or_try_init(&key, &zleaf);
   if (val) {
     val->stat1++;
   }

tests/python/test_trace3.c

@@ -47,7 +47,7 @@ int probe_blk_update_request(struct pt_regs *ctx) {
     index = SLOTS - 1;
 
   u64 zero = 0;
-  u64 *val = latency.lookup_or_init(&index, &zero);
+  u64 *val = latency.lookup_or_try_init(&index, &zero);
   if (val) {
     lock_xadd(val, 1);
   }

tests/python/test_trace4.py

@@ -14,14 +14,14 @@ def setUp(self):
         typedef struct { u64 val; } Val;
         BPF_HASH(stats, Key, Val, 3);
         int hello(void *ctx) {
-          Val *val = stats.lookup_or_init(&(Key){1}, &(Val){0});
+          Val *val = stats.lookup_or_try_init(&(Key){1}, &(Val){0});
           if (val) {
             val->val++;
           }
           return 0;
         }
         int goodbye(void *ctx) {
-          Val *val = stats.lookup_or_init(&(Key){2}, &(Val){0});
+          Val *val = stats.lookup_or_try_init(&(Key){2}, &(Val){0});
           if (val) {
             val->val++;
           }

tests/python/test_trace_maxactive.py

@@ -15,16 +15,12 @@ def setUp(self):
         BPF_HASH(stats, Key, Val, 3);
         int hello(void *ctx) {
           Val *val = stats.lookup_or_init(&(Key){1}, &(Val){0});
-          if (val) {
-            val->val++;
-          }
+          val->val++;
           return 0;
         }
         int goodbye(void *ctx) {
           Val *val = stats.lookup_or_init(&(Key){2}, &(Val){0});
-          if (val) {
-            val->val++;
-          }
+          val->val++;
           return 0;
         }
         """)