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

From: Or Gerlitz <[email-masked]>
Date: Tue, 21 Apr 2015 16:30:40 +0300

On 4/14/2015 9:51 PM, David Miller wrote:
> From: Amir Vadai <[email-masked]>
> Date: Tue, 14 Apr 2015 11:20:35 +0300
>
>> Signed-off-by: Amir Vadai <[email-masked]>
> 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.