Skip to content

Commit

Permalink
Modify hentry flags before adding to free list
Browse files Browse the repository at this point in the history
Currently when a flow entry is deleted, the corresponding hash entry is
kept at the head of the free list and after successfully inserting into
the list the entry's flags are reset. If any other core is
attempting to fetch a free entry, there is a likely chance that just
added entry will be given as free entry before the flags are reset as
this entry is at the head of the list. Because of this there is a race
condition for setting the flags between deletion of entry and additin of
entry. If deletion overwrites the addition flags, the Valid flags gets
overwritten. Because of this the entry would not be in free list but
still would not be considered a valid entry. Searching this entry by
index fails as this is not a valid entry.

As a fix, the flags while inserting the entry to free list are modified
before inserting so that there would not be any race condition.

Change-Id: I67b1077e75a8a9f4ca5ddcfe3a7513061fd20681
closes-bug: #1552544
  • Loading branch information
divakardhar committed Mar 16, 2016
1 parent 81ac86b commit 5d2fcbc
Showing 1 changed file with 19 additions and 19 deletions.
38 changes: 19 additions & 19 deletions dp-core/vr_htable.c
Expand Up @@ -95,12 +95,14 @@ vr_htable_put_free_oentry(struct vr_htable *table, vr_hentry_t *ent)

vr_hentry_t *tmp;

if (!table || !ent)
if (!table || !ent || ent->hentry_index < table->ht_hentries)
return;

if (ent->hentry_flags & VR_HENTRY_FLAG_IN_FREE_LIST)
return;

ent->hentry_flags |= VR_HENTRY_FLAG_IN_FREE_LIST;

tmp = NULL;
do {

Expand All @@ -113,7 +115,6 @@ vr_htable_put_free_oentry(struct vr_htable *table, vr_hentry_t *ent)
if (__sync_bool_compare_and_swap(&table->ht_free_oentry_head,
tmp, ent)) {
(void)__sync_sub_and_fetch(&table->ht_used_oentries, 1);
ent->hentry_flags |= VR_HENTRY_FLAG_IN_FREE_LIST;
return;
}

Expand Down Expand Up @@ -160,23 +161,22 @@ vr_htable_get_hentry_by_index(vr_htable_t htable, unsigned int index)
}

static void
vr_htable_hentry_invalidate(struct vr_htable *table, vr_hentry_t *ent)
vr_htable_oentry_invalidate(struct vr_htable *table, vr_hentry_t *ent)
{
if (!table || !ent)
if (!table || !ent || (ent->hentry_index < table->ht_hentries))
return;

/* Clear all the flags except free list */
ent->hentry_flags &= VR_HENTRY_FLAG_IN_FREE_LIST;
ent->hentry_bucket_index = VR_INVALID_HENTRY_INDEX;
ent->hentry_next_index = VR_INVALID_HENTRY_INDEX;
ent->hentry_next = NULL;

if (ent->hentry_index >= table->ht_hentries) {
ent->hentry_next_index = VR_INVALID_HENTRY_INDEX;
vr_htable_put_free_oentry(table, ent);
}

ent->hentry_flags &= VR_HENTRY_FLAG_IN_FREE_LIST;
vr_htable_put_free_oentry(table, ent);
}

static void
__vr_htable_hentry_invalidate(struct vr_htable *table, vr_hentry_t *ent)
__vr_htable_oentry_invalidate(struct vr_htable *table, vr_hentry_t *ent)
{
vr_hentry_t *prev, *head_ent;

Expand Down Expand Up @@ -205,7 +205,7 @@ __vr_htable_hentry_invalidate(struct vr_htable *table, vr_hentry_t *ent)
}
}

vr_htable_hentry_invalidate(table, ent);
vr_htable_oentry_invalidate(table, ent);
}

return;
Expand All @@ -224,7 +224,7 @@ vr_htable_hentry_defer_delete(struct vrouter *router, void *arg)

ent = __vr_htable_get_hentry_by_index((vr_htable_t)table,
defer_data->hd_index);
vr_htable_hentry_invalidate(table, ent);
vr_htable_oentry_invalidate(table, ent);

return;
}
Expand Down Expand Up @@ -318,7 +318,7 @@ vr_htable_hentry_scheduled_delete(void *arg)
vr_htable_hentry_defer_delete, (void *)defer_data);
}
} else {
vr_htable_hentry_invalidate(table, ent);
vr_htable_oentry_invalidate(table, ent);
ent = next;
continue;
}
Expand All @@ -345,17 +345,17 @@ vr_htable_release_hentry(vr_htable_t htable, vr_hentry_t *ent)
if (!(ent->hentry_flags & VR_HENTRY_FLAG_VALID))
return;

if (ent->hentry_index < table->ht_hentries) {
vr_htable_hentry_invalidate(table, ent);
/* Mark it as Invalid */
ent->hentry_flags &= ~VR_HENTRY_FLAG_VALID;

if (ent->hentry_index < table->ht_hentries)
return;
}

if (vr_not_ready) {
__vr_htable_hentry_invalidate(table, ent);
__vr_htable_oentry_invalidate(table, ent);
return;
}

ent->hentry_flags &= ~VR_HENTRY_FLAG_VALID;
ent->hentry_flags |= VR_HENTRY_FLAG_DELETE_MARKED;

head_ent = __vr_htable_get_hentry_by_index(htable, ent->hentry_bucket_index);
Expand Down

0 comments on commit 5d2fcbc

Please sign in to comment.