From 5343bef216bd7bdb02495f6bdff5c32c49b45a4b Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Fri, 25 Apr 2025 15:36:17 +0200 Subject: [PATCH 01/22] incusd/ip/utils: Switch to netlink Signed-off-by: Gwendolyn --- internal/server/ip/utils.go | 116 ++++++++++++++++++++++++------------ 1 file changed, 77 insertions(+), 39 deletions(-) diff --git a/internal/server/ip/utils.go b/internal/server/ip/utils.go index 905cd929238..f6215bbf21b 100644 --- a/internal/server/ip/utils.go +++ b/internal/server/ip/utils.go @@ -1,10 +1,11 @@ package ip import ( - "encoding/json" "fmt" - "io" - "os/exec" + "strconv" + "strings" + + "github.com/vishvananda/netlink" ) // FamilyV4 represents IPv4 protocol family. @@ -15,63 +16,100 @@ const FamilyV6 = "-6" // LinkInfo represents the IP link details. type LinkInfo struct { - InterfaceName string `json:"ifname"` - Link string `json:"link"` - Master string `json:"master"` - Address string `json:"address"` - TXQueueLength uint32 `json:"txqlen"` - MTU uint32 `json:"mtu"` - OperationalState string `json:"operstate"` + InterfaceName string + Link string + Master string + Address string + TXQueueLength uint32 + MTU uint32 + OperationalState string Info struct { - Kind string `json:"info_kind"` - SlaveKind string `json:"info_slave_kind"` + Kind string + SlaveKind string Data struct { - Protocol string `json:"protocol"` - ID int `json:"id"` - } `json:"info_data"` - } `json:"linkinfo"` + Protocol string + ID int + } + } +} + +func linkByName(name string) (netlink.Link, error) { + link, err := netlink.LinkByName(name) + if err != nil { + return nil, fmt.Errorf("failed to get link %q: %w", name, err) + } + + return link, nil } // GetLinkInfoByName returns the detailed information for the given link. func GetLinkInfoByName(name string) (LinkInfo, error) { - ipPath, err := exec.LookPath("ip") + info := LinkInfo{} + + link, err := linkByName(name) if err != nil { - return LinkInfo{}, fmt.Errorf("ip command not found") + return info, err } - cmd := exec.Command(ipPath, "-j", "-d", "link", "show", name) - stdout, err := cmd.StdoutPipe() - if err != nil { - return LinkInfo{}, err + info.InterfaceName = link.Attrs().Name + + if link.Attrs().ParentIndex != 0 { + parentLink, err := netlink.LinkByIndex(link.Attrs().ParentIndex) + if err != nil { + return info, fmt.Errorf("failed to get parent link %d of %q: %w", link.Attrs().ParentIndex, name, err) + } + + info.Link = parentLink.Attrs().Name } - defer func() { _ = stdout.Close() }() + if link.Attrs().MasterIndex != 0 { + masterLink, err := netlink.LinkByIndex(link.Attrs().MasterIndex) + if err != nil { + return info, fmt.Errorf("failed to get master link %d of %q: %w", link.Attrs().ParentIndex, name, err) + } - err = cmd.Start() - if err != nil { - return LinkInfo{}, err + info.Master = masterLink.Attrs().Name + } + + info.Address = link.Attrs().HardwareAddr.String() + info.TXQueueLength = uint32(link.Attrs().TxQLen) + info.MTU = uint32(link.Attrs().MTU) + info.OperationalState = link.Attrs().OperState.String() + info.Info.Kind = link.Type() + + if link.Attrs().Slave != nil { + info.Info.Kind = link.Attrs().Slave.SlaveType() + } + + vlan, ok := link.(*netlink.Vlan) + if ok { + info.Info.Data.ID = vlan.VlanId + info.Info.Data.Protocol = vlan.VlanProtocol.String() } - defer func() { _ = cmd.Wait() }() + return info, nil +} - // Struct to decode ip output into. - var linkInfoJSON []LinkInfo +func parseHandle(id string) (uint32, error) { + if id == "root" { + return netlink.HANDLE_ROOT, nil + } - // Decode JSON output. - dec := json.NewDecoder(stdout) - err = dec.Decode(&linkInfoJSON) - if err != nil && err != io.EOF { - return LinkInfo{}, err + majorStr, minorStr, found := strings.Cut(id, ":") + + if !found { + return 0, fmt.Errorf("invalid handle %q", id) } - err = cmd.Wait() + major, err := strconv.ParseUint(majorStr, 16, 16) if err != nil { - return LinkInfo{}, fmt.Errorf("no matching link found") + return 0, fmt.Errorf("invalid handle %q: %w", id, err) } - if len(linkInfoJSON) == 0 { - return LinkInfo{}, fmt.Errorf("no matching link found") + minor, err := strconv.ParseUint(minorStr, 16, 16) + if err != nil { + return 0, fmt.Errorf("invalid handle %q: %w", id, err) } - return linkInfoJSON[0], nil + return netlink.MakeHandle(uint16(major), uint16(minor)), nil } From f981481563c6f1569c21d15e35256b5b824d8f8f Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Thu, 24 Apr 2025 00:22:06 +0200 Subject: [PATCH 02/22] incusd/ip/addr: Switch to netlink Signed-off-by: Gwendolyn --- internal/server/ip/addr.go | 82 +++++++++++++++++++++++++++++++++----- 1 file changed, 73 insertions(+), 9 deletions(-) diff --git a/internal/server/ip/addr.go b/internal/server/ip/addr.go index a871ae86138..5263b556c48 100644 --- a/internal/server/ip/addr.go +++ b/internal/server/ip/addr.go @@ -1,7 +1,9 @@ package ip import ( - "github.com/lxc/incus/v6/shared/subprocess" + "fmt" + + "github.com/vishvananda/netlink" ) // Addr represents arguments for address protocol manipulation. @@ -14,30 +16,92 @@ type Addr struct { // Add adds new protocol address. func (a *Addr) Add() error { - _, err := subprocess.RunCommand("ip", a.Family, "addr", "add", "dev", a.DevName, a.Address) + addr, err := netlink.ParseIPNet(a.Address) + if err != nil { + return err + } + + scope, err := a.scopeNum() if err != nil { return err } + err = netlink.AddrAdd(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: a.DevName, + }, + }, &netlink.Addr{ + IPNet: addr, + Scope: scope, + }) + if err != nil { + return fmt.Errorf("failed to add address %v: %w", addr, err) + } + return nil } +func (a *Addr) scopeNum() (int, error) { + var scope netlink.Scope + switch a.Scope { + case "global", "universe", "": + scope = netlink.SCOPE_UNIVERSE + case "site": + scope = netlink.SCOPE_SITE + case "link": + scope = netlink.SCOPE_LINK + case "host": + scope = netlink.SCOPE_HOST + case "nowhere": + scope = netlink.SCOPE_NOWHERE + default: + return 0, fmt.Errorf("unknown address scope %q", a.Scope) + } + + return int(scope), nil +} + // Flush flushes protocol addresses. func (a *Addr) Flush() error { - cmd := []string{} - if a.Family != "" { - cmd = append(cmd, a.Family) + var family int + switch a.Family { + case FamilyV4: + family = netlink.FAMILY_V4 + case FamilyV6: + family = netlink.FAMILY_V6 + case "": + family = netlink.FAMILY_ALL + default: + return fmt.Errorf("unknown address family %q", a.Family) + } + + link, err := linkByName(a.DevName) + if err != nil { + return err } - cmd = append(cmd, "addr", "flush", "dev", a.DevName) - if a.Scope != "" { - cmd = append(cmd, "scope", a.Scope) + addrs, err := netlink.AddrList(link, family) + if err != nil { + return fmt.Errorf("failed to get addresses for device %s: %w", a.DevName, err) } - _, err := subprocess.RunCommand("ip", cmd...) + scope, err := a.scopeNum() if err != nil { return err } + // TODO: iproute2 supposedly does some kind of batch delete in multiple rounds, which is probably more performant than deleting addresses one by one, but the iproute2 code is arcane and uncommented and I haven't yet figured out how it works + + for _, addr := range addrs { + if a.Scope != "" && scope != addr.Scope { + continue + } + + err := netlink.AddrDel(link, &addr) + if err != nil { + return fmt.Errorf("failed to delete address %v: %w", addr, err) + } + } + return nil } From e178d2aec177a9d5bd4c6d7f72da3f6c66803552 Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Thu, 24 Apr 2025 00:39:38 +0200 Subject: [PATCH 03/22] incusd/ip/class: Switch to netlink Signed-off-by: Gwendolyn --- internal/server/ip/class.go | 44 +++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/internal/server/ip/class.go b/internal/server/ip/class.go index 1669a2ba9c6..d69b70ccc15 100644 --- a/internal/server/ip/class.go +++ b/internal/server/ip/class.go @@ -1,7 +1,11 @@ package ip import ( - "github.com/lxc/incus/v6/shared/subprocess" + "fmt" + + "github.com/vishvananda/netlink" + + "github.com/lxc/incus/v6/shared/units" ) // Class represents qdisc class object. @@ -19,19 +23,45 @@ type ClassHTB struct { // Add adds class to a node. func (class *ClassHTB) Add() error { - cmd := []string{"class", "add", "dev", class.Dev, "parent", class.Parent} + link, err := linkByName(class.Dev) + if err != nil { + return err + } + + parent, err := parseHandle(class.Parent) + if err != nil { + return err + } + + classAttrs := netlink.ClassAttrs{ + LinkIndex: link.Attrs().Index, + Parent: parent, + Statistics: nil, + } + + htbClassAttrs := netlink.HtbClassAttrs{} + if class.Classid != "" { - cmd = append(cmd, "classid", class.Classid) + handle, err := parseHandle(class.Classid) + if err != nil { + return err + } + + classAttrs.Handle = handle } - cmd = append(cmd, "htb") if class.Rate != "" { - cmd = append(cmd, "rate", class.Rate) + rate, err := units.ParseBitSizeString(class.Rate) + if err != nil { + return fmt.Errorf("invalid rate %q: %w", class.Rate, err) + } + + htbClassAttrs.Rate = uint64(rate) } - _, err := subprocess.RunCommand("tc", cmd...) + err = netlink.ClassAdd(netlink.NewHtbClass(classAttrs, htbClassAttrs)) if err != nil { - return err + return fmt.Errorf("failed to add htb class: %w", err) } return nil From ab64cea516bbac34ceaad13b424f5d4afc0abdb8 Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Thu, 24 Apr 2025 01:38:56 +0200 Subject: [PATCH 04/22] incusd/ip/filter: Switch to netlink Signed-off-by: Gwendolyn --- .../server/device/device_utils_network.go | 2 +- internal/server/ip/filter.go | 124 +++++++++++++++--- 2 files changed, 105 insertions(+), 21 deletions(-) diff --git a/internal/server/device/device_utils_network.go b/internal/server/device/device_utils_network.go index 0ae4c5cb6f8..798d503dff5 100644 --- a/internal/server/device/device_utils_network.go +++ b/internal/server/device/device_utils_network.go @@ -561,7 +561,7 @@ func networkSetupHostVethLimits(d *deviceCommon, oldConfig deviceConfig.Device, return fmt.Errorf("Failed to create ingress tc qdisc: %s", err) } - police := &ip.ActionPolice{Rate: fmt.Sprintf("%dbit", egressInt), Burst: fmt.Sprintf("%d", egressInt/40), Mtu: "64kb", Drop: true} + police := &ip.ActionPolice{Rate: fmt.Sprintf("%dbit", egressInt), Burst: fmt.Sprintf("%d", egressInt/40), Mtu: "64kB", Drop: true} filter := &ip.U32Filter{Filter: ip.Filter{Dev: veth, Parent: "ffff:0", Protocol: "all"}, Value: "0", Mask: "0", Actions: []ip.Action{police}} err = filter.Add() if err != nil { diff --git a/internal/server/ip/filter.go b/internal/server/ip/filter.go index 185ce855c2f..bdb9c70537f 100644 --- a/internal/server/ip/filter.go +++ b/internal/server/ip/filter.go @@ -1,12 +1,18 @@ package ip import ( - "github.com/lxc/incus/v6/shared/subprocess" + "fmt" + "strconv" + + "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" + + "github.com/lxc/incus/v6/shared/units" ) // Action represents an action in filter. type Action interface { - AddAction() []string + toNetlink() (netlink.Action, error) } // ActionPolice represents an action of 'police' type. @@ -17,26 +23,46 @@ type ActionPolice struct { Drop bool } -// AddAction generates a part of command specific for 'police' action. -func (a *ActionPolice) AddAction() []string { - result := []string{"police"} +func (a *ActionPolice) toNetlink() (netlink.Action, error) { + action := netlink.NewPoliceAction() + + action.ExceedAction = netlink.TC_POLICE_RECLASSIFY + if a.Rate != "" { - result = append(result, "rate", a.Rate) + // TODO: just pass around as number? + rate, err := units.ParseBitSizeString(a.Rate) + if err != nil { + return nil, fmt.Errorf("invalid rate %q: %w", a.Rate, err) + } + + action.Rate = uint32(rate / 8) // netlink wants the rate in bytes/s } if a.Burst != "" { - result = append(result, "burst", a.Burst) + // TODO: just pass around as number? + burst, err := strconv.ParseUint(a.Burst, 10, 32) + if err != nil { + return nil, fmt.Errorf("invalid burst %q: %w", a.Burst, err) + } + + action.Burst = uint32(burst) } if a.Mtu != "" { - result = append(result, "mtu", a.Mtu) + // TODO: just pass around as number? + mtu, err := units.ParseByteSizeString(a.Mtu) + if err != nil { + return nil, fmt.Errorf("invalid mtu %q: %w", a.Mtu, err) + } + + action.Mtu = uint32(mtu) } if a.Drop { - result = append(result, "drop") + action.ExceedAction = netlink.TC_POLICE_SHOT } - return result + return action, nil } // Filter represents filter object. @@ -55,28 +81,86 @@ type U32Filter struct { Actions []Action } +func parseProtocol(proto string) (uint16, error) { + // TODO: add other proto values + switch proto { + case "all": + return unix.ETH_P_ALL, nil + default: + return 0, fmt.Errorf("unknown protocol %q", proto) + } +} + // Add adds universal 32bit traffic control filter to a node. func (u32 *U32Filter) Add() error { - cmd := []string{"filter", "add", "dev", u32.Dev} - if u32.Parent != "" { - cmd = append(cmd, "parent", u32.Parent) + link, err := linkByName(u32.Dev) + if err != nil { + return err } - cmd = append(cmd, "protocol", u32.Protocol) - cmd = append(cmd, "u32", "match", "u32", u32.Value, u32.Mask) + proto, err := parseProtocol(u32.Protocol) + if err != nil { + return err + } + + // TODO: should just be passed around as an int + mask, err := strconv.ParseUint(u32.Mask, 10, 32) + if err != nil { + return fmt.Errorf("invalid mask %v: %w", u32.Mask, err) + } + + value, err := strconv.ParseUint(u32.Value, 10, 32) + if err != nil { + return fmt.Errorf("invalid value %v: %w", u32.Mask, err) + } + + filter := &netlink.U32{ + FilterAttrs: netlink.FilterAttrs{ + LinkIndex: link.Attrs().Index, + Protocol: proto, + Chain: nil, + }, + Sel: &netlink.TcU32Sel{ + Nkeys: 1, + Keys: []netlink.TcU32Key{ + { + Mask: uint32(mask), + Val: uint32(value), + }, + }, + }, + } for _, action := range u32.Actions { - actionCmd := action.AddAction() - cmd = append(cmd, actionCmd...) + netlinkAction, err := action.toNetlink() + if err != nil { + return err + } + + filter.Actions = append(filter.Actions, netlinkAction) + } + + if u32.Parent != "" { + parent, err := parseHandle(u32.Parent) + if err != nil { + return err + } + + filter.Parent = parent } if u32.Flowid != "" { - cmd = append(cmd, "flowid", u32.Flowid) + flowid, err := parseHandle(u32.Flowid) + if err != nil { + return err + } + + filter.ClassId = flowid } - _, err := subprocess.RunCommand("tc", cmd...) + err = netlink.FilterAdd(filter) if err != nil { - return err + return fmt.Errorf("failed to add filter %v: %w", filter, err) } return nil From ff84cd85c491b024af27dd340e90a4120d0244f7 Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Thu, 24 Apr 2025 11:25:49 +0200 Subject: [PATCH 05/22] incusd/ip/link: Switch to netlink (incomplete) Signed-off-by: Gwendolyn --- .../server/device/device_utils_network.go | 4 +- internal/server/ip/link.go | 511 +++++++----------- internal/server/ip/link_bridge.go | 13 +- internal/server/ip/link_dummy.go | 13 +- internal/server/ip/link_gretap.go | 33 +- internal/server/ip/link_macvlan.go | 33 +- internal/server/ip/link_macvtap.go | 16 +- internal/server/ip/link_veth.go | 3 +- internal/server/ip/link_vlan.go | 1 + internal/server/ip/link_vxlan.go | 90 ++- 10 files changed, 384 insertions(+), 333 deletions(-) diff --git a/internal/server/device/device_utils_network.go b/internal/server/device/device_utils_network.go index 798d503dff5..d7eeb421351 100644 --- a/internal/server/device/device_utils_network.go +++ b/internal/server/device/device_utils_network.go @@ -717,9 +717,9 @@ func networkSRIOVSetupVF(d deviceCommon, vfParent string, vfDevice string, vfID // Record properties of VF settings on the parent device. volatile["last_state.vf.parent"] = vfParent - volatile["last_state.vf.hwaddr"] = vfInfo.Address + volatile["last_state.vf.hwaddr"] = vfInfo.Address.String() volatile["last_state.vf.id"] = fmt.Sprintf("%d", vfID) - volatile["last_state.vf.vlan"] = fmt.Sprintf("%d", vfInfo.VLANs[0]["vlan"]) + volatile["last_state.vf.vlan"] = fmt.Sprintf("%d", vfInfo.VLAN) volatile["last_state.vf.spoofcheck"] = fmt.Sprintf("%t", vfInfo.SpoofCheck) // Record the host interface we represents the VF device which we will move into instance. diff --git a/internal/server/ip/link.go b/internal/server/ip/link.go index a7841ae3a70..2a0d1e7c1bd 100644 --- a/internal/server/ip/link.go +++ b/internal/server/ip/link.go @@ -1,17 +1,13 @@ package ip import ( - "bufio" - "encoding/json" "fmt" - "io" "net" - "os/exec" - "regexp" "strconv" + "github.com/vishvananda/netlink" + "github.com/lxc/incus/v6/shared/subprocess" - "github.com/lxc/incus/v6/shared/util" ) // Link represents base arguments for link device. @@ -27,21 +23,6 @@ type Link struct { Up bool } -type jsonLink struct { - Name string `json:"ifname"` - MTU uint32 `json:"mtu"` - Parent string `json:"link"` - Address string `json:"address"` - TXQueueLength uint32 `json:"txqlen"` - AllMulticast int `json:"allmulti"` - Master string `json:"master"` - Up string `json:"operstate"` - Type string `json:"link_type"` - Info struct { - Kind string `json:"info_kind"` - } `json:"linkinfo"` -} - // args generate common arguments for the virtual link. func (l *Link) args() []string { var result []string @@ -95,415 +76,337 @@ func (l *Link) add(linkType string, additionalArgs []string) error { return nil } -// LinkFromName returns a Link from a device name. -func LinkFromName(name string) (*Link, error) { - out, err := subprocess.RunCommand("ip", "-d", "-j", "link", "show", name) - if err != nil { - return nil, err +func (l *Link) netlinkAttrs() (netlink.LinkAttrs, error) { + linkAttrs := netlink.NewLinkAttrs() + + linkAttrs.Name = l.Name + + if l.MTU != 0 { + linkAttrs.MTU = int(l.MTU) } - var links []jsonLink - err = json.Unmarshal([]byte(out), &links) - if err != nil { - return nil, fmt.Errorf("Failed to decode JSON link representation: %w", err) + if l.Address != nil { + linkAttrs.HardwareAddr = l.Address } - jl := &links[0] - l := &Link{ - Name: jl.Name, - Kind: jl.Info.Kind, - MTU: jl.MTU, - Parent: jl.Parent, - TXQueueLength: jl.TXQueueLength, - Master: jl.Master, + if l.TXQueueLength != 0 { + linkAttrs.TxQLen = int(l.TXQueueLength) } - if jl.Type == "ether" && jl.Address != "" { - l.Address, err = net.ParseMAC(jl.Address) + if l.Parent != "" { + parentLink, err := linkByName(l.Parent) if err != nil { - return nil, err + return netlink.LinkAttrs{}, err } + + linkAttrs.ParentIndex = parentLink.Attrs().Index } - if jl.AllMulticast == 1 { - l.AllMulticast = true + if l.Master != "" { + masterLink, err := linkByName(l.Master) + if err != nil { + return netlink.LinkAttrs{}, err + } + + linkAttrs.MasterIndex = masterLink.Attrs().Index } - if jl.Up == "UP" { - l.Up = true + if l.Up { + linkAttrs.Flags |= net.FlagUp + } + + if l.AllMulticast { + linkAttrs.Allmulti = 1 } - return l, err + return linkAttrs, nil } -// SetUp enables the link device. -func (l *Link) SetUp() error { - _, err := subprocess.RunCommand("ip", "link", "set", "dev", l.Name, "up") +// LinkFromName returns a Link from a device name. +func LinkFromName(name string) (*Link, error) { + link, err := linkByName(name) if err != nil { - return err + return nil, err } - return nil + var parent, master string + + if link.Attrs().ParentIndex != 0 { + parentLink, err := netlink.LinkByIndex(link.Attrs().ParentIndex) + if err != nil { + return nil, err + } + + parent = parentLink.Attrs().Name + } + + if link.Attrs().MasterIndex != 0 { + masterLink, err := netlink.LinkByIndex(link.Attrs().MasterIndex) + if err != nil { + return nil, err + } + + master = masterLink.Attrs().Name + } + + return &Link{ + Name: link.Attrs().Name, + Kind: link.Type(), + MTU: uint32(link.Attrs().MTU), + Parent: parent, + Address: link.Attrs().HardwareAddr, + TXQueueLength: uint32(link.Attrs().TxQLen), + AllMulticast: link.Attrs().Allmulti == 1, + Master: master, + Up: (link.Attrs().Flags & net.FlagUp) != 0, + }, nil +} + +// SetUp enables the link device. +func (l *Link) SetUp() error { + return netlink.LinkSetUp(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }) } // SetDown disables the link device. func (l *Link) SetDown() error { - _, err := subprocess.RunCommand("ip", "link", "set", "dev", l.Name, "down") - if err != nil { - return err - } - - return nil + return netlink.LinkSetDown(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }) } // SetMTU sets the MTU of the link device. func (l *Link) SetMTU(mtu uint32) error { - _, err := subprocess.RunCommand("ip", "link", "set", "dev", l.Name, "mtu", fmt.Sprintf("%d", mtu)) - if err != nil { - return err - } - - return nil + return netlink.LinkSetMTU(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }, int(mtu)) } // SetTXQueueLength sets the txqueuelen of the link device. func (l *Link) SetTXQueueLength(queueLength uint32) error { - _, err := subprocess.RunCommand("ip", "link", "set", "dev", l.Name, "txqueuelen", fmt.Sprintf("%d", queueLength)) - if err != nil { - return err - } - - return nil + return netlink.LinkSetTxQLen(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }, int(queueLength)) } // SetAddress sets the address of the link device. func (l *Link) SetAddress(address net.HardwareAddr) error { - _, err := subprocess.RunCommand("ip", "link", "set", "dev", l.Name, "address", address.String()) - if err != nil { - return err - } - - return nil + return netlink.LinkSetHardwareAddr(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }, address) } // SetAllMulticast when enabled instructs network driver to retrieve all multicast packets from the network to the // kernel for further processing. func (l *Link) SetAllMulticast(enabled bool) error { - mode := "off" + link := &netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + } + if enabled { - mode = "on" + return netlink.LinkSetAllmulticastOn(link) } - _, err := subprocess.RunCommand("ip", "link", "set", "dev", l.Name, "allmulticast", mode) - return err + return netlink.LinkSetAllmulticastOff(link) } // SetMaster sets the master of the link device. func (l *Link) SetMaster(master string) error { - _, err := subprocess.RunCommand("ip", "link", "set", "dev", l.Name, "master", master) - if err != nil { - return err - } - - return nil + return netlink.LinkSetMaster( + &netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }, + &netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: master, + }, + }, + ) } // SetNoMaster removes the master of the link device. func (l *Link) SetNoMaster() error { - _, err := subprocess.RunCommand("ip", "link", "set", "dev", l.Name, "nomaster") - if err != nil { - return err - } - - return nil + return netlink.LinkSetNoMaster(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }) } // SetName sets the name of the link device. func (l *Link) SetName(newName string) error { - _, err := subprocess.RunCommand("ip", "link", "set", "dev", l.Name, "name", newName) - if err != nil { - return err - } - - return nil + return netlink.LinkSetName(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }, newName) } // SetNetns moves the link to the selected network namespace. -func (l *Link) SetNetns(netns string) error { - _, err := subprocess.RunCommand("ip", "link", "set", "dev", l.Name, "netns", netns) +func (l *Link) SetNetns(netnsPid string) error { + pid, err := strconv.Atoi(netnsPid) if err != nil { return err } - return nil + return netlink.LinkSetNsPid(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }, pid) } // SetVfAddress changes the address for the specified vf. func (l *Link) SetVfAddress(vf string, address string) error { - _, err := subprocess.TryRunCommand("ip", "link", "set", "dev", l.Name, "vf", vf, "mac", address) + vfInt, err := strconv.Atoi(vf) if err != nil { return err } - return nil + hwAddress, err := net.ParseMAC(address) + if err != nil { + return err + } + + return netlink.LinkSetVfHardwareAddr(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }, vfInt, hwAddress) } // SetVfVlan changes the assigned VLAN for the specified vf. func (l *Link) SetVfVlan(vf string, vlan string) error { - _, err := subprocess.TryRunCommand("ip", "link", "set", "dev", l.Name, "vf", vf, "vlan", vlan) + vfInt, err := strconv.Atoi(vf) if err != nil { return err } - return nil + vlanInt, err := strconv.Atoi(vlan) + if err != nil { + return err + } + + return netlink.LinkSetVfVlan(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }, vfInt, vlanInt) } // SetVfSpoofchk turns packet spoof checking on or off for the specified VF. func (l *Link) SetVfSpoofchk(vf string, mode string) error { - _, err := subprocess.TryRunCommand("ip", "link", "set", "dev", l.Name, "vf", vf, "spoofchk", mode) + vfInt, err := strconv.Atoi(vf) if err != nil { return err } - return nil + // TODO: pass as bool + check := mode == "on" + + return netlink.LinkSetVfSpoofchk(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }, vfInt, check) } // VirtFuncInfo holds information about vf. type VirtFuncInfo struct { - VF int `json:"vf"` - Address string `json:"address"` - MAC string `json:"mac"` // Deprecated - VLANs []map[string]int `json:"vlan_list"` - SpoofCheck bool `json:"spoofchk"` + VF int + Address net.HardwareAddr + VLAN int + SpoofCheck bool } // GetVFInfo returns info about virtual function. func (l *Link) GetVFInfo(vfID int) (VirtFuncInfo, error) { - vf := VirtFuncInfo{} - vfNotFoundErr := fmt.Errorf("no matching virtual function found") - - ipPath, err := exec.LookPath("ip") + link, err := linkByName(l.Name) if err != nil { - return vf, fmt.Errorf("ip command not found") + return VirtFuncInfo{}, err } - // Function to get VF info using regex matching, for older versions of ip tool. Less reliable. - vfFindByRegex := func(devName string, vfID int) (VirtFuncInfo, error) { - cmd := exec.Command(ipPath, "link", "show", devName) - stdout, err := cmd.StdoutPipe() - if err != nil { - return vf, err - } - - defer func() { _ = stdout.Close() }() - - err = cmd.Start() - if err != nil { - return vf, err - } - - defer func() { _ = cmd.Wait() }() - - // Try and match: "vf 1 MAC 00:00:00:00:00:00, vlan 4095, spoof checking off" - reVlan, err := regexp.Compile(fmt.Sprintf(`vf %d MAC ((?:[[:xdigit:]]{2}:){5}[[:xdigit:]]{2}).*, vlan (\d+), spoof checking (\w+)`, vfID)) - if err != nil { - return vf, err - } - - // IP link command doesn't show the vlan property if its set to 0, so we need to detect that. - // Try and match: "vf 1 MAC 00:00:00:00:00:00, spoof checking off" - reNoVlan, err := regexp.Compile(fmt.Sprintf(`vf %d MAC ((?:[[:xdigit:]]{2}:){5}[[:xdigit:]]{2}).*, spoof checking (\w+)`, vfID)) - if err != nil { - return vf, err - } - - scanner := bufio.NewScanner(stdout) - for scanner.Scan() { - // First try and find VF and read its properties with VLAN activated. - res := reVlan.FindStringSubmatch(scanner.Text()) - if len(res) == 4 { - vlan, err := strconv.Atoi(res[2]) - if err != nil { - return vf, err - } - - vf.Address = res[1] - vf.VLANs = append(vf.VLANs, map[string]int{"vlan": vlan}) - vf.SpoofCheck = util.IsTrue(res[3]) - - return vf, err - } - - // Next try and find VF and read its properties with VLAN missing. - res = reNoVlan.FindStringSubmatch(scanner.Text()) - if len(res) == 3 { - vf.Address = res[1] - // Missing VLAN ID means 0 when resetting later. - vf.VLANs = append(vf.VLANs, map[string]int{"vlan": 0}) - vf.SpoofCheck = util.IsTrue(res[2]) - - return vf, err - } - } - - err = scanner.Err() - if err != nil { - return vf, err + for _, vf := range link.Attrs().Vfs { + if vf.ID == vfID { + return VirtFuncInfo{ + VF: vfID, + Address: vf.Mac, + VLAN: vf.Vlan, + SpoofCheck: vf.Spoofchk, + }, nil } - - return vf, vfNotFoundErr - } - - // First try using the JSON output format as is more reliable to parse. - cmd := exec.Command(ipPath, "-j", "link", "show", l.Name) - stdout, err := cmd.StdoutPipe() - if err != nil { - return vf, err - } - - defer func() { _ = stdout.Close() }() - - err = cmd.Start() - if err != nil { - return vf, err - } - - defer func() { _ = cmd.Wait() }() - - // Temporary struct to decode ip output into. - var ifInfo []struct { - VFList []VirtFuncInfo `json:"vfinfo_list"` } - // Decode JSON output. - dec := json.NewDecoder(stdout) - err = dec.Decode(&ifInfo) - if err != nil && err != io.EOF { - return vf, err - } - - err = cmd.Wait() - if err != nil { - // If JSON command fails, fallback to using regex match mode for older versions of ip tool. - // This does not support the newer VF "link/ether" output prefix. - return vfFindByRegex(l.Name, vfID) - } - - if len(ifInfo) == 0 { - return vf, vfNotFoundErr - } - - // Search VFs returned for match. - found := false - for _, vfInfo := range ifInfo[0].VFList { - if vfInfo.VF == vfID { - vf = vfInfo // Found a match. - found = true - } - } - - if !found { - return vf, vfNotFoundErr - } - - // Always populate VLANs slice if not already populated. Missing VLAN ID means 0 when resetting later. - if len(vf.VLANs) == 0 { - vf.VLANs = append(vf.VLANs, map[string]int{"vlan": 0}) - } - - // Ensure empty VLAN entry is consistently populated. - if _, found = vf.VLANs[0]["vlan"]; !found { - vf.VLANs[0]["vlan"] = 0 - } - - // If ip tool has provided old mac field, copy into newer address field. - if vf.MAC != "" && vf.Address == "" { - vf.Address = vf.MAC - } - - return vf, nil + return VirtFuncInfo{}, fmt.Errorf("no matching virtual function found") } // Delete deletes the link device. func (l *Link) Delete() error { - _, err := subprocess.RunCommand("ip", "link", "delete", "dev", l.Name) - if err != nil { - return err - } - - return nil + return netlink.LinkDel(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }) } // BridgeVLANAdd adds a new vlan filter entry. func (l *Link) BridgeVLANAdd(vid string, pvid bool, untagged bool, self bool) error { - cmd := []string{"vlan", "add", "dev", l.Name, "vid", vid} - - if pvid { - cmd = append(cmd, "pvid") - } - - if untagged { - cmd = append(cmd, "untagged") - } - - if self { - cmd = append(cmd, "self") - } else { - cmd = append(cmd, "master") - } - - _, err := subprocess.RunCommand("bridge", cmd...) + vidInt, err := strconv.Atoi(vid) if err != nil { return err } - return nil + return netlink.BridgeVlanAdd(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }, uint16(vidInt), pvid, untagged, self, !self) } // BridgeVLANDelete removes an existing vlan filter entry. func (l *Link) BridgeVLANDelete(vid string, self bool) error { - cmd := []string{"vlan", "del", "dev", l.Name, "vid", vid} - - if self { - cmd = append(cmd, "self") - } else { - cmd = append(cmd, "master") - } - - _, err := subprocess.RunCommand("bridge", cmd...) + vidInt, err := strconv.Atoi(vid) if err != nil { return err } - return nil + // TODO: what about the `pvid` and `untagged` flags? + return netlink.BridgeVlanDel(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }, uint16(vidInt), false, false, self, !self) } // BridgeLinkSetIsolated sets bridge 'isolated' attribute on a port. func (l *Link) BridgeLinkSetIsolated(isolated bool) error { - isolatedState := "on" - if !isolated { - isolatedState = "off" - } - - _, err := subprocess.RunCommand("bridge", "link", "set", "dev", l.Name, "isolated", isolatedState) - if err != nil { - return err - } - - return nil + return netlink.LinkSetIsolated(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }, isolated) } // BridgeLinkSetHairpin sets bridge 'hairpin' attribute on a port. func (l *Link) BridgeLinkSetHairpin(hairpin bool) error { - hairpinState := "on" - if !hairpin { - hairpinState = "off" - } - - _, err := subprocess.RunCommand("bridge", "link", "set", "dev", l.Name, "hairpin", hairpinState) - if err != nil { - return err - } - - return nil + return netlink.LinkSetHairpin(&netlink.GenericLink{ + LinkAttrs: netlink.LinkAttrs{ + Name: l.Name, + }, + }, hairpin) } diff --git a/internal/server/ip/link_bridge.go b/internal/server/ip/link_bridge.go index 48821356903..d0f299e49c1 100644 --- a/internal/server/ip/link_bridge.go +++ b/internal/server/ip/link_bridge.go @@ -1,5 +1,9 @@ package ip +import ( + "github.com/vishvananda/netlink" +) + // Bridge represents arguments for link device of type bridge. type Bridge struct { Link @@ -7,5 +11,12 @@ type Bridge struct { // Add adds new virtual link. func (b *Bridge) Add() error { - return b.Link.add("bridge", nil) + attrs, err := b.netlinkAttrs() + if err != nil { + return err + } + + return netlink.LinkAdd(&netlink.Bridge{ + LinkAttrs: attrs, + }) } diff --git a/internal/server/ip/link_dummy.go b/internal/server/ip/link_dummy.go index 3fe4e6428ad..4a923b04b53 100644 --- a/internal/server/ip/link_dummy.go +++ b/internal/server/ip/link_dummy.go @@ -1,5 +1,9 @@ package ip +import ( + "github.com/vishvananda/netlink" +) + // Dummy represents arguments for link device of type dummy. type Dummy struct { Link @@ -7,5 +11,12 @@ type Dummy struct { // Add adds new virtual link. func (d *Dummy) Add() error { - return d.Link.add("dummy", nil) + attrs, err := d.netlinkAttrs() + if err != nil { + return err + } + + return netlink.LinkAdd(&netlink.Dummy{ + LinkAttrs: attrs, + }) } diff --git a/internal/server/ip/link_gretap.go b/internal/server/ip/link_gretap.go index c7b083a7d80..9050b7aee0f 100644 --- a/internal/server/ip/link_gretap.go +++ b/internal/server/ip/link_gretap.go @@ -1,5 +1,12 @@ package ip +import ( + "fmt" + "net" + + "github.com/vishvananda/netlink" +) + // Gretap represents arguments for link of type gretap. type Gretap struct { Link @@ -7,12 +14,26 @@ type Gretap struct { Remote string } -// additionalArgs generates gretap specific arguments. -func (g *Gretap) additionalArgs() []string { - return []string{"local", g.Local, "remote", g.Remote} -} - // Add adds new virtual link. func (g *Gretap) Add() error { - return g.Link.add("gretap", g.additionalArgs()) + attrs, err := g.netlinkAttrs() + if err != nil { + return err + } + + local := net.ParseIP(g.Local) + if local == nil { + return fmt.Errorf("invalid local address %q", g.Local) + } + + remote := net.ParseIP(g.Remote) + if remote == nil { + return fmt.Errorf("invalid remote address %q", g.Remote) + } + + return netlink.LinkAdd(&netlink.Gretap{ + LinkAttrs: attrs, + Local: local, + Remote: remote, + }) } diff --git a/internal/server/ip/link_macvlan.go b/internal/server/ip/link_macvlan.go index d3683036803..cab7861a17e 100644 --- a/internal/server/ip/link_macvlan.go +++ b/internal/server/ip/link_macvlan.go @@ -1,12 +1,43 @@ package ip +import ( + "github.com/vishvananda/netlink" +) + // Macvlan represents arguments for link of type macvlan. type Macvlan struct { Link Mode string } +func (macvlan *Macvlan) netlinkMode() netlink.MacvlanMode { + switch macvlan.Mode { + case "default": + return netlink.MACVLAN_MODE_DEFAULT + case "private": + return netlink.MACVLAN_MODE_PRIVATE + case "vepa": + return netlink.MACVLAN_MODE_VEPA + case "bridge": + return netlink.MACVLAN_MODE_BRIDGE + case "passthru": + return netlink.MACVLAN_MODE_PASSTHRU + case "source": + return netlink.MACVLAN_MODE_SOURCE + default: + return netlink.MACVLAN_MODE_DEFAULT + } +} + // Add adds new virtual link. func (macvlan *Macvlan) Add() error { - return macvlan.Link.add("macvlan", []string{"mode", macvlan.Mode}) + attrs, err := macvlan.netlinkAttrs() + if err != nil { + return err + } + + return netlink.LinkAdd(&netlink.Macvlan{ + LinkAttrs: attrs, + Mode: macvlan.netlinkMode(), + }) } diff --git a/internal/server/ip/link_macvtap.go b/internal/server/ip/link_macvtap.go index ba1bd1a6878..39282e62041 100644 --- a/internal/server/ip/link_macvtap.go +++ b/internal/server/ip/link_macvtap.go @@ -1,5 +1,9 @@ package ip +import ( + "github.com/vishvananda/netlink" +) + // Macvtap represents arguments for link of type macvtap. type Macvtap struct { Macvlan @@ -7,5 +11,15 @@ type Macvtap struct { // Add adds new virtual link. func (macvtap *Macvtap) Add() error { - return macvtap.Macvlan.Link.add("macvtap", []string{"mode", macvtap.Macvlan.Mode}) + attrs, err := macvtap.netlinkAttrs() + if err != nil { + return err + } + + return netlink.LinkAdd(&netlink.Macvtap{ + Macvlan: netlink.Macvlan{ + LinkAttrs: attrs, + Mode: macvtap.netlinkMode(), + }, + }) } diff --git a/internal/server/ip/link_veth.go b/internal/server/ip/link_veth.go index 0f822ca295a..bfd86270153 100644 --- a/internal/server/ip/link_veth.go +++ b/internal/server/ip/link_veth.go @@ -8,11 +8,12 @@ import ( type Veth struct { Link Peer Link - Master string + Master string // TODO: Link already has a master, and the way this one is used does not suggest that it is different } // Add adds new virtual link. func (veth *Veth) Add() error { + // TODO: the netlink library does not support configuring anything except name and hwaddr on the peer err := veth.Link.add("veth", append([]string{"peer"}, veth.Peer.args()...)) if err != nil { return err diff --git a/internal/server/ip/link_vlan.go b/internal/server/ip/link_vlan.go index d7bcf652c20..3f24e81ef1a 100644 --- a/internal/server/ip/link_vlan.go +++ b/internal/server/ip/link_vlan.go @@ -19,5 +19,6 @@ func (vlan *Vlan) additionalArgs() []string { // Add adds new virtual link. func (vlan *Vlan) Add() error { + // TODO: netlink library does not support vlan flags (necessary for gvrp) return vlan.Link.add("vlan", vlan.additionalArgs()) } diff --git a/internal/server/ip/link_vxlan.go b/internal/server/ip/link_vxlan.go index bf7bf1db580..9430df8994f 100644 --- a/internal/server/ip/link_vxlan.go +++ b/internal/server/ip/link_vxlan.go @@ -1,5 +1,13 @@ package ip +import ( + "fmt" + "net" + "strconv" + + "github.com/vishvananda/netlink" +) + // Vxlan represents arguments for link of type vxlan. type Vxlan struct { Link @@ -12,38 +20,88 @@ type Vxlan struct { TTL string } -// additionalArgs generates vxlan specific arguments. -func (vxlan *Vxlan) additionalArgs() []string { - args := []string{} - args = append(args, "id", vxlan.VxlanID) +// Add adds new virtual link. +func (vxlan *Vxlan) Add() error { + attrs, err := vxlan.netlinkAttrs() + if err != nil { + return err + } + + // TODO: all of these these can be passed as int or net.IP + + vxlanID, err := strconv.Atoi(vxlan.VxlanID) + if err != nil { + return err + } + + var devIndex int if vxlan.DevName != "" { - args = append(args, "dev", vxlan.DevName) + dev, err := linkByName(vxlan.DevName) + if err != nil { + return err + } + + devIndex = dev.Attrs().Index } + var group net.IP if vxlan.Group != "" { - args = append(args, "group", vxlan.Group) + group = net.ParseIP(vxlan.Group) + if group == nil { + return fmt.Errorf("invalid group address %q", vxlan.Group) + } + + if !group.IsMulticast() { + return fmt.Errorf("group address must be multicast, got %q", vxlan.Group) + } } if vxlan.Remote != "" { - args = append(args, "remote", vxlan.Remote) + if group != nil { + return fmt.Errorf("group and remote can not be specified together") + } + + group = net.ParseIP(vxlan.Remote) + if group == nil { + return fmt.Errorf("invalid remote address %q", vxlan.Remote) + } + + if group.IsMulticast() { + return fmt.Errorf("remote address must not be multicast, got %q", vxlan.Remote) + } } + var local net.IP if vxlan.Local != "" { - args = append(args, "local", vxlan.Local) + local = net.ParseIP(vxlan.Local) + if local == nil { + return fmt.Errorf("invalid local address %q", vxlan.Local) + } } + var ttl int if vxlan.TTL != "" { - args = append(args, "ttl", vxlan.TTL) + ttl, err = strconv.Atoi(vxlan.TTL) + if err != nil { + return err + } } + var dstport int if vxlan.DstPort != "" { - args = append(args, "dstport", vxlan.DstPort) + dstport, err = strconv.Atoi(vxlan.DstPort) + if err != nil { + return err + } } - return args -} - -// Add adds new virtual link. -func (vxlan *Vxlan) Add() error { - return vxlan.Link.add("vxlan", vxlan.additionalArgs()) + return netlink.LinkAdd(&netlink.Vxlan{ + LinkAttrs: attrs, + VxlanId: vxlanID, + VtepDevIndex: devIndex, + SrcAddr: local, + Group: group, + TTL: ttl, + Port: dstport, + }) } From d8cc6409b5985d6705e7cbb2df92a89053678eb5 Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Thu, 24 Apr 2025 12:28:01 +0200 Subject: [PATCH 06/22] incusd/ip/neigh: Switch to netlink Signed-off-by: Gwendolyn --- internal/server/ip/neigh.go | 66 +++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/internal/server/ip/neigh.go b/internal/server/ip/neigh.go index 749d2fc6098..a0209fe8df4 100644 --- a/internal/server/ip/neigh.go +++ b/internal/server/ip/neigh.go @@ -1,12 +1,10 @@ package ip import ( - "bytes" + "fmt" "net" - "strings" - "github.com/lxc/incus/v6/shared/subprocess" - "github.com/lxc/incus/v6/shared/util" + "github.com/vishvananda/netlink" ) // NeighbourIPState can be { PERMANENT | NOARP | REACHABLE | STALE | NONE | INCOMPLETE | DELAY | PROBE | FAILED }. @@ -41,6 +39,31 @@ const NeighbourIPStateProbe = "PROBE" // NeighbourIPStateFailed max number of probes exceeded without success, neighbor validation has ultimately failed. const NeighbourIPStateFailed = "FAILED" +func mapNetlinkState(state int) NeighbourIPState { + switch state { + case netlink.NUD_NONE: + return NeighbourIPStateNone + case netlink.NUD_INCOMPLETE: + return NeighbourIPStateIncomplete + case netlink.NUD_REACHABLE: + return NeighbourIPStateReachable + case netlink.NUD_STALE: + return NeighbourIPStateStale + case netlink.NUD_DELAY: + return NeighbourIPStateDelay + case netlink.NUD_PROBE: + return NeighbourIPStateProbe + case netlink.NUD_FAILED: + return NeighbourIPStateFailed + case netlink.NUD_NOARP: + return NeighbourIPStateNoARP + case netlink.NUD_PERMANENT: + return NeighbourIPStatePermanent + default: + return NeighbourIPStateNone + } +} + // Neigh represents arguments for neighbour manipulation. type Neigh struct { DevName string @@ -51,38 +74,23 @@ type Neigh struct { // Show list neighbour entries filtered by DevName and optionally MAC address. func (n *Neigh) Show() ([]Neigh, error) { - out, err := subprocess.RunCommand("ip", "neigh", "show", "dev", n.DevName) + link, err := linkByName(n.DevName) if err != nil { return nil, err } - neighbours := []Neigh{} - - for _, line := range util.SplitNTrimSpace(out, "\n", -1, true) { - // Split fields and early validation. - fields := strings.Fields(line) - if len(fields) != 4 { - continue - } - - addr := net.ParseIP(fields[0]) - if addr == nil { - continue - } - - mac, _ := net.ParseMAC(fields[2]) + netlinkNeighbours, err := netlink.NeighList(link.Attrs().Index, netlink.FAMILY_ALL) + if err != nil { + return nil, fmt.Errorf("failed to get neighbours for link %q: %w", n.DevName, err) + } - // Check neighbour matches desired MAC address if specified. - if n.MAC != nil { - if !bytes.Equal(n.MAC, mac) { - continue - } - } + neighbours := make([]Neigh, 0, len(netlinkNeighbours)) + for _, neighbour := range netlinkNeighbours { neighbours = append(neighbours, Neigh{ - Addr: addr, - MAC: mac, - State: NeighbourIPState(fields[3]), + Addr: neighbour.IP, + MAC: neighbour.HardwareAddr, + State: mapNetlinkState(neighbour.State), }) } From 6a9435823f8d4c1de9bdcedc34f713815f17f3ff Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Thu, 24 Apr 2025 12:33:47 +0200 Subject: [PATCH 07/22] incusd/ip/neigh_proxy: Switch to netlink Signed-off-by: Gwendolyn --- internal/server/ip/neigh_proxy.go | 55 ++++++++++++++++++++----------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/internal/server/ip/neigh_proxy.go b/internal/server/ip/neigh_proxy.go index 059f5c921b2..bf1b42c8813 100644 --- a/internal/server/ip/neigh_proxy.go +++ b/internal/server/ip/neigh_proxy.go @@ -1,11 +1,11 @@ package ip import ( + "fmt" "net" - "strings" - "github.com/lxc/incus/v6/shared/subprocess" - "github.com/lxc/incus/v6/shared/util" + "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" ) // NeighProxy represents arguments for neighbour proxy manipulation. @@ -16,50 +16,67 @@ type NeighProxy struct { // Show list neighbour proxy entries. func (n *NeighProxy) Show() ([]NeighProxy, error) { - out, err := subprocess.RunCommand("ip", "neigh", "show", "proxy", "dev", n.DevName) + link, err := linkByName(n.DevName) if err != nil { return nil, err } - lines := util.SplitNTrimSpace(out, "\n", -1, true) - entries := make([]NeighProxy, 0, len(lines)) - - for _, line := range lines { - fields := strings.Fields(line) - if len(fields) <= 0 { - continue - } + list, err := netlink.NeighProxyList(link.Attrs().Index, netlink.FAMILY_ALL) + if err != nil { + return nil, fmt.Errorf("failed to get neighbour proxies for link %q: %w", n.DevName, err) + } - ip := net.ParseIP(fields[0]) - if ip == nil { - continue - } + entries := make([]NeighProxy, 0, len(list)) + for _, neigh := range list { entries = append(entries, NeighProxy{ DevName: n.DevName, - Addr: ip, + Addr: neigh.IP, }) } return entries, nil } +func (n *NeighProxy) netlinkNeigh() (*netlink.Neigh, error) { + link, err := linkByName(n.DevName) + if err != nil { + return nil, err + } + + return &netlink.Neigh{ + LinkIndex: link.Attrs().Index, + Flags: unix.NTF_PROXY, + IP: n.Addr, + }, nil +} + // Add a neighbour proxy entry. func (n *NeighProxy) Add() error { - _, err := subprocess.RunCommand("ip", "neigh", "add", "proxy", n.Addr.String(), "dev", n.DevName) + neigh, err := n.netlinkNeigh() if err != nil { return err } + err = netlink.NeighAdd(neigh) + if err != nil { + return fmt.Errorf("failed to add neighbour proxy %v: %w", neigh, err) + } + return nil } // Delete a neighbour proxy entry. func (n *NeighProxy) Delete() error { - _, err := subprocess.RunCommand("ip", "neigh", "delete", "proxy", n.Addr.String(), "dev", n.DevName) + neigh, err := n.netlinkNeigh() if err != nil { return err } + err = netlink.NeighDel(neigh) + if err != nil { + return fmt.Errorf("failed to delete neighbour proxy %v: %w", neigh, err) + } + return nil } From d3c2e07b7424a685ff3f3de14bf14731d9bcfb0c Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Thu, 24 Apr 2025 13:16:30 +0200 Subject: [PATCH 08/22] incusd/ip/qdisc: Switch to netlink Signed-off-by: Gwendolyn --- .../server/device/device_utils_network.go | 23 +++-- internal/server/ip/init.go | 9 ++ internal/server/ip/qdisc.go | 94 +++++++------------ internal/server/ip/qdisc_htb.go | 57 +++++++++++ internal/server/ip/qdisc_ingress.go | 59 ++++++++++++ 5 files changed, 173 insertions(+), 69 deletions(-) create mode 100644 internal/server/ip/init.go create mode 100644 internal/server/ip/qdisc_htb.go create mode 100644 internal/server/ip/qdisc_ingress.go diff --git a/internal/server/device/device_utils_network.go b/internal/server/device/device_utils_network.go index d7eeb421351..dca6585a645 100644 --- a/internal/server/device/device_utils_network.go +++ b/internal/server/device/device_utils_network.go @@ -2,6 +2,7 @@ package device import ( "context" + "errors" "fmt" "net" "net/netip" @@ -14,6 +15,7 @@ import ( "github.com/mdlayher/arp" "github.com/mdlayher/ndp" + "golang.org/x/sys/unix" deviceConfig "github.com/lxc/incus/v6/internal/server/device/config" pcidev "github.com/lxc/incus/v6/internal/server/device/pci" @@ -528,14 +530,21 @@ func networkSetupHostVethLimits(d *deviceCommon, oldConfig deviceConfig.Device, } // Clean any existing entry - qdisc := &ip.Qdisc{Dev: veth, Root: true} - _ = qdisc.Delete() - qdisc = &ip.Qdisc{Dev: veth, Ingress: true} - _ = qdisc.Delete() + qdiscIngress := &ip.QdiscIngress{Qdisc: ip.Qdisc{Dev: veth, Handle: "ffff:0"}} + err = qdiscIngress.Delete() + if err != nil && !errors.Is(err, unix.ENOENT) { + return err + } + + qdiscHTB := &ip.QdiscHTB{Qdisc: ip.Qdisc{Dev: veth, Handle: "1:0", Parent: "root"}} + err = qdiscHTB.Delete() + if err != nil && !errors.Is(err, unix.ENOENT) { + return err + } // Apply new limits if d.config["limits.ingress"] != "" { - qdiscHTB := &ip.QdiscHTB{Qdisc: ip.Qdisc{Dev: veth, Handle: "1:0", Root: true}, Default: "10"} + qdiscHTB = &ip.QdiscHTB{Qdisc: ip.Qdisc{Dev: veth, Handle: "1:0", Parent: "root"}, Default: "10"} err := qdiscHTB.Add() if err != nil { return fmt.Errorf("Failed to create root tc qdisc: %s", err) @@ -555,8 +564,8 @@ func networkSetupHostVethLimits(d *deviceCommon, oldConfig deviceConfig.Device, } if d.config["limits.egress"] != "" { - qdisc = &ip.Qdisc{Dev: veth, Handle: "ffff:0", Ingress: true} - err := qdisc.Add() + qdiscIngress = &ip.QdiscIngress{Qdisc: ip.Qdisc{Dev: veth, Handle: "ffff:0"}} + err := qdiscIngress.Add() if err != nil { return fmt.Errorf("Failed to create ingress tc qdisc: %s", err) } diff --git a/internal/server/ip/init.go b/internal/server/ip/init.go new file mode 100644 index 00000000000..f873b956bfe --- /dev/null +++ b/internal/server/ip/init.go @@ -0,0 +1,9 @@ +package ip + +import ( + "github.com/vishvananda/netlink/nl" +) + +func init() { + nl.EnableErrorMessageReporting = true +} diff --git a/internal/server/ip/qdisc.go b/internal/server/ip/qdisc.go index 69147ddc541..c8b60b5a1d6 100644 --- a/internal/server/ip/qdisc.go +++ b/internal/server/ip/qdisc.go @@ -1,83 +1,53 @@ package ip import ( - "github.com/lxc/incus/v6/shared/subprocess" + "errors" + "strings" + + "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" ) // Qdisc represents 'queueing discipline' object. type Qdisc struct { - Dev string - Handle string - Root bool - Ingress bool -} - -func (qdisc *Qdisc) mainCmd() []string { - cmd := []string{"qdisc", "add", "dev", qdisc.Dev} - if qdisc.Handle != "" { - cmd = append(cmd, "handle", qdisc.Handle) - } - - if qdisc.Root { - cmd = append(cmd, "root") - } - - if qdisc.Ingress { - cmd = append(cmd, "ingress") - } - - return cmd + Dev string + Handle string + Parent string } -// Add adds qdisc to a node. -func (qdisc *Qdisc) Add() error { - cmd := qdisc.mainCmd() - _, err := subprocess.RunCommand("tc", cmd...) +func (q *Qdisc) netlinkAttrs() (netlink.QdiscAttrs, error) { + link, err := linkByName(q.Dev) if err != nil { - return err - } - - return nil -} - -// Delete deletes qdisc from node. -func (qdisc *Qdisc) Delete() error { - cmd := []string{"qdisc", "del", "dev", qdisc.Dev} - if qdisc.Root { - cmd = append(cmd, "root") + return netlink.QdiscAttrs{}, err } - if qdisc.Ingress { - cmd = append(cmd, "ingress") + var handle uint32 + if q.Handle != "" { + handle, err = parseHandle(q.Handle) + if err != nil { + return netlink.QdiscAttrs{}, err + } } - _, err := subprocess.RunCommand("tc", cmd...) - if err != nil { - return err + var parent uint32 + if q.Parent != "" { + parent, err = parseHandle(q.Parent) + if err != nil { + return netlink.QdiscAttrs{}, err + } } - return nil -} - -// QdiscHTB represents the hierarchy token bucket qdisc object. -type QdiscHTB struct { - Qdisc - Default string + return netlink.QdiscAttrs{ + LinkIndex: link.Attrs().Index, + Handle: handle, + Parent: parent, + }, nil } -// Add adds qdisc to a node. -func (qdisc *QdiscHTB) Add() error { - cmd := qdisc.mainCmd() - cmd = append(cmd, "htb") - - if qdisc.Default != "" { - cmd = append(cmd, "default", qdisc.Default) - } - - _, err := subprocess.RunCommand("tc", cmd...) - if err != nil { - return err +func mapQdiscErr(err error) error { + if errors.Is(err, unix.EINVAL) && strings.Contains(err.Error(), "Invalid handle") { + return unix.ENOENT } - return nil + return err } diff --git a/internal/server/ip/qdisc_htb.go b/internal/server/ip/qdisc_htb.go new file mode 100644 index 00000000000..4293b9b22b2 --- /dev/null +++ b/internal/server/ip/qdisc_htb.go @@ -0,0 +1,57 @@ +package ip + +import ( + "fmt" + "strconv" + + "github.com/vishvananda/netlink" +) + +// QdiscHTB represents the hierarchy token bucket qdisc object. +type QdiscHTB struct { + Qdisc + Default string +} + +// Add adds a htb qdisc to a device. +func (q *QdiscHTB) Add() error { + attrs, err := q.netlinkAttrs() + if err != nil { + return err + } + + htb := netlink.NewHtb(attrs) + + if q.Default != "" { + defcls, err := strconv.Atoi(q.Default) + if err != nil { + return fmt.Errorf("invalid htb default class %q: %w", q.Default, err) + } + + htb.Defcls = uint32(defcls) + } + + err = netlink.QdiscAdd(htb) + if err != nil { + return fmt.Errorf("failed to add qdisc htb %v: %w", htb, mapQdiscErr(err)) + } + + return nil +} + +// Delete deletes a htb qdisc from a device. +func (q *QdiscHTB) Delete() error { + attrs, err := q.netlinkAttrs() + if err != nil { + return err + } + + htb := netlink.NewHtb(attrs) + + err = netlink.QdiscDel(htb) + if err != nil { + return fmt.Errorf("failed to delete qdisc htb %v: %w", htb, mapQdiscErr(err)) + } + + return nil +} diff --git a/internal/server/ip/qdisc_ingress.go b/internal/server/ip/qdisc_ingress.go new file mode 100644 index 00000000000..189d052f0e8 --- /dev/null +++ b/internal/server/ip/qdisc_ingress.go @@ -0,0 +1,59 @@ +package ip + +import ( + "errors" + "fmt" + + "github.com/vishvananda/netlink" +) + +// QdiscIngress represents an ingress qdisc object. +type QdiscIngress struct { + Qdisc +} + +// Add adds an ingress qdisc to a device. +func (q *QdiscIngress) Add() error { + attrs, err := q.netlinkAttrs() + if err != nil { + return err + } + + if q.Parent != "" { + return errors.New("ingress qdisc cannot have parent") + } + + attrs.Parent = netlink.HANDLE_INGRESS + + ingress := &netlink.Ingress{ + QdiscAttrs: attrs, + } + + err = netlink.QdiscAdd(ingress) + if err != nil { + return fmt.Errorf("failed to add ingress qdisc %v: %w", ingress, mapQdiscErr(err)) + } + + return nil +} + +// Delete deletes an ingress qdisc from a device. +func (q *QdiscIngress) Delete() error { + attrs, err := q.netlinkAttrs() + if err != nil { + return err + } + + attrs.Parent = netlink.HANDLE_INGRESS + + ingress := &netlink.Ingress{ + QdiscAttrs: attrs, + } + + err = netlink.QdiscDel(ingress) + if err != nil { + return fmt.Errorf("failed to delete ingress qdisc %v: %w", ingress, mapQdiscErr(err)) + } + + return nil +} From 70a007e6f1e51e852a7fb36de7a19af3ab938a21 Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Fri, 25 Apr 2025 14:56:42 +0200 Subject: [PATCH 09/22] incusd/ip/route: Switch to netlink Signed-off-by: Gwendolyn --- internal/server/ip/route.go | 301 ++++++++++++++++++----- internal/server/network/driver_bridge.go | 24 +- 2 files changed, 245 insertions(+), 80 deletions(-) diff --git a/internal/server/ip/route.go b/internal/server/ip/route.go index 860c839f8b0..04156e2ab21 100644 --- a/internal/server/ip/route.go +++ b/internal/server/ip/route.go @@ -1,9 +1,12 @@ package ip import ( - "strings" + "fmt" + "net" + "strconv" - "github.com/lxc/incus/v6/shared/subprocess" + "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" ) // Route represents arguments for route manipulation. @@ -18,129 +21,303 @@ type Route struct { VRF string } -// Add adds new route. -func (r *Route) Add() error { - cmd := []string{r.Family, "route", "add"} +func (r *Route) netlinkRoute() (*netlink.Route, error) { + link, err := linkByName(r.DevName) + if err != nil { + return nil, err + } + + family, err := r.netlinkFamily() + if err != nil { + return nil, err + } + + route := &netlink.Route{ + LinkIndex: link.Attrs().Index, + Family: family, + } + + if r.Route != "" { + _, dst, err := net.ParseCIDR(r.Route) + if err != nil { + return nil, fmt.Errorf("invalid destination %q: %w", r.Route, err) + } + + route.Dst = dst + } + if r.Table != "" { - cmd = append(cmd, "table", r.Table) + tableID, err := r.tableID() + if err != nil { + return nil, fmt.Errorf("invalid table %q: %w", r.Table, err) + } + + route.Table = tableID + } else if r.VRF != "" { + vrfDev, err := linkByName(r.VRF) + if err != nil { + return nil, err + } + + vrf, ok := vrfDev.(*netlink.Vrf) + if !ok { + return nil, fmt.Errorf("%q is not a vrf", r.VRF) + } + + route.Table = int(vrf.Table) } if r.Via != "" { - cmd = append(cmd, "via", r.Via) + via := net.ParseIP(r.Via) + if via == nil { + return nil, fmt.Errorf("invalid via address %q", r.Via) + } + + route.Gw = via + } else { + route.Scope = netlink.SCOPE_LINK } - cmd = append(cmd, r.Route, "dev", r.DevName) if r.Src != "" { - cmd = append(cmd, "src", r.Src) + src := net.ParseIP(r.Src) + if src == nil { + return nil, fmt.Errorf("invalid src address %q", r.Src) + } + + route.Src = src } if r.Proto != "" { - cmd = append(cmd, "proto", r.Proto) + proto, err := r.netlinkProto() + if err != nil { + return nil, err + } + + route.Protocol = proto } - if r.VRF != "" { - cmd = append(cmd, "vrf", r.VRF) + return route, nil +} + +func (r *Route) tableID() (int, error) { + switch r.Table { + case "default": + return unix.RT_TABLE_DEFAULT, nil + case "main": + return unix.RT_TABLE_MAIN, nil + case "local": + return unix.RT_TABLE_LOCAL, nil + default: + return strconv.Atoi(r.Table) } +} + +func (r *Route) netlinkFamily() (int, error) { + switch r.Family { + case FamilyV4: + return netlink.FAMILY_V4, nil + case FamilyV6: + return netlink.FAMILY_V6, nil + default: + return 0, fmt.Errorf("invalid family %q", r.Family) + } +} + +func (r *Route) netlinkProto() (netlink.RouteProtocol, error) { + switch r.Proto { + case "babel": + return unix.RTPROT_BABEL, nil + case "bgp": + return unix.RTPROT_BGP, nil + case "bird": + return unix.RTPROT_BIRD, nil + case "boot": + return unix.RTPROT_BOOT, nil + case "dhcp": + return unix.RTPROT_DHCP, nil + case "dnrouted": + return unix.RTPROT_DNROUTED, nil + case "eigrp": + return unix.RTPROT_EIGRP, nil + case "gated": + return unix.RTPROT_GATED, nil + case "isis": + return unix.RTPROT_ISIS, nil + case "keepalived": + return unix.RTPROT_KEEPALIVED, nil + case "kernel": + return unix.RTPROT_KERNEL, nil + case "mrouted": + return unix.RTPROT_MROUTED, nil + case "mrt": + return unix.RTPROT_MRT, nil + case "ntk": + return unix.RTPROT_NTK, nil + case "ospf": + return unix.RTPROT_OSPF, nil + case "ra": + return unix.RTPROT_RA, nil + case "redirect": + return unix.RTPROT_REDIRECT, nil + case "rip": + return unix.RTPROT_RIP, nil + case "static": + return unix.RTPROT_STATIC, nil + case "unspec": + return unix.RTPROT_UNSPEC, nil + case "xorp": + return unix.RTPROT_XORP, nil + case "zebra": + return unix.RTPROT_ZEBRA, nil + default: + proto, err := strconv.Atoi(r.Proto) + if err != nil { + return 0, err + } + + return netlink.RouteProtocol(proto), nil + } +} - _, err := subprocess.RunCommand("ip", cmd...) +// Add adds new route. +func (r *Route) Add() error { + route, err := r.netlinkRoute() if err != nil { return err } + err = netlink.RouteAdd(route) + if err != nil { + return fmt.Errorf("failed to add route %v: %w", route, err) + } + return nil } // Delete deletes routing table. func (r *Route) Delete() error { - cmd := []string{r.Family, "route", "delete", r.Route, "dev", r.DevName} - - if r.VRF != "" { - cmd = append(cmd, "vrf", r.VRF) - } else if r.Table != "" { - cmd = append(cmd, "table", r.Table) + route, err := r.netlinkRoute() + if err != nil { + return err } - _, err := subprocess.RunCommand("ip", cmd...) + err = netlink.RouteDel(route) if err != nil { - return err + return fmt.Errorf("failed to delete route %v: %w", route, err) } return nil } -// Flush flushes routing tables. -func (r *Route) Flush() error { - cmd := []string{} - if r.Family != "" { - cmd = append(cmd, r.Family) - } +func routeFilterMask(route *netlink.Route) uint64 { + var filterMask uint64 - cmd = append(cmd, "route", "flush") - if r.Route != "" { - cmd = append(cmd, r.Route) + if route.Dst != nil { + filterMask |= netlink.RT_FILTER_DST } - if r.Via != "" { - cmd = append(cmd, "via", r.Via) + if route.Gw != nil { + filterMask |= netlink.RT_FILTER_GW } - cmd = append(cmd, "dev", r.DevName) - if r.Proto != "" { - cmd = append(cmd, "proto", r.Proto) + if route.Protocol != 0 { + filterMask |= netlink.RT_FILTER_PROTOCOL } - if r.VRF != "" { - cmd = append(cmd, "vrf", r.VRF) + if route.Table != 0 { + filterMask |= netlink.RT_FILTER_TABLE } - _, err := subprocess.RunCommand("ip", cmd...) + return filterMask +} + +// Flush flushes routing tables. +func (r *Route) Flush() error { + route, err := r.netlinkRoute() if err != nil { return err } + var iterErr error + + err = netlink.RouteListFilteredIter(route.Family, route, routeFilterMask(route), func(route netlink.Route) (cont bool) { + iterErr = netlink.RouteDel(&route) + return iterErr == nil + }) + if err != nil { + return fmt.Errorf("could not flush routes %v: %w", route, err) + } + + if iterErr != nil { + return fmt.Errorf("could not flush routes %v: %w", route, iterErr) + } + return nil } // Replace changes or adds new route. -func (r *Route) Replace(routes []string) error { - cmd := []string{r.Family, "route", "replace", "dev", r.DevName, "proto", r.Proto} - - if r.VRF != "" { - cmd = append(cmd, "vrf", r.VRF) +// If there is already a route with the same destination, metric, tos and table then that route is updated, +// otherwise a new route is added. +func (r *Route) Replace() error { + route, err := r.netlinkRoute() + if err != nil { + return err } - cmd = append(cmd, routes...) - _, err := subprocess.RunCommand("ip", cmd...) + err = netlink.RouteReplace(route) if err != nil { - return err + return fmt.Errorf("could not replace route %s: %w", route, err) } return nil } -// Show lists routes. -func (r *Route) Show() ([]string, error) { - routes := []string{} - - cmd := []string{r.Family, "route", "show", "dev", r.DevName, "proto", r.Proto} - - if r.VRF != "" { - cmd = append(cmd, "vrf", r.VRF) +// Show lists matching routes. +func (r *Route) Show() ([]Route, error) { + route, err := r.netlinkRoute() + if err != nil { + return nil, err } - out, err := subprocess.RunCommand("ip", cmd...) + netlinkRoutes, err := netlink.RouteListFiltered(route.Family, route, routeFilterMask(route)) if err != nil { - return routes, err + return nil, fmt.Errorf("failed to list routes matching %v: %w", route, err) } - for _, line := range strings.Split(out, "\n") { - line = strings.TrimSpace(line) - if len(line) == 0 { - continue + routes := make([]Route, 0, len(netlinkRoutes)) + + for _, netlinkRoute := range netlinkRoutes { + var src, gw, table string + + if len(netlinkRoute.Src) > 0 { + src = netlinkRoute.Src.String() + } + + if len(netlinkRoute.Gw) > 0 { + gw = netlinkRoute.Gw.String() + } + + switch netlinkRoute.Table { + case unix.RT_TABLE_MAIN: + table = "main" + case unix.RT_TABLE_LOCAL: + table = "local" + case unix.RT_TABLE_DEFAULT: + table = "default" + default: + table = strconv.Itoa(netlinkRoute.Table) } - route := strings.ReplaceAll(line, "linkdown", "") - routes = append(routes, route) + routes = append(routes, Route{ + DevName: r.DevName, + Route: netlinkRoute.Dst.String(), + Src: src, + Via: gw, + Table: table, + VRF: "", // adding a route to a VRF just adds it to the table associated with the VRF, so when retrieving routes that information is not available anymore and we just set the table + Proto: netlinkRoute.Protocol.String(), + Family: r.Family, // routes are filtered by family + }) } return routes, nil diff --git a/internal/server/network/driver_bridge.go b/internal/server/network/driver_bridge.go index 2880d032fe4..c33077e857b 100644 --- a/internal/server/network/driver_bridge.go +++ b/internal/server/network/driver_bridge.go @@ -2102,7 +2102,7 @@ func (n *bridge) getTunnels() []string { } // bootRoutesV4 returns a list of IPv4 boot routes on the network's device. -func (n *bridge) bootRoutesV4() ([]string, error) { +func (n *bridge) bootRoutesV4() ([]ip.Route, error) { r := &ip.Route{ DevName: n.name, Proto: "boot", @@ -2118,7 +2118,7 @@ func (n *bridge) bootRoutesV4() ([]string, error) { } // bootRoutesV6 returns a list of IPv6 boot routes on the network's device. -func (n *bridge) bootRoutesV6() ([]string, error) { +func (n *bridge) bootRoutesV6() ([]ip.Route, error) { r := &ip.Route{ DevName: n.name, Proto: "boot", @@ -2134,15 +2134,9 @@ func (n *bridge) bootRoutesV6() ([]string, error) { } // applyBootRoutesV4 applies a list of IPv4 boot routes to the network's device. -func (n *bridge) applyBootRoutesV4(routes []string) { +func (n *bridge) applyBootRoutesV4(routes []ip.Route) { for _, route := range routes { - r := &ip.Route{ - DevName: n.name, - Proto: "boot", - Family: ip.FamilyV4, - } - - err := r.Replace(strings.Fields(route)) + err := route.Replace() if err != nil { // If it fails, then we can't stop as the route has already gone, so just log and continue. n.logger.Error("Failed to restore route", logger.Ctx{"err": err}) @@ -2151,15 +2145,9 @@ func (n *bridge) applyBootRoutesV4(routes []string) { } // applyBootRoutesV6 applies a list of IPv6 boot routes to the network's device. -func (n *bridge) applyBootRoutesV6(routes []string) { +func (n *bridge) applyBootRoutesV6(routes []ip.Route) { for _, route := range routes { - r := &ip.Route{ - DevName: n.name, - Proto: "boot", - Family: ip.FamilyV6, - } - - err := r.Replace(strings.Fields(route)) + err := route.Replace() if err != nil { // If it fails, then we can't stop as the route has already gone, so just log and continue. n.logger.Error("Failed to restore route", logger.Ctx{"err": err}) From 12e8d1e8cf2d03d5626aff682dd48049c27df870 Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Fri, 25 Apr 2025 15:10:15 +0200 Subject: [PATCH 10/22] incusd/ip/tuntap: Switch to netlink Signed-off-by: Gwendolyn --- internal/server/ip/tuntap.go | 38 +++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/internal/server/ip/tuntap.go b/internal/server/ip/tuntap.go index 83c88d1bbdb..fd933c15b94 100644 --- a/internal/server/ip/tuntap.go +++ b/internal/server/ip/tuntap.go @@ -1,7 +1,9 @@ package ip import ( - "github.com/lxc/incus/v6/shared/subprocess" + "fmt" + + "github.com/vishvananda/netlink" ) // Tuntap represents arguments for tuntap manipulation. @@ -14,21 +16,35 @@ type Tuntap struct { // Add adds new tuntap interface. func (t *Tuntap) Add() error { - cmd := []string{"tuntap", "add", "name", t.Name, "mode", t.Mode} + var mode netlink.TuntapMode + + switch t.Mode { + case "tun": + mode = netlink.TUNTAP_MODE_TUN + case "tap": + mode = netlink.TUNTAP_MODE_TAP + default: + return fmt.Errorf("invalid tuntap mode %q", t.Mode) + } + + // TODO: there is TUNTAP_DEFAULTS and TUNTAP_MULTI_QUEUE_DEFAULTS in the netlink package, I don't know if these should be used + var flags netlink.TuntapFlag + if t.MultiQueue { - cmd = append(cmd, "multi_queue") + flags |= netlink.TUNTAP_MULTI_QUEUE } - _, err := subprocess.RunCommand("ip", cmd...) - if err != nil { - return err + tuntap := &netlink.Tuntap{ + LinkAttrs: netlink.LinkAttrs{ + Name: t.Name, + }, + Mode: mode, + Flags: flags, } - if t.Master != "" { - _, err := subprocess.RunCommand("ip", "link", "set", t.Name, "master", t.Master) - if err != nil { - return err - } + err := netlink.LinkAdd(tuntap) + if err != nil { + return fmt.Errorf("failed to create tuntap %q: %w", t.Name, err) } return nil From dabad9109ac32cae7c3d6d8a55e5321a0beaeb3b Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Fri, 25 Apr 2025 15:58:50 +0200 Subject: [PATCH 11/22] incusd/ip/vdpa: Switch to vishvananda/netlink library instead of doing netlink ourselves (incomplete) Signed-off-by: Gwendolyn --- internal/server/ip/vdpa.go | 312 ++++++------------------------------- 1 file changed, 44 insertions(+), 268 deletions(-) diff --git a/internal/server/ip/vdpa.go b/internal/server/ip/vdpa.go index 8f70d5feec4..f09f7d80d31 100644 --- a/internal/server/ip/vdpa.go +++ b/internal/server/ip/vdpa.go @@ -1,7 +1,6 @@ package ip import ( - "encoding/binary" "fmt" "os" "path/filepath" @@ -9,45 +8,6 @@ import ( "syscall" "github.com/vishvananda/netlink" - "github.com/vishvananda/netlink/nl" -) - -// vDPA Netlink name. -const ( - vDPAGenlName = "vdpa" -) - -// vDPA Netlink command. -const ( - _ uint8 = iota - _ - vDPACmdMgmtDevGet - vDPACmdDevNew - vDPACmdDevDel - vDPACmdDevGet - _ -) - -// vDPA Netlink Attributes. -const ( - _ = iota - - // bus name (optional) + dev name together make the parent device handle. - vDPAAttrMgmtDevBusName // string - vDPAAttrMgmtDevDevName // string - vDPAAttrMgmtDevSupportedClasses // u64 - - vDPAAttrDevName // string - vDPAAttrDevID // u32 - vDPAAttrDevVendorID // u32 - vDPAAttrDevMaxVqs // u32 - vDPAAttrDevMaxVqSize // u16 - vDPAAttrDevMinVqSize // u16 - - vDPAAttrDevNetCfgMacAddr // binary - vDPAAttrDevNetStatus // u8 - vDPAAttrDevNetCfgMaxVqp // u16 - vDPAAttrGetNetCfgMTU // u16 ) // Base flags passed to all Netlink requests. @@ -66,7 +26,7 @@ const ( vdpaVhostDevDir = "/dev" ) -// VhostVdpa is the vhost-vdpa device information. +// VhostVDPA is the vhost-vdpa device information. type VhostVDPA struct { Name string Path string @@ -90,19 +50,6 @@ type VDPADev struct { VhostVDPA *VhostVDPA } -// ParseAttributes parses the attributes of a netlink message for a vDPA management device. -func (d *MgmtVDPADev) parseAttributes(attrs []syscall.NetlinkRouteAttr) error { - for _, attr := range attrs { - switch attr.Attr.Type { - case vDPAAttrMgmtDevBusName: - d.BusName = string(attr.Value[:len(attr.Value)-1]) - case vDPAAttrMgmtDevDevName: - d.DevName = string(attr.Value[:len(attr.Value)-1]) - } - } - return nil -} - // getVhostVDPADevInPath returns the VhostVDPA found in the provided parent device's path. func getVhostVDPADevInPath(parentPath string) (*VhostVDPA, error) { fd, err := os.Open(parentPath) @@ -139,177 +86,44 @@ func getVhostVDPADevInPath(parentPath string) (*VhostVDPA, error) { return nil, fmt.Errorf("No vhost-vdpa device found in %s", parentPath) } -// ParseAttributes parses the attributes of a netlink message for a vDPA device. -func (d *VDPADev) parseAttributes(attrs []syscall.NetlinkRouteAttr) error { - d.MgmtDev = &MgmtVDPADev{} - for _, attr := range attrs { - switch attr.Attr.Type { - case vDPAAttrDevName: - d.Name = string(attr.Value[:len(attr.Value)-1]) - case vDPAAttrDevMaxVqs: - d.MaxVQs = binary.LittleEndian.Uint32(attr.Value[:len(attr.Value)]) - case vDPAAttrMgmtDevBusName: - d.MgmtDev.BusName = string(attr.Value[:len(attr.Value)-1]) - case vDPAAttrMgmtDevDevName: - d.MgmtDev.DevName = string(attr.Value[:len(attr.Value)-1]) - } - } - - // Get the vhost-vdpa device associated with the vDPA device. - vhostVDPA, err := getVhostVDPADevInPath(filepath.Join(vdpaBusDevDir, d.Name)) - if err != nil { - return err - } - - d.VhostVDPA = vhostVDPA - return nil -} - -// runVDPANetlinkCmd executes a vDPA netlink command and returns the response. -func runVDPANetlinkCmd(command uint8, flags int, data []*nl.RtAttr) ([][]byte, error) { - f, err := netlink.GenlFamilyGet(vDPAGenlName) - if err != nil { - return nil, fmt.Errorf("Could not get the vDPA Netlink family : %v", err) - } - - msg := &nl.Genlmsg{ - Command: command, - Version: nl.GENL_CTRL_VERSION, - } - - req := nl.NewNetlinkRequest(int(f.ID), commonNLFlags|flags) - // Pass the data into the request header - req.AddData(msg) - for _, d := range data { - req.AddData(d) - } - - // Execute the request - msgs, err := req.Execute(syscall.NETLINK_GENERIC, 0) - if err != nil { - return nil, fmt.Errorf("Could not execute vDPA Netlink request : %v", err) - } - - return msgs, nil -} - -// newNetlinkAttribute creates a new netlink attribute based on the attribute type and data. -func newNetlinkAttribute(attrType int, data any) (*nl.RtAttr, error) { - switch attrType { - case vDPAAttrMgmtDevBusName, vDPAAttrMgmtDevDevName, vDPAAttrDevName: - strData, ok := data.(string) - if !ok { - return nil, fmt.Errorf("Netlink attribute type %d requires string data", attrType) - } - - bytes := make([]byte, len(strData)+1) - copy(bytes, strData) - return nl.NewRtAttr(attrType, bytes), nil - case vDPAAttrMgmtDevSupportedClasses: - u64Data, ok := data.(uint64) - if !ok { - return nil, fmt.Errorf("Netlink attribute type %d requires uint64 data", attrType) - } - - return nl.NewRtAttr(attrType, nl.Uint64Attr(u64Data)), nil - case vDPAAttrDevID, vDPAAttrDevVendorID, vDPAAttrDevMaxVqs: - u32Data, ok := data.(uint32) - if !ok { - return nil, fmt.Errorf("Netlink attribute type %d requires uint32 data", attrType) - } - - return nl.NewRtAttr(attrType, nl.Uint32Attr(u32Data)), nil - case vDPAAttrDevMaxVqSize, vDPAAttrDevMinVqSize, vDPAAttrDevNetCfgMaxVqp, vDPAAttrGetNetCfgMTU: - u16Data, ok := data.(uint16) - if !ok { - return nil, fmt.Errorf("Netlink attribute type %d requires uint16 data", attrType) - } - - return nl.NewRtAttr(attrType, nl.Uint16Attr(u16Data)), nil - case vDPAAttrDevNetStatus: - u8Data, ok := data.(uint8) - if !ok { - return nil, fmt.Errorf("Netlink attribute type %d requires uint8 data", attrType) - } - - return nl.NewRtAttr(attrType, nl.Uint8Attr(u8Data)), nil - case vDPAAttrDevNetCfgMacAddr: - binData, ok := data.([]byte) - if !ok { - return nil, fmt.Errorf("Netlink attribute type %d requires []byte data", attrType) - } - - return nl.NewRtAttr(attrType, binData), nil - default: - return nil, fmt.Errorf("Unknown netlink attribute type %d", attrType) - } -} - -// parseMgmtVDPADevList parses a list of vDPA management device netlink messages. -func parseMgmtVDPADevList(msgs [][]byte) ([]*MgmtVDPADev, error) { - devices := make([]*MgmtVDPADev, 0, len(msgs)) - for _, m := range msgs { - attrs, err := nl.ParseRouteAttr(m[nl.SizeofGenlmsg:]) - if err != nil { - return nil, fmt.Errorf("Could not parse Netlink vDPA management device route attributes : %v", err) - } - - dev := &MgmtVDPADev{} - if err = dev.parseAttributes(attrs); err != nil { - return nil, err - } - - devices = append(devices, dev) - } - - return devices, nil -} - -// parseVDPADevList parses a list of vDPA device netlink messages. -func parseVDPADevList(msgs [][]byte) ([]*VDPADev, error) { - devices := make([]*VDPADev, 0, len(msgs)) - for _, m := range msgs { - attrs, err := nl.ParseRouteAttr(m[nl.SizeofGenlmsg:]) - if err != nil { - return nil, fmt.Errorf("Could not parse Netlink vDPA device route attributes : %v", err) - } - - dev := &VDPADev{} - if err = dev.parseAttributes(attrs); err != nil { - return nil, err - } - - devices = append(devices, dev) - } - - return devices, nil -} - // ListVDPAMgmtDevices returns the list of all vDPA management devices. func ListVDPAMgmtDevices() ([]*MgmtVDPADev, error) { - resp, err := runVDPANetlinkCmd(vDPACmdMgmtDevGet, syscall.NLM_F_DUMP, nil) + netlinkDevices, err := netlink.VDPAGetMGMTDevList() if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get vdpa mgmt dev list: %w", err) } - mgtmDevs, err := parseMgmtVDPADevList(resp) - if err != nil { - return nil, err + devices := make([]*MgmtVDPADev, 0, len(netlinkDevices)) + for _, dev := range netlinkDevices { + devices = append(devices, &MgmtVDPADev{ + BusName: dev.BusName, + DevName: dev.DevName, + }) } - return mgtmDevs, nil + return devices, nil } // ListVDPADevices returns the list of all vDPA devices. func ListVDPADevices() ([]*VDPADev, error) { - resp, err := runVDPANetlinkCmd(vDPACmdDevGet, syscall.NLM_F_DUMP, nil) + netlinkDevices, err := netlink.VDPAGetDevList() if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get vdpa dev list: %w", err) } - devices, err := parseVDPADevList(resp) - if err != nil { - return nil, err + devices := make([]*VDPADev, 0, len(netlinkDevices)) + for _, dev := range netlinkDevices { + vhostVDPA, err := getVhostVDPADevInPath(filepath.Join(vdpaBusDevDir, dev.Name)) + if err != nil { + return nil, err + } + + devices = append(devices, &VDPADev{ + Name: dev.Name, + MaxVQs: dev.MaxVQS, + MgmtDev: nil, // TODO: the netlink library does not expose the associated management device information + VhostVDPA: vhostVDPA, + }) } return devices, nil @@ -317,33 +131,16 @@ func ListVDPADevices() ([]*VDPADev, error) { // AddVDPADevice adds a new vDPA device. func AddVDPADevice(pciDevSlotName string, volatile map[string]string) (*VDPADev, error) { - // List existing vDPA devices - vdpaDevs, err := ListVDPADevices() + existingDevices, err := netlink.VDPAGetDevList() if err != nil { - return nil, err + return nil, fmt.Errorf("failed to get vdpa dev list: %w", err) } existingVDPADevNames := make(map[string]struct{}) - for _, vdpaDev := range vdpaDevs { - existingVDPADevNames[vdpaDev.Name] = struct{}{} - } - - // Create the netlink attributes - header := []*nl.RtAttr{} - busName, err := newNetlinkAttribute(vDPAAttrMgmtDevBusName, "pci") - if err != nil { - return nil, fmt.Errorf("Failed creating vDPA `busName` netlink attr : %v", err) + for _, device := range existingDevices { + existingVDPADevNames[device.Name] = struct{}{} } - header = append(header, busName) - - mgmtDevDevName, err := newNetlinkAttribute(vDPAAttrMgmtDevDevName, pciDevSlotName) - if err != nil { - return nil, fmt.Errorf("Failed creating vDPA `pciDevSlotName` netlink attr : %v", err) - } - - header = append(header, mgmtDevDevName) - // Generate a unique attribute name for the vDPA device (i.e, vdpa0, vdpa1, etc.) baseVDPAName, idx, generatedVDPADevName := "vdpa", 0, "" for { @@ -356,56 +153,35 @@ func AddVDPADevice(pciDevSlotName string, volatile map[string]string) (*VDPADev, idx++ } - devName, err := newNetlinkAttribute(vDPAAttrDevName, generatedVDPADevName) - if err != nil { - return nil, fmt.Errorf("Failed creating vDPA `generatedVDPADevName` netlink attr : %v", err) - } - - header = append(header, devName) - - maxVQP, err := newNetlinkAttribute(vDPAAttrDevNetCfgMaxVqp, vDPAMaxVQP) + err = netlink.VDPANewDev(generatedVDPADevName, "pci", pciDevSlotName, netlink.VDPANewDevParams{ + MaxVQP: vDPAMaxVQP, + }) if err != nil { - return nil, fmt.Errorf("Failed creating vDPA `maxVQP` netlink attr : %v", err) - } - - header = append(header, maxVQP) - - _, err = runVDPANetlinkCmd(vDPACmdDevNew, 0, header) - if err != nil { - return nil, fmt.Errorf("Failed creating vDPA device : %v", err) + return nil, err } - // Now that the vDPA device has been created in the kernel, return the VDPADev struct - msgs, err := runVDPANetlinkCmd(vDPACmdDevGet, 0, []*nl.RtAttr{devName}) + dev, err := netlink.VDPAGetDevByName(generatedVDPADevName) if err != nil { - return nil, fmt.Errorf("Failed getting vDPA device : %v", err) + return nil, fmt.Errorf("failed to get vdpa device %q: %w", generatedVDPADevName, err) } - vdpaDevs, err = parseVDPADevList(msgs) + vhostVDPA, err := getVhostVDPADevInPath(filepath.Join(vdpaBusDevDir, dev.Name)) if err != nil { - return nil, fmt.Errorf("Failed parsing vDPA device : %v", err) + return nil, fmt.Errorf("failed to get vdpa vhost %q: %w", dev.Name, err) } // Update the volatile map volatile["last_state.vdpa.name"] = generatedVDPADevName - return vdpaDevs[0], nil + return &VDPADev{ + Name: dev.Name, + MaxVQs: dev.MaxVQS, + MgmtDev: nil, // TODO: the netlink library does not expose the associated management device information + VhostVDPA: vhostVDPA, + }, nil } // DeleteVDPADevice deletes a vDPA management device. func DeleteVDPADevice(vDPADevName string) error { - header := []*nl.RtAttr{} - devName, err := newNetlinkAttribute(vDPAAttrDevName, vDPADevName) - if err != nil { - return fmt.Errorf("Failed creating vDPA `vDPADevName` netlink attr : %v", err) - } - - header = append(header, devName) - - _, err = runVDPANetlinkCmd(vDPACmdDevDel, 0, header) - if err != nil { - return fmt.Errorf("Cannot delete VDPA dev: %v", err) - } - - return nil + return netlink.VDPADelDev(vDPADevName) } From 37dccfc68c23634607ea1be80a1a827a7f0217d2 Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Sat, 26 Apr 2025 23:13:10 +0200 Subject: [PATCH 12/22] incusd/ip: Refactor family from string to `Family` type Signed-off-by: Gwendolyn --- internal/server/device/nic_ipvlan.go | 20 ++++++++++---------- internal/server/ip/addr.go | 16 ++-------------- internal/server/ip/route.go | 20 ++------------------ internal/server/ip/utils.go | 17 +++++++++++++---- 4 files changed, 27 insertions(+), 46 deletions(-) diff --git a/internal/server/device/nic_ipvlan.go b/internal/server/device/nic_ipvlan.go index d9a4658f429..e80e4e71eb2 100644 --- a/internal/server/device/nic_ipvlan.go +++ b/internal/server/device/nic_ipvlan.go @@ -364,13 +364,13 @@ func (d *nicIPVLAN) Start() (*deviceConfig.RunConfig, error) { // Perform network configuration. for _, keyPrefix := range []string{"ipv4", "ipv6"} { - var ipFamilyArg string + var ipFamily ip.Family switch keyPrefix { case "ipv4": - ipFamilyArg = ip.FamilyV4 + ipFamily = ip.FamilyV4 case "ipv6": - ipFamilyArg = ip.FamilyV6 + ipFamily = ip.FamilyV6 } addresses := util.SplitNTrimSpace(d.config[fmt.Sprintf("%s.address", keyPrefix)], ",", -1, true) @@ -394,7 +394,7 @@ func (d *nicIPVLAN) Start() (*deviceConfig.RunConfig, error) { DevName: "lo", Route: addr.String(), Table: "main", - Family: ipFamilyArg, + Family: ipFamily, } err = r.Add() @@ -411,7 +411,7 @@ func (d *nicIPVLAN) Start() (*deviceConfig.RunConfig, error) { DevName: "lo", Route: addr.String(), Table: d.config[hostTableKey], - Family: ipFamilyArg, + Family: ipFamily, } err := r.Add() @@ -538,13 +538,13 @@ func (d *nicIPVLAN) postStop() error { // Clean up host-side network configuration. for _, keyPrefix := range []string{"ipv4", "ipv6"} { - var ipFamilyArg string + var ipFamily ip.Family switch keyPrefix { case "ipv4": - ipFamilyArg = ip.FamilyV4 + ipFamily = ip.FamilyV4 case "ipv6": - ipFamilyArg = ip.FamilyV6 + ipFamily = ip.FamilyV6 } addresses := util.SplitNTrimSpace(d.config[fmt.Sprintf("%s.address", keyPrefix)], ",", -1, true) @@ -563,7 +563,7 @@ func (d *nicIPVLAN) postStop() error { DevName: "lo", Route: addr.String(), Table: "main", - Family: ipFamilyArg, + Family: ipFamily, } err := r.Delete() @@ -588,7 +588,7 @@ func (d *nicIPVLAN) postStop() error { DevName: "lo", Route: addr.String(), Table: d.config[hostTableKey], - Family: ipFamilyArg, + Family: ipFamily, } err := r.Delete() diff --git a/internal/server/ip/addr.go b/internal/server/ip/addr.go index 5263b556c48..1bc7d0a8454 100644 --- a/internal/server/ip/addr.go +++ b/internal/server/ip/addr.go @@ -11,7 +11,7 @@ type Addr struct { DevName string Address string Scope string - Family string + Family Family } // Add adds new protocol address. @@ -63,24 +63,12 @@ func (a *Addr) scopeNum() (int, error) { // Flush flushes protocol addresses. func (a *Addr) Flush() error { - var family int - switch a.Family { - case FamilyV4: - family = netlink.FAMILY_V4 - case FamilyV6: - family = netlink.FAMILY_V6 - case "": - family = netlink.FAMILY_ALL - default: - return fmt.Errorf("unknown address family %q", a.Family) - } - link, err := linkByName(a.DevName) if err != nil { return err } - addrs, err := netlink.AddrList(link, family) + addrs, err := netlink.AddrList(link, int(a.Family)) if err != nil { return fmt.Errorf("failed to get addresses for device %s: %w", a.DevName, err) } diff --git a/internal/server/ip/route.go b/internal/server/ip/route.go index 04156e2ab21..59094c4f3db 100644 --- a/internal/server/ip/route.go +++ b/internal/server/ip/route.go @@ -16,7 +16,7 @@ type Route struct { Table string Src string Proto string - Family string + Family Family Via string VRF string } @@ -27,14 +27,9 @@ func (r *Route) netlinkRoute() (*netlink.Route, error) { return nil, err } - family, err := r.netlinkFamily() - if err != nil { - return nil, err - } - route := &netlink.Route{ LinkIndex: link.Attrs().Index, - Family: family, + Family: int(r.Family), } if r.Route != "" { @@ -112,17 +107,6 @@ func (r *Route) tableID() (int, error) { } } -func (r *Route) netlinkFamily() (int, error) { - switch r.Family { - case FamilyV4: - return netlink.FAMILY_V4, nil - case FamilyV6: - return netlink.FAMILY_V6, nil - default: - return 0, fmt.Errorf("invalid family %q", r.Family) - } -} - func (r *Route) netlinkProto() (netlink.RouteProtocol, error) { switch r.Proto { case "babel": diff --git a/internal/server/ip/utils.go b/internal/server/ip/utils.go index f6215bbf21b..b63d39b04bf 100644 --- a/internal/server/ip/utils.go +++ b/internal/server/ip/utils.go @@ -6,13 +6,22 @@ import ( "strings" "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" ) -// FamilyV4 represents IPv4 protocol family. -const FamilyV4 = "-4" +// Family can be { FamilyAll, FamilyV4, FamilyV6 }. +type Family int -// FamilyV6 represents IPv6 protocol family. -const FamilyV6 = "-6" +const ( + // FamilyAll specifies any/all family. + FamilyAll Family = unix.AF_UNSPEC + + // FamilyV4 specifies the IPv4 family. + FamilyV4 Family = unix.AF_INET + + // FamilyV6 specifies the IPv6 family. + FamilyV6 Family = unix.AF_INET6 +) // LinkInfo represents the IP link details. type LinkInfo struct { From 2ed3317ac3e05a13e8cc293ca480af1ccca40c87 Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Sun, 27 Apr 2025 09:18:21 +0200 Subject: [PATCH 13/22] incusd/ip/filter: Refactor Rate, Burst and Mtu from strings to integers Signed-off-by: Gwendolyn --- .../server/device/device_utils_network.go | 2 +- internal/server/ip/filter.go | 44 ++++--------------- 2 files changed, 9 insertions(+), 37 deletions(-) diff --git a/internal/server/device/device_utils_network.go b/internal/server/device/device_utils_network.go index dca6585a645..72412188fe8 100644 --- a/internal/server/device/device_utils_network.go +++ b/internal/server/device/device_utils_network.go @@ -570,7 +570,7 @@ func networkSetupHostVethLimits(d *deviceCommon, oldConfig deviceConfig.Device, return fmt.Errorf("Failed to create ingress tc qdisc: %s", err) } - police := &ip.ActionPolice{Rate: fmt.Sprintf("%dbit", egressInt), Burst: fmt.Sprintf("%d", egressInt/40), Mtu: "64kB", Drop: true} + police := &ip.ActionPolice{Rate: uint32(egressInt / 8), Burst: uint32(egressInt / 40), Mtu: 65535, Drop: true} filter := &ip.U32Filter{Filter: ip.Filter{Dev: veth, Parent: "ffff:0", Protocol: "all"}, Value: "0", Mask: "0", Actions: []ip.Action{police}} err = filter.Add() if err != nil { diff --git a/internal/server/ip/filter.go b/internal/server/ip/filter.go index bdb9c70537f..295341e2327 100644 --- a/internal/server/ip/filter.go +++ b/internal/server/ip/filter.go @@ -6,8 +6,6 @@ import ( "github.com/vishvananda/netlink" "golang.org/x/sys/unix" - - "github.com/lxc/incus/v6/shared/units" ) // Action represents an action in filter. @@ -17,49 +15,23 @@ type Action interface { // ActionPolice represents an action of 'police' type. type ActionPolice struct { - Rate string - Burst string - Mtu string + Rate uint32 // in byte/s + Burst uint32 // in byte + Mtu uint32 // in byte Drop bool } func (a *ActionPolice) toNetlink() (netlink.Action, error) { action := netlink.NewPoliceAction() - action.ExceedAction = netlink.TC_POLICE_RECLASSIFY - - if a.Rate != "" { - // TODO: just pass around as number? - rate, err := units.ParseBitSizeString(a.Rate) - if err != nil { - return nil, fmt.Errorf("invalid rate %q: %w", a.Rate, err) - } - - action.Rate = uint32(rate / 8) // netlink wants the rate in bytes/s - } - - if a.Burst != "" { - // TODO: just pass around as number? - burst, err := strconv.ParseUint(a.Burst, 10, 32) - if err != nil { - return nil, fmt.Errorf("invalid burst %q: %w", a.Burst, err) - } - - action.Burst = uint32(burst) - } - - if a.Mtu != "" { - // TODO: just pass around as number? - mtu, err := units.ParseByteSizeString(a.Mtu) - if err != nil { - return nil, fmt.Errorf("invalid mtu %q: %w", a.Mtu, err) - } - - action.Mtu = uint32(mtu) - } + action.Rate = a.Rate + action.Burst = a.Burst + action.Mtu = a.Mtu if a.Drop { action.ExceedAction = netlink.TC_POLICE_SHOT + } else { + action.ExceedAction = netlink.TC_POLICE_RECLASSIFY } return action, nil From 7ffc44d9fa247d38e6790975b5cafbef2a39358d Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Sun, 27 Apr 2025 09:35:11 +0200 Subject: [PATCH 14/22] incusd/ip/filter: Refactor Mask and Value from strings to integers Signed-off-by: Gwendolyn --- .../server/device/device_utils_network.go | 4 ++-- internal/server/ip/filter.go | 20 ++++--------------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/internal/server/device/device_utils_network.go b/internal/server/device/device_utils_network.go index 72412188fe8..7cf29f79393 100644 --- a/internal/server/device/device_utils_network.go +++ b/internal/server/device/device_utils_network.go @@ -556,7 +556,7 @@ func networkSetupHostVethLimits(d *deviceCommon, oldConfig deviceConfig.Device, return fmt.Errorf("Failed to create limit tc class: %s", err) } - filter := &ip.U32Filter{Filter: ip.Filter{Dev: veth, Parent: "1:0", Protocol: "all", Flowid: "1:1"}, Value: "0", Mask: "0"} + filter := &ip.U32Filter{Filter: ip.Filter{Dev: veth, Parent: "1:0", Protocol: "all", Flowid: "1:1"}, Value: 0, Mask: 0} err = filter.Add() if err != nil { return fmt.Errorf("Failed to create tc filter: %s", err) @@ -571,7 +571,7 @@ func networkSetupHostVethLimits(d *deviceCommon, oldConfig deviceConfig.Device, } police := &ip.ActionPolice{Rate: uint32(egressInt / 8), Burst: uint32(egressInt / 40), Mtu: 65535, Drop: true} - filter := &ip.U32Filter{Filter: ip.Filter{Dev: veth, Parent: "ffff:0", Protocol: "all"}, Value: "0", Mask: "0", Actions: []ip.Action{police}} + filter := &ip.U32Filter{Filter: ip.Filter{Dev: veth, Parent: "ffff:0", Protocol: "all"}, Value: 0, Mask: 0, Actions: []ip.Action{police}} err = filter.Add() if err != nil { return fmt.Errorf("Failed to create ingress tc filter: %s", err) diff --git a/internal/server/ip/filter.go b/internal/server/ip/filter.go index 295341e2327..c986a138b4b 100644 --- a/internal/server/ip/filter.go +++ b/internal/server/ip/filter.go @@ -2,7 +2,6 @@ package ip import ( "fmt" - "strconv" "github.com/vishvananda/netlink" "golang.org/x/sys/unix" @@ -48,8 +47,8 @@ type Filter struct { // U32Filter represents universal 32bit traffic control filter. type U32Filter struct { Filter - Value string - Mask string + Value uint32 + Mask uint32 Actions []Action } @@ -75,17 +74,6 @@ func (u32 *U32Filter) Add() error { return err } - // TODO: should just be passed around as an int - mask, err := strconv.ParseUint(u32.Mask, 10, 32) - if err != nil { - return fmt.Errorf("invalid mask %v: %w", u32.Mask, err) - } - - value, err := strconv.ParseUint(u32.Value, 10, 32) - if err != nil { - return fmt.Errorf("invalid value %v: %w", u32.Mask, err) - } - filter := &netlink.U32{ FilterAttrs: netlink.FilterAttrs{ LinkIndex: link.Attrs().Index, @@ -96,8 +84,8 @@ func (u32 *U32Filter) Add() error { Nkeys: 1, Keys: []netlink.TcU32Key{ { - Mask: uint32(mask), - Val: uint32(value), + Mask: u32.Mask, + Val: u32.Value, }, }, }, From eafa5c67dcd2c5f975aa0b09d0912b586859b08b Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Sun, 27 Apr 2025 10:02:07 +0200 Subject: [PATCH 15/22] incusd/ip/link_vxlan: Refactor VxLanID, DstPort and TTL from string to int Signed-off-by: Gwendolyn --- internal/server/ip/link_vxlan.go | 37 +++++------------------- internal/server/network/driver_bridge.go | 28 +++++++++++------- 2 files changed, 25 insertions(+), 40 deletions(-) diff --git a/internal/server/ip/link_vxlan.go b/internal/server/ip/link_vxlan.go index 9430df8994f..7e95ec5021a 100644 --- a/internal/server/ip/link_vxlan.go +++ b/internal/server/ip/link_vxlan.go @@ -3,7 +3,6 @@ package ip import ( "fmt" "net" - "strconv" "github.com/vishvananda/netlink" ) @@ -11,13 +10,13 @@ import ( // Vxlan represents arguments for link of type vxlan. type Vxlan struct { Link - VxlanID string + VxlanID int DevName string Local string Remote string Group string - DstPort string - TTL string + DstPort int + TTL int } // Add adds new virtual link. @@ -27,13 +26,6 @@ func (vxlan *Vxlan) Add() error { return err } - // TODO: all of these these can be passed as int or net.IP - - vxlanID, err := strconv.Atoi(vxlan.VxlanID) - if err != nil { - return err - } - var devIndex int if vxlan.DevName != "" { dev, err := linkByName(vxlan.DevName) @@ -44,6 +36,7 @@ func (vxlan *Vxlan) Add() error { devIndex = dev.Attrs().Index } + // TODO: all of these these can be passed net.IP var group net.IP if vxlan.Group != "" { group = net.ParseIP(vxlan.Group) @@ -79,29 +72,13 @@ func (vxlan *Vxlan) Add() error { } } - var ttl int - if vxlan.TTL != "" { - ttl, err = strconv.Atoi(vxlan.TTL) - if err != nil { - return err - } - } - - var dstport int - if vxlan.DstPort != "" { - dstport, err = strconv.Atoi(vxlan.DstPort) - if err != nil { - return err - } - } - return netlink.LinkAdd(&netlink.Vxlan{ LinkAttrs: attrs, - VxlanId: vxlanID, + VxlanId: vxlan.VxlanID, VtepDevIndex: devIndex, SrcAddr: local, Group: group, - TTL: ttl, - Port: dstport, + TTL: vxlan.TTL, + Port: vxlan.DstPort, }) } diff --git a/internal/server/network/driver_bridge.go b/internal/server/network/driver_bridge.go index c33077e857b..4151e946f6f 100644 --- a/internal/server/network/driver_bridge.go +++ b/internal/server/network/driver_bridge.go @@ -1596,6 +1596,7 @@ func (n *bridge) setup(oldConfig map[string]string) error { // Configure tunnels. for _, tunnel := range tunnels { + getConfig := func(key string) string { return n.config[fmt.Sprintf("tunnel.%s.%s", tunnel, key)] } @@ -1661,26 +1662,33 @@ func (n *bridge) setup(oldConfig map[string]string) error { } tunPort := getConfig("port") - if tunPort == "" { - tunPort = "0" + if tunPort != "" { + vxlan.DstPort, err = strconv.Atoi(tunPort) + if err != nil { + return err + } } - vxlan.DstPort = tunPort - tunID := getConfig("id") if tunID == "" { - tunID = "1" + vxlan.VxlanID = 1 + } else { + vxlan.VxlanID, err = strconv.Atoi(tunID) + if err != nil { + return err + } } - vxlan.VxlanID = tunID - tunTTL := getConfig("ttl") if tunTTL == "" { - tunTTL = "1" + vxlan.TTL = 1 + } else { + vxlan.TTL, err = strconv.Atoi(tunTTL) + if err != nil { + return err + } } - vxlan.TTL = tunTTL - err := vxlan.Add() if err != nil { return err From dbea83005996ec3243248469b93cf29279197b2b Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Sun, 27 Apr 2025 10:11:37 +0200 Subject: [PATCH 16/22] incusd/ip/neigh: Refactor NeighbourIPState from string to int Signed-off-by: Gwendolyn --- internal/server/device/nic_bridged.go | 12 ++-- internal/server/ip/neigh.go | 92 ++++++++++----------------- 2 files changed, 41 insertions(+), 63 deletions(-) diff --git a/internal/server/device/nic_bridged.go b/internal/server/device/nic_bridged.go index 6fa48bdbad3..adb35e01199 100644 --- a/internal/server/device/nic_bridged.go +++ b/internal/server/device/nic_bridged.go @@ -1985,22 +1985,22 @@ func (d *nicBridged) State() (*api.InstanceStateNetwork, error) { // Get IP addresses from IP neighbour cache if present. neighIPs, err := network.GetNeighbourIPs(d.config["parent"], hwAddr) if err == nil { - validStates := []string{ - string(ip.NeighbourIPStatePermanent), - string(ip.NeighbourIPStateNoARP), - string(ip.NeighbourIPStateReachable), + validStates := []ip.NeighbourIPState{ + ip.NeighbourIPStatePermanent, + ip.NeighbourIPStateNoARP, + ip.NeighbourIPStateReachable, } // Add any valid-state neighbour IP entries first. for _, neighIP := range neighIPs { - if slices.Contains(validStates, string(neighIP.State)) { + if slices.Contains(validStates, neighIP.State) { ipStore(neighIP.Addr) } } // Add any non-failed-state entries. for _, neighIP := range neighIPs { - if neighIP.State != ip.NeighbourIPStateFailed && !slices.Contains(validStates, string(neighIP.State)) { + if neighIP.State != ip.NeighbourIPStateFailed && !slices.Contains(validStates, neighIP.State) { ipStore(neighIP.Addr) } } diff --git a/internal/server/ip/neigh.go b/internal/server/ip/neigh.go index a0209fe8df4..3b46b09cea8 100644 --- a/internal/server/ip/neigh.go +++ b/internal/server/ip/neigh.go @@ -5,64 +5,42 @@ import ( "net" "github.com/vishvananda/netlink" + "golang.org/x/sys/unix" ) -// NeighbourIPState can be { PERMANENT | NOARP | REACHABLE | STALE | NONE | INCOMPLETE | DELAY | PROBE | FAILED }. -type NeighbourIPState string - -// NeighbourIPStatePermanent the neighbour entry is valid forever and can be only be removed administratively. -const NeighbourIPStatePermanent = "PERMANENT" - -// NeighbourIPStateNoARP the neighbour entry is valid. No attempts to validate this entry will be made but it can -// be removed when its lifetime expires. -const NeighbourIPStateNoARP = "NOARP" - -// NeighbourIPStateReachable the neighbour entry is valid until the reachability timeout expires. -const NeighbourIPStateReachable = "REACHABLE" - -// NeighbourIPStateStale the neighbour entry is valid but suspicious. -const NeighbourIPStateStale = "STALE" - -// NeighbourIPStateNone this is a pseudo state used when initially creating a neighbour entry or after trying to -// remove it before it becomes free to do so. -const NeighbourIPStateNone = "NONE" - -// NeighbourIPStateIncomplete the neighbour entry has not (yet) been validated/resolved. -const NeighbourIPStateIncomplete = "INCOMPLETE" - -// NeighbourIPStateDelay neighbor entry validation is currently delayed. -const NeighbourIPStateDelay = "DELAY" - -// NeighbourIPStateProbe neighbor is being probed. -const NeighbourIPStateProbe = "PROBE" - -// NeighbourIPStateFailed max number of probes exceeded without success, neighbor validation has ultimately failed. -const NeighbourIPStateFailed = "FAILED" - -func mapNetlinkState(state int) NeighbourIPState { - switch state { - case netlink.NUD_NONE: - return NeighbourIPStateNone - case netlink.NUD_INCOMPLETE: - return NeighbourIPStateIncomplete - case netlink.NUD_REACHABLE: - return NeighbourIPStateReachable - case netlink.NUD_STALE: - return NeighbourIPStateStale - case netlink.NUD_DELAY: - return NeighbourIPStateDelay - case netlink.NUD_PROBE: - return NeighbourIPStateProbe - case netlink.NUD_FAILED: - return NeighbourIPStateFailed - case netlink.NUD_NOARP: - return NeighbourIPStateNoARP - case netlink.NUD_PERMANENT: - return NeighbourIPStatePermanent - default: - return NeighbourIPStateNone - } -} +// NeighbourIPState can be { NeighbourIPStatePermanent | NeighbourIPStateNoARP | NeighbourIPStateReachable | NeighbourIPStateStale | NeighbourIPStateNone | NeighbourIPStateIncomplete | NeighbourIPStateDelay | NeighbourIPStateProbe | NeighbourIPStateFailed }. +type NeighbourIPState int + +const ( + // NeighbourIPStatePermanent the neighbour entry is valid forever and can be only be removed administratively. + NeighbourIPStatePermanent NeighbourIPState = unix.NUD_PERMANENT + + // NeighbourIPStateNoARP the neighbour entry is valid. No attempts to validate this entry will be made but it can + // be removed when its lifetime expires. + NeighbourIPStateNoARP NeighbourIPState = unix.NUD_NOARP + + // NeighbourIPStateReachable the neighbour entry is valid until the reachability timeout expires. + NeighbourIPStateReachable NeighbourIPState = unix.NUD_REACHABLE + + // NeighbourIPStateStale the neighbour entry is valid but suspicious. + NeighbourIPStateStale NeighbourIPState = unix.NUD_STALE + + // NeighbourIPStateNone this is a pseudo state used when initially creating a neighbour entry or after trying to + // remove it before it becomes free to do so. + NeighbourIPStateNone NeighbourIPState = unix.NUD_NONE + + // NeighbourIPStateIncomplete the neighbour entry has not (yet) been validated/resolved. + NeighbourIPStateIncomplete NeighbourIPState = unix.NUD_INCOMPLETE + + // NeighbourIPStateDelay neighbor entry validation is currently delayed. + NeighbourIPStateDelay NeighbourIPState = unix.NUD_DELAY + + // NeighbourIPStateProbe neighbor is being probed. + NeighbourIPStateProbe NeighbourIPState = unix.NUD_PROBE + + // NeighbourIPStateFailed max number of probes exceeded without success, neighbor validation has ultimately failed. + NeighbourIPStateFailed NeighbourIPState = unix.NUD_FAILED +) // Neigh represents arguments for neighbour manipulation. type Neigh struct { @@ -90,7 +68,7 @@ func (n *Neigh) Show() ([]Neigh, error) { neighbours = append(neighbours, Neigh{ Addr: neighbour.IP, MAC: neighbour.HardwareAddr, - State: mapNetlinkState(neighbour.State), + State: NeighbourIPState(neighbour.State), }) } From a01c26c1dab608a24157f05a9b711c88a12b64ff Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Sun, 27 Apr 2025 10:12:52 +0200 Subject: [PATCH 17/22] incusd/ip/qdisc_htb: Refactor Default from string to int Signed-off-by: Gwendolyn --- internal/server/device/device_utils_network.go | 2 +- internal/server/ip/qdisc_htb.go | 12 ++---------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/internal/server/device/device_utils_network.go b/internal/server/device/device_utils_network.go index 7cf29f79393..18e13ef7c19 100644 --- a/internal/server/device/device_utils_network.go +++ b/internal/server/device/device_utils_network.go @@ -544,7 +544,7 @@ func networkSetupHostVethLimits(d *deviceCommon, oldConfig deviceConfig.Device, // Apply new limits if d.config["limits.ingress"] != "" { - qdiscHTB = &ip.QdiscHTB{Qdisc: ip.Qdisc{Dev: veth, Handle: "1:0", Parent: "root"}, Default: "10"} + qdiscHTB = &ip.QdiscHTB{Qdisc: ip.Qdisc{Dev: veth, Handle: "1:0", Parent: "root"}, Default: 10} err := qdiscHTB.Add() if err != nil { return fmt.Errorf("Failed to create root tc qdisc: %s", err) diff --git a/internal/server/ip/qdisc_htb.go b/internal/server/ip/qdisc_htb.go index 4293b9b22b2..f48e6f0681d 100644 --- a/internal/server/ip/qdisc_htb.go +++ b/internal/server/ip/qdisc_htb.go @@ -2,7 +2,6 @@ package ip import ( "fmt" - "strconv" "github.com/vishvananda/netlink" ) @@ -10,7 +9,7 @@ import ( // QdiscHTB represents the hierarchy token bucket qdisc object. type QdiscHTB struct { Qdisc - Default string + Default uint32 } // Add adds a htb qdisc to a device. @@ -22,14 +21,7 @@ func (q *QdiscHTB) Add() error { htb := netlink.NewHtb(attrs) - if q.Default != "" { - defcls, err := strconv.Atoi(q.Default) - if err != nil { - return fmt.Errorf("invalid htb default class %q: %w", q.Default, err) - } - - htb.Defcls = uint32(defcls) - } + htb.Defcls = q.Default err = netlink.QdiscAdd(htb) if err != nil { From 828bb6956b43ab41806726b051e334fe3f7e514f Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Sun, 27 Apr 2025 10:26:46 +0200 Subject: [PATCH 18/22] incusd/ip/vdpa: Remove unused Signed-off-by: Gwendolyn --- internal/server/ip/vdpa.go | 58 -------------------------------------- 1 file changed, 58 deletions(-) diff --git a/internal/server/ip/vdpa.go b/internal/server/ip/vdpa.go index f09f7d80d31..906da89af28 100644 --- a/internal/server/ip/vdpa.go +++ b/internal/server/ip/vdpa.go @@ -5,16 +5,10 @@ import ( "os" "path/filepath" "strings" - "syscall" "github.com/vishvananda/netlink" ) -// Base flags passed to all Netlink requests. -const ( - commonNLFlags = syscall.NLM_F_REQUEST | syscall.NLM_F_ACK -) - // MAX_VQP is the maximum number of VQPs supported by the vDPA device and is always the same as of now. const ( vDPAMaxVQP = uint16(16) @@ -32,20 +26,12 @@ type VhostVDPA struct { Path string } -// MgmtVDPADev represents the vDPA management device information. -type MgmtVDPADev struct { - BusName string // e.g. "pci" - DevName string // e.g. "0000:00:08.2" -} - // VDPADev represents the vDPA device information. type VDPADev struct { // Name of the vDPA created device. e.g. "vdpa0" (note: the iproute2 associated command would look like `vdpa dev add mgmtdev pci/ name vdpa0 max_vqp `). Name string // Max VQs supported by the vDPA device. MaxVQs uint32 - // Associated vDPA management device. - MgmtDev *MgmtVDPADev // Associated vhost-vdpa device. VhostVDPA *VhostVDPA } @@ -86,49 +72,6 @@ func getVhostVDPADevInPath(parentPath string) (*VhostVDPA, error) { return nil, fmt.Errorf("No vhost-vdpa device found in %s", parentPath) } -// ListVDPAMgmtDevices returns the list of all vDPA management devices. -func ListVDPAMgmtDevices() ([]*MgmtVDPADev, error) { - netlinkDevices, err := netlink.VDPAGetMGMTDevList() - if err != nil { - return nil, fmt.Errorf("failed to get vdpa mgmt dev list: %w", err) - } - - devices := make([]*MgmtVDPADev, 0, len(netlinkDevices)) - for _, dev := range netlinkDevices { - devices = append(devices, &MgmtVDPADev{ - BusName: dev.BusName, - DevName: dev.DevName, - }) - } - - return devices, nil -} - -// ListVDPADevices returns the list of all vDPA devices. -func ListVDPADevices() ([]*VDPADev, error) { - netlinkDevices, err := netlink.VDPAGetDevList() - if err != nil { - return nil, fmt.Errorf("failed to get vdpa dev list: %w", err) - } - - devices := make([]*VDPADev, 0, len(netlinkDevices)) - for _, dev := range netlinkDevices { - vhostVDPA, err := getVhostVDPADevInPath(filepath.Join(vdpaBusDevDir, dev.Name)) - if err != nil { - return nil, err - } - - devices = append(devices, &VDPADev{ - Name: dev.Name, - MaxVQs: dev.MaxVQS, - MgmtDev: nil, // TODO: the netlink library does not expose the associated management device information - VhostVDPA: vhostVDPA, - }) - } - - return devices, nil -} - // AddVDPADevice adds a new vDPA device. func AddVDPADevice(pciDevSlotName string, volatile map[string]string) (*VDPADev, error) { existingDevices, err := netlink.VDPAGetDevList() @@ -176,7 +119,6 @@ func AddVDPADevice(pciDevSlotName string, volatile map[string]string) (*VDPADev, return &VDPADev{ Name: dev.Name, MaxVQs: dev.MaxVQS, - MgmtDev: nil, // TODO: the netlink library does not expose the associated management device information VhostVDPA: vhostVDPA, }, nil } From 7031eebd6e9c9f8de1756d3951336246b3aa45b0 Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Sun, 27 Apr 2025 10:54:20 +0200 Subject: [PATCH 19/22] incusd/ip: Merge GetLinkInfoByName and LinkFromName into LinkByName Signed-off-by: Gwendolyn --- internal/server/ip/link.go | 48 +++++++++++------ internal/server/ip/utils.go | 67 ------------------------ internal/server/network/driver_bridge.go | 6 +-- internal/server/network/driver_ovn.go | 4 +- 4 files changed, 38 insertions(+), 87 deletions(-) diff --git a/internal/server/ip/link.go b/internal/server/ip/link.go index 2a0d1e7c1bd..41c49ba110a 100644 --- a/internal/server/ip/link.go +++ b/internal/server/ip/link.go @@ -23,6 +23,14 @@ type Link struct { Up bool } +// LinkInfo has additional information about a link. +type LinkInfo struct { + Link + OperationalState string + SlaveKind string + VlanID int +} + // args generate common arguments for the virtual link. func (l *Link) args() []string { var result []string @@ -122,11 +130,11 @@ func (l *Link) netlinkAttrs() (netlink.LinkAttrs, error) { return linkAttrs, nil } -// LinkFromName returns a Link from a device name. -func LinkFromName(name string) (*Link, error) { +// LinkByName returns a Link from a device name. +func LinkByName(name string) (LinkInfo, error) { link, err := linkByName(name) if err != nil { - return nil, err + return LinkInfo{}, err } var parent, master string @@ -134,7 +142,7 @@ func LinkFromName(name string) (*Link, error) { if link.Attrs().ParentIndex != 0 { parentLink, err := netlink.LinkByIndex(link.Attrs().ParentIndex) if err != nil { - return nil, err + return LinkInfo{}, err } parent = parentLink.Attrs().Name @@ -143,22 +151,32 @@ func LinkFromName(name string) (*Link, error) { if link.Attrs().MasterIndex != 0 { masterLink, err := netlink.LinkByIndex(link.Attrs().MasterIndex) if err != nil { - return nil, err + return LinkInfo{}, err } master = masterLink.Attrs().Name } - return &Link{ - Name: link.Attrs().Name, - Kind: link.Type(), - MTU: uint32(link.Attrs().MTU), - Parent: parent, - Address: link.Attrs().HardwareAddr, - TXQueueLength: uint32(link.Attrs().TxQLen), - AllMulticast: link.Attrs().Allmulti == 1, - Master: master, - Up: (link.Attrs().Flags & net.FlagUp) != 0, + var vlanID int + vlan, ok := link.(*netlink.Vlan) + if ok { + vlanID = vlan.VlanId + } + + return LinkInfo{ + Link: Link{ + Name: link.Attrs().Name, + Kind: link.Type(), + MTU: uint32(link.Attrs().MTU), + Parent: parent, + Address: link.Attrs().HardwareAddr, + TXQueueLength: uint32(link.Attrs().TxQLen), + AllMulticast: link.Attrs().Allmulti == 1, + Master: master, + Up: (link.Attrs().Flags & net.FlagUp) != 0, + }, + OperationalState: link.Attrs().OperState.String(), + VlanID: vlanID, }, nil } diff --git a/internal/server/ip/utils.go b/internal/server/ip/utils.go index b63d39b04bf..1091e7771a3 100644 --- a/internal/server/ip/utils.go +++ b/internal/server/ip/utils.go @@ -23,25 +23,6 @@ const ( FamilyV6 Family = unix.AF_INET6 ) -// LinkInfo represents the IP link details. -type LinkInfo struct { - InterfaceName string - Link string - Master string - Address string - TXQueueLength uint32 - MTU uint32 - OperationalState string - Info struct { - Kind string - SlaveKind string - Data struct { - Protocol string - ID int - } - } -} - func linkByName(name string) (netlink.Link, error) { link, err := netlink.LinkByName(name) if err != nil { @@ -51,54 +32,6 @@ func linkByName(name string) (netlink.Link, error) { return link, nil } -// GetLinkInfoByName returns the detailed information for the given link. -func GetLinkInfoByName(name string) (LinkInfo, error) { - info := LinkInfo{} - - link, err := linkByName(name) - if err != nil { - return info, err - } - - info.InterfaceName = link.Attrs().Name - - if link.Attrs().ParentIndex != 0 { - parentLink, err := netlink.LinkByIndex(link.Attrs().ParentIndex) - if err != nil { - return info, fmt.Errorf("failed to get parent link %d of %q: %w", link.Attrs().ParentIndex, name, err) - } - - info.Link = parentLink.Attrs().Name - } - - if link.Attrs().MasterIndex != 0 { - masterLink, err := netlink.LinkByIndex(link.Attrs().MasterIndex) - if err != nil { - return info, fmt.Errorf("failed to get master link %d of %q: %w", link.Attrs().ParentIndex, name, err) - } - - info.Master = masterLink.Attrs().Name - } - - info.Address = link.Attrs().HardwareAddr.String() - info.TXQueueLength = uint32(link.Attrs().TxQLen) - info.MTU = uint32(link.Attrs().MTU) - info.OperationalState = link.Attrs().OperState.String() - info.Info.Kind = link.Type() - - if link.Attrs().Slave != nil { - info.Info.Kind = link.Attrs().Slave.SlaveType() - } - - vlan, ok := link.(*netlink.Vlan) - if ok { - info.Info.Data.ID = vlan.VlanId - info.Info.Data.Protocol = vlan.VlanProtocol.String() - } - - return info, nil -} - func parseHandle(id string) (uint32, error) { if id == "root" { return netlink.HANDLE_ROOT, nil diff --git a/internal/server/network/driver_bridge.go b/internal/server/network/driver_bridge.go index 4151e946f6f..919e63dbde9 100644 --- a/internal/server/network/driver_bridge.go +++ b/internal/server/network/driver_bridge.go @@ -1121,12 +1121,12 @@ func (n *bridge) setup(oldConfig map[string]string) error { } } else if vlanID > 0 { // If the interface exists and VLAN ID was provided, ensure it has the same parent and VLAN ID and is not attached to a different network. - linkInfo, err := ip.GetLinkInfoByName(entry) + linkInfo, err := ip.LinkByName(entry) if err != nil { return fmt.Errorf("Failed to get link info for external interface %q", entry) } - if linkInfo.Info.Kind != "vlan" || linkInfo.Link != ifParent || linkInfo.Info.Data.ID != vlanID || !(linkInfo.Master == "" || linkInfo.Master == n.name) { + if linkInfo.Kind != "vlan" || linkInfo.Parent != ifParent || linkInfo.VlanID != vlanID || (linkInfo.Master != "" && linkInfo.Master != n.name) { return fmt.Errorf("External interface %q already in use", entry) } } @@ -3129,7 +3129,7 @@ func (n *bridge) deleteChildren() error { } for _, iface := range ifaces { - l, err := ip.LinkFromName(iface.Name) + l, err := ip.LinkByName(iface.Name) if err != nil { return err } diff --git a/internal/server/network/driver_ovn.go b/internal/server/network/driver_ovn.go index 739b15700ca..da1c773a1b0 100644 --- a/internal/server/network/driver_ovn.go +++ b/internal/server/network/driver_ovn.go @@ -2491,12 +2491,12 @@ func (n *ovn) setup(update bool) error { } } else if vlanID > 0 { // If the interface exists and VLAN ID was provided, ensure it has the same parent and VLAN ID and is not attached to a different network. - linkInfo, err := ip.GetLinkInfoByName(entry) + linkInfo, err := ip.LinkByName(entry) if err != nil { return fmt.Errorf("Failed to get link info for external interface %q", entry) } - if linkInfo.Info.Kind != "vlan" || linkInfo.Link != ifParent || linkInfo.Info.Data.ID != vlanID || !(linkInfo.Master == "" || linkInfo.Master == n.name) { + if linkInfo.Kind != "vlan" || linkInfo.Parent != ifParent || linkInfo.VlanID != vlanID || (linkInfo.Master != "" && linkInfo.Master != n.name) { return fmt.Errorf("External interface %q already in use", entry) } } From 924191d96c5150c6bd246af2d06bc1d0719f1f1c Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Sun, 27 Apr 2025 11:08:17 +0200 Subject: [PATCH 20/22] incusd/ip/link: Refactor SetVfSpoofchk mode parameter to bool Signed-off-by: Gwendolyn --- internal/server/device/device_utils_network.go | 11 +++-------- internal/server/ip/link.go | 7 ++----- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/internal/server/device/device_utils_network.go b/internal/server/device/device_utils_network.go index 18e13ef7c19..8dfd265c06d 100644 --- a/internal/server/device/device_utils_network.go +++ b/internal/server/device/device_utils_network.go @@ -786,7 +786,7 @@ func networkSRIOVSetupVF(d deviceCommon, vfParent string, vfDevice string, vfID } // Now that MAC is set on VF, we can enable spoof checking. - err = link.SetVfSpoofchk(volatile["last_state.vf.id"], "on") + err = link.SetVfSpoofchk(volatile["last_state.vf.id"], true) if err != nil { return vfPCIDev, 0, fmt.Errorf("Failed enabling spoof check for VF %q: %w", volatile["last_state.vf.id"], err) } @@ -799,7 +799,7 @@ func networkSRIOVSetupVF(d deviceCommon, vfParent string, vfDevice string, vfID if useSpoofCheck { // Ensure spoof checking is disabled if not enabled in instance (only for real VF). - err = link.SetVfSpoofchk(volatile["last_state.vf.id"], "off") + err = link.SetVfSpoofchk(volatile["last_state.vf.id"], false) if err != nil { return vfPCIDev, 0, fmt.Errorf("Failed disabling spoof check for VF %q: %w", volatile["last_state.vf.id"], err) } @@ -912,13 +912,8 @@ func networkSRIOVRestoreVF(d deviceCommon, useSpoofCheck bool, volatile map[stri // Reset VF MAC spoofing protection if recorded. Do this first before resetting the MAC // to avoid any issues with zero MACs refusing to be set whilst spoof check is on. if useSpoofCheck && volatile["last_state.vf.spoofcheck"] != "" { - mode := "off" - if util.IsTrue(volatile["last_state.vf.spoofcheck"]) { - mode = "on" - } - link := &ip.Link{Name: parent} - err := link.SetVfSpoofchk(volatile["last_state.vf.id"], mode) + err := link.SetVfSpoofchk(volatile["last_state.vf.id"], util.IsTrue(volatile["last_state.vf.spoofcheck"])) if err != nil { return err } diff --git a/internal/server/ip/link.go b/internal/server/ip/link.go index 41c49ba110a..d9291747ed4 100644 --- a/internal/server/ip/link.go +++ b/internal/server/ip/link.go @@ -328,20 +328,17 @@ func (l *Link) SetVfVlan(vf string, vlan string) error { } // SetVfSpoofchk turns packet spoof checking on or off for the specified VF. -func (l *Link) SetVfSpoofchk(vf string, mode string) error { +func (l *Link) SetVfSpoofchk(vf string, on bool) error { vfInt, err := strconv.Atoi(vf) if err != nil { return err } - // TODO: pass as bool - check := mode == "on" - return netlink.LinkSetVfSpoofchk(&netlink.GenericLink{ LinkAttrs: netlink.LinkAttrs{ Name: l.Name, }, - }, vfInt, check) + }, vfInt, on) } // VirtFuncInfo holds information about vf. From 45bed3e0b89b431fa17ea8e555d1437a65e6abac Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Sun, 27 Apr 2025 12:27:50 +0200 Subject: [PATCH 21/22] fixup! incusd/ip/link: Switch to netlink (incomplete) Signed-off-by: Gwendolyn --- internal/server/device/device_utils_network.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/server/device/device_utils_network.go b/internal/server/device/device_utils_network.go index 8dfd265c06d..e9cfba3609c 100644 --- a/internal/server/device/device_utils_network.go +++ b/internal/server/device/device_utils_network.go @@ -700,7 +700,7 @@ func bgpRemovePrefix(d *deviceCommon, config map[string]string) error { return nil } -// networkSRIOVParentVFInfo returns info about an SR-IOV virtual function from the parent NIC using the ip tool. +// networkSRIOVParentVFInfo returns info about an SR-IOV virtual function from the parent NIC. func networkSRIOVParentVFInfo(vfParent string, vfID int) (ip.VirtFuncInfo, error) { link := &ip.Link{Name: vfParent} vfi, err := link.GetVFInfo(vfID) From d3207fe9cf11a7380eb1c121239a19513c92dcbd Mon Sep 17 00:00:00 2001 From: Gwendolyn Date: Sun, 27 Apr 2025 21:30:10 +0200 Subject: [PATCH 22/22] fixup! incusd/ip/link: Switch to netlink (incomplete) Signed-off-by: Gwendolyn --- internal/server/ip/link_veth.go | 2 +- internal/server/ip/link_vlan.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/server/ip/link_veth.go b/internal/server/ip/link_veth.go index bfd86270153..f01ed5b6ff1 100644 --- a/internal/server/ip/link_veth.go +++ b/internal/server/ip/link_veth.go @@ -13,7 +13,7 @@ type Veth struct { // Add adds new virtual link. func (veth *Veth) Add() error { - // TODO: the netlink library does not support configuring anything except name and hwaddr on the peer + // TODO: blocked on https://github.com/vishvananda/netlink/pull/1079 err := veth.Link.add("veth", append([]string{"peer"}, veth.Peer.args()...)) if err != nil { return err diff --git a/internal/server/ip/link_vlan.go b/internal/server/ip/link_vlan.go index 3f24e81ef1a..3fab5546bfd 100644 --- a/internal/server/ip/link_vlan.go +++ b/internal/server/ip/link_vlan.go @@ -19,6 +19,6 @@ func (vlan *Vlan) additionalArgs() []string { // Add adds new virtual link. func (vlan *Vlan) Add() error { - // TODO: netlink library does not support vlan flags (necessary for gvrp) + // TODO: blocked on https://github.com/vishvananda/netlink/pull/1078 return vlan.Link.add("vlan", vlan.additionalArgs()) }