netdev - Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZOPZLhQmec_mgnCTbxOmxkgwu35Rp3gQm2w-ZWEJxoojY8iA@mail.gmail.com>
Date:	Wed, 3 Jul 2013 00:15:19 +0300
From:	Or Gerlitz <or.gerlitz@...il.com>
To:	Roland Dreier <roland@...nel.org>
Cc:	Eli Cohen <eli@....mellanox.co.il>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	Eli Cohen <eli@...lanox.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Jack Morgenstein <jackm@....mellanox.co.il>
Subject: Re: [PATCH for-next 0/8] Add Mellanox mlx5 driver for Connect-IB devices

On Mon, Jul 1, 2013 at 9:03 PM, Roland Dreier <roland@...nel.org> wrote:
> In general I don't think overriding the CFLAGS (as you do in the mlx5
> Makefiles) is a good idea, and in particular here your -Wall -Werror
> break the build, at least for my gcc 4.7.3:
>
>   CC      drivers/infiniband/hw/mlx5/qp.o
> /home/roland/Src/linux-merge.git/drivers/infiniband/hw/mlx5/qp.c: In
> function ‘sq_overhead’:
> /home/roland/Src/linux-merge.git/drivers/infiniband/hw/mlx5/qp.c:234:2:
> error: case value ‘4671’ not in enumerated type ‘enum ib_qp_type’
> [-Werror=switch]


Will do both (A) remove the flags added on the driver makefile and (B)
fix the issues pointed by these flags...

[...]

> What is this IB_QPT_REG_UMR stuff anyway?  Shouldn't we strip out all
> that from the mlx5 driver until it's available in the core code?

IB_QPT_REG_UMR is the type of QP used internally by the driver, to do
plain memory registration by verbs consumers. Will apply here a
similar practice to the one done by mlx4 driver to create the proxy
and tunnel QP types  for SRIOV, e.g will define MLX5_IB_QPT_REG_UMR
and use that under driver specific QP creation flags for which we have
the foundations in the IB verbs header file to go and use.

[...]

> /* ===> this should be passed to the vergbs layer */
> enum {
>         IB_WR_SET_PSV = IB_WR_BIND_MW + 10,
>         IB_WR_GET_PSV,
>         IB_WR_CHECK_PSV,
>         IB_WR_RGET_PSV,
>         IB_WR_RCHECK_PSV,
>         IB_WR_UMR,
> };
>
> enum {
>         IB_SEND_UMR_UNREG       = IB_SEND_IP_CSUM << 1,
> };
>
> enum ib_latency_class {
>         IB_LATENCY_CLASS_LOW,
>         IB_LATENCY_CLASS_MEDIUM,
>         IB_LATENCY_CLASS_HIGH,
>         IB_LATENCY_CLASS_FAST_PATH
> };
> /* <=== this should be passed to the vergbs layer */
>
> looks like it shouldn't be in your submission.  (What are "vergbs" anyway? :)

Will fix that, basically, will remove things we can get along for now,
e.g unused, even not internally such as IB_WR_YYY_PSV, and internalize
what we do need internally e.g use MLX5_IB_XXX where IB_XXX was used

and "vergbs" is a typo whose fix missed the version submitted...
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html