| Message ID | 20210428111737.3611480-1-matthieu.baerts@tessares.net |
|---|---|
| State | Accepted, archived |
| Commit | e96c1cbd9dfabbf612eb2036ea953e767f4367b1 |
| Delegated to: | Matthieu Baerts |
| Headers | show |
| Series | [mptcp-next] mptcp: restrict values of 'enabled' sysctl | expand |
Hi Florian, On 28/04/2021 13:17, Matthieu Baerts wrote: > To avoid confusions, it seems better to parse this sysctl parameter as a > boolean. We use it as a boolean, no need to parse an integer and bring > confusions if we see a value different from 0 and 1, especially with > this parameter name: enabled. > > It seems fine to do this modification because the default value is 1 > (enabled). Then the only other interesting value to set is 0 (disabled). > All other values would not have changed the default behaviour. > > Suggested-by: Florian Westphal <fw@strlen.de> > Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> Thank you for your review (on IRC). Just applied with your ACK: - e96c1cbd9dfa: mptcp: restrict values of 'enabled' sysctl - Results: be556cd0eaa6..837feee36a25 Builds and tests are now in progress: https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210428T140248 https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210428T140248 Cheers, Matt
diff --git a/Documentation/networking/mptcp-sysctl.rst b/Documentation/networking/mptcp-sysctl.rst index 6af0196c4297..3b352e5f6300 100644 --- a/Documentation/networking/mptcp-sysctl.rst +++ b/Documentation/networking/mptcp-sysctl.rst @@ -7,13 +7,13 @@ MPTCP Sysfs variables /proc/sys/net/mptcp/* Variables =============================== -enabled - INTEGER +enabled - BOOLEAN Control whether MPTCP sockets can be created. - MPTCP sockets can be created if the value is nonzero. This is - a per-namespace sysctl. + MPTCP sockets can be created if the value is 1. This is a + per-namespace sysctl. - Default: 1 + Default: 1 (enabled) add_addr_timeout - INTEGER (seconds) Set the timeout after which an ADD_ADDR control message will be diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c index 96ba616f59bf..08c152199b89 100644 --- a/net/mptcp/ctrl.c +++ b/net/mptcp/ctrl.c @@ -17,7 +17,7 @@ static int mptcp_pernet_id; struct mptcp_pernet { struct ctl_table_header *ctl_table_hdr; - int mptcp_enabled; + u8 mptcp_enabled; unsigned int add_addr_timeout; }; @@ -39,12 +39,14 @@ unsigned int mptcp_get_add_addr_timeout(struct net *net) static struct ctl_table mptcp_sysctl_table[] = { { .procname = "enabled", - .maxlen = sizeof(int), + .maxlen = sizeof(u8), .mode = 0644, /* users with CAP_NET_ADMIN or root (not and) can change this * value, same as other sysctl or the 'net' tree. */ - .proc_handler = proc_dointvec, + .proc_handler = proc_dou8vec_minmax, + .extra1 = SYSCTL_ZERO, + .extra2 = SYSCTL_ONE }, { .procname = "add_addr_timeout",
To avoid confusions, it seems better to parse this sysctl parameter as a boolean. We use it as a boolean, no need to parse an integer and bring confusions if we see a value different from 0 and 1, especially with this parameter name: enabled. It seems fine to do this modification because the default value is 1 (enabled). Then the only other interesting value to set is 0 (disabled). All other values would not have changed the default behaviour. Suggested-by: Florian Westphal <fw@strlen.de> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net> --- Documentation/networking/mptcp-sysctl.rst | 8 ++++---- net/mptcp/ctrl.c | 8 +++++--- 2 files changed, 9 insertions(+), 7 deletions(-)