[PATCH] dlm_controld: Support for extended value of kernel DLM_MAX_ADDR_COUNT

[PATCH] dlm_controld: Support for extended value of kernel DLM_MAX_ADDR_COUNT

[Heming Zhao]

This patch should work with the updated kernel dlm module, because
the code changes need to be aligned with the DLM_MAX_ADDR_COUNT value.

Another notice is that with the current dlm_controld code, users
should explicitly add 'protocol=sctp' to the dlm.conf in a corosync
3.x env.

Signed-off-by: Heming Zhao <heming.zhao@suse.com>
---
 dlm_controld/dlm_daemon.h | 6 +++---
 dlm_sand/sand_internal.h  | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/dlm_controld/dlm_daemon.h b/dlm_controld/dlm_daemon.h
index 4a533e3451e2..c902228f890f 100644
--- a/dlm_controld/dlm_daemon.h
+++ b/dlm_controld/dlm_daemon.h
@@ -175,10 +175,10 @@ EXTERN struct dlm_option dlm_options[dlm_options_max];
 
 #define MAX_NODES	128
 
-/* Maximum number of IP addresses per node, when using SCTP and multi-ring in
-   corosync  In dlm-kernel this is DLM_MAX_ADDR_COUNT, currently 3. */
+/* Maximum number of IP addresses per node, when using SCTP and multi-[ring|link]
+   in corosync [2|3]. In dlm-kernel this is DLM_MAX_ADDR_COUNT, currently 8. */
 
-#define MAX_NODE_ADDRESSES 4
+#define MAX_NODE_ADDRESSES 8
 
 #define PROTO_TCP  0
 #define PROTO_SCTP 1
diff --git a/dlm_sand/sand_internal.h b/dlm_sand/sand_internal.h
index 4c2fc0897051..c17287abcd47 100644
--- a/dlm_sand/sand_internal.h
+++ b/dlm_sand/sand_internal.h
@@ -138,9 +138,9 @@ EXTERN struct dlm_option dlm_options[dlm_options_max];
    Copied in libdlm.h so apps don't need to include the kernel header. */
 
 /* Maximum number of IP addresses per node, when using SCTP.
-   In dlm-kernel this is DLM_MAX_ADDR_COUNT, currently 3. */
+   In dlm-kernel this is DLM_MAX_ADDR_COUNT, currently 8. */
 
-#define MAX_NODE_ADDRESSES 4
+#define MAX_NODE_ADDRESSES 8
 
 #define PROTO_TCP  0
 #define PROTO_SCTP 1
-- 
2.48.1

Re: [PATCH] dlm_controld: Support for extended value of kernel DLM_MAX_ADDR_COUNT

[Alexander Aring]

Hi,

On Fri, Feb 21, 2025 at 10:11 AM Heming Zhao <heming.zhao@suse.com> wrote:
>
> This patch should work with the updated kernel dlm module, because
> the code changes need to be aligned with the DLM_MAX_ADDR_COUNT value.
>
> Another notice is that with the current dlm_controld code, users
> should explicitly add 'protocol=sctp' to the dlm.conf in a corosync
> 3.x env.
>

I applied this patch to dlm upstream repo [0].

Thanks.

What are we doing with the detect_protocol() change? You want to
submit the current version again? I would like to test it with
corosync 2 vs 3 with no configuration changes on an upgrade (if this
is even possible).

- Alex

[0] https://pagure.io/dlm/tree/main

Re: [PATCH] dlm_controld: Support for extended value of kernel DLM_MAX_ADDR_COUNT

[David Teigland]

On Mon, Feb 24, 2025 at 10:27:04AM -0500, Alexander Aring wrote:
> What are we doing with the detect_protocol() change? You want to
> submit the current version again? I would like to test it with
> corosync 2 vs 3 with no configuration changes on an upgrade (if this
> is even possible).

I suggest we stop trying to detect, and always use tcp unless protocol is
set to sctp.

Dave

Re: [PATCH] dlm_controld: Support for extended value of kernel DLM_MAX_ADDR_COUNT

[Heming Zhao]

On 2/25/25 03:35, David Teigland wrote:
> On Mon, Feb 24, 2025 at 10:27:04AM -0500, Alexander Aring wrote:
>> What are we doing with the detect_protocol() change? You want to
>> submit the current version again? I would like to test it with
>> corosync 2 vs 3 with no configuration changes on an upgrade (if this
>> is even possible).
> 
> I suggest we stop trying to detect, and always use tcp unless protocol is
> set to sctp.
> 
> Dave
> 

That's fine with me
For corosync 3.x, there is a lot of work to support dynamic multi-link.
However, to support config mode, the current dlm code only requires a few modifications.

- Heming

Re: [PATCH] dlm_controld: Support for extended value of kernel DLM_MAX_ADDR_COUNT

[Heming Zhao]

Hello Dave,

