Re: [PATCH net-next v2 11/15] net/smc: Add SMC-D Linkgroup diagnostic support

Saeed Mahameed
2020-11-04T01:26:09Z

On Tue, 2020-11-03 at 11:25 +0100, Karsten Graul wrote:
> From: Guvenc Gulce <guvenc@linux.ibm.com>
>
> Deliver SMCD Linkgroup information via netlink based
> diagnostic interface.
>
> Signed-off-by: Guvenc Gulce <guvenc@linux.ibm.com>
> Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
> ---
> include/uapi/linux/smc_diag.h | 7 +++
> net/smc/smc_diag.c | 108
> ++++++++++++++++++++++++++++++++++
> net/smc/smc_ism.c | 2 +
> 3 files changed, 117 insertions(+)
>
> diff --git a/include/uapi/linux/smc_diag.h
> b/include/uapi/linux/smc_diag.h
> index a57df0296aa4..5a80172df757 100644
> --- a/include/uapi/linux/smc_diag.h
> +++ b/include/uapi/linux/smc_diag.h
> @@ -81,6 +81,7 @@ enum {
> enum {
> SMC_DIAG_LGR_INFO_SMCR = 1,
> SMC_DIAG_LGR_INFO_SMCR_LINK,
> + SMC_DIAG_LGR_INFO_SMCD,
> };
>
>
> +
> +static int smc_diag_fill_smcd_dev(struct smcd_dev_list *dev_list,
> + struct sk_buff *skb,
> + struct netlink_callback *cb,
> + struct smc_diag_req_v2 *req)
> +{
> + struct smc_diag_dump_ctx *cb_ctx = smc_dump_context(cb);
> + struct smcd_dev *smcd_dev;
> + int snum = cb_ctx->pos[0];
> + int rc = 0, num = 0;
> +
> + mutex_lock(&dev_list->mutex);
> + list_for_each_entry(smcd_dev, &dev_list->list, list) {
> + if (!list_empty(&smcd_dev->lgr_list)) {

You could use early continue every where in this patch to avoid
indentation.

> + if (num < snum)
> + goto next;
> + rc = smc_diag_handle_smcd_lgr(smcd_dev, skb,
> + cb, req);
> + if (rc < 0)
> + goto errout;
> +next:
> + num++;
> + }
> + }
> +errout:
> + mutex_unlock(&dev_list->mutex);
> + cb_ctx->pos[0] = num;
> + return rc;
> +}
> +
> static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
> struct netlink_callback *cb,
> const struct smc_diag_req *req)
> @@ -441,6 +546,9 @@ static int smc_diag_dump_ext(struct sk_buff *skb,
> struct netlink_callback *cb)
> if ((req->cmd_ext & (1 << (SMC_DIAG_LGR_INFO_SMCR -
> 1))))
> smc_diag_fill_lgr_list(&smc_lgr_list, skb, cb,
> req);
> + if ((req->cmd_ext & (1 << (SMC_DIAG_LGR_INFO_SMCD -
> 1))))
> + smc_diag_fill_smcd_dev(&smcd_dev_list, skb, cb,
> + req);
> }
>
> return skb->len;
> diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
> index 6abbdd09a580..5bb2c7fb4ea8 100644
> --- a/net/smc/smc_ism.c
> +++ b/net/smc/smc_ism.c
> @@ -20,6 +20,7 @@ struct smcd_dev_list smcd_dev_list = {
> .list = LIST_HEAD_INIT(smcd_dev_list.list),
> .mutex = __MUTEX_INITIALIZER(smcd_dev_list.mutex)
> };
> +EXPORT_SYMBOL_GPL(smcd_dev_list);
>
> bool smc_ism_v2_capable;
>
> @@ -50,6 +51,7 @@ u16 smc_ism_get_chid(struct smcd_dev *smcd)
> {
> return smcd->ops->get_chid(smcd);
> }
> +EXPORT_SYMBOL_GPL(smc_ism_get_chid);
>

This is the 3rd EXPORT SYMBOL until now in this series,
IMHO it is unhealthy to contaminate the kernel symbol table just for
device specific diag purposes.