Re: [UNIKRAFT PATCH v2 15/15] plat/linuxu: Convert linux errno to unikraft errno

[Hugo Lefeuvre <hugo.lefeuvre@xxxxxxxxx>]

Hi Sharan,

this patch is enough for a first release of the tap driver, however it
should probably not be our final solution. EAGAIN is not the only error
code that has this problem; EDQUOT and EDESTADDRREQ for instance are
both returned by the write system call and differ between Linux and
BSD/Unikraft.

I think that a better solution would be to have a linuxu_errno_conv()
function that converts errno from Linux to Unikraft/BSD in a clean and
generic way.

I added a few more specific comments inline.

thanks!

regards,
Hugo

On Tue, 2020-06-30 at 11:58 +0200, Sharan Santhanam wrote:
> There are differences in errno between linux and unikraft. This patch
> converts EAGAIN which is 11 in linux to unikraft which is 35.
> 
> Signed-off-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx>
> ---
>  plat/linuxu/tap_io.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/plat/linuxu/tap_io.c b/plat/linuxu/tap_io.c
> index 030c192..ac1d0ae 100644
> --- a/plat/linuxu/tap_io.c
> +++ b/plat/linuxu/tap_io.c
> @@ -117,7 +117,10 @@ ssize_t tap_read(int fd, void *buf, size_t
> count)
>       while (rc == -EINTR)
>               rc = sys_read(fd, buf, count);
>  
> -     if (rc < 0)
> +     if (rc == -11)
> +             /* Explicitly added since linux errno has -11 for
> EAGAIN */
> +             rc = -EWOULDBLOCK;

We should return -EAGAIN to be consistent with the comment and the
commit message.

I would also add a comment:

/* FIXME: EAGAIN is not the only error code affected by this issue
(e.g., EDESTADDRREQ, EDQUOT). We should have a more generic solution
that converts errno from Linux to BSD/Unikraft in a clean and generic
way. */

> +     else if (rc < 0)
>               uk_pr_err("Failed(%ld) to read from the tap
> device\n", rc);
>  
>       return rc;
> @@ -132,7 +135,12 @@ ssize_t tap_write(int fd, const void *buf,
> size_t count)
>               rc = sys_write(fd, buf, count);
>               if (rc == -EINTR)
>                       continue;
> -             else if (rc < 0) {
> +             else if (rc == -11) {
> +                     /* 
> +                      * Explicitly added since linux errno has
> -11 for EAGAIN
> +                      */
> +                     rc = -EWOULDBLOCK;

same: rc =  -EAGAIN.

> +             } else if (rc < 0) {
>                       uk_pr_err("Failed(%ld) to write to the tap
> device\n",
>                                 rc);
>                       return rc;