cvs commit: src/sys/conf files src/sys/netinet tcp_ofld.c tcp_ofld.h tcp_var.h toedev.h src/sys/sys socket.h

Kip Macy kip.macy at gmail.com
Thu Dec 13 17:48:07 PST 2007


On Dec 13, 2007 5:30 PM, Robert Watson <rwatson at freebsd.org> wrote
>
> On Thu, 13 Dec 2007, Scott Long wrote:
>
> >> Let's not discourage that just yet.
> >
> > Yes, I would like to discourage disrespectful nit-picking of an important
> > piece of work.
>
> I think you're reading too much into Bjoern's comments.
>
> >> I'd like to see all significant changes to TCP discussed on public mailing
> >> lists well before they are committed -- at that point, someone saying
> >> "actually, I'd name the files a bit differently" is a lot easier to deal
> >> with than, say, immediately after they are committed. This needs to be
> >> communally owned and maintained code, or in two years time we'll find
> >> ourselves in the same position: architectural well-meant changes that are
> >> mostly right, but with no review of the details leading to the inevitable
> >> failures.
> >
> > A failure of what, exactly?  Will the names that Kip chose lead to failures
> > of TCP sessions?  Please enlighten me here.
>
> This thread is a symptom off a specific problem: a failure to seek review for
> the work before committing.

Actually, I asked several people to look at ethng on the wiki. I was
fairly clear that that was very close to what I wanted to commit. I
also said that I wanted to commit at this time. The only person to
actually take the time to give me feedback prior to commit was Mike.
In the future I will request commentary in a more pubilc forum and
make my timelines more specific.


> I'm sure I'm not the only person who saw this
> commit and went, "So where was the public request for review for a major
> change to our TCP stack?"  Requests for more consistent naming, etc, are
> coming out now precisely because that review wasn't sort *before* committing.
> TOE represents a significant architectural modification, including a new KPI
> for device drivers to implement: details matter.  Some of these new filenames,
> function names, field names, etc, will be embedded in third-party source code
> for the forseeable future.  No one is saying that Kip's work isn't appreciated
> or valued -- rather, that at some point with a piece of code as sensitive and
> critical at TCP, it needs to go through careful review and refinement.  I sent
> Kip a large patch within an hour of his commit to clean up similar sorts of
> problems within the file,s making it comply more with the general TCP style
> but also to follow conventions for field-naming in data structures, etc, which
> he committed along with refinments of his own.

Sadly, often the only way to get a real discussion going is to make
the immediacy of it relevant. To date I haven't made any material
structural changes to TCP, I've only added the hooks that will be
needed. As requested by another I will add some commentary on the
purpose of each of the individual hooks to the header file.


> And, FWIW, this doesn't appear to be a bikeshed, because other than you
> arguing that this is turning into a bikeshed, no one seems to disagree with
> the proposed renaming so far.

I have no strong feelings about naming and I'm more than happy to
follow whatever consensus is. My natural inclination is towards
shorter names but I don't identify with my function names I just need
the functionality to be present.


 -Kip


More information about the cvs-all mailing list