Skip to content

Commit

Permalink
MGMT-19927: Modify MTU Check to Only MTUs Greater Than 1500
Browse files Browse the repository at this point in the history
  • Loading branch information
@linoyaslan
linoyaslan committed Feb 18, 2025
1 parent 4327b8d commit 4caa4eb
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 56 deletions.
26 changes: 18 additions & 8 deletions src/connectivity_check/mtu_checker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@ import (
log "github.com/sirupsen/logrus"
)

// Since the exact headers in use are uncertain, the total overhead is estimated to be a maximum of ~100 bytes.
// This includes 18 bytes for VLAN, a maximum of 60 bytes for IPv4/IPv6, 8 bytes for ICMP, and potential additional headers.
// To be on the safe side, we assume an overhead of 150 bytes.
const headers = 150
const defaultMTUThreshold = 1500
const ipv4Header = 28
const ipv6Header = 48

type mtuChecker struct {
executer Executer
Expand All @@ -38,6 +37,13 @@ func (m *mtuChecker) Check(attributes Attributes) ResultReporter {

var localIP string

mtu := attributes.OutgoingNIC.MTU

// Only consider hosts with MTU != 1500
if mtu == defaultMTUThreshold {
return nil
}

for _, addr := range attributes.OutgoingNIC.Addresses {
ipN, ok := addr.(*net.IPNet)
if !ok {
Expand All @@ -53,18 +59,22 @@ func (m *mtuChecker) Check(attributes Attributes) ResultReporter {
continue
}

mtu := attributes.OutgoingNIC.MTU
sizeWithoutHeaders := mtu - headers

// Perform an initial ping without specifying the MTU to rule out the possibility of failure due to issues unrelated to MTU.
_, err := m.executer.Execute("ping", attributes.RemoteIPAddress, "-c", "3", "-I", attributes.OutgoingNIC.Name)
if err != nil {
log.WithError(err).Errorf("MTU checker: failed first ping. Remote address: %s, nic: %s, local address: %s", attributes.RemoteIPAddress, attributes.OutgoingNIC.Name, localIP)
return nil
}

var sizeWithoutIPHeader int
if isLocalIPv6 {
sizeWithoutIPHeader = mtu - ipv6Header
} else {
sizeWithoutIPHeader = mtu - ipv4Header
}

// Second ping with MTU
_, err = m.executer.Execute("ping", attributes.RemoteIPAddress, "-c", "3", "-M", "do", "-s", strconv.Itoa(sizeWithoutHeaders), "-I", attributes.OutgoingNIC.Name)
_, err = m.executer.Execute("ping", attributes.RemoteIPAddress, "-c", "3", "-M", "do", "-s", strconv.Itoa(sizeWithoutIPHeader), "-I", attributes.OutgoingNIC.Name)
if err != nil {
log.WithError(err).Errorf("MTU checker: failed to ping address %s nic %s mtu %d", attributes.RemoteIPAddress, attributes.OutgoingNIC.Name, mtu)
return newMtuResultReporter(&ret)
Expand Down
69 changes: 21 additions & 48 deletions src/connectivity_check/mtu_checker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,13 @@ import (

var _ = Describe("MTU checker", func() {
const (
remoteIPv4Address = "192.168.127.31"
remoteIPv6Address = "3001:db9::22"
outgoingIPv4Address = "192.168.127.30"
outgoingIPv6Address = "3001:db9::1f"
outgoingNIC = "ens3"
bigSize = 9000
regularSize = 1500
bigSizeWithoutHeaders = bigSize - headers
regularSizeWithoutHeaders = regularSize - headers
remoteIPv4Address = "192.168.127.31"
remoteIPv6Address = "3001:db9::22"
outgoingIPv4Address = "192.168.127.30"
outgoingIPv6Address = "3001:db9::1f"
outgoingNIC = "ens3"
bigSize = 9000
regularSize = 1500
)

var (
Expand All @@ -33,13 +31,8 @@ var _ = Describe("MTU checker", func() {
Return("success output", nil).Once()
}

mockSuccessPingWithSize := func(remoteIPAddress string) {
mockExecuter.On("Execute", "ping", remoteIPAddress, "-c", "3", "-M", "do", "-s", strconv.Itoa(regularSizeWithoutHeaders), "-I", outgoingNIC).
Return("success output", nil).Once()
}

mockFailPingWithSize := func(remoteIPAddress string) {
mockExecuter.On("Execute", "ping", remoteIPAddress, "-c", "3", "-M", "do", "-s", strconv.Itoa(bigSizeWithoutHeaders), "-I", outgoingNIC).
mockFailPingWithSize := func(remoteIPAddress string, sizeWithoutIPHeader int) {
mockExecuter.On("Execute", "ping", remoteIPAddress, "-c", "3", "-M", "do", "-s", strconv.Itoa(sizeWithoutIPHeader), "-I", outgoingNIC).
Return("failure output", fmt.Errorf("some error")).Once()
}

Expand All @@ -50,17 +43,16 @@ var _ = Describe("MTU checker", func() {
AfterEach(func() {
mockExecuter.AssertExpectations(GinkgoT())
})
It("MTU Check Failure - IPv4", func() {

It("MTU > 1500, IPv4 - unsuccessful report", func() {
attributes := Attributes{
RemoteIPAddress: remoteIPv4Address,
OutgoingNIC: OutgoingNic{Name: outgoingNIC, MTU: 9000, Addresses: []net.Addr{&net.IPNet{
OutgoingNIC: OutgoingNic{Name: outgoingNIC, MTU: bigSize, Addresses: []net.Addr{&net.IPNet{
IP: net.ParseIP(outgoingIPv4Address),
Mask: net.CIDRMask(24, 32),
}}},
}
mockSuccessPingWithoutSize(remoteIPv4Address)
mockFailPingWithSize(remoteIPv4Address)
mockFailPingWithSize(remoteIPv4Address, bigSize-ipv4Header)
reporter := checker.Check(attributes)
Expect(reporter).ToNot(BeNil())
var resultingHost models.ConnectivityRemoteHost
Expand All @@ -70,17 +62,16 @@ var _ = Describe("MTU checker", func() {
Expect(resultingHost.MtuReport[0].OutgoingNic).To(Equal(outgoingNIC))
Expect(resultingHost.MtuReport[0].MtuSuccessful).To(BeFalse())
})
It("MTU Check Failure - IPv6", func() {

It("MTU > 1500, IPv6 - unsuccessful report", func() {
attributes := Attributes{
RemoteIPAddress: remoteIPv6Address,
OutgoingNIC: OutgoingNic{Name: outgoingNIC, MTU: 9000, Addresses: []net.Addr{&net.IPNet{
OutgoingNIC: OutgoingNic{Name: outgoingNIC, MTU: bigSize, Addresses: []net.Addr{&net.IPNet{
IP: net.ParseIP(outgoingIPv6Address),
Mask: net.CIDRMask(64, 128),
}}},
}
mockSuccessPingWithoutSize(remoteIPv6Address)
mockFailPingWithSize(remoteIPv6Address)
mockFailPingWithSize(remoteIPv6Address, bigSize-ipv6Header)
reporter := checker.Check(attributes)
Expect(reporter).ToNot(BeNil())
var resultingHost models.ConnectivityRemoteHost
Expand All @@ -90,44 +81,26 @@ var _ = Describe("MTU checker", func() {
Expect(resultingHost.MtuReport[0].OutgoingNic).To(Equal(outgoingNIC))
Expect(resultingHost.MtuReport[0].MtuSuccessful).To(BeFalse())
})
It("MTU Check Success - IPv4", func() {

It("MTU equal 1500, IPv4 - should not check", func() {
attributes := Attributes{
RemoteIPAddress: remoteIPv4Address,
OutgoingNIC: OutgoingNic{Name: outgoingNIC, MTU: 1500, Addresses: []net.Addr{&net.IPNet{
OutgoingNIC: OutgoingNic{Name: outgoingNIC, MTU: regularSize, Addresses: []net.Addr{&net.IPNet{
IP: net.ParseIP(outgoingIPv4Address),
Mask: net.CIDRMask(24, 32),
}}},
}
mockSuccessPingWithoutSize(remoteIPv4Address)
mockSuccessPingWithSize(remoteIPv4Address)
reporter := checker.Check(attributes)
Expect(reporter).ToNot(BeNil())
var resultingHost models.ConnectivityRemoteHost
Expect(reporter.Report(&resultingHost)).ToNot(HaveOccurred())
Expect(resultingHost.MtuReport).To(HaveLen(1))
Expect(resultingHost.MtuReport[0].RemoteIPAddress).To(Equal(remoteIPv4Address))
Expect(resultingHost.MtuReport[0].OutgoingNic).To(Equal(outgoingNIC))
Expect(resultingHost.MtuReport[0].MtuSuccessful).To(BeTrue())
Expect(reporter).To(BeNil())
})
It("MTU Check Success - IPv6", func() {

It("MTU equal 1500, IPv6 - should not check", func() {
attributes := Attributes{
RemoteIPAddress: remoteIPv6Address,
OutgoingNIC: OutgoingNic{Name: outgoingNIC, MTU: 1500, Addresses: []net.Addr{&net.IPNet{
OutgoingNIC: OutgoingNic{Name: outgoingNIC, MTU: regularSize, Addresses: []net.Addr{&net.IPNet{
IP: net.ParseIP(outgoingIPv6Address),
Mask: net.CIDRMask(64, 128),
}}},
}
mockSuccessPingWithoutSize(remoteIPv6Address)
mockSuccessPingWithSize(remoteIPv6Address)
reporter := checker.Check(attributes)
Expect(reporter).ToNot(BeNil())
var resultingHost models.ConnectivityRemoteHost
Expect(reporter.Report(&resultingHost)).ToNot(HaveOccurred())
Expect(resultingHost.MtuReport).To(HaveLen(1))
Expect(resultingHost.MtuReport[0].RemoteIPAddress).To(Equal(remoteIPv6Address))
Expect(resultingHost.MtuReport[0].OutgoingNic).To(Equal(outgoingNIC))
Expect(resultingHost.MtuReport[0].MtuSuccessful).To(BeTrue())
Expect(reporter).To(BeNil())
})
})

0 comments on commit 4caa4eb

Please sign in to comment.