Fixed sys_socket() plumbing

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Fixed sys_socket() plumbing

Martin Pieuchot
I found my mistake in the previous diff.  `SS_NBIO' was never set on
non blocking sockets.  Diff below fixes that by checking `nonblock'
like it is done in sys_socketpair().

Tests & oks welcome :)

Index: kern/uipc_syscalls.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.164
diff -u -p -r1.164 uipc_syscalls.c
--- kern/uipc_syscalls.c 11 Feb 2018 21:53:57 -0000 1.164
+++ kern/uipc_syscalls.c 12 Feb 2018 08:19:30 -0000
@@ -83,7 +83,7 @@ sys_socket(struct proc *p, void *v, regi
  struct file *fp;
  int type = SCARG(uap, type);
  int domain = SCARG(uap, domain);
- int fd, error;
+ int fd, cloexec, nonblock, fflag, error;
  unsigned int ss = 0;
 
  if ((type & SOCK_DNS) && !(domain == AF_INET || domain == AF_INET6))
@@ -95,24 +95,25 @@ sys_socket(struct proc *p, void *v, regi
  if (error)
  return (error);
 
- fdplock(fdp);
- error = falloc(p, (type & SOCK_CLOEXEC) ? UF_EXCLOSE : 0, &fp, &fd);
- fdpunlock(fdp);
+ type &= ~(SOCK_CLOEXEC | SOCK_NONBLOCK | SOCK_DNS);
+ cloexec = (SCARG(uap, type) & SOCK_CLOEXEC) ? UF_EXCLOSE : 0;
+ nonblock = SCARG(uap, type) & SOCK_NONBLOCK;
+ fflag = FREAD | FWRITE | (nonblock ? FNONBLOCK : 0);
+
+ error = socreate(SCARG(uap, domain), &so, type, SCARG(uap, protocol));
  if (error != 0)
  goto out;
 
- fp->f_flag = FREAD | FWRITE | (type & SOCK_NONBLOCK ? FNONBLOCK : 0);
- fp->f_type = DTYPE_SOCKET;
- fp->f_ops = &socketops;
- error = socreate(SCARG(uap, domain), &so,
-    type & ~(SOCK_CLOEXEC | SOCK_NONBLOCK | SOCK_DNS), SCARG(uap, protocol));
+ fdplock(fdp);
+ error = falloc(p, cloexec, &fp, &fd);
+ fdpunlock(fdp);
  if (error) {
- fdplock(fdp);
- fdremove(fdp, fd);
- closef(fp, p);
- fdpunlock(fdp);
+ soclose(so);
  } else {
- if (type & SOCK_NONBLOCK)
+ fp->f_flag = fflag;
+ fp->f_type = DTYPE_SOCKET;
+ fp->f_ops = &socketops;
+ if (nonblock)
  so->so_state |= SS_NBIO;
  so->so_state |= ss;
  fp->f_data = so;
@@ -451,7 +452,7 @@ sys_socketpair(struct proc *p, void *v,
 
  type  = SCARG(uap, type) & ~(SOCK_CLOEXEC | SOCK_NONBLOCK);
  cloexec = (SCARG(uap, type) & SOCK_CLOEXEC) ? UF_EXCLOSE : 0;
- nonblock = SCARG(uap, type) &  SOCK_NONBLOCK;
+ nonblock = SCARG(uap, type) & SOCK_NONBLOCK;
  fflag = FREAD | FWRITE | (nonblock ? FNONBLOCK : 0);
 
  error = socreate(SCARG(uap, domain), &so1, type, SCARG(uap, protocol));

Reply | Threaded
Open this post in threaded view
|

Re: Fixed sys_socket() plumbing

Landry Breuil-5
On Mon, Feb 12, 2018 at 09:22:54AM +0100, Martin Pieuchot wrote:
> I found my mistake in the previous diff.  `SS_NBIO' was never set on
> non blocking sockets.  Diff below fixes that by checking `nonblock'
> like it is done in sys_socketpair().
>
> Tests & oks welcome :)

Running with it now on my $work desktop (where i had also seen the
"firefox waiting forever on dns resolution" issue).

Landry

Reply | Threaded
Open this post in threaded view
|

Re: Fixed sys_socket() plumbing

Martin Pieuchot
In reply to this post by Martin Pieuchot
On 12/02/18(Mon) 09:22, Martin Pieuchot wrote:
> I found my mistake in the previous diff.  `SS_NBIO' was never set on
> non blocking sockets.  Diff below fixes that by checking `nonblock'
> like it is done in sys_socketpair().
>
> Tests & oks welcome :)

Anyone?

> Index: kern/uipc_syscalls.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_syscalls.c,v
> retrieving revision 1.164
> diff -u -p -r1.164 uipc_syscalls.c
> --- kern/uipc_syscalls.c 11 Feb 2018 21:53:57 -0000 1.164
> +++ kern/uipc_syscalls.c 12 Feb 2018 08:19:30 -0000
> @@ -83,7 +83,7 @@ sys_socket(struct proc *p, void *v, regi
>   struct file *fp;
>   int type = SCARG(uap, type);
>   int domain = SCARG(uap, domain);
> - int fd, error;
> + int fd, cloexec, nonblock, fflag, error;
>   unsigned int ss = 0;
>  
>   if ((type & SOCK_DNS) && !(domain == AF_INET || domain == AF_INET6))
> @@ -95,24 +95,25 @@ sys_socket(struct proc *p, void *v, regi
>   if (error)
>   return (error);
>  
> - fdplock(fdp);
> - error = falloc(p, (type & SOCK_CLOEXEC) ? UF_EXCLOSE : 0, &fp, &fd);
> - fdpunlock(fdp);
> + type &= ~(SOCK_CLOEXEC | SOCK_NONBLOCK | SOCK_DNS);
> + cloexec = (SCARG(uap, type) & SOCK_CLOEXEC) ? UF_EXCLOSE : 0;
> + nonblock = SCARG(uap, type) & SOCK_NONBLOCK;
> + fflag = FREAD | FWRITE | (nonblock ? FNONBLOCK : 0);
> +
> + error = socreate(SCARG(uap, domain), &so, type, SCARG(uap, protocol));
>   if (error != 0)
>   goto out;
>  
> - fp->f_flag = FREAD | FWRITE | (type & SOCK_NONBLOCK ? FNONBLOCK : 0);
> - fp->f_type = DTYPE_SOCKET;
> - fp->f_ops = &socketops;
> - error = socreate(SCARG(uap, domain), &so,
> -    type & ~(SOCK_CLOEXEC | SOCK_NONBLOCK | SOCK_DNS), SCARG(uap, protocol));
> + fdplock(fdp);
> + error = falloc(p, cloexec, &fp, &fd);
> + fdpunlock(fdp);
>   if (error) {
> - fdplock(fdp);
> - fdremove(fdp, fd);
> - closef(fp, p);
> - fdpunlock(fdp);
> + soclose(so);
>   } else {
> - if (type & SOCK_NONBLOCK)
> + fp->f_flag = fflag;
> + fp->f_type = DTYPE_SOCKET;
> + fp->f_ops = &socketops;
> + if (nonblock)
>   so->so_state |= SS_NBIO;
>   so->so_state |= ss;
>   fp->f_data = so;
> @@ -451,7 +452,7 @@ sys_socketpair(struct proc *p, void *v,
>  
>   type  = SCARG(uap, type) & ~(SOCK_CLOEXEC | SOCK_NONBLOCK);
>   cloexec = (SCARG(uap, type) & SOCK_CLOEXEC) ? UF_EXCLOSE : 0;
> - nonblock = SCARG(uap, type) &  SOCK_NONBLOCK;
> + nonblock = SCARG(uap, type) & SOCK_NONBLOCK;
>   fflag = FREAD | FWRITE | (nonblock ? FNONBLOCK : 0);
>  
>   error = socreate(SCARG(uap, domain), &so1, type, SCARG(uap, protocol));
>