Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-25 Thread Wei Wang
> Note that I am not sure we correctly test that a second connect() can be > done, and I am not sure kernel would check that the remote IP and > destination port is the same. > Ie what should happen for > setsockopt(fd, SOL_TCP, TCP_FASTOPEN_CONNECT, , 4) > connect(fd, "1.2.3.4:80") >

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-25 Thread David Miller
From: Wei Wang Date: Wed, 25 Jan 2017 10:54:50 -0800 >> Yes sorry David, for me it's OK. If Wei runs his whole series of tests >> on it again, it should be perfect. > > I just ran all the TFO related tests with Willy's patch on top of this > patch series. > And everything

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-25 Thread Eric Dumazet
On Wed, 2017-01-25 at 10:54 -0800, Wei Wang wrote: > > Yes sorry David, for me it's OK. If Wei runs his whole series of tests > > on it again, it should be perfect. > > I just ran all the TFO related tests with Willy's patch on top of this > patch series. > And everything passes. Note that I am

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-25 Thread Wei Wang
> Yes sorry David, for me it's OK. If Wei runs his whole series of tests > on it again, it should be perfect. I just ran all the TFO related tests with Willy's patch on top of this patch series. And everything passes. On Wed, Jan 25, 2017 at 9:54 AM, Willy Tarreau wrote: > On Wed,

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-25 Thread Willy Tarreau
On Wed, Jan 25, 2017 at 12:22:05PM -0500, David Miller wrote: > From: Wei Wang > Date: Wed, 25 Jan 2017 09:15:34 -0800 > > > Looks like you sent a separate patch on top of this patch series to > > address double connect(). Then I think this patch series should be > > good

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-25 Thread Willy Tarreau
Hi Wei, On Wed, Jan 25, 2017 at 09:15:34AM -0800, Wei Wang wrote: > Willy, > > Looks like you sent a separate patch on top of this patch series to address > double connect(). Yes, sorry, I wanted to reply to this thread after the git-send-email and got caught immediately after :-) So as

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-25 Thread David Miller
From: Wei Wang Date: Wed, 25 Jan 2017 09:15:34 -0800 > Looks like you sent a separate patch on top of this patch series to > address double connect(). Then I think this patch series should be > good to go. Indeed, Willy please give some kind of ACK. Thanks.

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-24 Thread Willy Tarreau
On Tue, Jan 24, 2017 at 10:51:25AM -0800, Eric Dumazet wrote: > We do not return -1 / EINPROGRESS but 0 > > Do not call connect() twice, it is clearly not supposed to work. Yes it is, it normally returns -1 / EISCONN on a regular socket : EISCONN The socket is already

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-24 Thread Eric Dumazet
On Tue, 2017-01-24 at 19:34 +0100, Willy Tarreau wrote: > Hi Eric, > > On Tue, Jan 24, 2017 at 09:44:49AM -0800, Eric Dumazet wrote: > > I believe there is a bug in this application. > > > > It does not check connect() return value. > > Yes in fact it does but I noticed the same thing, there's

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-24 Thread Willy Tarreau
On Tue, Jan 24, 2017 at 09:42:07AM -0800, Eric Dumazet wrote: > On Tue, 2017-01-24 at 09:26 -0800, Yuchung Cheng wrote: > > > > > > > Do you think there's a compelling reason for adding a new option or > > > are you interested in a small patch to perform the change above ? > > I like the proposal

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-24 Thread Willy Tarreau
Hi Eric, On Tue, Jan 24, 2017 at 09:44:49AM -0800, Eric Dumazet wrote: > I believe there is a bug in this application. > > It does not check connect() return value. Yes in fact it does but I noticed the same thing, there's something causing the event not to be registered or something like this.

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-24 Thread Eric Dumazet
On Mon, 2017-01-23 at 22:16 +0100, Willy Tarreau wrote: > Hi Wei, > > first, thanks a lot for doing this, it's really awesome! > > I'm testing it on 4.9 on haproxy and I met a corner case : when I > perform a connect() to a server and I have nothing to send, upon > POLLOUT notification since I

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-24 Thread Eric Dumazet
On Tue, 2017-01-24 at 09:26 -0800, Yuchung Cheng wrote: > > > > Do you think there's a compelling reason for adding a new option or > > are you interested in a small patch to perform the change above ? > I like the proposal especially other stack also uses TCP_FASTOPEN >

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-24 Thread Yuchung Cheng
On Mon, Jan 23, 2017 at 11:30 PM, Willy Tarreau wrote: > > On Mon, Jan 23, 2017 at 10:59:22AM -0800, Wei Wang wrote: > > This patch adds a new socket option, TCP_FASTOPEN_CONNECT, as an > > alternative way to perform Fast Open on the active side (client). > > Wei, I think that

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-23 Thread Willy Tarreau
On Mon, Jan 23, 2017 at 10:59:22AM -0800, Wei Wang wrote: > This patch adds a new socket option, TCP_FASTOPEN_CONNECT, as an > alternative way to perform Fast Open on the active side (client). Wei, I think that nothing prevents from reusin the original TCP_FASTOPEN sockopt instead of adding a new

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-23 Thread Willy Tarreau
On Mon, Jan 23, 2017 at 02:57:31PM -0800, Wei Wang wrote: > Yes. That seems to be a valid fix to it. > Let me try it with my existing test cases as well to see if it works for > all scenarios I have. Perfect. Note that since the state 2 is transient I initially thought about abusing the flags

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-23 Thread Willy Tarreau
On Mon, Jan 23, 2017 at 11:01:21PM +0100, Willy Tarreau wrote: > On Mon, Jan 23, 2017 at 10:37:32PM +0100, Willy Tarreau wrote: > > On Mon, Jan 23, 2017 at 01:28:53PM -0800, Wei Wang wrote: > > > Hi Willy, > > > > > > True. If you call connect() multiple times on a socket which already has > > >

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-23 Thread Willy Tarreau
On Mon, Jan 23, 2017 at 10:37:32PM +0100, Willy Tarreau wrote: > On Mon, Jan 23, 2017 at 01:28:53PM -0800, Wei Wang wrote: > > Hi Willy, > > > > True. If you call connect() multiple times on a socket which already has > > cookie without a write(), the second and onward connect() call will return

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-23 Thread Willy Tarreau
On Mon, Jan 23, 2017 at 01:28:53PM -0800, Wei Wang wrote: > Hi Willy, > > True. If you call connect() multiple times on a socket which already has > cookie without a write(), the second and onward connect() call will return > EINPROGRESS. > It is basically because the following code block in

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-23 Thread Willy Tarreau
Hi Wei, first, thanks a lot for doing this, it's really awesome! I'm testing it on 4.9 on haproxy and I met a corner case : when I perform a connect() to a server and I have nothing to send, upon POLLOUT notification since I have nothing to send I simply probe the connection using connect()

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-23 Thread Yuchung Cheng
On Mon, Jan 23, 2017 at 10:59 AM, Wei Wang wrote: > From: Wei Wang > > This patch adds a new socket option, TCP_FASTOPEN_CONNECT, as an > alternative way to perform Fast Open on the active side (client). Prior > to this patch, a client needs to replace the

Re: [PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-23 Thread Eric Dumazet
On Mon, 2017-01-23 at 10:59 -0800, Wei Wang wrote: > From: Wei Wang > > This patch adds a new socket option, TCP_FASTOPEN_CONNECT, as an > alternative way to perform Fast Open on the active side (client). Prior > to this patch, a client needs to replace the connect() call with

[PATCH net-next 3/3] net/tcp-fastopen: Add new API support

2017-01-23 Thread Wei Wang
From: Wei Wang This patch adds a new socket option, TCP_FASTOPEN_CONNECT, as an alternative way to perform Fast Open on the active side (client). Prior to this patch, a client needs to replace the connect() call with sendto(MSG_FASTOPEN). This can be cumbersome for