On 2/25/25 14:27, Heming Zhao wrote:
> On 2/25/25 03:35, David Teigland wrote:
>> On Mon, Feb 24, 2025 at 10:27:04AM -0500, Alexander Aring wrote:
>>> What are we doing with the detect_protocol() change? You want to
>>> submit the current version again? I would like to test it with
>>> corosync 2 vs 3 with no configuration changes on an upgrade (if this
>>> is even possible).
>>
>> I suggest we stop trying to detect, and always use tcp unless protocol is
>> set to sctp.
>>
>> Dave
>>
> 
> That's fine with me
> For corosync 3.x, there is a lot of work to support dynamic multi-link.
> However, to support config mode, the current dlm code only requires a few modifications.
> 
> - Heming

Following your idea, if TCP is used by default, what will happen when a user
sets up multiple links in Corosync 3.x without changing config file?
The TCP mode of the current DLM only accepts one link. Therefore, in this
scenario, DLM will refuse to run and report an error.

Do you think we need to invest effort in changing the TCP mode code to allow
TCP to work when the Corosync layer sets up multiple links?
For example, to simplify the code, we could take only the first network link
from the Corosync callback and ignore the others. Or we could keep the current
behavior and prevent running in multi-link mode under TCP.

- Heming

Re: [PATCH] dlm_controld: Support for extended value of kernel DLM_MAX_ADDR_COUNT

[Alexander Aring]

Hi,

On Tue, Feb 25, 2025 at 2:34 AM Heming Zhao <heming.zhao@suse.com> wrote:
>
> Hello Dave,
>
> On 2/25/25 14:27, Heming Zhao wrote:
> > On 2/25/25 03:35, David Teigland wrote:
> >> On Mon, Feb 24, 2025 at 10:27:04AM -0500, Alexander Aring wrote:
> >>> What are we doing with the detect_protocol() change? You want to
> >>> submit the current version again? I would like to test it with
> >>> corosync 2 vs 3 with no configuration changes on an upgrade (if this
> >>> is even possible).
> >>
> >> I suggest we stop trying to detect, and always use tcp unless protocol is
> >> set to sctp.
> >>
> >> Dave
> >>
> >
> > That's fine with me
> > For corosync 3.x, there is a lot of work to support dynamic multi-link.
> > However, to support config mode, the current dlm code only requires a few modifications.
> >
> > - Heming
>
> Following your idea, if TCP is used by default, what will happen when a user
> sets up multiple links in Corosync 3.x without changing config file?
> The TCP mode of the current DLM only accepts one link. Therefore, in this
> scenario, DLM will refuse to run and report an error.
>
> Do you think we need to invest effort in changing the TCP mode code to allow
> TCP to work when the Corosync layer sets up multiple links?
> For example, to simplify the code, we could take only the first network link
> from the Corosync callback and ignore the others. Or we could keep the current
> behavior and prevent running in multi-link mode under TCP.

I would ignore the others, there exists ways to run multipath TCP on
top of multiple TCP sockets (similar to what mptcp is doing but mptcp
might be overkill, lookup "mptcp redudant scheduler").
This can be future work, existing code regarding sequence numbers of
dlm messages can be reused.

If somebody applies multiple links we assume any of them would work.
Later additional handling as mentioned above could just begin to work
in a magical way.

- Alex

Re: [PATCH] dlm_controld: Support for extended value of kernel DLM_MAX_ADDR_COUNT

[Heming Zhao]

On 2/25/25 22:19, Alexander Aring wrote:
> Hi,
> 
> On Tue, Feb 25, 2025 at 2:34 AM Heming Zhao <heming.zhao@suse.com> wrote:
>>
>> Hello Dave,
>>
>> On 2/25/25 14:27, Heming Zhao wrote:
>>> On 2/25/25 03:35, David Teigland wrote:
>>>> On Mon, Feb 24, 2025 at 10:27:04AM -0500, Alexander Aring wrote:
>>>>> What are we doing with the detect_protocol() change? You want to
>>>>> submit the current version again? I would like to test it with
>>>>> corosync 2 vs 3 with no configuration changes on an upgrade (if this
>>>>> is even possible).
>>>>
>>>> I suggest we stop trying to detect, and always use tcp unless protocol is
>>>> set to sctp.
>>>>
>>>> Dave
>>>>
>>>
>>> That's fine with me
>>> For corosync 3.x, there is a lot of work to support dynamic multi-link.
>>> However, to support config mode, the current dlm code only requires a few modifications.
>>>
>>> - Heming
>>
>> Following your idea, if TCP is used by default, what will happen when a user
>> sets up multiple links in Corosync 3.x without changing config file?
>> The TCP mode of the current DLM only accepts one link. Therefore, in this
>> scenario, DLM will refuse to run and report an error.
>>
>> Do you think we need to invest effort in changing the TCP mode code to allow
>> TCP to work when the Corosync layer sets up multiple links?
>> For example, to simplify the code, we could take only the first network link
>> from the Corosync callback and ignore the others. Or we could keep the current
>> behavior and prevent running in multi-link mode under TCP.
> 
> I would ignore the others, there exists ways to run multipath TCP on
> top of multiple TCP sockets (similar to what mptcp is doing but mptcp
> might be overkill, lookup "mptcp redudant scheduler").
> This can be future work, existing code regarding sequence numbers of
> dlm messages can be reused.
> 
> If somebody applies multiple links we assume any of them would work.
> Later additional handling as mentioned above could just begin to work
> in a magical way.
> 
> - Alex
> 

