On 6/9/2020 9:40 AM, Ophir Munk wrote: > Hi Ferruh, > Please see inline > >> -----Original Message----- >> From: Ferruh Yigit <ferruh.yi...@intel.com> >> Sent: Monday, June 8, 2020 2:20 PM >> To: Ophir Munk <ophi...@mellanox.com>; dev@dpdk.org; Matan Azrad >> <ma...@mellanox.com>; Raslan Darawsheh <rasl...@mellanox.com> >> Subject: Re: [dpdk-dev] [PATCH v1 2/8] net/mlx5: add mlx5 Linux specific file >> with getter functions >> >> On 6/3/2020 4:05 PM, Ophir Munk wrote: >>> 'ctx' type (field in 'struct mlx5_ctx_shared') is changed from 'struct >>> ibv_context *' to 'void *'. 'ctx' members which are verbs dependent >>> (e.g. device_name) will be accessed through getter functions which are >>> added to a new file under Linux directory: linux/mlx5_os.c. >>> >>> Signed-off-by: Ophir Munk <ophi...@mellanox.com> >>> Acked-by: Matan Azrad <ma...@mellanox.com> >> >> <...> >> >>> @@ -0,0 +1,87 @@ >>> +/* SPDX-License-Identifier: BSD-3-Clause >>> + * Copyright 2015 6WIND S.A. >>> + * Copyright 2020 Mellanox Technologies, Ltd */ >> >> Just to double check if '6WIND' is copy/paste error in this new file? >> > > Some functions were moved from file mlx5.c (with 6WIND copyright) to this file > and renamed. > Should 6WIND copyright be kept or removed in this file (mlx5_os.c)?
No. I just want to confirm this is not just copy/paste error, which happens on new files, but done intentionally. As you did this intentionally, that is good. > >> <...> >> >>> @@ -677,13 +677,14 @@ mlx5_dev_shared_handler_install(struct >> mlx5_dev_ctx_shared *sh) >>> int flags; >>> >>> sh->intr_handle.fd = -1; >>> - flags = fcntl(sh->ctx->async_fd, F_GETFL); >>> - ret = fcntl(sh->ctx->async_fd, F_SETFL, flags | O_NONBLOCK); >>> + flags = fcntl(((struct ibv_context *)sh->ctx)->async_fd, F_GETFL); >>> + ret = fcntl(((struct ibv_context *)sh->ctx)->async_fd, >>> + F_SETFL, flags | O_NONBLOCK); >> >> As far as I understand you are trying to remove to the dependency to ibverbs, >> at least in root level, linux/x.c will have that dependency. (I assume this >> is for >> Windows support) The 'mlx5_os_get_ctx_device_path()' wrapper seems can >> work for it but what is the point of above usage, that you explicitly cast >> "void >> *" to "(struct ibv_context *)", so you still keep the ibv dependency? > > The reason for keeping an explicit cast for async_fd (and not creating a new > getter API) > is that this code snippet will be moved under linux in next commits where no > getter function is needed. > I wanted to avoid adding a getter function here and then remove it in a > follow up commit. > Make sense if it is removed later, thanks for clarification.
