Skip to content

Commit fd84c56

Browse files
davem330davem330
committed
Merge branch 'act_pedit-minor-improvements'
Pedro Tammela says: ==================== net/sched: act_pedit: minor improvements This series aims to improve the code and usability of act_pedit for netlink users. Patches 1-2 improves error reporting for extended keys parsing with extack. Patch 3 checks the static offsets a priori on create/update. Currently, this is done at the datapath for both static and runtime offsets. Patch 4 removes a check from the datapath which is redundant since the netlink parsing validates the key types. Patch 5 changes the 'pr_info()' calls in the datapath to rate limited versions. ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents 2f0f946 + e3c9673 commit fd84c56

1 file changed

Lines changed: 40 additions & 45 deletions

File tree

‎net/sched/act_pedit.c‎

Lines changed: 40 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,13 @@ static const struct nla_policy pedit_policy[TCA_PEDIT_MAX + 1] = {
3030
};
3131

3232
static const struct nla_policy pedit_key_ex_policy[TCA_PEDIT_KEY_EX_MAX + 1] = {
33-
[TCA_PEDIT_KEY_EX_HTYPE] = { .type = NLA_U16 },
34-
[TCA_PEDIT_KEY_EX_CMD] = { .type = NLA_U16 },
33+
[TCA_PEDIT_KEY_EX_HTYPE] =
34+
NLA_POLICY_MAX(NLA_U16, TCA_PEDIT_HDR_TYPE_MAX),
35+
[TCA_PEDIT_KEY_EX_CMD] = NLA_POLICY_MAX(NLA_U16, TCA_PEDIT_CMD_MAX),
3536
};
3637

3738
static struct tcf_pedit_key_ex *tcf_pedit_keys_ex_parse(struct nlattr *nla,
38-
u8 n)
39+
u8 n, struct netlink_ext_ack *extack)
3940
{
4041
struct tcf_pedit_key_ex *keys_ex;
4142
struct tcf_pedit_key_ex *k;
@@ -56,12 +57,14 @@ static struct tcf_pedit_key_ex *tcf_pedit_keys_ex_parse(struct nlattr *nla,
5657
struct nlattr *tb[TCA_PEDIT_KEY_EX_MAX + 1];
5758

5859
if (!n) {
60+
NL_SET_ERR_MSG_MOD(extack, "Can't parse more extended keys than requested");
5961
err = -EINVAL;
6062
goto err_out;
6163
}
6264
n--;
6365

6466
if (nla_type(ka) != TCA_PEDIT_KEY_EX) {
67+
NL_SET_ERR_MSG_ATTR(extack, ka, "Unknown attribute, expected extended key");
6568
err = -EINVAL;
6669
goto err_out;
6770
}
@@ -72,25 +75,26 @@ static struct tcf_pedit_key_ex *tcf_pedit_keys_ex_parse(struct nlattr *nla,
7275
if (err)
7376
goto err_out;
7477

75-
if (!tb[TCA_PEDIT_KEY_EX_HTYPE] ||
76-
!tb[TCA_PEDIT_KEY_EX_CMD]) {
78+
if (NL_REQ_ATTR_CHECK(extack, nla, tb, TCA_PEDIT_KEY_EX_HTYPE)) {
79+
NL_SET_ERR_MSG(extack, "Missing required attribute");
7780
err = -EINVAL;
7881
goto err_out;
7982
}
8083

81-
k->htype = nla_get_u16(tb[TCA_PEDIT_KEY_EX_HTYPE]);
82-
k->cmd = nla_get_u16(tb[TCA_PEDIT_KEY_EX_CMD]);
83-
84-
if (k->htype > TCA_PEDIT_HDR_TYPE_MAX ||
85-
k->cmd > TCA_PEDIT_CMD_MAX) {
84+
if (NL_REQ_ATTR_CHECK(extack, nla, tb, TCA_PEDIT_KEY_EX_CMD)) {
85+
NL_SET_ERR_MSG(extack, "Missing required attribute");
8686
err = -EINVAL;
8787
goto err_out;
8888
}
8989

90+
k->htype = nla_get_u16(tb[TCA_PEDIT_KEY_EX_HTYPE]);
91+
k->cmd = nla_get_u16(tb[TCA_PEDIT_KEY_EX_CMD]);
92+
9093
k++;
9194
}
9295

9396
if (n) {
97+
NL_SET_ERR_MSG_MOD(extack, "Not enough extended keys to parse");
9498
err = -EINVAL;
9599
goto err_out;
96100
}
@@ -222,7 +226,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
222226
}
223227

224228
nparms->tcfp_keys_ex =
225-
tcf_pedit_keys_ex_parse(tb[TCA_PEDIT_KEYS_EX], parm->nkeys);
229+
tcf_pedit_keys_ex_parse(tb[TCA_PEDIT_KEYS_EX], parm->nkeys, extack);
226230
if (IS_ERR(nparms->tcfp_keys_ex)) {
227231
ret = PTR_ERR(nparms->tcfp_keys_ex);
228232
goto out_free;
@@ -247,8 +251,16 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
247251
memcpy(nparms->tcfp_keys, parm->keys, ksize);
248252

249253
for (i = 0; i < nparms->tcfp_nkeys; ++i) {
254+
u32 offmask = nparms->tcfp_keys[i].offmask;
250255
u32 cur = nparms->tcfp_keys[i].off;
251256

257+
/* The AT option can be added to static offsets in the datapath */
258+
if (!offmask && cur % 4) {
259+
NL_SET_ERR_MSG_MOD(extack, "Offsets must be on 32bit boundaries");
260+
ret = -EINVAL;
261+
goto put_chain;
262+
}
263+
252264
/* sanitize the shift value for any later use */
253265
nparms->tcfp_keys[i].shift = min_t(size_t,
254266
BITS_PER_TYPE(int) - 1,
@@ -257,7 +269,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
257269
/* The AT option can read a single byte, we can bound the actual
258270
* value with uchar max.
259271
*/
260-
cur += (0xff & nparms->tcfp_keys[i].offmask) >> nparms->tcfp_keys[i].shift;
272+
cur += (0xff & offmask) >> nparms->tcfp_keys[i].shift;
261273

262274
/* Each key touches 4 bytes starting from the computed offset */
263275
nparms->tcfp_off_max_hint =
@@ -313,37 +325,28 @@ static bool offset_valid(struct sk_buff *skb, int offset)
313325
return true;
314326
}
315327

316-
static int pedit_skb_hdr_offset(struct sk_buff *skb,
317-
enum pedit_header_type htype, int *hoffset)
328+
static void pedit_skb_hdr_offset(struct sk_buff *skb,
329+
enum pedit_header_type htype, int *hoffset)
318330
{
319-
int ret = -EINVAL;
320-
331+
/* 'htype' is validated in the netlink parsing */
321332
switch (htype) {
322333
case TCA_PEDIT_KEY_EX_HDR_TYPE_ETH:
323-
if (skb_mac_header_was_set(skb)) {
334+
if (skb_mac_header_was_set(skb))
324335
*hoffset = skb_mac_offset(skb);
325-
ret = 0;
326-
}
327336
break;
328337
case TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK:
329338
case TCA_PEDIT_KEY_EX_HDR_TYPE_IP4:
330339
case TCA_PEDIT_KEY_EX_HDR_TYPE_IP6:
331340
*hoffset = skb_network_offset(skb);
332-
ret = 0;
333341
break;
334342
case TCA_PEDIT_KEY_EX_HDR_TYPE_TCP:
335343
case TCA_PEDIT_KEY_EX_HDR_TYPE_UDP:
336-
if (skb_transport_header_was_set(skb)) {
344+
if (skb_transport_header_was_set(skb))
337345
*hoffset = skb_transport_offset(skb);
338-
ret = 0;
339-
}
340346
break;
341347
default:
342-
ret = -EINVAL;
343348
break;
344349
}
345-
346-
return ret;
347350
}
348351

349352
TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
@@ -376,10 +379,9 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
376379

377380
for (i = parms->tcfp_nkeys; i > 0; i--, tkey++) {
378381
int offset = tkey->off;
382+
int hoffset = 0;
379383
u32 *ptr, hdata;
380-
int hoffset;
381384
u32 val;
382-
int rc;
383385

384386
if (tkey_ex) {
385387
htype = tkey_ex->htype;
@@ -388,36 +390,30 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
388390
tkey_ex++;
389391
}
390392

391-
rc = pedit_skb_hdr_offset(skb, htype, &hoffset);
392-
if (rc) {
393-
pr_info("tc action pedit bad header type specified (0x%x)\n",
394-
htype);
395-
goto bad;
396-
}
393+
pedit_skb_hdr_offset(skb, htype, &hoffset);
397394

398395
if (tkey->offmask) {
399396
u8 *d, _d;
400397

401398
if (!offset_valid(skb, hoffset + tkey->at)) {
402-
pr_info("tc action pedit 'at' offset %d out of bounds\n",
403-
hoffset + tkey->at);
399+
pr_info_ratelimited("tc action pedit 'at' offset %d out of bounds\n",
400+
hoffset + tkey->at);
404401
goto bad;
405402
}
406403
d = skb_header_pointer(skb, hoffset + tkey->at,
407404
sizeof(_d), &_d);
408405
if (!d)
409406
goto bad;
410-
offset += (*d & tkey->offmask) >> tkey->shift;
411-
}
412407

413-
if (offset % 4) {
414-
pr_info("tc action pedit offset must be on 32 bit boundaries\n");
415-
goto bad;
408+
offset += (*d & tkey->offmask) >> tkey->shift;
409+
if (offset % 4) {
410+
pr_info_ratelimited("tc action pedit offset must be on 32 bit boundaries\n");
411+
goto bad;
412+
}
416413
}
417414

418415
if (!offset_valid(skb, hoffset + offset)) {
419-
pr_info("tc action pedit offset %d out of bounds\n",
420-
hoffset + offset);
416+
pr_info_ratelimited("tc action pedit offset %d out of bounds\n", hoffset + offset);
421417
goto bad;
422418
}
423419

@@ -434,8 +430,7 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
434430
val = (*ptr + tkey->val) & ~tkey->mask;
435431
break;
436432
default:
437-
pr_info("tc action pedit bad command (%d)\n",
438-
cmd);
433+
pr_info_ratelimited("tc action pedit bad command (%d)\n", cmd);
439434
goto bad;
440435
}
441436

0 commit comments

Comments
 (0)