Page MenuHomeFreeBSD

[ggated/ggatec] Simplify Gate Handshake.
Needs ReviewPublic

Authored by ota_j.email.ne.jp on Thu, Sep 24, 2:06 AM.

Details

Reviewers
markj
pjd
Group Reviewers
manpages
Summary

Existing handshake, version 0, exchanges a token twice to pass input and output separately with different TCP connections.

New handshake, version 1, uses a single connection for both input and output.
This reduces the number of TCP connections between client and server by a half and also simplifies server side code( after version 0 is removed.)

New client will try newer protocol version first and if fails, it falls back to the old protocol, by default. -0 or -1 may be used to explicitly specify the protocol version to use for a connection.

The server side will accept both versions.

This idea was mentioned in https://reviews.freebsd.org/D26168 .
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=132798 was the original bug report I filed and proposed a fix. Back then, same change was suggested but without compatibilities.

Test Plan

Try all combination of new and old protocol with new implemtaton and with older version, i.e. 12.x-RELEASE.

server% ggated
client% ggatec <host> <path>
client% ggatec -0 <host> <path>
client% ggatec -1 <host> <path>

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 34253
Build 31397: arc lint + arc unit

Event Timeline

ota_j.email.ne.jp retitled this revision from Simplify Gate Handshake. to [ggated/ggatec] Simplify Gate Handshake..Thu, Sep 24, 3:48 AM
ota_j.email.ne.jp edited the summary of this revision. (Show Details)
ota_j.email.ne.jp added a reviewer: markj.

Could you explain why using a single TCP connection improves performance? I find it easy to believe that it does, but I'm fairly ignorant of TCP internals and would like to understand the reason for it.

The slowness issue existed back in a decade ago around 6.1-RELEASE and some years/releases thereafter.

That's when I posted https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=132798 and according to my note, the transmission rate was 1 packet per sec. I recall that happened to a specific pair of machines. Some pairs of machines or local connections worked okay. I don't see these issues any longer in the past couple of years. I suspect it was more like driver issues. The patch on the bugzilla allowed all of combinations of machines to work at maximum capacities. The original patch wasn't backward compatible.

Given TCP problem seem to be gone now, the change is to simplify connection negotiation and reduce open TCP ports. The existing implementation opens 2 TCP connections per client. So, more TCP ports are used. The new code only needs to one. The current protocol will run out of TCP ports sooner than the new one.

Ggated maintains a list of connection in order to match read TCP and write TCP connections for a single session and pair up during hand shake. New protocol won't need data structure on ggated side. I don't know if there is a chance to remove the old protocol, though.

I don't know if there is a chance to remove the old protocol, though.

I'm not sure. What do you use ggate for? I don't think it gets a lot of use in general.

sbin/ggate/ggatec/ggatec.c
77

This is rather confusing. 2 apparently means that versions 0 and 1 are acceptable. It might make more sense to have a bitmask of permitted versions.

295

This can just be %u.

295–297

Extra whitespace after "?".

407

Extra whitespace between the parentheses.

420

Can just be if (g_gatec_connect_v1()) { ...

424

The style is wrong here, see style(9).

558

This looks wrong. Shouldn't it be protocol_version = ch - '0'?

sbin/ggate/ggated/ggated.c
850

"handshake() returns true..."

930

How can we have gv_version != GGATE_VERSION0 here? If it is equal to GGATE_VERSION1 then conn == NULL.

986–987

Isn't this statement unreachable?

sbin/ggate/shared/ggate.h
49

I would leave "ggatec" uncapitalized, ditto below.

Address some of problems pointed by code review.

I make multiple read-only connection for installworld/installkernel from different machines.
Otherwise, I use ggate to export some ufs file systems.
I could use nfs for these as well.

Few cases, I had to export devises to run installworld/kernel for upgrading from RELEASE-12.0 to RELEASE-12.1 due to ifunc as installkernel/installworld failed due to difference in compile/link flags. That was an convenient use case.

Another rare case is I had diskless boot and wanted to nfs export from the diskless boot for local dis file sharing. NFS exports from diskless boot seem to have problems and I had use ggated to export.

I will address other comments in following updates.

Use bitmask to indicate which protocol versions to try.
Also remove duplicate check.

ota_j.email.ne.jp added inline comments.
sbin/ggate/ggated/ggated.c
986–987

Yes, that's the case because of another switch statement returning earlier above.

What's the best practice in FreeBSD?

Remove extra spaces around a function argument.