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.

closes-bug: #1572397

Change-Id: I216d8b6c55025a6053b808fd2807338265b307e0
  • Loading branch information
@divakardhar
divakardhar committed Jul 26, 2016
1 parent 10655af commit e906799
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,
fe->fe_mirror_id = VR_MAX_MIRROR_INDICES;
vrouter_put_mirror(router, fe->fe_sec_mirror_id);
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 @@ -1586,11 +1589,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 @@ -298,12 +298,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 @@ -313,29 +313,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 @@ -346,25 +348,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 @@ -422,8 +430,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 @@ -543,12 +550,6 @@ vr_mirror_exit(struct vrouter *router, bool soft_reset)
if (router->vr_mirrors[i])
vrouter_put_mirror(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 @@ -561,7 +562,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 @@ -572,27 +572,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 @@ -321,6 +321,7 @@ struct vr_dummy_flow_entry {
uint8_t fe_drop_reason;
uint8_t fe_type;
unsigned short fe_udp_src_port;
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 @@ -348,6 +349,7 @@ struct vr_flow_entry {
uint8_t fe_drop_reason;
uint8_t fe_type;
unsigned short fe_udp_src_port;
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 @@ -283,7 +283,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;

struct vr_btable *vr_fragment_table;
Expand Down

0 comments on commit e906799

Please sign in to comment.