Re: [Minios-devel] [UNIKRAFT NGINX 5/5] Add stubs.c and ngx_modules.c

[Costin Lupu <costin.lup@xxxxxxxxx>]

Hi Mihai,

Actually here we have 2 patches here: one for stubs and the other for
ngx_modules.c.

Now, I have some comments regarding the two patches. The patch with the
stubs should also add the entry in the Makefile.

Regarding the ngx modules, I would add them all in one patch (all the
modules supported by nginx) and in a another patch I would disable those
that we currently do not support with our port for Unikraft. Or we can
squish them both in a single one, but I really want all the modules to
be listed here, even the disabled ones. It will help keeping track.

For other comments, please see inline.

On 10/9/19 1:42 AM, Mihai Pogonaru wrote:
> The ngx_modules.c is auto-generated and modified to disable
> the epoll module.
> 
> Signed-off-by: Mihai Pogonaru <pogonarumihai@xxxxxxxxx>
> ---
>  ngx_modules.c | 162 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  stubs.c       |  78 ++++++++++++++++++++++++++++
>  2 files changed, 240 insertions(+)
>  create mode 100644 ngx_modules.c
>  create mode 100644 stubs.c
> 
> diff --git a/ngx_modules.c b/ngx_modules.c
> new file mode 100644
> index 0000000..3bae7ea
> --- /dev/null
> +++ b/ngx_modules.c
> @@ -0,0 +1,162 @@
> +
> +#include <ngx_config.h>
> +#include <ngx_core.h>
> +
> +
> +
> +extern ngx_module_t  ngx_core_module;
> +extern ngx_module_t  ngx_errlog_module;
> +extern ngx_module_t  ngx_conf_module;
> +extern ngx_module_t  ngx_events_module;
> +extern ngx_module_t  ngx_event_core_module;
> +//extern ngx_module_t  ngx_epoll_module;
> +extern ngx_module_t  ngx_select_module;
> +extern ngx_module_t  ngx_http_module;
> +extern ngx_module_t  ngx_http_core_module;
> +extern ngx_module_t  ngx_http_log_module;
> +extern ngx_module_t  ngx_http_upstream_module;
> +extern ngx_module_t  ngx_http_static_module;
> +extern ngx_module_t  ngx_http_autoindex_module;
> +extern ngx_module_t  ngx_http_index_module;
> +extern ngx_module_t  ngx_http_mirror_module;
> +extern ngx_module_t  ngx_http_try_files_module;
> +extern ngx_module_t  ngx_http_access_module;
> +extern ngx_module_t  ngx_http_limit_conn_module;
> +extern ngx_module_t  ngx_http_limit_req_module;
> +extern ngx_module_t  ngx_http_geo_module;
> +extern ngx_module_t  ngx_http_map_module;
> +extern ngx_module_t  ngx_http_split_clients_module;
> +extern ngx_module_t  ngx_http_referer_module;
> +extern ngx_module_t  ngx_http_proxy_module;
> +extern ngx_module_t  ngx_http_fastcgi_module;
> +extern ngx_module_t  ngx_http_uwsgi_module;
> +extern ngx_module_t  ngx_http_scgi_module;
> +extern ngx_module_t  ngx_http_memcached_module;
> +extern ngx_module_t  ngx_http_empty_gif_module;
> +extern ngx_module_t  ngx_http_browser_module;
> +extern ngx_module_t  ngx_http_upstream_hash_module;
> +extern ngx_module_t  ngx_http_upstream_ip_hash_module;
> +extern ngx_module_t  ngx_http_upstream_least_conn_module;
> +extern ngx_module_t  ngx_http_upstream_random_module;
> +extern ngx_module_t  ngx_http_upstream_keepalive_module;
> +extern ngx_module_t  ngx_http_upstream_zone_module;
> +extern ngx_module_t  ngx_http_write_filter_module;
> +extern ngx_module_t  ngx_http_header_filter_module;
> +extern ngx_module_t  ngx_http_chunked_filter_module;
> +extern ngx_module_t  ngx_http_range_header_filter_module;
> +extern ngx_module_t  ngx_http_postpone_filter_module;
> +extern ngx_module_t  ngx_http_ssi_filter_module;
> +extern ngx_module_t  ngx_http_charset_filter_module;
> +extern ngx_module_t  ngx_http_sub_filter_module;
> +extern ngx_module_t  ngx_http_userid_filter_module;
> +extern ngx_module_t  ngx_http_headers_filter_module;
> +extern ngx_module_t  ngx_http_copy_filter_module;
> +extern ngx_module_t  ngx_http_range_body_filter_module;
> +extern ngx_module_t  ngx_http_not_modified_filter_module;
> +
> +ngx_module_t *ngx_modules[] = {
> +    &ngx_core_module,
> +    &ngx_errlog_module,
> +    &ngx_conf_module,
> +    &ngx_events_module,
> +    &ngx_event_core_module,
> +//  &ngx_epoll_module,
> +    &ngx_select_module,
> +    &ngx_http_module,
> +    &ngx_http_core_module,
> +    &ngx_http_log_module,
> +    &ngx_http_upstream_module,
> +    &ngx_http_static_module,
> +    &ngx_http_autoindex_module,
> +    &ngx_http_index_module,
> +    &ngx_http_mirror_module,
> +    &ngx_http_try_files_module,
> +    &ngx_http_access_module,
> +    &ngx_http_limit_conn_module,
> +    &ngx_http_limit_req_module,
> +    &ngx_http_geo_module,
> +    &ngx_http_map_module,
> +    &ngx_http_split_clients_module,
> +    &ngx_http_referer_module,
> +    &ngx_http_proxy_module,
> +    &ngx_http_fastcgi_module,
> +    &ngx_http_uwsgi_module,
> +    &ngx_http_scgi_module,
> +    &ngx_http_memcached_module,
> +    &ngx_http_empty_gif_module,
> +    &ngx_http_browser_module,
> +    &ngx_http_upstream_hash_module,
> +    &ngx_http_upstream_ip_hash_module,
> +    &ngx_http_upstream_least_conn_module,
> +    &ngx_http_upstream_random_module,
> +    &ngx_http_upstream_keepalive_module,
> +    &ngx_http_upstream_zone_module,
> +    &ngx_http_write_filter_module,
> +    &ngx_http_header_filter_module,
> +    &ngx_http_chunked_filter_module,
> +    &ngx_http_range_header_filter_module,
> +    &ngx_http_postpone_filter_module,
> +    &ngx_http_ssi_filter_module,
> +    &ngx_http_charset_filter_module,
> +    &ngx_http_sub_filter_module,
> +    &ngx_http_userid_filter_module,
> +    &ngx_http_headers_filter_module,
> +    &ngx_http_copy_filter_module,
> +    &ngx_http_range_body_filter_module,
> +    &ngx_http_not_modified_filter_module,
> +    NULL
> +};
> +
> +char *ngx_module_names[] = {
> +    "ngx_core_module",
> +    "ngx_errlog_module",
> +    "ngx_conf_module",
> +    "ngx_events_module",
> +    "ngx_event_core_module",
> +//  "ngx_epoll_module",
> +    "ngx_select_module",
> +    "ngx_http_module",
> +    "ngx_http_core_module",
> +    "ngx_http_log_module",
> +    "ngx_http_upstream_module",
> +    "ngx_http_static_module",
> +    "ngx_http_autoindex_module",
> +    "ngx_http_index_module",
> +    "ngx_http_mirror_module",
> +    "ngx_http_try_files_module",
> +    "ngx_http_access_module",
> +    "ngx_http_limit_conn_module",
> +    "ngx_http_limit_req_module",
> +    "ngx_http_geo_module",
> +    "ngx_http_map_module",
> +    "ngx_http_split_clients_module",
> +    "ngx_http_referer_module",
> +    "ngx_http_proxy_module",
> +    "ngx_http_fastcgi_module",
> +    "ngx_http_uwsgi_module",
> +    "ngx_http_scgi_module",
> +    "ngx_http_memcached_module",
> +    "ngx_http_empty_gif_module",
> +    "ngx_http_browser_module",
> +    "ngx_http_upstream_hash_module",
> +    "ngx_http_upstream_ip_hash_module",
> +    "ngx_http_upstream_least_conn_module",
> +    "ngx_http_upstream_random_module",
> +    "ngx_http_upstream_keepalive_module",
> +    "ngx_http_upstream_zone_module",
> +    "ngx_http_write_filter_module",
> +    "ngx_http_header_filter_module",
> +    "ngx_http_chunked_filter_module",
> +    "ngx_http_range_header_filter_module",
> +    "ngx_http_postpone_filter_module",
> +    "ngx_http_ssi_filter_module",
> +    "ngx_http_charset_filter_module",
> +    "ngx_http_sub_filter_module",
> +    "ngx_http_userid_filter_module",
> +    "ngx_http_headers_filter_module",
> +    "ngx_http_copy_filter_module",
> +    "ngx_http_range_body_filter_module",
> +    "ngx_http_not_modified_filter_module",
> +    NULL
> +};
> +
> diff --git a/stubs.c b/stubs.c
> new file mode 100644
> index 0000000..b6a3e32
> --- /dev/null
> +++ b/stubs.c
> @@ -0,0 +1,78 @@
> +/*  SPDX-License-Identifier: BSD-3-Clause */
> +/*
> + *  Nginx stubs.c
> + *
> + *  Authors: Mihai Pogonaru <pogonarumihai@xxxxxxxxx>
> + *
> + *  Copyright (c) 2019, University Politehnica of Bucharest. All rights 
> reserved.
> + *
> + *  Redistribution and use in source and binary forms, with or without
> + *  modification, are permitted provided that the following conditions
> + *  are met:
> + *
> + *  1. Redistributions of source code must retain the above copyright
> + *     notice, this list of conditions and the following disclaimer.
> + *  2. Redistributions in binary form must reproduce the above copyright
> + *     notice, this list of conditions and the following disclaimer in the
> + *     documentation and/or other materials provided with the distribution.
> + *  3. Neither the name of the copyright holder nor the names of its
> + *     contributors may be used to endorse or promote products derived from
> + *     this software without specific prior written permission.
> + *
> + *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS 
> IS"
> + *  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + *  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR 
> PURPOSE
> + *  ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
> + *  LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + *  CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + *  SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + *  INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + *  CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + *  ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF 
> THE
> + *  POSSIBILITY OF SUCH DAMAGE.
> + *
> + *  THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
> + */
> +
> +#include <grp.h>
> +#include <sys/types.h>
> +#include <sys/time.h>
> +#include <uk/print.h>
> +
> +long timezone = 0;

