On 11/30/20 2:44 PM, Bjoern A. Zeeb wrote:
On 30 Nov 2020, at 9:27, Emmanuel Vadot wrote:

On Mon, 30 Nov 2020 01:13:12 +0000
"Bjoern A. Zeeb" <bzeeb-li...@lists.zabbadoz.net> wrote:

On 29 Nov 2020, at 19:38, Matt Macy wrote:

Author: mmacy
Date: Sun Nov 29 19:38:03 2020
New Revision: 368163
URL: https://svnweb.freebsd.org/changeset/base/368163

Log:
  Import kernel WireGuard support

  Data path largely shared with the OpenBSD implementation by
  Matt Dunwoodie <n...@nconroy.net>

  Reviewed by:    gre...@freebsd.org
  MFC after:    1 month
  Sponsored by:    Rubicon LLC, (Netgate)
  Differential Revision:    https://reviews.freebsd.org/D26137

Added:
  head/sbin/ifconfig/ifwg.c   (contents, props changed)
  head/sys/dev/if_wg/
  head/sys/dev/if_wg/include/
  head/sys/dev/if_wg/include/crypto/blake2s.h   (contents, props
changed)
  head/sys/dev/if_wg/include/crypto/curve25519.h   (contents, props
changed)
  head/sys/dev/if_wg/include/crypto/zinc.h   (contents, props changed)
  head/sys/dev/if_wg/include/sys/
  head/sys/dev/if_wg/include/sys/if_wg_session.h   (contents, props
changed)
  head/sys/dev/if_wg/include/sys/if_wg_session_vars.h   (contents,
props changed)
  head/sys/dev/if_wg/include/sys/simd-x86_64.h   (contents, props
changed)
  head/sys/dev/if_wg/include/sys/support.h   (contents, props changed)
  head/sys/dev/if_wg/include/sys/wg_cookie.h   (contents, props
changed)
  head/sys/dev/if_wg/include/sys/wg_module.h   (contents, props
changed)
  head/sys/dev/if_wg/include/sys/wg_noise.h   (contents, props
changed)
  head/sys/dev/if_wg/include/zinc/blake2s.h   (contents, props
changed)
  head/sys/dev/if_wg/include/zinc/chacha20.h   (contents, props
changed)
  head/sys/dev/if_wg/include/zinc/chacha20poly1305.h   (contents,
props changed)
  head/sys/dev/if_wg/include/zinc/curve25519.h   (contents, props
changed)
  head/sys/dev/if_wg/include/zinc/poly1305.h   (contents, props
changed)
  head/sys/dev/if_wg/module/
  head/sys/dev/if_wg/module/blake2s.c   (contents, props changed)
  head/sys/dev/if_wg/module/blake2s.h   (contents, props changed)
  head/sys/dev/if_wg/module/chacha20-x86_64.S   (contents, props
changed)
  head/sys/dev/if_wg/module/crypto/zinc/chacha20/chacha20-arm-glue.c
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/chacha20/chacha20-arm.pl
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/chacha20/chacha20-arm64.pl
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/chacha20/chacha20-mips-glue.c
 (contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/chacha20/chacha20-mips.S
(contents, props changed)
head/sys/dev/if_wg/module/crypto/zinc/chacha20/chacha20-unrolled-arm.S
  (contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/chacha20/chacha20-x86_64-glue.c
  (contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/chacha20/chacha20-x86_64.pl
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/chacha20/chacha20.c
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/chacha20poly1305.c
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/poly1305/poly1305-arm-glue.c
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/poly1305/poly1305-arm.pl
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/poly1305/poly1305-arm64.pl
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/poly1305/poly1305-donna32.c
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/poly1305/poly1305-donna64.c
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/poly1305/poly1305-mips-glue.c
 (contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/poly1305/poly1305-mips.S
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/poly1305/poly1305-mips64.pl
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/poly1305/poly1305-x86_64-glue.c
  (contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/poly1305/poly1305-x86_64.pl
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/poly1305/poly1305.c
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/selftest/blake2s.c
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/selftest/chacha20.c
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/selftest/chacha20poly1305.c
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/selftest/curve25519.c
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/selftest/poly1305.c
(contents, props changed)
  head/sys/dev/if_wg/module/crypto/zinc/selftest/run.h   (contents,
props changed)
  head/sys/dev/if_wg/module/curve25519.c   (contents, props changed)
  head/sys/dev/if_wg/module/if_wg_session.c   (contents, props
changed)
  head/sys/dev/if_wg/module/module.c   (contents, props changed)
  head/sys/dev/if_wg/module/poly1305-x86_64.S   (contents, props
changed)
  head/sys/dev/if_wg/module/wg_cookie.c   (contents, props changed)
  head/sys/dev/if_wg/module/wg_noise.c   (contents, props changed)
  head/sys/modules/if_wg/
  head/sys/modules/if_wg/Makefile   (contents, props changed)
Directory Properties:
  head/sys/dev/if_wg/include/crypto/   (props changed)
  head/sys/dev/if_wg/include/zinc/   (props changed)
  head/sys/dev/if_wg/module/crypto/   (props changed)
  head/sys/dev/if_wg/module/crypto/zinc/   (props changed)
  head/sys/dev/if_wg/module/crypto/zinc/chacha20/   (props changed)
  head/sys/dev/if_wg/module/crypto/zinc/poly1305/   (props changed)
  head/sys/dev/if_wg/module/crypto/zinc/selftest/   (props changed)


Looking at sys/dev/if_wg/include/sys/support.h I wonder why zinc was not
done as linuxkpi code?  Could it be?


/bz

 Adding a dependancy on linuxkpi for just a few compat funcs looks
overkill, also having it done that way means that mallocs are typed
with M_WG instead of the global M_LINUXKPI so it's better to track
leaks, if any.

I am sorry, but I am getting tired of hearing this same sentence all over:

(a) for a lot of simple defines including the header files is purely enough
    and doesn’t need the module dependency.   You are not redefining uint32_t     in every single driver either but include sys/types.h (same goes for byte     swapping functions, likely(), ..) and the same does go for the linuxkpi
     header files.
     That avoids having re-typed, re-defined definitions of these things
     n+1 times in kernel.

(b) the alloc compat #defines in support.h are used in two of the crypto
    compat code bits for function local buffers, which are freed before the
     only return.  Tracking those is hopefully not a problem.

(c) There are bits in this change which linuxkpi does not have yet,
     so we’ll implement them a 2nd time in the kernel again one day and
     linuxkpi is all about not doing exactly that.

     zinc is a Linux KPI and the majority of files in this commit and the
     2nd half of my question was if it could be move into linuxkpi (unless
     we’ll take it natively as part of our crypto KPI, which was put on
     the table by others already from my understanding).


Hi,

I had a look at support.h and have the following additional suggestions:

https://svnweb.freebsd.org/base/head/sys/dev/if_wg/include/sys/support.h?view=markup&pathrev=368163

        #define rw_enter_write rw_wlock
67      #define rw_exit_write rw_wunlock
68      #define rw_enter_read rw_rlock
69      #define rw_exit_read rw_runlock
70      #define rw_exit rw_unlock

Please use function macros for functions!

337     #define kmalloc(size, flag) malloc((size), M_WG, M_WAITOK)
338     #define kfree(p) free(p, M_WG)
339     #define vzalloc(size) malloc((size), M_WG, M_WAITOK|M_ZERO)
340     #define vfree(p) free(p, M_WG)

Instead of overloading the existing kmalloc() definitions in the LinuxKPI, I suggest you create some uppercased macros like:

WG_ALLOC(size, flag)
WG_FREE(ptr)
WG_ZALLOC(size, flag)
WG_ZFREE(ptr)

This will reduce the amount of namespace conflicts when using tools like Doxygen.

Else I agree with Bjoern. Try to use the LinuxKPI in FreeBSD when possible for code that use Linux compatible APIs.

--HPS
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Reply via email to