All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
To: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Matan Barak <matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>,
	Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>,
	"Hefty,
	Sean" <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	Moni Shoua <monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>,
	Somnath Kotur
	<Somnath.Kotur-xCNTGe7nOvUAvxtiuMwx3w@public.gmane.org>,
	"linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: [PATCH for-next V5 00/12] Move RoCE GID management to IB/Core
Date: Wed, 10 Jun 2015 23:38:23 -0600	[thread overview]
Message-ID: <20150611053823.GA22369@obsidianresearch.com> (raw)
In-Reply-To: <1433998199.71666.144.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

On Thu, Jun 11, 2015 at 12:49:59AM -0400, Doug Ledford wrote:

> fact that the mlx4 driver and the ocrdma driver had their own gid
> management code, there were some distinct differences between the two.
> The gid at index 0 never matched up in my testing for example.  One
> supported bonding, the other didn't.  Even if you tried to limit things
> to a cleanup, you would still end up altering behavior of both drivers
> just because the cleanup would have to merge the two implementations and
> the result would be different than either one of them.

This is very interesting detail, I would be very happy to see the user
visible functional changes to ocrdma listed in it's commit message.

I always understood that ocrdma would have some change, at least from
bonding, my 'no functional change' litmus test was that mlx4 operation
doesn't have a change. My expectation is the current v5 achieves that?

> I get the impression that a lot of the extra is scalability changes for
> perceived need (probably due to upcoming namespace/container additions).
> But, just to be fair, the entire mlx4 gid code was -500 lines and
> included bonding support.  The base gid code addition was +500 for the
> gid table, +600 for the table population functions, +170 for default
> gids, +290 for bonding support, and +140 lines to hook it into the
> existing cache routines.  While part of this might be would seem to be
> the over-designed locking, part of it is probably exactly as Matan said,
> code growth due to generalization and standardization.

Line count is such a funny metric.. It is very, very hard to write
succinct code, and I have no magic wand to shrink this particular
patch set down further. I suspect it could be made smaller, but that
would require more study than I care to employ :)

