Modify

Opened 7 years ago

Closed 7 years ago

#1178 closed enhancement (fixed)

Provide configuration for OpenVPN Max Clients

Reported by: Jon "The Nice Guy" Spriggs <jon@…> Owned by:
Priority: normal Milestone: Firmware 2.3.7.0
Component: fon-plugin-openvpn Version: N/A
Severity: minor
Cc: Hardware: both

Description

This change permits the configuration of the maximum number of OpenVPN clients.

This needs to be added to the following file: /luci/applications/luci-openvpn/luasrc/model/cbi/openvpn/firewall.lua

local x = s:option(ListValue, "max_clients", translate("openvpn_max_clients", "Configure Maximum Concurrent VPN Connections"))
x:value("1", "1")
x:value("2", translate("openvpn_max_clients_default", "2 (default)"))
x:value("3", "3")
x:value("4", "4")
x:value("5", "5")
x:value("6", "6")
x:value("7", "7")
x:value("8", "8")
x:value("9", "9")
x:value("10", "10")
x.default = require("luci.model.uci").cursor():get("openvpn", "openvpn", "max_clients") or "2"

I'm away from my usual dev environment, but I was prompted by another ticket being followed up, and it was mentioned that 2.3.7.0 was having some OpenVPN changes applied to it, so I thought it was worthwhile to get this in while they are being reviewed :)

Attachments (0)

Change History (8)

comment:1 Changed 7 years ago by matthijs

  • Milestone set to Firmware 2.3.7.0
  • Severity changed from unknown to minor
  • Status changed from new to confirmed
  • Version changed from 2.3.7.0 beta1 to N/A

Yipes, do we ship with a max_clients of 2 by default... Hmm, makes sense to make it modifiable, indeed.

Two things regarding the patch: I don't think it's useful to explicitly append the "(default)" to the entry for "2", since that entry will be selected automatically when people first look at the settings page. Adding the extra test only makes translation more difficult and makes it near impossible to change the default later on. The x.default should also not be needed, since the openvpn config file is shipped with a default value of "2" already.

The second point is, perhaps you could autogenerate the list of 1-10 instead of spelling them out like this? For an example, see the QoS application. Perhaps it also makes sense to add a very big number as well (e.g., 100 or something), to act as "unlimited" (it seems OpenVPN doesn't have an unlimited option an defaults to 1024 when the option is not given...).

Could you update the code and test if it still works then?

comment:2 Changed 7 years ago by anonymous

local x = s:option(ListValue, "max_clients", translate("openvpn_max_clients", "Configure Maximum Concurrent VPN Connections"))
for i = 1, 10 do
        x:value(i, i.." clients")
end
for i = 15, 30, 5 do
        x:value(i, i.." clients")
end
x:value(1024, "Unlimited")
x.default = require("luci.model.uci").cursor():get("openvpn", "openvpn", "max_clients")

Works.

comment:3 Changed 7 years ago by Jon "The Nice Guy" Spriggs <jon@…>

Oops, the last update was by me :)

comment:4 follow-up: Changed 7 years ago by Jon "The Nice Guy" Spriggs <jon@…>

Having accessed that this morning, it looked pretty messy, so here's the tidied version :)

local x = s:option(ListValue, "max_clients", translate("openvpn_max_clients", "Concurrent Connections"))
x:value('', translate("openvpn_max_clients_unlimited", "Unlimited"))
for i = 1, 5 do
        x:value(i, i)
end
for i = 10, 30, 10 do
        x:value(i, i)
end
x.default = require("luci.model.uci").cursor():get("openvpn", "openvpn", "max_clients")

comment:5 in reply to: ↑ 4 ; follow-up: Changed 7 years ago by Jon "The Nice Guy" Spriggs <jon@…>

Further to the discussion in #1180, consider this revised version. This supports up to /24's worth of /30 networks. It steps up in binary jumps (1, 10, 100, etc)

local x = s:option(ListValue, "max_clients", translate("openvpn_max_clients", "Concurrent Connections"))
x:value(1, 1)
x:value(2, 2)
x:value(4, 4)
x:value(8, 8)
x:value(16, 16)
x:value(32, 32)
x:value(64, 64)
x.default = require("luci.model.uci").cursor():get("openvpn", "openvpn", "max_clients") or 2

comment:6 in reply to: ↑ 5 Changed 7 years ago by Jon "The Nice Guy" Spriggs <jon@…>

Thinking through it, this makes a little more sense - especially as we're talking about having 2x/24's

local x = s:option(ListValue, "max_clients", translate("openvpn_max_clients", "Concurrent Connections"))
x:value(2, 2 .. translate("openvpn_static_or_dynamic_clients", " static or dynamic clients"))
x:value(4, 4 .. translate("openvpn_static_or_dynamic_clients", " static or dynamic clients"))
x:value(8, 8 .. translate("openvpn_static_or_dynamic_clients", " static or dynamic clients"))
x:value(16, 16 .. translate("openvpn_static_or_dynamic_clients", " static or dynamic clients"))
x:value(32, 32 .. translate("openvpn_static_or_dynamic_clients", " static or dynamic clients"))
x:value(64, 64 .. translate("openvpn_static_or_dynamic_clients", " static or dynamic clients"))
x:value(128, translate("openvpn_64_static_and_dynamic_clients", "64 static clients and 64 dynamic clients"))
x.default = require("luci.model.uci").cursor():get("openvpn", "openvpn", "max_clients") or 2

comment:7 Changed 7 years ago by matthijs

  • Status changed from confirmed to testing-fix

I'll be using your next-to-last patch, going up to 64, since I think the last patch actually makes things more confusing for not-so-advanced users that don't want to mess with static address assignments. Also, I doubt that the Fonera will actually be able to handle (more than) 64 concurrent clients, so I'd say 64 is big enough (and real power users can always edit luci directly if they really want).

I'll push out this patch to SVN this week.

comment:8 Changed 7 years ago by matthijs

  • Resolution set to fixed
  • Status changed from testing-fix to closed

(In [2175]) luci-openvpn: Allow changing max_clients setting.

The maximum number is 64, which is exactly the number of /30 IP blocks fitting in the /24 network used (though this is probably a lot more than the Foneras are actually capable of handling).

Thanks to Jon Spriggs for this patch.

Closes: #1178

Add Comment

Modify Ticket

Action
as closed The ticket will remain with no owner.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.