[PATCH compat] Update and improve detection of CentOS Stream 8

2021-04-19 Thread Peter Georg
CentOS Stream 8 by now (4.18.0-301.1.el8) reports RHEL_MINOR=5. The
current RHEL 8 minor release is still 3. RHEL 8.4 is in beta. Replace
equal comparison by greater equal to (hopefully) be a little bit more
future proof.

Signed-off-by: Peter Georg 
---
 src/compat/compat-asm.h | 2 +-
 src/compat/compat.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/compat/compat-asm.h b/src/compat/compat-asm.h
index 7067afd..fde21da 100644
--- a/src/compat/compat-asm.h
+++ b/src/compat/compat-asm.h
@@ -15,7 +15,7 @@
 #define ISRHEL7
 #elif RHEL_MAJOR == 8
 #define ISRHEL8
-#if RHEL_MINOR == 4
+#if RHEL_MINOR >= 4
 #define ISCENTOS8S
 #endif
 #endif
diff --git a/src/compat/compat.h b/src/compat/compat.h
index 0227204..26a6d7e 100644
--- a/src/compat/compat.h
+++ b/src/compat/compat.h
@@ -16,7 +16,7 @@
 #define ISRHEL7
 #elif RHEL_MAJOR == 8
 #define ISRHEL8
-#if RHEL_MINOR == 4
+#if RHEL_MINOR >= 4
 #define ISCENTOS8S
 #endif
 #endif
-- 
2.31.1



Re: [PATCH] babel: Drop check for IF_MULTICAST interface flag

2021-04-19 Thread Ondrej Zajicek
On Mon, Apr 19, 2021 at 03:55:18PM +0200, Toke Høiland-Jørgensen wrote:
> Ondrej Zajicek  writes:
> 
> > Is there a reason why to disregard the IF_MULTICAST flag? This seems to me
> > more like a bug in FreeBSD Wireguard implementation that should be fixed
> > there. Is this flag properly checked on Linux, or is there some reason why
> > the flag is missing?
> 
> We did fix Wireguard - see:
> https://git.zx2c4.com/wireguard-freebsd/patch/?id=a7a84a17faf784857f076e37aa4818f6b6c12a95
> 
> However, that didn't help, Babel still refused to use the interface.
> Looking at krt-sock.c, the IF_MULTICAST flag is only set on
> IFF_POINTOPOINT or IFF_BROADCAST on bsd. The Linux code (in netlink.c)
> has a further:
> 
>   if (fl & IFF_MULTICAST)
>   f.flags |= IF_MULTICAST;
> 
> beneath the other flag checks, so maybe that's really what's missing on
> the BSD side?

Yes, it is likely that it is an issue in sysdep/bsd code.


> > Routing protocols in BIRD generally follow this flag (and perhaps use
> > it to switch to unicast-only mode), so i do not see why Babel should
> > behave differently.
> 
> Yeah, I do believe I originally copied that check from one of the other
> protocols. I can see how it makes sense to check the flag and change
> operation mode based on it, but given that Babel doesn't do that it just
> seems kinda redundant? If the interface *actually* is unable to send
> multicast packets, the subsequent socket operation is going to fail, and
> at least that produces an error message instead of just silently
> ignoring the interface like that flag check does :)

Well, i am OK with generating a warning in cases of non-matching interface
type, instead of ignoring it silently. (In contrast to iface down or missing
lladdr, which should be silent, as it may correct later.)

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."


Re: [PATCH] babel: Drop check for IF_MULTICAST interface flag

2021-04-19 Thread Ondrej Zajicek
On Thu, Apr 15, 2021 at 03:44:50PM +0200, Toke Høiland-Jørgensen wrote:
> The babel protocol code was checking interfaces for the IF_MULTICAST flag
> and refusing to run if this isn't present. However, there are cases where
> this flag doesn't correspond to the actual capability of sending multicast
> packets. For instance, Wireguard interfaces on FreeBSD doesn't set the
> required flags, but Babel will run just fine over such an interface given
> the right configuration.

Hi

Is there a reason why to disregard the IF_MULTICAST flag? This seems to me
more like a bug in FreeBSD Wireguard implementation that should be fixed
there. Is this flag properly checked on Linux, or is there some reason why
the flag is missing?

