Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xia/lpm: remove routing/forwarding deadlock #17

Open
wants to merge 2 commits into
base: xia
Choose a base branch
from
Open

xia/lpm: remove routing/forwarding deadlock #17

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6d895df
xia/lpm: remove routing/forwarding deadlock
cjdoucette May 16, 2016
407b4ff
xia: add fxid_copy to FIB interface
cjdoucette Sep 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions include/net/xia_fib.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,16 @@ struct xia_ppal_rt_iops {
*/
void (*fxid_init)(struct fib_xid *fxid, int table_id, int entry_type);

/* fxid_copy - copy @old_fxid, including FIB-specific data.
*
* NOTE
* This function must copy any FIB-specific data in addition
* to the fields from struct fib_xid. @old_fxid and @new_fxid
* should reference FIB entries previously allocated by
* fxid_ppal_alloc().
*/
void (*fxid_copy)(struct fib_xid *old_fxid, struct fib_xid *new_fxid);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind swapping the position of the parameters? The assigning order is followed by the C library, and it bothers to go against it. A few examples: memcpy(), memmove(), and strcpy().


/** fxid_find_rcu - Find struct fib_xid in @xtbl that has key @xid.
*
* RETURN
Expand Down
7 changes: 7 additions & 0 deletions net/xia/list_fib.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,12 @@ static void list_fxid_init(struct fib_xid *fxid, int table_id, int entry_type)
fxid->dead.xtbl = NULL;
}

static void list_fxid_copy(struct fib_xid *old_fxid, struct fib_xid *new_fxid)
{
memcpy(new_fxid, old_fxid,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it's unlikely that new_fxid == old_fxid, it would be safer to replace memcpy() with memmove().

sizeof(*old_fxid) + sizeof(struct list_fib_xid));
}

static void list_xtbl_death_work(struct work_struct *work)
{
struct fib_xid_table *xtbl = container_of(work, struct fib_xid_table,
Expand Down Expand Up @@ -621,6 +627,7 @@ const struct xia_ppal_rt_iops xia_ppal_list_rt_iops = {

.fxid_ppal_alloc = list_fxid_ppal_alloc,
.fxid_init = list_fxid_init,
.fxid_copy = list_fxid_copy,

.fxid_find_rcu = list_fxid_find_rcu,
.fxid_find_lock = list_fxid_find_lock,
Expand Down
2 changes: 2 additions & 0 deletions net/xia/ppal_lpm/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ static int newroute_flush_anchor_unlock(struct fib_xid_table *xtbl,
* node with the new one in the tree.
*/
xdst_init_anchor(&dup_llpm->anchor);
lpm_rt_iops->fxid_copy(&dup_llpm->common, pred_fxid);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you've inverted the parameters of fxid_copy() above, you fixed a bug here.

lpm_rt_iops->fxid_replace_locked(xtbl, pred_fxid,
&dup_llpm->common);

Expand Down Expand Up @@ -136,6 +137,7 @@ static int newroute_flush_anchor_unlock(struct fib_xid_table *xtbl,
}

dup_mrd->gw = fxid_mrd(pred_fxid)->gw;
lpm_rt_iops->fxid_copy(&dup_mrd->common, pred_fxid);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you've inverted the parameters of fxid_copy() above, you fixed another bug here.

lpm_rt_iops->fxid_replace_locked(xtbl, pred_fxid,
&dup_mrd->common);

Expand Down
7 changes: 7 additions & 0 deletions net/xia/ppal_lpm/tree_fib.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,12 @@ static void tree_fxid_init(struct fib_xid *fxid, int table_id, int entry_type)
fxid->dead.xtbl = NULL;
}

static void tree_fxid_copy(struct fib_xid *old_fxid, struct fib_xid *new_fxid)
{
memcpy(new_fxid, old_fxid,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it's unlikely that new_fxid == old_fxid, it would be safer to replace memcpy() with memmove().

sizeof(*old_fxid) + sizeof(struct tree_fib_xid));
}

static inline void disconnect_from_parent(struct tree_fib_xid *node)
{
if (node && node->parent) {
Expand Down Expand Up @@ -683,6 +689,7 @@ const struct xia_ppal_rt_iops xia_ppal_tree_rt_iops = {

.fxid_ppal_alloc = tree_fxid_ppal_alloc,
.fxid_init = tree_fxid_init,
.fxid_copy = tree_fxid_copy,

/* Note that there is no RCU-specific version. */
.fxid_find_rcu = tree_fxid_find_rcu,
Expand Down