Skip to content

Commit

Permalink
Multicast traffic gets dropped.
Browse files Browse the repository at this point in the history 
Problem:
When all the VMI's are gone multicast object is deleted. However VN is still not
gone and meanwhile new VMI gets added. During this window where all VMI were
gone to new VMI gets added, olist for replication tree is received from
control-node. This update is neglected in absence of multicast object for same.
This results in no multicast tree.

Solution:
Retain multicast object till two conditions are met:
- All VMIS are gone
- VN is no more in use by multicast.

Change-Id: Ica43ce9ed4951f644d9049b354e0764c860ef4d4
Closes-bug: 1529829
  • Loading branch information
@manishsing
manishsing committed Feb 19, 2016
1 parent d2f2957 commit 1903658
Show file tree
Hide file tree
Showing 7 changed files with 196 additions and 58 deletions.
4 changes: 3 additions & 1 deletion src/vnsw/agent/oper/agent_path.cc
View file
Expand Up @@ -871,7 +871,9 @@ bool MulticastRoute::CopyPathParameters(Agent *agent,
path->set_dest_vn_name(vn_name);
path->set_unresolved(unresolved);
path->set_vxlan_id(vxlan_id);
path->set_label(label);
if ((path->peer() != agent->local_vm_peer()) &&
(path->peer() != agent->local_peer()))
path->set_label(label);

//Setting of tunnel is only for simulated TOR.
path->set_tunnel_bmap(tunnel_type);
Expand Down
105 changes: 96 additions & 9 deletions src/vnsw/agent/oper/bridge_route.cc
View file
Expand Up @@ -571,6 +571,89 @@ bool BridgeRouteEntry::ReComputePathDeletion(AgentPath *path) {
return false;
}

void BridgeRouteEntry::HandleMulticastLabel(const Agent *agent,
AgentPath *path,
const AgentPath *local_peer_path,
const AgentPath *local_vm_peer_path,
bool del,
uint32_t *evpn_label) {
*evpn_label = MplsTable::kInvalidLabel;

//EVPN label is present in two paths:
// local_vm_peer(courtesy: vmi) or local_peer(courtesy: vn)
// Irrespective of delete/add operation if one of them is present and is not
// the affected path, then extract the label from same.
// By default pick it from available path (local or local_vm).
switch (path->peer()->GetType()) {
case Peer::LOCAL_VM_PEER:
//Use local_peer path for label
if (local_peer_path) {
*evpn_label = local_peer_path->label();
assert(*evpn_label != MplsTable::kInvalidLabel);
}
break;
case Peer::LOCAL_PEER:
//Use local_peer path for label
if (local_vm_peer_path) {
*evpn_label = local_vm_peer_path->label();
assert(*evpn_label != MplsTable::kInvalidLabel);
}
break;
default:
if (local_vm_peer_path) {
*evpn_label = local_vm_peer_path->label();
assert(*evpn_label != MplsTable::kInvalidLabel);
} else if (local_peer_path) {
*evpn_label = local_peer_path->label();
assert(*evpn_label != MplsTable::kInvalidLabel);
}
break;
}

//Delete path evpn label if path is local_peer or local_vm_peer.
//Delete fabric label if path is multicast_fabric_tree
if (del) {
bool delete_label = false;
// On deletion of fabric path delete fabric label.
// Other type of label is evpn mcast label.
// EVPN label is deleted when both local peer and local_vm_peer path are
// gone.
if (path->peer()->GetType() == Peer::MULTICAST_FABRIC_TREE_BUILDER)
delete_label = true;
else if ((path->peer() == agent->local_vm_peer()) ||
(path->peer() == agent->local_peer())) {
if (local_peer_path == NULL &&
local_vm_peer_path == NULL)
delete_label = true;
}
if (delete_label)
agent->mpls_table()->DeleteMcastLabel(path->label());

return;
}

// Currently other than evpn label no other multicast path requires dynamic
// allocation so return.
if ((path != local_peer_path) && (path != local_vm_peer_path))
return;

// Path already has label, return.
if (path->label() != MplsTable::kInvalidLabel)
return;

// If this is the first time i.e. local_peer has come with no local_vm_peer
// and vice versa then allocate label.
// If its not then we should have valid evpn label calculated above.
if (*evpn_label == MplsTable::kInvalidLabel) {
// XOR use - we shud never reach here when both are NULL or set.
// Only one should be present.
assert((local_vm_peer_path != NULL) ^ (local_peer_path != NULL));
*evpn_label = agent->mpls_table()->AllocLabel();
}
assert(*evpn_label != MplsTable::kInvalidLabel);
path->set_label(*evpn_label);
}

bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) {
if (path->peer() == NULL) {
return false;
Expand Down Expand Up @@ -600,13 +683,9 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) {
AgentPath *evpn_peer_path = NULL;
AgentPath *fabric_peer_path = NULL;
AgentPath *tor_peer_path = NULL;
AgentPath *local_peer_path = NULL;
bool tor_path = false;

//Delete path label
if (del && (path->peer()->GetType() != Peer::BGP_PEER)) {
agent->mpls_table()->DeleteMcastLabel(path->label());
}

const CompositeNH *cnh =
static_cast<const CompositeNH *>(path->nexthop());
if (cnh && (cnh->composite_nh_type() == Composite::TOR)) {
Expand Down Expand Up @@ -659,6 +738,8 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) {
fabric_peer_path = it_path;
} else if (it_path->peer() == agent->multicast_peer()) {
multicast_peer_path = it_path;
} else if (it_path->peer() == agent->local_peer()) {
local_peer_path = it_path;
}
}

Expand All @@ -668,6 +749,13 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) {
}
}

uint32_t evpn_label = MplsTable::kInvalidLabel;
HandleMulticastLabel(agent, path,
local_peer_path,
local_vm_peer_path,
del,
&evpn_label);

//all paths are gone so delete multicast_peer path as well
if ((local_vm_peer_path == NULL) &&
(tor_peer_path == NULL) &&
Expand Down Expand Up @@ -746,7 +834,6 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) {
bool unresolved = false;
uint32_t vxlan_id = 0;
uint32_t label = 0;
uint32_t evpn_label = 0;
uint32_t tunnel_bmap = TunnelType::AllType();

//Select based on priority of path peer.
Expand All @@ -756,7 +843,6 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) {
vxlan_id = local_vm_peer_path->vxlan_id();
tunnel_bmap = TunnelType::AllType();
label = local_vm_peer_path->label();
evpn_label = label;
} else if (tor_peer_path) {
dest_vn_name = tor_peer_path->dest_vn_name();
unresolved = tor_peer_path->unresolved();
Expand Down Expand Up @@ -791,6 +877,7 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) {
label,
tunnel_bmap,
nh);

//Bake all MPLS label
if (fabric_peer_path) {
//Add new label
Expand All @@ -804,8 +891,8 @@ bool BridgeRouteEntry::ReComputeMulticastPaths(AgentPath *path, bool del) {
}
}

