Skip to content

Commit

Permalink
Keeping the mirror metadata entry in Flow entry
Browse files Browse the repository at this point in the history 
Currently mirror metadata is stored in index table using flow index
as key. Before the Flow eviction, the addition and deletion of this meta
data to Index table is always from Agent and hence no parallel
manipulation of Index table. After eviction, the metadata deletion from
index table can happen in parallel and hence leading to memory
corruption/leak in index table manipulation.

The right fix is making the index table SMP ready. As a stop gap fix,
the mirror metada data pointer is stored in the flow entry itself
directly.

Conflicts:
	include/vr_flow.h

closes-bug: #1572397, #1744369
Change-Id: I216d8b6c55025a6053b808fd2807338265b307e0
(cherry picked from commit a437faa)
  • Loading branch information
@divakardhar @haripk
divakardhar authored and haripk committed Mar 22, 2018
1 parent 6de6405 commit 9cb7dbd
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 55 deletions.
15 changes: 12 additions & 3 deletions dp-core/vr_flow.c
View file
Expand Up @@ -180,7 +180,10 @@ vr_flow_reset_mirror(struct vrouter *router, struct vr_flow_entry *fe,
if (fe->fe_flags & VR_FLOW_FLAG_MIRROR) {
fe->fe_mirror_id = VR_MAX_MIRROR_INDICES;
fe->fe_sec_mirror_id = VR_MAX_MIRROR_INDICES;
vr_mirror_meta_entry_del(router, index);
if (fe->fe_mme) {
vr_mirror_meta_entry_del(router, fe->fe_mme);
fe->fe_mme = NULL;
}
}
fe->fe_flags &= ~VR_FLOW_FLAG_MIRROR;
fe->fe_mirror_id = VR_MAX_MIRROR_INDICES;
Expand Down Expand Up @@ -1825,11 +1828,17 @@ vr_flow_set_mirror(struct vrouter *router, vr_flow_req *req,
}
}

if (req->fr_pcap_meta_data_size && req->fr_pcap_meta_data)
vr_mirror_meta_entry_set(router, req->fr_index,
if (req->fr_pcap_meta_data_size && req->fr_pcap_meta_data) {
if (fe->fe_mme) {
vr_mirror_meta_entry_del(router, fe->fe_mme);
fe->fe_mme = NULL;
}

fe->fe_mme = vr_mirror_meta_entry_set(router, req->fr_index,
req->fr_mir_sip, req->fr_mir_sport,
req->fr_pcap_meta_data, req->fr_pcap_meta_data_size,
req->fr_mir_vrf);
}

return;
}
Expand Down
72 changes: 26 additions & 46 deletions dp-core/vr_mirror.c
View file
Expand Up @@ -271,12 +271,12 @@ vr_mirror_meta_destructor(struct vrouter *router, void *arg)
}

