making wireguard work on RHEL7/etc.

making wireguard work on RHEL7/etc.

[wireguard]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

hello,

the two diffs attached (hopefully not eaten by mailman) made wireguard
work for me on Scientific Linux 7.3 and CentOS Linux 7.3.1611

hopefully they are useful for other people

they were patched against the source installed by
wireguard-dkms-0.0.20170613-1.el7.centos.noarch in /usr/src/wireguard-*

also available here:

https://pastebin.com/uWGWuF8f
https://pastebin.com/P3VqR1H6

[-- Attachment #2: compat-rhel7-patch.diff --]
[-- Type: text/plain, Size: 2341 bytes --]

--- compat/compat.h.orig	2017-06-13 16:33:26.000000000 +0100
+++ compat/compat.h	2017-06-24 15:28:38.866637683 +0100
@@ -7,6 +7,9 @@
 #include <linux/version.h>
 #include <linux/types.h>
 
+#undef  LINUX_VERSION_CODE
+#define LINUX_VERSION_CODE KERNEL_VERSION(3, 18, 0)
+
 #if LINUX_VERSION_CODE < KERNEL_VERSION(3, 10, 0)
 #error "WireGuard requires Linux >= 3.10"
 #endif
@@ -29,9 +32,9 @@
 #define headers_end data
 #endif
 
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 17, 0)
-#include "udp_tunnel/udp_tunnel_partial_compat.h"
-#endif
+//#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 17, 0)
+//#include "udp_tunnel/udp_tunnel_partial_compat.h"
+//#endif
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0) && !defined(DEBUG) && defined(net_dbg_ratelimited)
 #undef net_dbg_ratelimited
@@ -43,14 +46,14 @@
 #endif
 
 #if (LINUX_VERSION_CODE > KERNEL_VERSION(3, 19, 0) && LINUX_VERSION_CODE < KERNEL_VERSION(3, 19, 6)) || \
-    (LINUX_VERSION_CODE < KERNEL_VERSION(3, 18, 12) && LINUX_VERSION_CODE > KERNEL_VERSION(3, 17, 0)) || \
-    (LINUX_VERSION_CODE < KERNEL_VERSION(3, 16, 8) && LINUX_VERSION_CODE >= KERNEL_VERSION(3, 15, 0)) || \
-    LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 40)
-#define dev_recursion_level() 0
+   (LINUX_VERSION_CODE < KERNEL_VERSION(3, 18, 12) && LINUX_VERSION_CODE > KERNEL_VERSION(3, 17, 0)) || \
+   (LINUX_VERSION_CODE < KERNEL_VERSION(3, 16, 8) && LINUX_VERSION_CODE >= KERNEL_VERSION(3, 15, 0)) || \
+   LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 40)
+//#define dev_recursion_level() 0
 #endif
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0)
-#define ipv6_dst_lookup(a, b, c, d) ipv6_dst_lookup(b, c, d)
+//#define ipv6_dst_lookup(a, b, c, d) ipv6_dst_lookup(b, c, d)
 #endif
 
 #if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 5) && LINUX_VERSION_CODE >= KERNEL_VERSION(4, 2, 0)) || \
@@ -149,7 +152,7 @@
 })
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 0)
+//#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 0)
 #include "checksum/checksum_partial_compat.h"
 static inline void *our_pskb_put(struct sk_buff *skb, struct sk_buff *tail, int len)
 {
@@ -160,7 +163,7 @@
 	return skb_put(tail, len);
 }
 #define pskb_put our_pskb_put
-#endif
+//#endif
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(3, 11, 0)
 #include <net/xfrm.h>

[-- Attachment #3: siphash-rhel7-patch.diff --]
[-- Type: text/plain, Size: 694 bytes --]

--- compat/siphash/siphash.c	2017-06-24 15:32:19.139482133 +0100
+++ compat/siphash/siphash.c	2017-06-24 15:30:09.205021457 +0100
@@ -13,7 +13,7 @@
 #include <linux/siphash.h>
 #include <asm/unaligned.h>
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 0)
+//#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 0)
 #ifdef __LITTLE_ENDIAN
  #define HASH_LEN_DECLARE u32 hash; u32 len;
  #define bytemask_from_count(cnt)	(~(~0ul << (cnt)*8))
