Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os, syscall, x/sys/unix: add support for FreeBSD 12 #22448

Closed
mikioh opened this issue Oct 26, 2017 · 16 comments
Closed

os, syscall, x/sys/unix: add support for FreeBSD 12 #22448

mikioh opened this issue Oct 26, 2017 · 16 comments

Comments

@mikioh
Copy link
Contributor

mikioh commented Oct 26, 2017

As described in https://svnweb.freebsd.org/base?view=revision&revision=318736, FreeBSD 12 changes the ABI related to file status and directory file format like the following:

--- a/unix/ztypes_freebsd_${goarch}.go
+++ b/unix/ztypes_freebsd_$(goarch}.go
type Stat_t struct {
-       Dev           uint32
-       Ino           uint32
+       Dev           uint64
+       Ino           uint64
        Mode          uint16
-       Nlink         uint16
+       Pad_cgo_0     [6]byte
+       Nlink         uint64
        Uid           uint32
        Gid           uint32
-       Rdev          uint32
+       Rdev          uint64
        Atimespec     Timespec
        Mtimespec     Timespec
        Ctimespec     Timespec

type Statfs_t struct {
        Fsid        Fsid
        Charspare   [80]int8
        Fstypename  [16]int8
-       Mntfromname [88]int8
-       Mntonname   [88]int8
+       Mntfromname [1024]int8
+       Mntonname   [1024]int8
 }
 
 type Dirent struct {
-       Fileno uint32
+       Fileno uint64
+       Off    int64
        Reclen uint16
        Type   uint8
-       Namlen uint8
+       Pad0   uint8
+       Namlen uint16
+       Pad1   uint16
        Name   [256]int8
 }

We need to find out some good way to fill the ABI gap between FreeBSD 11 or below and FreeBSD 12 or above. Plus a few test cases. The existing test cases don't detect the ABI breakage and APIs in os, syscall and x/sys/unix packages return corrupted information to API users.

@mikioh mikioh added this to the Unplanned milestone Oct 26, 2017
@mikioh mikioh changed the title os, syscall, x/sys/unix: add support for FreeBSD12 os, syscall, x/sys/unix: add support for FreeBSD 12 Oct 26, 2017
@gopherbot
Copy link

Change https://golang.org/cl/136816 mentions this issue: unix: FreeBSD 12 ino64 support

@paulzhol
Copy link
Member

I've sent https://golang.org/cl/136816 to enable 64-bit inodes in x/sys/unix.
Should the syscall package be updated as well (it is frozen)? Stat_t is used in pkg archive/tar, os and internal/poll.

@ianlancetaylor
Copy link
Contributor

It sounds like we have to modify the syscall package for correctness; that is, if we don't modify the syscall package, programs will stop working correctly on unreasonably large file systems. Those kinds of changes to the syscall package are OK.

@paulzhol
Copy link
Member

@ianlancetaylor I don't think programs will stop working, I mean we can keep using the COMPAT11 ABI in std, at least while we also support FreeBSD 10 and 11. Please see my #22447 (comment).

@ianlancetaylor
Copy link
Contributor

OK, thanks. In that case, what is the reason for changing?

@emaste
Copy link

emaste commented Sep 24, 2018

Avoiding ino64 will result in failures on file systems with an unreasonably large number of inodes (and some other limitations that were addressed w/ ino64, like Mnt*name), but as long as those limits are not being encountered everything will remain functional.

FreeBSD 10's EOL is not far off but FreeBSD 11 has years left. In a couple of years FreeBSD 12 will be the earliest supported release, and all supported releases will have ino64.

@ianlancetaylor
Copy link
Contributor

Thanks. If programs will stop working in some cases, however obscure, then we should change the syscall package. But it doesn't sound like a high priority.

@paulzhol
Copy link
Member

paulzhol commented Sep 24, 2018

Can I take this as a go ahead to send a syscall CL just for the ino64 change, but keeping Kevent/keventt FREEBSD_COMPAT11 (I plan to have type Kevent C.struct_freebsd11_stat_t in the cgo -godefs comments)?

@ianlancetaylor
Copy link
Contributor

@paulzhol Sure, I guess so. I don't know the details of what should change, I'm just saying that change seems appropriate here.

@gopherbot
Copy link

Change https://golang.org/cl/138595 mentions this issue: syscall: FreeBSD 12 ino64 support

gopherbot pushed a commit that referenced this issue Oct 5, 2018
This is similar to CL 136816 for x/sys/unix, changing the FreeBSD ABI to use 64-bit inodes in
Stat_t, Statfs_t, and Dirent types.

The changes are forward compatible, that is FreeBSD 10.x, 11.x continue to use their current sysnum numbers.
The affected types are converted to the new layout (with some overhead).
Thus the same statically linked binary should work using the native sysnums (without any conversion) on FreeBSD 12.

Breaking API changes in package syscall are:
Mknod takes a uint64 (C dev_t) instead of int.
Stat_t: Dev, Ino, Nlink, Rdev, Gen became uint64.
  Atimespec, Mtimespec, Ctimespec, Birthtimespec renamed to Atim, Mtim, Ctim, Birthtim respectively.

Statfs_t: Mntonname and Mntfromname changed from [88]int8 to [1024]int8 arrays.

Dirent: Fileno became uint64, Namlen uint16 and an additional field Off int64 (currently unused) was added.

The following commands were run to generate ztypes_* and zsyscall_* on FreeBSD-12.0-ALPHA6 systems (GOARCH=386 were run on the same amd64 host):
GOOS=freebsd GOARCH=amd64 ./mksyscall.pl -tags freebsd,amd64 syscall_bsd.go syscall_freebsd.go syscall_freebsd_amd64.go |gofmt >zsyscall_freebsd_amd64.go
GOOS=freebsd GOARCH=amd64 go tool cgo -godefs types_freebsd.go | GOOS=freebsd GOARCH=amd64 go run mkpost.go >ztypes_freebsd_amd64.go

GOOS=freebsd GOARCH=386 ./mksyscall.pl -l32 -tags freebsd,386 syscall_bsd.go syscall_freebsd.go syscall_freebsd_386.go |gofmt >zsyscall_freebsd_386.go
GOOS=freebsd GOARCH=386 go tool cgo -godefs types_freebsd.go | GOOS=freebsd GOARCH=386 go run mkpost.go >ztypes_freebsd_386.go

GOOS=freebsd GOARCH=arm ./mksyscall.pl -l32 -arm -tags freebsd,arm syscall_bsd.go syscall_freebsd.go syscall_freebsd_arm.go |gofmt >zsyscall_freebsd_arm.go
GOOS=freebsd GOARCH=arm go tool cgo -godefs -- -fsigned-char types_freebsd.go | GOOS=freebsd GOARCH=arm go run mkpost.go >ztypes_freebsd_arm.go

The Kevent struct was changed to use the FREEBSD_COMPAT11 version always (requiring the COMPAT_FREEBSD11 kernel option FreeBSD-12, this is the default).

The definitions of ifData were not updated, their functionality in has have been replaced by vendored golang.org/x/net/route.

freebsdVersion initialization was dropped from init() in favor of a sync.Once based wrapper - supportsABI().

Updates #22448.

Change-Id: I359b756e2849c036d7ed7f75dbd6ec836e0b90b4
Reviewed-on: https://go-review.googlesource.com/c/138595
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
gopherbot pushed a commit to golang/sys that referenced this issue Oct 22, 2018
Background:
The 64-bit inode project was merged into the upcoming FreeBSD 12 release.
It changes the ABI for structs holding inodes: stat, statfs, dirent.
New system call numbers were introduced which accept the new struct layouts,
the old ones were marked as COMPAT11.
Their equivalent libc wrappers are using ELF symbol versioning.
The new wrappers have moved from @FBSD_1.0 to @FBSD_1.5.

Backward and forward compatability is achieved by always using the new struct
layouts while converting the old struct instance to the new layout on old kernels.
https://svnweb.freebsd.org/base?view=revision&revision=318736
https://svnweb.freebsd.org/base?view=revision&revision=320278

The same approach is used for Go:
The new Stat_t, Statfs_t and Dirent types hold 64-bit inodes and additional ABI
changes, they are generated from their C definitions in FreeBSD-12 using cgo -godefs.
Each type has an unexported *_freebsd11 counterpart generated the same way.
Previous directly exposed syscalls like Fstat have now a wrapper in place calling
either fstat or fstat_freebsd12 zsyscall wrapper based on the kern.osreldate.
If an old syscall needs to be used, then the returned *_freebsd11 result is converted
to the new layout before returning from the wrapper.

Introduce supportsABI() call to check the kern.osreldate sysctl for the ABI version.
Drop the old struct stat8 definition in favour of the <sys/stat.h> version.
Run the mktypes part of GOOS=freebsd GOARCH={386,amd64,arm} ./mkall.sh
on FreeBSD-12.0-ALPHA6 (r338675), updating all types except Kevent.

Expose Mknodat, both COMPAT11 version (currently missing) and the FreeBSD 12 one.
Some COMPAT11 syscalls have no direct FreeBSD 12 counterpart, in those cases
an *at(AT_FDCWD, ...) is used instead.

Updates golang/go#22448

Change-Id: I87940b88ae358db88103cdcd06f9cafbf4694cfc
Reviewed-on: https://go-review.googlesource.com/c/136816
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
@SylvainGarrigues
Copy link

Is this bug closed with the commit above?

@agnivade
Copy link
Contributor

agnivade commented Nov 4, 2018

The bug says "Updates #22448", so it's not done yet.

@paulzhol
Copy link
Member

paulzhol commented Nov 4, 2018

@SylvainGarrigues I did the ino64 change. You now have 64-bit inode support on FreeBSD 12 while on older releases the Stat/Statfs/Dirent return 64-bit inodes but only for compatibility (they use the old non 64-bit aware syscalls and just extend the result to the new format).

There's a new kevent syscall supporting absolute timers (https://svnweb.freebsd.org/base?view=revision&revision=320043) which changes the struct layout, but I don't think there's a requirement to use it on FreeBSD 12 as long as the old COMPAT11 kevent works fine.

I'm not aware of additional incompatible changes, but this issue also calls for additional tests to detect breakage, so I guess we keep it open for a while.

@mikioh
Copy link
Contributor Author

mikioh commented Nov 7, 2018

I'm not aware of additional incompatible changes

If anything, please file a new issue and make a linkage to #22447. I will try to skim the network subsystem code on 12-STABLE during Go 1.12 code freeze, well, by the end of this year.

@gopherbot
Copy link

Change https://golang.org/cl/155958 mentions this issue: syscall: revert to pre-FreeBSD 10 / POSIX-2008 timespec field names in Stat_t on FreeBSD

gopherbot pushed a commit that referenced this issue Dec 30, 2018
…n Stat_t on FreeBSD

CL 138595 introduced the new names when the hardcoded stat8 definitions was replaced
with a cgo generated one.

Fixes #29393
Updates #22448

Change-Id: I6309958306329ff301c17344b2e0ead0cc874224
Reviewed-on: https://go-review.googlesource.com/c/155958
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/160778 mentions this issue: doc: go1.12: document FreeBSD 12.0 requires COMPAT_FREEBSD11

nebulabox pushed a commit to nebulabox/go that referenced this issue Feb 18, 2019
Fixes golang#22447
Fixes golang#22448

Change-Id: Ia24f42c31e014c79040ff927f1247dfb2318de4f
Reviewed-on: https://go-review.googlesource.com/c/160778
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
nebulabox pushed a commit to nebulabox/go that referenced this issue Feb 20, 2019
Fixes golang#22447
Fixes golang#22448

Change-Id: Ia24f42c31e014c79040ff927f1247dfb2318de4f
Reviewed-on: https://go-review.googlesource.com/c/160778
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Tobias Klauser <tobias.klauser@gmail.com>
@golang golang locked and limited conversation to collaborators Feb 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants