Add fixes for punt plugin (#1389)

- do not check socket path for punt exceptions to prevent recreation - add depenency for RX interface to ip redirect punt - implement IP redirect dump (only for 19.08) - add workaround for publishing socket to punt exceptions as well - properly deregister punt exeption on delete - return pathname from registration when adding punt exception - add workaround with ! prefix to punt exceptions - publish socket paths to status with client TTL Signed-off-by: Ondrej Fabry <[email protected]>
ligato · Jun 18, 2019 · 85f7a75 · 85f7a75
1 parent c13e83b
commit 85f7a75
Show file tree

Hide file tree

Showing 12 changed files with 256 additions and 124 deletions.
diff --git a/plugins/vpp/puntplugin/descriptor/ip_punt_redirect.go b/plugins/vpp/puntplugin/descriptor/ip_punt_redirect.go
@@ -19,8 +19,10 @@ import (
 
 	"github.com/gogo/protobuf/proto"
 	"github.com/ligato/cn-infra/logging"
+
 	interfaces "github.com/ligato/vpp-agent/api/models/vpp/interfaces"
 	punt "github.com/ligato/vpp-agent/api/models/vpp/punt"
+	"github.com/ligato/vpp-agent/pkg/models"
 	kvs "github.com/ligato/vpp-agent/plugins/kvscheduler/api"
 	"github.com/ligato/vpp-agent/plugins/vpp/puntplugin/descriptor/adapter"
 	"github.com/ligato/vpp-agent/plugins/vpp/puntplugin/vppcalls"
@@ -32,6 +34,7 @@ const (
 
 	// dependency labels
 	ipRedirectTxInterfaceDep = "tx-interface-exists"
+	ipRedirectRxInterfaceDep = "rx-interface-exists"
 )
 
 // A list of non-retriable errors:
@@ -130,16 +133,36 @@ func (d *IPRedirectDescriptor) Delete(key string, redirect *punt.IPRedirect, met
 
 // Retrieve returns all configured VPP punt to host entries.
 func (d *IPRedirectDescriptor) Retrieve(correlate []adapter.IPPuntRedirectKVWithMetadata) (dump []adapter.IPPuntRedirectKVWithMetadata, err error) {
-	// TODO dump for IP redirect missing in api
-	d.log.Info("Dump IP punt redirect is not supported by the VPP")
-	return []adapter.IPPuntRedirectKVWithMetadata{}, nil
+	punts, err := d.puntHandler.DumpPuntRedirect()
+	if err != nil {
+		d.log.Error(err)
+		return nil, err
+	}
+
+	for _, p := range punts {
+		dump = append(dump, adapter.IPPuntRedirectKVWithMetadata{
+			Key:    models.Key(p),
+			Value:  p,
+			Origin: kvs.FromNB,
+		})
+	}
+
+	return dump, nil
 }
 
 // Dependencies for IP punt redirect are represented by tx interface
 func (d *IPRedirectDescriptor) Dependencies(key string, redirect *punt.IPRedirect) (dependencies []kvs.Dependency) {
+	// TX interface
 	dependencies = append(dependencies, kvs.Dependency{
 		Label: ipRedirectTxInterfaceDep,
 		Key:   interfaces.InterfaceKey(redirect.TxInterface),
 	})
+	// RX interface
+	if redirect.RxInterface != "" {
+		dependencies = append(dependencies, kvs.Dependency{
+			Label: ipRedirectRxInterfaceDep,
+			Key:   interfaces.InterfaceKey(redirect.RxInterface),
+		})
+	}
 	return dependencies
 }
diff --git a/plugins/vpp/puntplugin/descriptor/punt_exception.go b/plugins/vpp/puntplugin/descriptor/punt_exception.go
@@ -16,8 +16,8 @@ package descriptor
 
 import (
 	"errors"
+	"strings"
 
-	"github.com/gogo/protobuf/proto"
 	"github.com/ligato/cn-infra/logging"
 
 	punt "github.com/ligato/vpp-agent/api/models/vpp/punt"
@@ -40,6 +40,8 @@ var (
 
 // PuntExceptionDescriptor teaches KVScheduler how to configure VPP putn exception.
 type PuntExceptionDescriptor struct {
+	RegisterSocketFn func(register bool, toHost *punt.Exception, socketPath string)
+
 	// dependencies
 	log         logging.Logger
 	puntHandler vppcalls.PuntVppAPI
@@ -72,7 +74,17 @@ func (d *PuntExceptionDescriptor) GetDescriptor() *adapter.PuntExceptionDescript
 
 // EquivalentPuntException is case-insensitive comparison function for punt.Exception.
 func (d *PuntExceptionDescriptor) EquivalentPuntException(key string, oldPunt, newPunt *punt.Exception) bool {
-	return proto.Equal(oldPunt, newPunt)
+	if oldPunt.Reason != newPunt.Reason {
+		return false
+	}
+
+	// if the socket path contains '!' as prefix we return false
+	// to force scheduler to recreate (register) punt socket
+	if strings.HasPrefix(oldPunt.SocketPath, "!") {
+		return false
+	}
+
+	return true
 }
 
 // Validate validates VPP punt configuration.
@@ -82,30 +94,49 @@ func (d *PuntExceptionDescriptor) Validate(key string, puntCfg *punt.Exception)
 		return ErrPuntExceptionWithoutReason
 	}
 
+	if puntCfg.SocketPath == "" {
+		return kvs.NewInvalidValueError(ErrPuntWithoutSocketPath, "socket_path")
+	}
+
 	return nil
 }
 
 // Create adds new punt to host entry or registers new punt to unix domain socket.
 func (d *PuntExceptionDescriptor) Create(key string, punt *punt.Exception) (interface{}, error) {
-	// add punt exception
-	err := d.puntHandler.AddPuntException(punt)
+	// register punt exception
+	pathname, err := d.puntHandler.AddPuntException(punt)
 	if err != nil {
 		d.log.Error(err)
 		return nil, err
 	}
 
+	if d.RegisterSocketFn != nil {
+		d.RegisterSocketFn(true, punt, pathname)
+	}
+
 	return nil, nil
 }
 
 // Delete removes VPP punt configuration.
 func (d *PuntExceptionDescriptor) Delete(key string, punt *punt.Exception, metadata interface{}) error {
+	// check if the socketpath contains '!' as prefix from retrieve
+	p := punt
+	if strings.HasPrefix(p.SocketPath, "!") {
+		p = &(*punt)
+		p.SocketPath = strings.TrimPrefix(p.SocketPath, "!")
+	}
+
 	// delete punt exception
 	err := d.puntHandler.DeletePuntException(punt)
 	if err != nil {
 		d.log.Error(err)
 		return err
 	}
 
+	if d.RegisterSocketFn != nil {
+		d.RegisterSocketFn(false, punt, "")
+	}
+
 	return nil
 }
 
@@ -117,6 +148,16 @@ func (d *PuntExceptionDescriptor) Retrieve(correlate []adapter.PuntExceptionKVWi
 		return nil, err
 	}
 
+	// for all dumped punts that were not yet registered and for which
+	// the VPP socket is unknown we prepend '!' as prefix
+	// to allow descriptor to recognize this in equivalent
+	// and force recreation or make it possible to delete it
+	for _, p := range punts {
+		if p.Exception.SocketPath == "" && p.SocketPath != "" {
+			p.Exception.SocketPath = "!" + p.SocketPath
+		}
+	}
+
 	for _, p := range punts {
 		retrieved = append(retrieved, adapter.PuntExceptionKVWithMetadata{
 			Key:    models.Key(p.Exception),

diff --git a/plugins/vpp/puntplugin/descriptor/punt_to_host.go b/plugins/vpp/puntplugin/descriptor/punt_to_host.go
@@ -89,6 +89,8 @@ func (d *PuntToHostDescriptor) EquivalentPuntToHost(key string, oldPunt, newPunt
 		return false
 	}
 
+	// if the socket path contains '!' as prefix we return false
+	// to force scheduler to recreate (register) punt socket
 	if strings.HasPrefix(oldPunt.SocketPath, "!") {
 		return false
 	}
@@ -119,6 +121,7 @@ func (d *PuntToHostDescriptor) Validate(key string, puntCfg *punt.ToHost) error
 		return kvs.NewInvalidValueError(ErrPuntWithoutPort, "port")
 	}
 
+	// TODO: maybe this should also have dependency on socket file existing??
 	if puntCfg.SocketPath == "" {
 		return kvs.NewInvalidValueError(ErrPuntWithoutSocketPath, "socket_path")
 	}
@@ -128,14 +131,6 @@ func (d *PuntToHostDescriptor) Validate(key string, puntCfg *punt.ToHost) error
 
 // Create adds new punt to host entry or registers new punt to unix domain socket.
 func (d *PuntToHostDescriptor) Create(key string, punt *punt.ToHost) (interface{}, error) {
-	/*if punt.SocketPath == "" {
-		if err := d.puntHandler.AddPunt(punt); err != nil {
-			d.log.Error(err)
-			return nil, err
-		}
-		return nil, nil
-	}*/
-
 	// register punt to socket
 	pathname, err := d.puntHandler.RegisterPuntSocket(punt)
 	if err != nil {
@@ -152,12 +147,7 @@ func (d *PuntToHostDescriptor) Create(key string, punt *punt.ToHost) (interface{
 
 // Delete removes VPP punt configuration.
 func (d *PuntToHostDescriptor) Delete(key string, punt *punt.ToHost, metadata interface{}) error {
-	/*if punt.SocketPath == "" {
-		if err := d.puntHandler.DeletePunt(punt); err != nil {
-			d.log.Error(err)
-			return err
-		}
-	}*/
+	// check if the socketpath contains '!' as prefix from retrieve
 	p := punt
 	if strings.HasPrefix(p.SocketPath, "!") {
 		p = &(*punt)
@@ -185,6 +175,10 @@ func (d *PuntToHostDescriptor) Retrieve(correlate []adapter.PuntToHostKVWithMeta
 		return nil, err
 	}
 
+	// for all dumped punts that were not yet registered and for which
+	// the VPP socket is unknown we prepend '!' as prefix
+	// to allow descriptor to recognize this in equivalent
+	// and force recreation or make it possible to delete it
 	for _, s := range socks {
 		if s.PuntData.SocketPath == "" && s.SocketPath != "" {
 			s.PuntData.SocketPath = "!" + s.SocketPath

diff --git a/plugins/vpp/puntplugin/options.go b/plugins/vpp/puntplugin/options.go
@@ -16,6 +16,7 @@ package puntplugin
 
 import (
 	"github.com/ligato/cn-infra/logging"
+
 	"github.com/ligato/vpp-agent/plugins/govppmux"
 	"github.com/ligato/vpp-agent/plugins/kvscheduler"
 	"github.com/ligato/vpp-agent/plugins/vpp/ifplugin"

diff --git a/plugins/vpp/puntplugin/puntplugin.go b/plugins/vpp/puntplugin/puntplugin.go
@@ -101,7 +101,7 @@ func (p *PuntPlugin) Init() (err error) {
 		return err
 	}
 
-	// TODO: temporary workaround for publishing registered sockets
+	// FIXME: temporary workaround for publishing registered sockets
 	p.toHostDescriptor.RegisterSocketFn = func(register bool, toHost *vpp_punt.ToHost, socketPath string) {
 		if p.PublishState == nil {
 			return
@@ -110,12 +110,29 @@ func (p *PuntPlugin) Init() (err error) {
 		if register {
 			puntToHost := *toHost
 			puntToHost.SocketPath = socketPath
-			if err := p.PublishState.Put(key, &puntToHost); err != nil {
-				p.Log.Errorf("publishing registered socket failed: %v", err)
+			if err := p.PublishState.Put(key, &puntToHost, datasync.WithClientLifetimeTTL()); err != nil {
+				p.Log.Errorf("publishing registered punt socket failed: %v", err)
 			}
 		} else {
 			if err := p.PublishState.Put(key, nil); err != nil {
-				p.Log.Errorf("publishing unregistered socket failed: %v", err)
+				p.Log.Errorf("publishing unregistered punt socket failed: %v", err)
+			}
+		}
+	}
+	p.puntExceptionDescriptor.RegisterSocketFn = func(register bool, puntExc *vpp_punt.Exception, socketPath string) {
+		if p.PublishState == nil {
+			return
+		}
+		key := strings.Replace(models.Key(puntExc), "config/", "status/", -1)
+		if register {
+			punt := *puntExc
+			punt.SocketPath = socketPath
+			if err := p.PublishState.Put(key, &punt, datasync.WithClientLifetimeTTL()); err != nil {
+				p.Log.Errorf("publishing registered punt exception socket failed: %v", err)
+			}
+		} else {
+			if err := p.PublishState.Put(key, nil); err != nil {
+				p.Log.Errorf("publishing unregistered punt exception socket failed: %v", err)
 			}
 		}
 	}

diff --git a/plugins/vpp/puntplugin/vppcalls/punt_vppcalls.go b/plugins/vpp/puntplugin/vppcalls/punt_vppcalls.go
@@ -22,19 +22,22 @@ import (
 	"github.com/ligato/vpp-agent/plugins/vpp/ifplugin/ifaceidx"
 )
 
-// PuntDetails includes proto-modelled punt object and its socket path
+// PuntDetails includes punt model and socket path from VPP.
 type PuntDetails struct {
 	PuntData   *punt.ToHost
 	SocketPath string
 }
 
+// ReasonDetails includes reason model and its matching ID from VPP.
 type ReasonDetails struct {
 	Reason *punt.Reason
 	ID     uint32
 }
 
+// ExceptionDetails include punt model and socket path from VPP.
 type ExceptionDetails struct {
-	Exception *punt.Exception
+	Exception  *punt.Exception
+	SocketPath string
 }
 
 // PuntVppAPI provides methods for managing VPP punt configuration.
@@ -53,9 +56,9 @@ type PuntVppAPI interface {
 	AddPuntRedirect(punt *punt.IPRedirect) error
 	// DeletePuntRedirect removes existing redirect entry
 	DeletePuntRedirect(punt *punt.IPRedirect) error
-	// AddPuntException configures new punt exception
-	AddPuntException(punt *punt.Exception) error
-	// DeletePuntException removes punt exception entry
+	// AddPuntException registers new punt exception
+	AddPuntException(punt *punt.Exception) (string, error)
+	// DeletePuntException deregisters punt exception entry
 	DeletePuntException(punt *punt.Exception) error
 }
 
@@ -67,6 +70,8 @@ type PuntVPPRead interface {
 	DumpExceptions() ([]*ExceptionDetails, error)
 	// DumpPuntReasons returns all known punt reasons from VPP
 	DumpPuntReasons() ([]*ReasonDetails, error)
+	// DumpPuntRedirect dump IP redirect punts
+	DumpPuntRedirect() ([]*punt.IPRedirect, error)
 }
 
 var Versions = map[string]HandlerVersion{}

diff --git a/plugins/vpp/puntplugin/vppcalls/vpp1901/dump_vppcalls.go b/plugins/vpp/puntplugin/vppcalls/vpp1901/dump_vppcalls.go
@@ -24,6 +24,11 @@ import (
 // FIXME: temporary solutions for providing data in dump
 var socketPathMap = make(map[uint32]*vpp.PuntToHost)
 
+func (h *PuntVppHandler) DumpPuntRedirect() (punts []*vpp_punt.IPRedirect, err error) {
+	h.log.Debugf("dump for ip redirect punt not implemented")
+	return nil, nil
+}
+
 // DumpRegisteredPuntSockets returns punt to host via registered socket entries
 // TODO since the binary API is not available, all data are read from local cache for now
 func (h *PuntVppHandler) DumpRegisteredPuntSockets() (punts []*vppcalls.PuntDetails, err error) {

diff --git a/plugins/vpp/puntplugin/vppcalls/vpp1901/punt_vppcalls.go b/plugins/vpp/puntplugin/vppcalls/vpp1901/punt_vppcalls.go
@@ -79,10 +79,6 @@ func (h *PuntVppHandler) RegisterPuntSocket(toHost *punt.ToHost) (string, error)
 	p.SocketPath = strings.SplitN(string(reply.Pathname), "\x00", 2)[0]
 	socketPathMap[toHost.Port] = &p
 
-	/*if h.RegisterSocketFn != nil {
-		h.RegisterSocketFn(true, toHost, p.SocketPath)
-	}*/
-
 	return p.SocketPath, nil
 }
 
@@ -101,12 +97,6 @@ func (h *PuntVppHandler) DeregisterPuntSocket(toHost *punt.ToHost) error {
 		return err
 	}
 
-	/*if h.RegisterSocketFn != nil {
-		if p, ok := socketPathMap[toHost.Port]; ok {
-			h.RegisterSocketFn(false, toHost, p.SocketPath)
-		}
-	}*/
-
 	delete(socketPathMap, toHost.Port)
 
 	return nil
@@ -247,8 +237,8 @@ func boolToUint(input bool) uint8 {
 	return 0
 }
 
-func (h *PuntVppHandler) AddPuntException(punt *punt.Exception) error {
-	return fmt.Errorf("punt exceptions are not supported")
+func (h *PuntVppHandler) AddPuntException(punt *punt.Exception) (string, error) {
+	return "", fmt.Errorf("punt exceptions are not supported")
 }
 
 func (h *PuntVppHandler) DeletePuntException(punt *punt.Exception) error {

diff --git a/plugins/vpp/puntplugin/vppcalls/vpp1904/dump_vppcalls.go b/plugins/vpp/puntplugin/vppcalls/vpp1904/dump_vppcalls.go
@@ -24,6 +24,11 @@ import (
 // FIXME: temporary solutions for providing data in dump
 var socketPathMap = make(map[uint32]*vpp.PuntToHost)
 
+func (h *PuntVppHandler) DumpPuntRedirect() (punts []*vpp_punt.IPRedirect, err error) {
+	h.log.Debugf("dump for ip redirect punt not implemented")
+	return nil, nil
+}
+
 // DumpRegisteredPuntSockets returns punt to host via registered socket entries
 // TODO since the binary API is not available, all data are read from local cache for now
 func (h *PuntVppHandler) DumpRegisteredPuntSockets() (punts []*vppcalls.PuntDetails, err error) {