[RFC PATCH dlm/next 1/4] dlm: mask sk_shutdown value

[RFC PATCH dlm/next 1/4] dlm: mask sk_shutdown value

[Alexander Aring]

The sk->sk_shutdown value is flag value so use masking to check if
RCV_SHUTDOWN is set as other possible values like SEND_SHUTDOWN can set
as well.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/lowcomms.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 70abd4da17a6..50c42b368c83 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -533,7 +533,7 @@ static void lowcomms_state_change(struct sock *sk)
 	/* SCTP layer is not calling sk_data_ready when the connection
 	 * is done, so we catch the signal through here.
 	 */
-	if (sk->sk_shutdown == RCV_SHUTDOWN)
+	if (sk->sk_shutdown & RCV_SHUTDOWN)
 		lowcomms_data_ready(sk);
 }
 
-- 
2.43.0

[RFC PATCH dlm/next 2/4] dlm: use SHUT_RDWR for SCTP shutdown

[Alexander Aring]

Currently SCTP shutdown() call gets stuck because there is no incoming
EOF indicator on its socket. On the peer side the EOF indicator as
recvmsg() returns 0 will be triggered as mechanism to flush the socket
queue on the receive side. In SCTP recvmsg() function sctp_recvmsg() we
can see that only if sk_shutdown has the bit RCV_SHUTDOWN set SCTP will
recvmsg() will return EOF. The RCV_SHUTDOWN bit will only be set when
shutdown with SHUT_RD is called. We use now SHUT_RDWR to also get a EOF
indicator from recvmsg() call on the shutdown() initiator.

SCTP does not support half closed sockets and the semantic of SHUT_WR is
different here, it seems that calling SHUT_WR on sctp sockets keeps the
socket open to have the possibility to do some specific SCTP operations on
it that we don't do here.

There exists still a difference in the limitations of TCP vs SCTP in
case if we are required to have a half closed socket functionality. This
was tried to archieve with DLM protocol changes in the past and
hopefully we really don't require half closed socket functionality.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/lowcomms.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
index 50c42b368c83..e4373bce1bc2 100644
--- a/fs/dlm/lowcomms.c
+++ b/fs/dlm/lowcomms.c
@@ -160,6 +160,7 @@ struct dlm_proto_ops {
 	bool try_new_addr;
 	const char *name;
 	int proto;
+	int how;
 
 	void (*sockopts)(struct socket *sock);
 	int (*bind)(struct socket *sock);
@@ -810,7 +811,7 @@ static void shutdown_connection(struct connection *con, bool and_other)
 		return;
 	}
 
