Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions pkg/encapsulation/cilium.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,24 @@ import (
"fmt"
"net"

"github.com/vishvananda/netlink"

"github.com/cozystack/kilo/pkg/iproute"
"github.com/cozystack/kilo/pkg/iptables"
)

const ciliumHostIface = "cilium_host"
const (
ciliumHostIface = "cilium_host"
// ciliumTunlIface is the kernel's default IPIP tunnel (tunl0) renamed
// by Cilium when enable-ipip-termination is active. Unlike cilium_ipip4,
// which is receive-only (for DSR), cilium_tunl supports both TX and RX.
ciliumTunlIface = "cilium_tunl"
)

type cilium struct {
iface int
strategy Strategy
iface int
strategy Strategy
ownsTunnel bool
}

// NewCilium returns an encapsulator that uses IPIP tunnels
Expand All @@ -36,7 +45,11 @@ func NewCilium(strategy Strategy) Encapsulator {
}

// CleanUp will remove any created IPIP devices.
// If the tunnel is owned by Cilium, skip removal.
func (c *cilium) CleanUp() error {
if !c.ownsTunnel {
return nil
}
if err := iproute.DeleteAddresses(c.iface); err != nil {
return err
}
Expand Down Expand Up @@ -79,15 +92,25 @@ func (c *cilium) Index() int {
}

// Init initializes the IPIP tunnel interface.
// When Cilium's enable-ipip-termination is active, it renames the kernel's
// tunl0 to cilium_tunl and creates a receive-only cilium_ipip4 device.
// We use cilium_tunl because it supports both sending and receiving IPIP
// traffic, whereas cilium_ipip4 only handles incoming packets (DSR).
func (c *cilium) Init(base int) error {
iface, err := iproute.NewIPIP(base)
if link, err := netlink.LinkByName(ciliumTunlIface); err == nil {
c.iface = link.Attrs().Index
c.ownsTunnel = false
return nil
}
iface, err := iproute.NewIPIPWithName(base, ciliumTunlIface)
if err != nil {
return fmt.Errorf("failed to create tunnel interface: %v", err)
}
if err := iproute.Set(iface, true); err != nil {
return fmt.Errorf("failed to set tunnel interface up: %v", err)
}
c.iface = iface
c.ownsTunnel = true
return nil
Comment on lines +100 to 114

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current implementation of Init returns early if the cilium_tunl interface already exists. This bypasses the MTU configuration (performed inside NewIPIPWithName) and the call to iproute.Set(iface, true), which ensures the interface is UP. Even when reusing an existing interface, Kilo should ensure it is correctly configured for its use case.

Additionally, the ownership check can be integrated more cleanly to ensure the rest of the initialization logic always runs.

	_, err := netlink.LinkByName(ciliumTunlIface)
	c.ownsTunnel = err != nil
	iface, err := iproute.NewIPIPWithName(base, ciliumTunlIface)
	if err != nil {
		return fmt.Errorf("failed to create tunnel interface: %v", err)
	}
	if err := iproute.Set(iface, true); err != nil {
		return fmt.Errorf("failed to set tunnel interface up: %v", err)
	}
	c.iface = iface
	return nil

Comment on lines 99 to 114
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Reused tunnel skips MTU adjustment and link-up.

When cilium_tunl already exists (lines 100–104), Init sets c.iface and returns immediately without:

  1. Ensuring the link is administratively up (iproute.Set(iface, true)).
  2. Setting the MTU to base MTU − 20.

If Cilium has already configured and brought up the tunnel, this is fine in practice. However, if the MTU of the base interface differs from what Cilium assumed, the tunnel MTU could be mismatched, leading to packet drops or fragmentation on the IPIP path.

Consider at minimum setting the MTU on the reused tunnel to stay consistent with the creation path.

Suggested enhancement
 func (c *cilium) Init(base int) error {
 	if link, err := netlink.LinkByName(ciliumTunlIface); err == nil {
 		c.iface = link.Attrs().Index
 		c.ownsTunnel = false
+		// Ensure the MTU matches what Kilo expects.
+		baseLink, err := netlink.LinkByIndex(base)
+		if err != nil {
+			return fmt.Errorf("failed to get base device: %v", err)
+		}
+		mtu := baseLink.Attrs().MTU - 20
+		if err := netlink.LinkSetMTU(link, mtu); err != nil {
+			return fmt.Errorf("failed to set tunnel MTU: %v", err)
+		}
 		return nil
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (c *cilium) Init(base int) error {
iface, err := iproute.NewIPIP(base)
if link, err := netlink.LinkByName(ciliumTunlIface); err == nil {
c.iface = link.Attrs().Index
c.ownsTunnel = false
return nil
}
iface, err := iproute.NewIPIPWithName(base, ciliumTunlIface)
if err != nil {
return fmt.Errorf("failed to create tunnel interface: %v", err)
}
if err := iproute.Set(iface, true); err != nil {
return fmt.Errorf("failed to set tunnel interface up: %v", err)
}
c.iface = iface
c.ownsTunnel = true
return nil
func (c *cilium) Init(base int) error {
if link, err := netlink.LinkByName(ciliumTunlIface); err == nil {
c.iface = link.Attrs().Index
c.ownsTunnel = false
// Ensure the MTU matches what Kilo expects.
baseLink, err := netlink.LinkByIndex(base)
if err != nil {
return fmt.Errorf("failed to get base device: %v", err)
}
mtu := baseLink.Attrs().MTU - 20
if err := netlink.LinkSetMTU(link, mtu); err != nil {
return fmt.Errorf("failed to set tunnel MTU: %v", err)
}
return nil
}
iface, err := iproute.NewIPIPWithName(base, ciliumTunlIface)
if err != nil {
return fmt.Errorf("failed to create tunnel interface: %v", err)
}
if err := iproute.Set(iface, true); err != nil {
return fmt.Errorf("failed to set tunnel interface up: %v", err)
}
c.iface = iface
c.ownsTunnel = true
return nil
🤖 Prompt for AI Agents
In `@pkg/encapsulation/cilium.go` around lines 99 - 114, When Init finds an
existing cilium tunnel (the branch that sees
netlink.LinkByName(ciliumTunlIface)), also bring the link up and ensure its MTU
matches base-20 before returning: set c.iface as you do now, then call the same
helpers used in the creation path (iproute.Set to bring the link up and the
appropriate iproute MTU setter to set MTU = base - 20), and only then set
c.ownsTunnel=false and return; reuse ciliumTunlIface, Init, iproute.Set and the
MTU-setting helper names to locate where to add these calls.

}

Expand Down
18 changes: 12 additions & 6 deletions pkg/iproute/ipip.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,31 @@ import (
)

const (
ipipHeaderSize = 20
tunnelName = "tunl0"
ipipHeaderSize = 20
DefaultTunnelName = "tunl0"
)

// NewIPIP creates an IPIP interface using the base interface
// NewIPIP creates an IPIP interface named tunl0 using the base interface
// to derive the tunnel's MTU.
func NewIPIP(baseIndex int) (int, error) {
link, err := netlink.LinkByName(tunnelName)
return NewIPIPWithName(baseIndex, DefaultTunnelName)
}

// NewIPIPWithName creates a named IPIP interface using the base interface
// to derive the tunnel's MTU.
func NewIPIPWithName(baseIndex int, name string) (int, error) {
link, err := netlink.LinkByName(name)
if err != nil {
// If we failed to find the tunnel, then it probably simply does not exist.
cmd := exec.Command("ip", "tunnel", "add", tunnelName, "mode", "ipip")
cmd := exec.Command("ip", "tunnel", "add", name, "mode", "ipip")
var stderr bytes.Buffer
cmd.Stderr = &stderr
// Sometimes creating a tunnel returns the error "File exists,"
// in which case the interface exists and we can ignore the error.
if err := cmd.Run(); err != nil && !strings.Contains(stderr.String(), "File exists") {
return 0, fmt.Errorf("failed to create IPIP tunnel: %s", stderr.String())
}
link, err = netlink.LinkByName(tunnelName)
link, err = netlink.LinkByName(name)
if err != nil {
return 0, fmt.Errorf("failed to get tunnel device: %v", err)
}
Expand Down