]> git.ipfire.org Git - thirdparty/bird.git/commitdiff
fixup! io-loop.h: tcp ao dump fixing socket-dumping
authorKaterina Kubecova <katerina.kubecova@nic.cz>
Wed, 16 Apr 2025 12:50:36 +0000 (14:50 +0200)
committerKaterina Kubecova <katerina.kubecova@nic.cz>
Wed, 16 Apr 2025 12:50:36 +0000 (14:50 +0200)
sysdep/unix/io-loop.c
sysdep/unix/io-loop.h
sysdep/unix/io.c

index 55ed706a2862875e318df7514b8cc8a32dd85b9a..73914e9343693bdee5647ab291b461abb4379ad9 100644 (file)
@@ -403,7 +403,7 @@ socket_changed(struct birdsock *s, bool recalculate_sk_info)
 
   if (loop != &main_birdloop && recalculate_sk_info)
   {
-    int size = loop->sock_num * sk_max_dump_len() + 17;
+    int size = loop->sock_num * sk_max_dump_len + 17;
     char *new_info = mb_alloc(loop->pool, size);
 
     node *n;
@@ -427,7 +427,7 @@ socket_changed(struct birdsock *s, bool recalculate_sk_info)
       old_info = atomic_load_explicit(&loop->sockets_info, memory_order_relaxed);
     atomic_store_explicit(&loop->sockets_info, new_info, memory_order_relaxed);
 
-    synchronize_rcu();
+    synchronize_rcu(); // We are about to free old_info, which might be in use in dumping right now
 
     if (old_info)
       mb_free(old_info);
@@ -1702,6 +1702,17 @@ cmd_show_threads(int show_loops)
   bird_thread_sync_all(&tsd->sync, bird_thread_show, cmd_show_threads_done, "Show Threads");
 }
 