@@ -21,7 +21,7 @@
  #define HASH_LEN_DECLARE u32 len; u32 hash;
  #define bytemask_from_count(cnt)	(~(~0ul >> (cnt)*8))
 #endif
-#endif
+//#endif
 
 #if defined(CONFIG_DCACHE_WORD_ACCESS) && BITS_PER_LONG == 64
 #include <linux/dcache.h>

Re: making wireguard work on RHEL7/etc.

[Aaron Muir Hamilton]

This is bizarre, isn't the current RHEL/CentOS/Scientific Linux 7 kernel
a 3.10 series? Are these ifdefs only wrong on RHEL derivatives?

Re: making wireguard work on RHEL7/etc.

[wireguard]

Aaron Muir Hamilton wrote:

> This is bizarre, isn't the current RHEL/CentOS/Scientific Linux 7 kernel
> a 3.10 series? Are these ifdefs only wrong on RHEL derivatives?
>

I think 'yes' to your second question.

RedHat seem to patch and backport excessively from various versions. I am
no expert so tried a bit of a brute-force approach to it.

Given how many people run RedHat derrived kernels, it's something you have
to put up with if you want people to use something.

There might be a better way of detecting it is RHEL

Re: making wireguard work on RHEL7/etc.

[Jason A. Donenfeld]

Hey,

Thanks so much for doing the mind numbing work of trudging through all
those ifdefs. Maintaining compat.h is truly a horrifying experience,
and now somebody can commiserate with me :).

Obviously these patches aren't mergable as-is, but it functions as a
treasure map for how to actually go about it.

Are you aware of any RHEL-specific defines I might be able to use to
differentiate?

Regards,
Jason

Re: making wireguard work on RHEL7/etc.

[wireguard]

Jason A. Donenfeld wrote:

> Are you aware of any RHEL-specific defines I might be able to use to
> differentiate?

A quick look around found this, I think conditional code on RHEL_MAJOR is
probably the way to go, maybe "If KERNEL_VERSION() or RHEL_MAJOR then
<compat blah>"

$ cat /usr/include/linux/version.h

#define LINUX_VERSION_CODE 199168
#define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + (c))
#define RHEL_MAJOR 7
#define RHEL_MINOR 3
#define RHEL_RELEASE_VERSION(a,b) (((a) << 8) + (b))
#define RHEL_RELEASE_CODE 1795
#define RHEL_RELEASE "514.21.2"

Re: making wireguard work on RHEL7/etc.

[Jason A. Donenfeld]

Excellent!

Indeed let's roll with that. Perhaps something quite specific, though, like:

#if RHEL_RELEASE_CODE <= RHEL_RELEASE_VERSION(7,3)

If you want to reroll your patches around this, in a really clean way,
and submit them using git-send-email to the list, I can do a proper
review and we can get this merged.

Jason

Re: making wireguard work on RHEL7/etc.

[Jason A. Donenfeld]

Actually, you're right. This seems best:

#if defined(RHEL_MAJOR) && RHEL_MAJOR <= 7

Looking forward to your patches!

Re: making wireguard work on RHEL7/etc.

[wireguard]

[-- Attachment #1: Type: text/plain, Size: 88 bytes --]

I think this might work ...

I was still able to compile on Amazon AMI/4.9.32 Kernel




[-- Attachment #2: Type: text/plain, Size: 6176 bytes --]

diff --git a/src/compat/compat.h b/src/compat/compat.h
index 159abb1..903c9be 100644
--- a/src/compat/compat.h
+++ b/src/compat/compat.h
@@ -7,6 +7,13 @@
 #include <linux/version.h>
 #include <linux/types.h>
 
+//#undef  LINUX_VERSION_CODE
+//#define LINUX_VERSION_CODE KERNEL_VERSION(3, 18, 0)
+
+#if defined(RHEL_MAJOR) && RHEL_MAJOR <= 7
+#define ISRHEL7
+#endif
+
 #if LINUX_VERSION_CODE < KERNEL_VERSION(3, 10, 0)
 #error "WireGuard requires Linux >= 3.10"
 #endif
@@ -15,7 +22,7 @@
 #define CONFIG_AS_SSSE3
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 18, 0)
+#if (LINUX_VERSION_CODE < KERNEL_VERSION(3, 18, 0)) && !defined(ISRHEL7)
 #define headers_start data
 #define headers_end data
 #endif
@@ -34,13 +41,15 @@
 #endif
 
 #if (LINUX_VERSION_CODE > KERNEL_VERSION(3, 19, 0) && LINUX_VERSION_CODE < KERNEL_VERSION(3, 19, 6)) || \
-    (LINUX_VERSION_CODE < KERNEL_VERSION(3, 18, 12) && LINUX_VERSION_CODE > KERNEL_VERSION(3, 17, 0)) || \
-    (LINUX_VERSION_CODE < KERNEL_VERSION(3, 16, 8) && LINUX_VERSION_CODE >= KERNEL_VERSION(3, 15, 0)) || \
-    LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 40)
+   (LINUX_VERSION_CODE < KERNEL_VERSION(3, 18, 12) && LINUX_VERSION_CODE > KERNEL_VERSION(3, 17, 0)) || \
+   (LINUX_VERSION_CODE < KERNEL_VERSION(3, 16, 8) && LINUX_VERSION_CODE >= KERNEL_VERSION(3, 15, 0)) || \
+   LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 40)
+#if !defined(ISRHEL7)
 #define dev_recursion_level() 0
 #endif
