netdev - Re: [PATCH v1 iproute2-next 1/4] rdma: add helper rd_sendrecv

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 28 Feb 2019 21:56:12 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Steve Wise <swise@...ngridcomputing.com>
Cc:     dsahern@...il.com, stephen@...workplumber.org,
        netdev@...r.kernel.org, linux-rdma@...r.kernel.org
Subject: Re: [PATCH v1 iproute2-next 1/4] rdma: add helper rd_sendrecv_msg()

On Thu, Feb 28, 2019 at 01:41:20PM -0600, Steve Wise wrote:
>
> On 2/23/2019 3:26 AM, Leon Romanovsky wrote:
> > On Thu, Feb 21, 2019 at 08:19:03AM -0800, Steve Wise wrote:
> >> This function sends the constructed netlink message and then
> >> receives the response, displaying any error text.
> >>
> >> Change 'rdma dev set' to use it.
> >>
> >> Signed-off-by: Steve Wise <swise@...ngridcomputing.com>
> >> ---
> >>  rdma/dev.c   |  2 +-
> >>  rdma/rdma.h  |  1 +
> >>  rdma/utils.c | 21 +++++++++++++++++++++
> >>  3 files changed, 23 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/rdma/dev.c b/rdma/dev.c
> >> index 60ff4b31e320..d2949c378f08 100644
> >> --- a/rdma/dev.c
> >> +++ b/rdma/dev.c
> >> @@ -273,7 +273,7 @@ static int dev_set_name(struct rd *rd)
> >>  	mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_DEV_INDEX, rd->dev_idx);
> >>  	mnl_attr_put_strz(rd->nlh, RDMA_NLDEV_ATTR_DEV_NAME, rd_argv(rd));
> >>
> >> -	return rd_send_msg(rd);
> >> +	return rd_sendrecv_msg(rd, seq);
> >>  }
> >>
> >>  static int dev_one_set(struct rd *rd)
> >> diff --git a/rdma/rdma.h b/rdma/rdma.h
> >> index 547bb5749a39..20be2f12c4f8 100644
> >> --- a/rdma/rdma.h
> >> +++ b/rdma/rdma.h
> >> @@ -115,6 +115,7 @@ bool rd_check_is_key_exist(struct rd *rd, const char *key);
> >>   */
> >>  int rd_send_msg(struct rd *rd);
> >>  int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, uint32_t seq);
> >> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq);
> >>  void rd_prepare_msg(struct rd *rd, uint32_t cmd, uint32_t *seq, uint16_t flags);
> >>  int rd_dev_init_cb(const struct nlmsghdr *nlh, void *data);
> >>  int rd_attr_cb(const struct nlattr *attr, void *data);
> >> diff --git a/rdma/utils.c b/rdma/utils.c
> >> index 069d44fece10..a6f2826c9605 100644
> >> --- a/rdma/utils.c
> >> +++ b/rdma/utils.c
> >> @@ -664,6 +664,27 @@ int rd_recv_msg(struct rd *rd, mnl_cb_t callback, void *data, unsigned int seq)
> >>  	return ret;
> >>  }
> >>
> >> +static int null_cb(const struct nlmsghdr *nlh, void *data)
> >> +{
> >> +	return MNL_CB_OK;
> >> +}
> >> +
> >> +int rd_sendrecv_msg(struct rd *rd, unsigned int seq)
> >> +{
> >> +	int ret;
> >> +
> >> +	ret = rd_send_msg(rd);
> >> +	if (ret) {
> >> +		perror(NULL);
> > This is more or less already done in rd_send_msg() and that function
> > prints something in case of execution error. So the missing piece
> > is to update rd_recv_msg(), so all places will "magically" print errors
> > and not only dev_set_name().
>
> Hey Leon,
>
> Displaying errors in rd_recv_msg() as you suggested:
>
> @@ -693,10 +693,28 @@ int rd_recv_msg(struct rd *rd, mnl_cb_t callback,
> void *data, unsigned int seq)
>                 ret = mnl_cb_run(buf, ret, seq, portid, callback, data);
>         } while (ret > 0);
>
> +       if (ret < 0)
> +               perror(NULL);
> +
>         mnl_socket_close(rd->nl);
>         return ret;
>  }
>
> Causes problems when doing filtered dumps:
>
> [root@...vo1 iproute2]# ./rdma/rdma res show qp
> link mlx5_0/1 lqpn 1 type GSI state RTS sq-psn 0 comm [ib_core]
> link mlx5_0/1 lqpn 176 type UD state RTS sq-psn 0 comm [ib_core]
> link mlx5_1/1 lqpn 1 type GSI state RTS sq-psn 0 comm [ib_core]
> link mlx5_1/1 lqpn 2224 type UD state RTS sq-psn 0 comm [ib_core]
> link rxe_foo0/1 lqpn 1 type GSI state RTS sq-psn 0 comm [ib_core]
> [root@...vo1 iproute2]# ./rdma/rdma res show qp  lqpn 2224
> Invalid argument
> No such file or directory

Why do we it? We are supposed to check "invalid argument" before sending
message and are not supposed to see perror(). I'm not near code right
now, so most probably wrong in my assumption.

> Invalid argument
> link mlx5_1/1 lqpn 2224 type UD state RTS sq-psn 0 comm [ib_core]
> Invalid argument
> No such file or directory
> [root@...vo1 iproute2]#
>
>
> Steve.
>

Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)