Re: [lwip-devel] Desiderata for 2.2.0: better support for Debian packaging and VirtualSquare

[Renzo Davoli]

Hi Joan,

On Sun, Jan 08, 2023 at 12:29:54PM +0100, Joan Lledó via lwip-devel wrote:
> El 5/1/23 a les 13:51, Renzo Davoli ha escrit:
> > 1- The code generated by contrib/ports/unix/lib has unresolved symbols:
> > $ nm liblwip.so | grep ' U ' | grep -v @
> >                  U sio_read
> >                  U sio_tryread
> > slirpif requires sio_read and sio_tryread.
> > The patch named 1-libnoslip.patch excludes slip.c from the library
> The patch is OK, but I don't kinda like the idea of adding the slipif.c to a
> cmake target (lwipnetif_SRCS) and then remove it explicitely for the Unix
> library. Would it be possible to use another approach?
You are right. This approach was to minimize the impact for the other
architectures that may include src/Filelists.cmake.
Any other way to exclude slipif fits, e.g. a cmake variable used as a flag,
say if LWIP_EXCLUDE_SLIPIF is set slipif.c is *not* added.
> 
> > Does that mean everyone linking to that library has to provide
> > sio_read/sio_tryread even if not using slipif right now? Or do we have
> > lazy binding and this is only a "cleanup" thing?
> I wrote that patch in 2018 and I honestly don't remember why I needed to
> remove slipif.c. Yesterday I tried to build the package, install it, link it
> to a test program during compilation and run it, all without removing
> slipif.c from the cmake target. I only saw some warnings about the missing
> methods during the package building, everything else worked fine for me. So
> maybe we don't need the patch at all. Renzo, have you tried to
> build/install/link/use the debian package without removing slipif.c?

I have just:
* got the debian package source: 'apt-get source liblwip0'
* added slipif.c in /src/FileLists.cmake
  set(lwipnetif_SRCS
      ${LWIP_DIR}/src/netif/ethernet.c
      ${LWIP_DIR}/src/netif/bridgeif.c
      ${LWIP_DIR}/src/netif/bridgeif_fdb.c
      ${LWIP_DIR}/src/netif/slipif.c
  )

* built and installed the debian package:
  $ debuild -b -us -uc
  $ cd ..
  $ sudo dpkg -i liblwip0_2.1.3+dfsg1-2_amd64.deb
  $ sudo dpkg -i liblwip-dev_2.1.3+dfsg1-2_amd64.deb

* compiled a test source... here is what I got:
  $ gcc -o cli cli.c -I /usr/include/lwip -llwip
  /usr/bin/ld: 
/usr/lib/gcc/x86_64-linux-gnu/12/../../../x86_64-linux-gnu/liblwip.so: 
undefined reference to `sio_tryread'
  /usr/bin/ld: 
/usr/lib/gcc/x86_64-linux-gnu/12/../../../x86_64-linux-gnu/liblwip.so: 
undefined reference to `sio_read'
  collect2: error: ld returned 1 exit status

IMO slipif.c must be skipped, or an implementation of sio_tryread and
sio_read must be provided, maybe a stub implementation, otherwise
there are unresolved symbols.
> > 2- CMakeLists.txt in contrib/ports/unix/lib lacks support for install and
> > uninstall and the compatibility with linux sockaddr.
> > The patch named 2-linuxlib.patch adds a new dir
> contrib/ports/unix/linuxlib
> > to generate a more linux-consistent library.
> 
> I'm not sure about this. Would it be possible to add the new features to the
> already existing unix library? I would prefer to not have two libraries
> under the same port. If we wanted to create a new library specifically for
> Linux, it might be better to create a new port for linux instead of include
> a new library for the unix port. Besides, the current unix library is not
> only used for Linux, also for the Hurd. And the Debian package should work
> properly at least in Linux, Hurd and BSD systems, so calling it linuxlib
> doesn't fit its purpose. IMO the best thing we can do is to try to add as
> many features as possible to the current unix library.
> 
Again my approach was to be as conservative as possible to avoid
side effects for other architectures/projects.

Your "port" patch for Debian adds a specific lwipopts.h file and two
'external headers': posix/inet.h, posix/sockets.h
Your lwipopts.h is significantly different from that in contrib/port/unix/lib
and the include files are not in the lwip source tree, thus the idea to add
a specific linux lib directory under the unix port.

Maybe you have a better view and knowledge of the projects using
the unix/lib code and you can see that it is safe to merge/update the code of
the unix library.

I vote for a new library, it is a surely safe choice. If the name linuxlib is
misleading we can call it debianlib or posixlib (one main point
is that the new lib uses posix compatible socket definitions).
The code needed for the new lib is just:
CMakeLists + lwipopts.h + posix/inet.h + posix/sockets.h

> > Joan, can I take you in here? What do you think would be required to
> > make packaging lwIP into debian easier? This is kind of the last thing
> > before releasing 2.2.0 I'm waiting for...
> 
> Of course, I'm interested in simplifying the Debian packaging for lwip, the
> more code from my patches that could be upstream, the better. In particular
> there are a few patches which are upstream but aren't included in 2.1.3
> release:
> https://savannah.nongnu.org/patch/?9350
> https://savannah.nongnu.org/patch/?9807
> https://security-tracker.debian.org/tracker/CVE-2020-8597
confirmed. these patches are already included in mainstream for 2.2.0
> 
> Apart from that, the most time consuming patch is usually max_sockes, which
> implements the unlimited number of sockets for lwip. If there are changes
> src/api/sockets.c in upstream, I need to adapt this patch to the new code
> and then test it works properly.
>From a very quick check it seems that the 'max_sockets' patch applies
almost flawlessly.
Apart from some line offsets, the only changes needed appear to be:
* a problem with the context at src/api/sockets.c:1777, the current
patch fails due to a different name of a variable in the previous line, 
it needs to be patched by hand:
-  done_socket(&sockets[i - LWIP_SOCKET_OFFSET]);
+  done_socket(translate_socket(i - LWIP_SOCKET_OFFSET));
* the new path of the lwipopts.h file.
> 
> Thanks.
Thank you!

        renzo