+#endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0)
+#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 3, 0)) && !defined(ISRHEL7)
 #define ipv6_dst_lookup(a, b, c, d) ipv6_dst_lookup(b, c, d)
 #endif
 
@@ -49,21 +58,21 @@
     (LINUX_VERSION_CODE < KERNEL_VERSION(3, 18, 27) && LINUX_VERSION_CODE >= KERNEL_VERSION(3, 17, 0)) || \
     (LINUX_VERSION_CODE < KERNEL_VERSION(3, 16, 8) && LINUX_VERSION_CODE >= KERNEL_VERSION(3, 15, 0)) || \
     (LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 40) && LINUX_VERSION_CODE >= KERNEL_VERSION(3, 13, 0)) || \
-    (LINUX_VERSION_CODE < KERNEL_VERSION(3, 12, 54))
+    (LINUX_VERSION_CODE < KERNEL_VERSION(3, 12, 54) || defined(ISRHEL7))
 
 #include <linux/if.h>
 #include <net/ip_tunnels.h>
 #define IP6_ECN_set_ce(a, b) IP6_ECN_set_ce(b)
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 9, 0)
+#if (LINUX_VERSION_CODE < KERNEL_VERSION(4, 9, 0))
 #define time_is_before_jiffies64(a) time_after64(get_jiffies_64(), a)
 #define time_is_after_jiffies64(a) time_before64(get_jiffies_64(), a)
 #define time_is_before_eq_jiffies64(a) time_after_eq64(get_jiffies_64(), a)
 #define time_is_after_eq_jiffies64(a) time_before_eq64(get_jiffies_64(), a)
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 12, 0) && IS_ENABLED(CONFIG_IPV6)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 12, 0) && IS_ENABLED(CONFIG_IPV6) && !defined(ISRHEL7)
 #include <net/ipv6.h>
 struct ipv6_stub_type {
 	void *udpv6_encap_enable;
@@ -114,17 +123,17 @@ static inline u32 get_random_u32(void)
 }
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 18, 0)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 18, 0) && !defined(ISRHEL7)
 static inline void netif_keep_dst(struct net_device *dev)
 {
 	dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
 }
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 0)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 14, 0) && !defined(ISRHEL7)
 #define pcpu_sw_netstats pcpu_tstats
 #define netdev_alloc_pcpu_stats alloc_percpu
-#elif LINUX_VERSION_CODE < KERNEL_VERSION(3, 15, 0)
+#elif LINUX_VERSION_CODE < KERNEL_VERSION(3, 15, 0) && !defined(ISRHEL7)
 #define netdev_alloc_pcpu_stats(type)					\
 ({									\
 	typeof(type) __percpu *pcpu_stats = alloc_percpu(type);		\
@@ -153,7 +162,7 @@ static inline void *our_pskb_put(struct sk_buff *skb, struct sk_buff *tail, int
 #define pskb_put our_pskb_put
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 11, 0)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 11, 0) && !defined(ISRHEL7)
 #include <net/xfrm.h>
 static inline void skb_scrub_packet(struct sk_buff *skb, bool xnet)
 {
@@ -175,7 +184,7 @@ static inline void skb_scrub_packet(struct sk_buff *skb, bool xnet)
 }
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 12, 0)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 12, 0) && !defined(ISRHEL7)
 #include <linux/random.h>
 static inline u32 prandom_u32_max(u32 ep_ro)
 {
@@ -183,7 +192,7 @@ static inline u32 prandom_u32_max(u32 ep_ro)
 }
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 10, 75)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 10, 75) && !defined(ISRHEL7)
 #define U8_MAX ((u8)~0U)
 #define S8_MAX ((s8)(U8_MAX >> 1))
 #define S8_MIN ((s8)(-S8_MAX - 1))
