You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by am...@apache.org on 2011/02/11 00:20:31 UTC
svn commit: r1069608 - in /trafficserver/traffic/trunk/lib/wccp:
WccpEndPoint.cc WccpLocal.h WccpMsg.cc
Author: amc
Date: Thu Feb 10 23:20:31 2011
New Revision: 1069608
URL: http://svn.apache.org/viewvc?rev=1069608&view=rev
Log:
TS-663: additional fixes
Modified:
trafficserver/traffic/trunk/lib/wccp/WccpEndPoint.cc
trafficserver/traffic/trunk/lib/wccp/WccpLocal.h
trafficserver/traffic/trunk/lib/wccp/WccpMsg.cc
Modified: trafficserver/traffic/trunk/lib/wccp/WccpEndPoint.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/lib/wccp/WccpEndPoint.cc?rev=1069608&r1=1069607&r2=1069608&view=diff
==============================================================================
--- trafficserver/traffic/trunk/lib/wccp/WccpEndPoint.cc (original)
+++ trafficserver/traffic/trunk/lib/wccp/WccpEndPoint.cc Thu Feb 10 23:20:31 2011
@@ -505,34 +505,47 @@ CacheImpl::generateRedirectAssign(
msg.finalize();
}
-bool
+ts::Errata
CacheImpl::checkRouterAssignment(
GroupData const& group,
RouterViewComp const& comp
) const {
+ detail::Assignment const& ainfo = group.m_assign_info;
// If group doesn't have an active assignment, always match w/o checking.
- bool zret = ! group.m_assign_info.isActive();
- if (! zret
- && ! comp.isEmpty()
- && ServiceGroup::HASH_ONLY == group.m_cache_assign
- ) {
+ ts::Errata zret; // default is success.
+
+ logf(LVL_TMP, "Checking router assignment - %s %lx %lu/%lu",
+ ainfo.isActive() ? "active" : "inactive",
+ ainfo.getKey().getAddr(),
+ ainfo.getKey().getChangeNumber(),
+ comp.isEmpty() ? 0 : comp.getChangeNumber()
+ );
+ // if active assignment and data we can check, then check.
+ if (ainfo.isActive() && ! comp.isEmpty()) {
+ // Validate the assignment key.
+ if (ainfo.getKey().getAddr() != comp.getKeyAddr()
+ || ainfo.getKey().getChangeNumber() != comp.getChangeNumber()
+ ) {
+ log(zret, LVL_INFO, "Router assignment key did not match.");;
+ } else if (ServiceGroup::HASH_ONLY == group.m_cache_assign
+ ) {
+ // Still not sure how much checking we really want or should
+ // do here. Note that we have to handle the case where the
+ // router sends mask cache ID elements but we expect hash.
// uint32_t nc = comp.getCacheCount();
// HashAssignElt const& ha = *(group.m_assign_info.getHash());
// Nothing for it but simple brute force - iterate over every bucket
// in the assignment and verify the corresponding bucket in the
// cache bit array is set.
- zret = true;
// TBD: FIX THIS TO CHECK HASH BUCKETS!
// Need to fix up RouterViewComp extensively to make this work.
# if 0
for ( size_t b = 0 ; zret && b < N_BUCKETS ; ++b ) {
unsigned int c = ha[b].m_idx;
if (c >= nc || ! comp.cacheElt(c).getBucket(b)) zret = false;
- }
# endif
- } else {
- // TBD: Validate mask assignments. For now, always OK.
- zret = true;
+ } else if (ServiceGroup::MASK_ONLY == group.m_cache_assign) {
+ } // else still pending, can't be mismatch yet.
}
return zret;
}
@@ -751,9 +764,10 @@ CacheImpl::handleISeeYou(IpHeader const&
// Existing router. Update the receive ID in the assignment object.
group.m_assign_info.updateRouterId(router_addr, recv_id, msg.m_router_view.getChangeNumber());
// Check the assignment to see if we need to send it again.
- if (!this->checkRouterAssignment(group, msg.m_router_view)) {
+ ts::Errata status = this->checkRouterAssignment(group, msg.m_router_view);
+ if (status.size()) {
ar_spot->m_assign = true; // schedule an assignment message.
- logf(LVL_INFO, "Router assignment reported from "
+ logf(status, LVL_INFO, "Router assignment reported from "
ATS_IP_PRINTF_CODE
" did not match local assignment. Resending assignment.\n ",
ATS_IP_OCTETS(router_addr)
@@ -832,7 +846,7 @@ CacheImpl::handleRemovalQuery(IpHeader c
if (group.m_routers.end() != router) {
router->m_rapid = true; // do rapid responses.
router->m_recv.set(now, msg.m_query.getRecvId());
- logf(LVL_DEBUG, "WCCP2_REMOVAL_QUERY from router "
+ logf(LVL_INFO, "WCCP2_REMOVAL_QUERY from router "
ATS_IP_PRINTF_CODE ".\n",
ATS_IP_OCTETS(raddr)
);
Modified: trafficserver/traffic/trunk/lib/wccp/WccpLocal.h
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/lib/wccp/WccpLocal.h?rev=1069608&r1=1069607&r2=1069608&view=diff
==============================================================================
--- trafficserver/traffic/trunk/lib/wccp/WccpLocal.h (original)
+++ trafficserver/traffic/trunk/lib/wccp/WccpLocal.h Thu Feb 10 23:20:31 2011
@@ -667,6 +667,10 @@ class CacheIdBox;
This class provides basic control. Two subclasses specialize for the
two variants. Use @c isMask to detect which variant is present.
+ @note Do not add virtual methods, as reasonable as that seems because
+ this is a serialized object and the memory layout correspond to the
+ protocol definition.
+
@see CacheHashIdElt
@see CacheMaskIdElt
*/
@@ -712,8 +716,15 @@ protected:
unsigned int m_reserved_1:1; ///< Reserved (unused).
unsigned int m_is_mask:1; ///< Set -> mask, Clear -> hash.
unsigned int m_reserved_2:6; ///< Reserved (unused).
- // Unfortunately, although @c weight and @c status are common, they are
- // after the variable data and so can't be put in the base class.
+ /** Trailing elements common to all cache ID variants.
+ Unfortunately, although @c weight and @c status are common, they are
+ after the variable data and so can't be put in the base class.
+ Best we can do is declare a struct for them for later convenience.
+ */
+ struct Tail {
+ uint16_t m_weight; ///< Weight of assignment.
+ uint16_t m_status; ///< Cache status.
+ };
};
/** Cache ID for Hash assignment.
@@ -741,8 +752,10 @@ public:
protected:
/// Bit vector of buckets assigned to this cache.
HashBuckets m_buckets;
- uint16_t m_weight; ///< Assignment weight.
- uint16_t m_status; ///< Cache status.
+ Tail m_tail; /// Trailing values in element.
+
+ /// Get the address of the tail elements.
+ Tail* getTailPtr();
};
/** Cache ID for Mask assignment.
@@ -773,8 +786,11 @@ public:
/// Get object size in bytes.
size_t getSize() const;
protected:
+ /// Mask assignment data.
MaskAssignElt m_assign;
- // m_assign is variable sized so we can't layout the weight and status.
+ /// Get a pointer to where the tail data is.
+ /// Presumes the assignment is filled out.
+ Tail* getTailPtr();
};
/** Holder for a @c CacheIdElt.
@@ -853,8 +869,9 @@ protected:
);
CacheIdElt* m_base; ///< Base address of memory for element.
- size_t m_count; ///< Size of element.
- size_t m_size; ///< Size of allocated memory. Zero if external memory.
+ CacheIdElt::Tail* m_tail; ///< Base address of trailing data elements.
+ size_t m_size; ///< Size of element (valid data in buffer);
+ size_t m_cap; ///< Size of allocated memory. Zero if external memory.
};
/** Base class for all components.
@@ -2015,13 +2032,13 @@ namespace detail {
);
/// Get the assignment key.
- AssignmentKeyElt const* getKey() const;
+ AssignmentKeyElt const& getKey() const;
/// Get the router assignment list.
- RouterAssignListElt const* getRouterList() const;
+ RouterAssignListElt const& getRouterList() const;
/// Get the hash assignment.
- HashAssignElt const* getHash() const;
+ HashAssignElt const& getHash() const;
/// Get the mask assignment.
- MaskAssignElt const* getMask() const;
+ MaskAssignElt const& getMask() const;
protected:
Key m_key; ///< Assignment key.
@@ -2578,7 +2595,7 @@ public:
/** Check cache assignment reported by a router against internal assign.
@return @c true if they are the same, @c false otherwise.
*/
- virtual bool checkRouterAssignment(
+ virtual ts::Errata checkRouterAssignment(
GroupData const& group, ///< Group with assignment.
RouterViewComp const& comp ///< Assignment reported by router.
) const;
@@ -2868,7 +2885,7 @@ MaskValueSetElt::getSize() const {
}
inline size_t
-MaskAssignElt::getSize() const { return sizeof(self) + this->getVarSize(); }
+ MaskAssignElt::getSize() const { return sizeof(self) + this->getVarSize(); }
inline uint32_t CacheIdElt::getAddr() const { return m_addr; }
inline CacheIdElt&
@@ -2922,6 +2939,11 @@ CacheIdElt::setStatus(uint16_t s) {
}
# endif
+inline CacheIdElt::Tail*
+CacheHashIdElt::getTailPtr() {
+ return &m_tail;
+}
+
# if 0
inline bool
CacheHashIdElt::getBucket(int idx) const {
@@ -2936,7 +2958,14 @@ CacheMaskIdElt::getCount() const {
inline size_t
CacheMaskIdElt::getSize() const {
- return sizeof(self) + m_assign.getVarSize();
+ return sizeof(self) + sizeof(Tail) + m_assign.getVarSize();
+}
+
+inline CacheIdElt::Tail*
+CacheMaskIdElt::getTailPtr() {
+ return reinterpret_cast<Tail*>(
+ reinterpret_cast<char*>(this) + sizeof(self) + m_assign.getVarSize()
+ );
}
inline uint32_t
@@ -3179,21 +3208,24 @@ detail::Assignment::updateRouterId(uint3
return *this;
}
inline AssignmentKeyElt::AssignmentKeyElt() { }
-inline AssignmentKeyElt const*
+inline AssignmentKeyElt const&
detail::Assignment::getKey() const {
- return &m_key;
+ return m_key;
}
-inline RouterAssignListElt const*
+inline RouterAssignListElt const&
detail::Assignment::getRouterList() const {
- return m_router_list;
+ assert(m_router_list);
+ return *m_router_list;
}
-inline HashAssignElt const*
+inline HashAssignElt const&
detail::Assignment::getHash() const {
- return m_hash_assign;
+ assert(m_hash_assign);
+ return *m_hash_assign;
}
-inline MaskAssignElt const*
+inline MaskAssignElt const&
detail::Assignment::getMask() const {
- return m_mask_assign;
+ assert(m_mask_assign);
+ return *m_mask_assign;
}
inline MsgBuffer::MsgBuffer() : super(0,0), _count(0) { }
Modified: trafficserver/traffic/trunk/lib/wccp/WccpMsg.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/lib/wccp/WccpMsg.cc?rev=1069608&r1=1069607&r2=1069608&view=diff
==============================================================================
--- trafficserver/traffic/trunk/lib/wccp/WccpMsg.cc (original)
+++ trafficserver/traffic/trunk/lib/wccp/WccpMsg.cc Thu Feb 10 23:20:31 2011
@@ -77,39 +77,47 @@ CacheHashIdElt::setBuckets(bool state) {
CacheIdBox::CacheIdBox()
: m_base(0)
- , m_count(0)
+ , m_tail(0)
, m_size(0)
-{
+ , m_cap(0) {
}
-size_t CacheIdBox::getSize() const { return m_count; }
+size_t CacheIdBox::getSize() const { return m_size; }
CacheIdBox&
CacheIdBox::require(size_t n) {
- if (m_size < n) {
- if (m_base && m_size) free(m_base);
+ if (m_cap < n) {
+ if (m_base && m_cap) free(m_base);
m_base = static_cast<CacheIdElt*>(malloc(n));
- m_size = n;
+ m_cap = n;
}
- memset(m_base, 0, m_size);
- m_count = 0;
+ memset(m_base, 0, m_cap);
+ m_size = 0;
return *this;
}
CacheIdBox&
CacheIdBox::initDefaultHash(uint32_t addr) {
this->require(sizeof(CacheHashIdElt));
- m_count = sizeof(CacheHashIdElt);
+ m_size = sizeof(CacheHashIdElt);
m_base->initHashRev().setUnassigned(true).setMask(false).setAddr(addr);
+ m_tail = static_cast<CacheHashIdElt*>(m_base)->getTailPtr();
+ m_tail->m_weight = htons(0);
+ m_tail->m_status = htons(0);
return *this;
}
+
CacheIdBox&
CacheIdBox::initDefaultMask(uint32_t addr) {
- this->require(sizeof(CacheMaskIdElt));
+ // Base element plus set with 1 value plus tail.
+ this->require(sizeof(CacheMaskIdElt) + MaskValueSetElt::calcSize(1) + sizeof(CacheIdElt::Tail));
CacheMaskIdElt* mid = static_cast<CacheMaskIdElt*>(m_base);
mid->initHashRev().setUnassigned(true).setMask(true).setAddr(addr);
mid->m_assign.init(0,0,0,0)->addValue(addr, 0, 0, 0, 0);
- m_count = mid->getSize();
+ m_size = mid->getSize();
+ m_tail = mid->getTailPtr();
+ m_tail->m_weight = htons(0);
+ m_tail->m_status = htons(0);
return *this;
}
@@ -118,15 +126,24 @@ CacheIdBox::fill(self const& src) {
size_t n = src.getSize();
this->require(src.getSize());
memcpy(m_base, src.m_base, n);
+ m_size = src.m_size;
+ // If tail is set in src, use the same offset here.
+ if (src.m_tail)
+ m_tail = reinterpret_cast<CacheIdElt::Tail*>(
+ reinterpret_cast<char*>(m_base) + (
+ reinterpret_cast<char*>(src.m_tail)
+ - reinterpret_cast<char*>(src.m_base)
+ ));
+ else m_tail = 0;
return *this;
}
CacheIdBox&
CacheIdBox::fill(void* base, self const& src) {
- m_count = src.getSize();
- m_size = 0;
+ m_size = src.getSize();
+ m_cap = 0;
m_base = static_cast<CacheIdElt*>(base);
- memcpy(m_base, src.m_base, m_count);
+ memcpy(m_base, src.m_base, m_size);
return *this;
}
@@ -135,7 +152,7 @@ CacheIdBox::parse(MsgBuffer base) {
int zret = PARSE_SUCCESS;
CacheIdElt* ptr = reinterpret_cast<CacheIdElt*>(base.getTail());
size_t n = base.getSpace();
- m_size = 0;
+ m_cap = 0;
if (ptr->isMask()) {
CacheMaskIdElt* mptr = static_cast<CacheMaskIdElt*>(ptr);
size_t size = sizeof(CacheMaskIdElt);
@@ -144,10 +161,12 @@ CacheIdBox::parse(MsgBuffer base) {
|| n < size + MaskValueSetElt::calcSize(0) * mptr->getCount()) {
zret = PARSE_BUFFER_TOO_SMALL;
} else {
- m_count = mptr->getSize();
- if (n < m_count) {
+ m_size = mptr->getSize();
+ if (n < m_size) {
zret = PARSE_BUFFER_TOO_SMALL;
- logf(LVL_DEBUG, "I_SEE_YOU Cache Mask ID too small: %lu < %lu", n, m_count);
+ logf(LVL_DEBUG, "I_SEE_YOU Cache Mask ID too small: %lu < %lu", n, m_size);
+ } else {
+ m_tail = mptr->getTailPtr();
}
}
} else {
@@ -155,7 +174,8 @@ CacheIdBox::parse(MsgBuffer base) {
zret = PARSE_BUFFER_TOO_SMALL;
logf(LVL_DEBUG, "I_SEE_YOU Cache Hash ID too small: %lu < %lu", n, sizeof(CacheHashIdElt));
} else {
- m_count = sizeof(CacheHashIdElt);
+ m_size = sizeof(CacheHashIdElt);
+ m_tail = static_cast<CacheHashIdElt*>(m_base)->getTailPtr();
}
}
if (PARSE_SUCCESS == zret) m_base = ptr;
@@ -1080,10 +1100,10 @@ AssignInfoComp::fill(
MsgBuffer& buffer,
detail::Assignment const& assign
) {
- RouterAssignListElt const* ralist = assign.getRouterList();
- HashAssignElt const* ha = assign.getHash();
- size_t n_routers = ralist->getCount();
- size_t n_caches = ha->getCount();
+ RouterAssignListElt const& ralist = assign.getRouterList();
+ HashAssignElt const& ha = assign.getHash();
+ size_t n_routers = ralist.getCount();
+ size_t n_caches = ha.getCount();
size_t comp_size = this->calcSize(n_routers, n_caches);
if (buffer.getSpace() < comp_size)
@@ -1092,11 +1112,11 @@ AssignInfoComp::fill(
m_base = buffer.getTail();
this->setType(COMP_TYPE);
- this->keyElt() = *(assign.getKey());
- memcpy(&(access_field(&raw_t::m_routers, m_base)), ralist, ralist->getSize());
+ this->keyElt() = assign.getKey();
+ memcpy(&(access_field(&raw_t::m_routers, m_base)), &ralist, ralist.getSize());
// Set the pointer to the count of caches and write the count.
m_cache_count = this->calcCacheCountPtr();
- memcpy(m_cache_count, ha, ha->getSize());
+ memcpy(m_cache_count, &ha, ha.getSize());
this->setLength(comp_size - HEADER_SIZE);
buffer.use(comp_size);
@@ -1171,10 +1191,10 @@ AltHashAssignComp::fill(
MsgBuffer& buffer,
detail::Assignment const& assign
) {
- RouterAssignListElt const* ralist = assign.getRouterList();
- HashAssignElt const* ha = assign.getHash();
- size_t n_routers = ralist->getCount();
- size_t n_caches = ha->getCount();
+ RouterAssignListElt const& ralist = assign.getRouterList();
+ HashAssignElt const& ha = assign.getHash();
+ size_t n_routers = ralist.getCount();
+ size_t n_caches = ha.getCount();
size_t comp_size = this->calcSize(n_routers, n_caches);
if (buffer.getSpace() < comp_size)
@@ -1187,11 +1207,11 @@ AltHashAssignComp::fill(
.setAssignType(ALT_HASH_ASSIGNMENT)
.setAssignLength(comp_size - HEADER_SIZE - sizeof(local_header_t))
;
- this->keyElt() = *(assign.getKey());
- memcpy(&(access_field(&raw_t::m_routers, m_base)), ralist, ralist->getSize());
+ this->keyElt() = assign.getKey();
+ memcpy(&(access_field(&raw_t::m_routers, m_base)), &ralist, ralist.getSize());
// Set the pointer to the count of caches and write the count.
m_cache_count = static_cast<uint32_t*>(this->calcVarPtr());
- memcpy(m_cache_count, ha, ha->getSize());
+ memcpy(m_cache_count, &ha, ha.getSize());
buffer.use(comp_size);
@@ -1227,9 +1247,9 @@ AltMaskAssignComp::fill(
MsgBuffer& buffer,
detail::Assignment const& assign
) {
- RouterAssignListElt const* ralist = assign.getRouterList();
- MaskAssignElt const* ma = assign.getMask();
- size_t comp_size = sizeof(raw_t) + ralist->getVarSize() + ma->getSize();
+ RouterAssignListElt const& ralist = assign.getRouterList();
+ MaskAssignElt const& ma = assign.getMask();
+ size_t comp_size = sizeof(raw_t) + ralist.getVarSize() + ma.getSize();
if (buffer.getSpace() < comp_size)
throw ts::Exception(BUFFER_TOO_SMALL_FOR_COMP_TEXT);
@@ -1241,11 +1261,11 @@ AltMaskAssignComp::fill(
.setAssignType(ALT_MASK_ASSIGNMENT)
.setAssignLength(comp_size - HEADER_SIZE - sizeof(local_header_t))
;
- this->keyElt() = *(assign.getKey());
+ this->keyElt() = assign.getKey();
- memcpy(&(access_field(&raw_t::m_routers, m_base)), ralist, ralist->getSize());
+ memcpy(&(access_field(&raw_t::m_routers, m_base)), &ralist, ralist.getSize());
m_mask_elt = static_cast<MaskAssignElt*>(this->calcVarPtr());
- memcpy(m_mask_elt, ma, ma->getSize());
+ memcpy(m_mask_elt, &ma, ma.getSize());
buffer.use(comp_size);
@@ -1439,8 +1459,8 @@ AssignMapComp::getCount() const {
AssignMapComp&
AssignMapComp::fill(MsgBuffer& buffer, detail::Assignment const& assign) {
size_t comp_size = sizeof(raw_t);
- MaskAssignElt const* ma = assign.getMask();
- size_t ma_size = ma->getSize(); // Not constant time.
+ MaskAssignElt const& ma = assign.getMask();
+ size_t ma_size = ma.getSize(); // Not constant time.
// Can't be precise, but we need at least one mask/value set with
// at least one value. If we don't have that it's a clear fail.
@@ -1448,8 +1468,8 @@ AssignMapComp::fill(MsgBuffer& buffer, d
throw ts::Exception(BUFFER_TOO_SMALL_FOR_COMP_TEXT);
m_base = buffer.getTail();
- memcpy(&(access_field(&raw_t::m_assign, m_base)), ma, ma_size);
- comp_size += ma_size - sizeof(*ma);
+ memcpy(&(access_field(&raw_t::m_assign, m_base)), &ma, ma_size);
+ comp_size += ma_size - sizeof(ma);
this->setType(COMP_TYPE)
.setLength(comp_size - HEADER_SIZE)
@@ -1486,12 +1506,12 @@ detail::Assignment::Assignment()
bool
detail::Assignment::fill(cache::GroupData& group, uint32_t addr) {
- // Compute the last packet received times for the routers.
- // For each cache, compute how routers mentioned it in their
- // last packet. Prepare an assignment from those caches.
- // If there are no such caches, fail the assignment.
- // Any cache that wasn't mentioned by at least one router
- // are purged.
+ // Compute the last packet received times for the routers. For each
+ // cache, compute how many routers mentioned it in their last
+ // packet. Prepare an assignment from those caches. If there are no
+ // such caches, fail the assignment. Any cache that wasn't
+ // mentioned by at least one router are purged.
+
size_t n_routers = group.m_routers.size(); // routers in group
size_t n_caches = group.m_caches.size(); // caches in group
@@ -1532,7 +1552,9 @@ detail::Assignment::fill(cache::GroupDat
// For each router, update the assignment and run over the caches
// and bump the nr count if that cache was included in the most
- // recent packet.
+ // recent packet. Note that the router data gets updated from
+ // ISeeYou message processing as well, this is more of an initial
+ // setup.
for ( rdx = 0, rspot = rbegin ; rspot != rend ; ++rspot, ++rdx ) {
// Update router assignment.
m_router_list->elt(rdx)