Skip to content

Commit

Permalink
Removing ref counting for Mirror entry
Browse files Browse the repository at this point in the history 
Flow and VIF hold Mirroring index rather reference to mirroring entry.
Though only index is used, the ref counting is used to delete the
mirroring entry when Agent deletes it. This is unncecessary as no other
data structure is holding a reference to this. Also the current ref
counting is not atomic and is leading to wrong ref counting, leading to
mirroring entry's wrong deletion.

As a fix, the ref counting is remvoed for mirroring. When Agent deletes
the entry, it is removed from Mirroring database. While mirroring if
mirroring entry is not found the packet just does not get mirrored.

closes-bug: #1605874

Change-Id: Id6bce2425465402a456a989a72a5f142cf829e83
  • Loading branch information
@divakardhar
divakardhar committed Jul 26, 2016
1 parent 10655af commit 45776e5
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 89 deletions.
4 changes: 0 additions & 4 deletions dp-core/vr_flow.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,7 @@ vr_flow_reset_mirror(struct vrouter *router, struct vr_flow_entry *fe,
unsigned int index)
{
if (fe->fe_flags & VR_FLOW_FLAG_MIRROR) {
vrouter_put_mirror(router, fe->fe_mirror_id);
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);
}
Expand Down Expand Up @@ -1556,7 +1554,6 @@ vr_flow_set_mirror(struct vrouter *router, vr_flow_req *req,

if (fe->fe_mirror_id != req->fr_mir_id) {
if (fe->fe_mirror_id < router->vr_max_mirror_indices) {
vrouter_put_mirror(router, fe->fe_mirror_id);
fe->fe_mirror_id = router->vr_max_mirror_indices;
}

Expand All @@ -1575,7 +1572,6 @@ vr_flow_set_mirror(struct vrouter *router, vr_flow_req *req,

if (fe->fe_sec_mirror_id != req->fr_sec_mir_id) {
if (fe->fe_sec_mirror_id < router->vr_max_mirror_indices) {
vrouter_put_mirror(router, fe->fe_sec_mirror_id);
fe->fe_sec_mirror_id = router->vr_max_mirror_indices;
}

Expand Down
123 changes: 47 additions & 76 deletions dp-core/vr_mirror.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,8 @@
#include "vr_message.h"
#include "vr_mirror.h"

int vr_mirror_add(vr_mirror_req *);
int vr_mirror_del(vr_mirror_req *);

static struct vr_mirror_entry *
__vrouter_get_mirror(unsigned int rid, unsigned int index)
struct vr_mirror_entry *
vrouter_get_mirror(unsigned int rid, unsigned int index)
{
struct vrouter *router = vrouter_get(rid);

Expand All @@ -24,22 +21,22 @@ __vrouter_get_mirror(unsigned int rid, unsigned int index)
return router->vr_mirrors[index];
}

struct vr_mirror_entry *
vrouter_get_mirror(unsigned int rid, unsigned int index)
static void
vr_mirror_defer_delete(struct vrouter *router, void *arg)
{
struct vr_mirror_entry *mirror;
struct vr_defer_data *defer = (struct vr_defer_data *)arg;

mirror = __vrouter_get_mirror(rid, index);
if (mirror)
mirror->mir_users++;
vr_free(defer->vdd_data, VR_MIRROR_OBJECT);

return mirror;
return;
}

int
vrouter_put_mirror(struct vrouter *router, unsigned int index)
__vr_mirror_del(struct vrouter *router, unsigned int index)
{
struct vr_nexthop *nh;
struct vr_mirror_entry *mirror;
struct vr_defer_data *defer;

if (index >= router->vr_max_mirror_indices)
return -EINVAL;
Expand All @@ -48,83 +45,48 @@ vrouter_put_mirror(struct vrouter *router, unsigned int index)
if (!mirror)
return -EINVAL;

if (!--mirror->mir_users) {
router->vr_mirrors[index] = NULL;

if (!vr_not_ready)
nh = mirror->mir_nh;
router->vr_mirrors[index] = NULL;
mirror->mir_nh = NULL;

if (!vr_not_ready) {
defer = vr_get_defer_data(sizeof(*defer));
if (defer) {
defer->vdd_data = (void *)mirror;
vr_defer(router, vr_mirror_defer_delete, (void *)defer);
} else {
vr_delay_op();

vrouter_put_nexthop(mirror->mir_nh);
vr_free(mirror, VR_MIRROR_OBJECT);
}
} else {
vr_free(mirror, VR_MIRROR_OBJECT);
}
vrouter_put_nexthop(nh);

return 0;
}


int
vr_mirror_del(vr_mirror_req *req)
{
int ret = 0;
int ret = -EINVAL;
struct vrouter *router;
struct vr_mirror_entry *mirror;
struct vr_nexthop *nh;

router = vrouter_get(req->mirr_rid);
if (!router) {
ret = -EINVAL;
goto generate_resp;
}

mirror = __vrouter_get_mirror(req->mirr_rid, req->mirr_index);
if (!mirror) {
ret = -EINVAL;
goto generate_resp;
}

mirror->mir_flags |= VR_MIRROR_FLAG_MARKED_DELETE;
nh = mirror->mir_nh;
mirror->mir_nh = vrouter_get_nexthop(req->mirr_rid, NH_DISCARD_ID);
/* release the old nexthop */
vrouter_put_nexthop(nh);
if (router)
ret = __vr_mirror_del(router, req->mirr_index);

/* ...and finally try to release the mirror entry */
vrouter_put_mirror(router, req->mirr_index);

generate_resp:
vr_send_response(ret);

return ret;
}

static int
vr_mirror_change(struct vr_mirror_entry *mirror, vr_mirror_req *req,
struct vr_nexthop *nh_new)
{
struct vr_nexthop *nh_old = mirror->mir_nh;

if (mirror->mir_flags & VR_MIRROR_FLAG_MARKED_DELETE)
mirror->mir_users++;

mirror->mir_flags = req->mirr_flags;
mirror->mir_flags &= ~VR_MIRROR_FLAG_MARKED_DELETE;
mirror->mir_rid = req->mirr_rid;
mirror->mir_vni = req->mirr_vni;
if (nh_old != nh_new) {
mirror->mir_nh = nh_new;
if (nh_old)
vrouter_put_nexthop(nh_old);
}

return 0;
}

int
vr_mirror_add(vr_mirror_req *req)
{
struct vrouter *router;
struct vr_nexthop *nh;
int ret = 0;
struct vrouter *router;
struct vr_nexthop *nh, *old_nh = NULL;
struct vr_mirror_entry *mirror;

router = vrouter_get(req->mirr_rid);
Expand All @@ -138,22 +100,33 @@ vr_mirror_add(vr_mirror_req *req)
goto generate_resp;
}

req->mirr_flags &= ~VR_MIRROR_FLAG_MARKED_DELETE;

nh = vrouter_get_nexthop(req->mirr_rid, req->mirr_nhid);
if (!nh) {
ret = -EINVAL;
goto generate_resp;
}

mirror = __vrouter_get_mirror(req->mirr_rid, req->mirr_index);
mirror = router->vr_mirrors[req->mirr_index];
if (!mirror) {
mirror = vr_zalloc(sizeof(*mirror), VR_MIRROR_OBJECT);
mirror->mir_users++;
if (!mirror) {
ret = -ENOMEM;
vrouter_put_nexthop(nh);
goto generate_resp;
}
} else {
old_nh = mirror->mir_nh;
}
ret = vr_mirror_change(mirror, req, nh);

mirror->mir_nh = nh;
mirror->mir_rid = req->mirr_rid;
mirror->mir_flags = req->mirr_flags;
mirror->mir_vni = req->mirr_vni;
router->vr_mirrors[req->mirr_index] = mirror;

if (old_nh)
vrouter_put_nexthop(old_nh);

generate_resp:
vr_send_response(ret);

Expand All @@ -169,7 +142,6 @@ vr_mirror_make_req(vr_mirror_req *req, struct vr_mirror_entry *mirror,
if (mirror->mir_nh)
req->mirr_nhid = mirror->mir_nh->nh_id;

req->mirr_users = mirror->mir_users;
req->mirr_flags = mirror->mir_flags;
req->mirr_rid = mirror->mir_rid;
req->mirr_vni = mirror->mir_vni;
Expand Down Expand Up @@ -226,12 +198,11 @@ vr_mirror_get(vr_mirror_req *req)
(unsigned int)req->mirr_index >= router->vr_max_mirror_indices) {
ret = -ENODEV;
} else {
mirror = __vrouter_get_mirror(req->mirr_rid, req->mirr_index);
mirror = router->vr_mirrors[req->mirr_index];
if (!mirror)
ret = -ENOENT;
}


if (mirror) {
vr_mirror_make_req(req, mirror, req->mirr_index);
} else
Expand Down Expand Up @@ -541,7 +512,7 @@ vr_mirror_exit(struct vrouter *router, bool soft_reset)
if (router->vr_mirrors)
for (i = 0; i < router->vr_max_mirror_indices; i++)
if (router->vr_mirrors[i])
vrouter_put_mirror(router, i);
__vr_mirror_del(router, i);

if (router->vr_mirror_md) {
vr_itable_delete(router->vr_mirror_md,
Expand Down
7 changes: 3 additions & 4 deletions include/vr_mirror.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
#define __VR_MIRROR_H__

#define VR_MAX_MIRROR_INDICES 255
#define VR_MIRROR_FLAG_MARKED_DELETE 0x1
#define VR_MIRROR_FLAG_DYNAMIC 0x2

#define VR_MIRROR_FLAG_DYNAMIC 0x1


struct vrouter;
Expand All @@ -29,10 +29,9 @@ typedef enum {


struct vr_mirror_entry {
unsigned int mir_users:20;
unsigned int mir_flags:12;
unsigned int mir_rid;
int mir_vni;
unsigned int mir_flags;
struct vr_nexthop *mir_nh;
};

Expand Down
7 changes: 2 additions & 5 deletions utils/mirror.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,9 @@ vr_mirror_req_process(void *s_req)
memset(flags, 0, sizeof(flags));
if (req->mirr_flags & VR_MIRROR_FLAG_DYNAMIC)
strcat(flags, "D");
if (req->mirr_flags & VR_MIRROR_FLAG_MARKED_DELETE)
strcat(flags, "Md");

printf("%5d %7d", req->mirr_index, req->mirr_nhid);
printf(" %4s", flags);
printf(" %7u", req->mirr_users);
if (req->mirr_vni != -1)
printf(" %7d", req->mirr_vni);
printf("\n");
Expand Down Expand Up @@ -311,8 +308,8 @@ int main(int argc, char *argv[])
if ((mirror_op == SANDESH_OP_DUMP) ||
(mirror_op == SANDESH_OP_GET)) {
printf("Mirror Table\n\n");
printf("Flags:D=Dynamic Mirroring, Dm:Delete Marked\n\n");
printf("Index NextHop Flags References VNI\n");
printf("Flags:D=Dynamic Mirroring \n\n");
printf("Index NextHop Flags VNI\n");
printf("------------------------------------------------\n");
}

Expand Down

0 comments on commit 45776e5

Please sign in to comment.