@@ -198,7 +207,7 @@ static inline u32 prandom_u32_max(u32 ep_ro)
 #define S64_MIN ((s64)(-S64_MAX - 1))
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 10, 60)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 10, 60) && !defined(ISRHEL7)
 /* Making this static may very well invalidate its usefulness,
  * but so it goes with compat code. */
 static inline void memzero_explicit(void *s, size_t count)
@@ -208,7 +217,7 @@ static inline void memzero_explicit(void *s, size_t count)
 }
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 12, 0)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 12, 0) && !defined(ISRHEL7)
 static const struct in6_addr our_in6addr_any = IN6ADDR_ANY_INIT;
 #define in6addr_any our_in6addr_any
 #endif
@@ -267,11 +276,11 @@ static inline int get_random_bytes_wait(void *buf, int nbytes)
 }
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 11, 0)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 11, 0) && !defined(ISRHEL7)
 #define system_power_efficient_wq system_unbound_wq
 #endif
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 17, 0)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 17, 0) && !defined(ISRHEL7)
 #include <linux/ktime.h>
 static inline u64 ktime_get_ns(void)
 {
@@ -339,4 +348,7 @@ static inline struct net_device *netdev_pub(void *dev)
 #define net_dbg_skb_ratelimited(fmt, skb, ...)
 #endif
 
+#if defined(ISRHEL7)
+#undef ISRHEL7
+#endif
 #endif

Re: making wireguard work on RHEL7/etc.

[Jason A. Donenfeld]

On Mon, Jun 26, 2017 at 08:55:02PM +0100, wireguard@wut.to wrote:
> I think this might work ...
> 
> I was still able to compile on Amazon AMI/4.9.32 Kernel

Thanks! This isn't the proper way to submit a patch, but I'll examine
the diff and roll something satisfactory myself. Thanks a bunch for
doing all the hard work with this. Hopefully the next wireguard snapshot
will have out-of-the-box RHEL support.

Re: making wireguard work on RHEL7/etc.

[wireguard]

another fix for RHEL7/etc., the version of bash included doesn't support
'local -n' (neither does Amazon AMI Linux either)

diff --git a/src/tools/wg-quick.bash b/src/tools/wg-quick.bash
index 6edcd3a..afec996 100755
--- a/src/tools/wg-quick.bash
+++ b/src/tools/wg-quick.bash
@@ -54,7 +54,7 @@ parse_options() {
 }

 read_bool() {
-       local -n out="$1"
+       local out="$1"
        case "$2" in
        true) out=1 ;;
        false) out=0 ;;

Re: making wireguard work on RHEL7/etc.

[Jason A. Donenfeld]

On Mon, Jun 26, 2017 at 09:45:14PM +0100, wireguard@wut.to wrote:
> another fix for RHEL7/etc., the version of bash included doesn't support
> 'local -n' (neither does Amazon AMI Linux either)
> 
> diff --git a/src/tools/wg-quick.bash b/src/tools/wg-quick.bash
> index 6edcd3a..afec996 100755
> --- a/src/tools/wg-quick.bash
> +++ b/src/tools/wg-quick.bash
> @@ -54,7 +54,7 @@ parse_options() {
>  }
> 
>  read_bool() {
> -       local -n out="$1"
> +       local out="$1"
>         case "$2" in
>         true) out=1 ;;
>         false) out=0 ;;
> 

Before removing `-n`, did you even read the bash man page to learn what
it did? Things are most certainly broken with this patch, and the config
option that uses this will no longer do anything. Nack.

Re: making wireguard work on RHEL7/etc.

[wireguard]

Jason A. Donenfeld wrote:

> On Mon, Jun 26, 2017 at 09:45:14PM +0100, wireguard@wut.to wrote:
> > another fix for RHEL7/etc., the version of bash included doesn't support
> > 'local -n' (neither does Amazon AMI Linux either)
> >
> > diff --git a/src/tools/wg-quick.bash b/src/tools/wg-quick.bash
> > index 6edcd3a..afec996 100755
> > --- a/src/tools/wg-quick.bash
> > +++ b/src/tools/wg-quick.bash
> > @@ -54,7 +54,7 @@ parse_options() {
> >  }
> >
> >  read_bool() {
> > -       local -n out="$1"
> > +       local out="$1"
> >         case "$2" in
> >         true) out=1 ;;
> >         false) out=0 ;;
> >
>
> Before removing `-n`, did you even read the bash man page to learn what
> it did? Things are most certainly broken with this patch, and the config
> option that uses this will no longer do anything. Nack.

I don't have a system that has that option in it, so no. As it stands
wg-quick does not work on a RHEL/Amazon AMI instance. Feel free to ignore
the patch of course if it breaks it the logic.

wg-quick up/down worked for me after this, but it wasn't an extensive test
case.

Re: making wireguard work on RHEL7/etc.

[Jason A. Donenfeld]

On Tue, Jun 27, 2017 at 1:38 PM,  <wireguard@wut.to> wrote:
> I don't have a system that has that option in it, so no. As it stands
> wg-quick does not work on a RHEL/Amazon AMI instance.

Alright. Would you mind reporting the value of `bash --version` on
there? I don't really like to support bash 3, but maybe I'd be more
open to supporting earlier version of bash 4.

Re: making wireguard work on RHEL7/etc.

[wireguard]

Jason A. Donenfeld wrote:

> On Tue, Jun 27, 2017 at 1:38 PM,  <wireguard@wut.to> wrote:
> > I don't have a system that has that option in it, so no. As it stands
> > wg-quick does not work on a RHEL/Amazon AMI instance.
>
> Alright. Would you mind reporting the value of `bash --version` on
> there? I don't really like to support bash 3, but maybe I'd be more
> open to supporting earlier version of bash 4.

GNU bash, version 4.2.46(1)-release (x86_64-redhat-linux-gnu)

Re: making wireguard work on RHEL7/etc.

[Jason A. Donenfeld]

On Tue, Jun 27, 2017 at 08:43:30PM +0100, wireguard@wut.to wrote:
> GNU bash, version 4.2.46(1)-release (x86_64-redhat-linux-gnu)

Arg. Looks like namerefs were only added in bash 4.3. Annoying. The
fix looks something like this, but it's so ugly and bothers me:

diff --git a/src/tools/wg-quick.bash b/src/tools/wg-quick.bash
index 6edcd3a..5325f9d 100755
--- a/src/tools/wg-quick.bash
+++ b/src/tools/wg-quick.bash
@@ -54,10 +54,9 @@ parse_options() {
 }
 
 read_bool() {
-	local -n out="$1"
 	case "$2" in
-	true) out=1 ;;
-	false) out=0 ;;
+	true) eval "$1=1" ;;
+	false) eval "$1=0" ;;
 	*) die "\`$2' is neither true nor false"
 	esac
 }


Not sure whether or not I'll merge it yet.

Re: making wireguard work on RHEL7/etc.

[Jason A. Donenfeld]

Okay, nicer solution found:
https://git.zx2c4.com/WireGuard/commit/?id=1af4fc16054aeab9ef4aac0f9cb42f0dd924bf40

Re: making wireguard work on RHEL7/etc.

[Jason A. Donenfeld]

Okay, and for the kernel, please give this commit a try:

https://git.zx2c4.com/WireGuard/commit/?id=32e20ef627048ffa7f8cb3567e6b4c639cd42607

Or just run the latest git master.

Re: making wireguard work on RHEL7/etc.

[wireguard]

Jason A. Donenfeld wrote:

> Okay, and for the kernel, please give this commit a try:
>
> https://git.zx2c4.com/WireGuard/commit/?id=32e20ef627048ffa7f8cb3567e6b4c639cd42607
>
> Or just run the latest git master.
>

after a quick wg-quick stop / rmmod / insmod / wg-quick start / ping this
seems to work.

Re: making wireguard work on RHEL7/etc.

[Jason A. Donenfeld]