Where is the 'timezone' variable needed?

> +
> +#define WARN_STUBBED() uk_pr_warn("%s() stubbed\n", __func__)

We should actually define something like this in the Unikraft core and
use it everywhere. I guess it's safe to actually define it in <uk/print.h>.

> +
> +int prctl(int option, unsigned long arg2, unsigned long arg3, unsigned long 
> arg4, unsigned long arg5)
> +{
> +     WARN_STUBBED();
> +     return 0;
> +}

This should be actually moved to newlib's glue code in process.c.

> +
> +int setitimer(int which, const struct itimerval *new_value, struct itimerval 
> *old_value)
> +{
> +     WARN_STUBBED();
> +     return 0;
> +}

This should be moved to uktime internal lib.

> +
> +int setpriority(int which, id_t who, int prio)
> +{
> +     WARN_STUBBED();
> +     return 0;
> +}

This should be actually moved to newlib's glue code in process.c.

> +
> +struct group *getgrnam(const char *name)
> +{
> +     WARN_STUBBED();
> +     static struct group g__ = {
> +             .gr_gid = 0
> +     };
> +
> +     return &g__;
> +}

This should be moved to user.c in ukunistd internal lib.

> +
> +ssize_t sendfile(int out_fd, int in_fd, off_t *_offset, size_t count)
> +{
> +     WARN_STUBBED();
> +     return 0;
> +}
> 

We can move this to lwip glue, but I don't feel string about it. We can
keep it here if you want.


Cheers,
Costin

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