Skip to content

Commit

Permalink
Add BGP ExtendedLength attribute encoding.
Browse files Browse the repository at this point in the history 
Add support for Extended Length encoding for variable length BGP
attributes. Revamp the exploratory tests so that larger messages get
generated. Fix the encoding of Unknown Attributes. When generating
NOTIFICATIONs use a max message size buffer and do not assert if
encoding fails.

Closes-bug: #1454431

Change-Id: I545e28f61b4d7a840b73fb890da88a6322b2ecbf
  • Loading branch information
Pedro Marques committed May 17, 2015
1 parent 90bad7d commit 3d6486b
Show file tree
Hide file tree
Showing 12 changed files with 473 additions and 38 deletions.
13 changes: 8 additions & 5 deletions src/base/proto.h
View file
Expand Up @@ -439,7 +439,7 @@ class ProtoChoice : public ChoiceBase {
ChoiceEncoder(EncodeContext *context, const T *msg, uint8_t *data, int size,
int *resultp)
: context(context), msg(msg), data(data), size(size),
resultp(resultp) {
resultp(resultp), found(false) {
}
template <typename U, typename CtxType>
struct EncoderTrue {
Expand All @@ -465,8 +465,9 @@ class ProtoChoice : public ChoiceBase {
struct EncoderSetter {
int operator()(int opt, EncodeContext *context, const CtxType *msg,
uint8_t *data, int size) {
if (Derived::Setter::get(msg) == opt) {
return EncoderTrue<U, CtxType>()(opt, context, msg, data, size);
int value = Derived::Setter::get(msg);
if (value == opt || opt == -1) {
return EncoderTrue<U, CtxType>()(value, context, msg, data, size);
}
return 0;
}
Expand Down Expand Up @@ -508,7 +509,7 @@ class ProtoChoice : public ChoiceBase {
};
template <typename U>
void operator()(U x) {
if (*resultp < 0) {
if (*resultp < 0 || found) {
return;
}
// The choice element can be determined by:
Expand All @@ -534,7 +535,8 @@ class ProtoChoice : public ChoiceBase {
int result = choice(U::first::value, context, msg, data, size);
if (result < 0) {
*resultp = result;
} else {
} else if (result > 0) {
found = true;
*resultp += result;
}
}
Expand All @@ -544,6 +546,7 @@ class ProtoChoice : public ChoiceBase {
uint8_t *data;
int size;
int *resultp;
bool found;
};
};

Expand Down
9 changes: 9 additions & 0 deletions src/bgp/bgp_aspath.cc
View file
Expand Up @@ -56,6 +56,15 @@ std::string AsPathSpec::ToString() const {
return oss.str();
}

size_t AsPathSpec::EncodeLength() const {
size_t sz = 0;
for (size_t i = 0; i < path_segments.size(); i++) {
sz += 2;
sz += path_segments[i]->path_segment.size() * 2;
}
return sz;
}

bool AsPathSpec::AsPathLoop(as_t as) const {
for (size_t i = 0; i < path_segments.size(); i++)
for (size_t j = 0; j < path_segments[i]->path_segment.size(); j++)
Expand Down
1 change: 1 addition & 0 deletions src/bgp/bgp_aspath.h
View file
Expand Up @@ -63,6 +63,7 @@ struct AsPathSpec : public BgpAttribute {

virtual int CompareTo(const BgpAttribute &rhs_attr) const;
virtual void ToCanonical(BgpAttr *attr);
virtual size_t EncodeLength() const;
virtual std::string ToString() const;
AsPathSpec *Add(as_t asn) const;
std::vector<PathSegment *> path_segments;
Expand Down
110 changes: 101 additions & 9 deletions src/bgp/bgp_attr.cc
View file
Expand Up @@ -5,6 +5,7 @@
#include "bgp/bgp_attr.h"
#include "base/util.h"
#include "bgp/bgp_proto.h"
#include "net/bgp_af.h"

int BgpAttrOrigin::CompareTo(const BgpAttribute &rhs_attr) const {
int ret = BgpAttribute::CompareTo(rhs_attr);
Expand Down Expand Up @@ -149,6 +150,25 @@ int BgpMpNlri::CompareTo(const BgpAttribute &rhs_attr) const {
void BgpMpNlri::ToCanonical(BgpAttr *attr) {
}

size_t BgpMpNlri::EncodeLength() const {
size_t sz = 2 /* safi */ + 1 /* afi */ +
1 /* NlriNextHopLength */ +
1 /* Reserved */;
sz += nexthop.size();
for (std::vector<BgpProtoPrefix*>::const_iterator iter = nlri.begin();
iter != nlri.end(); ++iter) {
size_t bytes = 0;
if (afi == BgpAf::L2Vpn &&
(safi == BgpAf::EVpn || safi == BgpAf::ErmVpn)) {
bytes = (*iter)->prefixlen;
} else {
bytes = ((*iter)->prefixlen + 7) / 8;
}
sz += 1 + bytes;
}
return sz;
}

PmsiTunnelSpec::PmsiTunnelSpec()
: BgpAttribute(PmsiTunnel, kFlags),
tunnel_flags(0), tunnel_type(0), label(0) {
Expand All @@ -161,9 +181,11 @@ PmsiTunnelSpec::PmsiTunnelSpec(const BgpAttribute &rhs)
int PmsiTunnelSpec::CompareTo(const BgpAttribute &rhs_attr) const {
int ret = BgpAttribute::CompareTo(rhs_attr);
if (ret != 0) return ret;
const PmsiTunnelSpec &rhs =
static_cast<const PmsiTunnelSpec &>(rhs_attr);
KEY_COMPARE(this, &rhs);
const PmsiTunnelSpec &rhs = static_cast<const PmsiTunnelSpec &>(rhs_attr);
KEY_COMPARE(tunnel_flags, rhs.tunnel_flags);
KEY_COMPARE(tunnel_type, rhs.tunnel_type);
KEY_COMPARE(label, rhs.label);
KEY_COMPARE(identifier, rhs.identifier);
return 0;
}

Expand Down Expand Up @@ -255,13 +277,47 @@ void EdgeDiscoverySpec::Edge::SetLabels(
labels.push_back(last_label);
}

template <class InputIterator, class CompareOp>
int STLSortedCompare(InputIterator first1, InputIterator last1,
InputIterator first2, InputIterator last2,
CompareOp op) {
InputIterator iter1 = first1;
InputIterator iter2 = first2;
while (iter1 != last1 && iter2 != last2) {
int result = op(*iter1, *iter2);
if (result != 0) {
return result;
}
++iter1;
++iter2;
}
if (iter1 != last1) {
return 1;
}
if (iter2 != last2) {
return -1;
}
return 0;
}

struct EdgeDiscoverySpecEdgeCompare {
int operator()(const EdgeDiscoverySpec::Edge *lhs,
const EdgeDiscoverySpec::Edge *rhs) const {
KEY_COMPARE(lhs->address, rhs->address);
KEY_COMPARE(lhs->labels, rhs->labels);
return 0;
}
};

int EdgeDiscoverySpec::CompareTo(const BgpAttribute &rhs_attr) const {
int ret = BgpAttribute::CompareTo(rhs_attr);
if (ret != 0) return ret;
const EdgeDiscoverySpec &rhs =
static_cast<const EdgeDiscoverySpec &>(rhs_attr);
KEY_COMPARE(this, &rhs);
return 0;
static_cast<const EdgeDiscoverySpec &>(rhs_attr);
ret = STLSortedCompare(edge_list.begin(), edge_list.end(),
rhs.edge_list.begin(), rhs.edge_list.end(),
EdgeDiscoverySpecEdgeCompare());
return ret;
}

void EdgeDiscoverySpec::ToCanonical(BgpAttr *attr) {
Expand All @@ -285,6 +341,17 @@ std::string EdgeDiscoverySpec::ToString() const {
return oss.str();
}

size_t EdgeDiscoverySpec::EncodeLength() const {
size_t sz = 0;
for (EdgeList::const_iterator iter = edge_list.begin();
iter != edge_list.end(); ++iter) {
sz += 2; /* AddressLen + LabelLen */
sz += (*iter)->address.size();
sz += (*iter)->labels.size() * sizeof(uint32_t);
}
return sz;
}

EdgeDiscovery::Edge::Edge(const EdgeDiscoverySpec::Edge *spec_edge) {
address = spec_edge->GetIp4Address();
uint32_t first_label, last_label;
Expand Down Expand Up @@ -349,13 +416,26 @@ void EdgeForwardingSpec::Edge::SetOutboundIp4Address(Ip4Address addr) {
outbound_address.begin());
}

struct EdgeForwardingSpecEdgeCompare {
int operator()(const EdgeForwardingSpec::Edge *lhs,
const EdgeForwardingSpec::Edge *rhs) const {
KEY_COMPARE(lhs->inbound_address, rhs->inbound_address);
KEY_COMPARE(lhs->outbound_address, rhs->outbound_address);
KEY_COMPARE(lhs->inbound_label, rhs->inbound_label);
KEY_COMPARE(lhs->outbound_label, rhs->outbound_label);
return 0;
}
};

int EdgeForwardingSpec::CompareTo(const BgpAttribute &rhs_attr) const {
int ret = BgpAttribute::CompareTo(rhs_attr);
if (ret != 0) return ret;
const EdgeForwardingSpec &rhs =
static_cast<const EdgeForwardingSpec &>(rhs_attr);
KEY_COMPARE(this, &rhs);
return 0;
static_cast<const EdgeForwardingSpec &>(rhs_attr);
ret = STLSortedCompare(edge_list.begin(), edge_list.end(),
rhs.edge_list.begin(), rhs.edge_list.end(),
EdgeForwardingSpecEdgeCompare());
return ret;
}

void EdgeForwardingSpec::ToCanonical(BgpAttr *attr) {
Expand All @@ -380,6 +460,18 @@ std::string EdgeForwardingSpec::ToString() const {
return oss.str();
}

size_t EdgeForwardingSpec::EncodeLength() const {
size_t sz = 0;
for (EdgeList::const_iterator iter = edge_list.begin();
iter != edge_list.end(); ++iter) {
sz += 1 /* address len */ + 8 /* 2 labels */;
sz += (*iter)->inbound_address.size();
sz += (*iter)->outbound_address.size();
}

return sz;
}

EdgeForwarding::Edge::Edge(const EdgeForwardingSpec::Edge *spec_edge) {
inbound_address = spec_edge->GetInboundIp4Address();
outbound_address = spec_edge->GetOutboundIp4Address();
Expand Down
3 changes: 3 additions & 0 deletions src/bgp/bgp_attr.h
View file
Expand Up @@ -150,6 +150,7 @@ struct BgpMpNlri : public BgpAttribute {
}
virtual int CompareTo(const BgpAttribute &rhs_attr) const;
virtual void ToCanonical(BgpAttr *attr);
virtual size_t EncodeLength() const;

uint16_t afi;
uint8_t safi;
Expand Down Expand Up @@ -237,6 +238,7 @@ struct EdgeDiscoverySpec : public BgpAttribute {
virtual int CompareTo(const BgpAttribute &rhs_attr) const;
virtual void ToCanonical(BgpAttr *attr);
virtual std::string ToString() const;
virtual size_t EncodeLength() const;

struct Edge : public ParseObject {
Ip4Address GetIp4Address() const;
Expand Down Expand Up @@ -298,6 +300,7 @@ struct EdgeForwardingSpec : public BgpAttribute {
virtual int CompareTo(const BgpAttribute &rhs_attr) const;
virtual void ToCanonical(BgpAttr *attr);
virtual std::string ToString() const;
virtual size_t EncodeLength() const;

struct Edge : public ParseObject {
Ip4Address GetInboundIp4Address() const;
Expand Down
14 changes: 13 additions & 1 deletion src/bgp/bgp_attr_base.cc
View file
Expand Up @@ -32,10 +32,22 @@ void BgpProtoPrefix::WriteLabel(size_t label_offset, uint32_t label) {
int BgpAttribute::CompareTo(const BgpAttribute &rhs) const {
KEY_COMPARE(code, rhs.code);
KEY_COMPARE(subcode, rhs.subcode);
KEY_COMPARE(flags, rhs.flags);
KEY_COMPARE(flags & ~ExtendedLength, rhs.flags & ~ExtendedLength);
return 0;
}

size_t BgpAttribute::EncodeLength() const {
return 0;
}

uint8_t BgpAttribute::GetEncodeFlags() const {
uint8_t value = flags;
if (EncodeLength() >= sizeof(uint8_t) << 8) {
value |= BgpAttribute::ExtendedLength;
}
return value;
}

std::string BgpAttribute::ToString() const {
char repr[80];
snprintf(repr, sizeof(repr), "<code: %d, flags: %02x>", code, flags);
Expand Down
13 changes: 12 additions & 1 deletion src/bgp/bgp_attr_base.h
View file
Expand Up @@ -16,7 +16,8 @@

class BgpAttr;

struct BgpAttribute : public ParseObject {
class BgpAttribute : public ParseObject {
public:
enum Flag {
Optional = 1 << 7,
Transitive = 1 << 6,
Expand Down Expand Up @@ -59,6 +60,16 @@ struct BgpAttribute : public ParseObject {
uint8_t code;
uint8_t subcode;
uint8_t flags;
/*
* Variable length attributes should return the size of the attribute
* for encoding purposes in order to set the ExtendedLength flag.
*/
virtual size_t EncodeLength() const;
/*
* Helper method to compute flags used when encoding attributes.
*/
uint8_t GetEncodeFlags() const;

virtual std::string ToString() const;
virtual int CompareTo(const BgpAttribute &rhs) const;
virtual void ToCanonical(BgpAttr *attr) { }
Expand Down
18 changes: 13 additions & 5 deletions src/bgp/bgp_proto.cc
View file
Expand Up @@ -575,7 +575,7 @@ class BgpPathAttrLength : public ProtoElement<BgpPathAttrLength> {
typedef AttrLen SequenceLength;
struct AttrSizeSet {
static int get(const BgpAttribute *obj) {
if (obj->flags & BgpAttribute::ExtendedLength) {
if (obj->GetEncodeFlags() & BgpAttribute::ExtendedLength) {
// Extended Length: use 2 bytes to read
return 2;
} else {
Expand Down Expand Up @@ -807,7 +807,15 @@ class BgpPathAttributeAsPath : public ProtoSequence<BgpPathAttributeAsPath> {
class BgpPathAttributeFlags : public ProtoElement<BgpPathAttributeFlags> {
public:
static const int kSize = 1;
typedef Accessor<BgpAttribute, uint8_t, &BgpAttribute::flags> Setter;
struct FlagsAccessor {
static void set(BgpAttribute *obj, uint8_t value) {
obj->flags = value;
}
static uint8_t get(const BgpAttribute *obj) {
return obj->GetEncodeFlags();
}
};
typedef FlagsAccessor Setter;
};

class BgpPathAttributeCommunityList :
Expand Down Expand Up @@ -1379,6 +1387,7 @@ class BgpPathAttribute : public ProtoChoice<BgpPathAttribute> {
mpl::pair<mpl::int_<BgpAttribute::Origin>,
BgpAttrTemplate<BgpAttrOrigin, 1, int,
&BgpAttrOrigin::origin> >,
mpl::pair<mpl::int_<BgpAttribute::AsPath>, BgpPathAttributeAsPath>,
mpl::pair<mpl::int_<BgpAttribute::NextHop>,
BgpAttrTemplate<BgpAttrNextHop, 4, uint32_t,
&BgpAttrNextHop::nexthop> >,
Expand All @@ -1392,12 +1401,11 @@ class BgpPathAttribute : public ProtoChoice<BgpPathAttribute> {
BgpPathAttributeAtomicAggregate>,
mpl::pair<mpl::int_<BgpAttribute::Aggregator>,
BgpPathAttributeAggregator>,
mpl::pair<mpl::int_<BgpAttribute::Communities>,
BgpPathAttributeCommunities>,
mpl::pair<mpl::int_<BgpAttribute::OriginatorId>,
BgpAttrTemplate<BgpAttrOriginatorId, 4, uint32_t,
&BgpAttrOriginatorId::originator_id> >,
mpl::pair<mpl::int_<BgpAttribute::AsPath>, BgpPathAttributeAsPath>,
mpl::pair<mpl::int_<BgpAttribute::Communities>,
BgpPathAttributeCommunities>,
mpl::pair<mpl::int_<BgpAttribute::MPReachNlri>,
BgpPathAttributeMpReachNlriSequence>,
mpl::pair<mpl::int_<BgpAttribute::MPUnreachNlri>,
Expand Down
10 changes: 6 additions & 4 deletions src/bgp/bgp_session.cc
View file
Expand Up @@ -77,9 +77,8 @@ void BgpSession::SendNotification(int code, int subcode,
msg.error = code;
msg.subcode = subcode;
msg.data = data;
uint8_t buf[256];
int result = BgpProto::Encode(&msg, buf, sizeof(buf));
assert(result > BgpProto::kMinMessageSize);
uint8_t buf[BgpProto::kMaxMessageSize];
int msglen = BgpProto::Encode(&msg, buf, sizeof(buf));

// Use SYS_DEBUG for connection collision, SYS_NOTICE for the rest.
SandeshLevel::type log_level;
Expand All @@ -92,5 +91,8 @@ void BgpSession::SendNotification(int code, int subcode,
BGP_LOG(BgpPeerNotification, log_level, BGP_LOG_FLAG_ALL,
peer_ ? peer_->ToUVEKey() : ToString(),
BGP_PEER_DIR_OUT, code, subcode, msg.ToString());
Send(buf, result, NULL);

if (msglen > BgpProto::kMinMessageSize) {
Send(buf, msglen, NULL);
}
}

0 comments on commit 3d6486b

Please sign in to comment.