[-- Attachment #1: Type: text/plain, Size: 417 bytes --]

Great, any warnings when compiling?

On Jun 27, 2017 23:30, <wireguard@wut.to> wrote:

> Jason A. Donenfeld wrote:
>
> > Okay, and for the kernel, please give this commit a try:
> >
> > https://git.zx2c4.com/WireGuard/commit/?id=
> 32e20ef627048ffa7f8cb3567e6b4c639cd42607
> >
> > Or just run the latest git master.
> >
>
> after a quick wg-quick stop / rmmod / insmod / wg-quick start / ping this
> seems to work.
>

[-- Attachment #2: Type: text/html, Size: 880 bytes --]

Re: making wireguard work on RHEL7/etc.

[Andrej Kacian]

[-- Attachment #1: Type: text/plain, Size: 771 bytes --]

On Mon, 26 Jun 2017 11:57:56 +0200
"Jason A. Donenfeld" <Jason@zx2c4.com> wrote:

> Excellent!
> 
> Indeed let's roll with that. Perhaps something quite specific, though, like:
> 
> #if RHEL_RELEASE_CODE <= RHEL_RELEASE_VERSION(7,3)
> 
> If you want to reroll your patches around this, in a really clean way,
> and submit them using git-send-email to the list, I can do a proper
> review and we can get this merged.

I wonder - is it a good idea to depend on
nonstandard, distribution-specific defines like this? This kind of
modification IMHO belongs in a local patch applied during a rpmbuild
for a RHEL/Centos package.

I know it might sound like crying wolf now, but this is exactly how
embrace-extend-extinguish happens.

Regards,
-- 
Andrej

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: making wireguard work on RHEL7/etc.

[wireguard]

Andrej Kacian wrote:

> On Mon, 26 Jun 2017 11:57:56 +0200
> "Jason A. Donenfeld" <Jason@zx2c4.com> wrote:
>
> > Excellent!
> >
> > Indeed let's roll with that. Perhaps something quite specific, though,
> > like:
> >
> > #if RHEL_RELEASE_CODE <= RHEL_RELEASE_VERSION(7,3)
> >
> > If you want to reroll your patches around this, in a really clean way,
> > and submit them using git-send-email to the list, I can do a proper
> > review and we can get this merged.
>
> I wonder - is it a good idea to depend on
> nonstandard, distribution-specific defines like this? This kind of
> modification IMHO belongs in a local patch applied during a rpmbuild
> for a RHEL/Centos package.

I think that depends entirely on if you want it to be easy for users to
obtain and use your software. If it requires constant patching and
updates, most people won't bother and will use something else.

The issue for me was that I couldn't just install the software and use it,
I had to figure out how to access the RPM repo (RedHat 7 and the like do
not support dnf/Copr out of the box) I then had to figure out how DKMS was
working (you just get a RPM script error on install), spend time
commenting out random defines in a set of source I don't understand. As
soon as the RPM self-updates this will break and I'll have to do it all
again.

If you have no interest in support RedHat/CentoS/Sceintific Linux/etc.
then it might be worth not publishing RPMs that claim to work with it (and
don't). If you do have interest in it, I would strongly suggest making it
Actually Work.

Re: making wireguard work on RHEL7/etc.

[Andrej Kacian]

[-- Attachment #1: Type: text/plain, Size: 881 bytes --]

On Tue, 27 Jun 2017 08:25:31 +0100 (BST)
wireguard@wut.to wrote:

> > I wonder - is it a good idea to depend on
> > nonstandard, distribution-specific defines like this? This kind of
> > modification IMHO belongs in a local patch applied during a rpmbuild
> > for a RHEL/Centos package.  
> 
> I think that depends entirely on if you want it to be easy for users
> to obtain and use your software. If it requires constant patching and
> updates, most people won't bother and will use something else.

Absolutely. I'm not saying these fixes are not needed, I merely
expressed an opinion that their place is not in the main source code,
but rather in a custom patch applied when creating packages for the
affected distribution(s).

Note that I am not in any way speaking for the project or its
developers, I'm just an interested outsider.

Regards,
-- 
Andrej

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: making wireguard work on RHEL7/etc.

[Jason A. Donenfeld]

[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]

On Tue, Jun 27, 2017 at 11:39:06AM +0200, Andrej Kacian wrote:
> On Tue, 27 Jun 2017 08:25:31 +0100 (BST)
> wireguard@wut.to wrote:
> 
> > > I wonder - is it a good idea to depend on
> > > nonstandard, distribution-specific defines like this? This kind of
> > > modification IMHO belongs in a local patch applied during a rpmbuild
> > > for a RHEL/Centos package.  
> > 
> > I think that depends entirely on if you want it to be easy for users
> > to obtain and use your software. If it requires constant patching and
> > updates, most people won't bother and will use something else.
> 
> Absolutely. I'm not saying these fixes are not needed, I merely
> expressed an opinion that their place is not in the main source code,
> but rather in a custom patch applied when creating packages for the
> affected distribution(s).

compat.h is a dumpster fire already. Tons of people use the RHEL kernel.
I think supporting it won't make an already gross cess pool any more
disgusting. It's a file of hacks; I might as well add another.

(I probably won't add hacks, though, for the heaps of random custom
Android vendor kernels.)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: making wireguard work on RHEL7/etc.

[Daniel Kahn Gillmor]

On Tue 2017-06-27 13:08:14 +0200, Jason A. Donenfeld wrote:
> compat.h is a dumpster fire already. Tons of people use the RHEL kernel.
> I think supporting it won't make an already gross cess pool any more
> disgusting. It's a file of hacks; I might as well add another.
>
> (I probably won't add hacks, though, for the heaps of random custom
> Android vendor kernels.)

sorry to dredge this thread back up from the archives...

https://bugs.debian.org/894909 shows someone trying to build wireguard
against the debian 8 ("jessie" aka "oldstable") 3.16 kernel saying that
they needed a similar patch to make the kernel module build.

I know that debian's kernel team does backport a lot of fixes to our
supported older kernels, similar to older releases of RedHat.

i'm asking over on #debian-kernel (on OFTC) to see whether there is some
similar to RHEL_MAJOR or CONFIG_SUSE_KERNEL or UTS_UBUNTU_RELEASE_ABI
that we can use for jessie.

    --dkg

Re: making wireguard work on RHEL7/etc.

[Daniel Kahn Gillmor]

On Thu 2018-04-05 11:08:20 -0400, Daniel Kahn Gillmor wrote:
> On Tue 2017-06-27 13:08:14 +0200, Jason A. Donenfeld wrote:
>> compat.h is a dumpster fire already. Tons of people use the RHEL kernel.
>> I think supporting it won't make an already gross cess pool any more
>> disgusting. It's a file of hacks; I might as well add another.
>>
>> (I probably won't add hacks, though, for the heaps of random custom
>> Android vendor kernels.)
>
> sorry to dredge this thread back up from the archives...
>
> https://bugs.debian.org/894909 shows someone trying to build wireguard
> against the debian 8 ("jessie" aka "oldstable") 3.16 kernel saying that
> they needed a similar patch to make the kernel module build.
>
> I know that debian's kernel team does backport a lot of fixes to our
> supported older kernels, similar to older releases of RedHat.
>
> i'm asking over on #debian-kernel (on OFTC) to see whether there is some
> similar to RHEL_MAJOR or CONFIG_SUSE_KERNEL or UTS_UBUNTU_RELEASE_ABI
> that we can use for jessie.

Ben Hutchings of the debian kernel team followed up there suggesting
feature tests or autoconf-type checks:

11:02 < dkg> https://bugs.debian.org/894909 suggests that some of the
             backported fixes to 3.16 in jessie are getting in the way
             of compiling wireguard.  the referenced upstream thread
             shows that they worked around the problem for RHEL7 using
             some redhat-specific #defines
11:02 -zwiebelbot:#debian-kernel- Debian#894909: wireguard-dkms:
                                  dev_recursion_level definition problem
                                  - https://bugs.debian.org/894909
11:02 < dkg> is there a comparable fix for older debian kernels?
11:04 < dkg> they have tests for #if RHEL_MAJOR == 7
11:04 < dkg> and #ifdef UTS_UBUNTU_RELEASE_ABI
11:52 < bwh> dkg: I've been there, and the way to deal with this is autoconf-type checks
11:55 < bwh> Well, either '#ifdef feature' or an autoconf-type check

So i don't think there's a comparable #define we can use for the
maintained debian kernels :/

i don't know how dev_recursion_level works.  can we wrap the #define
dev_recursion_level() 0 inside an #ifndef dev_recursion_level safely?

           --dkg