Refresh delayed report timers on query messages

...as per As per RFC 2236 section 3 page 3 (for IGMPv2) and
RFC 2710 section 4 page 5 (for MLDv1).

See comments in code for more details.

Test: ip_test.TestHandleQuery
PiperOrigin-RevId: 354603068

2 files changed, 68 insertions, 42 deletions

diff --git a/pkg/tcpip/network/ip/generic_multicast_protocol.go b/pkg/tcpip/network/ip/generic_multicast_protocol.go
index a81f5c8c3..b9f129728 100644
--- a/pkg/tcpip/network/ip/generic_multicast_protocol.go
+++ b/pkg/tcpip/network/ip/generic_multicast_protocol.go
@@ -126,6 +126,16 @@ type multicastGroupState struct {
 	//
 	// Must not be nil.
 	delayedReportJob *tcpip.Job
+
+	// delyedReportJobFiresAt is the time when the delayed report job will fire.
+	//
+	// A zero value indicates that the job is not scheduled.
+	delayedReportJobFiresAt time.Time
+}
+
+func (m *multicastGroupState) cancelDelayedReportJob() {
+	m.delayedReportJob.Cancel()
+	m.delayedReportJobFiresAt = time.Time{}
 }
 
 // GenericMulticastProtocolOptions holds options for the generic multicast
