Add missing documentation

auth/chap/chap.go

@@ -11,7 +11,7 @@ import (
 
 const _defaultTimeout = 10 * time.Second
 
-// CHAP is the CHAP protocol implementation
+// CHAP is the CHAP authentication implementation
 type CHAP struct {
 	logger   *zerolog.Logger
 	sendChan chan []byte

auth/chap/enum.go

@@ -2,18 +2,16 @@ package chap
 
 import "fmt"
 
-// Code is the code of CHAP msg
 type Code uint8
 
-// List of CHAPCode
+// List of CHAP codes
 const (
 	CodeChallenge Code = 1
 	CodeResponse  Code = 2
 	CodeSuccess   Code = 3
 	CodeFailure   Code = 4
 )
 
-// String returns a string representation of c
 func (c Code) String() string {
 	switch c {
 	case CodeChallenge:

auth/pap/enum.go

@@ -2,17 +2,15 @@ package pap
 
 import "fmt"
 
-// Code is the PAP msg code
 type Code uint8
 
-// A list of PAP msg code
+// List of PAP codes
 const (
 	CodeAuthRequest Code = 1
 	CodeAuthACK     Code = 2
 	CodeAuthNAK     Code = 3
 )
 
-// String return a string representation of c
 func (c Code) String() string {
 	switch c {
 	case CodeAuthRequest:

auth/pap/pap.go

@@ -14,7 +14,7 @@ const (
 	_defaultRetryNumber = 3
 )
 
-// PAP is the PAP protocol implementation
+// PAP is the PAP authentication implementation
 type PAP struct {
 	logger            *zerolog.Logger
 	sendChan          chan []byte

client/client.go

@@ -12,7 +12,7 @@ import (
 	"time"
 )
 
-// Client represents a PPPoE/PPP sessions handler
+// Client represents a PPPoE/PPP sessions lifetime handler (aka a Pool)
 type Client struct {
 	cfg        *Setup
 	relayConn  etherconn.PacketRelay
@@ -32,18 +32,25 @@ func NewClient(relayConn etherconn.PacketRelay, cfg *Setup) (*Client, error) {
 		cfg:       cfg,
 		relayConn: relayConn,
 		sessions:  make([]*session, cfg.NumOfClients),
-		closed:    make(chan struct{}),
 		blacklist: lcp.NewDefaultBlacklist(),
+		closed:    make(chan struct{}),
 	}, nil
 }
 
+// SetBlacklist adds a custom IP blacklist to avoid the usage of
+// some specific addresses in the PPP sessions.
 func (c *Client) SetBlacklist(blacklist lcp.Blacklist) {
 	c.blacklist = blacklist
 }
 
 // Dial starts all the required initial sessions.
-// If it returns an error, the Client type can't be used anymore, and it's
-// the equivalent of a call to Close().
+// It returns an error only if all the sessions failed, and there is no
+// client running after the call to this function.
+// If at least one session handshake succeed, and there are some failed
+// sessions, the failed sessions will be restarted in another goroutine
+// asynchronously.
+// If this function returns an error, the current Client can't be used anymore,
+// and it's the equivalent of a call to Close().
 func (c *Client) Dial(ctx context.Context) error {
 	var wg sync.WaitGroup
 
@@ -84,10 +91,13 @@ func (c *Client) Dial(ctx context.Context) error {
 	return nil
 }
 
+// NumSessions returns the total number of target concurrent PPPoE sessions.
 func (c *Client) NumSessions() int {
 	return len(c.sessions)
 }
 
+// IsSessionValid returns true if the specified session at index i
+// (in the range 0 <= i < NumSessions) is currently up and ready.
 func (c *Client) IsSessionValid(index int) bool {
 	if index < 0 || index >= len(c.sessions) {
 		return false
@@ -103,6 +113,8 @@ func (c *Client) IsSessionValid(index int) bool {
 	return s.isReady.Load()
 }
 
+// GetValidSession returns a random index i (in the range 0 <= i < NumSessions) ,
+// that is currently up and ready.
 func (c *Client) GetValidSession(ctx context.Context) (int, error) {
 	length := c.NumSessions()
 	index := rand.IntN(length)
@@ -124,6 +136,10 @@ func (c *Client) GetValidSession(ctx context.Context) (int, error) {
 	}
 }
 
+// RestartSession manually restarts a PPPoE session specified at the index i
+// (in the range 0 <= i < NumSessions), and if it's successful, it returns the
+// list of old IP addresses assigned to previous session that has been
+// replaced.
 func (c *Client) RestartSession(ctx context.Context, index int) (oldIPs []net.IP, err error) {
 	if index < 0 || index >= len(c.sessions) {
 		return nil, errors.New("invalid session index")

client/config.go

@@ -12,7 +12,7 @@ import (
 	"time"
 )
 
-// _varName is the placeholder in PPP IfName that will be replaced by client id
+// _varName is the placeholder in PPP IfName that will be replaced by client ID.
 const _varName = "@ID"
 
 func genStrFunc(s string, id int) string {
@@ -22,10 +22,10 @@ func genStrFunc(s string, id int) string {
 	return strings.ReplaceAll(s, _varName, strconv.Itoa(id))
 }
 
-// _defaultPPPIfNameTemplate is the default PPP interface name
+// _defaultPPPIfNameTemplate is the default PPP interface name.
 const _defaultPPPIfNameTemplate = "souppp@ID"
 
-// Setup holds common configuration for creating one or multiple Client sessions
+// Setup holds configuration for creating PPPoE sessions.
 type Setup struct {
 	Logger   *zerolog.Logger
 	LogLevel LoggingLvl
@@ -45,10 +45,10 @@ type Setup struct {
 	InterfaceName string
 	// Name of PPP interface created after successfully dialing, must contain @ID
 	PPPInterfaceName string
-	// RID is the BBF remote-id PPPoE tag
-	RID string
-	// CID is the BBF circuit-id PPPoE tag
-	CID string
+	// RemoteID is the BBF remote-id PPPoE tag
+	RemoteID string
+	// CircuitID is the BBF circuit-id PPPoE tag
+	CircuitID string
 	// AuthProto is the authentication protocol to use, e.g. lcp.ProtoCHAP or lcp.ProtoPAP
 	AuthProto ppp.ProtocolNumber
 	// InitialAuthIdentifier is the starting value for the incremental authentication Identifier
@@ -69,7 +69,7 @@ type Setup struct {
 }
 
 // DefaultSetup returns a basic default Setup with following defaults:
-// - no vlan, use the mac of interface ifname
+// - no vlan, use the MAC address of InterfaceName
 // - no debug mode
 // - 10 seconds dial timeout
 // - single client
@@ -86,6 +86,8 @@ func DefaultSetup() *Setup {
 	}
 }
 
+// Validate checks the validity of current configuration and adds missing
+// fields value when they have adeguate default values.
 func (setup *Setup) Validate() error {
 	if setup.Logger == nil {
 		logger, err := newDefaultLogger(setup.LogLevel)
@@ -122,6 +124,8 @@ func (setup *Setup) Validate() error {
 	return nil
 }
 
+// Clone creates a deep copy of the configuration, preparing it
+// for a new session at the target index position.
 func (setup *Setup) Clone(index int) *Setup {
 	return &Setup{
 		Logger:                setup.Logger,
@@ -133,8 +137,8 @@ func (setup *Setup) Clone(index int) *Setup {
 		MacStep:               setup.MacStep,
 		InterfaceName:         setup.InterfaceName,
 		PPPInterfaceName:      genStrFunc(setup.PPPInterfaceName, index),
-		RID:                   genStrFunc(setup.RID, index),
-		CID:                   genStrFunc(setup.CID, index),
+		RemoteID:              genStrFunc(setup.RemoteID, index),
+		CircuitID:             genStrFunc(setup.CircuitID, index),
 		AuthProto:             setup.AuthProto,
 		InitialAuthIdentifier: setup.InitialAuthIdentifier,
 		ConcurrentAuthRetries: setup.ConcurrentAuthRetries,
@@ -147,19 +151,19 @@ func (setup *Setup) Clone(index int) *Setup {
 	}
 }
 
-// LoggingLvl is the logging level of client
+// LoggingLvl is the logging level of client.
 type LoggingLvl uint
 
 const (
-	// LogLvlErr only log error msg
+	// LogLvlErr only log Error messages
 	LogLvlErr LoggingLvl = iota
-	// LogLvlInfo logs error + info msg
+	// LogLvlInfo logs Error + Info messages
 	LogLvlInfo
-	// LogLvlDebug logs error + info + debug msg
+	// LogLvlDebug logs Error + Info + Debug messages
 	LogLvlDebug
 )
 
-// newDefaultLogger create a default Logger with specified log level
+// newDefaultLogger create a default Logger with specified log level.
 func newDefaultLogger(logl LoggingLvl) (*zerolog.Logger, error) {
 	logger := zerolog.New(os.Stdout).Level(logLvlToZapLvl(logl))
 	return &logger, nil

client/dhcp6.go

@@ -12,14 +12,14 @@ import (
 	"time"
 )
 
-// DHCP6Cfg holds configuration for DHCP6Clnterface
+// DHCP6Cfg holds configuration for DHCPv6.
 type DHCP6Cfg struct {
 	Mac            net.HardwareAddr
 	NeedPD, NeedNA bool
 	Debug          bool
 }
 
-// DHCP6Clnterface is a DHCPv6 client
+// DHCP6Clnterface is a DHCPv6 client.
 type DHCP6Clnterface struct {
 	client        *nclient6.Client
 	cfg           DHCP6Cfg
@@ -46,7 +46,7 @@ func NewDHCP6Clnt(conn net.PacketConn, cfg DHCP6Cfg) (*DHCP6Clnterface, error) {
 	}, nil
 }
 
-// Dial completes a DHCPv6 exchange with server
+// Dial completes a DHCPv6 exchange.
 func (dc *DHCP6Clnterface) Dial(ctx context.Context) error {
 	solicitMsg, err := dc.buildSolicit()
 	if err != nil {

client/mac.go

@@ -14,7 +14,7 @@ const (
 	_maximumMacAddr = (1 << (_macMaxBytes * 8)) - 1
 )
 
-// incrementMACAddress increment macaddr by step value (can be negative and return the result
+// incrementMACAddress increment MAC address value by step value (can be negative), and return the result
 func incrementMACAddress(macaddr net.HardwareAddr, step int64) (net.HardwareAddr, error) {
 	length := len(macaddr)
 	if length > _macMaxBytes {

client/session.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"github.com/gandalfast/souppp/auth"
 	"github.com/gandalfast/souppp/auth/chap"
 	"github.com/gandalfast/souppp/auth/pap"
 	"github.com/gandalfast/souppp/datapath"
@@ -66,8 +67,8 @@ func newSession(index int, cfg *Setup, relay etherconn.PacketRelay, blacklist lc
 	}
 
 	var tagList []pppoe.Tag
-	if cfg.CID != "" || cfg.RID != "" {
-		tagList = append(tagList, pppoe.NewCircuitRemoteIDTag(cfg.CID, cfg.RID))
+	if cfg.CircuitID != "" || cfg.RemoteID != "" {
+		tagList = append(tagList, pppoe.NewCircuitRemoteIDTag(cfg.CircuitID, cfg.RemoteID))
 	}
 
 	s.pppoeProto = pppoe.NewPPPoE(etherConn, cfg.Logger, pppoe.WithTags(tagList))
@@ -176,23 +177,14 @@ func (s *session) lcpEvtHandler(evt lcp.LayerNotifyEvent) {
 
 		startingAuthTime := time.Now()
 		authProto := opauthlist[0].(*lcp.OpAuthProto).Proto
-		switch authProto {
-		case ppp.ProtoCHAP:
-			chapProto := chap.NewCHAP(s.pppProto)
-			if err := chapProto.AuthSelf(ctx, s.cfg.UserName, s.cfg.Password); err != nil {
-				s.cfg.Logger.Error().Err(err).Msg("auth failed")
-				_ = s.Close()
-				return
-			}
-		case ppp.ProtoPAP:
-			papProto := pap.NewPAP(s.pppProto, s.cfg.InitialAuthIdentifier, s.cfg.ConcurrentAuthRetries)
-			if err := papProto.AuthSelf(ctx, s.cfg.UserName, s.cfg.Password); err != nil {
-				s.cfg.Logger.Error().Err(err).Msg("auth failed")
-				_ = s.Close()
-				return
-			}
-		default:
-			s.cfg.Logger.Error().Msgf("unkown auth method negoatied %v", authProto)
+		authenticator, err := s.getAuthHandler(authProto)
+		if err != nil {
+			s.cfg.Logger.Error().Err(err).Msg("unable to obtain authenticator")
+			_ = s.Close()
+			return
+		}
+		if err := authenticator.AuthSelf(ctx, s.cfg.UserName, s.cfg.Password); err != nil {
+			s.cfg.Logger.Error().Err(err).Msg("auth failed")
 			_ = s.Close()
 			return
 		}
@@ -229,6 +221,17 @@ func (s *session) lcpEvtHandler(evt lcp.LayerNotifyEvent) {
 	}
 }
 
+func (s *session) getAuthHandler(authProto ppp.ProtocolNumber) (auth.Authenticator, error) {
+	switch authProto {
+	case ppp.ProtoCHAP:
+		return chap.NewCHAP(s.pppProto), nil
+	case ppp.ProtoPAP:
+		return pap.NewPAP(s.pppProto, s.cfg.InitialAuthIdentifier, s.cfg.ConcurrentAuthRetries), nil
+	default:
+		return nil, errors.New("unkown auth method negoatied " + authProto.String())
+	}
+}
+
 func (s *session) ipcpEvtHandler(evt lcp.LayerNotifyEvent) {
 	s.cfg.Logger.Info().Msgf("IPCP layer %v", evt)
 	switch evt {

datapath/datapath_linux.go

@@ -15,7 +15,7 @@ import (
 	"net"
 )
 
-// TUNInterface is the TUN interface for a opened PPP session
+// TUNInterface is the TUN interface for an opened PPP session.
 type TUNInterface struct {
 	logger                 *zerolog.Logger
 	pppProto               *ppp.PPP
@@ -103,7 +103,7 @@ func (tif *TUNInterface) Close() error {
 	return tif.netInterface.Close()
 }
 
-// send pkt to outside network
+// send sends PPP packet to outside network
 func (tif *TUNInterface) send(ctx context.Context) {
 	for {
 		// Read IPv4 / IPv6 packet to send from TUN interface

ppp/lcp/blacklist.go

@@ -12,38 +12,42 @@ type Blacklist interface {
 }
 
 type DefaultBlacklist struct {
-	blacklist map[string]struct{}
+	blacklist map[[16]byte]struct{}
 	mtx       sync.RWMutex
 }
 
 func NewDefaultBlacklist() *DefaultBlacklist {
 	return &DefaultBlacklist{
-		blacklist: make(map[string]struct{}),
+		blacklist: make(map[[16]byte]struct{}),
 	}
 }
 
+// Add adds an IP address that needs to be skipped
+// if assigned by the AC.
 func (b *DefaultBlacklist) Add(ip []net.IP) error {
 	b.mtx.Lock()
 	for _, address := range ip {
-		b.blacklist[address.String()] = struct{}{}
+		b.blacklist[[16]byte(address.To16()[:16])] = struct{}{}
 	}
 	b.mtx.Unlock()
 	return nil
 }
 
+// Remove removes an IP address from existing blacklist, if present.
 func (b *DefaultBlacklist) Remove(ip []net.IP) error {
 	b.mtx.Lock()
 	for _, address := range ip {
-		delete(b.blacklist, address.String())
+		delete(b.blacklist, [16]byte(address.To16()[:16]))
 	}
 	b.mtx.Unlock()
 	return nil
 }
 
+// IsValid returns true if the specified IP is not in the blacklist.
 func (b *DefaultBlacklist) IsValid(ip net.IP) bool {
 	b.mtx.RLock()
 	defer b.mtx.RUnlock()
 
-	_, blacklisted := b.blacklist[ip.String()]
+	_, blacklisted := b.blacklist[[16]byte(ip.To16()[:16])]
 	return !blacklisted
 }