Skip to content

Commit 3fa1056

Browse files
committed
Merge branch 'net-sched-action-bind'
Pedro Tammela says: ==================== net/sched: fix action bind logic Some actions are not handling the case where an action can be created and bound to a filter independently. These actions are checking for parameters only passed in the netlink message for create/change/replace, which then errors out for valid uses like: tc filter ... action pedit index 1 In the iproute2 side, we saw a couple of actions with their parsers broken when passing "index 1" as the only action argument, while the kernel side accepted it correctly. We fixed those as well. ==================== Signed-off-by: David S. Miller <[email protected]>
2 parents aaa3c08 + 4a20056 commit 3fa1056

File tree

3 files changed

+77
-58
lines changed

3 files changed

+77
-58
lines changed

net/sched/act_mpls.c

Lines changed: 37 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -190,40 +190,67 @@ static int tcf_mpls_init(struct net *net, struct nlattr *nla,
190190
parm = nla_data(tb[TCA_MPLS_PARMS]);
191191
index = parm->index;
192192

193+
err = tcf_idr_check_alloc(tn, &index, a, bind);
194+
if (err < 0)
195+
return err;
196+
exists = err;
197+
if (exists && bind)
198+
return 0;
199+
200+
if (!exists) {
201+
ret = tcf_idr_create(tn, index, est, a, &act_mpls_ops, bind,
202+
true, flags);
203+
if (ret) {
204+
tcf_idr_cleanup(tn, index);
205+
return ret;
206+
}
207+
208+
ret = ACT_P_CREATED;
209+
} else if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
210+
tcf_idr_release(*a, bind);
211+
return -EEXIST;
212+
}
213+
193214
/* Verify parameters against action type. */
194215
switch (parm->m_action) {
195216
case TCA_MPLS_ACT_POP:
196217
if (!tb[TCA_MPLS_PROTO]) {
197218
NL_SET_ERR_MSG_MOD(extack, "Protocol must be set for MPLS pop");
198-
return -EINVAL;
219+
err = -EINVAL;
220+
goto release_idr;
199221
}
200222
if (!eth_proto_is_802_3(nla_get_be16(tb[TCA_MPLS_PROTO]))) {
201223
NL_SET_ERR_MSG_MOD(extack, "Invalid protocol type for MPLS pop");
202-
return -EINVAL;
224+
err = -EINVAL;
225+
goto release_idr;
203226
}
204227
if (tb[TCA_MPLS_LABEL] || tb[TCA_MPLS_TTL] || tb[TCA_MPLS_TC] ||
205228
tb[TCA_MPLS_BOS]) {
206229
NL_SET_ERR_MSG_MOD(extack, "Label, TTL, TC or BOS cannot be used with MPLS pop");
207-
return -EINVAL;
230+
err = -EINVAL;
231+
goto release_idr;
208232
}
209233
break;
210234
case TCA_MPLS_ACT_DEC_TTL:
211235
if (tb[TCA_MPLS_PROTO] || tb[TCA_MPLS_LABEL] ||
212236
tb[TCA_MPLS_TTL] || tb[TCA_MPLS_TC] || tb[TCA_MPLS_BOS]) {
213237
NL_SET_ERR_MSG_MOD(extack, "Label, TTL, TC, BOS or protocol cannot be used with MPLS dec_ttl");
214-
return -EINVAL;
238+
err = -EINVAL;
239+
goto release_idr;
215240
}
216241
break;
217242
case TCA_MPLS_ACT_PUSH:
218243
case TCA_MPLS_ACT_MAC_PUSH:
219244
if (!tb[TCA_MPLS_LABEL]) {
220245
NL_SET_ERR_MSG_MOD(extack, "Label is required for MPLS push");
221-
return -EINVAL;
246+
err = -EINVAL;
247+
goto release_idr;
222248
}
223249
if (tb[TCA_MPLS_PROTO] &&
224250
!eth_p_mpls(nla_get_be16(tb[TCA_MPLS_PROTO]))) {
225251
NL_SET_ERR_MSG_MOD(extack, "Protocol must be an MPLS type for MPLS push");
226-
return -EPROTONOSUPPORT;
252+
err = -EPROTONOSUPPORT;
253+
goto release_idr;
227254
}
228255
/* Push needs a TTL - if not specified, set a default value. */
229256
if (!tb[TCA_MPLS_TTL]) {
@@ -238,33 +265,14 @@ static int tcf_mpls_init(struct net *net, struct nlattr *nla,
238265
case TCA_MPLS_ACT_MODIFY:
239266
if (tb[TCA_MPLS_PROTO]) {
240267
NL_SET_ERR_MSG_MOD(extack, "Protocol cannot be used with MPLS modify");
241-
return -EINVAL;
268+
err = -EINVAL;
269+
goto release_idr;
242270
}
243271
break;
244272
default:
245273
NL_SET_ERR_MSG_MOD(extack, "Unknown MPLS action");
246-
return -EINVAL;
247-
}
248-
249-
err = tcf_idr_check_alloc(tn, &index, a, bind);
250-
if (err < 0)
251-
return err;
252-
exists = err;
253-
if (exists && bind)
254-
return 0;
255-
256-
if (!exists) {
257-
ret = tcf_idr_create(tn, index, est, a,
258-
&act_mpls_ops, bind, true, flags);
259-
if (ret) {
260-
tcf_idr_cleanup(tn, index);
261-
return ret;
262-
}
263-
264-
ret = ACT_P_CREATED;
265-
} else if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
266-
tcf_idr_release(*a, bind);
267-
return -EEXIST;
274+
err = -EINVAL;
275+
goto release_idr;
268276
}
269277

270278
err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack);

