Skip to content
This repository has been archived by the owner on Dec 7, 2023. It is now read-only.

Fix CNI networking issues (MAC duplication + bridge ageing) #638

Merged
merged 3 commits into from
Jul 9, 2020
Merged

Fix CNI networking issues (MAC duplication + bridge ageing) #638

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c80137e
Downstream fix for netlink MAC address changes
twelho Jul 8, 2020
aa075c5
Disable bridge ageing in the container to fix networking
twelho Jul 9, 2020
51bfa3f
Address review feedback
twelho Jul 9, 2020
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
7 changes: 6 additions & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ replace (
k8s.io/kube-openapi => k8s.io/kube-openapi v0.0.0-20200121204235-bf4fb3bd569c
)

// TODO: Remove this when https://github.com/vishvananda/netlink/pull/554 is merged
Copy link
Contributor

Choose a reason for hiding this comment

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

You can comment go.mod??? TIL, thx 🤯

Copy link
Contributor Author

@twelho twelho Jul 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

It ought to be possible, go.mod files seem to just be regular go sources with some special sauce builtins.

replace github.com/vishvananda/netlink => github.com/twelho/netlink v1.1.1-ageing

require (
github.com/alessio/shellescape v1.2.2
github.com/c2h5oh/datasize v0.0.0-20200112174442-28bbd4740fee
Expand Down Expand Up @@ -50,7 +53,9 @@ require (
github.com/syndtr/gocapability v0.0.0-20180916011248-d98352740cb2 // indirect
github.com/vishvananda/netlink v1.1.0
github.com/weaveworks/libgitops v0.0.0-20200611103311-2c871bbbbf0c
golang.org/x/crypto v0.0.0-20200406173513-056763e48d71
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9
golang.org/x/net v0.0.0-20200625001655-4c5254603344 // indirect
golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208 // indirect
golang.org/x/sys v0.0.0-20200610111108-226ff32320da
gotest.tools v2.2.0+incompatible
k8s.io/apimachinery v0.18.3
Expand Down
16 changes: 10 additions & 6 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,8 @@ github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhV
github.com/timakin/bodyclose v0.0.0-20190930140734-f7f2e9bca95e/go.mod h1:Qimiffbc6q9tBWlVV6x0P9sat/ao1xEkREYPPj9hphk=
github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
github.com/twelho/netlink v1.1.1-ageing h1:A5/0WIegqA+Br6hXHqma4MN+bmnWWrZG91LRuveV+Xk=
github.com/twelho/netlink v1.1.1-ageing/go.mod h1:FSQhuTO7eHT34mPzX+B04SUAjiqLxtXs1et0S6l9k4k=
github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0=
github.com/ulikunitz/xz v0.5.5/go.mod h1:2bypXElzHzzJZwzH67Y6wb67pO62Rzfn7BSiF4ABRW8=
Expand All @@ -728,9 +730,6 @@ github.com/valyala/fasttemplate v1.0.1/go.mod h1:UQGH1tvbgY+Nz5t2n7tXsz52dQxojPU
github.com/valyala/quicktemplate v1.2.0/go.mod h1:EH+4AkTd43SvgIbQHYu59/cJyxDoOVRUAfrukLPuGJ4=
github.com/valyala/tcplisten v0.0.0-20161114210144-ceec8f93295a/go.mod h1:v3UYOV9WzVtRmSR+PDvWpU/qWl4Wa5LApYYX4ZtKbio=
github.com/vektah/gqlparser v1.1.2/go.mod h1:1ycwN7Ij5njmMkPPAOaRFY4rET2Enx7IkVv3vaXspKw=
github.com/vishvananda/netlink v0.0.0-20181108222139-023a6dafdcdf/go.mod h1:+SR5DhBJrl6ZM7CoCKvpw5BKroDKQ+PJqOg65H/2ktk=
github.com/vishvananda/netlink v1.1.0 h1:1iyaYNBLmP6L0220aDnYQpo1QEV4t4hJ+xEEhhJH8j0=
github.com/vishvananda/netlink v1.1.0/go.mod h1:cTgwzPIzzgDAYoQrMm0EdrjRUBkTqKYppBueQtXaqoE=
github.com/vishvananda/netns v0.0.0-20180720170159-13995c7128cc/go.mod h1:ZjcWmFBXmLKZu9Nxj3WKYEafiSqer2rnvPr0en9UNpI=
github.com/vishvananda/netns v0.0.0-20191106174202-0a2b9b5464df h1:OviZH7qLw/7ZovXvuNyL3XQl8UFofeikI1NW1Gypu7k=
github.com/vishvananda/netns v0.0.0-20191106174202-0a2b9b5464df/go.mod h1:JP3t17pCcGlemwknint6hfoeCVQrEMVwxRLRjXpq+BU=
Expand Down Expand Up @@ -787,8 +786,8 @@ golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8U
golang.org/x/crypto v0.0.0-20200128174031-69ecbb4d6d5d/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20200220183623-bac4c82f6975/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20200302210943-78000ba7a073/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20200406173513-056763e48d71 h1:DOmugCavvUtnUD114C1Wh+UgTgQZ4pMLzXxi1pSt+/Y=
golang.org/x/crypto v0.0.0-20200406173513-056763e48d71/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 h1:psW17arqaxU48Z5kZ0CQnkZWQJsqcURM6tKiBApRjXI=
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20190312203227-4b39c73a6495/go.mod h1:ZjyILWgesfNpC6sMxTJOJm9Kp84zZh5NQWvqDGG3Qr8=
Expand Down Expand Up @@ -828,6 +827,8 @@ golang.org/x/net v0.0.0-20191004110552-13f9640d40b9/go.mod h1:z5CRVTTTmAJ677TzLL
golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20200301022130-244492dfa37a h1:GuSPYbZzB5/dcLNCwLQLsg3obCJtX9IJhpXkvY7kzk0=
golang.org/x/net v0.0.0-20200301022130-244492dfa37a/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20200625001655-4c5254603344 h1:vGXIOMxbNfDTk/aXCmfdLgkrSV+Z2tcbze+pEc3v5W4=
golang.org/x/net v0.0.0-20200625001655-4c5254603344/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
Expand All @@ -838,6 +839,8 @@ golang.org/x/sync v0.0.0-20190227155943-e225da77a7e6/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e h1:vcxGaoTs7kV8m5Np9uUNQin4BrLOthgV7252N8V+FwY=
golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208 h1:qwRHBd0NqMbJxfbotnDhm2ByMI1Shq4Y6oRJo21SGJA=
golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20170830134202-bb24a47a89ea/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
Expand All @@ -859,7 +862,6 @@ golang.org/x/sys v0.0.0-20190422165155-953cdadca894/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20190502145724-3ef323f4f1fd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20190514135907-3a4b5fb9f71f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20190602015325-4c4f7f33c9ed/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20190606203320-7fc4e5ec1444/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20190616124812-15dcb6c0061f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20190826190057-c7b8b68b1456/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Expand All @@ -871,8 +873,10 @@ golang.org/x/sys v0.0.0-20191022100944-742c48ecaeb7/go.mod h1:h1NjWce9XRLGQEsW7w
golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20191210023423-ac6580df4449/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200120151820-655fe14d7479/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200121082415-34d275377bf9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200122134326-e047566fdf82/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200302150141-5c8b2ff67527/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200610111108-226ff32320da h1:bGb80FudwxpeucJUjPYJXuJ8Hk91vNtfvrymzwiei38=
golang.org/x/sys v0.0.0-20200610111108-226ff32320da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.0.0-20160726164857-2910a502d2bf/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down
99 changes: 67 additions & 32 deletions pkg/container/network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,44 +112,33 @@ func networkSetup(dhcpIfaces *[]DHCPInterface) (bool, error) {
}

// bridge creates the TAP device and performs the bridging, returning the base configuration for a DHCP server
func bridge(iface *net.Interface) (d *DHCPInterface, err error) {
func bridge(iface *net.Interface) (*DHCPInterface, error) {
tapName := constants.TAP_PREFIX + iface.Name
bridgeName := constants.BRIDGE_PREFIX + iface.Name

var handle *netlink.Handle
var bridge *netlink.Bridge
var tuntap, link netlink.Link

if handle, err = netlink.NewHandle(); err != nil {
return
}

if tuntap, err = createTAPAdapter(tapName); err != nil {
return
}

if bridge, err = createBridge(bridgeName); err != nil {
return
eth, err := netlink.LinkByIndex(iface.Index)
if err != nil {
return nil, err
}

if err = handle.LinkSetMaster(tuntap, bridge); err != nil {
return
tuntap, err := createTAPAdapter(tapName)
if err != nil {
return nil, err
}

if link, err = netlink.LinkByName(iface.Name); err != nil {
return
bridge, err := createBridge(bridgeName)
if err != nil {
return nil, err
}

if err = handle.LinkSetMaster(link, bridge); err != nil {
return
if err := setMaster(bridge, tuntap, eth); err != nil {
return nil, err
}

d = &DHCPInterface{
return &DHCPInterface{
VMTAP: tapName,
Bridge: bridgeName,
}

return
}, nil
}

// takeAddress removes the first address of an interface and returns it
Expand All @@ -160,11 +149,6 @@ func takeAddress(iface *net.Interface) (*net.IPNet, bool, error) {
return nil, true, fmt.Errorf("interface %q has no address", iface.Name)
}

handle, err := netlink.NewHandle()
if err != nil {
return nil, false, fmt.Errorf("failed to acquire handle on network namespace: %v", err)
}

for _, addr := range addrs {
var ip net.IP
var mask net.IPMask
Expand Down Expand Up @@ -197,7 +181,7 @@ func takeAddress(iface *net.Interface) (*net.IPNet, bool, error) {
return nil, false, fmt.Errorf("failed to parse address from stringified IP %q: %v", addr.String(), err)
}

if err = handle.AddrDel(link, delAddr); err != nil {
if err = netlink.AddrDel(link, delAddr); err != nil {
return nil, false, fmt.Errorf("failed to remove address from interface %q: %v", iface.Name, err)
}

Expand All @@ -212,6 +196,7 @@ func takeAddress(iface *net.Interface) (*net.IPNet, bool, error) {
return nil, false, fmt.Errorf("interface %s has no valid addresses", iface.Name)
}

// createTAPAdapter creates a new TAP device with the given name
func createTAPAdapter(tapName string) (*netlink.Tuntap, error) {
la := netlink.NewLinkAttrs()
la.Name = tapName
Expand All @@ -222,13 +207,22 @@ func createTAPAdapter(tapName string) (*netlink.Tuntap, error) {
return tuntap, addLink(tuntap)
}

// createBridge creates a new bridge device with the given name
func createBridge(bridgeName string) (*netlink.Bridge, error) {
la := netlink.NewLinkAttrs()
la.Name = bridgeName
bridge := &netlink.Bridge{LinkAttrs: la}

// Disable MAC address age tracking. This causes issues in the container,
// the bridge is unable to resolve MACs from outside resulting in it never
// establishing the internal routes. This "optimization" is only really useful
// with more than two interfaces attached to the bridge anyways, so we're not
// taking any performance hit by disabling it here.
ageingTime := uint32(0)
bridge := &netlink.Bridge{LinkAttrs: la, AgeingTime: &ageingTime}
return bridge, addLink(bridge)
}

// addLink creates the given link and brings it up
func addLink(link netlink.Link) (err error) {
if err = netlink.LinkAdd(link); err == nil {
err = netlink.LinkSetUp(link)
Expand All @@ -237,6 +231,47 @@ func addLink(link netlink.Link) (err error) {
return
}

// This is a MAC address persistence workaround, netlink.LinkSetMaster{,ByIndex}()
// has a bug that arbitrarily changes the MAC addresses of the bridge and virtual
// device to be bound to it. TODO: Remove when fixed upstream
func setMaster(master netlink.Link, links ...netlink.Link) error {
masterIndex := master.Attrs().Index
masterMAC, err := getMAC(master)
if err != nil {
return err
}

for _, link := range links {
mac, err := getMAC(link)
if err != nil {
return err
}

if err = netlink.LinkSetMasterByIndex(link, masterIndex); err != nil {
return err
}

if err = netlink.LinkSetHardwareAddr(link, mac); err != nil {
return err
}
}

return netlink.LinkSetHardwareAddr(master, masterMAC)
}

// getMAC fetches the generated MAC address for the given link
func getMAC(link netlink.Link) (addr net.HardwareAddr, err error) {
// The attributes of the netlink.Link passed to this function do not contain HardwareAddr
// as it is expected to be generated by the networking subsystem. Thus, "reload" the Link
// by querying it to retrieve the generated attributes after the link has been created.
if link, err = netlink.LinkByIndex(link.Attrs().Index); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you "overwriting" this variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The passed link is the "constructor" and has no hardware address since we want it to be generated by the network subsystem, so I need to "reload" the link by requesting the properties from the system after it has been created, which fills all the fields. I'll add a comment to avoid confusion.

return
}

addr = link.Attrs().HardwareAddr
return
}

func maskString(mask net.IPMask) string {
if len(mask) < 4 {
return "<nil>"
Expand Down
1 change: 1 addition & 0 deletions vendor/github.com/vishvananda/netlink/addr.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

35 changes: 16 additions & 19 deletions vendor/github.com/vishvananda/netlink/addr_linux.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading