From ba2d5cb7e1f3ac69a65d0e790f1319082ee01de2 Mon Sep 17 00:00:00 2001 From: Ghanan Gowripalan Date: Mon, 23 Nov 2020 22:45:42 -0800 Subject: Use time.Duration for IGMP Max Response Time field Bug #4682 PiperOrigin-RevId: 343993297 --- pkg/tcpip/checker/checker.go | 5 +++-- pkg/tcpip/header/igmp.go | 17 ++++++++++++++- pkg/tcpip/header/igmp_test.go | 23 ++++++++++++++------- pkg/tcpip/network/ipv4/igmp.go | 40 +++++++++++++++--------------------- pkg/tcpip/network/ipv4/igmp_test.go | 41 ++++++++++++++++++++++--------------- 5 files changed, 77 insertions(+), 49 deletions(-) diff --git a/pkg/tcpip/checker/checker.go b/pkg/tcpip/checker/checker.go index 1c82c2c3b..13bb5a723 100644 --- a/pkg/tcpip/checker/checker.go +++ b/pkg/tcpip/checker/checker.go @@ -20,6 +20,7 @@ import ( "encoding/binary" "reflect" "testing" + "time" "github.com/google/go-cmp/cmp" "gvisor.dev/gvisor/pkg/tcpip" @@ -1267,7 +1268,7 @@ func IGMPType(want header.IGMPType) TransportChecker { } // IGMPMaxRespTime creates a checker that checks the IGMP Max Resp Time field. -func IGMPMaxRespTime(want byte) TransportChecker { +func IGMPMaxRespTime(want time.Duration) TransportChecker { return func(t *testing.T, h header.Transport) { t.Helper() @@ -1276,7 +1277,7 @@ func IGMPMaxRespTime(want byte) TransportChecker { t.Fatalf("got transport header = %T, want = header.IGMP", h) } if got := igmp.MaxRespTime(); got != want { - t.Errorf("got igmp.MaxRespTime() = %d, want = %d", got, want) + t.Errorf("got igmp.MaxRespTime() = %s, want = %s", got, want) } } } diff --git a/pkg/tcpip/header/igmp.go b/pkg/tcpip/header/igmp.go index e0f5d46f4..5c5be1b9d 100644 --- a/pkg/tcpip/header/igmp.go +++ b/pkg/tcpip/header/igmp.go @@ -17,6 +17,7 @@ package header import ( "encoding/binary" "fmt" + "time" "gvisor.dev/gvisor/pkg/tcpip" ) @@ -103,7 +104,15 @@ func (b IGMP) SetType(t IGMPType) { b[igmpTypeOffset] = byte(t) } // MaxRespTime gets the MaxRespTimeField. This is meaningful only in Membership // Query messages, in other cases it is set to 0 by the sender and ignored by // the receiver. -func (b IGMP) MaxRespTime() byte { return b[igmpMaxRespTimeOffset] } +func (b IGMP) MaxRespTime() time.Duration { + // As per RFC 2236 section 2.2, + // + // The Max Response Time field is meaningful only in Membership Query + // messages, and specifies the maximum allowed time before sending a + // responding report in units of 1/10 second. In all other messages, it + // is set to zero by the sender and ignored by receivers. + return DecisecondToDuration(b[igmpMaxRespTimeOffset]) +} // SetMaxRespTime sets the MaxRespTimeField. func (b IGMP) SetMaxRespTime(m byte) { b[igmpMaxRespTimeOffset] = m } @@ -164,3 +173,9 @@ func IGMPCalculateChecksum(h IGMP) uint16 { h.SetChecksum(existingXsum) return xsum } + +// DecisecondToDuration converts a value representing deci-seconds to a +// time.Duration. +func DecisecondToDuration(ds uint8) time.Duration { + return time.Duration(ds) * time.Second / 10 +} diff --git a/pkg/tcpip/header/igmp_test.go b/pkg/tcpip/header/igmp_test.go index 66e872880..b6126d29a 100644 --- a/pkg/tcpip/header/igmp_test.go +++ b/pkg/tcpip/header/igmp_test.go @@ -16,6 +16,7 @@ package header_test import ( "testing" + "time" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/header" @@ -23,10 +24,11 @@ import ( // TestIGMPHeader tests the functions within header.igmp func TestIGMPHeader(t *testing.T) { + const maxRespTimeTenthSec = 0xF0 b := []byte{ - 0x11, // IGMP Type, Membership Query - 0xF0, // Maximum Response Time - 0xC0, 0xC0, // Checksum + 0x11, // IGMP Type, Membership Query + maxRespTimeTenthSec, // Maximum Response Time + 0xC0, 0xC0, // Checksum 0x01, 0x02, 0x03, 0x04, // Group Address } @@ -36,8 +38,8 @@ func TestIGMPHeader(t *testing.T) { t.Errorf("got igmpHeader.Type() = %x, want = %x", got, want) } - if got, want := igmpHeader.MaxRespTime(), byte(0xF0); got != want { - t.Errorf("got igmpHeader.MaxRespTime() = %x, want = %x", got, want) + if got, want := igmpHeader.MaxRespTime(), header.DecisecondToDuration(maxRespTimeTenthSec); got != want { + t.Errorf("got igmpHeader.MaxRespTime() = %s, want = %s", got, want) } if got, want := igmpHeader.Checksum(), uint16(0xC0C0); got != want { @@ -59,8 +61,8 @@ func TestIGMPHeader(t *testing.T) { respTime := byte(0x02) igmpHeader.SetMaxRespTime(respTime) - if got := igmpHeader.MaxRespTime(); got != respTime { - t.Errorf("got igmpHeader.MaxRespTime() = %x, want = %x", got, respTime) + if got, want := igmpHeader.MaxRespTime(), header.DecisecondToDuration(respTime); got != want { + t.Errorf("got igmpHeader.MaxRespTime() = %s, want = %s", got, want) } checksum := uint16(0x0102) @@ -99,3 +101,10 @@ func TestIGMPChecksum(t *testing.T) { t.Errorf("got IGMPCalculateChecksum = %x, want %x", got, checksum) } } + +func TestDecisecondToDuration(t *testing.T) { + const valueInDeciseconds = 5 + if got, want := header.DecisecondToDuration(valueInDeciseconds), valueInDeciseconds*time.Second/10; got != want { + t.Fatalf("got header.DecisecondToDuration(%d) = %s, want = %s", valueInDeciseconds, got, want) + } +} diff --git a/pkg/tcpip/network/ipv4/igmp.go b/pkg/tcpip/network/ipv4/igmp.go index e1de58f73..18fe2fd2f 100644 --- a/pkg/tcpip/network/ipv4/igmp.go +++ b/pkg/tcpip/network/ipv4/igmp.go @@ -35,15 +35,18 @@ const ( // See note on igmpState.igmpV1Present for more detail. v1RouterPresentTimeout = 400 * time.Second - // v1MaxRespTimeTenthSec from RFC 2236 Section 4, Page 5. "The IGMPv1 router + // v1MaxRespTime from RFC 2236 Section 4, Page 5. "The IGMPv1 router // will send General Queries with the Max Response Time set to 0. This MUST // be interpreted as a value of 100 (10 seconds)." - v1MaxRespTimeTenthSec = 100 - - // UnsolicitedReportIntervalMaxTenthSec from RFC 2236 Section 8.10, Page 19. - // As all IGMP delay timers are set to a random value between 0 and the - // interval, this is technically a maximum. - UnsolicitedReportIntervalMaxTenthSec = 100 + // + // Note that the Max Response Time field is a value in units of deciseconds. + v1MaxRespTime = 10 * time.Second + + // UnsolicitedReportIntervalMax is the maximum delay between sending + // unsolicited IGMP reports. + // + // Obtained from RFC 2236 Section 8.10, Page 19. + UnsolicitedReportIntervalMax = 10 * time.Second ) // igmpState is the per-interface IGMP state. @@ -185,7 +188,7 @@ func (igmp *igmpState) handleIGMP(pkt *stack.PacketBuffer) { } } -func (igmp *igmpState) handleMembershipQuery(groupAddress tcpip.Address, maxRespTime byte) { +func (igmp *igmpState) handleMembershipQuery(groupAddress tcpip.Address, maxRespTime time.Duration) { igmp.mu.Lock() defer igmp.mu.Unlock() @@ -196,7 +199,7 @@ func (igmp *igmpState) handleMembershipQuery(groupAddress tcpip.Address, maxResp igmp.mu.igmpV1Job.Cancel() igmp.mu.igmpV1Job.Schedule(v1RouterPresentTimeout) igmp.mu.igmpV1Present = true - maxRespTime = v1MaxRespTimeTenthSec + maxRespTime = v1MaxRespTime } // IPv4Any is the General Query Address. @@ -215,7 +218,7 @@ func (igmp *igmpState) handleMembershipQuery(groupAddress tcpip.Address, maxResp // modify IGMP state directly. // // Precondition: igmp.mu MUST be read locked. -func (igmp *igmpState) setDelayTimerForAddressRLocked(groupAddress tcpip.Address, info *membershipInfo, maxRespTime byte) { +func (igmp *igmpState) setDelayTimerForAddressRLocked(groupAddress tcpip.Address, info *membershipInfo, maxRespTime time.Duration) { if info.state == delayingMember { // As per RFC 2236 Section 3, page 3: "If a timer for the group is already // running, it is reset to the random value only if the requested Max @@ -352,7 +355,7 @@ func (igmp *igmpState) joinGroup(groupAddress tcpip.Address) *tcpip.Error { // it should immediately transmit an unsolicited Version 2 Membership Report // for that group" ... "it is recommended that it be repeated" igmp.sendReportLocked(groupAddress) - igmp.setDelayTimerForAddressRLocked(groupAddress, &info, UnsolicitedReportIntervalMaxTenthSec) + igmp.setDelayTimerForAddressRLocked(groupAddress, &info, UnsolicitedReportIntervalMax) igmp.mu.memberships[groupAddress] = info return nil @@ -383,16 +386,7 @@ func (igmp *igmpState) leaveGroup(groupAddress tcpip.Address) { } // RFC 2236 Section 3, Page 3: The response time is set to a "random value... -// selected from the range (0, Max Response Time]" where Max Resp Time is given -// in units of 1/10 of a second. -func (igmp *igmpState) calculateDelayTimerDuration(maxRespTime byte) time.Duration { - maxRespTimeDuration := DecisecondToSecond(maxRespTime) - return time.Duration(igmp.ep.protocol.stack.Rand().Int63n(int64(maxRespTimeDuration))) -} - -// DecisecondToSecond converts a byte representing deci-seconds to a Duration -// type. This helper function exists because the IGMP stack sends and receives -// Max Response Times in deci-seconds. -func DecisecondToSecond(ds byte) time.Duration { - return time.Duration(ds) * time.Second / 10 +// selected from the range (0, Max Response Time]". +func (igmp *igmpState) calculateDelayTimerDuration(maxRespTime time.Duration) time.Duration { + return time.Duration(igmp.ep.protocol.stack.Rand().Int63n(int64(maxRespTime))) } diff --git a/pkg/tcpip/network/ipv4/igmp_test.go b/pkg/tcpip/network/ipv4/igmp_test.go index a0f37885a..4873a336f 100644 --- a/pkg/tcpip/network/ipv4/igmp_test.go +++ b/pkg/tcpip/network/ipv4/igmp_test.go @@ -15,7 +15,9 @@ package ipv4_test import ( + "fmt" "testing" + "time" "gvisor.dev/gvisor/pkg/tcpip" "gvisor.dev/gvisor/pkg/tcpip/buffer" @@ -35,9 +37,16 @@ const ( ) var ( - // unsolicitedReportIntervalMax is the maximum amount of time the NIC will - // wait before sending an unsolicited report after joining a multicast group. - unsolicitedReportIntervalMax = ipv4.DecisecondToSecond(ipv4.UnsolicitedReportIntervalMaxTenthSec) + // unsolicitedReportIntervalMaxTenthSec is the maximum amount of time the NIC + // will wait before sending an unsolicited report after joining a multicast + // group, in deciseconds. + unsolicitedReportIntervalMaxTenthSec = func() uint8 { + const decisecond = time.Second / 10 + if ipv4.UnsolicitedReportIntervalMax%decisecond != 0 { + panic(fmt.Sprintf("UnsolicitedReportIntervalMax of %d is a lossy conversion to deciseconds", ipv4.UnsolicitedReportIntervalMax)) + } + return uint8(ipv4.UnsolicitedReportIntervalMax / decisecond) + }() ) // validateIgmpPacket checks that a passed PacketInfo is an IPv4 IGMP packet @@ -51,7 +60,7 @@ func validateIgmpPacket(t *testing.T, p channel.PacketInfo, remoteAddress tcpip. checker.DstAddr(remoteAddress), checker.IGMP( checker.IGMPType(igmpType), - checker.IGMPMaxRespTime(maxRespTime), + checker.IGMPMaxRespTime(header.DecisecondToDuration(maxRespTime)), checker.IGMPGroupAddress(groupAddress), ), ) @@ -134,7 +143,7 @@ func TestIgmpDisabled(t *testing.T) { // Inject a General Membership Query, which is an IGMP Membership Query with // a zeroed Group Address (IPv4Any) to verify that it does not reach the // handler. - createAndInjectIGMPPacket(e, header.IGMPMembershipQuery, ipv4.UnsolicitedReportIntervalMaxTenthSec, header.IPv4Any) + createAndInjectIGMPPacket(e, header.IGMPMembershipQuery, unsolicitedReportIntervalMaxTenthSec, header.IPv4Any) if got := s.Stats().IGMP.PacketsReceived.MembershipQuery.Value(); got != 0 { t.Fatalf("got Membership Queries received = %d, want = 0", got) @@ -161,7 +170,7 @@ func TestIgmpReceivesIGMPMessages(t *testing.T) { { name: "General Membership Query", headerType: header.IGMPMembershipQuery, - maxRespTime: ipv4.UnsolicitedReportIntervalMaxTenthSec, + maxRespTime: unsolicitedReportIntervalMaxTenthSec, groupAddress: header.IPv4Any, statCounter: func(stats tcpip.IGMPReceivedPacketStats) *tcpip.StatCounter { return stats.MembershipQuery @@ -234,12 +243,12 @@ func TestIgmpJoinGroup(t *testing.T) { } // Verify the second Membership Report is sent after a random interval up to - // the unsolicitedReportIntervalMax. + // the maximum unsolicited report interval. p, ok = e.Read() if ok { t.Fatalf("sent unexpected packet, expected V2MembershipReport only after advancing the clock = %+v", p.Pkt) } - clock.Advance(unsolicitedReportIntervalMax) + clock.Advance(ipv4.UnsolicitedReportIntervalMax) p, ok = e.Read() if !ok { t.Fatal("unable to Read IGMP packet, expected V2MembershipReport") @@ -273,13 +282,13 @@ func TestIgmpLeaveGroup(t *testing.T) { } // Verify the second Membership Report is sent after a random interval up to - // the unsolicitedReportIntervalMax, and is sent to the multicast address - // being joined. + // the maximum unsolicited report interval, and is sent to the multicast + // address being joined. p, ok = e.Read() if ok { t.Fatalf("sent unexpected packet, expected V2MembershipReport only after advancing the clock = %+v", p.Pkt) } - clock.Advance(unsolicitedReportIntervalMax) + clock.Advance(ipv4.UnsolicitedReportIntervalMax) p, ok = e.Read() if !ok { t.Fatal("unable to Read IGMP packet, expected V2MembershipReport") @@ -334,7 +343,7 @@ func TestIgmpJoinLeaveGroup(t *testing.T) { // Wait for the standard IGMP Unsolicited Report Interval duration before // verifying that the unsolicited Membership Report was sent after leaving // the group. - clock.Advance(unsolicitedReportIntervalMax) + clock.Advance(ipv4.UnsolicitedReportIntervalMax) if got := s.Stats().IGMP.PacketsSent.V2MembershipReport.Value(); got != 1 { t.Fatalf("got V2MembershipReport messages sent = %d, want = 1", got) } @@ -365,7 +374,7 @@ func TestIgmpMembershipQueryReport(t *testing.T) { if ok { t.Fatalf("sent unexpected packet, expected V2MembershipReport only after advancing the clock = %+v", p.Pkt) } - clock.Advance(unsolicitedReportIntervalMax) + clock.Advance(ipv4.UnsolicitedReportIntervalMax) p, ok = e.Read() if !ok { t.Fatal("unable to Read IGMP packet, expected V2MembershipReport") @@ -384,7 +393,7 @@ func TestIgmpMembershipQueryReport(t *testing.T) { if ok { t.Fatalf("sent unexpected packet, expected V2MembershipReport only after advancing the clock = %+v", p.Pkt) } - clock.Advance(ipv4.DecisecondToSecond(maxRespTimeDS)) + clock.Advance(header.DecisecondToDuration(maxRespTimeDS)) p, ok = e.Read() if !ok { t.Fatal("unable to Read IGMP packet, expected V2MembershipReport") @@ -428,7 +437,7 @@ func TestIgmpMultipleHosts(t *testing.T) { // Wait to be sure that no Leave Group messages were sent up to the max // unsolicited report interval since it was not the last host to join this // group. - clock.Advance(unsolicitedReportIntervalMax) + clock.Advance(ipv4.UnsolicitedReportIntervalMax) if got := s.Stats().IGMP.PacketsSent.LeaveGroup.Value(); got != 0 { t.Fatalf("got LeaveGroup messages sent = %d, want = 0", got) } @@ -479,7 +488,7 @@ func TestIgmpV1Present(t *testing.T) { if ok { t.Fatalf("sent unexpected packet, expected V1MembershipReport only after advancing the clock = %+v", p.Pkt) } - clock.Advance(unsolicitedReportIntervalMax) + clock.Advance(ipv4.UnsolicitedReportIntervalMax) p, ok = e.Read() if !ok { t.Fatal("unable to Read IGMP packet, expected V1MembershipReport") -- cgit v1.2.3