From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0C2DDC61CE4 for ; Sun, 20 Jan 2019 11:28:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C451B2084C for ; Sun, 20 Jan 2019 11:28:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=resnulli-us.20150623.gappssmtp.com header.i=@resnulli-us.20150623.gappssmtp.com header.b="Z7gQYxN+" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730497AbfATL2w (ORCPT ); Sun, 20 Jan 2019 06:28:52 -0500 Received: from mail-wr1-f68.google.com ([209.85.221.68]:46160 "EHLO mail-wr1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730478AbfATL2w (ORCPT ); Sun, 20 Jan 2019 06:28:52 -0500 Received: by mail-wr1-f68.google.com with SMTP id l9so20022574wrt.13 for ; Sun, 20 Jan 2019 03:28:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=resnulli-us.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=iq+JJG2vCFTRZLiclgePGCtgSATXi0l/yaP0K4jJdk8=; b=Z7gQYxN+gIn5/0j5/+eJnwUKNrUSm4NqFGlXBG9b643UwBO5PmIBoULoUVui1jqojK asfuwBR6LvYkOibzn4EwYEywWesQnmVb1oiJjM2byTBNGfHLvqAZOYLJfIh1va6kZ7On DFKdl83u+jIl9yrovTWZaqaFiBEOaE9RuqWJ+bfzirKVznD40RMt4HP2Totbh4VUNBwL D/SHna4wVAlSf3mfe+4DmdBSRmPdnboAUtryjDysM5x/XenNFiCIkgtNXGFn+zJrOWI2 ddE9GbsJ/kOkiHbRwRLKEb0Kyo03gOtWDcENtEWbgZslNvH+BcdEWGpb97pjoSvBNfGC IFZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=iq+JJG2vCFTRZLiclgePGCtgSATXi0l/yaP0K4jJdk8=; b=MPdSbj6C2VazPknFLgl8siDivApkQ3ZVZ49oE9linWC7NLhxVfB1ApqYiEgEWJ8mIT A1W3yz+Mo4vN1RJbRKxbQAHx2RjdpcvLr5S2GaCAFcytiiRe3m70YJgVnJNznRyoGEc/ 9yrx1Jt0kFBJ7IjClrShGcCPyHmrJNHt5lhRDzUMVXPBQx/PydXORe+pFJsqNIX7jnXx ZcFrHHyx2DWO0IggRGMxr0hQa98zgvZK2u66dILXGDvVZIUQ8WSFuwlTnyPYXTH77mAF O6RwLbLrREflwiEbfQMyj2rm/mlLXE1chGkrbwrttL7kCViMWz3UEUD45rb1Lak502hb iLlg== X-Gm-Message-State: AJcUukdCc+tycupWugskVHo9oGG06A3LbQbDV2vXtx8TzXPz2boOjOAJ mn3XPXJX43ef0TGaABfReWQ7Ww== X-Google-Smtp-Source: ALg8bN4p2TEH1yO75h2jrl5e1/5woz4IfSN+ROvwlUJ0WYAR885g5JW0zENx5ct9gSBWL76x3Sw/RQ== X-Received: by 2002:a5d:5089:: with SMTP id a9mr24222475wrt.327.1547983730183; Sun, 20 Jan 2019 03:28:50 -0800 (PST) Received: from localhost ([193.47.165.251]) by smtp.gmail.com with ESMTPSA id m4sm67273971wmi.3.2019.01.20.03.28.49 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sun, 20 Jan 2019 03:28:49 -0800 (PST) Date: Sun, 20 Jan 2019 12:20:06 +0100 From: Jiri Pirko To: Eran Ben Elisha Cc: netdev@vger.kernel.org, Jiri Pirko , "David S. Miller" , Ariel Almog , Aya Levin , Moshe Shemesh Subject: Re: [PATCH net-next v2 01/11] devlink: Add health buffer support Message-ID: <20190120112006.GE2730@nanopsycho> References: <1547762360-7075-1-git-send-email-eranbe@mellanox.com> <1547762360-7075-2-git-send-email-eranbe@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1547762360-7075-2-git-send-email-eranbe@mellanox.com> User-Agent: Mutt/1.9.5 (2018-04-13) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Thu, Jan 17, 2019 at 10:59:10PM CET, eranbe@mellanox.com wrote: >Devlink health buffer is a mechanism to pass descriptors between drivers >and devlink. The API allows the driver to add objects, object pair, >value array (nested attributes), value and name. > >Driver can use this API to fill the buffers in a format which can be >translated by the devlink to the netlink message. > >In order to fulfill it, an internal buffer descriptor is defined. This >will hold the data and metadata per each attribute and by used to pass >actual commands to the netlink. > >This mechanism will be later used in devlink health for dump and diagnose >data store by the drivers. > >Signed-off-by: Eran Ben Elisha >Reviewed-by: Moshe Shemesh >--- > include/net/devlink.h | 76 ++++++ > include/uapi/linux/devlink.h | 8 + > net/core/devlink.c | 501 +++++++++++++++++++++++++++++++++++ > 3 files changed, 585 insertions(+) > [...] >+static int >+devlink_health_buffer_snd(struct genl_info *info, >+ enum devlink_command cmd, int flags, >+ struct devlink_health_buffer **buffers_array, >+ u64 num_of_buffers) >+{ >+ struct sk_buff *skb; >+ struct nlmsghdr *nlh; >+ void *hdr; >+ int err; >+ u64 i; >+ >+ for (i = 0; i < num_of_buffers; i++) { >+ /* Skip buffer if driver did not fill it up with any data */ >+ if (!buffers_array[i]->offset) >+ continue; >+ >+ skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL); >+ if (!skb) >+ return -ENOMEM; >+ >+ hdr = genlmsg_put(skb, info->snd_portid, info->snd_seq, >+ &devlink_nl_family, NLM_F_MULTI, cmd); >+ if (!hdr) >+ goto nla_put_failure; >+ >+ err = devlink_health_buffer_prepare_skb(skb, buffers_array[i]); >+ if (err) >+ goto nla_put_failure; >+ >+ genlmsg_end(skb, hdr); >+ err = genlmsg_reply(skb, info); >+ if (err) >+ return err; >+ } So you have an array of "buffers". I don't see a need for it. Mapping this "buffer" 1:1 to netlink skb leads to lots of skbs for info that could be send in one or a few skbs. The API to the driver should be different. The driver should not care about any "buffer" or size of it (in bytes) or how many of them should be. The driver should just construct a "message". The helpers should be similar to what you have, but the arg should be say "struct devlink_msg". It is not really anything special to "health". It is basically json-like formatted message. So the driver should use the API to open and close objects, to fill values etc. Devlink should take care of allocation of needed buffer/buffers/structs/objects on fly. It could be one linked list of object for all it matters. No byte buffer needed. Then, when devlink needs to send netlink skb, it should take this "struct devlink msg" and translate it to one or many skbs. Basically the whole API to the driver is wrong, I think it would be much easier to revert, redo and reapply. >+ >+ skb = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL); >+ if (!skb) >+ return -ENOMEM; >+ nlh = nlmsg_put(skb, info->snd_portid, info->snd_seq, >+ NLMSG_DONE, 0, flags | NLM_F_MULTI); >+ err = genlmsg_reply(skb, info); >+ if (err) >+ return err; >+ >+ return 0; >+ >+nla_put_failure: >+ err = -EIO; >+ nlmsg_free(skb); >+ return err; >+} >+ > static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = { > [DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING }, > [DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING }, >-- >2.17.1 >