veyron/runtimes/google/ipc: fix a bug in caching blessings.
o The bug makes a client to send its blessing for every new flow
without using a cached one causing the server validating the same
blessing repeatedly.
o IPC benchmark 'Benchmark___10B' shows the latency of a single Echo call
is reduced from 8.03ms to 5.66ms.
o Also fixed a bug in using blessings instance (pointer) as cache key,
which makes the cache under utilized.
Change-Id: Ibda83271236638a0c4aad550480fd78a11347191
diff --git a/runtimes/google/ipc/blessings_cache.go b/runtimes/google/ipc/blessings_cache.go
index effa8e8..119c3db 100644
--- a/runtimes/google/ipc/blessings_cache.go
+++ b/runtimes/google/ipc/blessings_cache.go
@@ -1,6 +1,7 @@
package ipc
import (
+ "crypto/sha256"
"fmt"
"reflect"
"sync"
@@ -12,7 +13,7 @@
// clientEncodeBlessings gets or inserts the blessings into the cache.
func clientEncodeBlessings(cache stream.VCDataCache, blessings security.Blessings) ipc.BlessingsRequest {
- blessingsCacheAny := cache.GetOrInsert(clientBlessingsKey{}, newClientBlessingsCache)
+ blessingsCacheAny := cache.GetOrInsert(clientBlessingsCacheKey{}, newClientBlessingsCache)
blessingsCache := blessingsCacheAny.(*clientBlessingsCache)
return blessingsCache.getOrInsert(blessings)
}
@@ -20,7 +21,7 @@
// clientAckBlessings verifies that the server has updated its cache to include blessings.
// This means that subsequent rpcs from the client with blessings can send only a cache key.
func clientAckBlessings(cache stream.VCDataCache, blessings security.Blessings) {
- blessingsCacheAny := cache.GetOrInsert(clientBlessingsKey{}, newClientBlessingsCache)
+ blessingsCacheAny := cache.GetOrInsert(clientBlessingsCacheKey{}, newClientBlessingsCache)
blessingsCache := blessingsCacheAny.(*clientBlessingsCache)
blessingsCache.acknowledge(blessings)
}
@@ -28,77 +29,94 @@
// serverDecodeBlessings insert the key and blessings into the cache or get the blessings if only
// key is provided in req.
func serverDecodeBlessings(cache stream.VCDataCache, req ipc.BlessingsRequest, stats *ipcStats) (security.Blessings, error) {
- blessingsCacheAny := cache.GetOrInsert(serverBlessingsKey{}, newServerBlessingsCache)
+ blessingsCacheAny := cache.GetOrInsert(serverBlessingsCacheKey{}, newServerBlessingsCache)
blessingsCache := blessingsCacheAny.(*serverBlessingsCache)
return blessingsCache.getOrInsert(req, stats)
}
// IMPLEMENTATION DETAILS BELOW
-// clientBlessingsCache is a thread-safe map from blessings to cache key.
+// clientBlessingsCache is a thread-safe map from blessings to cache id.
type clientBlessingsCache struct {
sync.RWMutex
- m map[security.Blessings]clientCacheValue
- key uint64
+ m map[[sha256.Size]byte]clientCacheValue
+ curId uint64
}
type clientCacheValue struct {
- key uint64
- // ack is set to true once the server has confirmed receipt of the cache key.
- // Clients that insert into the cache when ack is false must send both the key
+ id uint64
+ // ack is set to true once the server has confirmed receipt of the cache id.
+ // Clients that insert into the cache when ack is false must send both the id
// and the blessings.
ack bool
}
-// clientBlessingsKey is the key used to retrieve the clientBlessingsCache from the VCDataCache.
-type clientBlessingsKey struct{}
+// clientBlessingsCacheKey is the key used to retrieve the clientBlessingsCache from the VCDataCache.
+type clientBlessingsCacheKey struct{}
func newClientBlessingsCache() interface{} {
- return &clientBlessingsCache{m: make(map[security.Blessings]clientCacheValue)}
+ return &clientBlessingsCache{m: make(map[[sha256.Size]byte]clientCacheValue)}
+}
+
+func getBlessingsHashKey(blessings security.Blessings) (key [sha256.Size]byte) {
+ h := sha256.New()
+ for _, chain := range security.MarshalBlessings(blessings).CertificateChains {
+ if len(chain) == 0 {
+ continue
+ }
+ cert := chain[len(chain)-1]
+ s := sha256.Sum256(cert.Signature.R)
+ h.Write(s[:])
+ s = sha256.Sum256(cert.Signature.S)
+ h.Write(s[:])
+ }
+ copy(key[:], h.Sum(nil))
+ return
}
func (c *clientBlessingsCache) getOrInsert(blessings security.Blessings) ipc.BlessingsRequest {
+ key := getBlessingsHashKey(blessings)
c.RLock()
- val, exists := c.m[blessings]
+ val, exists := c.m[key]
c.RUnlock()
if exists {
return c.makeBlessingsRequest(val, blessings)
}
- // if the val doesn't exist we must create a new key, update the cache, and send the key and blessings.
+ // If the val doesn't exist we must create a new id, update the cache, and send the id and blessings.
c.Lock()
- defer c.Unlock()
- // we must check that the val wasn't inserted in the time we changed locks.
- val, exists = c.m[blessings]
- if exists {
- return c.makeBlessingsRequest(val, blessings)
+ // We must check that the val wasn't inserted in the time we changed locks.
+ val, exists = c.m[key]
+ if !exists {
+ val = clientCacheValue{id: c.nextIdLocked()}
+ c.m[key] = val
}
- newVal := clientCacheValue{key: c.nextKeyLocked()}
- c.m[blessings] = newVal
- return c.makeBlessingsRequest(newVal, blessings)
+ c.Unlock()
+ return c.makeBlessingsRequest(val, blessings)
}
func (c *clientBlessingsCache) acknowledge(blessings security.Blessings) {
+ key := getBlessingsHashKey(blessings)
c.Lock()
- val := c.m[blessings]
+ val := c.m[key]
val.ack = true
- c.m[blessings] = val
+ c.m[key] = val
c.Unlock()
}
func (c *clientBlessingsCache) makeBlessingsRequest(val clientCacheValue, blessings security.Blessings) ipc.BlessingsRequest {
if val.ack {
// when the value is acknowledged, only send the key, since the server has confirmed that it knows the key.
- return ipc.BlessingsRequest{Key: val.key}
+ return ipc.BlessingsRequest{Key: val.id}
}
// otherwise we still need to send both key and blessings, but we must ensure that we send the same key.
wireBlessings := security.MarshalBlessings(blessings)
- return ipc.BlessingsRequest{val.key, &wireBlessings}
+ return ipc.BlessingsRequest{val.id, &wireBlessings}
}
-// nextKeyLocked creates a new key for inserting blessings. It must be called after acquiring a writer lock.
-func (c *clientBlessingsCache) nextKeyLocked() uint64 {
- c.key++
- return c.key
+// nextIdLocked creates a new id for inserting blessings. It must be called after acquiring a writer lock.
+func (c *clientBlessingsCache) nextIdLocked() uint64 {
+ c.curId++
+ return c.curId
}
// serverBlessingsCache is a thread-safe map from cache key to blessings.
@@ -107,8 +125,8 @@
m map[uint64]security.Blessings
}
-// serverBlessingsKey is the key used to retrieve the serverBlessingsCache from the VCDataCache.
-type serverBlessingsKey struct{}
+// serverBlessingsCacheKey is the key used to retrieve the serverBlessingsCache from the VCDataCache.
+type serverBlessingsCacheKey struct{}
func newServerBlessingsCache() interface{} {
return &serverBlessingsCache{m: make(map[uint64]security.Blessings)}
diff --git a/runtimes/google/ipc/client.go b/runtimes/google/ipc/client.go
index 6ef039b..a472e68 100644
--- a/runtimes/google/ipc/client.go
+++ b/runtimes/google/ipc/client.go
@@ -736,8 +736,8 @@
// Encode the Blessings information for the client to authorize the flow.
var blessingsRequest ipc.BlessingsRequest
if fc.flow.LocalPrincipal() != nil {
- localBlessings := fc.flow.LocalPrincipal().BlessingStore().ForPeer(fc.server...)
- blessingsRequest = clientEncodeBlessings(fc.flow.VCDataCache(), localBlessings)
+ fc.blessings = fc.flow.LocalPrincipal().BlessingStore().ForPeer(fc.server...)
+ blessingsRequest = clientEncodeBlessings(fc.flow.VCDataCache(), fc.blessings)
}
// TODO(suharshs, ataly): Make security.Discharge a vdl type.
anyDischarges := make([]vdlutil.Any, len(fc.discharges))
diff --git a/runtimes/google/ipc/server.go b/runtimes/google/ipc/server.go
index 432242e..e9b7066 100644
--- a/runtimes/google/ipc/server.go
+++ b/runtimes/google/ipc/server.go
@@ -1118,7 +1118,9 @@
fs.server.streamMgr.ShutdownEndpoint(fs.RemoteEndpoint())
return old_verror.BadProtocolf("ipc: blessings cache failed: %v", err)
}
- fs.ackBlessings = true
+ if fs.clientBlessings != nil {
+ fs.ackBlessings = true
+ }
// TODO(suharshs, ataly): Make security.Discharge a vdl type.
for i, d := range req.Discharges {
