From 80f0f574cc615b2c61bdfb0e3c2449478d63c488 Mon Sep 17 00:00:00 2001 From: Roman Mashak Date: Wed, 27 Jun 2018 13:33:30 -0400 Subject: [PATCH 1/6] net sched actions: fix coding style in pedit action Fix coding style issues in tc pedit action detected by the checkpatch script. Reviewed-by: Simon Horman Signed-off-by: Roman Mashak Signed-off-by: David S. Miller --- net/sched/act_pedit.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index 8a925c72db5f..e4b29ee79ba8 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -136,15 +136,15 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla, { struct tc_action_net *tn = net_generic(net, pedit_net_id); struct nlattr *tb[TCA_PEDIT_MAX + 1]; - struct nlattr *pattr; - struct tc_pedit *parm; - int ret = 0, err; - struct tcf_pedit *p; struct tc_pedit_key *keys = NULL; struct tcf_pedit_key_ex *keys_ex; + struct tc_pedit *parm; + struct nlattr *pattr; + struct tcf_pedit *p; + int ret = 0, err; int ksize; - if (nla == NULL) + if (!nla) return -EINVAL; err = nla_parse_nested(tb, TCA_PEDIT_MAX, nla, pedit_policy, NULL); @@ -175,7 +175,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla, return ret; p = to_pedit(*a); keys = kmalloc(ksize, GFP_KERNEL); - if (keys == NULL) { + if (!keys) { tcf_idr_release(*a, bind); kfree(keys_ex); return -ENOMEM; @@ -220,6 +220,7 @@ static void tcf_pedit_cleanup(struct tc_action *a) { struct tcf_pedit *p = to_pedit(a); struct tc_pedit_key *keys = p->tcfp_keys; + kfree(keys); kfree(p->tcfp_keys_ex); } @@ -284,7 +285,8 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, if (p->tcfp_nkeys > 0) { struct tc_pedit_key *tkey = p->tcfp_keys; struct tcf_pedit_key_ex *tkey_ex = p->tcfp_keys_ex; - enum pedit_header_type htype = TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK; + enum pedit_header_type htype = + TCA_PEDIT_KEY_EX_HDR_TYPE_NETWORK; enum pedit_cmd cmd = TCA_PEDIT_KEY_EX_CMD_SET; for (i = p->tcfp_nkeys; i > 0; i--, tkey++) { @@ -316,16 +318,15 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, hoffset + tkey->at); goto bad; } - d = skb_header_pointer(skb, hoffset + tkey->at, 1, - &_d); + d = skb_header_pointer(skb, hoffset + tkey->at, + 1, &_d); if (!d) goto bad; offset += (*d & tkey->offmask) >> tkey->shift; } if (offset % 4) { - pr_info("tc filter pedit" - " offset must be on 32 bit boundaries\n"); + pr_info("tc filter pedit offset must be on 32 bit boundaries\n"); goto bad; } @@ -335,7 +336,8 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, goto bad; } - ptr = skb_header_pointer(skb, hoffset + offset, 4, &_data); + ptr = skb_header_pointer(skb, hoffset + offset, + 4, &_data); if (!ptr) goto bad; /* just do it, baby */ @@ -358,8 +360,9 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, } goto done; - } else + } else { WARN(1, "pedit BUG: index %d\n", p->tcf_index); + } bad: p->tcf_qstats.overlimits++; From d020d4559de9baf47cafa2669f29ea59d11a914c Mon Sep 17 00:00:00 2001 From: Roman Mashak Date: Wed, 27 Jun 2018 13:33:31 -0400 Subject: [PATCH 2/6] net sched actions: fix coding style in pedit headers Fix coding style issues in tc pedit headers detected by the checkpatch script. Reviewed-by: Simon Horman Signed-off-by: Roman Mashak Signed-off-by: David S. Miller --- include/net/tc_act/tc_pedit.h | 1 + include/uapi/linux/tc_act/tc_pedit.h | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/net/tc_act/tc_pedit.h b/include/net/tc_act/tc_pedit.h index 227a6f1d02f4..fac3ad4a86de 100644 --- a/include/net/tc_act/tc_pedit.h +++ b/include/net/tc_act/tc_pedit.h @@ -17,6 +17,7 @@ struct tcf_pedit { struct tc_pedit_key *tcfp_keys; struct tcf_pedit_key_ex *tcfp_keys_ex; }; + #define to_pedit(a) ((struct tcf_pedit *)a) static inline bool is_tcf_pedit(const struct tc_action *a) diff --git a/include/uapi/linux/tc_act/tc_pedit.h b/include/uapi/linux/tc_act/tc_pedit.h index 162d1094c41c..24ec792dacc1 100644 --- a/include/uapi/linux/tc_act/tc_pedit.h +++ b/include/uapi/linux/tc_act/tc_pedit.h @@ -17,13 +17,15 @@ enum { TCA_PEDIT_KEY_EX, __TCA_PEDIT_MAX }; + #define TCA_PEDIT_MAX (__TCA_PEDIT_MAX - 1) - + enum { TCA_PEDIT_KEY_EX_HTYPE = 1, TCA_PEDIT_KEY_EX_CMD = 2, __TCA_PEDIT_KEY_EX_MAX }; + #define TCA_PEDIT_KEY_EX_MAX (__TCA_PEDIT_KEY_EX_MAX - 1) /* TCA_PEDIT_KEY_EX_HDR_TYPE_NETWROK is a special case for legacy users. It @@ -38,6 +40,7 @@ enum pedit_header_type { TCA_PEDIT_KEY_EX_HDR_TYPE_UDP = 5, __PEDIT_HDR_TYPE_MAX, }; + #define TCA_PEDIT_HDR_TYPE_MAX (__PEDIT_HDR_TYPE_MAX - 1) enum pedit_cmd { @@ -45,6 +48,7 @@ enum pedit_cmd { TCA_PEDIT_KEY_EX_CMD_ADD = 1, __PEDIT_CMD_MAX, }; + #define TCA_PEDIT_CMD_MAX (__PEDIT_CMD_MAX - 1) struct tc_pedit_key { @@ -55,13 +59,14 @@ struct tc_pedit_key { __u32 offmask; __u32 shift; }; - + struct tc_pedit_sel { tc_gen; unsigned char nkeys; unsigned char flags; struct tc_pedit_key keys[0]; }; + #define tc_pedit tc_pedit_sel #endif From 544377cd2545f33cc6cd5458301749d828adacb0 Mon Sep 17 00:00:00 2001 From: Roman Mashak Date: Wed, 27 Jun 2018 13:33:32 -0400 Subject: [PATCH 3/6] net sched actions: fix sparse warning The variable _data in include/asm-generic/sections.h defines sections, this causes sparse warning in pedit: net/sched/act_pedit.c:293:35: warning: symbol '_data' shadows an earlier one ./include/asm-generic/sections.h:36:13: originally declared here Therefore rename the variable. Reviewed-by: Simon Horman Signed-off-by: Roman Mashak Signed-off-by: David S. Miller --- net/sched/act_pedit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index e4b29ee79ba8..9c2d8a31a5c5 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -290,7 +290,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, enum pedit_cmd cmd = TCA_PEDIT_KEY_EX_CMD_SET; for (i = p->tcfp_nkeys; i > 0; i--, tkey++) { - u32 *ptr, _data; + u32 *ptr, hdata; int offset = tkey->off; int hoffset; u32 val; @@ -337,7 +337,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, } ptr = skb_header_pointer(skb, hoffset + offset, - 4, &_data); + 4, &hdata); if (!ptr) goto bad; /* just do it, baby */ @@ -355,7 +355,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, } *ptr = ((*ptr & tkey->mask) ^ val); - if (ptr == &_data) + if (ptr == &hdata) skb_store_bits(skb, hoffset + offset, ptr, 4); } From 6ff7586e382cb4274adefd56501d428ea39a5af3 Mon Sep 17 00:00:00 2001 From: Roman Mashak Date: Wed, 27 Jun 2018 13:33:33 -0400 Subject: [PATCH 4/6] net sched actions: use sizeof operator for buffer length Replace constant integer with sizeof() to clearly indicate the destination buffer length in skb_header_pointer() calls. Reviewed-by: Simon Horman Signed-off-by: Roman Mashak Signed-off-by: David S. Miller --- net/sched/act_pedit.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index 9c2d8a31a5c5..3b775f54cee5 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -319,7 +319,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, goto bad; } d = skb_header_pointer(skb, hoffset + tkey->at, - 1, &_d); + sizeof(_d), &_d); if (!d) goto bad; offset += (*d & tkey->offmask) >> tkey->shift; @@ -337,7 +337,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, } ptr = skb_header_pointer(skb, hoffset + offset, - 4, &hdata); + sizeof(hdata), &hdata); if (!ptr) goto bad; /* just do it, baby */ From 95b0d2dc13c7e7ea51675836680732e8c16e378a Mon Sep 17 00:00:00 2001 From: Roman Mashak Date: Wed, 27 Jun 2018 13:33:34 -0400 Subject: [PATCH 5/6] net sched actions: fix misleading text strings in pedit action Change "tc filter pedit .." to "tc actions pedit .." in error messages to clearly refer to pedit action. Reviewed-by: Simon Horman Signed-off-by: Roman Mashak Signed-off-by: David S. Miller --- net/sched/act_pedit.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index 3b775f54cee5..caa6927a992c 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -305,7 +305,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, rc = pedit_skb_hdr_offset(skb, htype, &hoffset); if (rc) { - pr_info("tc filter pedit bad header type specified (0x%x)\n", + pr_info("tc action pedit bad header type specified (0x%x)\n", htype); goto bad; } @@ -314,7 +314,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, char *d, _d; if (!offset_valid(skb, hoffset + tkey->at)) { - pr_info("tc filter pedit 'at' offset %d out of bounds\n", + pr_info("tc action pedit 'at' offset %d out of bounds\n", hoffset + tkey->at); goto bad; } @@ -326,12 +326,12 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, } if (offset % 4) { - pr_info("tc filter pedit offset must be on 32 bit boundaries\n"); + pr_info("tc action pedit offset must be on 32 bit boundaries\n"); goto bad; } if (!offset_valid(skb, hoffset + offset)) { - pr_info("tc filter pedit offset %d out of bounds\n", + pr_info("tc action pedit offset %d out of bounds\n", hoffset + offset); goto bad; } @@ -349,7 +349,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, val = (*ptr + tkey->val) & ~tkey->mask; break; default: - pr_info("tc filter pedit bad command (%d)\n", + pr_info("tc action pedit bad command (%d)\n", cmd); goto bad; } From 430527415398cf7e741f5e2f11324a8df9093327 Mon Sep 17 00:00:00 2001 From: Roman Mashak Date: Wed, 27 Jun 2018 13:33:35 -0400 Subject: [PATCH 6/6] net sched actions: avoid bitwise operation on signed value in pedit Since char can be unsigned or signed, and bitwise operators may have implementation-dependent results when performed on signed operands, declare 'u8 *' operand instead. Suggested-by: Davide Caratti Signed-off-by: Roman Mashak Signed-off-by: David S. Miller --- net/sched/act_pedit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index caa6927a992c..ab151346d3d4 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -311,7 +311,7 @@ static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a, } if (tkey->offmask) { - char *d, _d; + u8 *d, _d; if (!offset_valid(skb, hoffset + tkey->at)) { pr_info("tc action pedit 'at' offset %d out of bounds\n",