Skip to content

felix/bpf/ut: drain shared ring buffer in BPF UT helpers #12515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lucastigera wants to merge 1 commit into
base: master
Choose a base branch
from
+27 −21
Open

felix/bpf/ut: drain shared ring buffer in BPF UT helpers #12515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions felix/bpf/ut/bpf_prog_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,21 @@ func initMapsOnce() {
})
}

// newTestRingBuf opens a reader on the shared cali_rb_evnt ring buffer used
// by all BPF UT programs and drains any events left over from earlier tests,
// so rb.Next() only observes events produced after this call. The caller is
// responsible for closing the returned reader.
//
// Prefer this helper over calling ringbuf.New directly — the ring buffer map
// is pinned and shared across tests, so a naked reader can pick up stray
// records (TYPE_LOST_EVENTS markers, kprobe stats, etc.) and mis-parse them.
func newTestRingBuf() *ringbuf.RingBuffer {
rb, err := ringbuf.New(ringBufMap, rbSize)
Expect(err).NotTo(HaveOccurred())
rb.Drain()
return rb
Comment on lines +696 to +700
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newTestRingBuf() depends on rbSize, which is currently declared in ringbuf_events_test.go, and initMapsOnce() hard-codes the ring buffer map size as 1024*1024. That cross-file coupling makes it easy for these values to drift or for the helper to stop compiling if the constant is moved/guarded by build tags. Consider defining a single shared constant (e.g., in bpf_prog_test.go) and using it both for ringbuf.Map(..., size) and ringbuf.New(..., size) (and have tests reference that constant).

Copilot uses AI. Check for mistakes.
}

