[ofa-general] Re: [PATCH v3 1/3] Create a new library libibnetdisc

[Sasha Khapyorsky]

On 15:42 Fri 03 Apr     , Ira Weiny wrote:
> diff --git a/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h b/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
> new file mode 100644
> index 0000000..a882994
> --- /dev/null
> +++ b/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
> @@ -0,0 +1,188 @@
> +/*
> + * Copyright (c) 2008 Lawrence Livermore National Lab.  All rights reserved.
> + *
> + * This software is available to you under a choice of one of two
> + * licenses.  You may choose to be licensed under the terms of the GNU
> + * General Public License (GPL) Version 2, available from the file
> + * COPYING in the main directory of this source tree, or the
> + * OpenIB.org BSD license below:
> + *
> + *     Redistribution and use in source and binary forms, with or
> + *     without modification, are permitted provided that the following
> + *     conditions are met:
> + *
> + *      - Redistributions of source code must retain the above
> + *        copyright notice, this list of conditions and the following
> + *        disclaimer.
> + *
> + *      - Redistributions in binary form must reproduce the above
> + *        copyright notice, this list of conditions and the following
> + *        disclaimer in the documentation and/or other materials
> + *        provided with the distribution.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + *
> + */
> +
> +#ifndef _IBNETDISC_H_
> +#define _IBNETDISC_H_
> +
> +#include <stdio.h>
> +#include <infiniband/mad.h>
> +#include <iba/ib_types.h>
> +
> +struct ib_fabric; /* forward declare */
> +struct chassis; /* forward declare */
> +struct port; /* forward declare */
> +
> +/** =========================================================================
> + * Node
> + */
> +typedef struct node {
> +	struct node *next; /* all node list in fabric */
> +	struct ib_fabric *fabric; /* the fabric node belongs to */
> +
> +	ib_portid_t path_portid; /* path from "from_node" */
> +	int dist; /* num of hops from "from_node" */
> +	int smalid;
> +	int smalmc;
> +
> +	/* quick cache of switchinfo below */
> +	int smaenhsp0;
> +	/* use libibmad decoder functions for switchinfo */
> +	//WHY does this not work???
> +	//uint8_t switchinfo[sizeof (ib_switch_info_t)];

This is a right question - sizeof(ib_switch_info_t) < 64.

> +	uint8_t switchinfo[64];
> +
> +	/* quick cache of info below */
> +	uint64_t guid;
> +	int type;
> +	int numports;
> +	/* use libibmad decoder functions for info */
> +	uint8_t info[sizeof(ib_node_info_t)];

Above, here and in some other places. Those buffers are used as rcvdata
with smp_query_via(), it assumes SMP MADs and 64 bytes of data is always
copied there. So when actual buffer is smaller bad things may happen.
I'm fixing this with such addition:

diff --git a/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h b/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
index a882994..bc108ab 100644
--- a/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
+++ b/infiniband-diags/libibnetdisc/include/infiniband/ibnetdisc.h
@@ -57,18 +57,16 @@ typedef struct node {
 	/* quick cache of switchinfo below */
 	int smaenhsp0;
 	/* use libibmad decoder functions for switchinfo */
-	//WHY does this not work???
-	//uint8_t switchinfo[sizeof (ib_switch_info_t)];
-	uint8_t switchinfo[64];
+	uint8_t switchinfo[IB_SMP_DATA_SIZE];
 
 	/* quick cache of info below */
 	uint64_t guid;
 	int type;
 	int numports;
 	/* use libibmad decoder functions for info */
-	uint8_t info[sizeof(ib_node_info_t)];
+	uint8_t info[IB_SMP_DATA_SIZE];
 
-	char nodedesc[IB_NODE_DESCRIPTION_SIZE];
+	char nodedesc[IB_SMP_DATA_SIZE];
 
 	struct port **ports; /* in order array of port pointers */
 				/* the size of this array is info.numports + 1 */
@@ -96,7 +94,7 @@ typedef struct port {
 	uint16_t base_lid;
 	uint8_t lmc;
 	/* use libibmad decoder functions for info */
-	uint8_t info[sizeof(ib_port_info_t)];
+	uint8_t info[IB_SMP_DATA_SIZE];
 } ibnd_port_t;
 
 
diff --git a/infiniband-diags/libibnetdisc/src/ibnetdisc.c b/infiniband-diags/libibnetdisc/src/ibnetdisc.c
index 479bae7..3fd3b76 100644
--- a/infiniband-diags/libibnetdisc/src/ibnetdisc.c
+++ b/infiniband-diags/libibnetdisc/src/ibnetdisc.c
@@ -231,7 +231,7 @@ ibnd_find_node_guid(ibnd_fabric_t *fabric, uint64_t guid)
 ibnd_node_t *
 ibnd_update_node(ibnd_node_t *node)
 {
-	char portinfo_port0[sizeof (ib_port_info_t)];
+	char portinfo_port0[IB_SMP_DATA_SIZE];
 	void *nd = node->nodedesc;
 	int p = 0;
 	struct ibnd_fabric *f = CONV_FABRIC_INTERNAL(node->fabric);

Sasha