[lustre-devel] [PATCH 11/34] LU-7734 lnet: configure local NI from DLC

NeilBrown neilb at suse.com
Mon Oct 1 20:19:36 PDT 2018


On Sat, Sep 29 2018, James Simmons wrote:

>> From: Amir Shehata <amir.shehata at intel.com>
>> 
>> This patch adds the ability to configure multiple network interfaces
>> on the same network. This can be done via the lnetctl CLI interface
>> or through a YAML configuration. Refer to the multi-rail HLD for
>> more details on the syntax.
>> 
>> It also deprecates ip2nets kernel parsing. All string parsing and
>> network maching now happens in the DLC userspace library.
>> 
>> New IOCTLs are added for adding/deleting local NIs, to keep backwards
>> compatibility with older version of the DLC and lnetctl.
>> 
>> The changes also include parsing and matching ip2nets syntax at the
>> user level and then passing down the network interfaces down to the
>> kernel to be configured.
>
> Nak. This patch introduces a regression in lnet_ioctl() found in
> module.c
>  
>> Signed-off-by: Amir Shehata <amir.shehata at intel.com>
>> Change-Id: I19ee7dc76514beb6f34de6517d19654d6468bcec
>> Reviewed-on: http://review.whamcloud.com/18886
>> Tested-by: Maloo <hpdd-maloo at intel.com>
>> Signed-off-by: NeilBrown <neilb at suse.com>
>> ---
>>  .../lustre/include/linux/libcfs/libcfs_string.h    |   12 -
>>  .../staging/lustre/include/linux/lnet/lib-lnet.h   |   13 -
>>  .../lustre/include/uapi/linux/lnet/libcfs_ioctl.h  |    6 
>>  .../lustre/include/uapi/linux/lnet/lnet-dlc.h      |   57 ++
>>  .../staging/lustre/lnet/klnds/socklnd/socklnd.c    |    3 
>>  drivers/staging/lustre/lnet/lnet/api-ni.c          |  479 +++++++++++++++++---
>>  drivers/staging/lustre/lnet/lnet/config.c          |  107 +++-
>>  drivers/staging/lustre/lnet/lnet/module.c          |   70 ++-
>>  drivers/staging/lustre/lnet/lnet/peer.c            |   21 +
>>  drivers/staging/lustre/lustre/ptlrpc/ptlrpcd.c     |    2 
>>  drivers/staging/lustre/lustre/ptlrpc/service.c     |    4 
>>  11 files changed, 650 insertions(+), 124 deletions(-)
>
> .....
>
>> diff --git a/drivers/staging/lustre/lnet/lnet/module.c b/drivers/staging/lustre/lnet/lnet/module.c
>> index 9d06664f0c17..c82d27592391 100644
>> --- a/drivers/staging/lustre/lnet/lnet/module.c
>> +++ b/drivers/staging/lustre/lnet/lnet/module.c
>> @@ -92,7 +92,7 @@ lnet_unconfigure(void)
>>  }
>>  
>>  static int
>> -lnet_dyn_configure(struct libcfs_ioctl_hdr *hdr)
>> +lnet_dyn_configure_net(struct libcfs_ioctl_hdr *hdr)
>>  {
>>  	struct lnet_ioctl_config_data *conf =
>>  		(struct lnet_ioctl_config_data *)hdr;
>> @@ -102,19 +102,17 @@ lnet_dyn_configure(struct libcfs_ioctl_hdr *hdr)
>>  		return -EINVAL;
>>  
>>  	mutex_lock(&lnet_config_mutex);
>> -	if (!the_lnet.ln_niinit_self) {
>> +	if (the_lnet.ln_niinit_self)
>> +		rc = lnet_dyn_add_net(conf);
>> +	else
>>  		rc = -EINVAL;
>> -		goto out_unlock;
>> -	}
>> -	rc = lnet_dyn_add_ni(LNET_PID_LUSTRE, conf);
>> -out_unlock:
>>  	mutex_unlock(&lnet_config_mutex);
>>  
>>  	return rc;
>>  }
>>  
>>  static int
>> -lnet_dyn_unconfigure(struct libcfs_ioctl_hdr *hdr)
>> +lnet_dyn_unconfigure_net(struct libcfs_ioctl_hdr *hdr)
>>  {
>>  	struct lnet_ioctl_config_data *conf =
>>  		(struct lnet_ioctl_config_data *)hdr;
>> @@ -124,12 +122,50 @@ lnet_dyn_unconfigure(struct libcfs_ioctl_hdr *hdr)
>>  		return -EINVAL;
>>  
>>  	mutex_lock(&lnet_config_mutex);
>> -	if (!the_lnet.ln_niinit_self) {
>> +	if (the_lnet.ln_niinit_self)
>> +		rc = lnet_dyn_del_net(conf->cfg_net);
>> +	else
>> +		rc = -EINVAL;
>> +	mutex_unlock(&lnet_config_mutex);
>> +
>> +	return rc;
>> +}
>> +
>> +static int
>> +lnet_dyn_configure_ni(struct libcfs_ioctl_hdr *hdr)
>> +{
>> +	struct lnet_ioctl_config_ni *conf =
>> +		(struct lnet_ioctl_config_ni *)hdr;
>> +	int rc;
>> +
>> +	if (conf->lic_cfg_hdr.ioc_len < sizeof(*conf))
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&lnet_config_mutex);
>> +	if (the_lnet.ln_niinit_self)
>> +		rc = lnet_dyn_add_ni(conf);
>> +	else
>> +		rc = -EINVAL;
>> +	mutex_unlock(&lnet_config_mutex);
>> +
>> +	return rc;
>> +}
>> +
>> +static int
>> +lnet_dyn_unconfigure_ni(struct libcfs_ioctl_hdr *hdr)
>> +{
>> +	struct lnet_ioctl_config_ni *conf =
>> +		(struct lnet_ioctl_config_ni *)hdr;
>> +	int rc;
>> +
>> +	if (conf->lic_cfg_hdr.ioc_len < sizeof(*conf))
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&lnet_config_mutex);
>> +	if (the_lnet.ln_niinit_self)
>> +		rc = lnet_dyn_del_ni(conf);
>> +	else
>>  		rc = -EINVAL;
>> -		goto out_unlock;
>> -	}
>> -	rc = lnet_dyn_del_ni(conf->cfg_net);
>> -out_unlock:
>>  	mutex_unlock(&lnet_config_mutex);
>>  
>>  	return rc;
>> @@ -161,11 +197,17 @@ lnet_ioctl(struct notifier_block *nb,
>>  		break;
>>  
>>  	case IOC_LIBCFS_ADD_NET:
>> -		rc = lnet_dyn_configure(hdr);
>> +		rc = lnet_dyn_configure_net(hdr);
>>  		break;
>>  
>>  	case IOC_LIBCFS_DEL_NET:
>> -		rc = lnet_dyn_unconfigure(hdr);
>> +		rc = lnet_dyn_unconfigure_net(hdr);
>> +
>> +	case IOC_LIBCFS_ADD_LOCAL_NI:
>> +		return lnet_dyn_configure_ni(hdr);
>> +
>> +	case IOC_LIBCFS_DEL_LOCAL_NI:
>> +		return lnet_dyn_unconfigure_ni(hdr);
>>  		break;
>>  
>>  	default:
>
> While the above is mostly correct for the OpenSFS branch that is
> not the case for the linux client version. The above code assumes
> we are using libc_register_ioctl() but that was replace by the
> block_notifier handling for ioctls instead. What we want instead is
>
>         case IOC_LIBCFS_DEL_NET:
>                 rc = lnet_dyn_unconfigure_net(hdr);
>                 break;
>
>         case IOC_LIBCFS_ADD_LOCAL_NI:
>                 rc = lnet_dyn_configure_ni(hdr);
>                 break;
>
>         case IOC_LIBCFS_DEL_LOCAL_NI:
>                 rc = lnet_dyn_unconfigure_ni(hdr);
>                 break;
>
> that way the return value is generated by notifier_from_ioctl_errno(rc).
> This resolves the last bug in see in this patch set with my testing.

Darn - I should have noticed that !  I've fixed it now.
Thanks a lot,
NeilBrown
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20181002/bb579732/attachment-0001.sig>


More information about the lustre-devel mailing list