Re: [Minios-devel] [UNIKRAFT/LWIP PATCH] sockets.c: Make select() not return errors when using it with file descriptor types other than sockets

[Stefan Teodorescu <stefanl.teodorescu@xxxxxxxxx>]

Reviewed-by: Stefan Teodorescu <stefanl.teodorescu@xxxxxxxxx>

On Sun, Nov 17, 2019 at 9:39 AM Stefan Teodorescu
<stefanl.teodorescu@xxxxxxxxx> wrote:
>
> Everything looks good, but I would write a help message for this
> option that makes it more clear to the user that their file descriptor
> (anything else than what is supported by lwip) *will be ignored*.
> The current message might make them understand that select() will
> support the fds not supported by lwip.
>
> On Sun, Nov 17, 2019 at 1:46 AM Costin Lupu <costin.lupu@xxxxxxxxx> wrote:
> >
> > lwip's select() implementation supports only sockets. Up until now, we 
> > returned
> > errors when using select() with other file descriptor types. These changes 
> > make
> > it possible to use other file descriptor types as well, even though we won't
> > receive any notifications for those.
> >
> > Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> > ---
> >  Config.uk | 10 ++++++++++
> >  sockets.c | 24 ++++++++++++++++++++++++
> >  2 files changed, 34 insertions(+)
> >
> > diff --git a/Config.uk b/Config.uk
> > index 92df4d3..8f501fc 100644
> > --- a/Config.uk
> > +++ b/Config.uk
> > @@ -162,6 +162,16 @@ config LWIP_SOCKET
> >         depends on LWIP_THREADS && (LWIP_UDP || LWIP_TCP)
> >         default y
> >
> > +if LWIP_SOCKET
> > +       config LWIP_SOCKET_SELECT_GENERIC_FDS
> > +               bool "Use select() with any file descriptor type"
> > +               default y
> > +               help
> > +                       lwip's select() implementation supports only 
> > sockets. This
> > +                       configuration option makes it possible to use other 
> > file descriptor
> > +                       types as well, even though they are not supported 
> > by lwip.
> > +endif
> > +
> >  menuconfig LWIP_DEBUG
> >         bool "Debug messages"
> >         default n
> > diff --git a/sockets.c b/sockets.c
> > index cd84b97..c2cf57f 100644
> > --- a/sockets.c
> > +++ b/sockets.c
> > @@ -497,12 +497,20 @@ int select(int nfds, fd_set *readfds, fd_set 
> > *writefds, fd_set *exceptfds,
> >                 if (readfds && FD_ISSET(i, readfds)) {
> >                         file = sock_net_file_get(i);
> >                         if (PTRISERR(file)) {
> > +#if CONFIG_LWIP_SOCKET_SELECT_GENERIC_FDS
> > +                               /* We allow other fd types, but we don't 
> > support them */
> > +                               if (PTR2ERR(file) == -EBADF) {
> > +                                       FD_CLR(i, readfds);
> > +                                       continue;
> > +                               }
> > +#else
> >                                 LWIP_DEBUGF(SOCKETS_DEBUG,
> >                                             ("failed to identify socket 
> > descriptor\n"));
> >                                 ret = -1;
> >                                 /* Setting the errno */
> >                                 SOCK_NET_SET_ERRNO(PTR2ERR(file));
> >                                 goto EXIT;
> > +#endif
> >                         }
> >                         if (maxfd < file->sock_fd)
> >                                 maxfd = file->sock_fd;
> > @@ -512,12 +520,20 @@ int select(int nfds, fd_set *readfds, fd_set 
> > *writefds, fd_set *exceptfds,
> >                 if (writefds && FD_ISSET(i, writefds)) {
> >                         file = sock_net_file_get(i);
> >                         if (PTRISERR(file)) {
> > +#if CONFIG_LWIP_SOCKET_SELECT_GENERIC_FDS
> > +                               /* We allow other fd types, but we don't 
> > support them */
> > +                               if (PTR2ERR(file) == -EBADF) {
> > +                                       FD_CLR(i, writefds);
> > +                                       continue;
> > +                               }
> > +#else
> >                                 LWIP_DEBUGF(SOCKETS_DEBUG,
> >                                             ("failed to identify socket 
> > descriptor\n"));
> >                                 ret = -1;
> >                                 /* Setting the errno */
> >                                 SOCK_NET_SET_ERRNO(PTR2ERR(file));
> >                                 goto EXIT;
> > +#endif
> >                         }
> >                         if (maxfd < file->sock_fd)
> >                                 maxfd = file->sock_fd;
> > @@ -527,12 +543,20 @@ int select(int nfds, fd_set *readfds, fd_set 
> > *writefds, fd_set *exceptfds,
> >                 if (exceptfds && FD_ISSET(i, exceptfds)) {
> >                         file = sock_net_file_get(i);
> >                         if (PTRISERR(file)) {
> > +#if CONFIG_LWIP_SOCKET_SELECT_GENERIC_FDS
> > +                               /* We allow other fd types, but we don't 
> > support them */
> > +                               if (PTR2ERR(file) == -EBADF) {
> > +                                       FD_CLR(i, exceptfds);
> > +                                       continue;
> > +                               }
> > +#else
> >                                 LWIP_DEBUGF(SOCKETS_DEBUG,
> >                                             ("failed to identify socket 
> > descriptor\n"));
> >                                 ret = -1;
> >                                 /* Setting the errno */
> >                                 SOCK_NET_SET_ERRNO(PTR2ERR(file));
> >                                 goto EXIT;
> > +#endif
> >                         }
> >                         if (maxfd < file->sock_fd)
> >                                 maxfd = file->sock_fd;
> > --
> > 2.20.1
> >
> >
> > _______________________________________________
> > Minios-devel mailing list
> > Minios-devel@xxxxxxxxxxxxxxxxxxxx
> > https://lists.xenproject.org/mailman/listinfo/minios-devel

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel