Package: release.debian.org
Severity: normal
Tags: bookworm
User: release.debian....@packages.debian.org
Usertags: pu
X-Debbugs-Cc: openvpn-dco-d...@packages.debian.org
Control: affects -1 + src:openvpn-dco-dkms

[ Reason ]
openvpn-dco-dkms packages an accelerator kernel module for OpenVPN (OpenVPN
data channel offload). There is one annoying bug tracked as Bug#1055809 where on
heavily loaded TCP servers a refcount issue might occur and the module will
become unusable.

The bug has been fixed upstream. The new upstream snapshot is already in sid.

Apart from that change there is a fix for compilation with Kernel >= 6.5 that
we might want to have in stable for backport kernels. Tracked as Bug#1043116.
That patch could be backported, but needs some fixup.

Apart from that there are only changes for building with RHEL9, which is a noop
on Debian.

https://github.com/OpenVPN/ovpn-dco/compare/1c2c84e99d0c1513db38ac7a3858f21229906fd7..master

Backporting the build fix would actually make the diff larger than the new
upstream version and harder to maintain, so I'm proposing two options:

a) backport only the fix for Bug#1055809 and upload as
openvpn-dco-dkms/0.0+git20230324-1+deb12u1

 changelog                            |    9 ++++++++
 gbp.conf                             |    2 +
 patches/fix-refcount-imbalance.patch |   38 +++++++++++++++++++++++++++++++++++
 patches/series                       |    1 
 4 files changed, 50 insertions(+)

b) upload the new upstream snapshot as 0.0+git20231103-0+deb12u1, fixing the
build error on kernel 6.5

 Makefile                    |   13 ++++++++++---
 compat-include/net/gso.h    |   20 ++++++++++++++++++++
 debian/changelog            |   21 +++++++++++++++++++++
 debian/rules                |    2 +-
 drivers/net/ovpn-dco/ovpn.c |    1 +
 drivers/net/ovpn-dco/tcp.c  |   14 +++++++++++---
 linux-compat.h              |    4 ++--
 7 files changed, 66 insertions(+), 9 deletions(-)

[ Impact ]
If neither update is allowed the kernel module will hang on busy servers

If we only fix the refcount imbalance the module will not build on future
backports kernels. Backporting that fix as well would be possible, but actually
the diff would be larger and we cannot be sure whether new fixes would apply
cleanly.

[ Tests ]
The code fix is trivial enough and has been verified upstream. The new module
is currently running on my employers eduVPN server.

[ Risks ]
The changes are pretty trivial and the vast majority of them is a noop on
Bookworm or Debian as a whole

[ Checklist ]
  [X] *all* changes are documented in the d/changelog (for the minimal version)
  [X] I reviewed all changes and I approve them
  [X] attach debdiff against the package in (old)stable
  [X] the issue is verified as fixed in unstable

[ Changes ]
See the explaination above for the minimal version and the github diff link for
the new upstream version
diffstat for openvpn-dco-dkms-0.0+git20230324 openvpn-dco-dkms-0.0+git20230324

 changelog                            |    9 ++++++++
 gbp.conf                             |    2 +
 patches/fix-refcount-imbalance.patch |   38 +++++++++++++++++++++++++++++++++++
 patches/series                       |    1 
 4 files changed, 50 insertions(+)

diff -Nru openvpn-dco-dkms-0.0+git20230324/debian/changelog 
openvpn-dco-dkms-0.0+git20230324/debian/changelog
--- openvpn-dco-dkms-0.0+git20230324/debian/changelog   2023-04-13 
09:47:41.000000000 +0200
+++ openvpn-dco-dkms-0.0+git20230324/debian/changelog   2023-11-14 
22:18:17.000000000 +0100
@@ -1,3 +1,12 @@
+openvpn-dco-dkms (0.0+git20230324-1+deb12u1) bookworm; urgency=medium
+
+  * Import upstream patch to fix refcount imbalance (Closes: 1055809)
+    - fixes "waiting for tunxxx to become free" seen on heavy loaded TCP
+      servers
+  * Add d/gbp.conf for debian/bookworm branch
+
+ -- Bernhard Schmidt <be...@debian.org>  Tue, 14 Nov 2023 22:18:17 +0100
+
 openvpn-dco-dkms (0.0+git20230324-1) unstable; urgency=medium
 
   * Release to unstable targetting bookworm.