-	ret = kernel_sock_shutdown(con->sock, SHUT_WR);
+	ret = kernel_sock_shutdown(con->sock, dlm_proto_ops->how);
 	up_read(&con->sock_lock);
 	if (ret) {
 		log_print("Connection %p failed to shutdown: %d will force close",
@@ -1858,6 +1859,7 @@ static int dlm_tcp_listen_bind(struct socket *sock)
 static const struct dlm_proto_ops dlm_tcp_ops = {
 	.name = "TCP",
 	.proto = IPPROTO_TCP,
+	.how = SHUT_WR,
 	.sockopts = dlm_tcp_sockopts,
 	.bind = dlm_tcp_bind,
 	.listen_validate = dlm_tcp_listen_validate,
@@ -1896,6 +1898,7 @@ static void dlm_sctp_sockopts(struct socket *sock)
 static const struct dlm_proto_ops dlm_sctp_ops = {
 	.name = "SCTP",
 	.proto = IPPROTO_SCTP,
+	.how = SHUT_RDWR,
 	.try_new_addr = true,
 	.sockopts = dlm_sctp_sockopts,
 	.bind = dlm_sctp_bind,
-- 
2.43.0

Re: [RFC PATCH dlm/next 2/4] dlm: use SHUT_RDWR for SCTP shutdown

[Heming Zhao]

On 4/30/25 04:29, Alexander Aring wrote:
> Currently SCTP shutdown() call gets stuck because there is no incoming
> EOF indicator on its socket. On the peer side the EOF indicator as
> recvmsg() returns 0 will be triggered as mechanism to flush the socket
> queue on the receive side. In SCTP recvmsg() function sctp_recvmsg() we
> can see that only if sk_shutdown has the bit RCV_SHUTDOWN set SCTP will
> recvmsg() will return EOF. The RCV_SHUTDOWN bit will only be set when
> shutdown with SHUT_RD is called. We use now SHUT_RDWR to also get a EOF
> indicator from recvmsg() call on the shutdown() initiator.
> 
> SCTP does not support half closed sockets and the semantic of SHUT_WR is
> different here, it seems that calling SHUT_WR on sctp sockets keeps the
> socket open to have the possibility to do some specific SCTP operations on
> it that we don't do here.
> 
> There exists still a difference in the limitations of TCP vs SCTP in
> case if we are required to have a half closed socket functionality. This
> was tried to archieve with DLM protocol changes in the past and
> hopefully we really don't require half closed socket functionality.
> 
> Signed-off-by: Alexander Aring <aahringo@redhat.com>

Tested-by: Heming zhao <heming.zhao@suse.com>
Reviewed-by: Heming zhao <heming.zhao@suse.com>

> ---
>   fs/dlm/lowcomms.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
> index 50c42b368c83..e4373bce1bc2 100644
> --- a/fs/dlm/lowcomms.c
> +++ b/fs/dlm/lowcomms.c
> @@ -160,6 +160,7 @@ struct dlm_proto_ops {
>   	bool try_new_addr;
>   	const char *name;
>   	int proto;
> +	int how;
>   
>   	void (*sockopts)(struct socket *sock);
>   	int (*bind)(struct socket *sock);
> @@ -810,7 +811,7 @@ static void shutdown_connection(struct connection *con, bool and_other)
>   		return;
>   	}
>   
> -	ret = kernel_sock_shutdown(con->sock, SHUT_WR);
> +	ret = kernel_sock_shutdown(con->sock, dlm_proto_ops->how);
>   	up_read(&con->sock_lock);
>   	if (ret) {
>   		log_print("Connection %p failed to shutdown: %d will force close",
> @@ -1858,6 +1859,7 @@ static int dlm_tcp_listen_bind(struct socket *sock)
>   static const struct dlm_proto_ops dlm_tcp_ops = {
>   	.name = "TCP",
>   	.proto = IPPROTO_TCP,
> +	.how = SHUT_WR,
>   	.sockopts = dlm_tcp_sockopts,
>   	.bind = dlm_tcp_bind,
>   	.listen_validate = dlm_tcp_listen_validate,
> @@ -1896,6 +1898,7 @@ static void dlm_sctp_sockopts(struct socket *sock)
>   static const struct dlm_proto_ops dlm_sctp_ops = {
>   	.name = "SCTP",
>   	.proto = IPPROTO_SCTP,
> +	.how = SHUT_RDWR,
>   	.try_new_addr = true,
>   	.sockopts = dlm_sctp_sockopts,
>   	.bind = dlm_sctp_bind,

[RFC PATCH dlm/next 3/4] dlm: reject SCTP configuration if not enabled

[Alexander Aring]

Reject SCTP dlm configuration if the kernel was never build with SCTP.
Currently the only one known user space tool "dlm_controld" will drop an
error in the logs and getting stuck. This behaviour should be fixed to
deliver an error to the user or fallback to TCP.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/config.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/dlm/config.c b/fs/dlm/config.c
index cf9ba6fd7a28..a23fd524a6ee 100644
--- a/fs/dlm/config.c
+++ b/fs/dlm/config.c
@@ -197,6 +197,9 @@ static int dlm_check_protocol_and_dlm_running(unsigned int x)
 		break;
 	case 1:
 		/* SCTP */
+		if (!IS_ENABLED(CONFIG_IP_SCTP))
+			return -EOPNOTSUPP;
+
 		break;
 	default:
 		return -EINVAL;
-- 
2.43.0

Re: [RFC PATCH dlm/next 3/4] dlm: reject SCTP configuration if not enabled

[Heming Zhao]

On 4/30/25 04:29, Alexander Aring wrote:
> Reject SCTP dlm configuration if the kernel was never build with SCTP.
> Currently the only one known user space tool "dlm_controld" will drop an
> error in the logs and getting stuck. This behaviour should be fixed to
> deliver an error to the user or fallback to TCP.
> 
> Signed-off-by: Alexander Aring <aahringo@redhat.com>

Reviewed-by: Heming zhao <heming.zhao@suse.com>

> ---
>   fs/dlm/config.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/fs/dlm/config.c b/fs/dlm/config.c
> index cf9ba6fd7a28..a23fd524a6ee 100644
> --- a/fs/dlm/config.c
> +++ b/fs/dlm/config.c
> @@ -197,6 +197,9 @@ static int dlm_check_protocol_and_dlm_running(unsigned int x)
>   		break;
>   	case 1:
>   		/* SCTP */
> +		if (!IS_ENABLED(CONFIG_IP_SCTP))
> +			return -EOPNOTSUPP;
> +
>   		break;
>   	default:
>   		return -EINVAL;

[RFC PATCH dlm/next 4/4] dlm: drop SCTP Kconfig dependency

[Alexander Aring]

DLM does not use any exported SCTP function. SCTP is registered
dynamically as protocol to the kernel and can be used over the right
protocol identifiers on the socket api. We drop the SCTP dependency as
DLM can also be used with TCP only.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/dlm/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/dlm/Kconfig b/fs/dlm/Kconfig
index f82a4952769d..b46165df5a91 100644
--- a/fs/dlm/Kconfig
+++ b/fs/dlm/Kconfig
@@ -3,7 +3,6 @@ menuconfig DLM
 	tristate "Distributed Lock Manager (DLM)"
 	depends on INET
 	depends on SYSFS && CONFIGFS_FS && (IPV6 || IPV6=n)
-	select IP_SCTP
 	help
 	A general purpose distributed lock manager for kernel or userspace
 	applications.
-- 
2.43.0

Re: [RFC PATCH dlm/next 4/4] dlm: drop SCTP Kconfig dependency

[Heming Zhao]

On 4/30/25 04:29, Alexander Aring wrote:
> DLM does not use any exported SCTP function. SCTP is registered
> dynamically as protocol to the kernel and can be used over the right
> protocol identifiers on the socket api. We drop the SCTP dependency as
> DLM can also be used with TCP only.
> 
> Signed-off-by: Alexander Aring <aahringo@redhat.com>

Reviewed-by: Heming zhao <heming.zhao@suse.com>

> ---
>   fs/dlm/Kconfig | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/fs/dlm/Kconfig b/fs/dlm/Kconfig
> index f82a4952769d..b46165df5a91 100644
> --- a/fs/dlm/Kconfig
> +++ b/fs/dlm/Kconfig
> @@ -3,7 +3,6 @@ menuconfig DLM
>   	tristate "Distributed Lock Manager (DLM)"
>   	depends on INET
>   	depends on SYSFS && CONFIGFS_FS && (IPV6 || IPV6=n)
> -	select IP_SCTP
>   	help
>   	A general purpose distributed lock manager for kernel or userspace
>   	applications.

Re: [RFC PATCH dlm/next 1/4] dlm: mask sk_shutdown value

[Heming Zhao]

On 4/30/25 04:29, Alexander Aring wrote:
> The sk->sk_shutdown value is flag value so use masking to check if
> RCV_SHUTDOWN is set as other possible values like SEND_SHUTDOWN can set
> as well.
> 
> Signed-off-by: Alexander Aring <aahringo@redhat.com>

Tested-by: Heming zhao <heming.zhao@suse.com>
Reviewed-by: Heming zhao <heming.zhao@suse.com>

> ---
>   fs/dlm/lowcomms.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c
> index 70abd4da17a6..50c42b368c83 100644
> --- a/fs/dlm/lowcomms.c
> +++ b/fs/dlm/lowcomms.c
> @@ -533,7 +533,7 @@ static void lowcomms_state_change(struct sock *sk)
>   	/* SCTP layer is not calling sk_data_ready when the connection
>   	 * is done, so we catch the signal through here.
>   	 */
> -	if (sk->sk_shutdown == RCV_SHUTDOWN)
> +	if (sk->sk_shutdown & RCV_SHUTDOWN)
>   		lowcomms_data_ready(sk);
>   }
>