@@ -428,7 +438,7 @@ func (g *GenericMulticastProtocolState) HandleReportLocked(groupAddress tcpip.Ad
 	//   on that interface, it stops its timer and does not send a Report for
 	//   that address, thus suppressing duplicate reports on the link.
 	if info, ok := g.memberships[groupAddress]; ok && info.state.isDelayingMember() {
-		info.delayedReportJob.Cancel()
+		info.cancelDelayedReportJob()
 		info.lastToSendReport = false
 		info.state = idleMember
 		g.memberships[groupAddress] = info
@@ -603,7 +613,7 @@ func (g *GenericMulticastProtocolState) transitionToNonMemberLocked(groupAddress
 		return
 	}
 
-	info.delayedReportJob.Cancel()
+	info.cancelDelayedReportJob()
 	g.maybeSendLeave(groupAddress, info.lastToSendReport)
 	info.lastToSendReport = false
 	info.state = nonMember
@@ -645,14 +655,24 @@ func (g *GenericMulticastProtocolState) setDelayTimerForAddressRLocked(groupAddr
 	//   If a timer for any address is already running, it is reset to the new
 	//   random value only if the requested Maximum Response Delay is less than
 	//   the remaining value of the running timer.
+	now := time.Unix(0 /* seconds */, g.opts.Clock.NowNanoseconds())
 	if info.state == delayingMember {
-		// TODO: Reset the timer if time remaining is greater than maxResponseTime.
-		return
+		if info.delayedReportJobFiresAt.IsZero() {
+			panic(fmt.Sprintf("delayed report unscheduled while in the delaying member state; group = %s", groupAddress))
+		}
+
+		if info.delayedReportJobFiresAt.Sub(now) <= maxResponseTime {
+			// The timer is scheduled to fire before the maximum response time so we
+			// leave our timer as is.
+			return
+		}
 	}
 
 	info.state = delayingMember
-	info.delayedReportJob.Cancel()
-	info.delayedReportJob.Schedule(g.calculateDelayTimerDuration(maxResponseTime))
+	info.cancelDelayedReportJob()
+	maxResponseTime = g.calculateDelayTimerDuration(maxResponseTime)
+	info.delayedReportJob.Schedule(maxResponseTime)
+	info.delayedReportJobFiresAt = now.Add(maxResponseTime)
 }
 
 // calculateDelayTimerDuration returns a random time between (0, maxRespTime].
diff --git a/pkg/tcpip/network/ip/generic_multicast_protocol_test.go b/pkg/tcpip/network/ip/generic_multicast_protocol_test.go
index d5d5a449e..60eaea37e 100644
--- a/pkg/tcpip/network/ip/generic_multicast_protocol_test.go
+++ b/pkg/tcpip/network/ip/generic_multicast_protocol_test.go
@@ -408,40 +408,46 @@ func TestHandleReport(t *testing.T) {
 
 func TestHandleQuery(t *testing.T) {
 	tests := []struct {
-		name             string
-		queryAddr        tcpip.Address
-		maxDelay         time.Duration
-		expectReportsFor []tcpip.Address
+		name                    string
+		queryAddr               tcpip.Address
+		maxDelay                time.Duration
+		expectQueriedReportsFor []tcpip.Address
+		expectDelayedReportsFor []tcpip.Address
 	}{
 		{
-			name:             "Unpecified empty",
-			queryAddr:        "",
-			maxDelay:         0,
-			expectReportsFor: []tcpip.Address{addr1, addr2},
+			name:                    "Unpecified empty",
+			queryAddr:               "",
+			maxDelay:                0,
+			expectQueriedReportsFor: []tcpip.Address{addr1, addr2},
+			expectDelayedReportsFor: nil,
 		},
 		{
-			name:             "Unpecified any",
-			queryAddr:        "\x00",
-			maxDelay:         1,
-			expectReportsFor: []tcpip.Address{addr1, addr2},
+			name:                    "Unpecified any",
+			queryAddr:               "\x00",
+			maxDelay:                1,
+			expectQueriedReportsFor: []tcpip.Address{addr1, addr2},
+			expectDelayedReportsFor: nil,
 		},
 		{
-			name:             "Specified",
-			queryAddr:        addr1,
-			maxDelay:         2,
-			expectReportsFor: []tcpip.Address{addr1},
+			name:                    "Specified",
+			queryAddr:               addr1,
+			maxDelay:                2,
+			expectQueriedReportsFor: []tcpip.Address{addr1},
+			expectDelayedReportsFor: []tcpip.Address{addr2},
 		},
 		{
-			name:             "Specified all-nodes",
-			queryAddr:        addr3,
-			maxDelay:         3,
-			expectReportsFor: nil,
+			name:                    "Specified all-nodes",
+			queryAddr:               addr3,
+			maxDelay:                3,
+			expectQueriedReportsFor: nil,
+			expectDelayedReportsFor: []tcpip.Address{addr1, addr2},
 		},
 		{
-			name:             "Specified other",
-			queryAddr:        addr4,
-			maxDelay:         4,
-			expectReportsFor: nil,
+			name:                    "Specified other",
+			queryAddr:               addr4,
+			maxDelay:                4,
+			expectQueriedReportsFor: nil,
+			expectDelayedReportsFor: []tcpip.Address{addr1, addr2},
 		},
 	}
 
@@ -469,20 +475,20 @@ func TestHandleQuery(t *testing.T) {
 			if diff := mgp.check(nil /* sendReportGroupAddresses */, nil /* sendLeaveGroupAddresses */); diff != "" {
 				t.Fatalf("mockMulticastGroupProtocol mismatch (-want +got):\n%s", diff)
 			}
-			// Generic multicast protocol timers are expected to take the job mutex.
-			clock.Advance(maxUnsolicitedReportDelay)
-			if diff := mgp.check([]tcpip.Address{addr1, addr2} /* sendReportGroupAddresses */, nil /* sendLeaveGroupAddresses */); diff != "" {
-				t.Fatalf("mockMulticastGroupProtocol mismatch (-want +got):\n%s", diff)
-			}
 
-			// Receiving a query should make us schedule a new delayed report if it
-			// is a query directed at us or a general query.
+			// Receiving a query should make us reschedule our delayed report timer
+			// to some time within the new max response delay.
 			mgp.handleQuery(test.queryAddr, test.maxDelay)
-			if len(test.expectReportsFor) != 0 {
-				clock.Advance(test.maxDelay)
-				if diff := mgp.check(test.expectReportsFor /* sendReportGroupAddresses */, nil /* sendLeaveGroupAddresses */); diff != "" {
-					t.Errorf("mockMulticastGroupProtocol mismatch (-want +got):\n%s", diff)
-				}
+			clock.Advance(test.maxDelay)
+			if diff := mgp.check(test.expectQueriedReportsFor /* sendReportGroupAddresses */, nil /* sendLeaveGroupAddresses */); diff != "" {
+				t.Errorf("mockMulticastGroupProtocol mismatch (-want +got):\n%s", diff)
+			}
+
+			// The groups that were not affected by the query should still send a
+			// report after the max unsolicited report delay.
+			clock.Advance(maxUnsolicitedReportDelay)
+			if diff := mgp.check(test.expectDelayedReportsFor /* sendReportGroupAddresses */, nil /* sendLeaveGroupAddresses */); diff != "" {
+				t.Errorf("mockMulticastGroupProtocol mismatch (-want +got):\n%s", diff)
 			}
 
 			// Should have no more messages to send.