Re: [PATCH v2 1/1] dlm_controld: support corosync3/knet multi-link

[Heming Zhao <heming.zhao@suse.com>]

On 1/8/25 23:54, Alexander Aring wrote:
> Hi,
> 
> On Mon, Jan 6, 2025 at 11:59 PM Heming Zhao <heming.zhao@suse.com> wrote:
>>
>> On 1/7/25 02:11, Alexander Aring wrote:
>>> Hi Heming,
>>>
>>> On Tue, Dec 24, 2024 at 3:42 AM Heming Zhao <heming.zhao@suse.com> wrote:
>>>>
>>>> The totem.rrp_mode config item was obsolete in corosync3. And
>>>> this patch gives dlm_controld the ability to detect multiple
>>>> links.
>>>>
>>>> The corosync and dlm network protocol relationship table:
>>>>
>>>>    -------------+-----------------------+---------------------
>>>>                 | totem.transport=udpu  | totem.transport=udp
>>>>                 +-----------------------+---------------------
>>>>    corosync 2.x |            |          |      multicast
>>>>                 |   1-ring   | 2-ring   |---------------------
>>>>                 |            |          |  default  | 2-ring
>>>>    -------------+------------+----------+---------------------
>>>>       dlm       |     tcp    | sctp     |   tcp     | sctp
>>>>    -------------+------------+----------+---------------------
>>>>
>>>>    -------------+----------------------------+----------------------
>>>>                 | totem.transport = udpu/udp | totem.transport=knet
>>>>    corosync 3.x |----------------------------+----------------------
>>>>                 |      1-ring                | 1-link  | multi-links
>>>>    -------------+----------------------------+---------+-----------
>>>>       dlm       |       tcp                  |  tcp    | sctp
>>>>    -------------+----------------------------+---------+-----------
>>>>
>>>> At last, this patch should be work with updated kernel dlm module.
>>>
>>> I am not getting why the network protocol configuration has anything
>>> to do with the corosync configuration.
>>> I know that we currently get the address configurations from corosync
>>> but with this patch we are forced to use SCTP when corosync provides
>>> more than one "ring" configuration?
>>
>> Yes. this patch will force dlm to change to SCTP when corosync provides
>> more than one "ring".
>>
>> The reason:
>> (without this patch) When a user sets up multi-links on corosync3
>> and corosync.conf with an incorrect or missing rrp_mode,
>> dlm_tcp_listen_validate() will trigger 'dlm_local_count > 1' and report
>> an error.
>> Please note, rrp_mode is obsolete; the dlm_daemon will fail to read this
>> config item in the further. Therefore, the network protocol will
>> always be TCP.
>>
>>>
>>> Even with corosync3 it should be possible to use corosync in SCTP
>>> (multiple rings) and the kernel dlm using TCP only, would this not be
>>> possible with dlm_controld then?
>>
>> Only one case for above case: corosync3 on single-link.
>> A new patch is needed for dlm to work over TCP when corosync3 in SCTP
>> (multi-link mode). i.e. dlm_tcp_listen_validate() shouldn't return
>> -EINVAL when 'dlm_local_count > 1'.
>>
> 
> I think we should change that condition then.
> 
>> A key point for dlm is that there is no way to get the corosync version.
>> This patch is compatible with corosync2 env. In corosync2, the user must
>> correctly config rrp_mode when using 2-ring.
>>
> 
> So far I looked into it, it is anyway for detecting a protocol
> according to some Corosync functionality it should still be possible
> to always force dlm_controld using a different protocol by setting the
> right config values/parameters.

Yes, I forgot the config item 'protocol=[detect|tcp|sctp]', which can bypass
the detection phase when its value is "tcp|sctp". But in general, dlm.conf
is seldom used.

Unfortunately, corosync doesn't provide the api.
ref: https://github.com/corosync/corosync/issues/771

> 
>> i.e.:
>> In corosync2, change to 2-ring from 1-ring (whatever multicast mode).
>> There must include rrp_mode item, if not, error report:
>>     corosync[1284]:   [MAIN  ] parse error in config: 2 is too many configured interfaces for the rrp_mode setting none.
>>     corosync[1284]:   [MAIN  ] Corosync Cluster Engine exiting with status 8 at main.c:1415.
>>
>> Even more, since corosync3 isn't compatible with corosync2,
>> in my view, latest version of dlm_tools should only focus on corosync3
>> and drop corosync2 support. If any Linux distribution stay with
>> corosync2, they should choose an old version of dlm_tools.
>>
> 
> Is dlm_tool not just a domain socket talking to dlm_controld? It does
> not use any library of the Corosync project?

detect_protocol(@dlm_controld/action.c) uses the cmap APIs to get corosync
settings.
dlm_controld registers a callback in setup_cluster(), then corosync uses
quorum_callback/quorum_nodelist_callback to notify the dlm_controld daemon
about quorum status.

- Heming