Deleting sessions into a buffer nobody would flush

This one is the direct sequel to the 8KB buffer bug, and it exists partly because that fix landed.

When bfdd shuts down it tears down every session. For dataplane sessions, bfd_session_free() calls bfd_dplane_delete_session(), which enqueues one DP_DELETE_SESSION message per session into the dataplane client’s output buffer.

Nothing ever writes those bytes to the socket.

Why they never leave

Enqueuing schedules a write event. On the shutdown path, the event loop does not run again, so that event never fires. bfd_dplane_ctx_free() sees bglobal.bg_shutdown set, goes straight to free_resources, closes the socket, and frees bdc->outbuf with the messages sitting in it.

What the dataplane actually receives depends on which version you are running, and this is the uncomfortable part:

• On 10.5.1, before #22645, enqueue only schedules the write. Zero deletes reach the dataplane. The whole burst is silently lost.
• On current master, with #22645, the synchronous flush I added fires once when the buffer fills mid-burst. So exactly one buffer’s worth is delivered, and the tail is freed unsent. At 140 bytes per message that is floor(8192/140) = 58 delivered, and everything past 58 dropped.

I would argue the second case is worse. Losing everything is at least unambiguous: the dataplane sees no teardown at all and its existing reconnect logic handles it. Losing an arbitrary subset means the dataplane observes a partial teardown, in hash order, with no signal that the rest were also meant to go. It looks like a successful operation.

So my own fix converted a clean failure into a misleading one. It did not cause this bug, which was there the whole time, but it changed its shape, and I would rather say that plainly than let someone else find it.

#22645 also cannot cover this case. After the last delete is enqueued there is no further enqueue to trigger a flush, and no event-loop iteration left to service the scheduled write.

Reproducing without a dataplane

Same approach as before, and the reason both reports were actionable: you do not need an offload engine. A TCP socket that accepts a connection, parses the bfddp message stream, and counts messages per type is enough. Configure 64 sessions, let the registration burst land, stop bfdd cleanly, and count the deletes.

The sessions never come up against a sink that does not answer, and that does not matter. The shutdown delete walk runs for every configured dataplane session regardless of state.

Unpatched, the sink reports 58 deletes and 8120 bytes: exactly one full buffer.

The fix

A synchronous drain in bfd_dplane_ctx_free(), before socket_close(), run only when bg_shutdown is set:

/* Nothing to write or nowhere to write it. */
if (bdc->sock == -1 || bdc->connecting)
        return;

while ((remaining = STREAM_READABLE(bdc->outbuf)) > 0) {
        rv = write(bdc->sock, stream_pnt(bdc->outbuf), remaining);
        if (rv <= 0)
                break;
        bdc->out_bytes += (uint64_t)rv;
        stream_forward_getp(bdc->outbuf, (size_t)rv);
}

remaining = STREAM_READABLE(bdc->outbuf);
if (remaining)
        zlog_warn("%s: %zu bytes of data plane messages lost on shutdown", __func__,
                  remaining);

It is deliberately best-effort. A single pass writing whatever the socket will take, giving up if it would block or errors, and saying so in the log if anything is left. The event loop is already gone by this point and the socket belongs to the frrevent machinery, so re-arming a write event is not available and blocking on shutdown would be worse than the problem. Unsent deletions are timed out by the dataplane instead, which is a mechanism that already has to exist for the case where bfdd is killed rather than stopped.

The warning matters as much as the loop. The original bug was not that messages were lost, it was that they were lost silently.

I deliberately did not reuse bfd_dplane_flush(). Its error path frees the very context this code is being called from, which would be a use-after-free.

Results, with the sink and with a real engine:

• 64 sessions, clean stop: unpatched 58/64, patched 64/64, byte count exact at 140 bytes per message, no loss warning.
• 128 sessions: patched delivers 128/128 and 17920 bytes, two full buffer-loads plus a tail, which confirms the drain loops rather than flushing once.
• Against a live dataplane holding 64 established sessions, 32 IPv4 and 32 IPv6: unpatched, a clean restart stranded 6 sessions as orphans. Patched, all 64 deletes arrive, zero orphans across the restart, and all 64 re-register with Output full events: 0. A second restart in the same run behaves identically, which shows the registration-side behaviour from #22645 is undisturbed.

Open as #22692.

What I took from it

Shutdown paths get written as if the work is already done. The event loop is usually still there to finish anything outstanding, so “enqueue and return” reads as complete. It is complete right up until the moment you are enqueueing because you are shutting the loop down.

The other thing: fixing a bug is a good time to look for the one next to it. The buffer flush I added made this failure visible by changing 0 into 58, and 58 is a much stranger number to see in a log than 0.