From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 3F1CE4F9B for ; Thu, 13 Sep 2018 11:23:26 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Sep 2018 02:23:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,368,1531810800"; d="scan'208";a="83151390" Received: from bricha3-mobl.ger.corp.intel.com ([10.237.221.107]) by orsmga003.jf.intel.com with SMTP; 13 Sep 2018 02:22:07 -0700 Received: by (sSMTP sendmail emulation); Thu, 13 Sep 2018 10:22:07 +0100 Date: Thu, 13 Sep 2018 10:22:06 +0100 From: Bruce Richardson To: Shahaf Shuler Cc: nelio.laranjeiro@6wind.com, yskoh@mellanox.com, matan@mellanox.com, bluca@debian.org, dev@dpdk.org Message-ID: <20180913092206.GA17576@bricha3-MOBL.ger.corp.intel.com> References: <9c646fc088242e679a3e4fda541f58674b064b1e.1535699721.git.nelio.laranjeiro@6wind.com> <20180905114746.117936-1-shahafs@mellanox.com> <20180907103429.GA12540@bricha3-MOBL.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180907103429.GA12540@bricha3-MOBL.ger.corp.intel.com> Organization: Intel Research and Development Ireland Ltd. User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH v5] net/mlx: add meson build support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Sep 2018 09:23:27 -0000 On Fri, Sep 07, 2018 at 11:34:29AM +0100, Bruce Richardson wrote: > On Wed, Sep 05, 2018 at 02:47:46PM +0300, Shahaf Shuler wrote: > > From: Nelio Laranjeiro > > > > Compile Mellanox drivers when their external dependencies are met. A > > glue version of the driver can still be requested by using the > > -Denable_driver_mlx_glue=true > > > > To avoid modifying the whole sources and keep the compatibility with > > current build systems (e.g. make), the mlx{4,5}_autoconf.h is still > > generated. > > > > Meson will try to find the required external libraries. When they are > > not installed system wide, they can be provided though CFLAGS, LDFLAGS > > and LD_LIBRARY_PATH environment variables, example (considering > > RDMA-Core is installed in /tmp/rdma-core): > > > > # CLFAGS=-I/tmp/rdma-core/build/include \ > > LDFLAGS=-L/tmp/rdma-core/build/lib \ > > LD_LIBRARY_PATH=/tmp/rdma-core/build/lib \ > > meson output > > # LD_LIBRARY_PATH=/tmp/rdma-core/build/lib \ > > ninja -C output install > > > > Note: LD_LIBRARY_PATH before ninja is necessary when the meson > > configuration has changed (e.g. meson configure has been called), in > > such situation the LD_LIBRARY_PATH is necessary to invoke the > > autoconfiguration script. > > > > Signed-off-by: Nelio Laranjeiro > > Signed-off-by: Shahaf Shuler > > Acked-by: Bruce Richardson > > --- > > Changes in v5: > > > > - use meson tool to generate Mellanox config file instead of DPDK custom scripts. > > > > > > > + # To maintain the compatibility with the make build system > > + # mlx5_autoconf.h file is still generated. > > + # input array for meson symbol search: > > + # [ "MACRO to define if found", "header for the search", > > + # "type/enum/define", "symbol to search", > > + # "struct member to search (type only)] > > + # > > + args = [ > > + [ 'HAVE_IBV_DEVICE_STRIDING_RQ_SUPPORT', 'infiniband/mlx5dv.h', > > + 'enum', 'MLX5DV_CQE_RES_FORMAT_CSUM_STRIDX' ], > > + [ 'HAVE_IBV_DEVICE_TUNNEL_SUPPORT', 'infiniband/mlx5dv.h', > > + 'enum', 'MLX5DV_CONTEXT_MASK_TUNNEL_OFFLOADS' ], > > + [ 'HAVE_IBV_MLX5_MOD_MPW', 'infiniband/mlx5dv.h', > > + 'enum', 'MLX5DV_CONTEXT_FLAGS_MPW_ALLOWED' ], > > + [ 'HAVE_IBV_MLX5_MOD_SWP', 'infiniband/mlx5dv.h', > > + 'type', 'struct mlx5dv_sw_parsing_caps', > > + 'sw_parsing_offloads' ], > > + [ 'HAVE_IBV_MLX5_MOD_CQE_128B_COMP', 'infiniband/mlx5dv.h', > > + 'enum', 'MLX5DV_CONTEXT_FLAGS_CQE_128B_COMP' ], > > + [ 'HAVE_IBV_DEVICE_MPLS_SUPPORT', 'infiniband/verbs.h', > > + 'enum', 'IBV_FLOW_SPEC_MPLS' ], > > + [ 'HAVE_IBV_WQ_FLAG_RX_END_PADDING', 'infiniband/verbs.h', > > + 'enum', 'IBV_WQ_FLAG_RX_END_PADDING' ], > > + [ 'HAVE_IBV_DEVICE_COUNTERS_SET_SUPPORT', 'infiniband/verbs.h', > > + 'type', 'struct ibv_counter_set_init_attr', > > + 'counter_set_id' ], > > + [ 'HAVE_SUPPORTED_40000baseKR4_Full', 'linux/ethtool.h', > > + 'define', 'SUPPORTED_40000baseKR4_Full' ], > > + [ 'HAVE_SUPPORTED_40000baseCR4_Full', 'linux/ethtool.h', > > + 'define', 'SUPPORTED_40000baseCR4_Full' ], > > + [ 'HAVE_SUPPORTED_40000baseSR4_Full', 'linux/ethtool.h', > > + 'define', 'SUPPORTED_40000baseSR4_Full' ], > > + [ 'HAVE_SUPPORTED_40000baseLR4_Full', 'linux/ethtool.h', > > + 'define', 'SUPPORTED_40000baseLR4_Full' ], > > + [ 'HAVE_SUPPORTED_56000baseKR4_Full', 'linux/ethtool.h', > > + 'define', 'SUPPORTED_56000baseKR4_Full' ], > > + [ 'HAVE_SUPPORTED_56000baseCR4_Full', 'linux/ethtool.h', > > + 'define', 'SUPPORTED_56000baseCR4_Full' ], > > + [ 'HAVE_SUPPORTED_56000baseSR4_Full', 'linux/ethtool.h', > > + 'define', 'SUPPORTED_56000baseSR4_Full' ], > > + [ 'HAVE_SUPPORTED_56000baseLR4_Full', 'linux/ethtool.h', > > + 'define', 'SUPPORTED_56000baseLR4_Full' ], > > + [ 'HAVE_ETHTOOL_LINK_MODE_25G', 'linux/ethtool.h', > > + 'enum', 'ETHTOOL_LINK_MODE_25000baseCR_Full_BIT' ], > > + [ 'HAVE_ETHTOOL_LINK_MODE_50G', 'linux/ethtool.h', > > + 'enum', 'ETHTOOL_LINK_MODE_50000baseCR2_Full_BIT' ], > > + [ 'HAVE_ETHTOOL_LINK_MODE_100G', 'linux/ethtool.h', > > + 'enum', 'ETHTOOL_LINK_MODE_100000baseKR4_Full_BIT' ], > > + [ 'HAVE_IFLA_PHYS_SWITCH_ID', 'linux/if_link.h', > > + 'enum', 'IFLA_PHYS_SWITCH_ID' ], > > + [ 'HAVE_IFLA_PHYS_PORT_NAME', 'linux/if_link.h', > > + 'enum', 'IFLA_PHYS_PORT_NAME' ], > > + [ 'HAVE_TCA_FLOWER_ACT', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_ACT' ], > > + [ 'HAVE_TCA_FLOWER_FLAGS', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_FLAGS' ], > > + [ 'HAVE_TCA_FLOWER_KEY_ETH_TYPE', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_ETH_TYPE' ], > > + [ 'HAVE_TCA_FLOWER_KEY_ETH_DST', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_ETH_DST' ], > > + [ 'HAVE_TCA_FLOWER_KEY_ETH_DST_MASK', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_ETH_DST_MASK' ], > > + [ 'HAVE_TCA_FLOWER_KEY_ETH_SRC', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_ETH_SRC' ], > > + [ 'HAVE_TCA_FLOWER_KEY_ETH_SRC_MASK', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_ETH_SRC_MASK' ], > > + [ 'HAVE_TCA_FLOWER_KEY_IP_PROTO', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_IP_PROTO' ], > > + [ 'HAVE_TCA_FLOWER_KEY_IPV4_SRC', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_IPV4_SRC' ], > > + [ 'HAVE_TCA_FLOWER_KEY_IPV4_SRC_MASK', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_IPV4_SRC_MASK' ], > > + [ 'HAVE_TCA_FLOWER_KEY_IPV4_DST', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_IPV4_DST' ], > > + [ 'HAVE_TCA_FLOWER_KEY_IPV4_DST_MASK', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_IPV4_DST_MASK' ], > > + [ 'HAVE_TCA_FLOWER_KEY_IPV6_SRC', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_IPV6_SRC' ], > > + [ 'HAVE_TCA_FLOWER_KEY_IPV6_SRC_MASK', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_IPV6_SRC_MASK' ], > > + [ 'HAVE_TCA_FLOWER_KEY_IPV6_DST', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_IPV6_DST' ], > > + [ 'HAVE_TCA_FLOWER_KEY_IPV6_DST_MASK', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_IPV6_DST_MASK' ], > > + [ 'HAVE_TCA_FLOWER_KEY_TCP_SRC', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_TCP_SRC' ], > > + [ 'HAVE_TCA_FLOWER_KEY_TCP_SRC_MASK', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_TCP_SRC_MASK' ], > > + [ 'HAVE_TCA_FLOWER_KEY_TCP_DST', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_TCP_DST' ], > > + [ 'HAVE_TCA_FLOWER_KEY_TCP_DST_MASK', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_TCP_DST_MASK' ], > > + [ 'HAVE_TCA_FLOWER_KEY_UDP_SRC', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_UDP_SRC' ], > > + [ 'HAVE_TCA_FLOWER_KEY_UDP_SRC_MASK', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_UDP_SRC_MASK' ], > > + [ 'HAVE_TCA_FLOWER_KEY_UDP_DST', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_UDP_DST' ], > > + [ 'HAVE_TCA_FLOWER_KEY_UDP_DST_MASK', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_UDP_DST_MASK' ], > > + [ 'HAVE_TCA_FLOWER_KEY_VLAN_ID', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_VLAN_ID' ], > > + [ 'HAVE_TCA_FLOWER_KEY_VLAN_PRIO', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_VLAN_PRIO' ], > > + [ 'HAVE_TCA_FLOWER_KEY_VLAN_ETH_TYPE', 'linux/pkt_cls.h', > > + 'enum', 'TCA_FLOWER_KEY_VLAN_ETH_TYPE' ], > > + [ 'HAVE_TC_ACT_VLAN', 'linux/tc_act/tc_vlan.h', 'enum', > > + 'TCA_VLAN_PUSH_VLAN_PRIORITY' ], > > + [ 'HAVE_RDMA_NL_NLDEV', 'rdma/rdma_netlink.h', 'enum', > > + 'RDMA_NL_NLDEV' ], > > + [ 'HAVE_RDMA_NLDEV_CMD_GET', 'rdma/rdma_netlink.h', 'enum', > > + 'RDMA_NLDEV_CMD_GET' ], > > + [ 'HAVE_RDMA_NLDEV_CMD_PORT_GET', 'rdma/rdma_netlink.h', > > + 'enum', 'RDMA_NLDEV_CMD_PORT_GET' ], > > + [ 'HAVE_RDMA_NLDEV_ATTR_DEV_INDEX', 'rdma/rdma_netlink.h', > > + 'enum', 'RDMA_NLDEV_ATTR_DEV_INDEX' ], > > + [ 'HAVE_RDMA_NLDEV_ATTR_DEV_NAME', 'rdma/rdma_netlink.h', > > + 'enum', 'RDMA_NLDEV_ATTR_DEV_NAME' ], > > + [ 'HAVE_RDMA_NLDEV_ATTR_PORT_INDEX', 'rdma/rdma_netlink.h', > > + 'enum', 'RDMA_NLDEV_ATTR_PORT_INDEX' ], > > + [ 'HAVE_RDMA_NLDEV_ATTR_NDEV_INDEX', 'rdma/rdma_netlink.h', > > + 'enum', 'RDMA_NLDEV_ATTR_NDEV_INDEX' ], > > + ] > > + run_command('rm', '-f', meson.current_build_dir() + '/mlx5_autoconf.h') > > + config = configuration_data() > > + foreach arg:args > > + if arg[2] == 'type' > > + file_prefix = '#include<' + arg[1] + '>' > > + has = cc.has_member(arg[3], arg[4], prefix : file_prefix) > > + else > > + has = cc.has_header_symbol(arg[1], arg[3]) > > + endif > > + if has > > + config.set(arg[0], 1) > > + endif > > + endforeach > > + configure_file(output : 'mlx5_autoconf.h', configuration : config) > > +endif > > couple of minor points: > * These new additions use tab rather than space for indent. The standard > with meson I think is normally spaces, but I (inadvertently) added all > other DPDK meson.build files with tabs, so for consistency I think it > would be good if the whole file used tabs i.e. change all the other lines > apart from your new ones in v5. > > * is the "rm" command really necessary? I would expect the configure_file() > function to just do the right thing here when replacing the file. > > * Would you consider splitting the args array into two arrays, one for the > "type" values and another for the enum/define ones? This would mean > having two loops, but the loops themselves would be clearer and shorter - > as would the arrays. Especially since according to > http://mesonbuild.com/Configuration.html > a boolean false converts to an "undef", rather than a 0/1 value. > For example: > > foreach arg:has_sym_args > config.set(arg[0], cc.has_header_symbol(arg[1], arg[3])) > endforeach > > * Very minor suggestion: is it neater to use "args: '-include ' + arg[1]", > instead of using prefix with the #include syntax? [Assuming the former > works] > Any plans for a new version of this? If so, can I also suggest splitting the patch into two, one for mlx4 and the other for mlx5. Regards, /Bruce