| Message ID | 20231230051956.82499-1-linma@zju.edu.cn (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [v1] RDMA/sa_query: use validate not parser in ib_nl_is_good_resolve_resp | expand |
On Sat, Dec 30, 2023 at 01:19:56PM +0800, Lin Ma wrote: > The attributes array `tb` in ib_nl_is_good_resolve_resp is never used > after the parsing. Therefore use nla_validate_deprecated function here > for improvement. What did this change improve? > > Signed-off-by: Lin Ma <linma@zju.edu.cn> > --- > drivers/infiniband/core/sa_query.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c > index 8175dde60b0a..c7407a53fcda 100644 > --- a/drivers/infiniband/core/sa_query.c > +++ b/drivers/infiniband/core/sa_query.c > @@ -1047,14 +1047,13 @@ int ib_nl_handle_set_timeout(struct sk_buff *skb, > > static inline int ib_nl_is_good_resolve_resp(const struct nlmsghdr *nlh) > { > - struct nlattr *tb[LS_NLA_TYPE_MAX]; > int ret; > > if (nlh->nlmsg_flags & RDMA_NL_LS_F_ERR) > return 0; > > - ret = nla_parse_deprecated(tb, LS_NLA_TYPE_MAX - 1, nlmsg_data(nlh), > - nlmsg_len(nlh), ib_nl_policy, NULL); > + ret = nla_validate_deprecated(nlmsg_data(nlh), nlmsg_len(nlh), > + LS_NLA_TYPE_MAX - 1, ib_nl_policy, NULL); > if (ret) > return 0; > > -- > 2.17.1 >
Hello Leon, > > The attributes array `tb` in ib_nl_is_good_resolve_resp is never used > > after the parsing. Therefore use nla_validate_deprecated function here > > for improvement. > > What did this change improve? > To my concern, the nla_validate_deprecated, compared to nla_parse_deprecated, will at lease save a memset in function nla_parse_deprecated ``` if (tb) memset(tb, 0, sizeof(struct nlattr *) * (maxtype + 1)); ``` Morever, because the `nla_validate_deprecated` just validate the attributes array and will not try to retrieve the nla pointers. It shall be faster and cleaner here :D. Regards Lin
On Sun, Dec 31, 2023 at 01:27:59PM +0800, Lin Ma wrote: > Hello Leon, > > > > The attributes array `tb` in ib_nl_is_good_resolve_resp is never used > > > after the parsing. Therefore use nla_validate_deprecated function here > > > for improvement. > > > > What did this change improve? > > > > To my concern, the nla_validate_deprecated, compared to nla_parse_deprecated, > will at lease save a memset in function nla_parse_deprecated > > ``` > if (tb) > memset(tb, 0, sizeof(struct nlattr *) * (maxtype + 1)); > ``` > > Morever, because the `nla_validate_deprecated` just validate the attributes > array and will not try to retrieve the nla pointers. It shall be faster > and cleaner here :D. We don't care about speed in this path. Let's leave the code as is. Thanks > > Regards > Lin
diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c index 8175dde60b0a..c7407a53fcda 100644 --- a/drivers/infiniband/core/sa_query.c +++ b/drivers/infiniband/core/sa_query.c @@ -1047,14 +1047,13 @@ int ib_nl_handle_set_timeout(struct sk_buff *skb, static inline int ib_nl_is_good_resolve_resp(const struct nlmsghdr *nlh) { - struct nlattr *tb[LS_NLA_TYPE_MAX]; int ret; if (nlh->nlmsg_flags & RDMA_NL_LS_F_ERR) return 0; - ret = nla_parse_deprecated(tb, LS_NLA_TYPE_MAX - 1, nlmsg_data(nlh), - nlmsg_len(nlh), ib_nl_policy, NULL); + ret = nla_validate_deprecated(nlmsg_data(nlh), nlmsg_len(nlh), + LS_NLA_TYPE_MAX - 1, ib_nl_policy, NULL); if (ret) return 0;
The attributes array `tb` in ib_nl_is_good_resolve_resp is never used after the parsing. Therefore use nla_validate_deprecated function here for improvement. Signed-off-by: Lin Ma <linma@zju.edu.cn> --- drivers/infiniband/core/sa_query.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)