Re: pending changes for TOE support

On Dec 15, 2007 11:16 AM, Robert Watson
<rwatson_at_freebsd.org> wrote:

> I think I would prefer that our policy switch be the capenable flag, so that
> compiling things in or out (or loading, which is the logical equivilent)
> doesn't change functional behavior for existing interfaces.

I believe I said something similar to that in my recent mail.


> I think it behooves us to find out, given that we're designing a KPI for those
> cards also.  I agree with the transistor argument, and given that TOE is a
> fairly undeployed technology at this point, it may quickly resolve itself if
> it hasn't.

I certainly agree. However, where do you stand if they are unwilling
to cooperate?

> Interesting point -- it's amazing how broken checksum processing in, and TCP
> is many orders of magnitude more complex.

Correct, it isn't a given that a vendor's TCP implementation will work
correctly.

> I think a phrase wouldn't hurt; also, I notice you did only address flow
> control in one direction in the comments, which is why I mentioned both
> sending and receiving.

It is fairly implicit for the sending case. There the driver returns
credits to the stack via sbdrop(). I'll have to think about how to
describe the two in a uniform fashion.


> Documenting locking semantics such as "You can rely on lock X being held, but
> do not drop it" takes an extra phrase and can save someone a lot of time.

I *think* I've done that, I guess I could be clearer and say that the
inpcb lock is held and expected not to be dropped.

>
> Most driver authors will not be intimately familiar with tcp_output()'s
> subleties, and documenting error-handling for a KPI is always a good idea.


I'll do my best. However, the TCP stack as it exists now really isn't
very modular at all. This is intended to allow developers to skip
duplicating large swaths of tcp code with small local changes the way
they do on Linux.


> > The interface is intended to drop in the place of tcp_output.
> <"see what tcp_output does" repeated many times>
>
> tcp_output() was previously an internal function of the TCP code, and now the
> semantics are being exposed to device drivers.  Let's not perpetuate poorly
> documented driver interfaces by adding another one.  I think it would be a
> reasonable expectation of a driver author to have consistent documentation of
> the life cycle of data structures and objects, locking expectations and
> requirements, and the semantics for error values from functions.  Certainly,
> they need to look at TCP a fair amount because they'll be pulling things out
> of inpcb, tcpcb, etc, but I'd rather we limit that requirement to simple
> things (addresses, socket options) that are relatively static and avoid it
> being for complex things (locking, error handling) that tend to be more
> subject to change.  Also, if you document what you think the behavior is or
> should be, we can then check to see if we agree.

To the extent possible, yes. I'm not convinced that anyone person
knows what all the existing invariants are in the stack as it is now.
Do you feel that a Stevens'-esque understanding of the environment
around the calls is necessary? I know it sounds like "other people
beat their wives so I can too", but even something as well documented
as ifnet gives no indication of what the locking conventions - e.g.
you can't sleep or acquire sx locks in if_ioctl. The demands placed
should be no greater than those placed on existing subsystems and
should take into account the hitherto somewhat black box nature of
TCP.



 -Kip