I feel line count is a hopeless avenue to argue, but also a red flag.

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

  parent reply	other threads:[~2015-06-11  5:38 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-08 14:12 [PATCH for-next V5 00/12] Move RoCE GID management to IB/Core Matan Barak
     [not found] ` <1433772735-22416-1-git-send-email-matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2015-06-08 14:12   ` [PATCH for-next V5 01/12] IB/core: Add RoCE GID table Matan Barak
2015-06-08 14:12   ` [PATCH for-next V5 02/12] IB/core: Add rwsem to allow reading device list or client list Matan Barak
2015-06-08 14:12   ` [PATCH for-next V5 03/12] IB/core: Add RoCE GID population Matan Barak
     [not found]     ` <1433772735-22416-4-git-send-email-matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2015-06-11  4:18       ` Jason Gunthorpe
2015-06-08 14:12   ` [PATCH for-next V5 04/12] net/ipv6: Export addrconf_ifid_eui48 Matan Barak
2015-06-08 14:12   ` [PATCH for-next V5 05/12] IB/core: Add default GID for RoCE GID table Matan Barak
     [not found]     ` <1433772735-22416-6-git-send-email-matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2015-06-11  6:20       ` Jason Gunthorpe
     [not found]         ` <20150611062017.GC22369-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2015-06-11 15:30           ` Matan Barak
2015-06-08 14:12   ` [PATCH for-next V5 06/12] net: Add info for NETDEV_CHANGEUPPER event Matan Barak
2015-06-08 14:12   ` [PATCH for-next V5 07/12] IB/core: Add RoCE table bonding support Matan Barak
     [not found]     ` <1433772735-22416-8-git-send-email-matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2015-06-11  6:18       ` Jason Gunthorpe
     [not found]         ` <20150611061818.GB22369-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2015-06-11 16:00           ` Matan Barak
2015-06-08 14:12   ` [PATCH for-next V5 08/12] IB/core: ib_cache routines should use roce_gid_table when needed Matan Barak
2015-06-08 14:12   ` [PATCH for-next V5 09/12] net/mlx4: Postpone the registration of net_device Matan Barak
2015-06-08 14:12   ` [PATCH for-next V5 10/12] IB/mlx4: Implement ib_device callbacks Matan Barak
     [not found]     ` <1433772735-22416-11-git-send-email-matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2015-06-11  6:31       ` Jason Gunthorpe
     [not found]         ` <20150611063108.GE22369-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2015-06-11  6:53           ` Moni Shoua
2015-06-08 14:12   ` [PATCH for-next V5 11/12] IB/mlx4: Replace mechanism for RoCE GID management Matan Barak
2015-06-08 14:12   ` [PATCH for-next V5 12/12] RDMA/ocrdma: Changes in driver to incorporate the moving of GID Table mgmt to IB/Core Matan Barak
     [not found]     ` <1433772735-22416-13-git-send-email-matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2015-06-11  4:11       ` Jason Gunthorpe
     [not found]         ` <20150611041124.GC16599-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2015-06-11  6:04           ` Somnath Kotur
2015-06-08 21:37   ` [PATCH for-next V5 00/12] Move RoCE GID management " Hefty, Sean
     [not found]     ` <1828884A29C6694DAF28B7E6B8A82373A8FE5D17-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2015-06-09  7:27       ` Matan Barak
     [not found]         ` <55769561.8000300-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2015-06-10  8:53           ` Or Gerlitz
     [not found]             ` <5577FAFB.8020205-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2015-06-10 15:00               ` Jason Gunthorpe
     [not found]                 ` <20150610150010.GA11243-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2015-06-10 15:08                   ` Matan Barak
     [not found]                     ` <557852EE.5030107-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2015-06-10 18:49                       ` Jason Gunthorpe
     [not found]                         ` <20150610184954.GA26404-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2015-06-10 20:19                           ` Matan Barak
     [not found]                             ` <CAAKD3BB90iZ98B2ADG+=ZYuEVtLq26a99BEjQCR8U1vzvcG+Gw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-06-10 22:01                               ` Jason Gunthorpe
     [not found]                                 ` <20150610220154.GA4391-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2015-06-11  9:49                                   ` Matan Barak
     [not found]                                     ` <CAAKD3BChd10Gd4P2Mwm+46aW+PJBT3j7K-BLex0Fkm5UdtUG3w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-06-11 16:27                                       ` Jason Gunthorpe
2015-06-12 12:29                                   ` Or Gerlitz
     [not found]                                     ` <CAJ3xEMiXWN9wC5u6iapKMVb4=bfzdnuy3CaZryV0nOFL_Cgmhw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-06-12 16:11                                       ` Jason Gunthorpe
2015-06-11  1:06                           ` Doug Ledford
     [not found]                             ` <1433984788.71666.78.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-06-11  3:57                               ` Jason Gunthorpe
     [not found]                                 ` <20150611035727.GA16599-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
2015-06-11  4:49                                   ` Doug Ledford
     [not found]                                     ` <1433998199.71666.144.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-06-11  5:38                                       ` Jason Gunthorpe [this message]
2015-06-11 10:15                                   ` Matan Barak
2015-06-11 10:09                               ` Matan Barak
2015-06-11  0:15                   ` Doug Ledford
     [not found]                     ` <1433981756.71666.60.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2015-06-11  4:07                       ` Jason Gunthorpe
2015-06-11  9:51                       ` Matan Barak
2015-06-10 15:09               ` Hefty, Sean
     [not found]                 ` <1828884A29C6694DAF28B7E6B8A82373A8FE6616-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2015-06-10 15:19                   ` Matan Barak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150611053823.GA22369@obsidianresearch.com \
    --to=jgunthorpe-epgobjl8dl3ta4ec/59zmfatqe2ktcn/@public.gmane.org \
    --cc=Somnath.Kotur-xCNTGe7nOvUAvxtiuMwx3w@public.gmane.org \
    --cc=dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=matanb-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org \
    --cc=monis-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org \
    --cc=ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org \
    --cc=sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.