Re: [PATCH 13/40] staging: lustre: Dynamic LNet Configuration (DLC) show command
From: Dan Carpenter
Date: Wed Dec 02 2015 - 06:21:41 EST
On Fri, Nov 20, 2015 at 06:35:49PM -0500, James Simmons wrote:
> From: Amir Shehata <amir.shehata@xxxxxxxxx>
>
> This is the fifth patch of a set of patches that enables DLC.
>
> This patch adds the new structures which will be used
> in the IOCTL communication. It also added a set of
> show operations to show buffers, networks, statistics
> and peer information.
>
> Signed-off-by: Amir Shehata <amir.shehata@xxxxxxxxx>
> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-2456
> Change-Id: I96e5cb3dcf07289c6cd1deb46f4acb3c263ae21e
> Reviewed-on: http://review.whamcloud.com/8022
> Reviewed-by: John L. Hammond <john.hammond@xxxxxxxxx>
> Reviewed-by: Doug Oucharek <doug.s.oucharek@xxxxxxxxx>
> Reviewed-by: James Simmons <uja.ornl@xxxxxxxxx>
> Reviewed-by: Oleg Drokin <oleg.drokin@xxxxxxxxx>
> ---
> .../lustre/include/linux/libcfs/libcfs_ioctl.h | 44 +++++++-
> .../staging/lustre/include/linux/lnet/lib-dlc.h | 118 ++++++++++++++++++++
> .../staging/lustre/include/linux/lnet/lib-lnet.h | 5 +
> drivers/staging/lustre/lnet/lnet/api-ni.c | 47 +++++++-
> drivers/staging/lustre/lnet/lnet/module.c | 4 +
> drivers/staging/lustre/lnet/lnet/peer.c | 61 ++++++++++
> .../lustre/lustre/libcfs/linux/linux-module.c | 3 +-
> drivers/staging/lustre/lustre/libcfs/module.c | 15 ++-
> 8 files changed, 282 insertions(+), 15 deletions(-)
> create mode 100644 drivers/staging/lustre/include/linux/lnet/lib-dlc.h
>
> diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_ioctl.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_ioctl.h
> index e14788c..f24330d 100644
> --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_ioctl.h
> +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_ioctl.h
> @@ -41,7 +41,8 @@
> #ifndef __LIBCFS_IOCTL_H__
> #define __LIBCFS_IOCTL_H__
>
> -#define LIBCFS_IOCTL_VERSION 0x0001000a
> +#define LIBCFS_IOCTL_VERSION 0x0001000a
> +#define LIBCFS_IOCTL_VERSION2 0x0001000b
>
> struct libcfs_ioctl_hdr {
> __u32 ioc_len;
> @@ -87,6 +88,13 @@ do { \
> data.ioc_hdr.ioc_len = sizeof(data); \
> } while (0)
>
> +#define LIBCFS_IOC_INIT_V2(data, hdr) \
> +do { \
> + memset(&(data), 0, sizeof(data)); \
> + (data).hdr.ioc_version = LIBCFS_IOCTL_VERSION2; \
> + (data).hdr.ioc_len = sizeof(data); \
> +} while (0)
> +
Do we really need this?
> struct libcfs_ioctl_handler {
> struct list_head item;
> int (*handle_ioctl)(unsigned int cmd, struct libcfs_ioctl_hdr *hdr);
> @@ -112,9 +120,6 @@ struct libcfs_ioctl_handler {
> /* lnet ioctls */
> #define IOC_LIBCFS_GET_NI _IOWR('e', 50, long)
> #define IOC_LIBCFS_FAIL_NID _IOWR('e', 51, long)
> -#define IOC_LIBCFS_ADD_ROUTE _IOWR('e', 52, long)
> -#define IOC_LIBCFS_DEL_ROUTE _IOWR('e', 53, long)
> -#define IOC_LIBCFS_GET_ROUTE _IOWR('e', 54, long)
> #define IOC_LIBCFS_NOTIFY_ROUTER _IOWR('e', 55, long)
> #define IOC_LIBCFS_UNCONFIGURE _IOWR('e', 56, long)
> #define IOC_LIBCFS_PORTALS_COMPATIBILITY _IOWR('e', 57, long)
> @@ -137,7 +142,36 @@ struct libcfs_ioctl_handler {
> #define IOC_LIBCFS_DEL_INTERFACE _IOWR('e', 79, long)
> #define IOC_LIBCFS_GET_INTERFACE _IOWR('e', 80, long)
>
> -#define IOC_LIBCFS_MAX_NR 80
> +/*
> + * DLC Specific IOCTL numbers.
> + * In order to maintain backward compatibility with any possible external
> + * tools which might be accessing the IOCTL numbers, a new group of IOCTL
> + * number have been allocated.
> + */
> +#define IOCTL_CONFIG_SIZE struct lnet_ioctl_config_data
> +#define IOC_LIBCFS_ADD_ROUTE _IOWR(IOC_LIBCFS_TYPE, 81, \
> + IOCTL_CONFIG_SIZE)
> +#define IOC_LIBCFS_DEL_ROUTE _IOWR(IOC_LIBCFS_TYPE, 82, \
> + IOCTL_CONFIG_SIZE)
> +#define IOC_LIBCFS_GET_ROUTE _IOWR(IOC_LIBCFS_TYPE, 83, \
> + IOCTL_CONFIG_SIZE)
> +#define IOC_LIBCFS_ADD_NET _IOWR(IOC_LIBCFS_TYPE, 84, \
> + IOCTL_CONFIG_SIZE)
> +#define IOC_LIBCFS_DEL_NET _IOWR(IOC_LIBCFS_TYPE, 85, \
> + IOCTL_CONFIG_SIZE)
> +#define IOC_LIBCFS_GET_NET _IOWR(IOC_LIBCFS_TYPE, 86, \
> + IOCTL_CONFIG_SIZE)
> +#define IOC_LIBCFS_CONFIG_RTR _IOWR(IOC_LIBCFS_TYPE, 87, \
> + IOCTL_CONFIG_SIZE)
> +#define IOC_LIBCFS_ADD_BUF _IOWR(IOC_LIBCFS_TYPE, 88, \
> + IOCTL_CONFIG_SIZE)
> +#define IOC_LIBCFS_GET_BUF _IOWR(IOC_LIBCFS_TYPE, 89, \
> + IOCTL_CONFIG_SIZE)
> +#define IOC_LIBCFS_GET_PEER_INFO _IOWR(IOC_LIBCFS_TYPE, 90, \
> + IOCTL_CONFIG_SIZE)
> +#define IOC_LIBCFS_GET_LNET_STATS _IOWR(IOC_LIBCFS_TYPE, 91, \
> + IOCTL_CONFIG_SIZE)
> +#define IOC_LIBCFS_MAX_NR 91
Do it like this:
#define IOC_LIBCFS_DEL_ROUTE _IOWR(IOC_LIBCFS_TYPE, 82, IOCTL_CONFIG_SIZE)
#define IOC_LIBCFS_GET_ROUTE _IOWR(IOC_LIBCFS_TYPE, 83, IOCTL_CONFIG_SIZE)
#define IOC_LIBCFS_ADD_NET _IOWR(IOC_LIBCFS_TYPE, 84, IOCTL_CONFIG_SIZE)
#define IOC_LIBCFS_DEL_NET _IOWR(IOC_LIBCFS_TYPE, 85, IOCTL_CONFIG_SIZE)
#define IOC_LIBCFS_GET_NET _IOWR(IOC_LIBCFS_TYPE, 86, IOCTL_CONFIG_SIZE)
#define IOC_LIBCFS_CONFIG_RTR _IOWR(IOC_LIBCFS_TYPE, 87, IOCTL_CONFIG_SIZE)
#define IOC_LIBCFS_ADD_BUF _IOWR(IOC_LIBCFS_TYPE, 88, IOCTL_CONFIG_SIZE)
#define IOC_LIBCFS_GET_BUF _IOWR(IOC_LIBCFS_TYPE, 89, IOCTL_CONFIG_SIZE)
#define IOC_LIBCFS_GET_PEER_INFO _IOWR(IOC_LIBCFS_TYPE, 90, IOCTL_CONFIG_SIZE)
#define IOC_LIBCFS_GET_LNET_STATS _IOWR(IOC_LIBCFS_TYPE, 91, IOCTL_CONFIG_SIZE)
#define IOC_LIBCFS_MAX_NR 91
>
> static inline int libcfs_ioctl_packlen(struct libcfs_ioctl_data *data)
> {
> diff --git a/drivers/staging/lustre/include/linux/lnet/lib-dlc.h b/drivers/staging/lustre/include/linux/lnet/lib-dlc.h
> new file mode 100644
> index 0000000..b6a2e91
> --- /dev/null
> +++ b/drivers/staging/lustre/include/linux/lnet/lib-dlc.h
> @@ -0,0 +1,118 @@
> +/*
> + * GPL HEADER START
> + *
> + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 only,
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License version 2 for more details (a copy is included
> + * in the LICENSE file that accompanied this code).
> + *
> + * You should have received a copy of the GNU General Public License
> + * version 2 along with this program; If not, see
> + * http://www.gnu.org/licenses/gpl-2.0.html
> + *
> + * GPL HEADER END
> + *
> + * Contributers:
> + * Amir Shehata
> + */
> +
> +#ifndef LNET_DLC_H
> +#define LNET_DLC_H
> +
> +#include "../libcfs/libcfs_ioctl.h"
> +#include "types.h"
> +
> +#define MAX_NUM_SHOW_ENTRIES 32
> +#define LNET_MAX_STR_LEN 128
> +#define LNET_MAX_SHOW_NUM_CPT 128
> +
> +struct lnet_ioctl_net_config {
> + char ni_interfaces[LNET_MAX_INTERFACES][LNET_MAX_STR_LEN];
> + __u32 ni_status;
> + __u32 ni_cpts[LNET_MAX_SHOW_NUM_CPT];
> +};
> +
> +#define LNET_TINY_BUF_IDX 0
> +#define LNET_SMALL_BUF_IDX 1
> +#define LNET_LARGE_BUF_IDX 2
> +
> +/* # different router buffer pools */
> +#define LNET_NRBPOOLS (LNET_LARGE_BUF_IDX + 1)
> +
> +struct lnet_ioctl_pool_cfg {
> + struct {
> + __u32 pl_npages;
> + __u32 pl_nbuffers;
> + __u32 pl_credits;
> + __u32 pl_mincredits;
> + } pl_pools[LNET_NRBPOOLS];
> + __u32 pl_routing;
> +};
> +
> +struct lnet_ioctl_config_data {
> + struct libcfs_ioctl_hdr cfg_hdr;
> +
> + __u32 cfg_net;
> + __u32 cfg_count;
> + __u64 cfg_nid;
> + __u32 cfg_ncpts;
> +
> + union {
> + struct {
> + __u32 rtr_hop;
> + __u32 rtr_priority;
> + __u32 rtr_flags;
> + } cfg_route;
> + struct {
> + char net_intf[LNET_MAX_STR_LEN];
> + __s32 net_peer_timeout;
> + __s32 net_peer_tx_credits;
> + __s32 net_peer_rtr_credits;
> + __s32 net_max_tx_credits;
> + __u32 net_cksum_algo;
> + __u32 net_pad;
> + } cfg_net;
> + struct {
> + __u32 buf_enable;
> + __s32 buf_tiny;
> + __s32 buf_small;
> + __s32 buf_large;
> + } cfg_buffers;
> + } cfg_config_u;
> +
> + char cfg_bulk[0];
> +};
> +
> +struct lnet_ioctl_peer {
> + struct libcfs_ioctl_hdr pr_hdr;
> + __u32 pr_count;
> + __u32 pr_pad;
> + __u64 pr_nid;
> +
> + union {
> + struct {
> + char cr_aliveness[LNET_MAX_STR_LEN];
> + __u32 cr_refcount;
> + __u32 cr_ni_peer_tx_credits;
> + __u32 cr_peer_tx_credits;
> + __u32 cr_peer_rtr_credits;
> + __u32 cr_peer_min_rtr_credits;
> + __u32 cr_peer_tx_qnob;
> + __u32 cr_ncpt;
> + } pr_peer_credits;
> + } pr_lnd_u;
> +};
> +
> +struct lnet_ioctl_lnet_stats {
> + struct libcfs_ioctl_hdr st_hdr;
> + struct lnet_counters st_cntrs;
> +};
> +
> +#endif /* LNET_DLC_H */
> diff --git a/drivers/staging/lustre/include/linux/lnet/lib-lnet.h b/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
> index 94d0dc5..f2874e0 100644
> --- a/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
> +++ b/drivers/staging/lustre/include/linux/lnet/lib-lnet.h
> @@ -694,6 +694,11 @@ void lnet_peer_tables_cleanup(lnet_ni_t *ni);
> void lnet_peer_tables_destroy(void);
> int lnet_peer_tables_create(void);
> void lnet_debug_peer(lnet_nid_t nid);
> +int lnet_get_peers(int count, __u64 *nid, char *alivness,
> + int *ncpt, int *refcount,
> + int *ni_peer_tx_credits, int *peer_tx_credits,
> + int *peer_rtr_credits, int *peer_min_rtr_credtis,
> + int *peer_tx_qnob);
>
> static inline void
> lnet_peer_set_alive(lnet_peer_t *lp)
> diff --git a/drivers/staging/lustre/lnet/lnet/api-ni.c b/drivers/staging/lustre/lnet/lnet/api-ni.c
> index 9661f6a..165345c 100644
> --- a/drivers/staging/lustre/lnet/lnet/api-ni.c
> +++ b/drivers/staging/lustre/lnet/lnet/api-ni.c
> @@ -39,6 +39,7 @@
> #include <linux/ktime.h>
>
> #include "../../include/linux/lnet/lib-lnet.h"
> +#include "../../include/linux/lnet/lib-dlc.h"
>
> #define D_LNI D_CONSOLE
>
> @@ -1741,6 +1742,7 @@ int
> LNetCtl(unsigned int cmd, void *arg)
> {
> struct libcfs_ioctl_data *data = arg;
> + struct lnet_ioctl_config_data *config;
> lnet_process_id_t id = {0};
> lnet_ni_t *ni;
> int rc;
> @@ -1765,16 +1767,51 @@ LNetCtl(unsigned int cmd, void *arg)
> return (rc != 0) ? rc : lnet_check_routes();
>
> case IOC_LIBCFS_DEL_ROUTE:
> + config = arg;
I think you need to verify:
if (config->cfg_hdr->ioc_len < sizeof(*config))
return -EINVAL;
> mutex_lock(&the_lnet.ln_api_mutex);
> - rc = lnet_del_route(data->ioc_net, data->ioc_nid);
> + rc = lnet_del_route(config->cfg_net, config->cfg_nid);
> mutex_unlock(&the_lnet.ln_api_mutex);
> return rc;
>
> case IOC_LIBCFS_GET_ROUTE:
> - return lnet_get_route(data->ioc_count,
> - &data->ioc_net, &data->ioc_count,
> - &data->ioc_nid, &data->ioc_flags,
> - &data->ioc_priority);
> + config = arg;
Verify ioc_len.
> + return lnet_get_route(config->cfg_count,
> + &config->cfg_net,
> + &config->cfg_config_u.cfg_route.rtr_hop,
> + &config->cfg_nid,
> + &config->cfg_config_u.cfg_route.rtr_flags,
> + &config->cfg_config_u.cfg_route.
> + rtr_priority);
> +
> + case IOC_LIBCFS_ADD_NET:
> + return 0;
> +
> + case IOC_LIBCFS_DEL_NET:
> + return 0;
> +
> + case IOC_LIBCFS_GET_NET:
> + return 0;
> +
> + case IOC_LIBCFS_GET_LNET_STATS:
> + {
Put this curly brace on the line before.
> + struct lnet_ioctl_lnet_stats *lnet_stats = arg;
> +
Verify ioc_len is large enough.
> + lnet_counters_get(&lnet_stats->st_cntrs);
> + return 0;
> + }
> +
> + case IOC_LIBCFS_CONFIG_RTR:
> + return 0;
> +
> + case IOC_LIBCFS_ADD_BUF:
> + return 0;
> +
> + case IOC_LIBCFS_GET_BUF:
> + return 0;
> +
> + case IOC_LIBCFS_GET_PEER_INFO:
> + return 0;
> +
> case IOC_LIBCFS_NOTIFY_ROUTER:
> secs_passed = (ktime_get_real_seconds() - data->ioc_u64[0]);
> return lnet_notify(NULL, data->ioc_nid, data->ioc_flags,
> diff --git a/drivers/staging/lustre/lnet/lnet/module.c b/drivers/staging/lustre/lnet/lnet/module.c
> index 0afdad0..ffc5700 100644
> --- a/drivers/staging/lustre/lnet/lnet/module.c
> +++ b/drivers/staging/lustre/lnet/lnet/module.c
> @@ -36,6 +36,7 @@
>
> #define DEBUG_SUBSYSTEM S_LNET
> #include "../../include/linux/lnet/lib-lnet.h"
> +#include "../../include/linux/lnet/lib-dlc.h"
>
> static int config_on_load;
> module_param(config_on_load, int, 0444);
> @@ -95,6 +96,9 @@ lnet_ioctl(unsigned int cmd, struct libcfs_ioctl_hdr *hdr)
> case IOC_LIBCFS_UNCONFIGURE:
> return lnet_unconfigure();
>
> + case IOC_LIBCFS_ADD_NET:
> + return LNetCtl(cmd, hdr);
> +
> default:
> /* Passing LNET_PID_ANY only gives me a ref if the net is up
> * already; I'll need it to ensure the net can't go down while
> diff --git a/drivers/staging/lustre/lnet/lnet/peer.c b/drivers/staging/lustre/lnet/lnet/peer.c
> index bb5a0bb..1402e27 100644
> --- a/drivers/staging/lustre/lnet/lnet/peer.c
> +++ b/drivers/staging/lustre/lnet/lnet/peer.c
> @@ -39,6 +39,7 @@
> #define DEBUG_SUBSYSTEM S_LNET
>
> #include "../../include/linux/lnet/lib-lnet.h"
> +#include "../../include/linux/lnet/lib-dlc.h"
>
> int
> lnet_peer_tables_create(void)
> @@ -392,3 +393,63 @@ lnet_debug_peer(lnet_nid_t nid)
>
> lnet_net_unlock(cpt);
> }
> +
> +int lnet_get_peers(int count, __u64 *nid, char *aliveness,
> + int *ncpt, int *refcount,
> + int *ni_peer_tx_credits, int *peer_tx_credits,
> + int *peer_rtr_credits, int *peer_min_rtr_credits,
> + int *peer_tx_qnob)
> +{
> + struct lnet_peer_table *peer_table;
> + lnet_peer_t *lp;
> + int j;
> + int lncpt, found = 0;
> +
> + /* get the number of CPTs */
> + lncpt = cfs_percpt_number(the_lnet.ln_peer_tables);
> +
> + /*
> + * if the cpt number to be examined is >= the number of cpts in
> + * the system then indicate that there are no more cpts to examin
> + */
> + if (*ncpt > lncpt)
> + return -1;
Add some documentation to the start of the function to say what -1 means
here. Or reading below it looks like normal error codes were intended.
> +
> + /* get the current table */
> + peer_table = the_lnet.ln_peer_tables[*ncpt];
> + /* if the ptable is NULL then there are no more cpts to examine */
> + if (!peer_table)
> + return -1;
> +
> + lnet_net_lock(*ncpt);
> +
> + for (j = 0; j < LNET_PEER_HASH_SIZE && !found; j++) {
> + struct list_head *peers = &peer_table->pt_hash[j];
> +
> + list_for_each_entry(lp, peers, lp_hashlist) {
> + if (count-- > 0)
> + continue;
> +
> + snprintf(aliveness, LNET_MAX_STR_LEN, "NA");
> + if (lnet_isrouter(lp) ||
> + lnet_peer_aliveness_enabled(lp))
> + snprintf(aliveness, LNET_MAX_STR_LEN,
> + lp->lp_alive ? "up" : "down");
> +
> + *nid = lp->lp_nid;
> + *refcount = lp->lp_refcount;
> + *ni_peer_tx_credits = lp->lp_ni->ni_peertxcredits;
> + *peer_tx_credits = lp->lp_txcredits;
> + *peer_rtr_credits = lp->lp_rtrcredits;
> + *peer_min_rtr_credits = lp->lp_mintxcredits;
> + *peer_tx_qnob = lp->lp_txqnob;
> +
> + found = 1;
> + }
> + }
> + lnet_net_unlock(*ncpt);
> +
> + *ncpt = lncpt;
> +
> + return found ? 0 : -ENOENT;
> +}
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/