Summary of changes after porting to Linux and Hurd

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Summary of changes after porting to Linux and Hurd

Joan Lledó-2
Hello lwip,

I recently updated the lwip debian package for the Hurd and Linux to 2.1.2, and during
the process I found some issues I'd like to fix, if you agree:

1.- include/lwip/if_api.h: IF_NAMESIZE could be defined by external socket headers, so
    it should be defined only if it's not defined before.

2.- contrib/ports/unix/lib/lwipopts.h: sys_check_core_locking() and pals are declared
    in lwipopts.h, is this the proper place? why aren't they declared in sys_arch.h?

3.- contrib/ports/unix/port/sys_arch.c: sys_arch_sem_wait() and cond_wait():
    - On the Hurd, pthread_cond_wait() and pthread_cond_timedwait() are awakened when
      glibc cancels the RPC that called them. I wrote some code to handle this case.
  - contrib/ports/unix/port/include/arch/sys_arch.h: I defined SYS_ARCH_INTR to be
    returned by sys_arch_sem_wait() when pthread_cond_wait() was awakened on the Hurd.

4.- contrib/ports/unix/check/CMakeLists.txt: Add support for the Hurd:
  - Line 7: add AND NOT CMAKE_SYSTEM_NAME STREQUAL GNU
  - Line 12: set(LWIP_USE_SANITIZERS true) only if (NOT CMAKE_SYSTEM_NAME STREQUAL GNU)
  - Line 48: add OR CMAKE_SYSTEM_NAME STREQUAL GNU

5.- apps/tftp/tftp.c: recv() could be declared by external socket headers. Rename it to
    tftp_recv()
  - Lines 243 and 471

6.- test/unit/api/test_sockets.c:
  - Line 156: write() could be declared by external socket headers. Call lwip_write()
    instead.
  - Line 333: it expects fcntl() to return 6, but O_RDWR could have another value if
    external socket headers are present. Replace 6 by O_RDWR.
  - In Linux, when using the system's sockets headers, struct msghdr->msg_iovlen is not
    defined as int.
    - POSIX[1] says it must be int
    - My solution was:
      - add a typedef int msg_iovlen_t; in lwip's sockets.h
      - declare msg_iovlen as msg_iovlen_t inside struct msghdr in lwip's sockets.h
      - let the user typedef msg_iovlen_t as the type they need for their system
      - Line 253: declare i as msg_iovlen_t i;

7.- src/api/tcpip.c:
  - New function tcpip_callback_wait() to call a function inside the tcpip thread
    and block the calling thread until the callback finishes.
  - I already wrote a patch[2] for this and am waiting for its review.

8.- src/apps/lwiperf/lwiperf.c:
  - Line 773: Calls htonl() but lwip/inet.h is not included
  - There are other apps and unit tests where htons and pals are being called w/o
    including lwip/inet.h

9.- src/include/lwip/sockets.h:
  - Lines 536 and following lines until the end: I think all definitions of the socket
    functions should be moved outside the #define LWIP_SOCKET_EXTERNAL_HEADERS, because
    a user that has set LWIP_SOCKETS to 1 will need them regardless they use lwip's or
    their own socket headers.
  - struct sockaddr includes sa_len, but some systems like Linux doesn't have this filed,
    which produces many compilation problems.
    - My solution was:
      - Create macros to write and read from sa_len/sin_len/sin6_len fields
      - Replace all occurrences this fields for the proper macro
      - Declare write macros as empty and read macros as default values on systems where
        sa_len and pals are not supported
      - Use _HAVE_SA_LEN to identify whether the system supports sa_len

I'll write patches for all this issues if you think they should be upstream.

Regards.

-------------------
[1] http://pubs.opengroup.org/onlinepubs/9699919799/
[2] https://savannah.nongnu.org/patch/?9807

_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

Re: Summary of changes after porting to Linux and Hurd

goldsimon@gmx.de


Am 7. Juli 2019 09:56:06 MESZ schrieb "Joan Lledó" <[hidden email]>:

>Hello lwip,
>
>I recently updated the lwip debian package for the Hurd and Linux to
>2.1.2, and during
>the process I found some issues I'd like to fix, if you agree:
>
>1.- include/lwip/if_api.h: IF_NAMESIZE could be defined by external
>socket headers, so
>    it should be defined only if it's not defined before.
>
>2.- contrib/ports/unix/lib/lwipopts.h: sys_check_core_locking() and
>pals are declared
>in lwipopts.h, is this the proper place? why aren't they declared in
>sys_arch.h?
>
>3.- contrib/ports/unix/port/sys_arch.c: sys_arch_sem_wait() and
>cond_wait():
>- On the Hurd, pthread_cond_wait() and pthread_cond_timedwait() are
>awakened when
>glibc cancels the RPC that called them. I wrote some code to handle
>this case.
>  - contrib/ports/unix/port/include/arch/sys_arch.h: I defined
>SYS_ARCH_INTR to be
>returned by sys_arch_sem_wait() when pthread_cond_wait() was awakened
>on the Hurd.
>
>4.- contrib/ports/unix/check/CMakeLists.txt: Add support for the Hurd:
>  - Line 7: add AND NOT CMAKE_SYSTEM_NAME STREQUAL GNU
>- Line 12: set(LWIP_USE_SANITIZERS true) only if (NOT CMAKE_SYSTEM_NAME
>STREQUAL GNU)
>  - Line 48: add OR CMAKE_SYSTEM_NAME STREQUAL GNU
>
>5.- apps/tftp/tftp.c: recv() could be declared by external socket
>headers. Rename it to
>    tftp_recv()
>  - Lines 243 and 471
>
>6.- test/unit/api/test_sockets.c:
>- Line 156: write() could be declared by external socket headers. Call
>lwip_write()
>    instead.
>- Line 333: it expects fcntl() to return 6, but O_RDWR could have
>another value if
>    external socket headers are present. Replace 6 by O_RDWR.
>- In Linux, when using the system's sockets headers, struct
>msghdr->msg_iovlen is not
>    defined as int.
>    - POSIX[1] says it must be int
>    - My solution was:
>      - add a typedef int msg_iovlen_t; in lwip's sockets.h
>- declare msg_iovlen as msg_iovlen_t inside struct msghdr in lwip's
>sockets.h
>- let the user typedef msg_iovlen_t as the type they need for their
>system
>      - Line 253: declare i as msg_iovlen_t i;
>
>7.- src/api/tcpip.c:
>- New function tcpip_callback_wait() to call a function inside the
>tcpip thread
>    and block the calling thread until the callback finishes.
>  - I already wrote a patch[2] for this and am waiting for its review.

I'll see to find the time to review it, sorry for the delay.

>8.- src/apps/lwiperf/lwiperf.c:
>  - Line 773: Calls htonl() but lwip/inet.h is not included
>- There are other apps and unit tests where htons and pals are being
>called w/o
>    including lwip/inet.h
>
>9.- src/include/lwip/sockets.h:
>- Lines 536 and following lines until the end: I think all definitions
>of the socket
>functions should be moved outside the #define
>LWIP_SOCKET_EXTERNAL_HEADERS, because
>a user that has set LWIP_SOCKETS to 1 will need them regardless they
>use lwip's or
>    their own socket headers.
>- struct sockaddr includes sa_len, but some systems like Linux doesn't
>have this filed,
>    which produces many compilation problems.
>    - My solution was:
>  - Create macros to write and read from sa_len/sin_len/sin6_len fields
>      - Replace all occurrences this fields for the proper macro
>- Declare write macros as empty and read macros as default values on
>systems where
>        sa_len and pals are not supported
>      - Use _HAVE_SA_LEN to identify whether the system supports sa_len
>
>I'll write patches for all this issues if you think they should be
>upstream.

I think those suggestions overall look ok. Just post the patches.

Regards,
Simon

_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel