tuntap: Add multiqueue support#293
Conversation
link_linux.go
Outdated
| _, _, _ = unix.Syscall(unix.SYS_IOCTL, fds[0].Fd(), uintptr(unix.TUNSETPERSIST), 0) | ||
| cleanupFds(fds) | ||
| } | ||
| return err |
There was a problem hiding this comment.
If we can successfully set the master interface, then we return here, without storing the list of opened fds into tuntap.Fds. Is it intentional ?
There was a problem hiding this comment.
@aboch that is a bug. I will fix this and update the PR.
| if err != nil { | ||
| _, _, _ = unix.Syscall(unix.SYS_IOCTL, fds[0].Fd(), uintptr(unix.TUNSETPERSIST), 0) | ||
| cleanupFds(fds) | ||
| } |
There was a problem hiding this comment.
Now we will not report this error to the caller. Is this the expected behavior ?
There was a problem hiding this comment.
@aboch that should also be fixed now... sorry about that.
Add multi queue support to tuntap without breaking legacy users of tuntap. Signed-off-by: Manohar Castelino <[email protected]>
| return fmt.Errorf("Tuntap IOCTL TUNSETIFF failed, errno %v", errno) | ||
|
|
||
| for i := 0; i < queues; i++ { | ||
| localReq := req |
There was a problem hiding this comment.
Do we really need this extra variable localReq ?
There was a problem hiding this comment.
We are sending it down as an unsafe pointer later. So it was just a way to ensure that we started with a clean slate each time around
unix.Syscall(unix.SYS_IOCTL, file.Fd(), uintptr(unix.TUNSETIFF), uintptr(unsafe.Pointer(&localReq)))
There was a problem hiding this comment.
I thought kernel would just read from that structure, but I see it does actually write to it in case of error
copy_to_user(argp, &ifr, ifreq_len)) in __tun_chr_ioctl().
Given ifReq member types, localReq := req will indeed create a deep copy of req.
|
LGTM |
2 participants
Add multi queue support to tuntap without breaking legacy users
of tuntap.
Signed-off-by: Manohar Castelino [email protected]