func cleanupMap(m maps.Map) {
log.WithField("map", m.GetName()).Info("Cleaning")
err := m.Iter(func(_, _ []byte) maps.IteratorAction {
Expand Down
10 changes: 5 additions & 5 deletions felix/bpf/ut/flow_log_events_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
. "github.com/onsi/gomega"

"github.com/projectcalico/calico/felix/bpf/events"
"github.com/projectcalico/calico/felix/bpf/ringbuf"
"github.com/projectcalico/calico/felix/bpf/routes"
)

Expand All @@ -34,8 +33,7 @@ func TestFlowLogV6Events(t *testing.T) {
ipv6 := ip6hdr.(*layers.IPv6)
udp := l4.(*layers.UDP)

rb, err := ringbuf.New(ringBufMap, rbSize)
Expect(err).NotTo(HaveOccurred())
rb := newTestRingBuf()
defer rb.Close()

rtKey := routes.NewKeyV6(srcV6CIDR).AsBytes()
Expand All @@ -54,6 +52,7 @@ func TestFlowLogV6Events(t *testing.T) {
Expect(err).NotTo(HaveOccurred())
e, err := events.ParseEvent(rawEvent)
Expect(err).NotTo(HaveOccurred())
Expect(e.Type()).To(Equal(events.TypePolicyVerdictV6))
evnt := events.ParsePolicyVerdict(e.Data(), true)
Expect(evnt.SrcAddr).To(Equal(ipv6.SrcIP))
Expect(evnt.DstAddr).To(Equal(ipv6.DstIP))
Expand All @@ -71,8 +70,7 @@ func TestFlowLogEvents(t *testing.T) {
ipv4 := iphdr.(*layers.IPv4)
udp := l4.(*layers.UDP)

rb, err := ringbuf.New(ringBufMap, rbSize)
Expect(err).NotTo(HaveOccurred())
rb := newTestRingBuf()
defer rb.Close()

rtKey := routes.NewKey(srcV4CIDR).AsBytes()
Expand All @@ -91,6 +89,7 @@ func TestFlowLogEvents(t *testing.T) {
Expect(err).NotTo(HaveOccurred())
e, err := events.ParseEvent(rawEvent)
Expect(err).NotTo(HaveOccurred())
Expect(e.Type()).To(Equal(events.TypePolicyVerdict))
evnt := events.ParsePolicyVerdict(e.Data(), false)
Expect(evnt.SrcAddr).To(Equal(ipv4.SrcIP))
Expect(evnt.DstAddr).To(Equal(ipv4.DstIP))
Expand Down Expand Up @@ -124,6 +123,7 @@ func TestFlowLogEvents(t *testing.T) {
Expect(err).NotTo(HaveOccurred())
e, err = events.ParseEvent(rawEvent)
Expect(err).NotTo(HaveOccurred())
Expect(e.Type()).To(Equal(events.TypePolicyVerdict))
evnt = events.ParsePolicyVerdict(e.Data(), false)
Expect(evnt.SrcAddr).To(Equal(ipv4.SrcIP))
Expect(evnt.DstAddr).To(Equal(ipv4.DstIP))
Expand Down
23 changes: 7 additions & 16 deletions felix/bpf/ut/ringbuf_events_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import (
"github.com/gopacket/gopacket/layers"
. "github.com/onsi/gomega"
log "github.com/sirupsen/logrus"

"github.com/projectcalico/calico/felix/bpf/ringbuf"
)

const (
Expand All @@ -42,8 +40,7 @@ func TestRingBufBasic(t *testing.T) {
ipv4 := iphdr.(*layers.IPv4)
udp := l4.(*layers.UDP)

rb, err := ringbuf.New(ringBufMap, rbSize)
Expect(err).NotTo(HaveOccurred())
rb := newTestRingBuf()
defer rb.Close()

// Send a UDP packet and verify the event.
Expand Down Expand Up @@ -98,8 +95,7 @@ func TestRingBufReaderRecovery(t *testing.T) {
_, _, _, _, pktBytes, err := testPacketUDPDefault()
Expect(err).NotTo(HaveOccurred())

rb, err := ringbuf.New(ringBufMap, rbSize)
Expect(err).NotTo(HaveOccurred())
rb := newTestRingBuf()

Comment on lines +98 to 99
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TestRingBufReaderRecovery creates a ring buffer reader without a defer rb.Close(). If an assertion fails before the explicit rb.Close() call, the reader can be leaked (mmap/epoll fds), which may impact subsequent tests. Consider adding a deferred close immediately after creation (even if you still close early to test re-opening).

Copilot uses AI. Check for mistakes.
runBpfUnitTest(t, "ringbuf_events.c", func(bpfrun bpfProgRunFn) {
res, err := bpfrun(pktBytes)
Expand All @@ -114,8 +110,7 @@ func TestRingBufReaderRecovery(t *testing.T) {
// Close the first reader and create a new one on the same map.
rb.Close()

rb2, err := ringbuf.New(ringBufMap, rbSize)
Expect(err).NotTo(HaveOccurred())
rb2 := newTestRingBuf()
defer rb2.Close()

// Send another event — the new reader should pick it up.
Expand Down Expand Up @@ -145,14 +140,11 @@ func TestRingBufFillup(t *testing.T) {
_, _, _, _, pktBytes, err := testPacketUDPDefault()
Expect(err).NotTo(HaveOccurred())

rb, err := ringbuf.New(ringBufMap, rbSize)
Expect(err).NotTo(HaveOccurred())
rb := newTestRingBuf()
defer rb.Close()

// Drain any leftover events from previous tests and reset the drops map
// so we start with a completely clean state.
rb.Drain()
// Reset the single-entry drops map (struct rb_drops_val = 16 bytes).
// Reset the single-entry drops map (struct rb_drops_val = 16 bytes) so
// this test's accounting isn't affected by earlier ring-buffer-full tests.
k := make([]byte, 4) // key = 0
zeroVal := make([]byte, 16)
err = ringBufDropsMap.Update(k, zeroVal)
Expand Down Expand Up @@ -232,8 +224,7 @@ func TestRingBufMultipleEvents(t *testing.T) {
RegisterTestingT(t)
hostIP = node1ip

rb, err := ringbuf.New(ringBufMap, rbSize)
Expect(err).NotTo(HaveOccurred())
rb := newTestRingBuf()
defer rb.Close()

numEvents := 10
Expand Down
Loading