Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
52 changes: 39 additions & 13 deletions src/network/tc/qdisc.c
Original file line number Diff line number Diff line change
Expand Up @@ -285,31 +285,57 @@ int link_find_qdisc(Link *link, uint32_t handle, const char *kind, QDisc **ret)
return -ENOENT;
}

QDisc* qdisc_drop(QDisc *qdisc) {
void qdisc_mark_recursive(QDisc *qdisc) {
TClass *tclass;
Link *link;

assert(qdisc);
assert(qdisc->link);

if (qdisc_is_marked(qdisc))
return;

link = ASSERT_PTR(qdisc->link);
qdisc_mark(qdisc);

/* also drop all child classes assigned to the qdisc. */
SET_FOREACH(tclass, link->tclasses) {
/* also mark all child classes assigned to the qdisc. */
SET_FOREACH(tclass, qdisc->link->tclasses) {
if (TC_H_MAJ(tclass->classid) != qdisc->handle)
continue;

tclass_drop(tclass);
tclass_mark_recursive(tclass);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Use iterative marking to prevent deep-tree stack overflow

When deleting a sufficiently deep alternating qdisc/class hierarchy, this call still mutually recurses through tclass_mark_recursive() and back into qdisc_mark_recursive(), consuming one or more stack frames per tree level and potentially crashing networkd before the sweep begins. The two-phase approach prevents concurrent set modification, but it does not address the stated stack-overflow scenario; the marking traversal needs an explicit work queue or stack.

Useful? React with 👍 / 👎.

}
}

qdisc_enter_removed(qdisc);
void link_qdisc_drop_marked(Link *link) {
QDisc *qdisc;

if (qdisc->state == 0) {
log_qdisc_debug(qdisc, link, "Forgetting");
qdisc = qdisc_free(qdisc);
} else
log_qdisc_debug(qdisc, link, "Removed");
assert(link);

SET_FOREACH(qdisc, link->qdiscs) {
if (!qdisc_is_marked(qdisc))
continue;

qdisc_unmark(qdisc);
qdisc_enter_removed(qdisc);

if (qdisc->state == 0) {
log_qdisc_debug(qdisc, link, "Forgetting");
qdisc_free(qdisc);
} else
log_qdisc_debug(qdisc, link, "Removed");
}
}

QDisc* qdisc_drop(QDisc *qdisc) {
assert(qdisc);
assert(qdisc->link);

qdisc_mark_recursive(qdisc);

/* link_qdisc_drop_marked() may invalidate qdisc, so run link_tclass_drop_marked() first. */
link_tclass_drop_marked(qdisc->link);
link_qdisc_drop_marked(qdisc->link);

return qdisc;
return NULL;
}

static int qdisc_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, QDisc *qdisc) {
Expand Down
2 changes: 2 additions & 0 deletions src/network/tc/qdisc.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ DEFINE_NETWORK_CONFIG_STATE_FUNCTIONS(QDisc, qdisc);
QDisc* qdisc_free(QDisc *qdisc);
int qdisc_new_static(QDiscKind kind, Network *network, const char *filename, unsigned section_line, QDisc **ret);

void qdisc_mark_recursive(QDisc *qdisc);
QDisc* qdisc_drop(QDisc *qdisc);
void link_qdisc_drop_marked(Link *link);

int link_find_qdisc(Link *link, uint32_t handle, const char *kind, QDisc **qdisc);

Expand Down
51 changes: 38 additions & 13 deletions src/network/tc/tclass.c
Original file line number Diff line number Diff line change
Expand Up @@ -252,31 +252,56 @@ static void log_tclass_debug(TClass *tclass, Link *link, const char *str) {
strna(tclass_get_tca_kind(tclass)));
}

TClass* tclass_drop(TClass *tclass) {
void tclass_mark_recursive(TClass *tclass) {
QDisc *qdisc;
Link *link;

assert(tclass);
assert(tclass->link);

if (tclass_is_marked(tclass))
return;

link = ASSERT_PTR(tclass->link);
tclass_mark(tclass);

/* Also drop all child qdiscs assigned to the class. */
SET_FOREACH(qdisc, link->qdiscs) {
/* Also mark all child qdiscs assigned to the class. */
SET_FOREACH(qdisc, tclass->link->qdiscs) {
if (qdisc->parent != tclass->classid)
continue;

qdisc_drop(qdisc);
qdisc_mark_recursive(qdisc);
}
}

tclass_enter_removed(tclass);
void link_tclass_drop_marked(Link *link) {
TClass *tclass;

if (tclass->state == 0) {
log_tclass_debug(tclass, link, "Forgetting");
tclass = tclass_free(tclass);
} else
log_tclass_debug(tclass, link, "Removed");
assert(link);

SET_FOREACH(tclass, link->tclasses) {
if (!tclass_is_marked(tclass))
continue;

tclass_unmark(tclass);
tclass_enter_removed(tclass);

if (tclass->state == 0) {
log_tclass_debug(tclass, link, "Forgetting");
tclass_free(tclass);
} else
log_tclass_debug(tclass, link, "Removed");
}
}

TClass* tclass_drop(TClass *tclass) {
assert(tclass);

tclass_mark_recursive(tclass);

/* link_tclass_drop_marked() may invalidate tclass, so run link_qdisc_drop_marked() first. */
link_qdisc_drop_marked(tclass->link);
link_tclass_drop_marked(tclass->link);

return tclass;
return NULL;
}

static int tclass_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, TClass *tclass) {
Expand Down
2 changes: 2 additions & 0 deletions src/network/tc/tclass.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ DEFINE_NETWORK_CONFIG_STATE_FUNCTIONS(TClass, tclass);
TClass* tclass_free(TClass *tclass);
int tclass_new_static(TClassKind kind, Network *network, const char *filename, unsigned section_line, TClass **ret);

void tclass_mark_recursive(TClass *tclass);
TClass* tclass_drop(TClass *tclass);
void link_tclass_drop_marked(Link *link);

int link_find_tclass(Link *link, uint32_t classid, TClass **ret);

Expand Down
Loading