Let me summarize the above discussion/description:
- TCP is the default.
- stop trying to detect:
   - use dlm.conf to switch from TCP to SCTP.
   - corosync.conf 'rrp_mode' is not supported in corosync 2.x.
- TCP can work with multi-link, but only accept the first ip addr from corosync.
- SCTP can support max 8 ip addr for corosync 3.x.

If there are no further discussions, I will provide a patch.

- Heming

Re: [PATCH] dlm_controld: Support for extended value of kernel DLM_MAX_ADDR_COUNT

[Alexander Aring]

Hi,

On Thu, Feb 27, 2025 at 8:09 PM Heming Zhao <heming.zhao@suse.com> wrote:
...
>
> Let me summarize the above discussion/description:
> - TCP is the default.
> - stop trying to detect:
>    - use dlm.conf to switch from TCP to SCTP.
>    - corosync.conf 'rrp_mode' is not supported in corosync 2.x.

yes, I would just drop the whole detect_protocol() function and don't
have the protocol setting depending on any corosync.conf related.

> - TCP can work with multi-link, but only accept the first ip addr from corosync.

yes.

> - SCTP can support max 8 ip addr for corosync 3.x.
>

It shouldn't depend on which corosync version we are using, or? It
should be the same way for 3.x and any other version, see [0]?

Thanks!

- Alex

[0[ https://pagure.io/dlm/blob/main/f/dlm_controld/member.c#_216

Re: [PATCH] dlm_controld: Support for extended value of kernel DLM_MAX_ADDR_COUNT

[Heming Zhao]

On 2/28/25 23:34, Alexander Aring wrote:
> Hi,
> 
> On Thu, Feb 27, 2025 at 8:09 PM Heming Zhao <heming.zhao@suse.com> wrote:
> ...
>>
>> Let me summarize the above discussion/description:
>> - TCP is the default.
>> - stop trying to detect:
>>     - use dlm.conf to switch from TCP to SCTP.
>>     - corosync.conf 'rrp_mode' is not supported in corosync 2.x.
> 
> yes, I would just drop the whole detect_protocol() function and don't
> have the protocol setting depending on any corosync.conf related.
> 
>> - TCP can work with multi-link, but only accept the first ip addr from corosync.
> 
> yes.
> 
>> - SCTP can support max 8 ip addr for corosync 3.x.
>>
> 
> It shouldn't depend on which corosync version we are using, or? It
> should be the same way for 3.x and any other version, see [0]?
> 

Sorry for my English.
Yes, there is the same way. There is no special code for corosync 3.x.
My mean:
After changing DLM_MAX_ADDR_COUNT (kernel) & MAX_NODE_ADDRESSES (dlm_tools),
the dlm layer supports max of 8 ip addrs (in SCTP mode). And the number 8
is equal to the max number of multi-links in corosync 3.x.

- Heming

> Thanks!
> 
> - Alex
> 
> [0[ https://pagure.io/dlm/blob/main/f/dlm_controld/member.c#_216
> 

Re: [PATCH] dlm_controld: Support for extended value of kernel DLM_MAX_ADDR_COUNT

[Alexander Aring]

Hi,

On Mon, Mar 3, 2025 at 10:07 PM Heming Zhao <heming.zhao@suse.com> wrote:
>
> On 2/28/25 23:34, Alexander Aring wrote:
> > Hi,
> >
> > On Thu, Feb 27, 2025 at 8:09 PM Heming Zhao <heming.zhao@suse.com> wrote:
> > ...
> >>
> >> Let me summarize the above discussion/description:
> >> - TCP is the default.
> >> - stop trying to detect:
> >>     - use dlm.conf to switch from TCP to SCTP.
> >>     - corosync.conf 'rrp_mode' is not supported in corosync 2.x.
> >
> > yes, I would just drop the whole detect_protocol() function and don't
> > have the protocol setting depending on any corosync.conf related.
> >
> >> - TCP can work with multi-link, but only accept the first ip addr from corosync.
> >
> > yes.
> >
> >> - SCTP can support max 8 ip addr for corosync 3.x.
> >>
> >
> > It shouldn't depend on which corosync version we are using, or? It
> > should be the same way for 3.x and any other version, see [0]?
> >
>
> Sorry for my English.
> Yes, there is the same way. There is no special code for corosync 3.x.
> My mean:
> After changing DLM_MAX_ADDR_COUNT (kernel) & MAX_NODE_ADDRESSES (dlm_tools),
> the dlm layer supports max of 8 ip addrs (in SCTP mode). And the number 8
> is equal to the max number of multi-links in corosync 3.x.
>

ok. Node address configuration shouldn't depend on any Corosync
configuration, however I agree that dlm_controld is currently in a
state that it just simply works in a way that it gets the node address
configuration of its configuration from Corosync and this is fine for
now.

- Alex