static void
vr_mirror_meta_entry_destroy(unsigned int index, void *arg)
vr_mirror_meta_entry_destroy(struct vrouter *router,
struct vr_mirror_meta_entry *me)
{
struct vr_mirror_meta_entry *me = (struct vr_mirror_meta_entry *)arg;
struct vr_defer_data *defer;

if (me && me != VR_ITABLE_ERR_PTR) {
if (me) {
if (!vr_not_ready) {
defer = vr_get_defer_data(sizeof(*defer));
if (!defer) {
Expand All @@ -286,29 +286,31 @@ vr_mirror_meta_entry_destroy(unsigned int index, void *arg)
}
defer->vdd_data = (void *)me;
vr_defer(me->mirror_router, vr_mirror_meta_destructor, (void *)defer);
} else {
vr_mirror_meta_destroy(me);
}
}

return;
}

int
struct vr_mirror_meta_entry *
vr_mirror_meta_entry_set(struct vrouter *router, unsigned int index,
unsigned int mir_sip, unsigned short mir_sport,
void *meta_data, unsigned int meta_data_len,
unsigned short mirror_vrf)
{
char *buf;
struct vr_mirror_meta_entry *me, *me_old;
struct vr_mirror_meta_entry *me;

me = vr_malloc(sizeof(*me), VR_MIRROR_META_OBJECT);
if (!me)
return -ENOMEM;
return NULL;

buf = vr_malloc(meta_data_len, VR_MIRROR_META_OBJECT);
if (!buf) {
vr_free(me, VR_MIRROR_META_OBJECT);
return -ENOMEM;
return NULL;
}

memcpy(buf, meta_data, meta_data_len);
Expand All @@ -319,25 +321,31 @@ vr_mirror_meta_entry_set(struct vrouter *router, unsigned int index,
me->mirror_sport = mir_sport;
me->mirror_vrf = mirror_vrf;

me_old = vr_itable_set(router->vr_mirror_md, index, me);
if (me_old && me_old != VR_ITABLE_ERR_PTR)
vr_mirror_meta_entry_destroy(index, (void *)me_old);

return 0;
return me;
}

void
vr_mirror_meta_entry_del(struct vrouter *router, unsigned int index)
vr_mirror_meta_entry_del(struct vrouter *router,
struct vr_mirror_meta_entry *me)
{
struct vr_mirror_meta_entry *me;

me = vr_itable_del(router->vr_mirror_md, index);
if (me)
vr_mirror_meta_entry_destroy(index, (void *)me);
vr_mirror_meta_entry_destroy(router, (void *)me);

return;
}

static struct vr_mirror_meta_entry *
vr_mirror_meta_entry_get(struct vrouter *router, unsigned int flow_index)
{
struct vr_flow_entry *fe;

fe = vr_flow_get_entry(router, flow_index);
if (fe)
return fe->fe_mme;

return NULL;
}

int
vr_mirror(struct vrouter *router, uint8_t mirror_id, struct vr_packet *pkt,
struct vr_forwarding_md *fmd, mirror_type_t mtype)
Expand Down Expand Up @@ -393,8 +401,7 @@ vr_mirror(struct vrouter *router, uint8_t mirror_id, struct vr_packet *pkt,

if (mtype == MIRROR_TYPE_ACL) {
if (fmd->fmd_flow_index >= 0) {
mme = (struct vr_mirror_meta_entry *)
vr_itable_get(router->vr_mirror_md, fmd->fmd_flow_index);
mme = vr_mirror_meta_entry_get(router, fmd->fmd_flow_index);
if (mme) {
mirror_md_len = mme->mirror_md_len;
mirror_md = mme->mirror_md;
Expand Down Expand Up @@ -511,12 +518,6 @@ vr_mirror_exit(struct vrouter *router, bool soft_reset)
if (router->vr_mirrors[i])
__vr_mirror_del(router, i);

if (router->vr_mirror_md) {
vr_itable_delete(router->vr_mirror_md,
vr_mirror_meta_entry_destroy);
router->vr_mirror_md = NULL;
}

if (!soft_reset) {
vr_free(router->vr_mirrors, VR_MIRROR_TABLE_OBJECT);
router->vr_mirrors = NULL;
Expand All @@ -529,7 +530,6 @@ vr_mirror_exit(struct vrouter *router, bool soft_reset)
int
vr_mirror_init(struct vrouter *router)
{
int ret = 0;
unsigned int size;

if (!router->vr_mirrors) {
Expand All @@ -540,27 +540,7 @@ vr_mirror_init(struct vrouter *router)
return vr_module_error(-ENOMEM, __FUNCTION__, __LINE__, size);
}

if (!router->vr_mirror_md) {
router->vr_mirror_md = vr_itable_create(32, 4, 8, 8, 8, 8);
if (!router->vr_mirror_md && (ret = -ENOMEM)) {
vr_module_error(ret, __FUNCTION__, __LINE__, 0);
goto cleanup;
}
}

return 0;

cleanup:
if (router->vr_mirrors) {
vr_free(router->vr_mirrors, VR_MIRROR_TABLE_OBJECT);
router->vr_mirrors = NULL;
}

if (router->vr_mirror_md) {
vr_itable_delete(router->vr_mirror_md, vr_mirror_meta_entry_destroy);
router->vr_mirror_md = NULL;
}

return ret;
}

2 changes: 2 additions & 0 deletions include/vr_flow.h
View file
Expand Up @@ -350,6 +350,7 @@ struct vr_dummy_flow_entry {
uint8_t fe_type;
unsigned short fe_udp_src_port;
uint32_t fe_src_info;
struct vr_mirror_meta_entry *fe_mme;;
} __attribute__((packed));

#define VR_FLOW_ENTRY_PACK (128 - sizeof(struct vr_dummy_flow_entry))
Expand Down Expand Up @@ -384,6 +385,7 @@ struct vr_flow_entry {
* component NH as this source
*/
uint32_t fe_src_info;
struct vr_mirror_meta_entry *fe_mme;
unsigned char fe_pack[VR_FLOW_ENTRY_PACK];
} __attribute__((packed));

Expand Down
12 changes: 7 additions & 5 deletions include/vr_mirror.h
View file
Expand Up @@ -53,10 +53,12 @@ extern int vr_mirror(struct vrouter *, uint8_t, struct vr_packet *,
struct vr_forwarding_md *, mirror_type_t);
extern struct vr_mirror_entry *vrouter_get_mirror(unsigned int, unsigned int);
extern int vrouter_put_mirror(struct vrouter *, unsigned int);
extern int vr_mirror_meta_entry_set(struct vrouter *, unsigned int,
unsigned int, unsigned short,
void *, unsigned int,
unsigned short);
extern void vr_mirror_meta_entry_del(struct vrouter *, unsigned int);
extern struct vr_mirror_meta_entry *
vr_mirror_meta_entry_set(struct vrouter *, unsigned int,
unsigned int, unsigned short,
void *, unsigned int,
unsigned short);
extern void vr_mirror_meta_entry_del(struct vrouter *,
struct vr_mirror_meta_entry *);

#endif /* __VR_MIRROR_H__ */
1 change: 0 additions & 1 deletion include/vrouter.h
View file
Expand Up @@ -353,7 +353,6 @@ struct vrouter {
unsigned int vr_max_vrfs;
unsigned int vr_max_mirror_indices;
struct vr_mirror_entry **vr_mirrors;
vr_itable_t vr_mirror_md;
vr_itable_t vr_vxlan_table;

vr_htable_t vr_fragment_table;
Expand Down

0 comments on commit 9cb7dbd

Please sign in to comment.