| Message ID | 20221125121127.40815-1-firo.yang@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,1/1] sctp: sysctl: make extra pointers netns aware | expand |
On Fri, Nov 25, 2022 at 08:11:27PM +0800, Firo Yang wrote: > Recently, a customer reported that from their container whose > net namespace is different to the host's init_net, they can't set > the container's net.sctp.rto_max to any value smaller than > init_net.sctp.rto_min. > > For instance, > Host: > sudo sysctl net.sctp.rto_min > net.sctp.rto_min = 1000 > > Container: > echo 100 > /mnt/proc-net/sctp/rto_min > echo 400 > /mnt/proc-net/sctp/rto_max > echo: write error: Invalid argument > > This is caused by the check made from this'commit 4f3fdf3bc59c > ("sctp: add check rto_min and rto_max in sysctl")' > When validating the input value, it's always referring the boundary > value set for the init_net namespace. > > Having container's rto_max smaller than host's init_net.sctp.rto_min > does make sense. Consider that the rto between two containers on the > same host is very likely smaller than it for two hosts. > > So to fix this problem, as suggested by Marcelo, this patch makes the > extra pointers of rto_min, rto_max, pf_retrans, and ps_retrans point > to the corresponding variables from the newly created net namespace while > the new net namespace is being registered in sctp_sysctl_net_register. > > Fixes: 4f3fdf3bc59c ("sctp: add check rto_min and rto_max in sysctl") > Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > Signed-off-by: Firo Yang <firo.yang@suse.com> and Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Thanks Firo.
On Fri, 25 Nov 2022 20:11:27 +0800 Firo Yang wrote: > +#define SCTP_RTO_MIN_IDX 1 > +#define SCTP_RTO_MAX_IDX 2 > +#define SCTP_PF_RETRANS_IDX 3 > +#define SCTP_PS_RETRANS_IDX 4 Use these to index the entries, please, like this: struct bla table[] = { [MY_INDEX_ONE] = { .whatever = 1, }, [MY_INDEX_TWO] = { .fields = 2, }, { .there = 3, }, { .are = 4, }, }; I think that works even without all entries in the table having the index.. ? > static struct ctl_table sctp_net_table[] = { > { > .procname = "rto_initial", > @@ -112,6 +122,24 @@ static struct ctl_table sctp_net_table[] = { > .extra1 = &init_net.sctp.rto_min, > .extra2 = &timer_max > }, > + { > + .procname = "pf_retrans", > + .data = &init_net.sctp.pf_retrans, > + .maxlen = sizeof(int), > + .mode = 0644, > + .proc_handler = proc_dointvec_minmax, > + .extra1 = SYSCTL_ZERO, > + .extra2 = &init_net.sctp.ps_retrans, > + }, > + { > + .procname = "ps_retrans", > + .data = &init_net.sctp.ps_retrans, > + .maxlen = sizeof(int), > + .mode = 0644, > + .proc_handler = proc_dointvec_minmax, > + .extra1 = &init_net.sctp.pf_retrans, > + .extra2 = &ps_retrans_max, > + },
The 11/29/2022 20:48, Jakub Kicinski wrote: > On Fri, 25 Nov 2022 20:11:27 +0800 Firo Yang wrote: > > +#define SCTP_RTO_MIN_IDX 1 > > +#define SCTP_RTO_MAX_IDX 2 > > +#define SCTP_PF_RETRANS_IDX 3 > > +#define SCTP_PS_RETRANS_IDX 4 > > Use these to index the entries, please, like this: > > struct bla table[] = { > [MY_INDEX_ONE] = { > .whatever = 1, > }, > [MY_INDEX_TWO] = { > .fields = 2, > }, > { > .there = 3, > }, > { > .are = 4, > }, > }; > > I think that works even without all entries in the table having the > index.. ? > Cool. I will send a V3. Thanks, // Firo
diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c index b46a416787ec..516ab67475e1 100644 --- a/net/sctp/sysctl.c +++ b/net/sctp/sysctl.c @@ -84,6 +84,16 @@ static struct ctl_table sctp_table[] = { { /* sentinel */ } }; +/* The following index defines are used in sctp_sysctl_net_register(). + * If you add new items to the sctp_net_table, please ensure that + * the index values of these defines hold the same meaning indicated by + * their macro names when they appear in sctp_net_table. + */ +#define SCTP_RTO_MIN_IDX 1 +#define SCTP_RTO_MAX_IDX 2 +#define SCTP_PF_RETRANS_IDX 3 +#define SCTP_PS_RETRANS_IDX 4 + static struct ctl_table sctp_net_table[] = { { .procname = "rto_initial", @@ -112,6 +122,24 @@ static struct ctl_table sctp_net_table[] = { .extra1 = &init_net.sctp.rto_min, .extra2 = &timer_max }, + { + .procname = "pf_retrans", + .data = &init_net.sctp.pf_retrans, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec_minmax, + .extra1 = SYSCTL_ZERO, + .extra2 = &init_net.sctp.ps_retrans, + }, + { + .procname = "ps_retrans", + .data = &init_net.sctp.ps_retrans, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec_minmax, + .extra1 = &init_net.sctp.pf_retrans, + .extra2 = &ps_retrans_max, + }, { .procname = "rto_alpha_exp_divisor", .data = &init_net.sctp.rto_alpha, @@ -207,24 +235,6 @@ static struct ctl_table sctp_net_table[] = { .extra1 = SYSCTL_ONE, .extra2 = SYSCTL_INT_MAX, }, - { - .procname = "pf_retrans", - .data = &init_net.sctp.pf_retrans, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec_minmax, - .extra1 = SYSCTL_ZERO, - .extra2 = &init_net.sctp.ps_retrans, - }, - { - .procname = "ps_retrans", - .data = &init_net.sctp.ps_retrans, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec_minmax, - .extra1 = &init_net.sctp.pf_retrans, - .extra2 = &ps_retrans_max, - }, { .procname = "sndbuf_policy", .data = &init_net.sctp.sndbuf_policy, @@ -586,6 +596,11 @@ int sctp_sysctl_net_register(struct net *net) for (i = 0; table[i].data; i++) table[i].data += (char *)(&net->sctp) - (char *)&init_net.sctp; + table[SCTP_RTO_MIN_IDX].extra2 = &net->sctp.rto_max; + table[SCTP_RTO_MAX_IDX].extra1 = &net->sctp.rto_min; + table[SCTP_PF_RETRANS_IDX].extra2 = &net->sctp.ps_retrans; + table[SCTP_PS_RETRANS_IDX].extra1 = &net->sctp.pf_retrans; + net->sctp.sysctl_header = register_net_sysctl(net, "net/sctp", table); if (net->sctp.sysctl_header == NULL) { kfree(table);
Recently, a customer reported that from their container whose net namespace is different to the host's init_net, they can't set the container's net.sctp.rto_max to any value smaller than init_net.sctp.rto_min. For instance, Host: sudo sysctl net.sctp.rto_min net.sctp.rto_min = 1000 Container: echo 100 > /mnt/proc-net/sctp/rto_min echo 400 > /mnt/proc-net/sctp/rto_max echo: write error: Invalid argument This is caused by the check made from this'commit 4f3fdf3bc59c ("sctp: add check rto_min and rto_max in sysctl")' When validating the input value, it's always referring the boundary value set for the init_net namespace. Having container's rto_max smaller than host's init_net.sctp.rto_min does make sense. Consider that the rto between two containers on the same host is very likely smaller than it for two hosts. So to fix this problem, as suggested by Marcelo, this patch makes the extra pointers of rto_min, rto_max, pf_retrans, and ps_retrans point to the corresponding variables from the newly created net namespace while the new net namespace is being registered in sctp_sysctl_net_register. Fixes: 4f3fdf3bc59c ("sctp: add check rto_min and rto_max in sysctl") Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Signed-off-by: Firo Yang <firo.yang@suse.com> --- net/sctp/sysctl.c | 51 ++++++++++++++++++++++++++++++----------------- 1 file changed, 33 insertions(+), 18 deletions(-)