netdev - Re: [PATCH net-next V2 12/12] net/mlx5: Ethernet driver

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] [day] [month] [year] [list]

Message-ID: <55365100.4060208@mellanox.com>
Date:	Tue, 21 Apr 2015 16:30:40 +0300
From:	Or Gerlitz <ogerlitz@...lanox.com>
To:	<amirv@...lanox.com>
CC:	David Miller <davem@...emloft.net>, <netdev@...r.kernel.org>,
	<yevgenyp@...lanox.com>, <saeedm@...lanox.com>,
	<achiad@...lanox.com>, <idos@...lanox.com>, <talal@...lanox.com>
Subject: Re: [PATCH net-next V2 12/12] net/mlx5: Ethernet driver

On 4/14/2015 9:51 PM, David Miller wrote:
> From: Amir Vadai <amirv@...lanox.com>
> Date: Tue, 14 Apr 2015 11:20:35 +0300
>
>> Signed-off-by: Amir Vadai <amirv@...lanox.com>
> What does "Ethernet driver" mean?
>
> Are you adding a new ethernet driver?  If so, what is it for and how
> does it interact with the existing mlx5 driver?
>
> It looks to me like you are adding a lot of code and objects to the
> existing mlx5 module.  An incredible amount, in fact.  This seems very
> suboptimal especially for users of the existing mlx5 chips.

Hi Dave,

To clarify things a bit, the mlx5 driver serves two NICs families: 
ConnectIB (device IDs 0x1011/12) and ConnectX4 (device IDs 1013-1016).

ConnectIB HW is IB only, ConnextX4 is IB and Ethernet.

This submission enhances the driver to support Ethernet for the 
ConnectX4 family.

In a similar manner to the mlx4 driver stacking, mlx5 has has IB driver 
and core driver. Per your comments, in V3, the Ethernet functionality 
would be added per dedicated config directive, such that if someone 
wants only the IB driver to be functional, the Ethernet netdev code and 
such doesn't get built.

> You haven't discussed this, what design decisions made you decide in the end to do it this way, etc.
>
> You absolutely have to say something other than "Ethernet driver"
> in this commit message, I expect several paragraphs of details
> and the hows and whys of the change as it is a non-trivial amount
> of code being added here.

Understood, will add more text to the cover letter, change-logs, etc.

> I still consider this patch series not ready yet, and the merge
> window is open thus closing the net-next tree.
>
> You will therefore need to wait until the net-next tree opens
> again before submitting this series again.

Sure, thanks for the review feedback provided so far.

BTW Sorry for the late reply, just realized today there has been no 
response on your comments.

Or.
--
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