diff mbox series

[Openvpn-devel] dco: define OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT on FreeBSD

Message ID 20230303110511.9569-1-kprovost@netgate.com
State Accepted
Headers
Series [Openvpn-devel] dco: define OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT on FreeBSD |

Commit Message

Kristof Provost March 3, 2023, 11:05 a.m. UTC 
  From: Kristof Provost <kp@FreeBSD.org>

FreeBSD's if_ovpn will never emit this as a peer deletion reason
(because it doesn't support TCP), but this allows us to align the
defines between Linux and FreeBSD, and remove a Linux-specific case from
process_incoming_del_peer().
---
 src/openvpn/dco_freebsd.h | 1 +
 src/openvpn/multi.c       | 3 ---
 2 files changed, 1 insertion(+), 3 deletions(-)

Comments

Antonio Quartulli March 3, 2023, 11:27 a.m. UTC | #1 
Hi,

On 03/03/2023 12:05, Kristof Provost via Openvpn-devel wrote:
> From: Kristof Provost <kp@FreeBSD.org>
> 
> FreeBSD's if_ovpn will never emit this as a peer deletion reason
> (because it doesn't support TCP), but this allows us to align the
> defines between Linux and FreeBSD, and remove a Linux-specific case from
> process_incoming_del_peer().

SoB missing

> ---
>   src/openvpn/dco_freebsd.h | 1 +
>   src/openvpn/multi.c       | 3 ---
>   2 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/src/openvpn/dco_freebsd.h b/src/openvpn/dco_freebsd.h
> index 2e35f3ac..970beca0 100644
> --- a/src/openvpn/dco_freebsd.h
> +++ b/src/openvpn/dco_freebsd.h
> @@ -41,6 +41,7 @@ enum ovpn_del_reason_t {
>       OVPN_DEL_PEER_REASON_EXPIRED,
>       OVPN_DEL_PEER_REASON_TRANSPORT_ERROR,
>       OVPN_DEL_PEER_REASON_USERSPACE,
> +    OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT,
>   };
>   
>   typedef struct dco_context {
> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
> index f2559016..99123c39 100644
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -3244,12 +3244,9 @@ process_incoming_del_peer(struct multi_context *m, struct multi_instance *mi,
>               reason = "ovpn-dco: transport error";
>               break;
>   
> -#ifdef TARGET_LINUX
> -        /* FIXME: this is linux-only today and breaks FreeBSD compilation */
>           case OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT:
>               reason = "ovpn-dco: transport disconnected";
>               break;
> -#endif
>   
>           case OVPN_DEL_PEER_REASON_USERSPACE:
>               /* We assume that is ourselves. Unfortunately, sometimes these
Antonio Quartulli March 3, 2023, 11:48 a.m. UTC | #2 
Hi,

On 03/03/2023 12:27, Antonio Quartulli wrote:
> Hi,
> 
> On 03/03/2023 12:05, Kristof Provost via Openvpn-devel wrote:
>> From: Kristof Provost <kp@FreeBSD.org>
>>
>> FreeBSD's if_ovpn will never emit this as a peer deletion reason
>> (because it doesn't support TCP), but this allows us to align the
>> defines between Linux and FreeBSD, and remove a Linux-specific case from
>> process_incoming_del_peer().
> 
> SoB missing

Sorry for being very condensed, however the patch looks good and gets my 
ACK:

Acked-by: Antonio Quartulli <a@unstable.cc>

I normally put my ACK under the SoB, so when I couldn't find it, my 
brain just threw that exception :-P

Cheers,

> 
>> ---
>>   src/openvpn/dco_freebsd.h | 1 +
>>   src/openvpn/multi.c       | 3 ---
>>   2 files changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/src/openvpn/dco_freebsd.h b/src/openvpn/dco_freebsd.h
>> index 2e35f3ac..970beca0 100644
>> --- a/src/openvpn/dco_freebsd.h
>> +++ b/src/openvpn/dco_freebsd.h
>> @@ -41,6 +41,7 @@ enum ovpn_del_reason_t {
>>       OVPN_DEL_PEER_REASON_EXPIRED,
>>       OVPN_DEL_PEER_REASON_TRANSPORT_ERROR,
>>       OVPN_DEL_PEER_REASON_USERSPACE,
>> +    OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT,
>>   };
>>   typedef struct dco_context {
>> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
>> index f2559016..99123c39 100644
>> --- a/src/openvpn/multi.c
>> +++ b/src/openvpn/multi.c
>> @@ -3244,12 +3244,9 @@ process_incoming_del_peer(struct multi_context 
>> *m, struct multi_instance *mi,
>>               reason = "ovpn-dco: transport error";
>>               break;
>> -#ifdef TARGET_LINUX
>> -        /* FIXME: this is linux-only today and breaks FreeBSD 
>> compilation */
>>           case OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT:
>>               reason = "ovpn-dco: transport disconnected";
>>               break;
>> -#endif
>>           case OVPN_DEL_PEER_REASON_USERSPACE:
>>               /* We assume that is ourselves. Unfortunately, sometimes 
>> these
>
Kristof Provost March 3, 2023, 11:58 a.m. UTC | #3 
On 3 Mar 2023, at 12:48, Antonio Quartulli wrote:
> On 03/03/2023 12:27, Antonio Quartulli wrote:
>> Hi,
>>
>> On 03/03/2023 12:05, Kristof Provost via Openvpn-devel wrote:
>>> From: Kristof Provost <kp@FreeBSD.org>
>>>
>>> FreeBSD's if_ovpn will never emit this as a peer deletion reason
>>> (because it doesn't support TCP), but this allows us to align the
>>> defines between Linux and FreeBSD, and remove a Linux-specific case from
>>> process_incoming_del_peer().
>>
>> SoB missing
>
> Sorry for being very condensed, however the patch looks good and gets my ACK:
>
> Acked-by: Antonio Quartulli <a@unstable.cc>
>
> I normally put my ACK under the SoB, so when I couldn't find it, my brain just threw that exception :-P
>
Ah yes. Do you want me to send an update with the line included, or is this sufficient:

Signed-off-by: Kristof Provost <kprovost@netgate.com>

Kristof
Gert Doering March 3, 2023, 12:04 p.m. UTC | #4 
Hi,

On Fri, Mar 03, 2023 at 12:58:55PM +0100, Kristof Provost via Openvpn-devel wrote:
> Ah yes. Do you want me to send an update with the line included, or is this sufficient:

Sufficient, thanks :)

gert
Gert Doering March 3, 2023, 4:16 p.m. UTC | #5 
Thanks for the patch - I did the #ifdef variant "because I did not
want to touch FreeBSD enums without checking with you first" - but
this is a better way forward.  Client-side tested on FreeBSD 14 and 12 :-)

Your patch has been applied to the master and release/2.6 branch.

commit 155cf11531e619cf24b2aa9d0acb4ff834b2e8fa (master)
commit 7fdf3e7ad711e1583e960798848c7d7f94e1ad8a (release/2.6)
Author: Kristof Provost
Date:   Fri Mar 3 12:05:11 2023 +0100

     dco: define OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT on FreeBSD

     Signed-off-by: Kristof Provost <kprovost@netgate.com>
     Acked-by: Antonio Quartulli <a@unstable.cc>
     Message-Id: <20230303110511.9569-1-kprovost@netgate.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26324.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/dco_freebsd.h b/src/openvpn/dco_freebsd.h
index 2e35f3ac..970beca0 100644
--- a/src/openvpn/dco_freebsd.h
+++ b/src/openvpn/dco_freebsd.h
@@ -41,6 +41,7 @@  enum ovpn_del_reason_t {
     OVPN_DEL_PEER_REASON_EXPIRED,
     OVPN_DEL_PEER_REASON_TRANSPORT_ERROR,
     OVPN_DEL_PEER_REASON_USERSPACE,
+    OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT,
 };
 
 typedef struct dco_context {
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f2559016..99123c39 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3244,12 +3244,9 @@  process_incoming_del_peer(struct multi_context *m, struct multi_instance *mi,
             reason = "ovpn-dco: transport error";
             break;
 
-#ifdef TARGET_LINUX
-        /* FIXME: this is linux-only today and breaks FreeBSD compilation */
         case OVPN_DEL_PEER_REASON_TRANSPORT_DISCONNECT:
             reason = "ovpn-dco: transport disconnected";
             break;
-#endif
 
         case OVPN_DEL_PEER_REASON_USERSPACE:
             /* We assume that is ourselves. Unfortunately, sometimes these