Skip to content

Commit 183ab45

Browse files
committed
review: move the bridge attach helper beside its caller and pass the lint gate
attachBridgeUp is bridge-specific (IFLA_MASTER is its whole point) and the network package carried no backend plumbing before it, so it moves unexported into network/bridge; the two tuning constants TuneTAP shares are exported instead. The G115 casts carry kernel-issued ifindexes and a guarded MTU, so they take nolint with reasons. Two-line godocs tightened to one.
1 parent 629fd82 commit 183ab45

2 files changed

Lines changed: 30 additions & 31 deletions

File tree

network/bridge/bridge_linux.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010

1111
"github.com/projecteru2/core/log"
1212
"github.com/vishvananda/netlink"
13+
"github.com/vishvananda/netlink/nl"
14+
"golang.org/x/sys/unix"
1315

1416
"github.com/cocoonstack/cocoon/config"
1517
"github.com/cocoonstack/cocoon/gc"
@@ -118,7 +120,7 @@ func (b *Bridge) Add(ctx context.Context, vmID string, vmCfg *types.VMConfig, sp
118120
}
119121
added = append(added, spec.Index)
120122

121-
if aErr := network.AttachBridgeUp(tapIndex, b.bridgeIdx, br.Attrs().MTU); aErr != nil {
123+
if aErr := attachBridgeUp(tapIndex, b.bridgeIdx, br.Attrs().MTU); aErr != nil {
122124
return nil, aErr
123125
}
124126

@@ -179,6 +181,26 @@ func CleanupTAPs(vmIDs []string) []string {
179181
return cleaned
180182
}
181183

184+
// attachBridgeUp enslaves a TAP to the bridge, applies MTU/txqlen/GRO tuning and brings it up in one RTM_SETLINK, paying the node-wide rtnl lock once.
185+
func attachBridgeUp(tapIndex, bridgeIndex, mtu int) error {
186+
req := nl.NewNetlinkRequest(unix.RTM_SETLINK, unix.NLM_F_ACK)
187+
msg := nl.NewIfInfomsg(unix.AF_UNSPEC)
188+
msg.Index = int32(tapIndex) //nolint:gosec // kernel-issued ifindex fits int32
189+
msg.Flags = unix.IFF_UP
190+
msg.Change = unix.IFF_UP
191+
req.AddData(msg)
192+
req.AddData(nl.NewRtAttr(unix.IFLA_MASTER, nl.Uint32Attr(uint32(bridgeIndex)))) //nolint:gosec // kernel-issued ifindex fits uint32
193+
req.AddData(nl.NewRtAttr(unix.IFLA_TXQLEN, nl.Uint32Attr(network.TAPTxQueueLen)))
194+
req.AddData(nl.NewRtAttr(unix.IFLA_GRO_MAX_SIZE, nl.Uint32Attr(network.GROMaxSize)))
195+
if mtu > 0 {
196+
req.AddData(nl.NewRtAttr(unix.IFLA_MTU, nl.Uint32Attr(uint32(mtu)))) //nolint:gosec // guarded > 0; kernel MTU fits uint32
197+
}
198+
if _, err := req.Execute(unix.NETLINK_ROUTE, 0); err != nil {
199+
return fmt.Errorf("attach tap %d to bridge %d: %w", tapIndex, bridgeIndex, err)
200+
}
201+
return nil
202+
}
203+
182204
func tearDownTAPs(vmID string, indices []int, bestEffort bool) error {
183205
for _, i := range indices {
184206
name := tapName(vmID, i)

network/tap_linux.go

Lines changed: 7 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,17 @@ import (
66
"fmt"
77

88
"github.com/vishvananda/netlink"
9-
"github.com/vishvananda/netlink/nl"
10-
"golang.org/x/sys/unix"
119
)
1210

1311
const (
14-
// tapTxQueueLen absorbs traffic bursts (especially UDP) without dropping; the kernel default of 1000 is too small for VM workloads.
15-
tapTxQueueLen = 10000
12+
// TAPTxQueueLen absorbs traffic bursts (especially UDP) without dropping; the kernel default of 1000 is too small for VM workloads.
13+
TAPTxQueueLen = 10000
1614

17-
// groMaxSize matches the maximum virtio-net segment size so the kernel aggregates inbound packets before CH reads them.
18-
groMaxSize = 65536
15+
// GROMaxSize matches the maximum virtio-net segment size so the kernel aggregates inbound packets before CH reads them.
16+
GROMaxSize = 65536
1917
)
2018

2119
// CreateTAP adds a multi-queue TAP sized for numQueues virtio-net queues and returns its index, then closes the kernel fds (CH/QEMU reopen it by name).
22-
// TUNSETIFF carries no index, so netlink resolves one after the ioctl and swallows a failure to; an unresolved index must not reach a caller as valid.
2320
func CreateTAP(name string, numQueues int) (int, error) {
2421
// queue_pairs = num_queues / 2 (TX+RX pair); multi-queue needs >1 and must match the VMM's IFF_MULTI_QUEUE expectation.
2522
queuePairs := max(1, numQueues/2) //nolint:mnd
@@ -41,38 +38,18 @@ func CreateTAP(name string, numQueues int) (int, error) {
4138
for _, fd := range tap.Fds {
4239
_ = fd.Close()
4340
}
41+
// netlink's post-TUNSETIFF index lookup drops its error, so 0 can land here.
4442
index := tap.Attrs().Index
4543
if index == 0 {
4644
return 0, fmt.Errorf("resolve index of tap %s", name)
4745
}
4846
return index, nil
4947
}
5048

51-
// AttachBridgeUp enslaves a TAP to a bridge, applies the queue and GRO tuning and brings it up, in a single RTM_SETLINK.
52-
// Every netlink write serializes on the kernel's node-wide rtnl lock, so on a dense fill the op count, not the work, is the cost.
53-
func AttachBridgeUp(tapIndex, bridgeIndex, mtu int) error {
54-
req := nl.NewNetlinkRequest(unix.RTM_SETLINK, unix.NLM_F_ACK)
55-
msg := nl.NewIfInfomsg(unix.AF_UNSPEC)
56-
msg.Index = int32(tapIndex)
57-
msg.Flags = unix.IFF_UP
58-
msg.Change = unix.IFF_UP
59-
req.AddData(msg)
60-
req.AddData(nl.NewRtAttr(unix.IFLA_MASTER, nl.Uint32Attr(uint32(bridgeIndex))))
61-
req.AddData(nl.NewRtAttr(unix.IFLA_TXQLEN, nl.Uint32Attr(tapTxQueueLen)))
62-
req.AddData(nl.NewRtAttr(unix.IFLA_GRO_MAX_SIZE, nl.Uint32Attr(groMaxSize)))
63-
if mtu > 0 {
64-
req.AddData(nl.NewRtAttr(unix.IFLA_MTU, nl.Uint32Attr(uint32(mtu))))
65-
}
66-
if _, err := req.Execute(unix.NETLINK_ROUTE, 0); err != nil {
67-
return fmt.Errorf("attach tap %d to bridge %d: %w", tapIndex, bridgeIndex, err)
68-
}
69-
return nil
70-
}
71-
7249
// TuneTAP applies best-effort performance tuning to a TAP device.
7350
func TuneTAP(link netlink.Link) error {
74-
if err := netlink.LinkSetTxQLen(link, tapTxQueueLen); err != nil {
51+
if err := netlink.LinkSetTxQLen(link, TAPTxQueueLen); err != nil {
7552
return err
7653
}
77-
return netlink.LinkSetGROMaxSize(link, groMaxSize)
54+
return netlink.LinkSetGROMaxSize(link, GROMaxSize)
7855
}

0 commit comments

Comments
 (0)