Routing protocols in BIRD generally follow this flag (and perhaps use it
to switch to unicast-only mode), so i do not see why Babel should behave
differently.

-- 
Elen sila lumenn' omentielvo

Ondrej 'Santiago' Zajicek (email: santi...@crfreenet.org)
OpenPGP encrypted e-mails preferred (KeyID 0x11DEADC3, wwwkeys.pgp.net)
"To err is human -- to blame it on a computer is even more so."


[PATCH] sysdep/bsd: propagate OS-level IFF_MULTICAST to internal IF_MULTICAST flag

2021-04-19 Thread Toke Høiland-Jørgensen
The BSD code did not propagate the OS-level IFF_MULTICAST flag to the
Bird-internal IF_MULTICAST flag, which causes problems with Wireguard
interfaces on FreeBSD. The Linux sysdep code does propagate the flag
already, so just copy over the same check and flag update.

Tested-by: Stefan Haller 
Signed-off-by: Toke Høiland-Jørgensen 
---
 sysdep/bsd/krt-sock.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sysdep/bsd/krt-sock.c b/sysdep/bsd/krt-sock.c
index c2faa23dd44f..cd89544063c7 100644
--- a/sysdep/bsd/krt-sock.c
+++ b/sysdep/bsd/krt-sock.c
@@ -665,6 +665,9 @@ krt_read_ifinfo(struct ks_msg *msg, int scan)
   else
 f.flags |= IF_MULTIACCESS;  /* NBMA */
 
+  if (fl & IFF_MULTICAST)
+f.flags |= IF_MULTICAST;
+
   iface = if_update();
 
   if (!scan)
-- 
2.31.1



Re: FreeBSD if_wg POINTTOPOINT and MULTICAST behaviour

2021-04-19 Thread Toke Høiland-Jørgensen
Stefan Haller  writes:

> On Mon, Apr 19, 2021 at 01:42:58PM -0600, Jason A. Donenfeld wrote:
>> On Mon, Apr 19, 2021 at 1:42 PM Stefan Haller  wrote:
>> >
>> > On Mon, Apr 19, 2021 at 08:25:46PM +0200, Toke Høiland-Jørgensen wrote:
>> > > Stefan, any chance you could test this patch to Bird (*instead of* the
>> > > previous one that removes the check from the Babel code)?
>> >
>> > The patch is working on FreeBSD 13.0.
>> 
>> Just FYI, the previous patch was added to ports, so I wanted to double
>> check that you removed that previous patch before adding this one...
>
> Yes, I did remove the old patch (in proto/babel/babel.c):
>
>> root@fbsd:/usr/ports/net/bird2 # git status .
>> On branch main
>> Changes not staged for commit:
>>   (use "git add/rm ..." to update what will be committed)
>>   (use "git restore ..." to discard changes in working directory)
>> deleted:files/patch-proto_babel_babel.c
>> 
>> Untracked files:
>>   (use "git add ..." to include in what will be committed)
>> files/patch-sysdep_bsd_krt-sock.c

Awesome! Thank you for testing! :)

-Toke


Re: FreeBSD if_wg POINTTOPOINT and MULTICAST behaviour

2021-04-19 Thread Stefan Haller
On Mon, Apr 19, 2021 at 01:42:58PM -0600, Jason A. Donenfeld wrote:
> On Mon, Apr 19, 2021 at 1:42 PM Stefan Haller  wrote:
> >
> > On Mon, Apr 19, 2021 at 08:25:46PM +0200, Toke Høiland-Jørgensen wrote:
> > > Stefan, any chance you could test this patch to Bird (*instead of* the
> > > previous one that removes the check from the Babel code)?
> >
> > The patch is working on FreeBSD 13.0.
> 
> Just FYI, the previous patch was added to ports, so I wanted to double
> check that you removed that previous patch before adding this one...

Yes, I did remove the old patch (in proto/babel/babel.c):

> root@fbsd:/usr/ports/net/bird2 # git status .
> On branch main
> Changes not staged for commit:
>   (use "git add/rm ..." to update what will be committed)
>   (use "git restore ..." to discard changes in working directory)
> deleted:files/patch-proto_babel_babel.c
> 
> Untracked files:
>   (use "git add ..." to include in what will be committed)
> files/patch-sysdep_bsd_krt-sock.c

