ibacm has a fixed number (4) of end-point addresses. An endpoint is
associated with the node GUID. Hence, if a system is brought up with
VFs enabled, ibacm will not be able to handle the end-point addresses.
When adding an address, the default provider, acmp, saves a pointer
into ibacm's acmc_ep addr_info attribute. But this may now be
re-allocated. Its generally a bad idea to have pointers in the
provider(s) pointing into acm's data structures.
This is fixed in acmp by changing struct acmp_addr to contain struct
acm_address instead of a pointer to it, and copying the address,
similar to how the info struct is handled.
In acm_svr_ep_query(), dynamic allocation and conditional reallocating
is implemented, in the case the number of end-point addresses exceeds
the fixed size of struct acm_msg. A re-factoring in
acm_nl_process_resolve() and in the acm_svr_foo_bar() functions was
necessary to accommodate the dynamic allocation of struct acm_msg.
In acm_if_iter_sys(), the number of struct ifreq's was increased to
256, in order to be able to test with up-to 256 addresses when ibacm
starts.
Finally, in ib_acm_enum_ep(), the read() was split in two, first read
the hdr, then determine the size of the rest of the data, allocate it,
and read them. To avoid endian conversion to/from the same structure,
two structures were allocated for simplicity.
Signed-off-by: Håkon Bugge <haakon.bugge@xxxxxxxxxx>
---
ibacm/prov/acmp/src/acmp.c | 6 +-
ibacm/src/acm.c | 155 +++++++++++++++++++++++++++++++--------------
ibacm/src/acm_util.c | 2 +-
ibacm/src/libacm.c | 50 ++++++++++-----
4 files changed, 145 insertions(+), 68 deletions(-)
diff --git a/ibacm/prov/acmp/src/acmp.c b/ibacm/prov/acmp/src/acmp.c
index c10e62c0..f63f1dbf 100644
--- a/ibacm/prov/acmp/src/acmp.c
+++ b/ibacm/prov/acmp/src/acmp.c
@@ -166,7 +166,7 @@ struct acmp_send_queue {
struct acmp_addr {
uint16_t type;
union acm_ep_info info;
- struct acm_address *addr;
+ struct acm_address addr;
struct acmp_ep *ep;
};
@@ -2379,7 +2379,7 @@ static int acmp_add_addr(const struct acm_address *addr, void *ep_context,
}
ep->addr_info[i].type = addr->type;
memcpy(&ep->addr_info[i].info, &addr->info, sizeof(addr->info));
- ep->addr_info[i].addr = (struct acm_address *) addr;
+ memcpy(&ep->addr_info[i].addr, addr, sizeof(*addr));
ep->addr_info[i].ep = ep;
if (loopback_prot != ACMP_LOOPBACK_PROT_LOCAL) {
@@ -2439,7 +2439,7 @@ static void acmp_remove_addr(void *addr_context)
pthread_mutex_lock(&port->lock);
list_for_each(&port->ep_list, ep, entry) {
pthread_mutex_unlock(&port->lock);
- dest = acmp_get_dest(ep, address->type, address->addr->info.addr);
+ dest = acmp_get_dest(ep, address->type, address->addr.info.addr);
if (dest) {
acm_log(2, "Found a dest addr, deleting it\n");
pthread_mutex_lock(&ep->lock);
diff --git a/ibacm/src/acm.c b/ibacm/src/acm.c
index 182e89d9..6a02f33d 100644
--- a/ibacm/src/acm.c
+++ b/ibacm/src/acm.c
@@ -72,7 +72,6 @@
#define src_index data[1]
#define dst_index data[2]
-#define MAX_EP_ADDR 4
#define NL_MSG_BUF_SIZE 4096
#define ACM_PROV_NAME_SIZE 64
#define NL_CLIENT_INDEX 0
@@ -139,7 +138,8 @@ struct acmc_ep {
struct acmc_port *port;
struct acm_endpoint endpoint;
void *prov_ep_context;
- struct acmc_addr addr_info[MAX_EP_ADDR];
+ int nmbr_ep_addrs;
+ struct acmc_addr *addr_info;
struct list_node entry;
};
@@ -421,7 +421,7 @@ static void acm_mark_addr_invalid(struct acmc_ep *ep,
{
int i;
- for (i = 0; i < MAX_EP_ADDR; i++) {
+ for (i = 0; i < ep->nmbr_ep_addrs; i++) {
if (!acm_addr_cmp(&ep->addr_info[i].addr, data->info.addr, data->type)) {
ep->addr_info[i].addr.type = ACM_ADDRESS_INVALID;
ep->port->prov->remove_address(ep->addr_info[i].prov_addr_context);
@@ -437,7 +437,7 @@ acm_addr_lookup(const struct acm_endpoint *endpoint, uint8_t *addr, uint8_t addr
int i;
ep = container_of(endpoint, struct acmc_ep, endpoint);
- for (i = 0; i < MAX_EP_ADDR; i++)
+ for (i = 0; i < ep->nmbr_ep_addrs; i++)
if (!acm_addr_cmp(&ep->addr_info[i].addr, addr, addr_type))
return &ep->addr_info[i].addr;
@@ -838,7 +838,7 @@ acm_get_port_ep_address(struct acmc_port *port, struct acm_ep_addr_data *data)
if ((data->type == ACM_EP_INFO_PATH) &&
(!data->info.path.pkey ||
(be16toh(data->info.path.pkey) == ep->endpoint.pkey))) {
- for (i = 0; i < MAX_EP_ADDR; i++) {
+ for (i = 0; i < ep->nmbr_ep_addrs; i++) {
if (ep->addr_info[i].addr.type)
return &ep->addr_info[i];
}
@@ -1118,17 +1118,22 @@ acm_svr_resolve_path(struct acmc_client *client, struct acm_msg *msg)
static int acm_svr_resolve(struct acmc_client *client, struct acm_msg *msg)
{
+ int ret = 0;
+
(void) atomic_inc(&client->refcnt);
if (msg->resolve_data[0].type == ACM_EP_INFO_PATH) {
if (msg->resolve_data[0].flags & ACM_FLAGS_QUERY_SA) {
- return acm_svr_query_path(client, msg);
+ ret = acm_svr_query_path(client, msg);
} else {
- return acm_svr_resolve_path(client, msg);
+ ret = acm_svr_resolve_path(client, msg);
}
} else {
- return acm_svr_resolve_dest(client, msg);
+ ret = acm_svr_resolve_dest(client, msg);
}
+
+ free(msg);
+ return ret;
}
static int acm_svr_perf_query(struct acmc_client *client, struct acm_msg *msg)
@@ -1181,15 +1186,48 @@ static int acm_svr_perf_query(struct acmc_client *client, struct acm_msg *msg)
else
ret = 0;
+ free(msg);
return ret;
}
+static int may_be_realloc(struct acm_msg **msg_ptr,
+ int len,
+ int cnt,
+ int *cur_msg_siz_ptr,
+ int max_msg_siz)
+{
+
+ /* Check if a new address exceeds the protocol constrained max size */
+ if (len + (cnt + 1) * ACM_MAX_ADDRESS > max_msg_siz) {
+ acm_log(0, "ERROR - unable to amend more addresses to acm_msg due to protocol constraints\n");
+ return ENOMEM;
+ }
+
+ /* Check if a new address exeeds current size of msg */
+ if (len + (cnt + 1) * ACM_MAX_ADDRESS > *cur_msg_siz_ptr) {
+ const size_t chunk_size = 16 * ACM_MAX_ADDRESS;
+ struct acm_msg *new_msg = realloc(*msg_ptr, *cur_msg_siz_ptr + chunk_size);
+
+ if (!new_msg) {
+ acm_log(0, "ERROR - failed to allocate longer acm_msg\n");
+ return ENOMEM;
+ }
+
+ *msg_ptr = new_msg;
+ *cur_msg_siz_ptr += chunk_size;
+ }
+
+ return 0;
+}
+
static int acm_svr_ep_query(struct acmc_client *client, struct acm_msg *msg)
{
int ret, i;
uint16_t len;
struct acmc_ep *ep;
int index, cnt = 0;
+ int cur_msg_siz = sizeof(*msg);
+ int max_msg_siz = USHRT_MAX;
acm_log(2, "client %d\n", client->index);
index = msg->hdr.data[0];
@@ -1203,8 +1241,10 @@ static int acm_svr_ep_query(struct acmc_client *client, struct acm_msg *msg)
ACM_MAX_PROV_NAME - 1);
msg->ep_data[0].prov_name[ACM_MAX_PROV_NAME - 1] = '\0';
len = ACM_MSG_HDR_LENGTH + sizeof(struct acm_ep_config_data);
- for (i = 0; i < MAX_EP_ADDR; i++) {
+ for (i = 0; i < ep->nmbr_ep_addrs; i++) {
if (ep->addr_info[i].addr.type != ACM_ADDRESS_INVALID) {
+ if (may_be_realloc(&msg, len, cnt, &cur_msg_siz, max_msg_siz))
+ break;
memcpy(msg->ep_data[0].addrs[cnt++].name,
ep->addr_info[i].string_buf,
ACM_MAX_ADDRESS);
@@ -1227,6 +1267,7 @@ static int acm_svr_ep_query(struct acmc_client *client, struct acm_msg *msg)
else
ret = 0;
+ free(msg);
return ret;
}
@@ -1238,38 +1279,47 @@ static int acm_msg_length(struct acm_msg *msg)
static void acm_svr_receive(struct acmc_client *client)
{
- struct acm_msg msg;
+ struct acm_msg *msg = malloc(sizeof(*msg));
int ret;
+ if (!msg) {
+ acm_log(0, "ERROR - Unable to alloc acm_msg\n");
+ ret = ENOMEM;
+ goto out;
+ }
+
acm_log(2, "client %d\n", client->index);
- ret = recv(client->sock, (char *) &msg, sizeof msg, 0);
- if (ret <= 0 || ret != acm_msg_length(&msg)) {
+ ret = recv(client->sock, (char *) msg, sizeof(*msg), 0);
+ if (ret <= 0 || ret != acm_msg_length(msg)) {
acm_log(2, "client disconnected\n");
ret = ACM_STATUS_ENOTCONN;
- goto out;
+ goto out_free;
}
- if (msg.hdr.version != ACM_VERSION) {
- acm_log(0, "ERROR - unsupported version %d\n", msg.hdr.version);
- goto out;
+ if (msg->hdr.version != ACM_VERSION) {
+ acm_log(0, "ERROR - unsupported version %d\n", msg->hdr.version);
+ goto out_free;
}
- switch (msg.hdr.opcode & ACM_OP_MASK) {
+ switch (msg->hdr.opcode & ACM_OP_MASK) {
case ACM_OP_RESOLVE:
atomic_inc(&counter[ACM_CNTR_RESOLVE]);
- ret = acm_svr_resolve(client, &msg);
- break;
+ ret = acm_svr_resolve(client, msg);
+ goto out;
case ACM_OP_PERF_QUERY:
- ret = acm_svr_perf_query(client, &msg);
- break;
+ ret = acm_svr_perf_query(client, msg);
+ goto out;
case ACM_OP_EP_QUERY:
- ret = acm_svr_ep_query(client, &msg);
- break;
+ ret = acm_svr_ep_query(client, msg);
+ goto out;
default:
- acm_log(0, "ERROR - unknown opcode 0x%x\n", msg.hdr.opcode);
+ acm_log(0, "ERROR - unknown opcode 0x%x\n", msg->hdr.opcode);
break;
}
+out_free:
+ free(msg);
+
out:
if (ret)
acm_disconnect_client(client);
@@ -1412,7 +1462,7 @@ static int resync_system_ips(void)
port = &dev->port[cnt];
list_for_each(&port->ep_list, ep, entry) {
- for (i = 0; i < MAX_EP_ADDR; i++) {
+ for (i = 0; i < ep->nmbr_ep_addrs; i++) {
if (ep->addr_info[i].addr.type == ACM_ADDRESS_IP ||
ep->addr_info[i].addr.type == ACM_ADDRESS_IP6)
ep->addr_info[i].addr.type = ACM_ADDRESS_INVALID;
@@ -1672,7 +1722,7 @@ static void acm_nl_process_invalid_request(struct acmc_client *client,
static void acm_nl_process_resolve(struct acmc_client *client,
struct acm_nl_msg *acmnlmsg)
{
- struct acm_msg msg;
+ struct acm_msg *msg = malloc(sizeof(*msg));
struct nlattr *attr;
int payload_len;
int resolve_hdr_len;
@@ -1681,21 +1731,26 @@ static void acm_nl_process_resolve(struct acmc_client *client,
int status;
unsigned char *data;
- memset(&msg, 0, sizeof(msg));
- msg.hdr.opcode = ACM_OP_RESOLVE;
- msg.hdr.version = ACM_VERSION;
- msg.hdr.length = ACM_MSG_HDR_LENGTH + ACM_MSG_EP_LENGTH;
- msg.hdr.status = ACM_STATUS_SUCCESS;
- msg.hdr.tid = (uintptr_t) acmnlmsg;
- msg.resolve_data[0].type = ACM_EP_INFO_PATH;
+ if (!msg) {
+ acm_log(0, "ERROR - Unable to allocate msg\n");
+ return;
+ }
+
+ memset(msg, 0, sizeof(*msg));
+ msg->hdr.opcode = ACM_OP_RESOLVE;
+ msg->hdr.version = ACM_VERSION;
+ msg->hdr.length = ACM_MSG_HDR_LENGTH + ACM_MSG_EP_LENGTH;
+ msg->hdr.status = ACM_STATUS_SUCCESS;
+ msg->hdr.tid = (uintptr_t) acmnlmsg;
+ msg->resolve_data[0].type = ACM_EP_INFO_PATH;
/* We support only one pathrecord */
acm_log(2, "path use 0x%x\n", acmnlmsg->resolve_header.path_use);
if (acmnlmsg->resolve_header.path_use ==
LS_RESOLVE_PATH_USE_UNIDIRECTIONAL)
- msg.resolve_data[0].info.path.reversible_numpath = 1;
+ msg->resolve_data[0].info.path.reversible_numpath = 1;
else
- msg.resolve_data[0].info.path.reversible_numpath =
+ msg->resolve_data[0].info.path.reversible_numpath =
IBV_PATH_RECORD_REVERSIBLE | 1;
data = (unsigned char *) &acmnlmsg->nlmsg_header + NLMSG_HDRLEN;
@@ -1710,7 +1765,7 @@ static void acm_nl_process_resolve(struct acmc_client *client,
attr->nla_len > rem)
break;
- status = acm_nl_parse_path_attr(attr, &msg.resolve_data[0]);
+ status = acm_nl_parse_path_attr(attr, &msg->resolve_data[0]);
if (status) {
acm_nl_process_invalid_request(client, acmnlmsg);
return;
@@ -1723,7 +1778,7 @@ static void acm_nl_process_resolve(struct acmc_client *client,
}
atomic_inc(&counter[ACM_CNTR_RESOLVE]);
- acm_svr_resolve(client, &msg);
+ acm_svr_resolve(client, msg);
}
static int acm_nl_is_valid_resolve_request(struct acm_nl_msg *acmnlmsg)
@@ -2018,12 +2073,21 @@ acm_ep_insert_addr(struct acmc_ep *ep, const char *name, uint8_t *addr,
memcpy(tmp, addr, acm_addr_len(addr_type));
if (!acm_addr_lookup(&ep->endpoint, addr, addr_type)) {
- for (i = 0; (i < MAX_EP_ADDR) &&
+ for (i = 0; (i < ep->nmbr_ep_addrs) &&
(ep->addr_info[i].addr.type != ACM_ADDRESS_INVALID); i++)
;
- if (i == MAX_EP_ADDR) {
- ret = ENOMEM;
- goto out;
+ if (i == ep->nmbr_ep_addrs) {
+ ++ep->nmbr_ep_addrs;
+ ep->addr_info = realloc(ep->addr_info, ep->nmbr_ep_addrs * sizeof(*ep->addr_info));
+ if (!ep->addr_info) {
+ ret = ENOMEM;
+ --ep->nmbr_ep_addrs;
+ goto out;
+ }
+ /* Added memory is not initialized */
+ memset(ep->addr_info + i, 0, sizeof(*ep->addr_info));
+ ep->addr_info[i].addr.endpoint = &ep->endpoint;
+ ep->addr_info[i].addr.id_string = ep->addr_info[i].string_buf;
}
/* Open the provider endpoint only if at least a name or
@@ -2194,7 +2258,7 @@ static void acm_ep_down(struct acmc_ep *ep)
acm_log(1, "%s %d pkey 0x%04x\n",
ep->port->dev->device.verbs->device->name,
ep->port->port.port_num, ep->endpoint.pkey);
- for (i = 0; i < MAX_EP_ADDR; i++) {
+ for (i = 0; i < ep->nmbr_ep_addrs; i++) {
if (ep->addr_info[i].addr.type &&
ep->addr_info[i].prov_addr_context)
ep->port->prov->remove_address(ep->addr_info[i].
@@ -2211,7 +2275,6 @@ static struct acmc_ep *
acm_alloc_ep(struct acmc_port *port, uint16_t pkey)
{
struct acmc_ep *ep;
- int i;
acm_log(1, "\n");
ep = calloc(1, sizeof *ep);
@@ -2221,11 +2284,7 @@ acm_alloc_ep(struct acmc_port *port, uint16_t pkey)
ep->port = port;
ep->endpoint.port = &port->port;
ep->endpoint.pkey = pkey;
-
- for (i = 0; i < MAX_EP_ADDR; i++) {
- ep->addr_info[i].addr.endpoint = &ep->endpoint;
- ep->addr_info[i].addr.id_string = ep->addr_info[i].string_buf;
- }
+ ep->nmbr_ep_addrs = 0;
return ep;
}
diff --git a/ibacm/src/acm_util.c b/ibacm/src/acm_util.c
index f4654f12..0cc23304 100644
--- a/ibacm/src/acm_util.c
+++ b/ibacm/src/acm_util.c
@@ -143,7 +143,7 @@ int acm_if_iter_sys(acm_if_iter_cb cb, void *ctx)
return -1;
}
- len = sizeof(*ifc) + sizeof(*ifr) * 64;
+ len = sizeof(*ifc) + sizeof(*ifr) * 256;
ifc = malloc(len);
if (!ifc) {
ret = -1;
diff --git a/ibacm/src/libacm.c b/ibacm/src/libacm.c
index 1d9d7145..8f0bfc2a 100644
--- a/ibacm/src/libacm.c
+++ b/ibacm/src/libacm.c
@@ -384,11 +384,13 @@ out:
int ib_acm_enum_ep(int index, struct acm_ep_config_data **data)
{
+ struct acm_ep_config_data *netw_edata = NULL;
+ struct acm_ep_config_data *host_edata = NULL;
struct acm_msg msg;
+ struct acm_hdr hdr;
int ret;
int len;
- int cnt;
- struct acm_ep_config_data *edata;
+ int i;
pthread_mutex_lock(&acm_lock);
memset(&msg, 0, sizeof msg);
@@ -401,33 +403,49 @@ int ib_acm_enum_ep(int index, struct acm_ep_config_data **data)
if (ret != ACM_MSG_HDR_LENGTH)
goto out;
- ret = recv(sock, (char *) &msg, sizeof msg, 0);
- if (ret < ACM_MSG_HDR_LENGTH || ret != be16toh(msg.hdr.length)) {
+ ret = recv(sock, (char *) &hdr, sizeof(hdr), 0);
+ if (ret != sizeof(hdr)) {
ret = ACM_STATUS_EINVAL;
goto out;
}
- if (msg.hdr.status) {
- ret = acm_error(msg.hdr.status);
+ if (hdr.status) {
+ ret = acm_error(hdr.status);
goto out;
}
- cnt = be16toh(msg.ep_data[0].addr_cnt);
- len = sizeof(struct acm_ep_config_data) +
- ACM_MAX_ADDRESS * cnt;
- edata = malloc(len);
- if (!edata) {
+ len = be16toh(hdr.length) - sizeof(hdr);
+ netw_edata = (struct acm_ep_config_data *)malloc(len);
+ host_edata = (struct acm_ep_config_data *)malloc(len);
+ if (!netw_edata || !host_edata) {
ret = ACM_STATUS_ENOMEM;
goto out;
}
- memcpy(edata, &msg.ep_data[0], len);
- edata->dev_guid = be64toh(msg.ep_data[0].dev_guid);
- edata->pkey = be16toh(msg.ep_data[0].pkey);
- edata->addr_cnt = cnt;
- *data = edata;
+ ret = recv(sock, (char *)netw_edata, len, 0);
+ if (ret != len) {
+ ret = ACM_STATUS_EINVAL;
+ goto out;
+ }
+
+ host_edata->dev_guid = be64toh(netw_edata->dev_guid);
+ host_edata->port_num = netw_edata->port_num;
+ host_edata->pkey = be16toh(netw_edata->pkey);
+ host_edata->addr_cnt = be16toh(netw_edata->addr_cnt);
+
+ memcpy(host_edata->prov_name, netw_edata->prov_name,
+ sizeof(host_edata->prov_name));
+
+ for (i = 0; i < host_edata->addr_cnt; ++i)
+ host_edata->addrs[i] = netw_edata->addrs[i];
+
+ *data = host_edata;
ret = 0;
out:
+ if (ret)
+ free(host_edata);
+ free(netw_edata);
+
pthread_mutex_unlock(&acm_lock);
return ret;
}
--
2.14.3
