On Tue, 24 Nov 2020 18:50:42 +0100 Karsten Graul wrote:
> @@ -214,6 +217,67 @@ static void smc_lgr_unregister_conn(struct smc_connection *conn)
> conn->lgr = NULL;
> }
>
> +int smc_nl_get_sys_info(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> + struct smc_nl_dmp_ctx *cb_ctx = smc_nl_dmp_ctx(cb);
> + char hostname[SMC_MAX_HOSTNAME_LEN + 1];
> + int snum = cb_ctx->pos[0], num = 0;
> + char smc_seid[SMC_MAX_EID_LEN + 1];
> + struct smcd_dev *smcd_dev;
> + struct nlattr *attrs;
> + u8 *seid = NULL;
> + u8 *host = NULL;
> + void *nlh;
> +
> + nlh = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
> + &smc_gen_nl_family, NLM_F_MULTI,
> + SMC_NETLINK_GET_SYS_INFO);
> + if (!nlh)
> + goto errout;
> + if (snum > num)
can just say if (snum) or if (cb_ctx->pos[0])
> + goto errout;
> + attrs = nla_nest_start_noflag(skb, SMC_GEN_SYS_INFO);
> + if (!attrs)
> + goto errout;
> + if (nla_put_u8(skb, SMC_NLA_SYS_VER, SMC_V2) < 0)
> + goto errattr;
> + if (nla_put_u8(skb, SMC_NLA_SYS_REL, SMC_RELEASE) < 0)
> + goto errattr;
> + if (nla_put_u8(skb, SMC_NLA_SYS_IS_ISM_V2,
> + smc_ism_is_v2_capable()) < 0)
> + goto errattr;
> + smc_clc_get_hostname(&host);
> + if (host) {
> + memset(hostname, 0, sizeof(hostname));
This memset looks unnecessary.
> + snprintf(hostname, sizeof(hostname), "%s", host);
> + if (nla_put_string(skb, SMC_NLA_SYS_LOCAL_HOST, hostname) < 0)
> + goto errattr;
> + }
> + mutex_lock(&smcd_dev_list.mutex);
> + smcd_dev = list_first_entry_or_null(&smcd_dev_list.list,
> + struct smcd_dev, list);
> + if (smcd_dev)
> + smc_ism_get_system_eid(smcd_dev, &seid);
> + mutex_unlock(&smcd_dev_list.mutex);
> + if (seid && smc_ism_is_v2_capable()) {
> + memset(smc_seid, 0, sizeof(smc_seid));
ditto
> + snprintf(smc_seid, sizeof(smc_seid), "%s", seid);
> + if (nla_put_string(skb, SMC_NLA_SYS_SEID, smc_seid) < 0)
> + goto errattr;
> + }
> + nla_nest_end(skb, attrs);
> + genlmsg_end(skb, nlh);
> + num++;
> + cb_ctx->pos[0] = num;
and set this to 1, num seems pointless (I don't see this function
extended in the next patches, maybe I missed it).
> + return skb->len;
> +
> +errattr:
> + nla_nest_cancel(skb, attrs);
> +errout:
> + genlmsg_cancel(skb, nlh);
> + return skb->len;
> +}
> static const struct nla_policy smc_gen_nl_policy[SMC_GEN_MAX + 1] = {
> [SMC_GEN_UNSPEC] = { .type = NLA_UNSPEC, },
> + [SMC_GEN_SYS_INFO] = { .type = NLA_NESTED, },
You never use this as an input attribute, you don't have to add it to
policy. Policy is for input validation.
> };
>
> +static int smc_nl_start(struct netlink_callback *cb)
> +{
> + struct smc_nl_dmp_ctx *cb_ctx = smc_nl_dmp_ctx(cb);
> +
> + cb_ctx->pos[0] = 0;
IIRC context is memset to 0 at the start, no need for this.
> + return 0;
> +}
