FreeBSD 32-bit build support #155
Conversation
hakansa
changed the title
internal/wgfreebsd/client_freebsd.go
Outdated
| Name: dname, | ||
| Data: mem, | ||
| Size: uint64(sz), | ||
| Size: uint32(sz), |
There was a problem hiding this comment.
I think an explanation on why these changes are needed would be helpful in the commit message + PR description. Why do 32 bit builds not work without this and does this impact 64 bit builds at all?
There was a problem hiding this comment.
@jrife thanks for the feedback, I've updated the PR description and added a commit. You can find the details in the PR description, 32-bit builds doesn't work because of type mismatches. These change has no impact to 64-bit builds since 64 bit builds still use data types that has "64 bits". It is tested with both 32-bit and 64-bit machines.
internal/wgfreebsd/client_freebsd.go
Outdated
| Name: dname, | ||
| Data: mem, | ||
| Size: uint64(sz), | ||
| Size: uint32(sz), |
There was a problem hiding this comment.
I think it doesn't since WGDataIO type uses C.struct_wg_data_io
There was a problem hiding this comment.
uint32 here makes the build fail on 64 bit systems. Tried this on my freebsd box:
root@freebsd:~/wgctrl-go # go test . -v
# golang.zx2c4.com/wireguard/wgctrl/internal/wgfreebsd
internal/wgfreebsd/client_freebsd.go:197:9: cannot use uint32(sz) (value of type uint32) as uint64 value in struct literal
FAIL golang.zx2c4.com/wireguard/wgctrl [build failed]
FAIL
root@freebsd:~/wgctrl-go #
This seems to fix it:
--- a/internal/wgfreebsd/client_freebsd.go
+++ b/internal/wgfreebsd/client_freebsd.go
@@ -194,7 +194,7 @@ func (c *Client) ConfigureDevice(name string, cfg wgtypes.Config) error {
data := wgh.WGDataIO{
Name: dname,
Data: mem,
- Size: uint32(sz),
+ Size: wgh.SizeT(sz),
}
if err := c.ioctlWGDataIO(wgh.SIOCSWG, &data); err != nil {
diff --git a/internal/wgfreebsd/internal/wgh/defs.go b/internal/wgfreebsd/internal/wgh/defs.go
index ccd593e..8f63dfb 100644
--- a/internal/wgfreebsd/internal/wgh/defs.go
+++ b/internal/wgfreebsd/internal/wgh/defs.go
@@ -49,3 +49,7 @@ type Ifgroupreq C.struct_go_ifgroupreq
type Ifgreq C.struct_ifg_req
type WGDataIO C.struct_wg_data_io
+
+func SizeT(i int) C.size_t {
+ return C.size_t(i)
+}
You can run wgctrl-go/internal/wgfreebsd/internal/wgh/generate.sh to regenerate all the defs_*.go files and will add a SizeT helper to each filling in the appropriate type for C.size_t:
diff --git a/internal/wgfreebsd/internal/wgh/defs_freebsd_amd64.go b/internal/wgfreebsd/internal/wgh/defs_freebsd_amd64.go
index 2b8aaa4..2be005a 100644
--- a/internal/wgfreebsd/internal/wgh/defs_freebsd_amd64.go
+++ b/internal/wgfreebsd/internal/wgh/defs_freebsd_amd64.go
@@ -30,3 +30,7 @@ type WGDataIO struct {
Data *byte
Size uint64
}
+
+func SizeT(i int) uint64 {
+ return uint64(i)
+}
There was a problem hiding this comment.
@jrife thanks! committed the changes and seems it is compling file both with 32-bit and 64-bit. (tested on arm arch.)
There was a problem hiding this comment.
Would it be possible to get this PR merged?
Signed-off-by: Matt Layher <[email protected]> Signed-off-by: Hakan Sariman <[email protected]>
Ensure WGDataIO.Size uses uint32 on ILP32 to match C struct. Use C.size_t for nvlist_unpack and C.uint64_t for nvlist_add_number to avoid build failures from size_t/ulong mismatches. Signed-off-by: Hakan Sariman <[email protected]>
Signed-off-by: Hakan Sariman <[email protected]>
.builds/openbsd.yml
Outdated
| go test -c . | ||
| doas bash -c 'WGCTRL_INTEGRATION=yesreallydoit ./wgctrl.test -test.v -test.run TestIntegration' | ||
| # TODO: re-enable once Go 1.19 is available in openbsd/latest and wireguard-go can be built | ||
| exit 0 |
There was a problem hiding this comment.
Is this change necessary? Seems unrelated to the purpose of the PR?
There was a problem hiding this comment.
I dont remember that I've committed these changes, @mdlayher might have the answer
|
|
||
| # Set up wireguard-go on all OSes. | ||
| git clone git://git.zx2c4.com/wireguard-go | ||
| git clone https://git.zx2c4.com/wireguard-go |
There was a problem hiding this comment.
Same, is this a local change you made that snuck in?
|
|
||
| require ( | ||
| github.com/google/go-cmp v0.5.9 | ||
| github.com/google/go-cmp v0.6.0 |
|
@hakansa you have a commit, go.mod: bump dependencies, adjust CI builds, that seems to have been added later on. Should this be included in this PR? What's the purpose? |
it's part of master: a9ab227 likely when @hakansa committed the requested changes (e.g. #155 (comment) |
|
is there anythng that i can do the make this pr merged? We have some customers waiting for 32-bit support 👐 |
4 participants
This PR adds support for building and running wgctrl on 32-bit FreeBSD systems.
On 32-bit FreeBSD,
size_tandulongare 32 bits under the ILP32 model, whereas on 64-bit FreeBSD they are 64 bits under LP64. The Go code was previously using uint64 (8 bytes) forWGDataIO.Size(which the C struct expects to be 4 bytes on ILP32),C.ulongto pass buffer lengths tonvlist_unpack, andC.ulongto pass numeric values tonvlist_add_number(which expects auint64_t). These mismatches cause build failures on ILP32.Changes
client_freebsd.go, updateWGDataIO.Sizefromuint64(sz) touint32(sz). On ILP32 this writes a 32-bit size, matching the C struct. On LP64, uint32(sz) zero-extends appropriately.sz := C.ulong(len(d))tosz := C.size_t(len(d))sonvlist_unpackalways receives exactly asize_t.C.nvlist_add_number(nvl, ckey, C.ulong(value))toC.nvlist_add_number(nvl, ckey, C.uint64_t(value)), ensuring 64-bit numbers are passed correctly on ILP32 and LP64.