- make changes based on discussion on PR #386 of insomniacslk/dhcp

Signed-off-by: Hu Jun <hujun.work@gmail.com>

3 files changed, 67 insertions, 216 deletions

diff --git a/dhcpv4/nclient4/client.go b/dhcpv4/nclient4/client.go
index 414a48e..3379df4 100644
--- a/dhcpv4/nclient4/client.go
+++ b/dhcpv4/nclient4/client.go
@@ -167,13 +167,6 @@ type Client struct {
 	//identify client other than the HWAddress,
 	//like client-id, option82/remote-id..etc
 	clientIDOptions dhcpv4.OptionCodeList
-
-	//lease info after DORA, nil before DORA
-	lease *DHCPv4ClientLease
-	//the handnler function for applying lease
-	leaseApplyHandler func(DHCPv4ClientLease, bool) error
-	//binding interface name
-	ifName string
 }
 
 // New returns a client usable with an unconfigured interface.
@@ -197,12 +190,9 @@ func new(iface string, conn net.PacketConn, ifaceHWAddr net.HardwareAddr, opts .
 		conn:        conn,
 		logger:      EmptyLogger{},
 
-		done:              make(chan struct{}),
-		pending:           make(map[dhcpv4.TransactionID]*pendingCh),
-		lease:             nil,
-		leaseApplyHandler: defaultLeaseApplyHandler,
-		clientIDOptions:   dhcpv4.OptionCodeList{},
-		ifName:            iface,
+		done:            make(chan struct{}),
+		pending:         make(map[dhcpv4.TransactionID]*pendingCh),
+		clientIDOptions: dhcpv4.OptionCodeList{},
 	}
 
 	for _, opt := range opts {
@@ -448,7 +438,7 @@ func (c *Client) DiscoverOffer(ctx context.Context, modifiers ...dhcpv4.Modifier
 // Request completes the 4-way Discover-Offer-Request-Ack handshake.
 //
 // Note that modifiers will be applied *both* to Discover and Request packets.
-func (c *Client) Request(ctx context.Context, modifiers ...dhcpv4.Modifier) (offer, ack *dhcpv4.DHCPv4, err error) {
+func (c *Client) Request(ctx context.Context, modifiers ...dhcpv4.Modifier) (offer *dhcpv4.DHCPv4, lease *Lease, err error) {
 	offer, err = c.DiscoverOffer(ctx, modifiers...)
 	if err != nil {
 		err = fmt.Errorf("unable to receive an offer: %w", err)
@@ -463,12 +453,19 @@ func (c *Client) Request(ctx context.Context, modifiers ...dhcpv4.Modifier) (off
 		return
 	}
 
-	ack, err = c.SendAndRead(ctx, c.serverAddr, request, nil)
+	ack, err := c.SendAndRead(ctx, c.serverAddr, request, nil)
 	if err != nil {
 		err = fmt.Errorf("got an error while processing the request: %w", err)
 		return
 	}
-
+	lease = &Lease{}
+	lease.ACK = ack
+	lease.CreationTime = time.Now()
+	lease.IDOptions = dhcpv4.Options{}
+	for _, optioncode := range c.clientIDOptions {
+		v := request.Options.Get(optioncode)
+		lease.IDOptions.Update(dhcpv4.OptGeneric(optioncode, v))
+	}
 	return
 }
 
@@ -537,15 +534,6 @@ var errDeadlineExceeded = errors.New("INTERNAL ERROR: deadline exceeded")
 // ClientHWAddr is returned.
 func (c *Client) SendAndRead(ctx context.Context, dest *net.UDPAddr, p *dhcpv4.DHCPv4, match Matcher) (*dhcpv4.DHCPv4, error) {
 	var response *dhcpv4.DHCPv4
-	//check if the request packet has all options required by c.clientIdOptions
-	for _, optioncode := range c.clientIDOptions {
-		if len(p.Options.Get(optioncode)) == 0 {
-			err := fmt.Errorf("Option %v required for client identification is missing in request", optioncode)
-			return nil, err
-		}
-
-	}
-
 	err := c.retryFn(func(timeout time.Duration) error {
 		ch, rem, err := c.send(dest, p)
 		if err != nil {
diff --git a/dhcpv4/nclient4/example_lease_test.go b/dhcpv4/nclient4/example_lease_test.go
index 876a075..a324162 100644
--- a/dhcpv4/nclient4/example_lease_test.go
+++ b/dhcpv4/nclient4/example_lease_test.go
@@ -1,18 +1,41 @@
-//this is an example for nclient4 with lease
+//this is an example for nclient4 with lease/release
 
-package nclient4_test
+package nclient4
 
 import (
 	"context"
+	"fmt"
 	"log"
 
 	"github.com/insomniacslk/dhcp/dhcpv4"
 	"github.com/insomniacslk/dhcp/dhcpv4/nclient4"
+	"github.com/vishvananda/netlink"
 )
 
-func Example_dHCPv4ClientLease() {
-	ifname := "eth0"
-	remoteID := "client-1"
+//applyLease adding the assigned ip to the interface specified by ifname
+func applyLease(lease *nclient4.Lease, ifname string) error {
+	link, err := netlink.LinkByName(ifname)
+	if err != nil {
+		return err
+	}
+	prefixlen := 32
+	if ipmask := lease.ACK.SubnetMask(); ipmask != nil {
+		prefixlen, _ = ipmask.Size()
+
+	}
+	prefixstr := fmt.Sprintf("%v/%v", lease.ACK.YourIPAddr, prefixlen)
+	naddr, err := netlink.ParseAddr(prefixstr)
+	if err != nil {
+		return err
+	}
+	err = netlink.AddrReplace(link, naddr)
+	return err
+
+}
+
+func main() {
+	ifname := "eth1.200"
+	remoteid := "client-1"
 	var idoptlist dhcpv4.OptionCodeList
 	//specify option82 is part of client identification used by DHCPv4 server
 	idoptlist.Add(dhcpv4.OptionRelayAgentInformation)
@@ -22,17 +45,19 @@ func Example_dHCPv4ClientLease() {
 		log.Fatalf("failed to create dhcpv4 client,%v", err)
 	}
 	//adding option82/remote-id option to discovery and request
-	remoteIDSubOpt := dhcpv4.OptGeneric(dhcpv4.AgentRemoteIDSubOption, []byte(remoteID))
-	option82 := dhcpv4.OptRelayAgentInfo(remoteIDSubOpt)
-	_, _, err = clnt.RequestSavingLease(context.Background(), dhcpv4.WithOption(option82))
+	remoteidsubopt := dhcpv4.OptGeneric(dhcpv4.AgentRemoteIDSubOption, []byte(remoteid))
+	option82 := dhcpv4.OptRelayAgentInfo(remoteidsubopt)
+	_, lease, err := clnt.Request(context.Background(), dhcpv4.WithOption(option82))
 	if err != nil {
 		log.Fatal(err)
 	}
 	//print the lease
-	log.Printf("Got lease:\n%v", clnt.GetLease())
+	log.Printf("Got lease:\n%+v", lease)
+	//apply the lease
+	applyLease(lease, ifname)
 	//release the lease
 	log.Print("Releasing lease...")
-	err = clnt.Release()
+	err = clnt.Release(lease)
 	if err != nil {
 		log.Fatal(err)
 	}
diff --git a/dhcpv4/nclient4/lease.go b/dhcpv4/nclient4/lease.go
index b4f317e..83c59e6 100644
--- a/dhcpv4/nclient4/lease.go
+++ b/dhcpv4/nclient4/lease.go
@@ -1,51 +1,21 @@
-//This is lease managment for nclient4
+//This is lease support for nclient4
 
 package nclient4
 
 import (
-	"context"
 	"fmt"
 	"net"
 	"time"
 
 	"github.com/insomniacslk/dhcp/dhcpv4"
-	"github.com/vishvananda/netlink"
 )
 
-const (
-	//default lease time if server doesn't return lease time option or return zero
-	defaultLeaseTime = time.Hour
-)
-
-//DHCPv4ClientLease contains a DHCPv4 lease after DORA,
-//could be used for creating a new Client with NewWithLease()
-type DHCPv4ClientLease struct {
-	IfName         string
-	MACAddr        net.HardwareAddr
-	ServerAddr     net.UDPAddr
-	AssignedIP     net.IP
-	AssignedIPMask net.IPMask
-	CreationTime   time.Time
-	LeaseDuration  time.Duration
-	RenewInterval  time.Duration
-	RebindInterval time.Duration
-	IDOptions      dhcpv4.Options //DHCPv4 options to identify the client like client-id, option82/remote-id
-	AckOptions     dhcpv4.Options //DHCPv4 options in ACK, could be used for applying lease
-
-}
-
-//return a string representation
-func (lease DHCPv4ClientLease) String() string {
-	const fmtstr = "%-35s\t%-35s\n"
-	const timefmtstr = "01/02/2006 15:04:05.000"
-	rstr := fmt.Sprintf(fmtstr, fmt.Sprintf("Interface:%v", lease.IfName), fmt.Sprintf("MAC:%v", lease.MACAddr))
-	rstr += fmt.Sprintf(fmtstr, fmt.Sprintf("Svr:%v", lease.ServerAddr.IP), fmt.Sprintf("Created:%v", lease.CreationTime.Format(timefmtstr)))
-	prefixlen, _ := lease.AssignedIPMask.Size()
-	rstr += fmt.Sprintf(fmtstr, fmt.Sprintf("IP:%v/%v", lease.AssignedIP, prefixlen), fmt.Sprintf("Lease time:%v", lease.LeaseDuration))
-	rstr += fmt.Sprintf(fmtstr, fmt.Sprintf("Renew interval:%v", lease.RenewInterval), fmt.Sprintf("Rebind interval:%v", lease.RebindInterval))
-	rstr += fmt.Sprintf("Id options:\n%v", lease.IDOptions)
-	rstr += fmt.Sprintf("ACK options:\n%v", lease.AckOptions)
-	return rstr
+//Lease contains a DHCPv4 lease after DORA.
+//note: Lease doesn't include binding interface name
+type Lease struct {
+	ACK          *dhcpv4.DHCPv4
+	CreationTime time.Time
+	IDOptions    dhcpv4.Options //DHCPv4 options to identify the client like client-id, option82/remote-id
 }
 
 // WithClientIDOptions configures a list of DHCPv4 option code that DHCP server
@@ -57,123 +27,12 @@ func WithClientIDOptions(cidl dhcpv4.OptionCodeList) ClientOpt {
 	}
 }
 
-// WithApplyLeaseHandler specifies a handler function which is called when
-// Client.ApplyLease() is called; without this, a default handler function is called.
-// the default handler will add/remove the assigned address to/from the binding interface;
-// bool parameter is true when lease is applied, false when lease is released
-func WithApplyLeaseHandler(h func(DHCPv4ClientLease, bool) error) ClientOpt {
-	return func(c *Client) (err error) {
-		c.leaseApplyHandler = h
-		return
-	}
-}
-
-//default lease apply handler
-//add/remove address to/from binding interface
-func defaultLeaseApplyHandler(l DHCPv4ClientLease, enable bool) error {
-	link, err := netlink.LinkByName(l.IfName)
-	if err != nil {
-		return err
-	}
-	plen, _ := l.AssignedIPMask.Size()
-	prefixstr := fmt.Sprintf("%v/%v", l.AssignedIP, plen)
-	naddr, err := netlink.ParseAddr(prefixstr)
-	if err != nil {
-		return err
-	}
-	if enable {
-		err = netlink.AddrReplace(link, naddr)
-
-	} else {
-		err = netlink.AddrDel(link, naddr)
-	}
-	return err
-
-}
-
-//ApplyLease apply/unapply the lease, call the c.leaseApplyHandler
-func (c *Client) ApplyLease(enable bool) error {
-	if c.lease == nil {
-		return fmt.Errorf("no lease to apply")
-	}
-	return c.leaseApplyHandler(c.GetLease(), enable)
-}
-
-//GetLease return the lease
-func (c *Client) GetLease() (clease DHCPv4ClientLease) {
-	clease = *c.lease
-	clease.MACAddr = c.ifaceHWAddr
-	clease.IfName = c.ifName
-	clease.ServerAddr = *c.serverAddr
-	return
-}
-
-// RequestSavingLease completes DORA handshake and store&apply the lease
-//
-// Note that modifiers will be applied *both* to Discover and Request packets.
-func (c *Client) RequestSavingLease(ctx context.Context, modifiers ...dhcpv4.Modifier) (offer, ack *dhcpv4.DHCPv4, err error) {
-	offer, err = c.DiscoverOffer(ctx, modifiers...)
-	if err != nil {
-		err = fmt.Errorf("unable to receive an offer: %w", err)
-		return
-	}
-
-	// TODO(chrisko): should this be unicast to the server?
-	request, err := dhcpv4.NewRequestFromOffer(offer, dhcpv4.PrependModifiers(modifiers,
-		dhcpv4.WithOption(dhcpv4.OptMaxMessageSize(MaxMessageSize)))...)
-	if err != nil {
-		err = fmt.Errorf("unable to create a request: %w", err)
-		return
-	}
-
-	ack, err = c.SendAndRead(ctx, c.serverAddr, request, nil)
-	if err != nil {
-		err = fmt.Errorf("got an error while processing the request: %w", err)
-		return
-	}
-	//save lease
-	c.lease = &DHCPv4ClientLease{}
-	c.lease.AssignedIP = ack.YourIPAddr
-	c.lease.AssignedIPMask = ack.SubnetMask()
-	c.lease.CreationTime = time.Now()
-	c.lease.LeaseDuration = ack.IPAddressLeaseTime(0)
-	if c.lease.LeaseDuration == 0 {
-		c.lease.LeaseDuration = defaultLeaseTime
-		c.logger.Printf("warning: server doesn't include Lease Time option or it is zero seconds, setting lease time to default %v", c.lease.LeaseDuration)
-
-	}
-	c.lease.RenewInterval = ack.IPAddressRenewalTime(0)
-	if c.lease.RenewInterval == 0 {
-		//setting default to half of lease time based on RFC2131,section 4.4.5
-		c.lease.RenewInterval = time.Duration(float64(c.lease.LeaseDuration) / 2)
-		c.logger.Printf("warning: server doesn't include Renew Time option or it is zero seconds, setting lease time to default %v", c.lease.RenewInterval)
-
-	}
-	c.lease.RebindInterval = ack.IPAddressRebindingTime(0)
-	if c.lease.RebindInterval == 0 {
-		//setting default to 0.875 of lease time based on RFC2131,section 4.4.5
-		c.lease.RebindInterval = time.Duration(float64(c.lease.LeaseDuration) * 0.875)
-		c.logger.Printf("warning: server doesn't include Renew Time option or it is zero seconds, setting lease time to default %v", c.lease.RebindInterval)
-
-	}
-	c.lease.IDOptions = dhcpv4.Options{}
-	for _, optioncode := range c.clientIDOptions {
-		v := request.Options.Get(optioncode)
-		c.lease.IDOptions.Update(dhcpv4.OptGeneric(optioncode, v))
-	}
-	c.lease.AckOptions = ack.Options
-	//update server address
-	c.serverAddr = &(net.UDPAddr{IP: ack.ServerIdentifier(), Port: 67})
-	err = c.ApplyLease(true)
-	return
-}
-
-//Release send DHCPv4 release messsage to server.
+//Release send DHCPv4 release messsage to server, based on specified lease.
 //release is sent as unicast per RFC2131, section 4.4.4.
-//The lease need to be applied with c.ApplyLease(true) first before calling Release.
-func (c *Client) Release() error {
-	if c.lease == nil {
-		return fmt.Errorf("There is no lease to release")
+//This function requires assigned address has been added on the binding interface.
+func (c *Client) Release(lease *Lease) error {
+	if lease == nil {
+		return fmt.Errorf("lease is nil")
 	}
 	req, err := dhcpv4.New()
 	if err != nil {
@@ -181,19 +40,19 @@ func (c *Client) Release() error {
 	}
 	//This is to make sure use same client identification options used during
 	//DORA, so that DHCP server could identify the required lease
-	req.Options = c.lease.IDOptions
+	req.Options = lease.IDOptions
 
 	req.UpdateOption(dhcpv4.OptMessageType(dhcpv4.MessageTypeRelease))
-	req.ClientHWAddr = c.ifaceHWAddr
-	req.ClientIPAddr = c.lease.AssignedIP
-	req.UpdateOption(dhcpv4.OptServerIdentifier(c.serverAddr.IP))
+	req.ClientHWAddr = lease.ACK.ClientHWAddr
+	req.ClientIPAddr = lease.ACK.YourIPAddr
+	req.UpdateOption(dhcpv4.OptServerIdentifier(lease.ACK.ServerIPAddr))
 	req.SetUnicast()
-	luaddr, err := net.ResolveUDPAddr("udp4", fmt.Sprintf("%v:%v", c.lease.AssignedIP, 68))
+	luaddr, err := net.ResolveUDPAddr("udp4", fmt.Sprintf("%v:%v", lease.ACK.YourIPAddr, 68))
 	if err != nil {
 		return err
 	}
 
-	uniconn, err := net.DialUDP("udp4", luaddr, c.serverAddr)
+	uniconn, err := net.DialUDP("udp4", luaddr, &net.UDPAddr{IP: lease.ACK.ServerIPAddr, Port: 67})
 	if err != nil {
 		return err
 	}
@@ -202,26 +61,5 @@ func (c *Client) Release() error {
 		return err
 	}
 	c.logger.PrintMessage("sent message:", req)
-	return c.ApplyLease(false)
-}
-
-//NewWithLease return a Client with populated lease.
-//this function could be used to release a saved lease.
-func NewWithLease(clease DHCPv4ClientLease, opts ...ClientOpt) (*Client, error) {
-	clntoptlist := []ClientOpt{
-		WithServerAddr(&clease.ServerAddr),
-		WithHWAddr(clease.MACAddr),
-	}
-	clntoptlist = append(clntoptlist, opts...)
-	clnt, err := New(clease.IfName, clntoptlist...)
-	if err != nil {
-		return nil, err
-	}
-	clnt.ifName = clease.IfName
-	clnt.lease = &clease
-	for optioncode := range clease.IDOptions {
-		clnt.clientIDOptions.Add(dhcpv4.GenericOptionCode(optioncode))
-	}
-	return clnt, nil
-
+	return nil
 }