Code reivew changes

Signed-off-by: Ron Federman <[email protected]>

ringbuf/reader.go

@@ -186,7 +186,8 @@ func (r *Reader) SetDeadline(t time.Time) {
 // Read the next record from the BPF ringbuf.
 //
 // Returns os.ErrClosed if Close is called on the Reader, or os.ErrDeadlineExceeded
-// if a deadline was set.
+// if a deadline was set and no valid entry was present. A producer might use BPF_RB_NO_WAKEUP
+// which may cause the deadline to expire but a valid entry will be present.
 func (r *Reader) Read() (Record, error) {
 	var rec Record
 	return rec, r.ReadInto(&rec)
@@ -205,7 +206,8 @@ func (r *Reader) ReadInto(rec *Record) error {
 		if !r.haveData {
 			_, err := r.poller.Wait(r.epollEvents[:cap(r.epollEvents)], r.deadline)
 			if errors.Is(err, os.ErrDeadlineExceeded) && !r.ring.isEmpty() {
-				// TODO: Explain why we're ignoring this
+				// Ignoring this for reading a valid entry after timeout
+				// This can occur if the producer submitted to the ring buffer with BPF_RB_NO_WAKEUP
 				err = nil
 			}
 			if err != nil {
@@ -216,10 +218,12 @@ func (r *Reader) ReadInto(rec *Record) error {
 
 		for {
 			err := readRecord(r.ring, rec, r.header)
-			if errors.Is(err, errBusy) || errors.Is(err, errDiscard) {
+			// Not using errors.Is which is quite a bit slower
+			// For a tight loop it might make a difference
+			if err == errBusy || err == errDiscard {
 				continue
 			}
-			if errors.Is(err, errEOR) {
+			if err == errEOR {
 				r.haveData = false
 				break
 			}

ringbuf/reader_test.go

@@ -264,32 +264,18 @@ func TestReaderNoWakeup(t *testing.T) {
 		t.Fatal("Expected 0 as return value, got", errno)
 	}
 
-	var timeoutMs int64 = 100
-	lowerBound := timeoutMs * 98 / 100
-	upperBound := timeoutMs * 102 / 100
-	nonBlocking := timeoutMs * 2 / 100
-	startTime := time.Now()
-
 	rd.SetDeadline(time.Now())
-
 	record, err := rd.Read()
 
-	if time.Since(startTime).Milliseconds() < lowerBound || time.Since(startTime).Milliseconds() > upperBound {
-		t.Errorf("Expected timeout after first read but %d ms passed", time.Since(startTime).Milliseconds())
-	}
 	if err != nil {
 		t.Error("Expected no error from first Read, got:", err)
 	}
 	if len(record.RawSample) != 5 {
 		t.Errorf("Expected to read 5 bytes bot got %d", len(record.RawSample))
 	}
 
-	startTime = time.Now()
 	record, err = rd.Read()
 
-	if time.Since(startTime).Milliseconds() >= nonBlocking {
-		t.Errorf("Expected non-blocking after second read but %d ms passed", time.Since(startTime).Milliseconds())
-	}
 	if err != nil {
 		t.Error("Expected no error from second Read, got:", err)
 	}