Testing with improper setting from user land exposed some bugs in
the kernel's code handling of these cases. For tunables sent from
user land we need to do proper range checking. An improper cast
in the new Netlink tunables code preventing setting the default
LND tunable settings. Also silently ignore trying to set LND
tunables when its not supported. We shouldn't stop NI setup in
this case. Lastly setup the NI tunables to -1 when user land
doesn't provide any input. This tells the LND driver to use it
default values for the tunables. Resolve a double free when
setting up a NI with a non-existing interface. Another fix is for
net locking in lnet_net_cmd().
For lnetctl fix the YAML handling when only conns_per_peer is
requested. I only tested conns_per_peer and NI tunables changes
together before which missed the mentioned case.
Fixes:
8f8f6e2f3 ("LU-10003 lnet: use Netlink to support old and new NI APIs.")
Test-Parameters: trivial testlist=sanity-lnet
Change-Id: I7c5e993de57e3d674ecb8e3cc1bd62506470d416
Signed-off-by: James Simmons <jsimmons@infradead.org>
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/49588
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Chris Horn <chris.horn@hpe.com>
Reviewed-by: Serguei Smirnov <ssmirnov@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
kiblnd_nl_set(int cmd, struct nlattr *attr, int type, void *data)
{
struct lnet_lnd_tunables *tunables = data;
+ s64 num;
if (cmd != LNET_CMD_NETS)
return -EOPNOTSUPP;
tunables->lnd_tun_u.lnd_o2ib.lnd_ntx = nla_get_s64(attr);
break;
case LNET_NET_O2IBLND_TUNABLES_ATTR_CONNS_PER_PEER:
- tunables->lnd_tun_u.lnd_o2ib.lnd_conns_per_peer = nla_get_s64(attr);
+ num = nla_get_s64(attr);
+ clamp_t(s64, num, 1, 127);
+ tunables->lnd_tun_u.lnd_o2ib.lnd_conns_per_peer = num;
+ fallthrough;
default:
break;
}
#include <linux/ethtool.h>
#include <linux/inetdevice.h>
-#include "socklnd.h"
+#include <linux/kernel.h>
#include <linux/sunrpc/addr.h>
#include <net/addrconf.h>
+#include "socklnd.h"
static const struct lnet_lnd the_ksocklnd;
struct ksock_nal_data ksocknal_data;
ksocknal_nl_set(int cmd, struct nlattr *attr, int type, void *data)
{
struct lnet_lnd_tunables *tunables = data;
+ s64 num;
if (cmd != LNET_CMD_NETS)
return -EOPNOTSUPP;
nla_type(attr) != LN_SCALAR_ATTR_INT_VALUE)
return -EINVAL;
- tunables->lnd_tun_u.lnd_sock.lnd_conns_per_peer = nla_get_s64(attr);
+ num = nla_get_s64(attr);
+ clamp_t(s64, num, 1, 127);
+ tunables->lnd_tun_u.lnd_sock.lnd_conns_per_peer = num;
return 0;
}
mutex_unlock(&the_lnet.ln_api_mutex);
- if (rc)
+ /* If NI already exist delete this new unused copy */
+ if (rc == -EEXIST)
lnet_ni_free(ni);
return rc;
num = nla_get_s64(param);
switch (type) {
case LNET_NET_LOCAL_NI_TUNABLES_ATTR_PEER_TIMEOUT:
- tun->lt_cmn.lct_peer_timeout = num;
+ if (num >= 0)
+ tun->lt_cmn.lct_peer_timeout = num;
break;
case LNET_NET_LOCAL_NI_TUNABLES_ATTR_PEER_CREDITS:
- tun->lt_cmn.lct_peer_tx_credits = num;
+ if (num > 0)
+ tun->lt_cmn.lct_peer_tx_credits = num;
break;
case LNET_NET_LOCAL_NI_TUNABLES_ATTR_PEER_BUFFER_CREDITS:
- tun->lt_cmn.lct_peer_rtr_credits = num;
+ if (num > 0)
+ tun->lt_cmn.lct_peer_rtr_credits = num;
break;
case LNET_NET_LOCAL_NI_TUNABLES_ATTR_CREDITS:
- tun->lt_cmn.lct_max_tx_credits = num;
+ if (num > 0)
+ tun->lt_cmn.lct_max_tx_credits = num;
break;
default:
rc = -EINVAL;
return rc;
}
-static int
-lnet_genl_parse_lnd_tunables(struct nlattr *settings,
- struct lnet_ioctl_config_lnd_tunables *tun,
- const struct lnet_lnd *lnd)
+static int lnet_genl_parse_lnd_tunables(struct nlattr *settings,
+ struct lnet_lnd_tunables *tun,
+ const struct lnet_lnd *lnd)
{
const struct ln_key_list *list = lnd->lnd_keys;
struct nlattr *param;
int rem, rc = 0;
int i = 1;
- if (!list)
+ /* silently ignore these setting if the LND driver doesn't
+ * support any LND tunables
+ */
+ if (!list || !lnd->lnd_nl_set || !list->lkl_maxattr)
return 0;
- if (!lnd->lnd_nl_set)
- return -EOPNOTSUPP;
-
- if (!list->lkl_maxattr)
- return -ERANGE;
-
nla_for_each_nested(param, settings, rem) {
if (nla_type(param) != LN_SCALAR_ATTR_VALUE)
continue;
}
rc = lnet_genl_parse_lnd_tunables(settings,
- tun, lnd);
+ &tun->lt_tun, lnd);
if (rc < 0) {
GENL_SET_ERR_MSG(info,
"failed to parse lnd tunables");
struct lnet_ioctl_config_lnd_tunables tun;
memset(&tun, 0, sizeof(tun));
+ /* Use LND defaults */
tun.lt_cmn.lct_peer_timeout = -1;
+ tun.lt_cmn.lct_peer_tx_credits = -1;
+ tun.lt_cmn.lct_peer_rtr_credits = -1;
+ tun.lt_cmn.lct_max_tx_credits = -1;
conf.lic_ncpts = 0;
rc = lnet_genl_parse_local_ni(entry, info,
if (!net) {
GENL_SET_ERR_MSG(info,
"LNet net doesn't exist");
+ lnet_net_unlock(LNET_LOCK_EX);
GOTO(out, rc);
}
list_for_each_entry(ni, &net->net_ni_list,
lnet_net_unlock(LNET_LOCK_EX);
rc = lnet_dyn_del_ni(&ni->ni_nid);
- lnet_net_lock(LNET_LOCK_EX);
if (rc < 0) {
GENL_SET_ERR_MSG(info,
"cannot del LNet NI");
break;
}
- lnet_net_unlock(LNET_LOCK_EX);
+ if (rc < 0) { /* will be -ENODEV */
+ GENL_SET_ERR_MSG(info,
+ "interface invalid for deleting LNet NI");
+ lnet_net_unlock(LNET_LOCK_EX);
+ }
} else {
rc = lnet_dyn_add_ni(&conf, net_id, &tun);
switch (rc) {
struct cfs_expr_list *global_cpts,
char *ip2net,
struct lnet_ioctl_config_lnd_tunables *tunables,
- long int cpp, int seq_no, struct cYAML **err_rc)
+ int seq_no, struct cYAML **err_rc)
{
char *data = NULL;
struct lnet_ioctl_config_ni *conf;
goto out;
}
- if (LNET_NETTYP(nw_descr->nw_id) == SOCKLND && (cpp > -1))
- tunables->lt_tun.lnd_tun_u.lnd_sock.lnd_conns_per_peer = cpp;
- else if (LNET_NETTYP(nw_descr->nw_id) == O2IBLND && (cpp > -1))
- tunables->lt_tun.lnd_tun_u.lnd_o2ib.lnd_conns_per_peer = cpp;
-
rc = lustre_lnet_intf2nids(nw_descr, &nids, &nnids,
err_str, sizeof(err_str));
if (rc != 0) {
rc = lustre_lnet_config_ni(&nw_descr, global_cpts,
(ip2net) ? ip2net->cy_valuestring : NULL,
- (found) ? &tunables : NULL, -1,
+ (found) ? &tunables : NULL,
(seq_no) ? seq_no->cy_valueint : -1,
err_rc);
struct cfs_expr_list *global_cpts,
char *ip2net,
struct lnet_ioctl_config_lnd_tunables *tunables,
- long int cpp, int seq_no, struct cYAML **err_rc);
+ int seq_no, struct cYAML **err_rc);
/*
* lustre_lnet_del_ni
static int yaml_add_ni_tunables(yaml_emitter_t *output,
struct lnet_ioctl_config_lnd_tunables *tunables,
- int cpp)
+ struct lnet_dlc_network_descr *nw_descr)
{
yaml_event_t event;
char num[23];
int rc;
+ if (tunables->lt_cmn.lct_peer_timeout < 0 &&
+ tunables->lt_cmn.lct_peer_tx_credits <= 0 &&
+ tunables->lt_cmn.lct_peer_rtr_credits <= 0 &&
+ tunables->lt_cmn.lct_max_tx_credits <= 0)
+ goto skip_general_settings;
+
yaml_scalar_event_initialize(&event, NULL,
(yaml_char_t *)YAML_STR_TAG,
(yaml_char_t *)"tunables",
if (rc == 0)
goto error;
- if (cpp > 0 || tunables->lt_tun.lnd_tun_u.lnd_kfi.lnd_auth_key > 0) {
+skip_general_settings:
+ if (tunables->lt_tun.lnd_tun_u.lnd_sock.lnd_conns_per_peer > 0 ||
+#ifdef HAVE_KFILND
+ tunables->lt_tun.lnd_tun_u.lnd_kfi.lnd_auth_key > 0 ||
+#endif
+ tunables->lt_tun.lnd_tun_u.lnd_o2ib.lnd_conns_per_peer > 0) {
yaml_scalar_event_initialize(&event, NULL,
(yaml_char_t *)YAML_STR_TAG,
(yaml_char_t *)"lnd tunables",
rc = yaml_emitter_emit(output, &event);
if (rc == 0)
goto error;
-
+#ifdef HAVE_KFILND
if (tunables->lt_tun.lnd_tun_u.lnd_kfi.lnd_auth_key > 0) {
yaml_scalar_event_initialize(&event, NULL,
(yaml_char_t *)YAML_STR_TAG,
snprintf(num, sizeof(num), "%u",
tunables->lt_tun.lnd_tun_u.lnd_kfi.lnd_auth_key);
- } else {
+
+ yaml_scalar_event_initialize(&event, NULL,
+ (yaml_char_t *)YAML_INT_TAG,
+ (yaml_char_t *)num,
+ strlen(num), 1, 0,
+ YAML_PLAIN_SCALAR_STYLE);
+ rc = yaml_emitter_emit(output, &event);
+ if (rc == 0)
+ goto error;
+ }
+#endif
+ if (tunables->lt_tun.lnd_tun_u.lnd_sock.lnd_conns_per_peer > 0 ||
+ tunables->lt_tun.lnd_tun_u.lnd_o2ib.lnd_conns_per_peer > 0) {
+ int cpp = 0;
+
yaml_scalar_event_initialize(&event, NULL,
(yaml_char_t *)YAML_STR_TAG,
(yaml_char_t *)"conns_per_peer",
if (rc == 0)
goto error;
+ if (LNET_NETTYP(nw_descr->nw_id) == SOCKLND)
+ cpp = tunables->lt_tun.lnd_tun_u.lnd_sock.lnd_conns_per_peer;
+ else if (LNET_NETTYP(nw_descr->nw_id) == O2IBLND)
+ cpp = tunables->lt_tun.lnd_tun_u.lnd_o2ib.lnd_conns_per_peer;
snprintf(num, sizeof(num), "%u", cpp);
- }
- yaml_scalar_event_initialize(&event, NULL,
- (yaml_char_t *)YAML_INT_TAG,
- (yaml_char_t *)num,
- strlen(num), 1, 0,
- YAML_PLAIN_SCALAR_STYLE);
- rc = yaml_emitter_emit(output, &event);
- if (rc == 0)
- goto error;
+ yaml_scalar_event_initialize(&event, NULL,
+ (yaml_char_t *)YAML_INT_TAG,
+ (yaml_char_t *)num,
+ strlen(num), 1, 0,
+ YAML_PLAIN_SCALAR_STYLE);
+ rc = yaml_emitter_emit(output, &event);
+ if (rc == 0)
+ goto error;
+ }
yaml_mapping_end_event_initialize(&event);
rc = yaml_emitter_emit(output, &event);
- if (rc == 0)
- goto error;
}
error:
return rc;
static int yaml_lnet_config_ni(char *net_id, char *ip2net,
struct lnet_dlc_network_descr *nw_descr,
struct lnet_ioctl_config_lnd_tunables *tunables,
- int cpp, struct cfs_expr_list *global_cpts,
+ struct cfs_expr_list *global_cpts,
int flags)
{
struct lnet_dlc_intf_descr *intf;
goto emitter_error;
if (tunables) {
- rc = yaml_add_ni_tunables(&output, tunables, cpp);
+ rc = yaml_add_ni_tunables(&output, tunables, nw_descr);
if (rc == 0)
goto emitter_error;
}
found = true;
}
#endif
+
+ if (LNET_NETTYP(nw_descr.nw_id) == SOCKLND && (cpp > -1)) {
+ tunables.lt_tun.lnd_tun_u.lnd_sock.lnd_conns_per_peer = cpp;
+ found = true;
+ } else if (LNET_NETTYP(nw_descr.nw_id) == O2IBLND && (cpp > -1)) {
+ tunables.lt_tun.lnd_tun_u.lnd_o2ib.lnd_conns_per_peer = cpp;
+ found = true;
+ }
+
if (pto >= 0 || pc > 0 || pbc > 0 || cre > 0 || cpp > -1) {
tunables.lt_cmn.lct_peer_timeout = pto;
tunables.lt_cmn.lct_peer_tx_credits = pc;
tunables.lt_tun.lnd_tun_u.lnd_o2ib.lnd_map_on_demand = UINT_MAX;
rc = yaml_lnet_config_ni(net_id, ip2net, &nw_descr,
- found ? &tunables : NULL, cpp,
+ found ? &tunables : NULL,
(cpt_rc == 0) ? global_cpts : NULL,
NLM_F_CREATE);
if (rc <= 0) {
rc = lustre_lnet_config_ni(&nw_descr,
(cpt_rc == 0) ? global_cpts: NULL,
ip2net, (found) ? &tunables : NULL,
- cpp, -1, &err_rc);
+ cpp, &err_rc);
if (global_cpts != NULL)
cfs_expr_list_free(global_cpts);
}
}
- rc = yaml_lnet_config_ni(net_id, NULL, &nw_descr, NULL, -1, NULL, 0);
+ rc = yaml_lnet_config_ni(net_id, NULL, &nw_descr, NULL, NULL, 0);
if (rc <= 0) {
if (rc != -EOPNOTSUPP)
return rc;