//Populate label only when local vm peer path is present.
if (evpn_label != 0 && local_vm_peer_path) {
// Rebake label with whatever comp NH has been calculated.
if (evpn_label != MplsTable::kInvalidLabel) {
agent->mpls_table()->CreateMcastLabel(evpn_label,
Composite::L2COMP,
component_nh_list,
Expand Down
5 changes: 5 additions & 0 deletions src/vnsw/agent/oper/bridge_route.h
View file
Expand Up @@ -144,6 +144,11 @@ class BridgeRouteEntry : public AgentRoute {
const AgentPath *FindOvsPath() const;

private:
void HandleMulticastLabel(const Agent *agent,
AgentPath *path,
const AgentPath *local_peer_path,
const AgentPath *local_vm_peer_path,
bool del, uint32_t *evpn_label);
bool ReComputeMulticastPaths(AgentPath *path, bool del);
AgentPath *FindEvpnPathUsingKeyData(const AgentRouteKey *key,
const AgentRouteData *data) const;
Expand Down
50 changes: 23 additions & 27 deletions src/vnsw/agent/oper/multicast.cc
View file
Expand Up @@ -116,7 +116,7 @@ void MulticastHandler::HandleVxLanChange(const VnEntry *vn) {
obj->set_vxlan_id(new_vxlan_id);
//Rebake new vxlan id in mcast route
AddL2BroadcastRoute(obj, vn->GetVrf()->GetName(), vn->GetName(),
broadcast, obj->evpn_mpls_label(), new_vxlan_id, 0);
broadcast, MplsTable::kInvalidLabel, new_vxlan_id, 0);
}
}

Expand All @@ -139,11 +139,11 @@ void MulticastHandler::HandleVnParametersChange(DBTablePartBase *partition,
boost::system::error_code ec;
Ip4Address broadcast = IpAddress::from_string("255.255.255.255",
ec).to_v4();

//Add operation
if (!deleted) {
MulticastGroupObject *all_broadcast =
FindFloodGroupObject(vn->GetVrf()->GetName());

if (!state) {
state = new MulticastDBState(vn->GetVrf()->GetName(),
vn_vxlan_id);
Expand All @@ -152,9 +152,10 @@ void MulticastHandler::HandleVnParametersChange(DBTablePartBase *partition,
state);
//Also create multicast object
if (all_broadcast == NULL) {
CreateMulticastGroupObject(state->vrf_name_, broadcast,
vn->GetName(), state->vxlan_id_);
all_broadcast = CreateMulticastGroupObject(state->vrf_name_, broadcast,
vn, state->vxlan_id_);
}
all_broadcast->set_vn(vn);
} else {
if (old_vxlan_id != vn_vxlan_id) {
state->vxlan_id_ = vn_vxlan_id;
Expand Down Expand Up @@ -183,9 +184,13 @@ void MulticastHandler::HandleVnParametersChange(DBTablePartBase *partition,
if (!state)
return;

if (deleted) {
DeleteMulticastObject(state->vrf_name_, broadcast);
MulticastGroupObject *all_broadcast =
FindFloodGroupObject(state->vrf_name_);
if (all_broadcast) {
all_broadcast->reset_vn();
}

DeleteMulticastObject(state->vrf_name_, broadcast);
BridgeAgentRouteTable::DeleteBroadcastReq(agent_->local_peer(),
state->vrf_name_,
old_vxlan_id,
Expand All @@ -206,19 +211,18 @@ void MulticastHandler::ModifyVN(DBTablePartBase *partition, DBEntryBase *e)
}

bool MulticastGroupObject::CanBeDeleted() const {
if (local_olist_.size() == 0)
if ((local_olist_.size() == 0) && (vn_.get() == NULL))
return true;
return false;
}

MulticastGroupObject *MulticastHandler::CreateMulticastGroupObject
(const string &vrf_name, const Ip4Address &ip_addr, const string &vn_name,
(const string &vrf_name, const Ip4Address &ip_addr, const VnEntry *vn,
uint32_t vxlan_id) {
MulticastGroupObject *obj =
new MulticastGroupObject(vrf_name, ip_addr, vn_name);
new MulticastGroupObject(vrf_name, ip_addr, vn->GetName());
obj->set_vxlan_id(vxlan_id);
AddToMulticastObjList(obj);
obj->CreateEvpnMplsLabel(agent_);
return obj;
}

Expand Down Expand Up @@ -384,18 +388,12 @@ MulticastGroupObject *MulticastHandler::FindActiveGroupObject(
return obj;
}

void MulticastGroupObject::CreateEvpnMplsLabel(const Agent *agent) {
//Already added
if ((evpn_mpls_label_ != MplsLabel::INVALID) &&
(evpn_mpls_label_ != 0)) {
return;
}
void MulticastGroupObject::set_vn(const VnEntry *vn) {
vn_.reset(vn);
}

evpn_mpls_label_ = agent->mpls_table()->AllocLabel();
if (evpn_mpls_label_ == MplsLabel::INVALID) {
MCTRACE(Log, "allocation of evpn mpls label failed",
vrf_name_, "FF:FF:FF:FF:FF:FF", 0);
}
void MulticastGroupObject::reset_vn() {
vn_.reset();
}

ComponentNHKeyList
Expand Down Expand Up @@ -427,7 +425,7 @@ void MulticastHandler::TriggerLocalRouteChange(MulticastGroupObject *obj,
AgentRouteData *data =
BridgeAgentRouteTable::BuildNonBgpPeerData(obj->vrf_name(),
obj->GetVnName(),
obj->evpn_mpls_label(),
MplsTable::kInvalidLabel,
obj->vxlan_id(),
route_tunnel_bmap,
Composite::L2INTERFACE,
Expand Down Expand Up @@ -577,7 +575,7 @@ void MulticastHandler::AddVmInterfaceInFloodGroup(const VmInterface *vm_itf) {
all_broadcast = this->FindGroupObject(vrf_name, broadcast);
if (all_broadcast == NULL) {
all_broadcast = CreateMulticastGroupObject(vrf_name, broadcast,
vn_name, vn->GetVxLanId());
vn, vn->GetVxLanId());
add_route = true;
}

Expand Down Expand Up @@ -660,8 +658,7 @@ void MulticastHandler::ModifyEvpnMembers(const Peer *peer,

TriggerRemoteRouteChange(obj, peer, vrf_name, olist,
peer_identifier, delete_op, Composite::EVPN,
(obj ? obj->evpn_mpls_label() : 0), false,
ethernet_tag);
ethernet_tag, false, ethernet_tag);
MCTRACE(Log, "Add EVPN TOR Olist ", vrf_name, grp.to_string(), 0);
}

Expand All @@ -686,8 +683,7 @@ void MulticastHandler::ModifyTorMembers(const Peer *peer,

TriggerRemoteRouteChange(obj, peer, vrf_name, olist,
peer_identifier, delete_op, Composite::TOR,
(obj ? obj->evpn_mpls_label() : 0), false,
ethernet_tag);
MplsTable::kInvalidLabel, false, ethernet_tag);
MCTRACE(Log, "Add external TOR Olist ", vrf_name, grp.to_string(), 0);
}

Expand Down
16 changes: 7 additions & 9 deletions src/vnsw/agent/oper/multicast.h
View file
Expand Up @@ -55,8 +55,7 @@ class MulticastGroupObject {
const Ip4Address &grp_addr,
const std::string &vn_name) :
vrf_name_(vrf_name), grp_address_(grp_addr), vn_name_(vn_name),
evpn_mpls_label_(0), vxlan_id_(0), peer_identifier_(0),
deleted_(false) {
vxlan_id_(0), peer_identifier_(0), deleted_(false), vn_(NULL) {
boost::system::error_code ec;
src_address_ = IpAddress::from_string("0.0.0.0", ec).to_v4();
local_olist_.clear();
Expand All @@ -65,15 +64,12 @@ class MulticastGroupObject {
const Ip4Address &grp_addr,
const Ip4Address &src_addr) :
vrf_name_(vrf_name), grp_address_(grp_addr), src_address_(src_addr),
evpn_mpls_label_(0), vxlan_id_(0), peer_identifier_(0),
deleted_(false) {
vxlan_id_(0), peer_identifier_(0), deleted_(false), vn_(NULL) {
local_olist_.clear();
};
virtual ~MulticastGroupObject() { };

bool CanBeDeleted() const;
uint32_t evpn_mpls_label() const {return evpn_mpls_label_;}
void set_evpn_mpls_label(uint32_t label) {evpn_mpls_label_ = label;}

//Add local member is local VM in server.
bool AddLocalMember(const boost::uuids::uuid &intf_uuid) {
Expand Down Expand Up @@ -116,19 +112,21 @@ class MulticastGroupObject {
uint32_t vxlan_id() const {return vxlan_id_;}
void set_peer_identifier(uint64_t peer_id) {peer_identifier_ = peer_id;}
uint64_t peer_identifier() {return peer_identifier_;}
void CreateEvpnMplsLabel(const Agent *agent);
void set_vn(const VnEntry *vn);
void reset_vn();
const VnEntry *vn() const {return vn_.get();}

private:

std::string vrf_name_;
Ip4Address grp_address_;
std::string vn_name_;
Ip4Address src_address_;
uint32_t evpn_mpls_label_;
uint32_t vxlan_id_;
uint64_t peer_identifier_;
bool deleted_;
std::list<boost::uuids::uuid> local_olist_; /* UUID of local i/f */
VnEntryConstRef vn_;

friend class MulticastHandler;
DISALLOW_COPY_AND_ASSIGN(MulticastGroupObject);
Expand All @@ -144,7 +142,7 @@ class MulticastHandler {

MulticastGroupObject *CreateMulticastGroupObject(const string &vrf_name,
const Ip4Address &ip_addr,
const string &vn_name,
const VnEntry *vn,
uint32_t vxlan_id);

/* Called by XMPP to add ctrl node sent olist and label */
Expand Down
3 changes: 2 additions & 1 deletion src/vnsw/agent/oper/test/test_intf.cc
View file
Expand Up @@ -1830,7 +1830,7 @@ TEST_F(IntfTest, VmPortServiceVlanAdd_2) {
NovaDel(1);
ConfigDel(1);
client->WaitForIdle();
EXPECT_TRUE(VmPortFindRetDel(1) == false);
EXPECT_TRUE(VmPortFind(1) == false);

//Cleanup
DelNode("virtual-machine-interface", input[0].name);
Expand All @@ -1845,6 +1845,7 @@ TEST_F(IntfTest, VmPortServiceVlanAdd_2) {
DelVn("vn2");
DeleteVmportEnv(input, 1, true);
client->WaitForIdle();
EXPECT_TRUE(VmPortFindRetDel(1) == false);
EXPECT_FALSE(VrfFind("vrf1"));
EXPECT_FALSE(VrfFind("vrf2"));
}
Expand Down

0 comments on commit 1903658

Please sign in to comment.