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 a814f8a commit ffe1a38
Show file tree
Hide file tree
Showing 13 changed files with 415 additions and 47 deletions.
13 changes: 8 additions & 5 deletions src/base/proto.h
View file
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,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
52 changes: 43 additions & 9 deletions src/bgp/bgp_attr.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#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 @@ -153,6 +154,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 @@ -165,9 +185,6 @@ 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);
return 0;
}

Expand Down Expand Up @@ -262,9 +279,6 @@ void EdgeDiscoverySpec::Edge::SetLabels(
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;
}

Expand All @@ -289,6 +303,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 @@ -356,9 +381,6 @@ void EdgeForwardingSpec::Edge::SetOutboundIp4Address(Ip4Address addr) {
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;
}

Expand All @@ -384,6 +406,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
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,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 @@ -252,6 +253,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 @@ -313,6 +315,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
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,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
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@

class BgpAttr;

struct BgpAttribute : public ParseObject {
class BgpAttribute : public ParseObject {
public:
enum Flag {
Optional = 1 << 7,
Transitive = 1 << 6,
Expand Down Expand Up @@ -62,6 +63,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
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,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 @@ -820,7 +820,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 @@ -1395,6 +1403,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 @@ -1408,12 +1417,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
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,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 @@ -94,5 +93,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);
}
}
8 changes: 8 additions & 0 deletions src/bgp/community.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ int Community::CompareTo(const Community &rhs) const {
return 0;
}

size_t CommunitySpec::EncodeLength() const {
return communities.size() * sizeof(uint32_t);
}

void Community::Remove() {
comm_db_->Delete(this);
}
Expand All @@ -73,6 +77,10 @@ string ExtCommunitySpec::ToString() const {
return string(repr);
}

size_t ExtCommunitySpec::EncodeLength() const {
return communities.size() * sizeof(uint64_t);
}

int ExtCommunitySpec::CompareTo(const BgpAttribute &rhs_attr) const {
int ret = BgpAttribute::CompareTo(rhs_attr);
if (ret != 0) return ret;
Expand Down
5 changes: 4 additions & 1 deletion src/bgp/community.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ struct CommunitySpec : public BgpAttribute {
}
virtual void ToCanonical(BgpAttr *attr);
virtual std::string ToString() const;
virtual size_t EncodeLength() const;
};

class Community {
Expand Down Expand Up @@ -113,11 +114,13 @@ class CommunityDB : public BgpPathAttributeDB<Community, CommunityPtr,
private:
};

struct ExtCommunitySpec : public BgpAttribute {
class ExtCommunitySpec : public BgpAttribute {
public:
static const int kSize = -1;
static const uint8_t kFlags = Optional | Transitive;
ExtCommunitySpec() : BgpAttribute(ExtendedCommunities, kFlags) { }
explicit ExtCommunitySpec(const BgpAttribute &rhs) : BgpAttribute(rhs) { }
virtual size_t EncodeLength() const;
std::vector<uint64_t> communities;
virtual int CompareTo(const BgpAttribute &rhs_attr) const;
virtual void ToCanonical(BgpAttr *attr);
Expand Down
9 changes: 0 additions & 9 deletions src/bgp/test/bgp_attr_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -681,10 +681,7 @@ TEST_F(BgpAttrTest, PmsiTunnel6) {

TEST_F(BgpAttrTest, PmsiTunnelSpecCompareTo) {
PmsiTunnelSpec pmsi_spec1;
PmsiTunnelSpec pmsi_spec2;
EXPECT_EQ(0, pmsi_spec1.CompareTo(pmsi_spec1));
EXPECT_NE(0, pmsi_spec1.CompareTo(pmsi_spec2));
EXPECT_NE(0, pmsi_spec2.CompareTo(pmsi_spec1));
}

TEST_F(BgpAttrTest, PmsiTunnelSpecToString1) {
Expand Down Expand Up @@ -846,10 +843,7 @@ TEST_F(BgpAttrTest, EdgeDiscovery6) {

TEST_F(BgpAttrTest, EdgeDiscoveryCompareTo) {
EdgeDiscoverySpec edspec1;
EdgeDiscoverySpec edspec2;
EXPECT_EQ(0, edspec1.CompareTo(edspec1));
EXPECT_NE(0, edspec1.CompareTo(edspec2));
EXPECT_NE(0, edspec2.CompareTo(edspec1));
}

TEST_F(BgpAttrTest, EdgeDiscoveryToString1) {
Expand Down Expand Up @@ -1023,10 +1017,7 @@ TEST_F(BgpAttrTest, EdgeForwarding6) {

TEST_F(BgpAttrTest, EdgeForwardingCompareTo) {
EdgeForwardingSpec efspec1;
EdgeForwardingSpec efspec2;
EXPECT_EQ(0, efspec1.CompareTo(efspec1));
EXPECT_NE(0, efspec1.CompareTo(efspec2));
EXPECT_NE(0, efspec2.CompareTo(efspec1));
}

TEST_F(BgpAttrTest, EdgeForwardingToString1) {
Expand Down

0 comments on commit ffe1a38

Please sign in to comment.