Kind regards,
Stefan


Re: FreeBSD if_wg POINTTOPOINT and MULTICAST behaviour

2021-04-19 Thread Jason A. Donenfeld
On Mon, Apr 19, 2021 at 1:42 PM Stefan Haller  wrote:
>
> On Mon, Apr 19, 2021 at 08:25:46PM +0200, Toke Høiland-Jørgensen wrote:
> > Stefan, any chance you could test this patch to Bird (*instead of* the
> > previous one that removes the check from the Babel code)?
>
> The patch is working on FreeBSD 13.0.

Just FYI, the previous patch was added to ports, so I wanted to double
check that you removed that previous patch before adding this one...


Re: FreeBSD if_wg POINTTOPOINT and MULTICAST behaviour

2021-04-19 Thread Stefan Haller
On Mon, Apr 19, 2021 at 08:25:46PM +0200, Toke Høiland-Jørgensen wrote:
> Stefan, any chance you could test this patch to Bird (*instead of* the
> previous one that removes the check from the Babel code)?

The patch is working on FreeBSD 13.0.

Kind regards,
Stefan


Re: FreeBSD if_wg POINTTOPOINT and MULTICAST behaviour

2021-04-19 Thread Toke Høiland-Jørgensen
Toke Høiland-Jørgensen  writes:

> Stefan Haller  writes:
>
>> Hi Jason,
>>
>> On Thu, Apr 15, 2021 at 06:05:03PM -0600, Jason A. Donenfeld wrote:
>>> I spent the day playing around with bird and babel and sorted out
>>> FreeBSD's v6 situation. Basically, ff00::/8 addresses are treated
>>> differently, and they're blocked unless the interface sets
>>> IFF_MULTICAST. So I've committed
>>> https://git.zx2c4.com/wireguard-freebsd/commit/?id=a7a84a17faf784857f076e37aa4818f6b6c12a95
>>> to do this.
>>
>> That is also what I observed. Without IFF_MULTICAST I see the following
>> error in bird's log:
>>
>> bird[8045]: babel1: Socket error: IPV6_MULTICAST_IF: Can't assign requested 
>> address
>> bird[8045]: babel1: Cannot open socket for wg1
>>
>>> Stefan - please let me know if those work for you. In my testing thus
>>> far, things seem to work for me.
>>
>> After applying Toke's patch for bird and your Wireguard patch in
>> a7a84a17faf784 everything is working as before (with minor config
>> changes).
>>
>> Just for the record, my previous configuration looked like this (using
>> POINTTOPOINT interfaces, I use ifconfig to set the peer address):
>>
>>> [Interface]
>>> ...
>>> Address = fe80::5/64
>>> PostUp = ifconfig %i inet 169.254.42.5/32 169.254.42.2
>>
>> My new configuration without POINTTOPOINT, but only a single peer
>> directly attached to other side of the wg tunnel:
>>
>>> [Interface]
>>> ...
>>> Address = 169.254.42.5/32, fe80::5/64
>>> PostUp = route add 169.254.42.2 -iface %i
>>
>> So for me everything works as expected again. A big thanks to all of you for
>> figuring out what was going wrong and getting it fixed so quickly.
>
> Great! You're welcome :)

Stefan, any chance you could test this patch to Bird (*instead of* the
previous one that removes the check from the Babel code)?

-Toke

diff --git a/sysdep/bsd/krt-sock.c b/sysdep/bsd/krt-sock.c
index c2faa23dd44f..cd89544063c7 100644
--- a/sysdep/bsd/krt-sock.c
+++ b/sysdep/bsd/krt-sock.c
@@ -665,6 +665,9 @@ krt_read_ifinfo(struct ks_msg *msg, int scan)
   else
 f.flags |= IF_MULTIACCESS;  /* NBMA */
 
+  if (fl & IFF_MULTICAST)
+f.flags |= IF_MULTICAST;
+
   iface = if_update();
 
   if (!scan)


Re: [PATCH] babel: Drop check for IF_MULTICAST interface flag