diff -Nru openvpn-dco-dkms-0.0+git20230324/debian/gbp.conf 
openvpn-dco-dkms-0.0+git20230324/debian/gbp.conf
--- openvpn-dco-dkms-0.0+git20230324/debian/gbp.conf    1970-01-01 
01:00:00.000000000 +0100
+++ openvpn-dco-dkms-0.0+git20230324/debian/gbp.conf    2023-11-14 
22:18:17.000000000 +0100
@@ -0,0 +1,2 @@
+[DEFAULT]
+debian-branch=debian/bookworm
diff -Nru 
openvpn-dco-dkms-0.0+git20230324/debian/patches/fix-refcount-imbalance.patch 
openvpn-dco-dkms-0.0+git20230324/debian/patches/fix-refcount-imbalance.patch
--- 
openvpn-dco-dkms-0.0+git20230324/debian/patches/fix-refcount-imbalance.patch    
    1970-01-01 01:00:00.000000000 +0100
+++ 
openvpn-dco-dkms-0.0+git20230324/debian/patches/fix-refcount-imbalance.patch    
    2023-11-14 22:18:17.000000000 +0100
@@ -0,0 +1,38 @@
+From 7b7a28fcb0c244c7182922f7c83cb09bcd840c84 Mon Sep 17 00:00:00 2001
+From: Antonio Quartulli <anto...@openvpn.net>
+Date: Wed, 1 Nov 2023 23:49:39 +0100
+Subject: [PATCH] ovpn-dco: fix refcount imbalance upon RX in case of full ring
+
+When the RX ring is full ovpn_recv() will stop and return -ENOSPC.
+At this point the incoming packet is dropped.
+
+However, a reference to the peer object is obtained right before
+invoking ovpn_recv() and failing to drop it will result in a reference
+imbalance.
+
+Make sure to drop reference in case of ring being full and prevent
+imbalance.
+
+NOTE: this problem only existed in the TCP codepath
+
+Fix suggested by David Lam <da...@openvpn.net>
+
+Signed-off-by: Antonio Quartulli <anto...@openvpn.net>
+---
+ drivers/net/ovpn-dco/tcp.c | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/drivers/net/ovpn-dco/tcp.c b/drivers/net/ovpn-dco/tcp.c
+index 288a691..8479c19 100644
+--- a/drivers/net/ovpn-dco/tcp.c
++++ b/drivers/net/ovpn-dco/tcp.c
+@@ -106,6 +106,9 @@ static int ovpn_tcp_read_sock(read_descriptor_t *desc, 
struct sk_buff *in_skb,
+                               /* hold reference to peer as requird by 
ovpn_recv() */
+                               ovpn_peer_hold(peer);
+                               status = ovpn_recv(peer->ovpn, peer, 
peer->tcp.skb);
++                              if (unlikely(status < 0))
++                                      ovpn_peer_put(peer);
++
+                       } else {
+                               /* prepend skb with packet len. this way 
userspace can parse
+                                * the packet as if it just arrived from the 
remote endpoint
diff -Nru openvpn-dco-dkms-0.0+git20230324/debian/patches/series 
openvpn-dco-dkms-0.0+git20230324/debian/patches/series
--- openvpn-dco-dkms-0.0+git20230324/debian/patches/series      1970-01-01 
01:00:00.000000000 +0100
+++ openvpn-dco-dkms-0.0+git20230324/debian/patches/series      2023-11-14 
22:18:17.000000000 +0100
@@ -0,0 +1 @@
+fix-refcount-imbalance.patch
diffstat for openvpn-dco-dkms-0.0+git20230324 openvpn-dco-dkms-0.0+git20231103

 Makefile                    |   13 ++++++++++---
 compat-include/net/gso.h    |   20 ++++++++++++++++++++
 debian/changelog            |   21 +++++++++++++++++++++
 debian/rules                |    2 +-
 drivers/net/ovpn-dco/ovpn.c |    1 +
 drivers/net/ovpn-dco/tcp.c  |   14 +++++++++++---
 linux-compat.h              |    4 ++--
 7 files changed, 66 insertions(+), 9 deletions(-)

diff -Nru openvpn-dco-dkms-0.0+git20230324/compat-include/net/gso.h 
openvpn-dco-dkms-0.0+git20231103/compat-include/net/gso.h
--- openvpn-dco-dkms-0.0+git20230324/compat-include/net/gso.h   1970-01-01 
01:00:00.000000000 +0100
+++ openvpn-dco-dkms-0.0+git20231103/compat-include/net/gso.h   2023-11-11 
22:20:11.000000000 +0100
@@ -0,0 +1,20 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* OpenVPN data channel accelerator
+ *
+ *  Copyright (C) 2023 OpenVPN, Inc.
+ *
+ *  Author:    Antonio Quartulli <anto...@openvpn.net>
+ */
+
+#ifndef _NET_OVPN_COMPAT_NET_GSO_H
+#define _NET_OVPN_COMPAT_NET_GSO_H
+
+#include <linux/version.h>
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(6, 4, 10)
+#include_next <net/gso.h>
+#else
+#include <linux/netdevice.h>
+#endif
+
+#endif /* _NET_OVPN_COMPAT_NET_GSO_H */
diff -Nru openvpn-dco-dkms-0.0+git20230324/debian/changelog 
openvpn-dco-dkms-0.0+git20231103/debian/changelog
--- openvpn-dco-dkms-0.0+git20230324/debian/changelog   2023-04-13 
09:47:41.000000000 +0200
+++ openvpn-dco-dkms-0.0+git20231103/debian/changelog   2023-11-11 
22:20:21.000000000 +0100
@@ -1,3 +1,24 @@
+openvpn-dco-dkms (0.0+git20231103-1) unstable; urgency=medium
+
+  * New upstream version 0.0+git20231103
+    - fixes refcount imbalance ("waiting for tunxxx to become free") seen
+      on heavy loaded TCP servers
+
+ -- Bernhard Schmidt <be...@debian.org>  Sat, 11 Nov 2023 22:20:21 +0100
+
+openvpn-dco-dkms (0.0+git20230816-2) unstable; urgency=medium
+
+  * Install compat-include directory (Closes: #1050211)
+
+ -- Bernhard Schmidt <be...@debian.org>  Tue, 22 Aug 2023 18:00:24 +0200
+
+openvpn-dco-dkms (0.0+git20230816-1) unstable; urgency=medium
+
+  * New upstream version 0.0+git20230816
+    - fix build error on kernel 6.5+ (Closes: #1043116)
+
+ -- Bernhard Schmidt <be...@debian.org>  Mon, 21 Aug 2023 22:51:04 +0200
+
 openvpn-dco-dkms (0.0+git20230324-1) unstable; urgency=medium
 
   * Release to unstable targetting bookworm.
diff -Nru openvpn-dco-dkms-0.0+git20230324/debian/rules 
openvpn-dco-dkms-0.0+git20231103/debian/rules
--- openvpn-dco-dkms-0.0+git20230324/debian/rules       2023-04-13 
09:47:41.000000000 +0200
+++ openvpn-dco-dkms-0.0+git20231103/debian/rules       2023-11-11 
22:20:21.000000000 +0100
@@ -18,7 +18,7 @@
 override_dh_auto_build:
 override_dh_auto_install:
        mkdir -p ${INSTALLDIR}
-       for f in drivers include linux-compat.h Makefile 
gen-compat-autoconf.sh; do \
+       for f in compat-include drivers include linux-compat.h Makefile 
gen-compat-autoconf.sh; do \
                cp -a $$f ${INSTALLDIR}; \
        done
 
diff -Nru openvpn-dco-dkms-0.0+git20230324/drivers/net/ovpn-dco/ovpn.c 
openvpn-dco-dkms-0.0+git20231103/drivers/net/ovpn-dco/ovpn.c
--- openvpn-dco-dkms-0.0+git20230324/drivers/net/ovpn-dco/ovpn.c        
2023-03-25 00:00:30.000000000 +0100
+++ openvpn-dco-dkms-0.0+git20231103/drivers/net/ovpn-dco/ovpn.c        
2023-11-11 22:20:11.000000000 +0100
@@ -22,6 +22,7 @@
 #include "udp.h"
 
 #include <linux/workqueue.h>
+#include <net/gso.h>
 #include <uapi/linux/if_ether.h>
 
 static const unsigned char ovpn_keepalive_message[] = {
diff -Nru openvpn-dco-dkms-0.0+git20230324/drivers/net/ovpn-dco/tcp.c 
openvpn-dco-dkms-0.0+git20231103/drivers/net/ovpn-dco/tcp.c
--- openvpn-dco-dkms-0.0+git20230324/drivers/net/ovpn-dco/tcp.c 2023-03-25 
00:00:30.000000000 +0100
+++ openvpn-dco-dkms-0.0+git20231103/drivers/net/ovpn-dco/tcp.c 2023-11-11 
22:20:11.000000000 +0100
@@ -103,9 +103,17 @@
                         * Queue skb for sending to userspace via recvmsg on 
the socket
                         */
                        if (likely(ovpn_opcode_from_skb(peer->tcp.skb, 0) == 
OVPN_DATA_V2)) {
-                               /* hold reference to peer as requird by 
ovpn_recv() */
-                               ovpn_peer_hold(peer);
+                               /* hold reference to peer as required by 
ovpn_recv().
+                                *
+                                * NOTE: in this context we should already be 
holding a
+                                * reference to this peer, therefore 
ovpn_peer_hold() is
+                                * not expected to fail
+                                */
+                               WARN_ON(!ovpn_peer_hold(peer));
                                status = ovpn_recv(peer->ovpn, peer, 
peer->tcp.skb);
+                               if (unlikely(status < 0))
+                                       ovpn_peer_put(peer);
+
                        } else {
                                /* prepend skb with packet len. this way 
userspace can parse
                                 * the packet as if it just arrived from the 
remote endpoint
@@ -169,7 +177,7 @@
 }
 
 static bool ovpn_tcp_sock_is_readable(
-#if LINUX_VERSION_CODE < KERNEL_VERSION(5, 15, 0)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(5, 15, 0) && !defined(EL9)
                                      const struct sock *sk
 #else
                                      struct sock *sk
diff -Nru openvpn-dco-dkms-0.0+git20230324/linux-compat.h 
openvpn-dco-dkms-0.0+git20231103/linux-compat.h
--- openvpn-dco-dkms-0.0+git20230324/linux-compat.h     2023-03-25 
00:00:30.000000000 +0100
+++ openvpn-dco-dkms-0.0+git20231103/linux-compat.h     2023-11-11 
22:20:11.000000000 +0100
@@ -64,13 +64,13 @@
 
 #endif /* LINUX_VERSION_CODE < KERNEL_VERSION(5, 11, 0) && !defined(EL8) */
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(5, 10, 0)
+#if LINUX_VERSION_CODE < KERNEL_VERSION(5, 10, 0) && !defined(EL8)
 
 #define genl_small_ops genl_ops
 #define small_ops ops
 #define n_small_ops n_ops
 
-#endif /* LINUX_VERSION_CODE < KERNEL_VERSION(5, 10, 0) */
+#endif /* LINUX_VERSION_CODE < KERNEL_VERSION(5, 10, 0) && !defined(EL8) */
 
 #if LINUX_VERSION_CODE < KERNEL_VERSION(5, 10, 0) && !defined(EL8)
 
diff -Nru openvpn-dco-dkms-0.0+git20230324/Makefile 
openvpn-dco-dkms-0.0+git20231103/Makefile
--- openvpn-dco-dkms-0.0+git20230324/Makefile   2023-03-25 00:00:30.000000000 
+0100
+++ openvpn-dco-dkms-0.0+git20231103/Makefile   2023-11-11 22:20:11.000000000 
+0100
@@ -24,11 +24,18 @@
 EL8FLAG := -DEL8
 endif
 
+EL9 := $(shell cat /etc/redhat-release 2>/dev/null | grep -c " 9." )
+ifeq (1, $(EL9))
+EL9FLAG := -DEL9
+endif
+
+ELFLAG := $(EL8FLAG) $(EL9FLAG)
+
 NOSTDINC_FLAGS += \
        -I$(PWD)/include/ \
-       $(CFLAGS) $(EL8FLAG) \
-       -include $(PWD)/linux-compat.h
-#      -I$(PWD)/compat-include/
+       $(CFLAGS) $(ELFLAG) \
+       -include $(PWD)/linux-compat.h \
+       -I$(PWD)/compat-include/
 
 ifneq ($(REVISION),)
 NOSTDINC_FLAGS += -DOVPN_DCO_VERSION=\"$(REVISION)\"

Reply via email to