[Openvpn-devel,2/5] xmit_hold is only required for port_share

Message ID 20201023113431.26691-2-arne@rfc2549.org
State New
Headers show
Series
  • [Openvpn-devel,1/5] Inline function tls_get_peer_info
Related show

Commit Message

Arne Schwabe Oct. 23, 2020, 11:34 a.m.
Make options.c only set xmit_hold when port_share is active to least
document this dependency. I have not actually tested if this dependency
is actually true (or if port_share could work without xmit_hold).

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/init.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Gert Doering Oct. 24, 2020, 7:43 p.m. | #1
Hi,

On Fri, Oct 23, 2020 at 01:34:28PM +0200, Arne Schwabe wrote:
> Make options.c only set xmit_hold when port_share is active to least
> document this dependency. I have not actually tested if this dependency
> is actually true (or if port_share could work without xmit_hold).

I'm not sure I understand this patch, or the flow.  Before this patch,
xmit_hold is always set to "true" for TCP_SERVER, after this patch,
only for server *and* port_share.

What am I overlooking?

gert
Arne Schwabe Oct. 26, 2020, 9:49 a.m. | #2
Am 24.10.20 um 21:43 schrieb Gert Doering:
> Hi,
> 
> On Fri, Oct 23, 2020 at 01:34:28PM +0200, Arne Schwabe wrote:
>> Make options.c only set xmit_hold when port_share is active to least
>> document this dependency. I have not actually tested if this dependency
>> is actually true (or if port_share could work without xmit_hold).
> 
> I'm not sure I understand this patch, or the flow.  Before this patch,
> xmit_hold is always set to "true" for TCP_SERVER, after this patch,
> only for server *and* port_share.
> 
> What am I overlooking?
> 

No. This feature was introduced as part of port-share and I am not even
sure it is really required for port-share. Normally we do not send
anything unless there is a valid OpenVPN RESET packet. So I want to
limit it again to part-share before I can really reliable say that even
port-share doesn't need it.

Arne

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 034edba0..fdd9a6cc 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2908,12 +2908,15 @@  do_init_crypto_tls(struct context *c, const unsigned int flags)
         to.push_peer_info_detail = 0;
     }
 
+#ifdef ENABLE_PORT_SHARE
     /* should we not xmit any packets until we get an initial
      * response from client? */
-    if (to.server && options->ce.proto == PROTO_TCP_SERVER)
+    if (to.server && options->ce.proto == PROTO_TCP_SERVER
+        && options->port_share_host)
     {
         to.xmit_hold = true;
     }
+#endif
 
     to.disable_occ = !options->occ;