Re: [rdma-core ibacm v2] ibacm: only open InfiniBand port

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> On 10 Jun 2019, at 15:55, Honggang Li <honli@xxxxxxxxxx> wrote:
> 
> The low 64 bits of cxgb3 and cxgb4 devices' GID are zeros. If the
> "provider" was set in the option file, ibacm will fail with
> segment fault.
> 
> $ sed -i -e 's/# provider ibacmp 0xFE80000000000000/provider ibacmp 0xFE80000000000000/g' /etc/rdma/ibacm_opts.cfg
> $ /usr/sbin/ibacm --systemd
> Segmentation fault (core dumped)
> $ gdb /usr/sbin/ibacm core.ibacm
> (gdb) bt
> 0  0x00005625a4809217 in acm_assign_provider (port=0x5625a4bc6f28) at /usr/src/debug/rdma-core-25.0-1.el8.x86_64/ibacm/src/acm.c:2285
> 1  acm_port_up (port=0x5625a4bc6f28) at /usr/src/debug/rdma-core-25.0-1.el8.x86_64/ibacm/src/acm.c:2372
> 2  0x00005625a48073d2 in acm_activate_devices () at /usr/src/debug/rdma-core-25.0-1.el8.x86_64/ibacm/src/acm.c:2564
> 3  main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/rdma-core-25.0-1.el8.x86_64/ibacm/src/acm.c:3270
> 
> Note: The rpm was built with tarball generated from upstream repo. The last
> commit is aa41a65ec86bdb9c1c86e57885ee588b39558238.
> 
> acm_open_dev function should not open an umad port for iWARP or RoCE devices.

It is "a umad port" (as suggested), even though the "u" is a vowel, it is pronounced as a consonant, hence "a umad port".

> Signed-off-by: Honggang Li <honli@xxxxxxxxxx>
> ---
> ibacm/src/acm.c | 26 ++++++++++++++++++++++----
> 1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/ibacm/src/acm.c b/ibacm/src/acm.c
> index a21069d4..5c8a5d3c 100644
> --- a/ibacm/src/acm.c
> +++ b/ibacm/src/acm.c
> @@ -2600,9 +2600,11 @@ static void acm_open_dev(struct ibv_device *ibdev)
> {
> 	struct acmc_device *dev;
> 	struct ibv_device_attr attr;
> +	struct ibv_port_attr port_attr;
> 	struct ibv_context *verbs;
> 	size_t size;
> 	int i, ret;
> +	unsigned int opened_ib_port_cnt = 0;
> 
> 	acm_log(1, "%s\n", ibdev->name);
> 	verbs = ibv_open_device(ibdev);
> @@ -2628,13 +2630,29 @@ static void acm_open_dev(struct ibv_device *ibdev)
> 	list_head_init(&dev->prov_dev_context_list);
> 
> 	for (i = 0; i < dev->port_cnt; i++) {
> +		acm_log(1, "%s port %d\n", ibdev->name, i + 1);
> +		ret = ibv_query_port(dev->device.verbs, i + 1, &port_attr);
> +		if (ret) {
> +			acm_log(0, "ERROR - ibv_query_port failed\n");

With the richness below when printing port or ports, may be add the value of ret here as well?

> +			continue;
> +		}
> +		if (port_attr.link_layer != IBV_LINK_LAYER_INFINIBAND) {
> +			acm_log(1, "not an InfiniBand port\n");
> +			continue;
> +		}
> +
> 		acm_open_port(&dev->port[i], dev, i + 1);
> +		opened_ib_port_cnt++;
> 	}
> 
> -	list_add(&dev_list, &dev->entry);
> -
> -	acm_log(1, "%s opened\n", ibdev->name);
> -	return;
> +	if (opened_ib_port_cnt > 0) {

or simpler, if (opened_ib_port_cnt) {

> +		list_add(&dev_list, &dev->entry);
> +		acm_log(1, "%d InfiniBand %s opened for %s\n",
> +				opened_ib_port_cnt,
> +				opened_ib_port_cnt == 1 ? "port":"ports",

Spaces around ":". I am also OK with the literal "port(s)" as well.

Otherwise, LGTM,

Signed-off-by: Håkon Bugge <haakon.bugge@xxxxxxxxxx>


Thxs, Håkon


> +				ibdev->name);
> +		return;
> +	}
> 
> err1:
> 	ibv_close_device(verbs);
> -- 
> 2.20.1
> 





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux