Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove clear_multicast_endpoints() in unsubscribe. #651

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

akhzarj
Copy link
Contributor

@akhzarj akhzarj commented Mar 16, 2024

Author: Aram Khzarjyan [email protected], MBition GmbH.

Remove clear_multicast_endpoints() in unsubscribe. #651

The clearing multicast endpoints stops any multicast messages
receiving without recovering if TTLs are not expiring.


The program was tested solely for our own use cases, which might differ from yours.

The submission is provided under the main project license (LICENSE file in root of project).

Provider Information

@akhzarj akhzarj marked this pull request as draft March 18, 2024 11:50
@akhzarj akhzarj changed the title Fix cleaning Muliticast during unsubscription. Remove clear_multicast_endpoints() in unsubscribe. Mar 21, 2024
The clearing multicast endpoints stops any multicast messages
receiving without recovering if TTLs are not expiring.

Co-authored-by: Aram Khzarjyan <[email protected]>
Signed-off-by: Gunnar Andersson <[email protected]>
@gunnar-mb
Copy link

PR looks good to me.

@akhzarj akhzarj marked this pull request as ready for review March 22, 2024 15:54
@duartenfonseca
Copy link
Collaborator

Hi @akhzarj could you give an example of how we can reproduce this issue?

@duartenfonseca
Copy link
Collaborator

duartenfonseca commented Oct 15, 2024

if one wants to unsubscribe to a set of service/instance, why would you want to receive any more messages from that specific endpoint?

@akhzarj
Copy link
Contributor Author

akhzarj commented Oct 15, 2024

Yes in your case that true.
Meanwhile in our case just one unsubscribe stops other events to receive.
If I remember correctly it was impacting also other services that was using the same multicast group address.

@duartenfonseca
Copy link
Collaborator

@akhzarj i am trying to reproduce one your issue. i am using the attached config files.
vsomeip-udp-client.json
vsomeip-udp-service.json

  1. my client subscribes initially to both 0x4465 and 0x4555.
  2. then I unsubscribe to 0x4555.
  3. my client still receives notifications from 0x4465.
    would this be reproducing it correctly?

thanks

@akhzarj
Copy link
Contributor Author

akhzarj commented Nov 22, 2024

Hi @duartenfonseca
One small but essential difference is the TTL value that is 3 in your configs.
That allows to recover the multicast endpoint.
If you will put max value like 0xFFFFFF (see https://www.autosar.org/fileadmin/standards/R22-11/FO/AUTOSAR_PRS_SOMEIPServiceDiscoveryProtocol.pdf#subsubsection.5.1.2.5 PRS_SOMEIPSD_00356])
Then it will not recover until next reboot.
And as I have mentioned in previews comment: #651 (comment)
It could stop receiving events for multiple clients/apps if they are using common service/multicast endpoint as well.

@duartenfonseca
Copy link
Collaborator

@akhzarj do you have any pcap logs of the behaviour? because even with the TTL 0xFFFFF it does not happen. When using the subscribe-sample/notify-sample, the new offer always restores the endpoint so it does not stay closed.
here is the pcap:
remove_multicast_endpoint.zip

@duartenfonseca
Copy link
Collaborator

on the logs we still see the messages being received on the client, for the other event which was not unsubscribed

@akhzarj
Copy link
Contributor Author

akhzarj commented Dec 9, 2024

Hi @duartenfonseca
Yes if the new offer message is sent, then definitely it will be recovered.
And new offer could be sent in some specific conditions (TTL expair, Repetition phase, etc) SomeIP SD
In case when no offer sent then no recovery happens. That issue we had faced.
If needed I can try to setup example that it will reproduced the issue.

@duartenfonseca
Copy link
Collaborator

@akhzarj an example would be helpful, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants