Skip to content

Commit

Permalink
Merge branch 'act_pedit-minor-improvements'
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
David S. Miller committed Apr 23, 2023
2 parents 2f0f946 + e3c9673 commit fd84c56
Showing 1 changed file with 40 additions and 45 deletions.
85 changes: 40 additions & 45 deletions net/sched/act_pedit.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,13 @@ static const struct nla_policy pedit_policy[TCA_PEDIT_MAX + 1] = {
};

static const struct nla_policy pedit_key_ex_policy[TCA_PEDIT_KEY_EX_MAX + 1] = {
[TCA_PEDIT_KEY_EX_HTYPE] = { .type = NLA_U16 },
[TCA_PEDIT_KEY_EX_CMD] = { .type = NLA_U16 },
[TCA_PEDIT_KEY_EX_HTYPE] =
NLA_POLICY_MAX(NLA_U16, TCA_PEDIT_HDR_TYPE_MAX),
[TCA_PEDIT_KEY_EX_CMD] = NLA_POLICY_MAX(NLA_U16, TCA_PEDIT_CMD_MAX),
};

static struct tcf_pedit_key_ex *tcf_pedit_keys_ex_parse(struct nlattr *nla,
u8 n)
u8 n, struct netlink_ext_ack *extack)
{
struct tcf_pedit_key_ex *keys_ex;
struct tcf_pedit_key_ex *k;
Expand All @@ -56,12 +57,14 @@ static struct tcf_pedit_key_ex *tcf_pedit_keys_ex_parse(struct nlattr *nla,
struct nlattr *tb[TCA_PEDIT_KEY_EX_MAX + 1];

if (!n) {
NL_SET_ERR_MSG_MOD(extack, "Can't parse more extended keys than requested");
err = -EINVAL;
goto err_out;
}
n--;

if (nla_type(ka) != TCA_PEDIT_KEY_EX) {
NL_SET_ERR_MSG_ATTR(extack, ka, "Unknown attribute, expected extended key");
err = -EINVAL;
goto err_out;
}
Expand All @@ -72,25 +75,26 @@ static struct tcf_pedit_key_ex *tcf_pedit_keys_ex_parse(struct nlattr *nla,
if (err)
goto err_out;

if (!tb[TCA_PEDIT_KEY_EX_HTYPE] ||
!tb[TCA_PEDIT_KEY_EX_CMD]) {
if (NL_REQ_ATTR_CHECK(extack, nla, tb, TCA_PEDIT_KEY_EX_HTYPE)) {
NL_SET_ERR_MSG(extack, "Missing required attribute");
err = -EINVAL;
goto err_out;
}

k->htype = nla_get_u16(tb[TCA_PEDIT_KEY_EX_HTYPE]);
k->cmd = nla_get_u16(tb[TCA_PEDIT_KEY_EX_CMD]);

if (k->htype > TCA_PEDIT_HDR_TYPE_MAX ||
k->cmd > TCA_PEDIT_CMD_MAX) {
if (NL_REQ_ATTR_CHECK(extack, nla, tb, TCA_PEDIT_KEY_EX_CMD)) {
NL_SET_ERR_MSG(extack, "Missing required attribute");
err = -EINVAL;
goto err_out;
}

k->htype = nla_get_u16(tb[TCA_PEDIT_KEY_EX_HTYPE]);
k->cmd = nla_get_u16(tb[TCA_PEDIT_KEY_EX_CMD]);

k++;
}

if (n) {
NL_SET_ERR_MSG_MOD(extack, "Not enough extended keys to parse");
err = -EINVAL;
goto err_out;
}
Expand Down Expand Up @@ -222,7 +226,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
}

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

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

/* The AT option can be added to static offsets in the datapath */
if (!offmask && cur % 4) {
NL_SET_ERR_MSG_MOD(extack, "Offsets must be on 32bit boundaries");
ret = -EINVAL;
goto put_chain;
}

/* sanitize the shift value for any later use */
nparms->tcfp_keys[i].shift = min_t(size_t,
BITS_PER_TYPE(int) - 1,
Expand All @@ -257,7 +269,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
/* The AT option can read a single byte, we can bound the actual
* value with uchar max.
*/
cur += (0xff & nparms->tcfp_keys[i].offmask) >> nparms->tcfp_keys[i].shift;
cur += (0xff & offmask) >> nparms->tcfp_keys[i].shift;

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

static int pedit_skb_hdr_offset(struct sk_buff *skb,
enum pedit_header_type htype, int *hoffset)
static void pedit_skb_hdr_offset(struct sk_buff *skb,
enum pedit_header_type htype, int *hoffset)
{
int ret = -EINVAL;

/* 'htype' is validated in the netlink parsing */
switch (htype) {
case TCA_PEDIT_KEY_EX_HDR_TYPE_ETH:
if (skb_mac_header_was_set(skb)) {
if (skb_mac_header_was_set(skb))
*hoffset = skb_mac_offset(skb);
ret = 0;
}
break;
case TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK:
case TCA_PEDIT_KEY_EX_HDR_TYPE_IP4:
case TCA_PEDIT_KEY_EX_HDR_TYPE_IP6:
*hoffset = skb_network_offset(skb);
ret = 0;
break;
case TCA_PEDIT_KEY_EX_HDR_TYPE_TCP:
case TCA_PEDIT_KEY_EX_HDR_TYPE_UDP:
if (skb_transport_header_was_set(skb)) {
if (skb_transport_header_was_set(skb))
*hoffset = skb_transport_offset(skb);
ret = 0;
}
break;
default:
ret = -EINVAL;
break;
}

return ret;
}

TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
Expand Down Expand Up @@ -376,10 +379,9 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,

for (i = parms->tcfp_nkeys; i > 0; i--, tkey++) {
int offset = tkey->off;
int hoffset = 0;
u32 *ptr, hdata;
int hoffset;
u32 val;
int rc;

if (tkey_ex) {
htype = tkey_ex->htype;
Expand All @@ -388,36 +390,30 @@ TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
tkey_ex++;
}

rc = pedit_skb_hdr_offset(skb, htype, &hoffset);
if (rc) {
pr_info("tc action pedit bad header type specified (0x%x)\n",
htype);
goto bad;
}
pedit_skb_hdr_offset(skb, htype, &hoffset);

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

if (!offset_valid(skb, hoffset + tkey->at)) {
pr_info("tc action pedit 'at' offset %d out of bounds\n",
hoffset + tkey->at);
pr_info_ratelimited("tc action pedit 'at' offset %d out of bounds\n",
hoffset + tkey->at);
goto bad;
}
d = skb_header_pointer(skb, hoffset + tkey->at,
sizeof(_d), &_d);
if (!d)
goto bad;
offset += (*d & tkey->offmask) >> tkey->shift;
}

if (offset % 4) {
pr_info("tc action pedit offset must be on 32 bit boundaries\n");
goto bad;
offset += (*d & tkey->offmask) >> tkey->shift;
if (offset % 4) {
pr_info_ratelimited("tc action pedit offset must be on 32 bit boundaries\n");
goto bad;
}
}

if (!offset_valid(skb, hoffset + offset)) {
pr_info("tc action pedit offset %d out of bounds\n",
hoffset + offset);
pr_info_ratelimited("tc action pedit offset %d out of bounds\n", hoffset + offset);
goto bad;
}

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

Expand Down

0 comments on commit fd84c56

Please sign in to comment.