Re: [PATCH for-next V5 00/12] Move RoCE GID management to IB/Core

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 6/9/2015 12:37 AM, Hefty, Sean wrote:
Previously, every vendor implemented its net device notifiers in its own
driver. This introduces a huge code duplication as figuring


  28 files changed, 2253 insertions(+), 860 deletions(-)

How does adding 1400 lines of code help reduce code duplication?

Can you please explain and justify why this change is actually needed?


Let's look at this change from several perspectives:

(1) Each vedor lost ~250 lines of GID management code just by this change. In the future it's very probable that more vendor drivers will implement RoCE. This removes the burden and code duplication required by them to implement a full RoCE support and is a lot more scalable than the current approach.

(2) All vendors are now aligned. For example, mlx4 driver had bonding support but ocrdma didn't have such support. The user expects the same behavior regardless the vendor's driver.

(3) When making something more general it usually requires more lines of code as it introduces API and doesn't cut corners assuming anything on the vendor's driver.

(4) This is a per-requisite to the RoCE V2 series. I'm sure you remember we first submitted this patch-set as a part of the RoCE V2 series. Adding more features to the RoCE GID management will make the code duplication a lot worse than just ~250 lines. I don't think it's fair playing "lets divide the RoCE V2 patch-set to several patch-sets" and then say "why do we need this <first part> at all". Let alone, the other there reasons are more than enough IMHO.

Matan
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux