On Fri, Oct 16, 2020 at 3:08 PM Xin Long [off-list ref] wrote:

On Fri, Oct 16, 2020 at 1:42 AM Marcelo Ricardo Leitner
[off-list ref] wrote:

quoted

Actually..

On Tue, Oct 13, 2020 at 03:27:41PM +0800, Xin Long wrote:
...

quoted

Also add sysctl udp_port to allow changing the listening
sock's port by users.

...

quoted

---
 net/sctp/protocol.c |  5 +++++
 net/sctp/sysctl.c   | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

Xin, sorry for not noticing this earlier, but we need a documentation
update here for this new sysctl. This is important. Please add its
entry in ip-sysctl.rst.

no problem, I will add it.

quoted

quoted

diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index be002b7..79fb4b5 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c

@@ -1469,6 +1469,10 @@ static int __net_init sctp_ctrlsock_init(struct net *net)
      if (status)
              pr_err("Failed to initialize the SCTP control sock\n");

+     status = sctp_udp_sock_start(net);
+     if (status)
+             pr_err("Failed to initialize the SCTP udp tunneling sock\n");

                                                      ^^^ upper case please.
Nit. There are other occurrences of this.

You mean we can remove this log, as it's been handled well in
sctp_udp_sock_start()?

This one is actually OK :D
I've updated 'udp' with 'UDP' for all code annotations in this patchset.

will post v4.

Thanks.

quoted

quoted

+
      return status;

...

quoted

+     ret = proc_dointvec(&tbl, write, buffer, lenp, ppos);
+     if (write && ret == 0) {
+             struct sock *sk = net->sctp.ctl_sock;
+
+             if (new_value > max || new_value < min)
+                     return -EINVAL;
+
+             net->sctp.udp_port = new_value;
+             sctp_udp_sock_stop(net);

So, if it would be disabling the encapsulation, it shouldn't be
calling _start() then, right? Like

                if (new_value)
                        ret = sctp_udp_sock_start(net);

Otherwise _start() here will call ..._bind() with port 0, which then
will be a random one.

right, somehow I thought it wouldn't bind with port 0.

Thanks.

quoted

quoted

+             ret = sctp_udp_sock_start(net);
+             if (ret)
+                     net->sctp.udp_port = 0;
+
+             /* Update the value in the control socket */
+             lock_sock(sk);
+             sctp_sk(sk)->udp_port = htons(net->sctp.udp_port);
+             release_sock(sk);
+     }
+
+     return ret;
+}
+
 int sctp_sysctl_net_register(struct net *net)
 {
      struct ctl_table *table;
--
2.1.0