Skip to content

Commit c5cd225

Browse files
EduPonzEduPonz
authored
Explicitly pass vendor ID to readFromCdrMessage (#4583)
* Refs #20641: Add regression test Signed-off-by: EduPonz <[email protected]> * Refs #20641: Explicitely pass vendor ID to all readFromCdrMessage calls Signed-off-by: EduPonz <[email protected]> * Refs #20641: Treat unkown vendor IDs as ours Signed-off-by: EduPonz <[email protected]> --------- Signed-off-by: EduPonz <[email protected]>
1 parent a146b23 commit c5cd225

11 files changed

+187
-52
lines changed

‎src/cpp/rtps/builtin/data/ParticipantProxyData.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -468,11 +468,17 @@ bool ParticipantProxyData::readFromCDRMessage(
468468
}
469469
case fastdds::dds::PID_NETWORK_CONFIGURATION_SET:
470470
{
471+
VendorId_t local_vendor_id = source_vendor_id;
472+
if (c_VendorId_Unknown == local_vendor_id)
473+
{
474+
local_vendor_id = ((c_VendorId_Unknown == m_VendorId) ? c_VendorId_eProsima : m_VendorId);
475+
}
476+
471477
// Ignore custom PID when coming from other vendors
472-
if (c_VendorId_eProsima != source_vendor_id)
478+
if (c_VendorId_eProsima != local_vendor_id)
473479
{
474480
EPROSIMA_LOG_INFO(RTPS_PROXY_DATA,
475-
"Ignoring custom PID" << pid << " from vendor " << source_vendor_id);
481+
"Ignoring custom PID" << pid << " from vendor " << local_vendor_id);
476482
return true;
477483
}
478484

‎src/cpp/rtps/builtin/data/ReaderProxyData.cpp

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,8 @@ bool ReaderProxyData::readFromCDRMessage(
644644
auto param_process = [this, &network, &is_shm_transport_available, &should_filter_locators, source_vendor_id](
645645
CDRMessage_t* msg, const ParameterId_t& pid, uint16_t plength)
646646
{
647+
VendorId_t vendor_id = c_VendorId_Unknown;
648+
647649
switch (pid)
648650
{
649651
case fastdds::dds::PID_VENDORID:
@@ -656,6 +658,7 @@ bool ReaderProxyData::readFromCDRMessage(
656658
}
657659

658660
is_shm_transport_available &= (p.vendorId == c_VendorId_eProsima);
661+
vendor_id = p.vendorId;
659662
break;
660663
}
661664
case fastdds::dds::PID_DURABILITY:
@@ -844,11 +847,17 @@ bool ReaderProxyData::readFromCDRMessage(
844847
}
845848
case fastdds::dds::PID_NETWORK_CONFIGURATION_SET:
846849
{
850+
VendorId_t local_vendor_id = source_vendor_id;
851+
if (c_VendorId_Unknown == local_vendor_id)
852+
{
853+
local_vendor_id = ((c_VendorId_Unknown == vendor_id) ? c_VendorId_eProsima : vendor_id);
854+
}
855+
847856
// Ignore custom PID when coming from other vendors
848-
if (c_VendorId_eProsima != source_vendor_id)
857+
if (c_VendorId_eProsima != local_vendor_id)
849858
{
850859
EPROSIMA_LOG_INFO(RTPS_PROXY_DATA,
851-
"Ignoring custom PID" << pid << " from vendor " << source_vendor_id);
860+
"Ignoring custom PID" << pid << " from vendor " << local_vendor_id);
852861
return true;
853862
}
854863

@@ -991,12 +1000,18 @@ bool ReaderProxyData::readFromCDRMessage(
9911000

9921001
case fastdds::dds::PID_DISABLE_POSITIVE_ACKS:
9931002
{
1003+
VendorId_t local_vendor_id = source_vendor_id;
1004+
if (c_VendorId_Unknown == local_vendor_id)
1005+
{
1006+
local_vendor_id = ((c_VendorId_Unknown == vendor_id) ? c_VendorId_eProsima : vendor_id);
1007+
}
1008+
9941009
// Ignore custom PID when coming from other vendors except RTI Connext
995-
if ((c_VendorId_eProsima != source_vendor_id) &&
996-
(fastdds::rtps::c_VendorId_rti_connext != source_vendor_id))
1010+
if ((c_VendorId_eProsima != local_vendor_id) &&
1011+
(fastdds::rtps::c_VendorId_rti_connext != local_vendor_id))
9971012
{
9981013
EPROSIMA_LOG_INFO(RTPS_PROXY_DATA,
999-
"Ignoring custom PID" << pid << " from vendor " << source_vendor_id);
1014+
"Ignoring custom PID" << pid << " from vendor " << local_vendor_id);
10001015
return true;
10011016
}
10021017

@@ -1044,11 +1059,17 @@ bool ReaderProxyData::readFromCDRMessage(
10441059

10451060
case fastdds::dds::PID_DATASHARING:
10461061
{
1062+
VendorId_t local_vendor_id = source_vendor_id;
1063+
if (c_VendorId_Unknown == local_vendor_id)
1064+
{
1065+
local_vendor_id = ((c_VendorId_Unknown == vendor_id) ? c_VendorId_eProsima : vendor_id);
1066+
}
1067+
10471068
// Ignore custom PID when coming from other vendors
1048-
if (c_VendorId_eProsima != source_vendor_id)
1069+
if (c_VendorId_eProsima != local_vendor_id)
10491070
{
10501071
EPROSIMA_LOG_INFO(RTPS_PROXY_DATA,
1051-
"Ignoring custom PID" << pid << " from vendor " << source_vendor_id);
1072+
"Ignoring custom PID" << pid << " from vendor " << local_vendor_id);
10521073
return true;
10531074
}
10541075

‎src/cpp/rtps/builtin/data/WriterProxyData.cpp

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,8 @@ bool WriterProxyData::readFromCDRMessage(
631631
auto param_process = [this, &network, &is_shm_transport_available, &should_filter_locators, source_vendor_id](
632632
CDRMessage_t* msg, const ParameterId_t& pid, uint16_t plength)
633633
{
634+
VendorId_t vendor_id = c_VendorId_Unknown;
635+
634636
switch (pid)
635637
{
636638
case fastdds::dds::PID_VENDORID:
@@ -643,6 +645,7 @@ bool WriterProxyData::readFromCDRMessage(
643645
}
644646

645647
is_shm_transport_available &= (p.vendorId == c_VendorId_eProsima);
648+
vendor_id = p.vendorId;
646649
break;
647650
}
648651
case fastdds::dds::PID_DURABILITY:
@@ -838,12 +841,18 @@ bool WriterProxyData::readFromCDRMessage(
838841
}
839842
case fastdds::dds::PID_PERSISTENCE_GUID:
840843
{
844+
VendorId_t local_vendor_id = source_vendor_id;
845+
if (c_VendorId_Unknown == local_vendor_id)
846+
{
847+
local_vendor_id = ((c_VendorId_Unknown == vendor_id) ? c_VendorId_eProsima : vendor_id);
848+
}
849+
841850
// Ignore custom PID when coming from other vendors except RTI Connext
842-
if ((c_VendorId_eProsima != source_vendor_id) &&
843-
(fastdds::rtps::c_VendorId_rti_connext != source_vendor_id))
851+
if ((c_VendorId_eProsima != local_vendor_id) &&
852+
(fastdds::rtps::c_VendorId_rti_connext != local_vendor_id))
844853
{
845854
EPROSIMA_LOG_INFO(RTPS_PROXY_DATA,
846-
"Ignoring custom PID" << pid << " from vendor " << source_vendor_id);
855+
"Ignoring custom PID" << pid << " from vendor " << local_vendor_id);
847856
return true;
848857
}
849858

@@ -858,11 +867,17 @@ bool WriterProxyData::readFromCDRMessage(
858867
}
859868
case fastdds::dds::PID_NETWORK_CONFIGURATION_SET:
860869
{
870+
VendorId_t local_vendor_id = source_vendor_id;
871+
if (c_VendorId_Unknown == local_vendor_id)
872+
{
873+
local_vendor_id = ((c_VendorId_Unknown == vendor_id) ? c_VendorId_eProsima : vendor_id);
874+
}
875+
861876
// Ignore custom PID when coming from other vendors
862-
if (c_VendorId_eProsima != source_vendor_id)
877+
if (c_VendorId_eProsima != local_vendor_id)
863878
{
864879
EPROSIMA_LOG_INFO(RTPS_PROXY_DATA,
865-
"Ignoring custom PID" << pid << " from vendor " << source_vendor_id);
880+
"Ignoring custom PID" << pid << " from vendor " << local_vendor_id);
866881
return true;
867882
}
868883

@@ -973,12 +988,18 @@ bool WriterProxyData::readFromCDRMessage(
973988
}
974989
case fastdds::dds::PID_DISABLE_POSITIVE_ACKS:
975990
{
991+
VendorId_t local_vendor_id = source_vendor_id;
992+
if (c_VendorId_Unknown == local_vendor_id)
993+
{
994+
local_vendor_id = ((c_VendorId_Unknown == vendor_id) ? c_VendorId_eProsima : vendor_id);
995+
}
996+
976997
// Ignore custom PID when coming from other vendors except RTI Connext
977-
if ((c_VendorId_eProsima != source_vendor_id) &&
978-
(fastdds::rtps::c_VendorId_rti_connext != source_vendor_id))
998+
if ((c_VendorId_eProsima != local_vendor_id) &&
999+
(fastdds::rtps::c_VendorId_rti_connext != local_vendor_id))
9791000
{
9801001
EPROSIMA_LOG_INFO(RTPS_PROXY_DATA,
981-
"Ignoring custom PID" << pid << " from vendor " << source_vendor_id);
1002+
"Ignoring custom PID" << pid << " from vendor " << local_vendor_id);
9821003
return true;
9831004
}
9841005

@@ -1032,11 +1053,17 @@ bool WriterProxyData::readFromCDRMessage(
10321053

10331054
case fastdds::dds::PID_DATASHARING:
10341055
{
1056+
VendorId_t local_vendor_id = source_vendor_id;
1057+
if (c_VendorId_Unknown == local_vendor_id)
1058+
{
1059+
local_vendor_id = ((c_VendorId_Unknown == vendor_id) ? c_VendorId_eProsima : vendor_id);
1060+
}
1061+
10351062
// Ignore custom PID when coming from other vendors
1036-
if (c_VendorId_eProsima != source_vendor_id)
1063+
if (c_VendorId_eProsima != local_vendor_id)
10371064
{
10381065
EPROSIMA_LOG_INFO(RTPS_PROXY_DATA,
1039-
"Ignoring custom PID" << pid << " from vendor " << source_vendor_id);
1066+
"Ignoring custom PID" << pid << " from vendor " << local_vendor_id);
10401067
return true;
10411068
}
10421069

‎src/cpp/rtps/builtin/discovery/endpoint/EDPSimpleListeners.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ void EDPBasePUBListener::add_writer_from_change(
7272
auto temp_writer_data = edp->get_temporary_writer_proxies_pool().get();
7373

7474
if (temp_writer_data->readFromCDRMessage(&tempMsg, network,
75-
edp->mp_RTPSParticipant->has_shm_transport(), true))
75+
edp->mp_RTPSParticipant->has_shm_transport(), true, change->vendor_id))
7676
{
7777
if (temp_writer_data->guid().guidPrefix == edp->mp_RTPSParticipant->getGuid().guidPrefix)
7878
{
@@ -183,7 +183,7 @@ void EDPBaseSUBListener::add_reader_from_change(
183183
auto temp_reader_data = edp->get_temporary_reader_proxies_pool().get();
184184

185185
if (temp_reader_data->readFromCDRMessage(&tempMsg, network,
186-
edp->mp_RTPSParticipant->has_shm_transport(), true))
186+
edp->mp_RTPSParticipant->has_shm_transport(), true, change->vendor_id))
187187
{
188188
if (temp_reader_data->guid().guidPrefix == edp->mp_RTPSParticipant->getGuid().guidPrefix)
189189
{

‎src/cpp/rtps/builtin/discovery/participant/PDPListener.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ void PDPListener::onNewCacheChangeAdded(
107107
CDRMessage_t msg(change->serializedPayload);
108108
temp_participant_data_.clear();
109109
if (temp_participant_data_.readFromCDRMessage(&msg, true, parent_pdp_->getRTPSParticipant()->network_factory(),
110-
parent_pdp_->getRTPSParticipant()->has_shm_transport(), true))
110+
parent_pdp_->getRTPSParticipant()->has_shm_transport(), true, change_in->vendor_id))
111111
{
112112
// After correctly reading it
113113
change->instanceHandle = temp_participant_data_.m_key;

‎src/cpp/rtps/builtin/discovery/participant/PDPServerListener.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,8 @@ void PDPServerListener::onNewCacheChangeAdded(
141141
true,
142142
pdp_server()->getRTPSParticipant()->network_factory(),
143143
pdp_server()->getRTPSParticipant()->has_shm_transport(),
144-
true))
144+
true,
145+
change_in->vendor_id))
145146
{
146147
if (parent_pdp_->getRTPSParticipant()->is_participant_ignored(participant_data.m_guid.guidPrefix))
147148
{

‎src/cpp/rtps/participant/RTPSParticipantImpl.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2972,7 +2972,8 @@ bool RTPSParticipantImpl::fill_discovery_data_from_cdr_message(
29722972
true,
29732973
network_factory(),
29742974
has_shm_transport(),
2975-
false);
2975+
false,
2976+
c_VendorId_eProsima);
29762977

29772978
return ret && (data.m_guid.entityId == c_EntityId_RTPSParticipant);
29782979
}
@@ -2992,7 +2993,8 @@ bool RTPSParticipantImpl::fill_discovery_data_from_cdr_message(
29922993
&serialized_msg,
29932994
network_factory(),
29942995
has_shm_transport(),
2995-
false);
2996+
false,
2997+
c_VendorId_eProsima);
29962998

29972999
return ret && (data.guid().entityId.is_writer());
29983000
}
@@ -3012,7 +3014,8 @@ bool RTPSParticipantImpl::fill_discovery_data_from_cdr_message(
30123014
&serialized_msg,
30133015
network_factory(),
30143016
has_shm_transport(),
3015-
false);
3017+
false,
3018+
c_VendorId_eProsima);
30163019

30173020
return ret && (data.guid().entityId.is_reader());
30183021
}

‎test/blackbox/common/BlackboxTestsDiscovery.cpp

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,30 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
#include <gtest/gtest.h>
15+
16+
#include <atomic>
17+
#include <thread>
18+
1619
#ifndef _WIN32
1720
#include <stdlib.h>
1821
#endif // _WIN32
1922

20-
#include <thread>
23+
#include <gtest/gtest.h>
2124

25+
#include <fastdds/dds/domain/DomainParticipant.hpp>
26+
#include <fastdds/dds/domain/DomainParticipantFactory.hpp>
27+
#include <fastdds/dds/domain/DomainParticipantListener.hpp>
28+
#include <fastdds/dds/domain/qos/DomainParticipantQos.hpp>
2229
#include <fastdds/rtps/attributes/ServerAttributes.h>
2330
#include <fastdds/rtps/common/CDRMessage_t.h>
31+
#include <fastdds/rtps/transport/UDPv4TransportDescriptor.h>
2432
#include <fastrtps/xmlparser/XMLProfileManager.h>
33+
2534
#include <rtps/transport/test_UDPv4Transport.h>
2635
#include <utils/SystemInfo.hpp>
2736

2837
#include "BlackboxTests.hpp"
38+
#include "DatagramInjectionTransport.hpp"
2939
#include "PubSubReader.hpp"
3040
#include "PubSubWriter.hpp"
3141
#include "PubSubWriterReader.hpp"
@@ -2131,3 +2141,69 @@ TEST(Discovery, MultipleXMLProfileLoad)
21312141
thr_reader.join();
21322142
thr_writer.join();
21332143
}
2144+
2145+
//! Regression test for redmine issue 20641
2146+
TEST(Discovery, discovery_cyclone_participant_with_custom_pid)
2147+
{
2148+
using namespace eprosima::fastdds::dds;
2149+
using namespace eprosima::fastrtps::rtps;
2150+
2151+
/* Custom participant listener to count number of discovered participants */
2152+
class DiscoveryListener : public DomainParticipantListener
2153+
{
2154+
public:
2155+
2156+
void on_participant_discovery(
2157+
DomainParticipant*,
2158+
ParticipantDiscoveryInfo&& info) override
2159+
{
2160+
if (ParticipantDiscoveryInfo::DISCOVERED_PARTICIPANT == info.status)
2161+
{
2162+
discovered_participants_++;
2163+
}
2164+
else if (ParticipantDiscoveryInfo::REMOVED_PARTICIPANT == info.status)
2165+
{
2166+
discovered_participants_--;
2167+
}
2168+
}
2169+
2170+
uint8_t discovered_participants() const
2171+
{
2172+
return discovered_participants_;
2173+
}
2174+
2175+
private:
2176+
2177+
std::atomic<uint8_t> discovered_participants_{0};
2178+
};
2179+
2180+
/* Create a datagram injection transport */
2181+
using eprosima::fastdds::rtps::DatagramInjectionTransportDescriptor;
2182+
using eprosima::fastdds::rtps::DatagramInjectionTransport;
2183+
auto low_level_transport = std::make_shared<UDPv4TransportDescriptor>();
2184+
auto transport = std::make_shared<DatagramInjectionTransportDescriptor>(low_level_transport);
2185+
2186+
/* Disable builtin transport and add custom one */
2187+
DomainParticipantQos participant_qos = PARTICIPANT_QOS_DEFAULT;
2188+
participant_qos.transport().use_builtin_transports = false;
2189+
participant_qos.transport().user_transports.clear();
2190+
participant_qos.transport().user_transports.push_back(transport);
2191+
2192+
/* Create participant with custom transport and listener */
2193+
DiscoveryListener listener;
2194+
uint32_t domain_id = static_cast<uint32_t>(GET_PID()) % 230;
2195+
DomainParticipantFactory* factory = DomainParticipantFactory::get_instance();
2196+
DomainParticipant* participant = factory->create_participant(domain_id, participant_qos, &listener);
2197+
ASSERT_NE(nullptr, participant);
2198+
2199+
/* Inject a Cyclone DDS Data(p) with a custom PID that we also use */
2200+
auto receivers = transport->get_receivers();
2201+
ASSERT_FALSE(receivers.empty());
2202+
DatagramInjectionTransport::deliver_datagram_from_file(receivers, "datagrams/20641.bin");
2203+
2204+
/* Assert that the participant is discovered */
2205+
ASSERT_EQ(listener.discovered_participants(), 1u);
2206+
2207+
/* Clean up */
2208+
factory->delete_participant(participant);
2209+
}

0 commit comments

Comments
 (0)