[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1055966: bookworm-pu: package openvpn-dco-dkms/0.0+git20230324-1+deb12u1 (or 0.0+git20231103-0+deb12u1?)



Package: release.debian.org
Severity: normal
Tags: bookworm
User: release.debian.org@packages.debian.org
Usertags: pu
X-Debbugs-Cc: openvpn-dco-dkms@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 <berni@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 <antonio@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 <david@openvpn.net>
+
+Signed-off-by: Antonio Quartulli <antonio@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 <antonio@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 <berni@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 <berni@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 <berni@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 to: