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

IPVS: Add support for GetConfig/SetConfig #2349

Merged
merged 3 commits into from
Mar 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions ipvs/ipvs.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ type Destination struct {
// DstStats defines IPVS destination (real server) statistics
type DstStats SvcStats

// Config defines IPVS timeout configuration
type Config struct {
TimeoutTCP time.Duration
TimeoutTCPFin time.Duration
TimeoutUDP time.Duration
}

// Handle provides a namespace specific ipvs handle to program ipvs
// rules.
type Handle struct {
Expand Down Expand Up @@ -188,3 +195,13 @@ func (i *Handle) GetService(s *Service) (*Service, error) {

return res[0], nil
}

// GetConfig returns the current timeout configuration
func (i *Handle) GetConfig() (*Config, error) {
return i.doGetConfigCmd()
}

// SetConfig set the current timeout configuration. 0: no change
func (i *Handle) SetConfig(c *Config) error {
return i.doSetConfigCmd(c)
}
31 changes: 31 additions & 0 deletions ipvs/ipvs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net"
"syscall"
"testing"
"time"

"github.com/docker/libnetwork/testutils"
"github.com/vishvananda/netlink"
Expand Down Expand Up @@ -342,3 +343,33 @@ func TestDestination(t *testing.T) {
}
}
}

func TestTimeouts(t *testing.T) {
if testutils.RunningOnCircleCI() {
t.Skip("Skipping as not supported on CIRCLE CI kernel")
}
defer testutils.SetupTestOSContext(t)()

i, err := New("")
assert.NilError(t, err)

_, err = i.GetConfig()
assert.NilError(t, err)

cfg := Config{66 * time.Second, 66 * time.Second, 66 * time.Second}
err = i.SetConfig(&cfg)
assert.NilError(t, err)

c2, err := i.GetConfig()
assert.NilError(t, err)
assert.DeepEqual(t, cfg, *c2)

// A timeout value 0 means that the current timeout value of the corresponding entry is preserved
cfg = Config{77 * time.Second, 0 * time.Second, 77 * time.Second}
err = i.SetConfig(&cfg)
assert.NilError(t, err)

c3, err := i.GetConfig()
assert.NilError(t, err)
assert.DeepEqual(t, *c3, Config{77 * time.Second, 66 * time.Second, 77 * time.Second})
}
55 changes: 55 additions & 0 deletions ipvs/netlink.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"sync"
"sync/atomic"
"syscall"
"time"
"unsafe"

"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -503,6 +504,60 @@ func (i *Handle) doGetDestinationsCmd(s *Service, d *Destination) ([]*Destinatio
return res, nil
}

// parseConfig given a ipvs netlink response this function will respond with a valid config entry, an error otherwise
func (i *Handle) parseConfig(msg []byte) (*Config, error) {
var c Config

//Remove General header for this message
hdr := deserializeGenlMsg(msg)
attrs, err := nl.ParseRouteAttr(msg[hdr.Len():])
if err != nil {
return nil, err
}

for _, attr := range attrs {
attrType := int(attr.Attr.Type)
switch attrType {
case ipvsCmdAttrTimeoutTCP:
c.TimeoutTCP = time.Duration(native.Uint32(attr.Value)) * time.Second
case ipvsCmdAttrTimeoutTCPFin:
c.TimeoutTCPFin = time.Duration(native.Uint32(attr.Value)) * time.Second
case ipvsCmdAttrTimeoutUDP:
c.TimeoutUDP = time.Duration(native.Uint32(attr.Value)) * time.Second
}
}

return &c, nil
}

// doGetConfigCmd a wrapper function to be used by GetConfig
func (i *Handle) doGetConfigCmd() (*Config, error) {
msg, err := i.doCmdWithoutAttr(ipvsCmdGetConfig)
if err != nil {
return nil, err
}

res, err := i.parseConfig(msg[0])
if err != nil {
return res, err
}
return res, nil
}

// doSetConfigCmd a wrapper function to be used by SetConfig
func (i *Handle) doSetConfigCmd(c *Config) error {
req := newIPVSRequest(ipvsCmdSetConfig)
req.Seq = atomic.AddUint32(&i.seq, 1)

req.AddData(nl.NewRtAttr(ipvsCmdAttrTimeoutTCP, nl.Uint32Attr(uint32(c.TimeoutTCP.Seconds()))))

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't it better to make them conditional? what is the config does not specify all the 3 values? is the 0 value ignored by the kernel?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, 0 means "unchanged"
Extract from the ipvsdam man page:

--set tcp tcpfin udp
Change the timeout values used for IPVS connections. This command always takes 3 parameters, representing the timeout values (in seconds) for TCP sessions, TCP sessions after receiving a FIN packet, and UDP packets, respectively. A timeout value 0 means that the current timeout value of the corresponding entry is preserved.

But I can definitely add a test for this

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would be great and maybe a comment saying that 0 means unchanged just for posterity, rest LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm doing it right now

req.AddData(nl.NewRtAttr(ipvsCmdAttrTimeoutTCPFin, nl.Uint32Attr(uint32(c.TimeoutTCPFin.Seconds()))))
req.AddData(nl.NewRtAttr(ipvsCmdAttrTimeoutUDP, nl.Uint32Attr(uint32(c.TimeoutUDP.Seconds()))))

_, err := execute(i.sock, req, 0)

return err
}

// IPVS related netlink message format explained

/* EACH NETLINK MSG is of the below format, this is what we will receive from execute() api.
Expand Down