net/sched/act_pedit.c

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -181,26 +181,6 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
181181
}
182182

183183
parm = nla_data(pattr);
184-
if (!parm->nkeys) {
185-
NL_SET_ERR_MSG_MOD(extack, "Pedit requires keys to be passed");
186-
return -EINVAL;
187-
}
188-
ksize = parm->nkeys * sizeof(struct tc_pedit_key);
189-
if (nla_len(pattr) < sizeof(*parm) + ksize) {
190-
NL_SET_ERR_MSG_ATTR(extack, pattr, "Length of TCA_PEDIT_PARMS or TCA_PEDIT_PARMS_EX pedit attribute is invalid");
191-
return -EINVAL;
192-
}
193-
194-
nparms = kzalloc(sizeof(*nparms), GFP_KERNEL);
195-
if (!nparms)
196-
return -ENOMEM;
197-
198-
nparms->tcfp_keys_ex =
199-
tcf_pedit_keys_ex_parse(tb[TCA_PEDIT_KEYS_EX], parm->nkeys);
200-
if (IS_ERR(nparms->tcfp_keys_ex)) {
201-
ret = PTR_ERR(nparms->tcfp_keys_ex);
202-
goto out_free;
203-
}
204184

205185
index = parm->index;
206186
err = tcf_idr_check_alloc(tn, &index, a, bind);
@@ -209,25 +189,49 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
209189
&act_pedit_ops, bind, flags);
210190
if (ret) {
211191
tcf_idr_cleanup(tn, index);
212-
goto out_free_ex;
192+
return ret;
213193
}
214194
ret = ACT_P_CREATED;
215195
} else if (err > 0) {
216196
if (bind)
217-
goto out_free;
197+
return 0;
218198
if (!(flags & TCA_ACT_FLAGS_REPLACE)) {
219199
ret = -EEXIST;
220200
goto out_release;
221201
}
222202
} else {
223-
ret = err;
224-
goto out_free_ex;
203+
return err;
204+
}
205+
206+
if (!parm->nkeys) {
207+
NL_SET_ERR_MSG_MOD(extack, "Pedit requires keys to be passed");
208+
ret = -EINVAL;
209+
goto out_release;
210+
}
211+
ksize = parm->nkeys * sizeof(struct tc_pedit_key);
212+
if (nla_len(pattr) < sizeof(*parm) + ksize) {
213+
NL_SET_ERR_MSG_ATTR(extack, pattr, "Length of TCA_PEDIT_PARMS or TCA_PEDIT_PARMS_EX pedit attribute is invalid");
214+
ret = -EINVAL;
215+
goto out_release;
216+
}
217+
218+
nparms = kzalloc(sizeof(*nparms), GFP_KERNEL);
219+
if (!nparms) {
220+
ret = -ENOMEM;
221+
goto out_release;
222+
}
223+
224+
nparms->tcfp_keys_ex =
225+
tcf_pedit_keys_ex_parse(tb[TCA_PEDIT_KEYS_EX], parm->nkeys);
226+
if (IS_ERR(nparms->tcfp_keys_ex)) {
227+
ret = PTR_ERR(nparms->tcfp_keys_ex);
228+
goto out_free;
225229
}
226230

227231
err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack);
228232
if (err < 0) {
229233
ret = err;
230-
goto out_release;
234+
goto out_free_ex;
231235
}
232236

233237
nparms->tcfp_off_max_hint = 0;
@@ -278,12 +282,12 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
278282
put_chain:
279283
if (goto_ch)
280284
tcf_chain_put_by_act(goto_ch);
281-
out_release:
282-
tcf_idr_release(*a, bind);
283285
out_free_ex:
284286
kfree(nparms->tcfp_keys_ex);
285287
out_free:
286288
kfree(nparms);
289+
out_release:
290+
tcf_idr_release(*a, bind);
287291
return ret;
288292
}
289293

net/sched/act_sample.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
5555
sample_policy, NULL);
5656
if (ret < 0)
5757
return ret;
58-
if (!tb[TCA_SAMPLE_PARMS] || !tb[TCA_SAMPLE_RATE] ||
59-
!tb[TCA_SAMPLE_PSAMPLE_GROUP])
58+
59+
if (!tb[TCA_SAMPLE_PARMS])
6060
return -EINVAL;
6161

6262
parm = nla_data(tb[TCA_SAMPLE_PARMS]);
@@ -80,6 +80,13 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
8080
tcf_idr_release(*a, bind);
8181
return -EEXIST;
8282
}
83+
84+
if (!tb[TCA_SAMPLE_RATE] || !tb[TCA_SAMPLE_PSAMPLE_GROUP]) {
85+
NL_SET_ERR_MSG(extack, "sample rate and group are required");
86+
err = -EINVAL;
87+
goto release_idr;
88+
}
89+
8390
err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack);
8491
if (err < 0)
8592
goto release_idr;

0 commit comments

Comments
 (0)