Skip to content

Commit 18d3eef

Browse files
w1ldptrdavem330
authored andcommitted
net: sched: refactor tcf_block_find() into standalone functions
Refactor tcf_block_find() code into three standalone functions: - __tcf_qdisc_find() to lookup Qdisc and increment its reference counter. - __tcf_qdisc_cl_find() to lookup class. - __tcf_block_find() to lookup block and increment its reference counter. This change is necessary to allow netlink tc rule update handlers to call these functions directly in order to conditionally take rtnl lock according to Qdisc class ops flags before calling any of class ops functions. Signed-off-by: Vlad Buslov <vladbu@mellanox.com> Acked-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent dfcd2a2 commit 18d3eef

File tree

1 file changed

+149
-92
lines changed

1 file changed

+149
-92
lines changed

net/sched/cls_api.c

Lines changed: 149 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,6 +1101,142 @@ static void tcf_block_flush_all_chains(struct tcf_block *block, bool rtnl_held)
11011101
}
11021102
}
11031103

1104+
/* Lookup Qdisc and increments its reference counter.
1105+
* Set parent, if necessary.
1106+
*/
1107+
1108+
static int __tcf_qdisc_find(struct net *net, struct Qdisc **q,
1109+
u32 *parent, int ifindex, bool rtnl_held,
1110+
struct netlink_ext_ack *extack)
1111+
{
1112+
const struct Qdisc_class_ops *cops;
1113+
struct net_device *dev;
1114+
int err = 0;
1115+
1116+
if (ifindex == TCM_IFINDEX_MAGIC_BLOCK)
1117+
return 0;
1118+
1119+
rcu_read_lock();
1120+
1121+
/* Find link */
1122+
dev = dev_get_by_index_rcu(net, ifindex);
1123+
if (!dev) {
1124+
rcu_read_unlock();
1125+
return -ENODEV;
1126+
}
1127+
1128+
/* Find qdisc */
1129+
if (!*parent) {
1130+
*q = dev->qdisc;
1131+
*parent = (*q)->handle;
1132+
} else {
1133+
*q = qdisc_lookup_rcu(dev, TC_H_MAJ(*parent));
1134+
if (!*q) {
1135+
NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists");
1136+
err = -EINVAL;
1137+
goto errout_rcu;
1138+
}
1139+
}
1140+
1141+
*q = qdisc_refcount_inc_nz(*q);
1142+
if (!*q) {
1143+
NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists");
1144+
err = -EINVAL;
1145+
goto errout_rcu;
1146+
}
1147+
1148+
/* Is it classful? */
1149+
cops = (*q)->ops->cl_ops;
1150+
if (!cops) {
1151+
NL_SET_ERR_MSG(extack, "Qdisc not classful");
1152+
err = -EINVAL;
1153+
goto errout_qdisc;
1154+
}
1155+
1156+
if (!cops->tcf_block) {
1157+
NL_SET_ERR_MSG(extack, "Class doesn't support blocks");
1158+
err = -EOPNOTSUPP;
1159+
goto errout_qdisc;
1160+
}
1161+
1162+
errout_rcu:
1163+
/* At this point we know that qdisc is not noop_qdisc,
1164+
* which means that qdisc holds a reference to net_device
1165+
* and we hold a reference to qdisc, so it is safe to release
1166+
* rcu read lock.
1167+
*/
1168+
rcu_read_unlock();
1169+
return err;
1170+
1171+
errout_qdisc:
1172+
rcu_read_unlock();
1173+
1174+
if (rtnl_held)
1175+
qdisc_put(*q);
1176+
else
1177+
qdisc_put_unlocked(*q);
1178+
*q = NULL;
1179+
1180+
return err;
1181+
}
1182+
1183+
static int __tcf_qdisc_cl_find(struct Qdisc *q, u32 parent, unsigned long *cl,
1184+
int ifindex, struct netlink_ext_ack *extack)
1185+
{
1186+
if (ifindex == TCM_IFINDEX_MAGIC_BLOCK)
1187+
return 0;
1188+
1189+
/* Do we search for filter, attached to class? */
1190+
if (TC_H_MIN(parent)) {
1191+
const struct Qdisc_class_ops *cops = q->ops->cl_ops;
1192+
1193+
*cl = cops->find(q, parent);
1194+
if (*cl == 0) {
1195+
NL_SET_ERR_MSG(extack, "Specified class doesn't exist");
1196+
return -ENOENT;
1197+
}
1198+
}
1199+
1200+
return 0;
1201+
}
1202+
1203+
static struct tcf_block *__tcf_block_find(struct net *net, struct Qdisc *q,
1204+
unsigned long cl, int ifindex,
1205+
u32 block_index,
1206+
struct netlink_ext_ack *extack)
1207+
{
1208+
struct tcf_block *block;
1209+
1210+
if (ifindex == TCM_IFINDEX_MAGIC_BLOCK) {
1211+
block = tcf_block_refcnt_get(net, block_index);
1212+
if (!block) {
1213+
NL_SET_ERR_MSG(extack, "Block of given index was not found");
1214+
return ERR_PTR(-EINVAL);
1215+
}
1216+
} else {
1217+
const struct Qdisc_class_ops *cops = q->ops->cl_ops;
1218+
1219+
block = cops->tcf_block(q, cl, extack);
1220+
if (!block)
1221+
return ERR_PTR(-EINVAL);
1222+
1223+
if (tcf_block_shared(block)) {
1224+
NL_SET_ERR_MSG(extack, "This filter block is shared. Please use the block index to manipulate the filters");
1225+
return ERR_PTR(-EOPNOTSUPP);
1226+
}
1227+
1228+
/* Always take reference to block in order to support execution
1229+
* of rules update path of cls API without rtnl lock. Caller
1230+
* must release block when it is finished using it. 'if' block
1231+
* of this conditional obtain reference to block by calling
1232+
* tcf_block_refcnt_get().
1233+
*/
1234+
refcount_inc(&block->refcnt);
1235+
}
1236+
1237+
return block;
1238+
}
1239+
11041240
static void __tcf_block_put(struct tcf_block *block, struct Qdisc *q,
11051241
struct tcf_block_ext_info *ei, bool rtnl_held)
11061242
{
@@ -1146,106 +1282,27 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
11461282
struct tcf_block *block;
11471283
int err = 0;
11481284

1149-
if (ifindex == TCM_IFINDEX_MAGIC_BLOCK) {
1150-
block = tcf_block_refcnt_get(net, block_index);
1151-
if (!block) {
1152-
NL_SET_ERR_MSG(extack, "Block of given index was not found");
1153-
return ERR_PTR(-EINVAL);
1154-
}
1155-
} else {
1156-
const struct Qdisc_class_ops *cops;
1157-
struct net_device *dev;
1158-
1159-
rcu_read_lock();
1160-
1161-
/* Find link */
1162-
dev = dev_get_by_index_rcu(net, ifindex);
1163-
if (!dev) {
1164-
rcu_read_unlock();
1165-
return ERR_PTR(-ENODEV);
1166-
}
1167-
1168-
/* Find qdisc */
1169-
if (!*parent) {
1170-
*q = dev->qdisc;
1171-
*parent = (*q)->handle;
1172-
} else {
1173-
*q = qdisc_lookup_rcu(dev, TC_H_MAJ(*parent));
1174-
if (!*q) {
1175-
NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists");
1176-
err = -EINVAL;
1177-
goto errout_rcu;
1178-
}
1179-
}
1180-
1181-
*q = qdisc_refcount_inc_nz(*q);
1182-
if (!*q) {
1183-
NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists");
1184-
err = -EINVAL;
1185-
goto errout_rcu;
1186-
}
1187-
1188-
/* Is it classful? */
1189-
cops = (*q)->ops->cl_ops;
1190-
if (!cops) {
1191-
NL_SET_ERR_MSG(extack, "Qdisc not classful");
1192-
err = -EINVAL;
1193-
goto errout_rcu;
1194-
}
1195-
1196-
if (!cops->tcf_block) {
1197-
NL_SET_ERR_MSG(extack, "Class doesn't support blocks");
1198-
err = -EOPNOTSUPP;
1199-
goto errout_rcu;
1200-
}
1201-
1202-
/* At this point we know that qdisc is not noop_qdisc,
1203-
* which means that qdisc holds a reference to net_device
1204-
* and we hold a reference to qdisc, so it is safe to release
1205-
* rcu read lock.
1206-
*/
1207-
rcu_read_unlock();
1285+
ASSERT_RTNL();
12081286

1209-
/* Do we search for filter, attached to class? */
1210-
if (TC_H_MIN(*parent)) {
1211-
*cl = cops->find(*q, *parent);
1212-
if (*cl == 0) {
1213-
NL_SET_ERR_MSG(extack, "Specified class doesn't exist");
1214-
err = -ENOENT;
1215-
goto errout_qdisc;
1216-
}
1217-
}
1287+
err = __tcf_qdisc_find(net, q, parent, ifindex, true, extack);
1288+
if (err)
1289+
goto errout;
12181290

1219-
/* And the last stroke */
1220-
block = cops->tcf_block(*q, *cl, extack);
1221-
if (!block) {
1222-
err = -EINVAL;
1223-
goto errout_qdisc;
1224-
}
1225-
if (tcf_block_shared(block)) {
1226-
NL_SET_ERR_MSG(extack, "This filter block is shared. Please use the block index to manipulate the filters");
1227-
err = -EOPNOTSUPP;
1228-
goto errout_qdisc;
1229-
}
1291+
err = __tcf_qdisc_cl_find(*q, *parent, cl, ifindex, extack);
1292+
if (err)
1293+
goto errout_qdisc;
12301294

1231-
/* Always take reference to block in order to support execution
1232-
* of rules update path of cls API without rtnl lock. Caller
1233-
* must release block when it is finished using it. 'if' block
1234-
* of this conditional obtain reference to block by calling
1235-
* tcf_block_refcnt_get().
1236-
*/
1237-
refcount_inc(&block->refcnt);
1238-
}
1295+
block = __tcf_block_find(net, *q, *cl, ifindex, block_index, extack);
1296+
if (IS_ERR(block))
1297+
goto errout_qdisc;
12391298

12401299
return block;
12411300

1242-
errout_rcu:
1243-
rcu_read_unlock();
12441301
errout_qdisc:
1245-
if (*q) {
1302+
if (*q)
12461303
qdisc_put(*q);
1247-
*q = NULL;
1248-
}
1304+
errout:
1305+
*q = NULL;
12491306
return ERR_PTR(err);
12501307
}
12511308

0 commit comments

Comments
 (0)