+
+/*
+  sk_dump_all uses cached info, because we need the dupms quickly and locking
+  (especially for thread_group loops) would be too complicated and slow.
+
+  sk_dump_ao_all has a different approach - in each thread we send an event to each loop
+  and dump from the loops. We do this because ao dump changes much more often and caching
+  would be too frequent. This means ao dump is slower than basic socket dump
+  and the basic socket dump should be used for quick debugging.
+*/
+
 void
 sk_dump_all(struct dump_request *dreq)
 {
@@ -1725,17 +1736,20 @@ sk_dump_all(struct dump_request *dreq)
       WALK_TLIST(thread, thr, &group->threads)
         WALK_TLIST(birdloop, loop, &thr->loops)
         {
+          /* The socket_info might be about to change (and free previous version) right now */
           rcu_read_lock();
             char *info = atomic_load_explicit(&loop->sockets_info, memory_order_relaxed);
+
+            if (info)
+              RDUMP(info);
           rcu_read_unlock();
-          if (info)
-            RDUMP(info);
         }
 
   WALK_TLIST_DELSAFE(thread_group, gpub, &global_thread_group_list)
   TG_LOCKED(gpub, group)
   {
-    WALK_TLIST_DELSAFE(birdloop, loop, &group->loops){
+    WALK_TLIST_DELSAFE(birdloop, loop, &group->loops)
+    {
       rcu_read_lock();
         char *info = atomic_load_explicit(&loop->sockets_info, memory_order_relaxed);
       rcu_read_unlock();
@@ -1751,7 +1765,9 @@ sk_dump_all(struct dump_request *dreq)
 struct bird_show_ao_socket {
   struct bird_thread_syncer sync;
   struct dump_request *dreq;
-  rw_spinlock lock;
+  DOMAIN(rtable) lock;
+  struct pool *pool;
+
   _Atomic int dump_finished; // the dump is finished when reached zero
 };
 
@@ -1764,7 +1780,7 @@ static void
 _sk_dump_ao_for_loop(struct bird_show_ao_socket *bsas, list sock_list)
 {
   struct dump_request *dreq = bsas->dreq;
-  rws_write_lock(&bsas->lock);
+
   WALK_LIST_(node, n, sock_list)
   {
     sock *s = SKIP_BACK(sock, n, n);
@@ -1779,8 +1795,6 @@ _sk_dump_ao_for_loop(struct bird_show_ao_socket *bsas, list sock_list)
     sk_dump_ao_keys(s, dreq);
   }
 
-  rws_write_unlock(&bsas->lock);
-
   if (atomic_fetch_sub_explicit(&bsas->dump_finished, 1, memory_order_relaxed) == 1)
   {
     RDUMP("\n");
@@ -1793,7 +1807,6 @@ sk_dump_ao_for_loop(void *data)
 {
   struct sk_dump_ao_event *sdae = (struct sk_dump_ao_event*) data;
   _sk_dump_ao_for_loop(sdae->bsas, birdloop_current->sock_list);
-  mb_free(sdae);
 }
 
 static void
@@ -1801,13 +1814,11 @@ _sk_dump_ao_send_event(struct bird_show_ao_socket *bsas)
 {
   WALK_TLIST(birdloop, loop, &this_thread->loops)
   {
-    birdloop_enter(loop);
-    struct sk_dump_ao_event *sdae = mb_allocz(loop->pool, sizeof(struct sk_dump_ao_event));
+    struct sk_dump_ao_event *sdae = mb_allocz(bsas->pool, sizeof(struct sk_dump_ao_event));
     sdae->event.hook = sk_dump_ao_for_loop;
     sdae->event.data = sdae;
     sdae->bsas = bsas;
     ev_send_loop(loop, &sdae->event);
-    birdloop_leave(loop);
   }
 }
 
@@ -1815,7 +1826,9 @@ static void
 sk_dump_ao_send_event(struct bird_thread_syncer *sync)
 {
   SKIP_BACK_DECLARE(struct bird_show_ao_socket, bsas, sync, sync);
+  LOCK_DOMAIN(rtable, bsas->lock);
   _sk_dump_ao_send_event(bsas);
+  UNLOCK_DOMAIN(rtable, bsas->lock);
 }
 
 static void
@@ -1827,16 +1840,25 @@ sk_dump_ao_thread_sync_done(struct bird_thread_syncer *sync)
   {
     struct dump_request *dreq = bsas->dreq;
     RDUMP("\n");
-    mb_free(bsas);
+    DOMAIN(rtable) lock = bsas->lock;
+    LOCK_DOMAIN(rtable, lock);
+    mb_free(bsas->pool);
+    UNLOCK_DOMAIN(rtable, lock);
   }
 }
 
 void
 sk_dump_ao_all(struct dump_request *dreq)
 {
-  struct bird_show_ao_socket *bsas = mb_allocz(&root_pool, sizeof(struct bird_show_ao_socket));
+  DOMAIN(rtable) lock = DOMAIN_NEW(rtable);
+  LOCK_DOMAIN(rtable, lock);
+
+  pool *pool = rp_new(&root_pool, lock.rtable, "Dump socket TCP-AO");
+
+  struct bird_show_ao_socket *bsas = mb_allocz(pool, sizeof(struct bird_show_ao_socket));
   bsas->dreq = dreq;
-  rws_init(&bsas->lock);
+  bsas->lock = lock;
+  bsas->pool = pool;
   atomic_store_explicit(&bsas->dump_finished, 1, memory_order_relaxed);
 
   RDUMP("TCP-AO listening sockets:\n");
@@ -1849,6 +1871,8 @@ sk_dump_ao_all(struct dump_request *dreq)
       _sk_dump_ao_send_event(bsas);
   }
 
+  UNLOCK_DOMAIN(rtable, lock);
+
   bird_thread_sync_all(&bsas->sync, sk_dump_ao_send_event, sk_dump_ao_thread_sync_done, "Show ao sockets");
 }
 
index 661266449872b019f89d3068332e25bb7762dc60..7e14bd8e3e86a819acfb381fb6f17bc93fd6ce05 100644 (file)
@@ -21,7 +21,6 @@ struct pfd {
   BUFFER(struct birdloop *) loop;
 };
 
-uint sk_max_dump_len(void);
 void sk_dump_to_buffer(buffer *buf, sock *s);
 void sockets_prepare(struct birdloop *, struct pfd *);
 void socket_changed(struct birdsock *, bool recalculate_sk_info);
@@ -32,6 +31,8 @@ void pipe_drain(struct pipe *);
 void pipe_kick(struct pipe *);
 void sk_dump(struct dump_request *dreq, resource *r);
 
+extern int sk_max_dump_len;
+
 #define TIME_BY_SEC_SIZE       16
 
 struct spent_time {
index aa43256625eeb3546be9c9a6b369072f76410246..bb5076e7d5afd5cef0cf4364f55afdc118dc052b 100644 (file)
@@ -1233,18 +1233,12 @@ sk_dump(struct dump_request *dreq, resource *r)
        s->iface ? s->iface->name : "none");
 }
 
-uint
-sk_max_dump_len(void)
-{
-  uint ret = strlen("(%s, ud=%p, sa=%I, sp=%d, da=%I, dp=%d, tos=%d, ttl=%d, if=%s)\n");
-  ret += 3; // max sk_type_name = 5
-  ret += 14; // looks like %p size is 16
-  ret += (IP6_MAX_TEXT_LENGTH -2) * 2;
-  ret += 14 * 4; // %d
-  ret += IFNAMSIZ - 2;
-  ret++; // terminating zero
-  return ret;
-}
+int sk_max_dump_len = sizeof("(%s, ud=%p, sa=%I, sp=%d, da=%I, dp=%d, tos=%d, ttl=%d, if=%s)\n") \
+  + 3 /* max sk_type_name = 5 */ \
+  + 14 /* looks like %p size is 16 */ \
+  + (IP6_MAX_TEXT_LENGTH -2) * 2 \
+  + 14 * 4 /* %d lengths */ \
+  + IFNAMSIZ - 2;
 
 void
 sk_dump_to_buffer(buffer *buf, sock *s)