[PATCH] pvcalls-back: do not return error on inet_accept EAGAIN

[PATCH] pvcalls-back: do not return error on inet_accept EAGAIN

[Stefano Stabellini]

When the client sends a regular blocking accept request, the backend is
expected to return only when the accept is completed, simulating a
blocking behavior, or return an error.

Specifically, on EAGAIN from inet_accept, the backend shouldn't return
"EAGAIN" to the client. Instead, it should simply continue the wait.
Otherwise, the client will send another accept request, which will cause
another EAGAIN to be sent back, which is a waste of resources and not
conforming to the expected behavior. Change the behavior by turning the
"goto error" into a return.

Signed-off-by: Stefano Stabellini <stefano@aporeto.com>

diff --git a/drivers/xen/pvcalls-back.c b/drivers/xen/pvcalls-back.c
index c7822d8..156e5ae 100644
--- a/drivers/xen/pvcalls-back.c
+++ b/drivers/xen/pvcalls-back.c
@@ -548,7 +548,7 @@ static void __pvcalls_back_accept(struct work_struct *work)
 	ret = inet_accept(mappass->sock, sock, O_NONBLOCK, true);
 	if (ret == -EAGAIN) {
 		sock_release(sock);
-		goto out_error;
+		return;
 	}
 
 	map = pvcalls_new_active_socket(fedata,

Re: [PATCH] pvcalls-back: do not return error on inet_accept EAGAIN

[Boris Ostrovsky]

On 02/02/2018 08:34 PM, Stefano Stabellini wrote:
> When the client sends a regular blocking accept request, the backend is
> expected to return only when the accept is completed, simulating a
> blocking behavior, or return an error.
>
> Specifically, on EAGAIN from inet_accept, the backend shouldn't return
> "EAGAIN" to the client. Instead, it should simply continue the wait.
> Otherwise, the client will send another accept request, which will cause
> another EAGAIN to be sent back, which is a waste of resources and not
> conforming to the expected behavior. Change the behavior by turning the
> "goto error" into a return.
>
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>


I am looking at SYSCALL_DEFINE4(accept4) and sock->ops->accept (which *I 
think* is inet_accept, at least in some cases) passes all errors 
(including EAGAIN)  back to the caller. Is this a different case?

-boris

Re: [PATCH] pvcalls-back: do not return error on inet_accept EAGAIN

[Stefano Stabellini]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1488 bytes --]

On Sun, 4 Feb 2018, Boris Ostrovsky wrote:
> On 02/02/2018 08:34 PM, Stefano Stabellini wrote:
> > When the client sends a regular blocking accept request, the backend is
> > expected to return only when the accept is completed, simulating a
> > blocking behavior, or return an error.
> > 
> > Specifically, on EAGAIN from inet_accept, the backend shouldn't return
> > "EAGAIN" to the client. Instead, it should simply continue the wait.
> > Otherwise, the client will send another accept request, which will cause
> > another EAGAIN to be sent back, which is a waste of resources and not
> > conforming to the expected behavior. Change the behavior by turning the
> > "goto error" into a return.
> > 
> > Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
> 
> 
> I am looking at SYSCALL_DEFINE4(accept4) and sock->ops->accept (which *I
> think* is inet_accept, at least in some cases) passes all errors (including
> EAGAIN)  back to the caller. Is this a different case?

Hi Boris,

I didn't explain myself well. You are right that inet_accept passes all
errors back to the caller, but this is different: not only it would be a
waste of resources to do so, but the different behavior is specified in
the PVCalls spec:

"The backend will reply to the request only when a new connection is
successfully accepted, i.e. the backend does not return EAGAIN or
EWOULDBLOCK."

https://xenbits.xen.org/docs/unstable/misc/pvcalls.html

Look under the "Accept" sub-chapter.

Cheers,

Stefano

Re: [PATCH] pvcalls-back: do not return error on inet_accept EAGAIN

[Boris Ostrovsky]

On 02/05/2018 01:01 PM, Stefano Stabellini wrote:
> On Sun, 4 Feb 2018, Boris Ostrovsky wrote:
>> On 02/02/2018 08:34 PM, Stefano Stabellini wrote:
>>> When the client sends a regular blocking accept request, the backend is
>>> expected to return only when the accept is completed, simulating a
>>> blocking behavior, or return an error.
>>>
>>> Specifically, on EAGAIN from inet_accept, the backend shouldn't return
>>> "EAGAIN" to the client. Instead, it should simply continue the wait.
>>> Otherwise, the client will send another accept request, which will cause
>>> another EAGAIN to be sent back, which is a waste of resources and not
>>> conforming to the expected behavior. Change the behavior by turning the
>>> "goto error" into a return.
>>>
>>> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>
>>
>> I am looking at SYSCALL_DEFINE4(accept4) and sock->ops->accept (which *I
>> think* is inet_accept, at least in some cases) passes all errors (including
>> EAGAIN)  back to the caller. Is this a different case?
> Hi Boris,
>
> I didn't explain myself well. You are right that inet_accept passes all
> errors back to the caller, but this is different: not only it would be a
> waste of resources to do so, but the different behavior is specified in
> the PVCalls spec:
>
> "The backend will reply to the request only when a new connection is
> successfully accepted, i.e. the backend does not return EAGAIN or
> EWOULDBLOCK."

Got it, thanks.

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>


-boris


>
> https://xenbits.xen.org/docs/unstable/misc/pvcalls.html
>
> Look under the "Accept" sub-chapter.
>
> Cheers,
>
> Stefano

Re: [PATCH] pvcalls-back: do not return error on inet_accept EAGAIN

[Juergen Gross]

On 03/02/18 02:34, Stefano Stabellini wrote:
> When the client sends a regular blocking accept request, the backend is
> expected to return only when the accept is completed, simulating a
> blocking behavior, or return an error.
> 
> Specifically, on EAGAIN from inet_accept, the backend shouldn't return
> "EAGAIN" to the client. Instead, it should simply continue the wait.
> Otherwise, the client will send another accept request, which will cause
> another EAGAIN to be sent back, which is a waste of resources and not
> conforming to the expected behavior. Change the behavior by turning the
> "goto error" into a return.
> 
> Signed-off-by: Stefano Stabellini <stefano@aporeto.com>

Committed to xen.tip for-linus-4.16


Juergen