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

vSomeIP 3.5.1 #733

Merged
merged 119 commits into from
Oct 3, 2024
Merged

vSomeIP 3.5.1 #733

merged 119 commits into from
Oct 3, 2024

Conversation

fcmonteiro
Copy link
Collaborator

@fcmonteiro fcmonteiro commented Jun 27, 2024

  • Load Policies Lazy Load

  • Test - Processing SD messages with unknown type option

  • ensure endpoints before deletion

  • Improve "end of file" error handling

  • Enable debouncing of events & selective events

  • Revert "Test - Processing SD messages with unknown type"

  • Logs added to points of failure on registration process

  • One *.json to ignorem all

  • Someip-tp remote address rework

  • Fix crash in multicast_receive receive_cb

  • Generate network_test configs directly to build

  • Fix deadlock if binding of TCP client endpoint fails

  • Added missing includes of iomanip to support compilation on Mint

  • Cache not yet registered events

  • Return true to make sure endpoints are deleted

  • Byteorder implementation

  • Reorder of prepare_stop method

  • Allows applications in the same process using different security configurations

  • Fix to not ignore stop offers when sd acceptance is not required

  • Release 3.5.0
    =====================

  • Restructure Network Tests CMakeLists

  • policy.cpp unit test

  • Remove deprecated usage of byteorder and use bithelper

  • unblock endpoint when closing it

  • Find_Debounce_Time made configurable

  • Check if configuration_ pointer exists before using

  • Initialize routing_state_

  • Solved data race in configuration_impl class

  • utility.cpp unit tests

  • unit tests payload_impl

  • unit tests serializer

  • unit tests deserializer

  • Unit Tests for policy_manager_impl.cpp

  • Unit Test - Routing Manager set_routing_state

  • Move documentation to markdown

  • clang-format to verify the code vsomeip-lib

  • fix deadlock with event and message debounce feature

  • disabled some groups of tests

  • Try to force connection reset on suspend

  • Call availability handler on request service instance

  • Handle endpoint queue size underflow

  • Add Valgrind massif tool

  • Create new train after scheduling to avoid duplicate messages

  • Re-Added offer_tests group in all sanitizers tests

  • removed extra DLT logs of the policies print

  • COVESA-615: vsomeip.lck file not removed upon application termination

  • COVESA-527: Locally switch off -Wstringop-overflow

  • Create network regression test for specific issue

  • Change IndentPPDirectives rule in clang-format

  • applied auto in some identified lines by sonarqube

  • Add Valgrind memcheck

  • Ensure buffer is valid before de-referencing pointer

  • Renaming folder test and fixing typos

  • Fix minor formatting issues from some commits

  • Adds interger overflow check

  • Adds application name on cout logs

  • Adding helgrind, to test output

  • Boost 1.65 cleanup

  • network_test - Offer Stop Offer test

  • Support host name (env) for internal TCP communication

  • Remove cached configuration after app stops

  • Optimize tests/network-tests/CMakeLists.txt

  • Check if pointer exists before dereferencing it

  • avoid requiring valgrind locally

  • Fix Lock-Order-Inversion in policy_manager_impl

  • Revert "Fix to not ignore stop offers when sd acceptance is not required"

  • Fix timeout on offer_stop_offer test

  • Restore config_plugin_impl mutex

  • Rework [STOP_]OFFER command handling

  • some-tp memory consumption increasing fast

  • Remove dlt traces from memory_test

  • restart_routing_test enabled

  • Fix cyclic events

  • Tracing LOI

  • improve connection log on error path

  • added subscribe_notify groups to non-leak verification

  • Relocate hostname config command

  • allow subscribeACK if at least one offer was sent by SD

  • Improve "end of file" error handling

  • Update Clang-Format to Version 18

  • Debounce tests fix

  • Wireshark dissector for vsomeip protocol

  • Remove logging on operation cancel in connect_cbk

  • Add additional info on failure to open TCP port

  • Implementation of SOMEIPSD_00577

  • Refactor how niceness values are applied to threads

  • prevent race between event expiration/forwarding

  • Fix subscribe_notify_one tests

  • Fix missing/blocked subscription requests

  • change references to C++14 into C++17

  • Explicitly check whether an endpoint is in use

  • Enabled all network tests with whitelist

  • Fix target client id in local_send

  • remove redundants package import definitions

  • run unit tests on windows

  • Fix debounce network tests

  • type upgrade and temporary disable of test for QNX build

  • Force abort hanging detached threads

  • Application tests fix

  • Remove behavior from catch block in ~message

  • Stop/Start (network) endpoints on suspend/resume

  • Reduce the number of copy operations on event payloads

  • Sets linger to 0 in local tcp clients

  • Prevent exception re-throw in ~message

  • remove linger on local_tcp

  • Fix android traces build

  • Introduce stateful availability handler

  • Update the availability state

  • Reintroduces the TIME_WAIT for ltcei

  • Faster handlers lookup

  • Increase app registration timeout

  • Force endpoint restart if re registering

  • fix semaphore logs

  • fix compile issue with pthreads in android

  • Disabling set routing state unit test

  • Fixing get_policy_manager error with security disabled

