On Sat, 15 Dec 2007, Kip Macy wrote:

>> BTW, could you prepare similar comments for toedev.h?

>
> Ok, toedev.h isn't actually originally from me. However, it is a part of the
> API and so should be documented.


Thanks -- when designing a KPI, it's important to keep in mind that the goal
of the author using it will not be to understand our TCP code, but instead, to
implement the driver as quickly and cheaply as possible. Therefore we should
make it as easy as possible for the authors of drivers to get it right. This
comes out a few other places in my requests for clarification or additions --
imagine you weren't familiar with our TCP, what could you get wrong?

>> This structure seems to introduce quite a bit more indirection for
>> non-offloaded cases than before, especially to do things like this:

>
> This should never be called as sc_tu will only be set by the TOE interface
> to the syncache. Also bear in mind that this and detach are only called once
> per connection The real overhead is in calling tcp_output via an indirect
> function call versus a direct function call. Considering the level of
> gratuitous overhead in the output path I would be surprised if this were
> measurable.


Undoubtably this is true for high-end systems sporting PCIe interfaces with
10gbps cards, but we also run in other configurations. A complaint we've had
a fair amount in the last few years is that our work has increasingly targeted
high-end systems where overhead is in cache misses, and decreasingly targeted
low-end systems where overhead is in instruction count. While you offer the
opportunity to compile out some of this, I think we should try to make these
things capable of being fast in both circumstances without multiple compile
paths where it's easy.

>> This would compile to a straight call to tcp_output() when offloading isn't
>> compiled in, and when it is compiled in and offloading isn't enabled, we do
>> a simple flag check rather than invoking a function via a series of
>> pointers.

>
> This is what the current code does. See tcp_ofld.h in CVS. The indirection
> was suggested by Sam as a cleaner abstraction.


I think I prefer the CVS version of the two, although I would like to collapse
the two ifdef parts per my example, so that ifdef and non-ifdef cases are
side-by-side, and so that function headers are shared by the two cases.
Ideally, we'd make the ifdef very, very small, as it's well-known that when we
have two mutually exclusive code paths and one isn't compiled or used
frequently, it rots.

>> +int tcp_offload_connect(struct socket *so, struct sockaddr *nam);
>>
>> This prototype appear not to be documented.

>
> Ok, it appears fairly self-documenting to me =-D


I'm sure you can find some insight to express that's not self-obvious in the
code :-).

>> I find myself wondering if the offload option should be a TCP socket option
>> rather than a socket-layer option, but don't have strong feelings about
>> this.

>
> The assumption is that, if you a) pay the extra for a version of the card
> that supports TOE and b) you load the toe module (it isn't part of the NIC
> driver) that you want your existing software to have its connections
> offloaded. I don't know of any customers that want to modify their user
> applications to selectively enable TOE.


I think you mis-understood my question -- I was wondering whether it should be
selectively disabled by an IP-layer socket option rather than a socket-layer
socket option.

>> What do you intend the policy model to be for enabling TOE, in general?
>> That if the TOE capability is available and the TOE capability is enabled,
>> all TCP sockets via the enabled interface will be offloaded unless
>> SO_NO_OFFLOAD is set?

>
> That is the current usage model. At some point we may tie it into
> pf/ipf/ipfw to provide for offload policy. However, currently we offload all
> connections on an interface until we run out of TCAM entries.
>
>> Are you currently anticipating enabling the capability by default?

>
> Yes, *if the TOE driver is loaded*.


I have somewhat mixed feelings about this, and feel like there should be
something eloquent to say about having functionality administratively enabled
rather than a default when compiled in, but can't quite figure out a nice
expression of that. I think it might have something to do with expecting
vendors of 10gbps cards to like shipping two modules for every device rather
than one, and having the right behavior out of the box if they do ship just
one.

Robert N M Watson
Computer Laboratory
University of Cambridge
_______________________________________________
freebsd-arch@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org"