Added port-share-max option

PATCH In review Farhan Ul Haq 2 participants 2 messages May 26, 2017 – May 29, 2017

Share & tools

Farhan Ul Haq

author

2017-05-26 17:21 UTC #0 lore raw

PATCH 1/1 Added port-share-max option

---
 src/openvpn/init.c    |  3 ++-
 src/openvpn/options.c |  9 +++++++++
 src/openvpn/options.h |  1 +
 src/openvpn/ps.c      | 54 +++++++++++++++++++++++++++++++++++++++++----------
 src/openvpn/ps.h      |  3 ++-
 5 files changed, 58 insertions(+), 12 deletions(-)

src/openvpn/init.c +2 -1

index 56bdbe3..5745556 100644
@@ -576,7 +576,8 @@ init_port_share (struct context *c)
       port_share = port_share_open (c->options.port_share_host,
 				    c->options.port_share_port,
 				    MAX_RW_SIZE_LINK (&c->c2.frame),
-				    c->options.port_share_journal_dir);
+				    c->options.port_share_journal_dir,
+				    c->options.port_share_max_concurrent_connections);
       if (port_share == NULL)
 	msg (M_FATAL, "Fatal error: Port sharing failed");
     }

src/openvpn/options.c +9 -0

index 79f861b..0d81479 100644
@@ -480,6 +480,7 @@ static const char usage_message[] =
   "--port-share host port [dir] : When run in TCP mode, proxy incoming HTTPS\n"
   "                  sessions to a web server at host:port.  dir specifies an\n"
   "                  optional directory to write origin IP:port data.\n"
+  "--port-share-max N : Max number of port-share concurrent connections\n"
 #endif
 #endif
   "\n"

@@ -878,6 +879,9 @@ init_options (struct options *o, const bool init_gc)
   }
 #endif /* WIN32 */
 #endif /* P2MP_SERVER */
+#if PORT_SHARE
+  o->port_share_max_concurrent_connections = 512;
+#endif
   o->allow_recursive_routing = false;
 }

@@ -5892,6 +5896,11 @@ add_option (struct options *options,
       options->port_share_port = port;
       options->port_share_journal_dir = p[3];
     }
+  else if (streq (p[0], "port-share-max") && p[1])
+    {
+      VERIFY_PERMISSION (OPT_P_GENERAL);
+      options->port_share_max_concurrent_connections = atoi (p[1]);
+    }
 #endif
   else if (streq (p[0], "client-to-client"))
     {

src/openvpn/options.h +1 -0

index e4235e9..c9b6dd8 100644
@@ -454,6 +454,7 @@ struct options
   char *port_share_host;
   int port_share_port;
   const char *port_share_journal_dir;
+  int port_share_max_concurrent_connections;
 #endif
 #endif

src/openvpn/ps.c +44 ~~-10~~

142 lines not on this page · raw message

src/openvpn/ps.h +2 -1

index 62af8b5..0c913bf 100644
@@ -46,7 +46,8 @@ extern struct port_share *port_share;
 struct port_share *port_share_open (const char *host,
 				    const int port,
 				    const int max_initial_buf,
-				    const char *journal_dir);
+				    const char *journal_dir,
+				    const int max_concurrent_connections);
 
 void port_share_close (struct port_share *ps);
 void port_share_abort (struct port_share *ps);

Signature · 1 line

2.11.0

Steffan Karger

2017-05-29 07:21 UTC #1 lore raw

Hi,

Welcome to the mailing list.

Quoted 1 line · Farhan Ul Haq

---

Well, that is a short commit message. Before we look into this patch, could you please supply a commit message that convinces why we would need this option? We already have a plethora of options, and don't want to add any more unless it is really necessary.

The commit message isn't just to make us understand, but also to provide context for future developers who are searching the git logs for reasons why something in the code is the way it is.

In general, I'd like to advise you to read this great blog post in detail before resubmitting:
https://chris.beams.io/posts/git-commit/

Furthermore, just glancing at the code, I noticed:
* this patch seems to be based on the release/2.3 branch, while
patches should be based on the master branch. (And we only accept bugfixes in the release/2.3 branch, so this is very unlikely to be backported to release/2.3.)
* this adds an options, but doesn't update the man page
* this doesn't update Changes.rst

Thanks,
-Steffan