From 933d6935ac9e2cce1a9d7f8fc48d159d099a0d13 Mon Sep 17 00:00:00 2001 From: jyyi1 Date: Mon, 9 Dec 2024 20:10:57 -0500 Subject: [PATCH] code review comment RD3 --- client/go/outline/client.go | 5 +--- .../{device_linux.go => client_linux.go} | 30 ++++++++++--------- client/go/outline/device.go | 10 ++----- client/go/outline/device_others.go | 23 -------------- client/go/outline/vpn/vpn_linux.go | 9 ++---- 5 files changed, 23 insertions(+), 54 deletions(-) rename client/go/outline/{device_linux.go => client_linux.go} (65%) delete mode 100644 client/go/outline/device_others.go diff --git a/client/go/outline/client.go b/client/go/outline/client.go index 4d00852313..cc39396bb0 100644 --- a/client/go/outline/client.go +++ b/client/go/outline/client.go @@ -42,16 +42,13 @@ type NewClientResult struct { // NewClient creates a new Outline client from a configuration string. func NewClient(transportConfig string) *NewClientResult { - client, err := newClientWithBaseDialers(transportConfig, defaultBaseTCPDialer(), defaultBaseUDPDialer()) + client, err := newClientWithBaseDialers(transportConfig, net.Dialer{KeepAlive: -1}, net.Dialer{}) return &NewClientResult{ Client: client, Error: platerrors.ToPlatformError(err), } } -func defaultBaseTCPDialer() net.Dialer { return net.Dialer{KeepAlive: -1} } -func defaultBaseUDPDialer() net.Dialer { return net.Dialer{} } - func newClientWithBaseDialers(transportConfig string, tcpDialer, udpDialer net.Dialer) (*Client, error) { conf, err := parseConfigFromJSON(transportConfig) if err != nil { diff --git a/client/go/outline/device_linux.go b/client/go/outline/client_linux.go similarity index 65% rename from client/go/outline/device_linux.go rename to client/go/outline/client_linux.go index a9d9fac7a6..007bebcf13 100644 --- a/client/go/outline/device_linux.go +++ b/client/go/outline/client_linux.go @@ -1,4 +1,4 @@ -// Copyright 2024 The Outline Authors +// Copyright 2023 The Outline Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -14,25 +14,27 @@ package outline -import "syscall" - -func createWithOpts(transport string, opts *DeviceOptions) (_ *Device, err error) { - d := &Device{} +import ( + "net" + "syscall" +) +// NewClient creates a new Outline client from a configuration string. +func NewClientWithFWMark(transportConfig string, fwmark uint32) (*Client, error) { control := func(network, address string, c syscall.RawConn) error { return c.Control(func(fd uintptr) { - syscall.SetsockoptInt(int(fd), syscall.SOL_SOCKET, syscall.SO_MARK, int(opts.LinuxOpts.FWMark)) + syscall.SetsockoptInt(int(fd), syscall.SOL_SOCKET, syscall.SO_MARK, int(fwmark)) }) } - tcp := defaultBaseTCPDialer() - tcp.Control = control - - udp := defaultBaseUDPDialer() - udp.Control = control + tcp := net.Dialer{ + Control: control, + KeepAlive: -1, + } - if d.c, err = newClientWithBaseDialers(transport, tcp, udp); err != nil { - return nil, err + udp := net.Dialer{ + Control: control, } - return d, nil + + return newClientWithBaseDialers(transportConfig, tcp, udp) } diff --git a/client/go/outline/device.go b/client/go/outline/device.go index ef844a3129..29e03d16d4 100644 --- a/client/go/outline/device.go +++ b/client/go/outline/device.go @@ -41,14 +41,10 @@ type DeviceOptions struct { LinuxOpts *LinuxOptions } -func NewDevice(transportConfig string, opts *DeviceOptions) (*Device, error) { - if opts == nil || opts.LinuxOpts == nil { - return nil, perrs.PlatformError{ - Code: perrs.InternalError, - Message: "must provide at least one platform specific Option", - } +func NewDevice(c *Client) *Device { + return &Device{ + c: c, } - return createWithOpts(transportConfig, opts) } func (d *Device) Connect() (err error) { diff --git a/client/go/outline/device_others.go b/client/go/outline/device_others.go deleted file mode 100644 index 0b84ceb7e4..0000000000 --- a/client/go/outline/device_others.go +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright 2024 The Outline Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -//go:build !linux - -package outline - -import "errors" - -func createWithOpts(transport string, opts *DeviceOptions) (_ *Device, err error) { - return nil, errors.ErrUnsupported -} diff --git a/client/go/outline/vpn/vpn_linux.go b/client/go/outline/vpn/vpn_linux.go index 57a55d7766..f67d00e4e1 100644 --- a/client/go/outline/vpn/vpn_linux.go +++ b/client/go/outline/vpn/vpn_linux.go @@ -83,14 +83,11 @@ func newVPNConnection(conf *configJSON) (_ *linuxVPNConn, err error) { return nil, errIllegalConfig("must provide a transport config") } - c.proxy, err = outline.NewDevice(conf.TransportConfig, &outline.DeviceOptions{ - LinuxOpts: &outline.LinuxOptions{ - FWMark: c.nmOpts.FWMark, - }, - }) + oc, err := outline.NewClientWithFWMark(conf.TransportConfig, c.nmOpts.FWMark) if err != nil { - return + return nil, err } + c.proxy = outline.NewDevice(oc) c.ctx, c.cancel = context.WithCancel(context.Background()) return c, nil