Merge branch 'master' into 4891-more-svcs

CHANGELOG.md

@@ -22,8 +22,12 @@ and this project adheres to
 
 ### Added
 
+- New `bool`, `dur`, `u8`, and `u16` DHCP options to provide more convenience on
+  options control by setting values in a human-readable format ([#4705]).  See
+  also a [Wiki page][wiki-dhcp-opts].
+
 - New `del` DHCP option which removes the corresponding option from server's
-  response ([#4337]).
+  response ([#4337]).  See also a [Wiki page][wiki-dhcp-opts].
 
   **NOTE:** This modifier affects all the parameters in the response and not
   only the requested ones.
@@ -32,6 +36,7 @@ and this project adheres to
 
 ### Changed
 
+- The DHCP options handling is now closer to the [RFC 2131][rfc-2131] ([#4705]).
 - When the DHCP server is enabled, queries for domain names under
   `dhcp.local_domain_name` not pointing to real DHCP client hostnames are now
   processed by filters ([#4865]).
@@ -57,12 +62,14 @@ and this project adheres to
 [#4337]: https://github.com/AdguardTeam/AdGuardHome/issues/4337
 [#4403]: https://github.com/AdguardTeam/AdGuardHome/issues/4403
 [#4535]: https://github.com/AdguardTeam/AdGuardHome/issues/4535
+[#4705]: https://github.com/AdguardTeam/AdGuardHome/issues/4705
 [#4745]: https://github.com/AdguardTeam/AdGuardHome/issues/4745
 [#4850]: https://github.com/AdguardTeam/AdGuardHome/issues/4850
 [#4863]: https://github.com/AdguardTeam/AdGuardHome/issues/4863
 [#4865]: https://github.com/AdguardTeam/AdGuardHome/issues/4865
 
 [rfc-2131]:       https://datatracker.ietf.org/doc/html/rfc2131
+[wiki-dhcp-opts]: https://github.com/adguardTeam/adGuardHome/wiki/DHCP#config-4
 
 
 

internal/dhcpd/options_unix.go

@@ -9,20 +9,28 @@ import (
 	"net"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/log"
 	"github.com/AdguardTeam/golibs/netutil"
+	"github.com/AdguardTeam/golibs/timeutil"
 	"github.com/insomniacslk/dhcp/dhcpv4"
 )
 
 // The aliases for DHCP option types available for explicit declaration.
+//
+// TODO(e.burkov):  Add an option for classless routes.
 const (
+	typDel  = "del"
+	typBool = "bool"
+	typDur  = "dur"
 	typHex  = "hex"
 	typIP   = "ip"
 	typIPs  = "ips"
 	typText = "text"
-	typDel  = "del"
+	typU8   = "u8"
+	typU16  = "u16"
 )
 
 // parseDHCPOptionHex parses a DHCP option as a hex-encoded string.
@@ -40,8 +48,8 @@ func parseDHCPOptionHex(s string) (val dhcpv4.OptionValue, err error) {
 func parseDHCPOptionIP(s string) (val dhcpv4.OptionValue, err error) {
 	var ip net.IP
 	// All DHCPv4 options require IPv4, so don't put the 16-byte version.
-	// Otherwise, the clients will receive weird data that looks like four
+	// Otherwise, the clients will receive weird data that looks like four IPv4
-	// IPv4 addresses.
+	// addresses.
 	//
 	// See https://github.com/AdguardTeam/AdGuardHome/issues/2688.
 	if ip, err = netutil.ParseIPv4(s); err != nil {
@@ -55,35 +63,76 @@ func parseDHCPOptionIP(s string) (val dhcpv4.OptionValue, err error) {
 // addresses.
 func parseDHCPOptionIPs(s string) (val dhcpv4.OptionValue, err error) {
 	var ips dhcpv4.IPs
-	var ip net.IP
+	var ip dhcpv4.OptionValue
 	for i, ipStr := range strings.Split(s, ",") {
-		// See notes in the ipDHCPOptionParserHandler.
+		ip, err = parseDHCPOptionIP(ipStr)
-		if ip, err = netutil.ParseIPv4(ipStr); err != nil {
+		if err != nil {
 			return nil, fmt.Errorf("parsing ip at index %d: %w", i, err)
 		}
 
-		ips = append(ips, ip)
+		ips = append(ips, net.IP(ip.(dhcpv4.IP)))
 	}
 
 	return ips, nil
 }
 
-// parseDHCPOptionText parses a DHCP option as a simple UTF-8 encoded
+// parseDHCPOptionDur parses a DHCP option as a duration in a human-readable
-// text.
+// form.
-func parseDHCPOptionText(s string) (val dhcpv4.OptionValue) {
+func parseDHCPOptionDur(s string) (val dhcpv4.OptionValue, err error) {
-	return dhcpv4.OptionGeneric{Data: []byte(s)}
+	var v timeutil.Duration
+	err = v.UnmarshalText([]byte(s))
+	if err != nil {
+		return nil, fmt.Errorf("decoding dur: %w", err)
 	}
 
-// parseDHCPOptionVal parses a DHCP option value considering typ.  For the del
+	return dhcpv4.Duration(v.Duration), nil
-// option the value string is ignored.  The examples of possible value pairs:
+}
-//
+
-//   - hex  736f636b733a2f2f70726f78792e6578616d706c652e6f7267
+// parseDHCPOptionUint parses a DHCP option as an unsigned integer.  bitSize is
-//   - ip   192.168.1.1
+// expected to be 8 or 16.
-//   - ips  192.168.1.1,192.168.1.2
+func parseDHCPOptionUint(s string, bitSize int) (val dhcpv4.OptionValue, err error) {
-//   - text http://192.168.1.1/wpad.dat
+	var v uint64
-//   - del
+	v, err = strconv.ParseUint(s, 10, bitSize)
+	if err != nil {
+		return nil, fmt.Errorf("decoding u%d: %w", bitSize, err)
+	}
+
+	switch bitSize {
+	case 8:
+		return dhcpv4.OptionGeneric{Data: []byte{uint8(v)}}, nil
+	case 16:
+		return dhcpv4.Uint16(v), nil
+	default:
+		return nil, fmt.Errorf("unsupported size of integer %d", bitSize)
+	}
+}
+
+// parseDHCPOptionBool parses a DHCP option as a boolean value.  See
+// [strconv.ParseBool] for available values.
+func parseDHCPOptionBool(s string) (val dhcpv4.OptionValue, err error) {
+	var v bool
+	v, err = strconv.ParseBool(s)
+	if err != nil {
+		return nil, fmt.Errorf("decoding bool: %w", err)
+	}
+
+	rawVal := [1]byte{}
+	if v {
+		rawVal[0] = 1
+	}
+
+	return dhcpv4.OptionGeneric{Data: rawVal[:]}, nil
+}
+
+// parseDHCPOptionVal parses a DHCP option value considering typ.
 func parseDHCPOptionVal(typ, valStr string) (val dhcpv4.OptionValue, err error) {
 	switch typ {
+	case typBool:
+		val, err = parseDHCPOptionBool(valStr)
+	case typDel:
+		val = dhcpv4.OptionGeneric{Data: nil}
+	case typDur:
+		val, err = parseDHCPOptionDur(valStr)
 	case typHex:
 		val, err = parseDHCPOptionHex(valStr)
 	case typIP:
@@ -91,9 +140,11 @@ func parseDHCPOptionVal(typ, valStr string) (val dhcpv4.OptionValue, err error)
 	case typIPs:
 		val, err = parseDHCPOptionIPs(valStr)
 	case typText:
-		val = parseDHCPOptionText(valStr)
+		val = dhcpv4.String(valStr)
-	case typDel:
+	case typU8:
-		val = dhcpv4.OptionGeneric{Data: nil}
+		val, err = parseDHCPOptionUint(valStr, 8)
+	case typU16:
+		val, err = parseDHCPOptionUint(valStr, 16)
 	default:
 		err = fmt.Errorf("unknown option type %q", typ)
 	}
@@ -101,9 +152,19 @@ func parseDHCPOptionVal(typ, valStr string) (val dhcpv4.OptionValue, err error)
 	return val, err
 }
 
-// parseDHCPOption parses an option.  See the documentation of
+// parseDHCPOption parses an option.  For the del option value is ignored.  The
-// parseDHCPOptionVal for more info.
+// examples of possible option strings:
-func parseDHCPOption(s string) (opt dhcpv4.Option, err error) {
+//
+//   - 1  bool true
+//   - 2  del
+//   - 3  dur  2h5s
+//   - 4  hex  736f636b733a2f2f70726f78792e6578616d706c652e6f7267
+//   - 5  ip   192.168.1.1
+//   - 6  ips  192.168.1.1,192.168.1.2
+//   - 7  text http://192.168.1.1/wpad.dat
+//   - 8  u8   255
+//   - 9  u16  65535
+func parseDHCPOption(s string) (code dhcpv4.OptionCode, val dhcpv4.OptionValue, err error) {
 	defer func() { err = errors.Annotate(err, "invalid option string %q: %w", s) }()
 
 	s = strings.TrimSpace(s)
@@ -112,7 +173,7 @@ func parseDHCPOption(s string) (opt dhcpv4.Option, err error) {
 	var valStr string
 	if pl := len(parts); pl < 3 {
 		if pl < 2 || parts[1] != typDel {
-			return opt, errors.Error("bad option format")
+			return nil, nil, errors.Error("bad option format")
 		}
 	} else {
 		valStr = parts[2]
@@ -121,85 +182,226 @@ func parseDHCPOption(s string) (opt dhcpv4.Option, err error) {
 	var code64 uint64
 	code64, err = strconv.ParseUint(parts[0], 10, 8)
 	if err != nil {
-		return opt, fmt.Errorf("parsing option code: %w", err)
+		return nil, nil, fmt.Errorf("parsing option code: %w", err)
 	}
 
-	val, err := parseDHCPOptionVal(parts[1], valStr)
+	val, err = parseDHCPOptionVal(parts[1], valStr)
 	if err != nil {
 		// Don't wrap an error since it's informative enough as is and there
 		// also the deferred annotation.
-		return opt, err
+		return nil, nil, err
 	}
 
-	return dhcpv4.Option{
+	return dhcpv4.GenericOptionCode(code64), val, nil
-		Code:  dhcpv4.GenericOptionCode(code64),
-		Value: val,
-	}, nil
 }
 
 // prepareOptions builds the set of DHCP options according to host requirements
 // document and values from conf.
-func prepareOptions(conf V4ServerConf) (opts dhcpv4.Options) {
+func prepareOptions(conf V4ServerConf) (implicit, explicit dhcpv4.Options) {
-	// Set default values for host configuration parameters listed in Appendix
+	// Set default values of host configuration parameters listed in Appendix A
-	// A of RFC-2131.  Those parameters, if requested by client, should be
+	// of RFC-2131.
-	// returned with values defined by Host Requirements Document.
+	implicit = dhcpv4.OptionsFromList(
-	//
-	// See https://datatracker.ietf.org/doc/html/rfc2131#appendix-A.
-	//
-	// See also https://datatracker.ietf.org/doc/html/rfc1122,
-	// https://datatracker.ietf.org/doc/html/rfc1123, and
-	// https://datatracker.ietf.org/doc/html/rfc2132.
-	opts = dhcpv4.OptionsFromList(
 		// IP-Layer Per Host
 
-		dhcpv4.OptGeneric(dhcpv4.OptionNonLocalSourceRouting, []byte{0}),
+		// An Internet host that includes embedded gateway code MUST have a
+		// configuration switch to disable the gateway function, and this switch
+		// MUST default to the non-gateway mode.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-3.3.5.
+		dhcpv4.OptGeneric(dhcpv4.OptionIPForwarding, []byte{0x0}),
 
-		// Set the current recommended default time to live for the
+		// A host that supports non-local source-routing MUST have a
-		// Internet Protocol which is 64, see
+		// configurable switch to disable forwarding, and this switch MUST
-		// https://datatracker.ietf.org/doc/html/rfc1700.
+		// default to disabled.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-3.3.5.
+		dhcpv4.OptGeneric(dhcpv4.OptionNonLocalSourceRouting, []byte{0x0}),
+
+		// Do not set the Policy Filter Option since it only makes sense when
+		// the non-local source routing is enabled.
+
+		// The minimum legal value is 576.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc2132#section-4.4.
+		dhcpv4.Option{
+			Code:  dhcpv4.OptionMaximumDatagramAssemblySize,
+			Value: dhcpv4.Uint16(576),
+		},
+
+		// Set the current recommended default time to live for the Internet
+		// Protocol which is 64.
+		//
+		// See https://www.iana.org/assignments/ip-parameters/ip-parameters.xhtml#ip-parameters-2.
 		dhcpv4.OptGeneric(dhcpv4.OptionDefaultIPTTL, []byte{0x40}),
 
-		dhcpv4.OptGeneric(dhcpv4.OptionPerformMaskDiscovery, []byte{0}),
+		// For example, after the PTMU estimate is decreased, the timeout should
-		dhcpv4.OptGeneric(dhcpv4.OptionMaskSupplier, []byte{0}),
+		// be set to 10 minutes; once this timer expires and a larger MTU is
-		dhcpv4.OptGeneric(dhcpv4.OptionPerformRouterDiscovery, []byte{1}),
+		// attempted, the timeout can be set to a much smaller value.
-		// The all-routers address is preferred wherever possible, see
+		//
-		// https://datatracker.ietf.org/doc/html/rfc1256#section-5.1.
+		// See https://datatracker.ietf.org/doc/html/rfc1191#section-6.6.
+		dhcpv4.Option{
+			Code:  dhcpv4.OptionPathMTUAgingTimeout,
+			Value: dhcpv4.Duration(10 * time.Minute),
+		},
+
+		// There is a table describing the MTU values representing all major
+		// data-link technologies in use in the Internet so that each set of
+		// similar MTUs is associated with a plateau value equal to the lowest
+		// MTU in the group.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1191#section-7.
+		dhcpv4.OptGeneric(dhcpv4.OptionPathMTUPlateauTable, []byte{
+			0x0, 0x44,
+			0x1, 0x28,
+			0x1, 0xFC,
+			0x3, 0xEE,
+			0x5, 0xD4,
+			0x7, 0xD2,
+			0x11, 0x0,
+			0x1F, 0xE6,
+			0x45, 0xFA,
+		}),
+
+		// IP-Layer Per Interface
+
+		// Since nearly all networks in the Internet currently support an MTU of
+		// 576 or greater, we strongly recommend the use of 576 for datagrams
+		// sent to non-local networks.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-3.3.3.
+		dhcpv4.Option{
+			Code:  dhcpv4.OptionInterfaceMTU,
+			Value: dhcpv4.Uint16(576),
+		},
+
+		// Set the All Subnets Are Local Option to false since commonly the
+		// connected hosts aren't expected to be multihomed.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-3.3.3.
+		dhcpv4.OptGeneric(dhcpv4.OptionAllSubnetsAreLocal, []byte{0x00}),
+
+		// Set the Perform Mask Discovery Option to false to provide the subnet
+		// mask by options only.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-3.2.2.9.
+		dhcpv4.OptGeneric(dhcpv4.OptionPerformMaskDiscovery, []byte{0x00}),
+
+		// A system MUST NOT send an Address Mask Reply unless it is an
+		// authoritative agent for address masks.  An authoritative agent may be
+		// a host or a gateway, but it MUST be explicitly configured as a
+		// address mask agent.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-3.2.2.9.
+		dhcpv4.OptGeneric(dhcpv4.OptionMaskSupplier, []byte{0x00}),
+
+		// Set the Perform Router Discovery Option to true as per Router
+		// Discovery Document.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1256#section-5.1.
+		dhcpv4.OptGeneric(dhcpv4.OptionPerformRouterDiscovery, []byte{0x01}),
+
+		// The all-routers address is preferred wherever possible.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1256#section-5.1.
 		dhcpv4.Option{
 			Code:  dhcpv4.OptionRouterSolicitationAddress,
 			Value: dhcpv4.IP(netutil.IPv4allrouter()),
 		},
+
+		// Don't set the Static Routes Option since it should be set up by
+		// system administrator.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-3.3.1.2.
+
+		// A datagram with the destination address of limited broadcast will be
+		// received by every host on the connected physical network but will not
+		// be forwarded outside that network.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-3.2.1.3.
 		dhcpv4.OptBroadcastAddress(netutil.IPv4bcast()),
 
 		// Link-Layer Per Interface
 
-		dhcpv4.OptGeneric(dhcpv4.OptionTrailerEncapsulation, []byte{0}),
+		// If the system does not dynamically negotiate use of the trailer
-		dhcpv4.OptGeneric(dhcpv4.OptionEthernetEncapsulation, []byte{0}),
+		// protocol on a per-destination basis, the default configuration MUST
+		// disable the protocol.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-2.3.1.
+		dhcpv4.OptGeneric(dhcpv4.OptionTrailerEncapsulation, []byte{0x00}),
+
+		// For proxy ARP situations, the timeout needs to be on the order of a
+		// minute.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-2.3.2.1.
+		dhcpv4.Option{
+			Code:  dhcpv4.OptionArpCacheTimeout,
+			Value: dhcpv4.Duration(time.Minute),
+		},
+
+		// An Internet host that implements sending both the RFC-894 and the
+		// RFC-1042 encapsulations MUST provide a configuration switch to select
+		// which is sent, and this switch MUST default to RFC-894.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-2.3.3.
+		dhcpv4.OptGeneric(dhcpv4.OptionEthernetEncapsulation, []byte{0x00}),
 
 		// TCP Per Host
 
+		// A fixed value must be at least big enough for the Internet diameter,
+		// i.e., the longest possible path.  A reasonable value is about twice
+		// the diameter, to allow for continued Internet growth.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-3.2.1.7.
+		dhcpv4.Option{
+			Code:  dhcpv4.OptionDefaulTCPTTL,
+			Value: dhcpv4.Duration(60 * time.Second),
+		},
+
+		// The interval MUST be configurable and MUST default to no less than
+		// two hours.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-4.2.3.6.
 		dhcpv4.Option{
 			Code:  dhcpv4.OptionTCPKeepaliveInterval,
-			Value: dhcpv4.Duration(0),
+			Value: dhcpv4.Duration(2 * time.Hour),
 		},
-		dhcpv4.OptGeneric(dhcpv4.OptionTCPKeepaliveGarbage, []byte{0}),
+
+		// Unfortunately, some misbehaved TCP implementations fail to respond to
+		// a probe segment unless it contains data.
+		//
+		// See https://datatracker.ietf.org/doc/html/rfc1122#section-4.2.3.6.
+		dhcpv4.OptGeneric(dhcpv4.OptionTCPKeepaliveGarbage, []byte{0x01}),
 
 		// Values From Configuration
 
+		// Set the Router Option to working subnet's IP since it's initialized
+		// with the address of the gateway.
 		dhcpv4.OptRouter(conf.subnet.IP),
+
 		dhcpv4.OptSubnetMask(conf.subnet.Mask),
 	)
 
 	// Set values for explicitly configured options.
+	explicit = dhcpv4.Options{}
 	for i, o := range conf.Options {
-		opt, err := parseDHCPOption(o)
+		code, val, err := parseDHCPOption(o)
 		if err != nil {
 			log.Error("dhcpv4: bad option string at index %d: %s", i, err)
 
 			continue
 		}
 
-		opts.Update(opt)
+		explicit.Update(dhcpv4.Option{Code: code, Value: val})
+		// Remove those from the implicit options.
+		delete(implicit, code.Code())
 	}
 
-	return opts
+	log.Debug("dhcpv4: implicit options:\n%s", implicit.Summary(nil))
+	log.Debug("dhcpv4: explicit options:\n%s", explicit.Summary(nil))
+
+	if len(explicit) == 0 {
+		explicit = nil
+	}
+
+	return implicit, explicit
 }

internal/dhcpd/options_unix_test.go

@@ -7,7 +7,9 @@ import (
 	"fmt"
 	"net"
 	"testing"
+	"time"
 
+	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/insomniacslk/dhcp/dhcpv4"
 	"github.com/stretchr/testify/assert"
@@ -17,150 +19,199 @@ func TestParseOpt(t *testing.T) {
 	testCases := []struct {
 		name       string
 		in         string
-		wantOpt    dhcpv4.Option
+		wantCode   dhcpv4.OptionCode
+		wantVal    dhcpv4.OptionValue
 		wantErrMsg string
 	}{{
 		name:     "hex_success",
 		in:       "6 hex c0a80101c0a80102",
-		wantOpt: dhcpv4.OptGeneric(
+		wantCode: dhcpv4.GenericOptionCode(6),
-			dhcpv4.GenericOptionCode(6),
+		wantVal: dhcpv4.OptionGeneric{Data: []byte{
-			[]byte{
 			0xC0, 0xA8, 0x01, 0x01,
 			0xC0, 0xA8, 0x01, 0x02,
-			},
+		}},
-		),
 		wantErrMsg: "",
 	}, {
 		name:       "ip_success",
 		in:         "6 ip 1.2.3.4",
-		wantOpt: dhcpv4.Option{
+		wantCode:   dhcpv4.GenericOptionCode(6),
-			Code:  dhcpv4.GenericOptionCode(6),
+		wantVal:    dhcpv4.IP(net.IP{0x01, 0x02, 0x03, 0x04}),
-			Value: dhcpv4.IP(net.IP{0x01, 0x02, 0x03, 0x04}),
-		},
 		wantErrMsg: "",
-	}, {
-		name:       "ip_fail_v6",
-		in:         "6 ip ::1234",
-		wantOpt:    dhcpv4.Option{},
-		wantErrMsg: "invalid option string \"6 ip ::1234\": bad ipv4 address \"::1234\"",
 	}, {
 		name:     "ips_success",
 		in:       "6 ips 192.168.1.1,192.168.1.2",
-		wantOpt: dhcpv4.Option{
+		wantCode: dhcpv4.GenericOptionCode(6),
-			Code: dhcpv4.GenericOptionCode(6),
+		wantVal: dhcpv4.IPs([]net.IP{
-			Value: dhcpv4.IPs([]net.IP{
 			{0xC0, 0xA8, 0x01, 0x01},
 			{0xC0, 0xA8, 0x01, 0x02},
 		}),
-		},
 		wantErrMsg: "",
 	}, {
 		name:       "text_success",
 		in:         "252 text http://192.168.1.1/",
-		wantOpt: dhcpv4.OptGeneric(
+		wantCode:   dhcpv4.GenericOptionCode(252),
-			dhcpv4.GenericOptionCode(252),
+		wantVal:    dhcpv4.String("http://192.168.1.1/"),
-			[]byte("http://192.168.1.1/"),
-		),
 		wantErrMsg: "",
 	}, {
 		name:       "del_success",
 		in:         "61 del",
-		wantOpt: dhcpv4.Option{
+		wantCode:   dhcpv4.GenericOptionCode(dhcpv4.OptionClientIdentifier),
-			Code:  dhcpv4.GenericOptionCode(dhcpv4.OptionClientIdentifier),
+		wantVal:    dhcpv4.OptionGeneric{Data: nil},
-			Value: dhcpv4.OptionGeneric{Data: nil},
+		wantErrMsg: "",
-		},
+	}, {
+		name:       "bool_success",
+		in:         "19 bool true",
+		wantCode:   dhcpv4.GenericOptionCode(dhcpv4.OptionIPForwarding),
+		wantVal:    dhcpv4.OptionGeneric{Data: []byte{0x01}},
+		wantErrMsg: "",
+	}, {
+		name:       "bool_success_false",
+		in:         "19 bool F",
+		wantCode:   dhcpv4.GenericOptionCode(dhcpv4.OptionIPForwarding),
+		wantVal:    dhcpv4.OptionGeneric{Data: []byte{0x00}},
+		wantErrMsg: "",
+	}, {
+		name:       "dur_success",
+		in:         "24 dur 2h5s",
+		wantCode:   dhcpv4.GenericOptionCode(dhcpv4.OptionPathMTUAgingTimeout),
+		wantVal:    dhcpv4.Duration(2*time.Hour + 5*time.Second),
+		wantErrMsg: "",
+	}, {
+		name:       "u8_success",
+		in:         "23 u8 64",
+		wantCode:   dhcpv4.GenericOptionCode(dhcpv4.OptionDefaultIPTTL),
+		wantVal:    dhcpv4.OptionGeneric{Data: []byte{0x40}},
+		wantErrMsg: "",
+	}, {
+		name:       "u16_success",
+		in:         "22 u16 1234",
+		wantCode:   dhcpv4.GenericOptionCode(dhcpv4.OptionMaximumDatagramAssemblySize),
+		wantVal:    dhcpv4.Uint16(1234),
 		wantErrMsg: "",
 	}, {
 		name:       "bad_parts",
 		in:         "6 ip",
-		wantOpt:    dhcpv4.Option{},
+		wantCode:   nil,
+		wantVal:    nil,
 		wantErrMsg: `invalid option string "6 ip": bad option format`,
 	}, {
 		name:     "bad_code",
 		in:       "256 ip 1.1.1.1",
-		wantOpt: dhcpv4.Option{},
+		wantCode: nil,
+		wantVal:  nil,
 		wantErrMsg: `invalid option string "256 ip 1.1.1.1": parsing option code: ` +
 			`strconv.ParseUint: parsing "256": value out of range`,
 	}, {
 		name:       "bad_type",
 		in:         "6 bad 1.1.1.1",
-		wantOpt:    dhcpv4.Option{},
+		wantCode:   nil,
+		wantVal:    nil,
 		wantErrMsg: `invalid option string "6 bad 1.1.1.1": unknown option type "bad"`,
 	}, {
 		name:     "hex_error",
 		in:       "6 hex ZZZ",
-		wantOpt: dhcpv4.Option{},
+		wantCode: nil,
+		wantVal:  nil,
 		wantErrMsg: `invalid option string "6 hex ZZZ": decoding hex: ` +
 			`encoding/hex: invalid byte: U+005A 'Z'`,
 	}, {
 		name:       "ip_error",
 		in:         "6 ip 1.2.3.x",
-		wantOpt:    dhcpv4.Option{},
+		wantCode:   nil,
+		wantVal:    nil,
 		wantErrMsg: "invalid option string \"6 ip 1.2.3.x\": bad ipv4 address \"1.2.3.x\"",
+	}, {
+		name:       "ip_error_v6",
+		in:         "6 ip ::1234",
+		wantCode:   nil,
+		wantVal:    nil,
+		wantErrMsg: "invalid option string \"6 ip ::1234\": bad ipv4 address \"::1234\"",
 	}, {
 		name:     "ips_error",
 		in:       "6 ips 192.168.1.1,192.168.1.x",
-		wantOpt: dhcpv4.Option{},
+		wantCode: nil,
+		wantVal:  nil,
 		wantErrMsg: "invalid option string \"6 ips 192.168.1.1,192.168.1.x\": " +
 			"parsing ip at index 1: bad ipv4 address \"192.168.1.x\"",
+	}, {
+		name:     "bool_error",
+		in:       "19 bool yes",
+		wantCode: nil,
+		wantVal:  nil,
+		wantErrMsg: "invalid option string \"19 bool yes\": decoding bool: " +
+			"strconv.ParseBool: parsing \"yes\": invalid syntax",
+	}, {
+		name:     "dur_error",
+		in:       "24 dur 3y",
+		wantCode: nil,
+		wantVal:  nil,
+		wantErrMsg: "invalid option string \"24 dur 3y\": decoding dur: " +
+			"unmarshaling duration: time: unknown unit \"y\" in duration \"3y\"",
+	}, {
+		name:     "u8_error",
+		in:       "23 u8 256",
+		wantCode: nil,
+		wantVal:  nil,
+		wantErrMsg: "invalid option string \"23 u8 256\": decoding u8: " +
+			"strconv.ParseUint: parsing \"256\": value out of range",
+	}, {
+		name:     "u16_error",
+		in:       "23 u16 65536",
+		wantCode: nil,
+		wantVal:  nil,
+		wantErrMsg: "invalid option string \"23 u16 65536\": decoding u16: " +
+			"strconv.ParseUint: parsing \"65536\": value out of range",
 	}}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			opt, err := parseDHCPOption(tc.in)
+			code, val, err := parseDHCPOption(tc.in)
 			testutil.AssertErrorMsg(t, tc.wantErrMsg, err)
 
-			// assert.Equal(t, tc.wantOpt.Code.Code(), opt.Code.Code())
+			assert.Equal(t, tc.wantCode, code)
-			// assert.Equal(t, tc.wantOpt.Value.ToBytes(), opt.Value.ToBytes())
+			assert.Equal(t, tc.wantVal, val)
-			assert.Equal(t, tc.wantOpt, opt)
 		})
 	}
 }
 
 func TestPrepareOptions(t *testing.T) {
-	allDefault := dhcpv4.Options{
-		dhcpv4.OptionNonLocalSourceRouting.Code():     []byte{0},
-		dhcpv4.OptionDefaultIPTTL.Code():              []byte{64},
-		dhcpv4.OptionPerformMaskDiscovery.Code():      []byte{0},
-		dhcpv4.OptionMaskSupplier.Code():              []byte{0},
-		dhcpv4.OptionPerformRouterDiscovery.Code():    []byte{1},
-		dhcpv4.OptionRouterSolicitationAddress.Code(): []byte{224, 0, 0, 2},
-		dhcpv4.OptionBroadcastAddress.Code():          []byte{255, 255, 255, 255},
-		dhcpv4.OptionTrailerEncapsulation.Code():      []byte{0},
-		dhcpv4.OptionEthernetEncapsulation.Code():     []byte{0},
-		dhcpv4.OptionTCPKeepaliveInterval.Code():      []byte{0, 0, 0, 0},
-		dhcpv4.OptionTCPKeepaliveGarbage.Code():       []byte{0},
-	}
 	oneIP, otherIP := net.IP{1, 2, 3, 4}, net.IP{5, 6, 7, 8}
 
 	testCases := []struct {
 		name         string
-		checks dhcpv4.Options
+		wantExplicit dhcpv4.Options
 		opts         []string
 	}{{
 		name:         "all_default",
-		checks: allDefault,
+		wantExplicit: nil,
 		opts:         nil,
 	}, {
 		name: "configured_ip",
-		checks: dhcpv4.Options{
+		wantExplicit: dhcpv4.OptionsFromList(
-			dhcpv4.OptionBroadcastAddress.Code(): oneIP,
+			dhcpv4.OptBroadcastAddress(oneIP),
-		},
+		),
 		opts: []string{
 			fmt.Sprintf("%d ip %s", dhcpv4.OptionBroadcastAddress, oneIP),
 		},
 	}, {
 		name: "configured_ips",
-		checks: dhcpv4.Options{
+		wantExplicit: dhcpv4.OptionsFromList(
-			dhcpv4.OptionDomainNameServer.Code(): append(oneIP, otherIP...),
+			dhcpv4.Option{
+				Code:  dhcpv4.OptionDomainNameServer,
+				Value: dhcpv4.IPs{oneIP, otherIP},
 			},
+		),
 		opts: []string{
 			fmt.Sprintf("%d ips %s,%s", dhcpv4.OptionDomainNameServer, oneIP, otherIP),
 		},
 	}, {
 		name:         "configured_bad",
-		checks: allDefault,
+		wantExplicit: nil,
 		opts: []string{
+			"19 bool yes",
+			"24 dur 3y",
+			"23 u8 256",
+			"23 u16 65536",
 			"20 hex",
 			"23 hex abc",
 			"32 ips 1,2,3,4",
@@ -168,26 +219,29 @@ func TestPrepareOptions(t *testing.T) {
 		},
 	}, {
 		name: "configured_del",
-		checks: dhcpv4.Options{
+		wantExplicit: dhcpv4.OptionsFromList(
-			dhcpv4.OptionBroadcastAddress.Code(): nil,
+			dhcpv4.OptBroadcastAddress(nil),
-		},
+		),
 		opts: []string{
 			"28 del",
 		},
 	}, {
 		name: "rewritten_del",
-		checks: dhcpv4.Options{
+		wantExplicit: dhcpv4.OptionsFromList(
-			dhcpv4.OptionBroadcastAddress.Code(): []byte{255, 255, 255, 255},
+			dhcpv4.OptBroadcastAddress(netutil.IPv4bcast()),
-		},
+		),
 		opts: []string{
 			"28 del",
 			"28 ip 255.255.255.255",
 		},
 	}, {
 		name: "configured_and_del",
-		checks: dhcpv4.Options{
+		wantExplicit: dhcpv4.OptionsFromList(
-			123: []byte("cba"),
+			dhcpv4.Option{
+				Code:  dhcpv4.OptionGeoConf,
+				Value: dhcpv4.String("cba"),
 			},
+		),
 		opts: []string{
 			"123 text abc",
 			"123 del",
@@ -197,18 +251,16 @@ func TestPrepareOptions(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			if tc.name == "configured_del" {
+			implicit, explicit := prepareOptions(V4ServerConf{
-				assert.True(t, true)
-			}
-			opts := prepareOptions(V4ServerConf{
 				// Just to avoid nil pointer dereference.
 				subnet:  &net.IPNet{},
 				Options: tc.opts,
 			})
-			for c, v := range tc.checks {
+
-				val := opts.Get(dhcpv4.GenericOptionCode(c))
+			assert.Equal(t, tc.wantExplicit, explicit)
-				assert.Lenf(t, val, len(v), "Code: %v", c)
+
-				assert.Equal(t, v, val)
+			for c := range explicit {
+				assert.NotContains(t, implicit, c)
 			}
 		})
 	}

internal/dhcpd/v4.go

@@ -33,6 +33,17 @@ type v4Server struct {
 	conf V4ServerConf
 	srv  *server4.Server
 
+	// implicitOpts are the options listed in Appendix A of RFC 2131 initialized
+	// with default values.  It must not have intersections with [explicitOpts].
+	implicitOpts dhcpv4.Options
+
+	// explicitOpts are the options parsed from the configuration.  It must not
+	// have intersections with [implicitOpts].
+	explicitOpts dhcpv4.Options
+
+	// leasesLock protects leases, leaseHosts, and leasedOffsets.
+	leasesLock sync.Mutex
+
 	// leasedOffsets contains offsets from conf.ipRange.start that have been
 	// leased.
 	leasedOffsets *bitSet
@@ -42,12 +53,6 @@ type v4Server struct {
 
 	// leases contains all dynamic and static leases.
 	leases []*Lease
-
-	// leasesLock protects leases, leaseHosts, and leasedOffsets.
-	leasesLock sync.Mutex
-
-	// options holds predefined DHCP options to return to clients.
-	options dhcpv4.Options
 }
 
 // WriteDiskConfig4 - write configuration
@@ -999,34 +1004,42 @@ func (s *v4Server) handle(req, resp *dhcpv4.DHCPv4) int {
 		resp.YourIPAddr = netutil.CloneIP(l.IP)
 	}
 
+	s.updateOptions(req, resp)
+
+	return 1
+}
+
+// updateOptions updates the options of the response in accordance with the
+// request and RFC 2131.
+//
+// See https://datatracker.ietf.org/doc/html/rfc2131#section-4.3.1.
+func (s *v4Server) updateOptions(req, resp *dhcpv4.DHCPv4) {
 	// Set IP address lease time for all DHCPOFFER messages and DHCPACK messages
 	// replied for DHCPREQUEST.
 	//
 	// TODO(e.burkov):  Inspect why this is always set to configured value.
 	resp.UpdateOption(dhcpv4.OptIPAddressLeaseTime(s.conf.leaseTime))
 
+	// If the server recognizes the parameter as a parameter defined in the Host
+	// Requirements Document, the server MUST include the default value for that
+	// parameter.
+	for _, code := range req.ParameterRequestList() {
+		if val := s.implicitOpts.Get(code); val != nil {
+			resp.UpdateOption(dhcpv4.OptGeneric(code, val))
+		}
+	}
+
+	// If the server has been explicitly configured with a default value for the
+	// parameter or the parameter has a non-default value on the client's
+	// subnet, the server MUST include that value in an appropriate option.
+	for code, val := range s.explicitOpts {
+		if val != nil {
+			resp.Options[code] = val
+		} else {
 			// Delete options explicitly configured to be removed.
-	for code := range resp.Options {
-		if val, ok := s.options[code]; ok && val == nil {
 			delete(resp.Options, code)
 		}
 	}
-
-	// Update values for each explicitly configured parameter requested by
-	// client.
-	//
-	// See https://datatracker.ietf.org/doc/html/rfc2131#section-4.3.1.
-	requested := req.ParameterRequestList()
-	for _, code := range requested {
-		if val := s.options.Get(code); val != nil {
-			resp.UpdateOption(dhcpv4.Option{
-				Code:  code,
-				Value: dhcpv4.OptionGeneric{Data: s.options.Get(code)},
-			})
-		}
-	}
-
-	return 1
 }
 
 // client(0.0.0.0:68) -> (Request:ClientMAC,Type=Discover,ClientID,ReqIP,HostName) -> server(255.255.255.255:67)
@@ -1151,8 +1164,11 @@ func (s *v4Server) Start() (err error) {
 	}
 	// Update the value of Domain Name Server option separately from others if
 	// not assigned yet since its value is available only at server's start.
-	if !s.options.Has(dhcpv4.OptionDomainNameServer) {
+	//
-		s.options.Update(dhcpv4.OptDNS(dnsIPAddrs...))
+	// TODO(e.burkov):  Initialize as implicit option with the rest of default
+	// options when it will be possible to do before the call to Start.
+	if !s.explicitOpts.Has(dhcpv4.OptionDomainNameServer) {
+		s.implicitOpts.Update(dhcpv4.OptDNS(dnsIPAddrs...))
 	}
 
 	s.conf.dnsIPAddrs = dnsIPAddrs
@@ -1279,7 +1295,7 @@ func v4Create(conf V4ServerConf) (srv DHCPServer, err error) {
 		s.conf.leaseTime = time.Second * time.Duration(conf.LeaseDuration)
 	}
 
-	s.options = prepareOptions(s.conf)
+	s.implicitOpts, s.explicitOpts = prepareOptions(s.conf)
 
 	return s, nil
 }

internal/dhcpd/v4_test.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
+	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/golibs/stringutil"
 	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/insomniacslk/dhcp/dhcpv4"
@@ -346,7 +347,7 @@ func TestV4Server_handle_optionsPriority(t *testing.T) {
 			}
 			conf.Options = []string{b.String()}
 		} else {
-			defer func() { s.options.Update(dhcpv4.OptDNS(defaultIP)) }()
+			defer func() { s.implicitOpts.Update(dhcpv4.OptDNS(defaultIP)) }()
 		}
 
 		ss, err := v4Create(conf)
@@ -402,6 +403,111 @@ func TestV4Server_handle_optionsPriority(t *testing.T) {
 	})
 }
 
+func TestV4Server_updateOptions(t *testing.T) {
+	testIP := net.IP{1, 2, 3, 4}
+
+	dontWant := func(c dhcpv4.OptionCode) (opt dhcpv4.Option) {
+		return dhcpv4.OptGeneric(c, nil)
+	}
+
+	testCases := []struct {
+		name     string
+		wantOpts dhcpv4.Options
+		reqMods  []dhcpv4.Modifier
+		confOpts []string
+	}{{
+		name: "requested_default",
+		wantOpts: dhcpv4.OptionsFromList(
+			dhcpv4.OptBroadcastAddress(netutil.IPv4bcast()),
+		),
+		reqMods: []dhcpv4.Modifier{
+			dhcpv4.WithRequestedOptions(dhcpv4.OptionBroadcastAddress),
+		},
+		confOpts: nil,
+	}, {
+		name: "requested_non-default",
+		wantOpts: dhcpv4.OptionsFromList(
+			dhcpv4.OptBroadcastAddress(testIP),
+		),
+		reqMods: []dhcpv4.Modifier{
+			dhcpv4.WithRequestedOptions(dhcpv4.OptionBroadcastAddress),
+		},
+		confOpts: []string{
+			fmt.Sprintf("%d ip %s", dhcpv4.OptionBroadcastAddress, testIP),
+		},
+	}, {
+		name: "non-requested_default",
+		wantOpts: dhcpv4.OptionsFromList(
+			dontWant(dhcpv4.OptionBroadcastAddress),
+		),
+		reqMods:  nil,
+		confOpts: nil,
+	}, {
+		name: "non-requested_non-default",
+		wantOpts: dhcpv4.OptionsFromList(
+			dhcpv4.OptBroadcastAddress(testIP),
+		),
+		reqMods: nil,
+		confOpts: []string{
+			fmt.Sprintf("%d ip %s", dhcpv4.OptionBroadcastAddress, testIP),
+		},
+	}, {
+		name: "requested_deleted",
+		wantOpts: dhcpv4.OptionsFromList(
+			dontWant(dhcpv4.OptionBroadcastAddress),
+		),
+		reqMods: []dhcpv4.Modifier{
+			dhcpv4.WithRequestedOptions(dhcpv4.OptionBroadcastAddress),
+		},
+		confOpts: []string{
+			fmt.Sprintf("%d del", dhcpv4.OptionBroadcastAddress),
+		},
+	}, {
+		name: "requested_non-default_deleted",
+		wantOpts: dhcpv4.OptionsFromList(
+			dontWant(dhcpv4.OptionBroadcastAddress),
+		),
+		reqMods: []dhcpv4.Modifier{
+			dhcpv4.WithRequestedOptions(dhcpv4.OptionBroadcastAddress),
+		},
+		confOpts: []string{
+			fmt.Sprintf("%d ip %s", dhcpv4.OptionBroadcastAddress, testIP),
+			fmt.Sprintf("%d del", dhcpv4.OptionBroadcastAddress),
+		},
+	}}
+
+	for _, tc := range testCases {
+		req, err := dhcpv4.New(tc.reqMods...)
+		require.NoError(t, err)
+
+		resp, err := dhcpv4.NewReplyFromRequest(req)
+		require.NoError(t, err)
+
+		conf := defaultV4ServerConf()
+		conf.Options = tc.confOpts
+
+		s, err := v4Create(conf)
+		require.NoError(t, err)
+
+		require.IsType(t, (*v4Server)(nil), s)
+		s4, _ := s.(*v4Server)
+
+		t.Run(tc.name, func(t *testing.T) {
+			s4.updateOptions(req, resp)
+
+			for c, v := range tc.wantOpts {
+				if v == nil {
+					assert.NotContains(t, resp.Options, c)
+
+					continue
+				}
+
+				assert.Equal(t, v, resp.Options.Get(dhcpv4.GenericOptionCode(c)))
+			}
+		})
+	}
+}
+
 func TestV4StaticLease_Get(t *testing.T) {
 	sIface := defaultSrv(t)
 
@@ -409,7 +515,7 @@ func TestV4StaticLease_Get(t *testing.T) {
 	require.True(t, ok)
 
 	s.conf.dnsIPAddrs = []net.IP{{192, 168, 10, 1}}
-	s.options.Update(dhcpv4.OptDNS(s.conf.dnsIPAddrs...))
+	s.implicitOpts.Update(dhcpv4.OptDNS(s.conf.dnsIPAddrs...))
 
 	l := &Lease{
 		Hostname: "static-1.local",
@@ -498,7 +604,7 @@ func TestV4DynamicLease_Get(t *testing.T) {
 	require.True(t, ok)
 
 	s.conf.dnsIPAddrs = []net.IP{{192, 168, 10, 1}}
-	s.options.Update(dhcpv4.OptDNS(s.conf.dnsIPAddrs...))
+	s.implicitOpts.Update(dhcpv4.OptDNS(s.conf.dnsIPAddrs...))
 
 	var req, resp *dhcpv4.DHCPv4
 	mac := net.HardwareAddr{0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA}

internal/stats/stats_internal_test.go

@@ -1,8 +1,14 @@
 package stats
 
 import (
+	"fmt"
+	"path/filepath"
+	"sync"
+	"sync/atomic"
 	"testing"
+	"time"
 
+	"github.com/AdguardTeam/golibs/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -24,3 +30,76 @@ func TestStatsCollector(t *testing.T) {
 		}
 	})
 }
+
+func TestStats_races(t *testing.T) {
+	var r uint32
+	idGen := func() (id uint32) { return atomic.LoadUint32(&r) }
+	conf := Config{
+		UnitID:    idGen,
+		Filename:  filepath.Join(t.TempDir(), "./stats.db"),
+		LimitDays: 1,
+	}
+
+	s, err := New(conf)
+	require.NoError(t, err)
+
+	s.Start()
+	startTime := time.Now()
+	testutil.CleanupAndRequireSuccess(t, s.Close)
+
+	type signal = struct{}
+
+	writeFunc := func(start, fin *sync.WaitGroup, waitCh <-chan unit, i int) {
+		e := Entry{
+			Domain: fmt.Sprintf("example-%d.org", i),
+			Client: fmt.Sprintf("client_%d", i),
+			Result: Result(i)%(resultLast-1) + 1,
+			Time:   uint32(time.Since(startTime).Milliseconds()),
+		}
+
+		start.Done()
+		defer fin.Done()
+
+		<-waitCh
+
+		s.Update(e)
+	}
+	readFunc := func(start, fin *sync.WaitGroup, waitCh <-chan unit) {
+		start.Done()
+		defer fin.Done()
+
+		<-waitCh
+
+		_, _ = s.getData(24)
+	}
+
+	const (
+		roundsNum = 3
+
+		writersNum = 10
+		readersNum = 5
+	)
+
+	for round := 0; round < roundsNum; round++ {
+		atomic.StoreUint32(&r, uint32(round))
+
+		startWG, finWG := &sync.WaitGroup{}, &sync.WaitGroup{}
+		waitCh := make(chan unit)
+
+		for i := 0; i < writersNum; i++ {
+			startWG.Add(1)
+			finWG.Add(1)
+			go writeFunc(startWG, finWG, waitCh, i)
+		}
+
+		for i := 0; i < readersNum; i++ {
+			startWG.Add(1)
+			finWG.Add(1)
+			go readFunc(startWG, finWG, waitCh)
+		}
+
+		startWG.Wait()
+		close(waitCh)
+		finWG.Wait()
+	}
+}

scripts/make/go-lint.sh

@@ -220,9 +220,9 @@ exit_on_output gofumpt --extra -e -l .
 "$GO" vet ./...
 
 # Apply more lax standards to the code we haven't properly refactored yet.
-gocyclo --over 17 ./internal/dhcpd ./internal/querylog/
+gocyclo --over 17 ./internal/querylog/
 gocyclo --over 16 ./internal/dnsforward/
-gocyclo --over 15 ./internal/home/
+gocyclo --over 15 ./internal/home/ ./internal/dhcpd
 gocyclo --over 13 ./internal/filtering/
 
 # Apply stricter standards to new or somewhat refactored code.