.clang-format Outdated
# Webkit style was loosely based on the Qt style
BasedOnStyle: WebKit

Standard: c++14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was changed to c++17 in cf49723

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will check with the team

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in d452c10

@kheaactua
Copy link
Contributor

kheaactua commented Jul 26, 2024

I love that a clang-format file is being added!

Might I also propose a .cmake-format file?

@@ -1,6 +1,7 @@
/CMakeFiles
/build*/*
/examples/hello_world/build
/.idea/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add a few other common IDE/build artifacts? I have a PR for this too but it's still in a very early draft state.

/.cache
/compile_commands.json
.ninja
Makefile

@@ -362,7 +380,11 @@ endif ()
target_link_libraries(${VSOMEIP_NAME}-e2e ${VSOMEIP_NAME} ${Boost_LIBRARIES} ${USE_RT} ${DL_LIBRARY} ${SystemD_LIBRARIES})

if(${CMAKE_SYSTEM_NAME} MATCHES "QNX")
target_link_directories(${VSOMEIP_NAME} PUBLIC "${QNX_TARGET}/${CPUVARDIR}/io-sock/lib")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how well this generalizes to different QNX deployments. I would recommend that QNX linker paths be handled by a toolchain managed by the user (outside of this repo)

Comment on lines -625 to -642
find_program(ASCIIDOC_PATH asciidoc)
find_program(SOURCE_HIGHLIGHT_PATH source-highlight)
if ("${ASCIIDOC_PATH}" STREQUAL "ASCIIDOC_PATH-NOTFOUND")
message(WARNING "asciidoc is not installed. Readme can not be built.")
elseif("${SOURCE_HIGHLIGHT_PATH}" STREQUAL "SOURCE_HIGHLIGHT_PATH-NOTFOUND")
message(WARNING "source-highlight is not installed. Readme can not be built.")
else()
message("asciidoc found")
message("source-highlight found")
add_custom_command(TARGET doc
POST_BUILD
COMMAND asciidoc
-a version=${VSOMEIP_VERSION}
-b html
-o documentation/vsomeipUserGuide.html
${PROJECT_SOURCE_DIR}/documentation/vsomeipUserGuide)
endif()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this removed? I have some small changes that make it actually work (produce a PDF)

(formatting is different because I ran cmake-format on it)

find_program(ASCIIDOC_PATH asciidoc)
find_program(SOURCE_HIGHLIGHT_PATH source-highlight)
if("${ASCIIDOC_PATH}" STREQUAL "ASCIIDOC_PATH-NOTFOUND")
  message(WARNING "asciidoc is not installed. Readme can not be built. This may require a pip3 install asciidoc3")
elseif("${SOURCE_HIGHLIGHT_PATH}" STREQUAL "SOURCE_HIGHLIGHT_PATH-NOTFOUND")
  message(WARNING "source-highlight is not installed. Readme can not be built.")
else()
  message("asciidoc found")
  message("source-highlight found")
  add_custom_command(
    TARGET doc
    POST_BUILD
    COMMAND asciidoc -a version=${VSOMEIP_VERSION} -b html -o documentation/vsomeipUserGuide.html
            ${PROJECT_SOURCE_DIR}/documentation/vsomeipUserGuide
  )
endif()

find_program(A2X_PATH a2x)
if(NOT A2X_PATH)
  message(WARNING "a2x is not installed. Readme can not be built as a pdf.")
else()
  add_custom_command(
    TARGET doc
    POST_BUILD
    COMMAND
      a2x -v --no-xmllint -a version=${VSOMEIP_VERSION} -f pdf -a tabsize=4 --dblatex-opts
      '-P latex.output.revhistory=0 -P imagedata.default.scale=maxwidth=10cm,maxheight=8cm' -D
      ${PROJECT_BINARY_DIR}/documentation ${PROJECT_SOURCE_DIR}/documentation/vsomeipUserGuide
    BYPRODUCTS ${PROJECT_BINARY_DIR}/documentation/vsomeipUserGuide.pdf
  )
endif()

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We removed this because the documentation was moved to markdown

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, awesome.

CONTRIBUTING.md Outdated
Comment on lines 68 to 70
Standard: c++14
```
The C++ standard to be used, in this case, C++14.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c++17 (see cf49723 )

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in d452c10

CONTRIBUTING.md Outdated

```yaml
BasedOnStyle: WebKit
Standard: c++14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C++ 17 (see cf49723 )

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in d452c10

@kheaactua
Copy link
Contributor

These weren't touched in this PR, but Aram's team found several bad reference captures that should probably be addressed in this PR if not a separate PR.

I can't upload a patch file here, so embedding it:

From 6c77e2c07080e68272f180c5b235b0c4e9f4f5f9 Mon Jul 15 15:42:35 2024 -0400
From: Matthew Russell <[email protected]>
Date: Mon, 15 Jul 2024 15:42:35 -0400
Subject: [PATCH] Fix capturing references to stack variables

---
 implementation/configuration/src/configuration_impl.cpp   | 3 +++
 implementation/endpoints/src/tcp_client_endpoint_impl.cpp | 6 +++---
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/implementation/configuration/src/configuration_impl.cpp b/implementation/configuration/src/configuration_impl.cpp
index 65adb0e2..b693163a 100644
--- a/implementation/configuration/src/configuration_impl.cpp
+++ b/implementation/configuration/src/configuration_impl.cpp
@@ -132,6 +132,7 @@ configuration_impl::configuration_impl(const std::string &_path)
 #endif
       logfile_("/tmp/vsomeip.log"),
       loglevel_(vsomeip_v3::logger::level_e::LL_INFO),
+      is_suppress_events_enabled_(false),
       is_sd_enabled_(VSOMEIP_SD_DEFAULT_ENABLED),
       sd_protocol_(VSOMEIP_SD_DEFAULT_PROTOCOL),
       sd_multicast_(VSOMEIP_SD_DEFAULT_MULTICAST),
@@ -251,6 +252,8 @@ configuration_impl::configuration_impl(const configuration_impl &_other)
     sd_port_ = _other.sd_port_;
     sd_protocol_ = _other.sd_protocol_;
 
+    is_suppress_events_enabled_ = _other.is_suppress_events_enabled_;
+
     sd_initial_delay_min_ = _other.sd_initial_delay_min_;
     sd_initial_delay_max_ = _other.sd_initial_delay_max_;
     sd_repetitions_base_delay_= _other.sd_repetitions_base_delay_;
diff --git a/implementation/endpoints/src/tcp_client_endpoint_impl.cpp b/implementation/endpoints/src/tcp_client_endpoint_impl.cpp
index 2a4f0356..703b057f 100644
--- a/implementation/endpoints/src/tcp_client_endpoint_impl.cpp
+++ b/implementation/endpoints/src/tcp_client_endpoint_impl.cpp
@@ -268,7 +268,7 @@ void tcp_client_endpoint_impl::receive() {
         its_recv_buffer = recv_buffer_;
     }
     auto self = std::dynamic_pointer_cast< tcp_client_endpoint_impl >(shared_from_this());
-    strand_.dispatch([self, &its_recv_buffer](){
+    strand_.dispatch([self, its_recv_buffer](){
         self->receive(its_recv_buffer, 0, 0);
     });
 }
@@ -775,7 +775,7 @@ void tcp_client_endpoint_impl::receive_cbk(
             }
             its_lock.unlock();
             auto self = std::dynamic_pointer_cast< tcp_client_endpoint_impl >(shared_from_this());
-            strand_.dispatch([self, &_recv_buffer, _recv_buffer_size, its_missing_capacity](){
+            strand_.dispatch([self, _recv_buffer, _recv_buffer_size, its_missing_capacity](){
                 self->receive(_recv_buffer, _recv_buffer_size, its_missing_capacity);
             });
         } else {
@@ -947,7 +947,7 @@ void tcp_client_endpoint_impl::send_cbk(boost::system::error_code const &_error,
                 if (its_entry.first) {
                     auto self = std::dynamic_pointer_cast< tcp_client_endpoint_impl >(shared_from_this());
                     strand_.dispatch(
-                        [self, &its_entry]() { self->send_queued(its_entry);}
+                        [self, its_entry]() mutable { self->send_queued(its_entry);}
                     );
                 }
             }
-- 
2.45.2

@fcmonteiro
Copy link
Collaborator Author

These weren't touched in this PR, but Aram's team found several bad reference captures that should probably be addressed in this PR if not a separate PR.

To be honest I would prefer to have this as a separate PR.
This one is already quite big.

1. Place `vsomeip-dissector.lua` file in `~/.config/wireshark/plugins/vsomeip/vsomeip-dissector.lua`
(create `plugins` directory if it doesn't exist)
2. In wireshark go to `Analyze` > `Reload Lua Plugins`
3. In wireshark go to `Analyze` > `Enable Protocols` and search for `vsomeip` and enable it
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed these steps, is there any configuration for it? I see that vsomeip is now a protocol in my display filters, but I don't see it in the protocol list in the list of protocols (along sign SOME/IP and the 1000 other ones.)

Also, when I filter on vsomeip, I get no results. With SOME/IP I had to configure the port. When I forcefully tell it to decode as vsomeip (adding decode_as_entry: tcp.port,30510,SOME/IP,VSOMEIP in .config/wireshark/decode_as_entries) then nothing shows up.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @kheaactua
No extra configurations should be needed.
This plugin will only work for the vsomeip internal communication via tcp.
External someip messages will still be dissected by the default someip protocol on wireshark.

for the internal vsomeip communication via tcp, it should look something like this:
image
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that explains why I didn't see anything, the pcap file I used only had external TCP (all local was done with UDS.)

This is pretty cool.

Thanks for the info!

@@ -10,7 +10,7 @@ find_package(Threads REQUIRED)

set(VSOMEIP_NAME "vsomeip3")

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++14")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++17")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works, but it's a deprecated way of doing it. It should be:

target_compile_features(<target> PRIVATE cxx_std_17)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a fix for this here: #752

@@ -1129,6 +1130,7 @@ service_discovery_impl::on_message(
if(is_suspended_) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this be a good place to use std::scoped_lock?

@fcmonteiro fcmonteiro changed the title vSomeIP 3.5.x vSomeIP 3.5.1 Sep 12, 2024
@fcmonteiro fcmonteiro marked this pull request as ready for review September 30, 2024 14:59
@kheaactua
Copy link
Contributor

kheaactua commented Oct 1, 2024

hey @fcmonteiro

I get build errors on gcc (11.4) and clang (18.1.8):

implementation/runtime/src/application_impl.cpp:1513:46: error: could not convert ‘vsomeip_v3::configuration::get_policy_manager()’ from ‘shared_ptr<vsomeip_v3::policy_manager_impl>’ to ‘shared_ptr<vsomeip_v3::policy_manager>’
 1513 |     return configuration_->get_policy_manager();
      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
      |                                              |
      |                                              shared_ptr<vsomeip_v3::policy_manager_impl>

Building with VSOMEIP_DISABLE_SECURITY enabled.

return configuration_->get_policy_manager();
#endif
VSOMEIP_ERROR << __func__ << ": Must not be called with security disabled.";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be a VSOMEIP_FATAL?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i will check this

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kheaactua I ended up changing this to a warning instead of an error, since trying to get the manager isn't exactly an error, but could lead to one if this case is not properly handled.
If in some instance you use it, it shouldn't create any big issues

@duartenfonseca
Copy link
Collaborator

@kheaactua could you try it now, does it work correctly?

@kheaactua
Copy link
Contributor

@kheaactua could you try it now, does it work correctly?

hey @fcmonteiro, it appears to work, I just built this with gcc and clang with -DVSOMEIP_DISABLE_SECURITY

Even the build_tests target seemed to build now that I've changed from gtest 1.10 to 1.12

@fcmonteiro
Copy link
Collaborator Author

@kheaactua can you prepare a PR with these changes?
#733 (comment)

Thanks

@kheaactua
Copy link
Contributor

Any chance #734 can be integrated (full or in part) into this branch? :)

@duartenfonseca
Copy link
Collaborator

duartenfonseca commented Oct 3, 2024

Any chance #734 can be integrated (full or in part) into this branch? :)

Sorry @kheaactua it will have to be on the next version. i will make sure it will happen. in any case, the next versions will take less time be merged

@kheaactua
Copy link
Contributor

kheaactua commented Oct 3, 2024

@kheaactua can you prepare a PR with these changes? #733 (comment)

I pushed commit kheaactua@2f5ffe4 but I can't seem to target a PR to https://github.com/fcmonteiro/vsomeip/tree/vsomeip_3.5.x

I created #774 targeted to covesa/master. The commit is pretty independent from any of the changes so it can be pulled in in any order.

Fábio Monteiro and others added 27 commits October 3, 2024 16:47
Attempt to lock already destroyed mutex originates tombstone on
application shutdown. Threads being detatched sometimes destroy
a static mutex which will still be needed by other thread.
Shutdown mechanism shall be reviewed

Add method to force abort hanging detached threads
Use OS native methods to forcefully abort hanging detached threads
Add possibility to configure how long the application should wait
for detached threads (max_detached_thread_wait_time)
Sumary
Fix application and configuration network_test sanatizer and
valgrind errors

Details

- Locally the application_test is taking ~120sec to complete and
failing because of that.
- In application_test_single_process added a sleep before
its_client.stop(...)
- Reduce the number of threads in application_test_multiple_init,
as the test was failing with,
LeakSanitizer: CHECK failed: sanitizer_thread_registry.cpp:58
"((ThreadStatusFinished)) == ((status))" (0x3, 0x4) (tid=3860)
- ThreaSanitizers logger warnings are fixed by PR#558
- Remove shared_ptr of configuration_impl from the logger_impl.
- Added std::atomic protection to several class members.
- Added mutex on runtime_impl::get to prevent a data race on calling
std::make_shared
- Fix offer_test_external data race while accessing filters_ (event.cpp)
Removes behavior from `catch` block in `~message`.

This prevents a crash where an exception is thrown by `std::cout` when
trying to parse the exception:

```txt
/opt/idcevo/sysroots/cortexa76-poky-linux/usr/lib/libstdc++.so.6
No symbol table info available.
(this=this@entry=0x7fae98d270, __in_chrg=<optimized out>,
__vtt_parm=<optimized out>) at
/usr/src/debug/vsomeip/3_3.5.0+gitrAUTOINC+233db02be3-r12/git/implementation/logger/src/message.cpp:198
e = warning: could not convert 'std::exception' from the host
encoding (ANSI_X3.4-1968) to UTF-32.
This normally should not happen, please file a bug report.
```

Since logging is probably dead by this point already, there is no point
in trying to log anything else anyway.
Stop/Start (network) endpoints on suspend/resume

Stop all network endpoints on suspend to clear their buffers and start
them again after resume. The goal is to avoid "leftovers" from the
previus lifecycle being processed after resume as this might end in
additional load and "strange" behavior. This, e.g. might explain the
service availability toggling that can be seen in some tickets.
Reduce the number of copy operations on event payloads

Each event is storing its current payload. On the send-path the object
that is representing the payload is provided
by the user space, it is copied before being used. The copying is needed
as the object is still being used after the
send operation returns and it might be reused in user space (actually,
even the examples do so).
The current implementation does the copying in the
"event::prepare_update_payload_unlocked" method. This method is
used on the send- as well as on the receive-path. But for received
messages the copying is not needed, because the
payload object is created on the receive path and is never reused.
Additionally, the copying is done before checking
whether or not the payload object it will actually be used.

Therefore, the implementation was changed in the following way:
- The copying within "prepare_update_payload_unlocked" was removed
- For the send-path, copying is done within the
"application_impl::notify[_one]" methods
- Additionally, the manual comparison of the payload values in
"event::has_changed" was replaced by
the payload comparison operator. The operator was reimplemented to get
rid of the dynamic cast.

- [Performance]
Sets linger to 0 in local tcp clients

Also enables keep_alive for the local tcp server connections.
Adds the endpoint addr to the vsomeip logs (no new logs added)
By adding a `return` statement to the `catch` block, we can prevent the
caught exception from being re-thrown and causing an unhandled exception
crash.

This works by preventing the execution flow from reaching the end of the
exception handle which would re-throw the exception as specified in the
C++ [reference]:

> The currently handled exception is rethrown if control reaches the end
> of a handler of the function try block of a constructor or destructor.

[reference]: https://en.cppreference.com/w/cpp/language/try
Revert regression in local_tcp_client

subscribe_notify_test_diff_client_ids_diff_ports_autoconfig_udp_local_tcp
fails when setting linger in local tcp client endpoint.

This revert may fix the test, but will cause other issues in the local
tcp communication.
Remove its_data_size const

its_data_size value is being changed if the build has the following
definitions
present:
- USE_DLT
- ANDROID

Because it was a const value it makes the build fail.
 Introduce stateful availability handler

Save the last used state for each availability handler. Schedule
handlers in case of handler registration and
service requests. Only schedule a handler if the state has changed and
is not unknown.
Update the availability state on availability handler dispatching

The availability state is just set once at the time the handler is
registered. While this fixes thenproblem of multiple calls of the
registered handler with the same state at startup it also prevents it
from being called for either the available or the unavailabe state at
all if the initial state was one of these two instead of being unknown
at registration time. And this can happen if a re-registration of the
handler is being done, because the availability is already known on
client side. The result (in all known cases) are missing calls with the
state available, which results in subscriptions not being done and
functions that do not work.

This fixes this by storing the last reported availabilty state for each
(service, instance, major version, minor version) tuple for each
registered handler and updating the stored values whenever a change to
an availability state is reported.
Reintroduces the TIME_WAIT for ltcei

The linger option is necessary for the local tcp client endpoints,
as when closing a socket it will stay in TIME_WAIT for a default time,
in linux systems, of 120 seconds.
In this change we change the TIME_WAIT to 5 seconds.

Setting the TIME_WAIT to 0 seconds forces RST to always be sent
in response to a FIN. So a normal shutdown would look like this:
[FIN]
[ACK, FIN]
[ACK, RST]

Since this is endpoint for internal communication setting the TIME_WAIT
to 5 seconds should be enough to ensure the final ACK to the FIN arrives
to the server endpoint. With the 5 seconds the shutdown will look like
this:
[FIN]
[ACK, FIN]
[ACK]
Improve the performance of finding message handlers.

- Replace `std::map` with `std::unordered_map`, to get `O(1)` instead of
`O(log(n))` lookup complexity.
- Flatten the structure to reduce amount of lookups, going from 3 to 1
in the common case of first query succeeding.
- Use const ref to access the handler deque, avoiding the heap
allocation by the default constructor of `std::deque`.
- Preliminary result: CPU % reduction from 0.74% for the old
`find_service_handlers` to 0.22% with the new
  `find_handlers` for a nPDU-tunnel scenario with 25k messages/sec.
Increase app registration timeout to 3 seconds

On local tcp, when the multiple clients try to connect after a suspend,
the re registration can sometimes take more than the current 1 second
timeout.

This issue should be investigated later, but this change is urgently
needed.
Force restart of endpoint if re registering client

Forced restart is needed because in the case that the server end is no
longer connected we would only know after a send receiving a RST or
another error.
This would be already too late if we will need to send the routing_info
to re add the client id
Change dispatcher_counter_ to uint16_t so it can be printed out
correctly on the logs
Fix compile issue with pthreads in android

Change from pthread_cancel to pthread_kill in android builds, since the
former is not supported
The getter would try to access a policy_manager that is unavailable
when using the VSOMEIP_DISABLE_SECURITY flag.
It now returns an empty policy_manager object.
Changes:
- Restructure Network Tests CMakeLists
- policy.cpp unit test
- Remove deprecated usage of byteorder and use bithelper
- unblock endpoint when closing it
- Find_Debounce_Time made configurable
- Check if configuration_ pointer exists before using
- Initialize routing_state_
- Solved data race in configuration_impl class
- utility.cpp unit tests
- unit tests payload_impl
- unit tests serializer
- unit tests deserializer
- Unit Tests for policy_manager_impl.cpp
- Unit Test - Routing Manager set_routing_state
- Move documentation to markdown
- clang-format to verify the code vsomeip-lib
- fix deadlock with event and message debounce feature
- disabled some groups of tests
- Try to force connection reset on suspend
- Call availability handler on request service instance
- Handle endpoint queue size underflow
- Add Valgrind massif tool
- Create new train after scheduling to avoid duplicate messages
- Re-Added offer_tests group in all sanitizers tests
- removed extra DLT logs of the policies print
- COVESA-615: vsomeip.lck file not removed upon application termination
- COVESA-527: Locally switch off -Wstringop-overflow
- Create network regression test for specific issue
- Change IndentPPDirectives rule in clang-format
- applied auto in some identified lines by sonarqube
- Add Valgrind memcheck
- Ensure buffer is valid before de-referencing pointer
- Renaming folder test and fixing typos
- Fix minor formatting issues from some commits
- Adds interger overflow check
- Adds application name on cout logs
- Adding helgrind, to test output
- Boost 1.65 cleanup
- network_test - Offer Stop Offer test
- Support host name (env) for internal TCP communication
- Remove cached configuration after app stops
- Optimize tests/network-tests/CMakeLists.txt
- Check if pointer exists before dereferencing it
- avoid requiring valgrind locally
- Fix Lock-Order-Inversion in policy_manager_impl
- Revert "Fix to not ignore stop offers when sd acceptance is not required"
- Fix timeout on offer_stop_offer test
- Restore config_plugin_impl mutex
- Rework [STOP_]OFFER command handling
- some-tp memory consumption increasing fast
- Remove dlt traces from memory_test
- restart_routing_test enabled
- Fix cyclic events
- Tracing LOI
- improve connection log on error path
- added subscribe_notify groups to non-leak verification
- Relocate hostname config command
- allow subscribeACK if at least one offer was sent by SD
- Improve "end of file" error handling
- Update Clang-Format to Version 18
- Debounce tests fix
- Wireshark dissector for vsomeip protocol
- Remove logging on operation cancel in connect_cbk
- Add additional info on failure to open TCP port
- Implementation of SOMEIPSD_00577
- Refactor how niceness values are applied to threads
- prevent race between event expiration/forwarding
- Fix subscribe_notify_one tests
- Fix missing/blocked subscription requests
- change references to C++14 into C++17
- Explicitly check whether an endpoint is in use
- Enabled all network tests with whitelist
- Fix target client id in local_send
- remove redundants package import definitions
- run unit tests on windows
- Fix debounce network tests
- type upgrade and temporary disable of test for QNX build
- Force abort hanging detached threads
- Application tests fix
- Remove behavior from catch block in ~message
- Stop/Start (network) endpoints on suspend/resume
- Reduce the number of copy operations on event payloads
- Sets linger to 0 in local tcp clients
- Prevent exception re-throw in ~message
- remove linger on local_tcp
- Fix android traces build
- Introduce stateful availability handler
- Update the availability state
- Reintroduces the TIME_WAIT for ltcei
- Faster handlers lookup
- Increase app registration timeout
- Force endpoint restart if re registering
- fix semaphore logs
- fix compile issue with pthreads in android
- Disabling set routing state unit test
- Fixing get_policy_manager error with security disabled
@fcmonteiro fcmonteiro merged commit 0b83e24 into COVESA:master Oct 3, 2024
2 checks passed
#include <netinet/in.h>

#define SO_BINDTODEVICE 0x0800 /* restrict traffic to an interface */
#define IP_PKTINFO 25 /* int; send interface and src addr */
Copy link

@danielfr-haleytek danielfr-haleytek Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

I'm a bit puzzled by hard-coding this constant. Could you please elaborate why this is necessary?

To my understanding, this constant is defined in in.h that should be provided by the environment while building.

Background: This constant is not defined when using io-sock on QNX (or other FreeBSD-like network stacks) and we planned to use the availability of the constant to identify whether the code building for Linux- or FreeBSD-like network stacks in our PRs #772 #773 .

@fcmonteiro, tagging you as the author of the relevant commit.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @danielfr-haleytek this changes were done because our QNX build was failing, for the reasons you mentioned. The PR you mentioned will be tested on our side, we will give feedback there.

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.

5 participants