Skip to content

Commit

Permalink
Always use config pointer for New and Reload
Browse files Browse the repository at this point in the history
Always use pointer for the Config object for New and Reload methods.
This reduces the memory, but needs a bit of care to not modifying the
passed configuration within daemon. To avoid that, implement DeepCopy
and always copy configuration at the beginning of the New and Reload.

Signed-off-by: Yutaro Hayakawa <[email protected]>
  • Loading branch information
YutaroHayakawa committed May 2, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent 7e01202 commit 79a8feb
Showing 10 changed files with 100 additions and 37 deletions.
10 changes: 10 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
MAKEFLAGS += --silent

all:

deepcopy:
deep-copy -pointer-receiver --type Config --type InterfaceConfig -o zz_generated_deepcopy.go .

check-deepcopy:
deep-copy -pointer-receiver --type Config --type InterfaceConfig -o zz_generated_deepcopy.go .
git diff --exit-code zz_generated_deepcopy.go || echo "deepcopy is not up to date. Please commit the changes."; git diff zz_generated_deepcopy.go
2 changes: 1 addition & 1 deletion config.go
Original file line number Diff line number Diff line change
@@ -39,7 +39,7 @@ func (e *ParameterError) Error() string {
// Config represents the configuration of the daemon
type Config struct {
// Interface-specific configuration parameters
Interfaces []InterfaceConfig `yaml:"interfaces"`
Interfaces []*InterfaceConfig `yaml:"interfaces"`
}

func (c *Config) validate() error {
8 changes: 4 additions & 4 deletions config_test.go
Original file line number Diff line number Diff line change
@@ -42,7 +42,7 @@ func TestConfigValidation(t *testing.T) {
{
name: "Valid Config",
config: &Config{
Interfaces: []InterfaceConfig{
Interfaces: []*InterfaceConfig{
{
Name: "net0",
RAIntervalMilliseconds: 1000,
@@ -58,7 +58,7 @@ func TestConfigValidation(t *testing.T) {
{
name: "Duplicated Interface Name",
config: &Config{
Interfaces: []InterfaceConfig{
Interfaces: []*InterfaceConfig{
{
Name: "net0",
RAIntervalMilliseconds: 1000,
@@ -74,7 +74,7 @@ func TestConfigValidation(t *testing.T) {
{
name: "RAIntervalMilliseconds < 70",
config: &Config{
Interfaces: []InterfaceConfig{
Interfaces: []*InterfaceConfig{
{
Name: "net0",
RAIntervalMilliseconds: 69,
@@ -86,7 +86,7 @@ func TestConfigValidation(t *testing.T) {
{
name: "RAIntervalMilliseconds > 1800000",
config: &Config{
Interfaces: []InterfaceConfig{
Interfaces: []*InterfaceConfig{
{
Name: "net0",
RAIntervalMilliseconds: 1800001,
33 changes: 23 additions & 10 deletions daemon.go
Original file line number Diff line number Diff line change
@@ -8,23 +8,27 @@ import (

// Daemon is the main struct for the radv daemon
type Daemon struct {
initialConfig Config
reloadCh chan Config
initialConfig *Config
reloadCh chan *Config
stopCh any
logger *slog.Logger
socketConstructor rAdvSocketCtor
}

// New creates a new Daemon instance with the provided configuration and options
func New(c Config, opts ...DaemonOption) (*Daemon, error) {
func New(config *Config, opts ...DaemonOption) (*Daemon, error) {
// Take a copy of the new configuration. c.validate() will modify it to
// set default values.
c := config.DeepCopy()

// Validate the configuration first
if err := c.validate(); err != nil {
return nil, err
}

d := &Daemon{
initialConfig: c,
reloadCh: make(chan Config),
reloadCh: make(chan *Config),
logger: slog.Default(),
socketConstructor: newRAdvSocket,
}
@@ -50,13 +54,13 @@ func (d *Daemon) Run(ctx context.Context) error {
// Main loop
for {
var (
toAdd []InterfaceConfig
toAdd []*InterfaceConfig
toUpdate []*raSender
toRemove []*raSender
)

// Cache the interface => config mapping for later use
ifaceConfigs := map[string]InterfaceConfig{}
ifaceConfigs := map[string]*InterfaceConfig{}

// Find out which raSender to add, update and remove
for _, c := range config.Interfaces {
@@ -86,9 +90,9 @@ func (d *Daemon) Run(ctx context.Context) error {
iface := raSender.initialConfig.Name
d.logger.Info("Updating RA sender", slog.String("interface", iface))
// Set timeout to guarantee progress
timeout, cancel := context.WithTimeout(ctx, time.Second*3)
timeout, cancelTimeout := context.WithTimeout(ctx, time.Second*3)
raSender.reload(timeout, ifaceConfigs[iface])
cancel()
cancelTimeout()
}

// Remove unnecessary workers
@@ -117,12 +121,21 @@ func (d *Daemon) Run(ctx context.Context) error {
// the reload process. Currently, the result of the unsucecssful or cancelled
// reload is undefined and the daemon may be running with either the old or the
// new configuration or both.
func (d *Daemon) Reload(ctx context.Context, newConfig Config) error {
func (d *Daemon) Reload(ctx context.Context, newConfig *Config) error {
// Take a copy of the new configuration. c.validate() will modify it to
// set default values.
c := newConfig.DeepCopy()

if err := c.validate(); err != nil {
return err
}

select {
case d.reloadCh <- newConfig:
case d.reloadCh <- c:
case <-ctx.Done():
return ctx.Err()
}

return nil
}

12 changes: 6 additions & 6 deletions daemon_test.go
Original file line number Diff line number Diff line change
@@ -43,8 +43,8 @@ outer:
}

func TestDaemonHappyPath(t *testing.T) {
config := Config{
Interfaces: []InterfaceConfig{
config := &Config{
Interfaces: []*InterfaceConfig{
{
Name: "net0",
RAIntervalMilliseconds: 100,
@@ -92,10 +92,10 @@ func TestDaemonHappyPath(t *testing.T) {
config.Interfaces[1].RAIntervalMilliseconds = 200

// Reload
timeout, cancel := context.WithTimeout(context.Background(), time.Second*1)
timeout, cancelTimeout := context.WithTimeout(context.Background(), time.Second*1)
err := d.Reload(timeout, config)
require.NoError(t, err)
cancel()
cancelTimeout()

eventully(t, func() bool {
sock0, err := reg.getSock("net0")
@@ -116,10 +116,10 @@ func TestDaemonHappyPath(t *testing.T) {
config.Interfaces = config.Interfaces[:1]

// Reload
timeout, cancel := context.WithTimeout(context.Background(), time.Second*1)
timeout, cancelTimeout := context.WithTimeout(context.Background(), time.Second*1)
err := d.Reload(timeout, config)
require.NoError(t, err)
cancel()
cancelTimeout()

eventully(t, func() bool {
sock0, err := reg.getSock("net0")
3 changes: 3 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
@@ -15,6 +15,9 @@ require (

require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/kr/pretty v0.3.1 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/rogpeppe/go-internal v1.11.0 // indirect
golang.org/x/text v0.14.0 // indirect
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
)
15 changes: 14 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
@@ -1,11 +1,23 @@
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/mdlayher/ndp v1.1.0 h1:QylGKGVtH60sKZUE88+IW5ila1Z/M9/OXhWdsVKuscs=
github.com/mdlayher/ndp v1.1.0/go.mod h1:FmgESgemgjl38vuOIyAHWUUL6vQKA/pQNkvXdWsdQFM=
github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs=
github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M=
github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA=
github.com/sethvargo/go-retry v0.2.4 h1:T+jHEQy/zKJf5s95UkguisicE0zuF9y7+/vgz08Ocec=
github.com/sethvargo/go-retry v0.2.4/go.mod h1:1afjQuvh7s4gflMObvjLPaWgluLLyhA1wmVZ6KLpICw=
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
@@ -16,7 +28,8 @@ golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o=
golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ=
golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
4 changes: 2 additions & 2 deletions goradvd/main.go
Original file line number Diff line number Diff line change
@@ -29,7 +29,7 @@ func main() {

slog.Info("Starting radv daemon", slog.Any("config", *config))

daemon, err := radv.New(*config)
daemon, err := radv.New(config)
if err != nil {
slog.Error("Failed to create daemon. Aborting.", "error", err.Error())
return
@@ -52,7 +52,7 @@ func main() {
slog.Error("Failed to parse config file. Skip reloading.", "error", err.Error())
continue
}
if err := daemon.Reload(ctx, *config); err != nil {
if err := daemon.Reload(ctx, config); err != nil {
slog.Error("Failed to reload configuration", "error", err.Error())
continue
}
26 changes: 13 additions & 13 deletions ra_sender.go
Original file line number Diff line number Diff line change
@@ -6,6 +6,7 @@ import (
"fmt"
"log/slog"
"net/netip"
"reflect"
"sync"
"time"

@@ -17,26 +18,26 @@ import (
type raSender struct {
logger *slog.Logger

initialConfig InterfaceConfig
reloadCh chan InterfaceConfig
initialConfig *InterfaceConfig
reloadCh chan *InterfaceConfig
stopCh any
sock rAdvSocket

childWg *sync.WaitGroup
childReloadCh []chan InterfaceConfig
childReloadCh []chan *InterfaceConfig
childStopCh []chan any

socketCtor rAdvSocketCtor
}

func newRASender(initialConfig InterfaceConfig, ctor rAdvSocketCtor, logger *slog.Logger) *raSender {
func newRASender(initialConfig *InterfaceConfig, ctor rAdvSocketCtor, logger *slog.Logger) *raSender {
return &raSender{
logger: logger.With(slog.String("interface", initialConfig.Name)),
initialConfig: initialConfig,
reloadCh: make(chan InterfaceConfig),
stopCh: make(chan InterfaceConfig),
reloadCh: make(chan *InterfaceConfig),
stopCh: make(chan any),
childWg: &sync.WaitGroup{},
childReloadCh: []chan InterfaceConfig{},
childReloadCh: []chan *InterfaceConfig{},
childStopCh: []chan any{},
socketCtor: ctor,
}
@@ -68,7 +69,7 @@ func (s *raSender) run(ctx context.Context) {
s.sock.close()
}

func (s *raSender) reload(ctx context.Context, newConfig InterfaceConfig) error {
func (s *raSender) reload(ctx context.Context, newConfig *InterfaceConfig) error {
for _, ch := range s.childReloadCh {
select {
case ch <- newConfig:
@@ -85,16 +86,16 @@ func (s *raSender) stop() {
}
}

func (s *raSender) spawnChild(ctx context.Context, f func(context.Context, chan InterfaceConfig, chan any)) {
func (s *raSender) spawnChild(ctx context.Context, f func(context.Context, chan *InterfaceConfig, chan any)) {
s.childWg.Add(1)
reloadCh := make(chan InterfaceConfig)
reloadCh := make(chan *InterfaceConfig)
stopCh := make(chan any)
s.childReloadCh = append(s.childReloadCh, reloadCh)
s.childStopCh = append(s.childStopCh, stopCh)
go f(ctx, reloadCh, stopCh)
}

func (s *raSender) runUnsolicitedRASender(ctx context.Context, reloadCh chan InterfaceConfig, stopCh chan any) {
func (s *raSender) runUnsolicitedRASender(ctx context.Context, reloadCh chan *InterfaceConfig, stopCh chan any) {
defer s.childWg.Done()

// The current desired configuration
@@ -116,8 +117,7 @@ reload:
continue
}
case newConfig := <-reloadCh:
if config == newConfig {
// No change. Ignore.
if reflect.DeepEqual(config, newConfig) {
s.logger.Info("No configuration change. Skip reloading.")
continue
}
24 changes: 24 additions & 0 deletions zz_generated_deepcopy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// generated by deep-copy -pointer-receiver --type Config --type InterfaceConfig -o zz_generated_deepcopy.go .; DO NOT EDIT.

package radv

// DeepCopy generates a deep copy of *Config
func (o *Config) DeepCopy() *Config {
var cp Config = *o
if o.Interfaces != nil {
cp.Interfaces = make([]*InterfaceConfig, len(o.Interfaces))
copy(cp.Interfaces, o.Interfaces)
for i2 := range o.Interfaces {
if o.Interfaces[i2] != nil {
cp.Interfaces[i2] = o.Interfaces[i2].DeepCopy()
}
}
}
return &cp
}

// DeepCopy generates a deep copy of *InterfaceConfig
func (o *InterfaceConfig) DeepCopy() *InterfaceConfig {
var cp InterfaceConfig = *o
return &cp
}

0 comments on commit 79a8feb

Please sign in to comment.