2021-04-19 Thread Toke Høiland-Jørgensen
Ondrej Zajicek  writes:

> On Mon, Apr 19, 2021 at 03:55:18PM +0200, Toke Høiland-Jørgensen wrote:
>> Ondrej Zajicek  writes:
>> 
>> > Is there a reason why to disregard the IF_MULTICAST flag? This seems to me
>> > more like a bug in FreeBSD Wireguard implementation that should be fixed
>> > there. Is this flag properly checked on Linux, or is there some reason why
>> > the flag is missing?
>> 
>> We did fix Wireguard - see:
>> https://git.zx2c4.com/wireguard-freebsd/patch/?id=a7a84a17faf784857f076e37aa4818f6b6c12a95
>> 
>> However, that didn't help, Babel still refused to use the interface.
>> Looking at krt-sock.c, the IF_MULTICAST flag is only set on
>> IFF_POINTOPOINT or IFF_BROADCAST on bsd. The Linux code (in netlink.c)
>> has a further:
>> 
>>   if (fl & IFF_MULTICAST)
>>  f.flags |= IF_MULTICAST;
>> 
>> beneath the other flag checks, so maybe that's really what's missing on
>> the BSD side?
>
> Yes, it is likely that it is an issue in sysdep/bsd code.

Alright, I'll send a patch for that then :)

>> > Routing protocols in BIRD generally follow this flag (and perhaps use
>> > it to switch to unicast-only mode), so i do not see why Babel should
>> > behave differently.
>> 
>> Yeah, I do believe I originally copied that check from one of the other
>> protocols. I can see how it makes sense to check the flag and change
>> operation mode based on it, but given that Babel doesn't do that it just
>> seems kinda redundant? If the interface *actually* is unable to send
>> multicast packets, the subsequent socket operation is going to fail, and
>> at least that produces an error message instead of just silently
>> ignoring the interface like that flag check does :)
>
> Well, i am OK with generating a warning in cases of non-matching interface
> type, instead of ignoring it silently. (In contrast to iface down or missing
> lladdr, which should be silent, as it may correct later.)

OK, fine with me; I'll send an updated patch that adds a warning instead
of dropping the check...

-Toke


Re: [PATCH] babel: Drop check for IF_MULTICAST interface flag

2021-04-19 Thread Toke Høiland-Jørgensen
Ondrej Zajicek  writes:

> On Thu, Apr 15, 2021 at 03:44:50PM +0200, Toke Høiland-Jørgensen wrote:
>> The babel protocol code was checking interfaces for the IF_MULTICAST flag
>> and refusing to run if this isn't present. However, there are cases where
>> this flag doesn't correspond to the actual capability of sending multicast
>> packets. For instance, Wireguard interfaces on FreeBSD doesn't set the
>> required flags, but Babel will run just fine over such an interface given
>> the right configuration.
>
> Hi
>
> Is there a reason why to disregard the IF_MULTICAST flag? This seems to me
> more like a bug in FreeBSD Wireguard implementation that should be fixed
> there. Is this flag properly checked on Linux, or is there some reason why
> the flag is missing?

We did fix Wireguard - see:
https://git.zx2c4.com/wireguard-freebsd/patch/?id=a7a84a17faf784857f076e37aa4818f6b6c12a95

However, that didn't help, Babel still refused to use the interface.
Looking at krt-sock.c, the IF_MULTICAST flag is only set on
IFF_POINTOPOINT or IFF_BROADCAST on bsd. The Linux code (in netlink.c)
has a further:

  if (fl & IFF_MULTICAST)
f.flags |= IF_MULTICAST;

beneath the other flag checks, so maybe that's really what's missing on
the BSD side?

> Routing protocols in BIRD generally follow this flag (and perhaps use
> it to switch to unicast-only mode), so i do not see why Babel should
> behave differently.

Yeah, I do believe I originally copied that check from one of the other
protocols. I can see how it makes sense to check the flag and change
operation mode based on it, but given that Babel doesn't do that it just
seems kinda redundant? If the interface *actually* is unable to send
multicast packets, the subsequent socket operation is going to fail, and
at least that produces an error message instead of just silently
ignoring the interface like that flag check does :)

-Toke