[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT/CLICK PATCH v2 01/11] Initial public release: basic unikraft files



Hi Simon,

Felipe and I decided to split the work on those. I'll comment on what I'll fix.

On 6/5/19 1:13 PM, Simon Kuenzer wrote:
Hey all,

I have actually some comments about this patch. See my comments inline.

On 05.06.19 10:56, Costin Lupu wrote:
Reviewed-by: Costin Lupu <costin.lupu@xxxxxxxxx>

On 6/5/19 10:35 AM, Florian Schmidt wrote:
--- /dev/null
+++ b/Config.uk
@@ -0,0 +1,86 @@
+### Invisible option for dependencies
+config APP_DEPENDENCIES
+    bool
+    default y
+
+menuconfig LIBCLICK
+    bool "The Click modular router"
+    depends on HAVE_LIBC
+    select LIBLWIP
+    default y

It is probably better if you select LIBNEWLIB instead of checking if a LIBC is there. If we add more libc's later, we anyway need to check if it compiles. newlib is the working configuration.

OK, I'll select LIBNEWLIB. I also realized that click needs ukunistd, so I'll also add select UKUNISTD.


--- /dev/null
+++ b/Makefile
@@ -0,0 +1,9 @@
+UK_ROOT ?= $(PWD)/../unikraft
+UK_LIBS ?= $(PWD)/..
+LIBS := $(UK_LIBS)/newlib:$(UK_LIBS)/lwip
+

So far, we had the following default definition for UK_ROOT and UK_LIBS:
  UK_ROOT ?= $(PWD)/../../unikraft
  UK_LIBS ?= $(PWD)/../../libs
For simplicity we should use the as we do with the applications `helloworld` and `httpreply` so that user can expect the same default directory structure.


I'll change the paths accordingly.

+################################################################################ +# Check requirements: Click requires netdev access, so LWIP must not attach them +################################################################################
+ifeq ($(CONFIG_LWIP_AUTOIFACE),y)
+$(error Click cannot be built with LWIP option "Automatically attach netifs"! Please disable)
+endif

Could you add a comment why you are doing this with an error?: KConfig unfortunately does not support to `unselect` an configuration option. I hope problems like this can be fixed by updating to a recent KConfig system.


I'll add a comment.

The rest is up to Felipe.

Cheers,
Florian

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.