From c76b03723e614707d105c3f3be8fcfe8250e2e74 Mon Sep 17 00:00:00 2001
From: Ghanan Gowripalan <ghanan@google.com>
Date: Fri, 13 Jan 2023 13:58:05 -0800
Subject: [PATCH] Coalesce records sent by the state changed timer

When the state changed timer fires, instead of sending a single record
per report, send as many records as possible per report message.

Updates #8346

PiperOrigin-RevId: 501932735
---
 .../internal/ip/generic_multicast_protocol.go       |  9 +++++----
 pkg/tcpip/network/ipv4/igmp_test.go                 | 11 ++++++++++-
 pkg/tcpip/network/ipv6/mld_test.go                  | 13 ++++++++++---
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/pkg/tcpip/network/internal/ip/generic_multicast_protocol.go b/pkg/tcpip/network/internal/ip/generic_multicast_protocol.go
index 887a13ba2..583afbc4e 100644
--- a/pkg/tcpip/network/internal/ip/generic_multicast_protocol.go
+++ b/pkg/tcpip/network/internal/ip/generic_multicast_protocol.go
@@ -520,6 +520,7 @@ func (g *GenericMulticastProtocolState) sendV2ReportAndMaybeScheduleChangedTimer
 					g.protocolMU.Lock()
 					defer g.protocolMU.Unlock()
 
+					reportBuilder := g.opts.Protocol.NewReportV2Builder()
 					nonEmptyReport := false
 					for groupAddress, info := range g.memberships {
 						if info.transmissionLeft == 0 || !g.shouldPerformForGroup(groupAddress) {
@@ -529,15 +530,11 @@ func (g *GenericMulticastProtocolState) sendV2ReportAndMaybeScheduleChangedTimer
 						info.transmissionLeft--
 						nonEmptyReport = true
 
-						reportBuilder := g.opts.Protocol.NewReportV2Builder()
 						mode := MulticastGroupProtocolV2ReportRecordChangeToExcludeMode
 						if info.deleteScheduled {
 							mode = MulticastGroupProtocolV2ReportRecordChangeToIncludeMode
 						}
 						reportBuilder.AddRecord(mode, groupAddress)
-						// Nothing meaningful we can do with the error here. We will retry
-						// sending a state changed report again anyways.
-						_, _ = reportBuilder.Send()
 
 						if info.deleteScheduled && info.transmissionLeft == 0 {
 							// No more transmissions left so we can actually delete the
@@ -548,6 +545,10 @@ func (g *GenericMulticastProtocolState) sendV2ReportAndMaybeScheduleChangedTimer
 						}
 					}
 
+					// Nothing meaningful we can do with the error here. We will retry
+					// sending a state changed report again anyways.
+					_, _ = reportBuilder.Send()
+
 					if nonEmptyReport {
 						g.stateChangedReportV2Timer.Reset(g.calculateDelayTimerDuration(g.opts.MaxUnsolicitedReportDelay))
 					} else {
diff --git a/pkg/tcpip/network/ipv4/igmp_test.go b/pkg/tcpip/network/ipv4/igmp_test.go
index df62c712b..fb4e0ca00 100644
--- a/pkg/tcpip/network/ipv4/igmp_test.go
+++ b/pkg/tcpip/network/ipv4/igmp_test.go
@@ -332,7 +332,16 @@ func TestSendQueuedIGMPReports(t *testing.T) {
 			// We expect two batches of reports to be sent (1 batch when the address
 			// is assigned, and another after the maximum unsolicited report interval.
 			for i := 0; i < 2; i++ {
-				reportCounter += uint64(len(multicastAddrs))
+				// IGMPv2 always sends a single message per group.
+				//
+				// IGMPv3 sends a single message per group when we first get an
+				// address assigned, but later reports (sent by the state changed
+				// timer) coalesce records for groups.
+				if test.v2Compatibility || i == 0 {
+					reportCounter += uint64(len(multicastAddrs))
+				} else {
+					reportCounter++
+				}
 				test.checkStats(t, s, reportCounter, doneCounter, reportV2Counter)
 				test.validate(t, e, stackAddr, multicastAddrs)
 
diff --git a/pkg/tcpip/network/ipv6/mld_test.go b/pkg/tcpip/network/ipv6/mld_test.go
index 4ce304fd3..4882c6396 100644
--- a/pkg/tcpip/network/ipv6/mld_test.go
+++ b/pkg/tcpip/network/ipv6/mld_test.go
@@ -405,9 +405,16 @@ func TestSendQueuedMLDReports(t *testing.T) {
 					// link-local address is assigned, and another after the maximum
 					// unsolicited report interval.
 					for i := 0; i < 2; i++ {
-						// We expect reports to be sent (one for globalMulticastAddr and another
-						// for linkLocalAddrSNMC).
-						reportCounter += maxReports
+						// MLDv1 always sends a single message per group.
+						//
+						// MLDv2 sends a single message per group when we first get an
+						// IPv6 link-local address assigned, but later reports (sent by
+						// the state changed timer) coalesce records for groups.
+						if subTest.v1Compatibility || i == 0 {
+							reportCounter += maxReports
+						} else {
+							reportCounter++
+						}
 						subTest.checkStats(t, s, reportCounter, doneCounter, reportV2Counter)
 
 						subTest.validate(