From a292e800acf11d67a13fc7fe26bc747769b2ea60 Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Sat, 16 May 2020 11:12:45 +0200 Subject: [PATCH] Do not respond to an ignored SPDP message Signed-off-by: Erik Boasson --- src/core/ddsi/include/dds/ddsi/q_entity.h | 2 +- src/core/ddsi/src/q_ddsi_discovery.c | 67 ++++++++++------------- src/core/ddsi/src/q_entity.c | 5 +- 3 files changed, 32 insertions(+), 42 deletions(-) diff --git a/src/core/ddsi/include/dds/ddsi/q_entity.h b/src/core/ddsi/include/dds/ddsi/q_entity.h index 289ab84..8a4f22f 100644 --- a/src/core/ddsi/include/dds/ddsi/q_entity.h +++ b/src/core/ddsi/include/dds/ddsi/q_entity.h @@ -695,7 +695,7 @@ int writer_set_notalive (struct writer *wr, bool notify); /* Set when this proxy participant is not to be announced on the built-in topics yet */ #define CF_PROXYPP_NO_SPDP (1 << 2) -void new_proxy_participant (struct ddsi_domaingv *gv, const struct ddsi_guid *guid, uint32_t bes, const struct ddsi_guid *privileged_pp_guid, struct addrset *as_default, struct addrset *as_meta, const struct ddsi_plist *plist, dds_duration_t tlease_dur, nn_vendorid_t vendor, unsigned custom_flags, ddsrt_wctime_t timestamp, seqno_t seq); +bool new_proxy_participant (struct ddsi_domaingv *gv, const struct ddsi_guid *guid, uint32_t bes, const struct ddsi_guid *privileged_pp_guid, struct addrset *as_default, struct addrset *as_meta, const struct ddsi_plist *plist, dds_duration_t tlease_dur, nn_vendorid_t vendor, unsigned custom_flags, ddsrt_wctime_t timestamp, seqno_t seq); DDS_EXPORT int delete_proxy_participant_by_guid (struct ddsi_domaingv *gv, const struct ddsi_guid *guid, ddsrt_wctime_t timestamp, int isimplicit); int update_proxy_participant_plist_locked (struct proxy_participant *proxypp, seqno_t seq, const struct ddsi_plist *datap, ddsrt_wctime_t timestamp); diff --git a/src/core/ddsi/src/q_ddsi_discovery.c b/src/core/ddsi/src/q_ddsi_discovery.c index 1bc06c4..1f031d2 100644 --- a/src/core/ddsi/src/q_ddsi_discovery.c +++ b/src/core/ddsi/src/q_ddsi_discovery.c @@ -764,28 +764,17 @@ static int handle_SPDP_alive (const struct receiver_state *rst, seqno_t seq, dds maybe_add_pp_as_meta_to_as_disc (gv, as_meta); - new_proxy_participant - ( - gv, - &datap->participant_guid, - builtin_endpoint_set, - &privileged_pp_guid, - as_default, - as_meta, - datap, - lease_duration, - rst->vendor, - custom_flags, - timestamp, - seq - ); - - /* Force transmission of SPDP messages - we're not very careful - in avoiding the processing of SPDP packets addressed to others - so filter here */ + if (!new_proxy_participant (gv, &datap->participant_guid, builtin_endpoint_set, &privileged_pp_guid, as_default, as_meta, datap, lease_duration, rst->vendor, custom_flags, timestamp, seq)) { - int have_dst = - (rst->dst_guid_prefix.u[0] != 0 || rst->dst_guid_prefix.u[1] != 0 || rst->dst_guid_prefix.u[2] != 0); + /* If no proxy participant was created, don't respond */ + return 0; + } + else + { + /* Force transmission of SPDP messages - we're not very careful + in avoiding the processing of SPDP packets addressed to others + so filter here */ + int have_dst = (rst->dst_guid_prefix.u[0] != 0 || rst->dst_guid_prefix.u[1] != 0 || rst->dst_guid_prefix.u[2] != 0); if (!have_dst) { GVLOGDISC ("broadcasted SPDP packet -> answering"); @@ -795,27 +784,27 @@ static int handle_SPDP_alive (const struct receiver_state *rst, seqno_t seq, dds { GVLOGDISC ("directed SPDP packet -> not responding\n"); } - } - if (custom_flags & CF_PARTICIPANT_IS_DDSI2) - { - /* If we just discovered DDSI2, make sure any existing - participants served by it are made dependent on it */ - make_participants_dependent_on_ddsi2 (gv, &datap->participant_guid, timestamp); - } - else if (privileged_pp_guid.prefix.u[0] || privileged_pp_guid.prefix.u[1] || privileged_pp_guid.prefix.u[2]) - { - /* If we just created a participant dependent on DDSI2, make sure - DDSI2 still exists. There is a risk of racing the lease expiry - of DDSI2. */ - if (entidx_lookup_proxy_participant_guid (gv->entity_index, &privileged_pp_guid) == NULL) + if (custom_flags & CF_PARTICIPANT_IS_DDSI2) { - GVLOGDISC ("make_participants_dependent_on_ddsi2: ddsi2 "PGUIDFMT" is no more, delete "PGUIDFMT"\n", - PGUID (privileged_pp_guid), PGUID (datap->participant_guid)); - delete_proxy_participant_by_guid (gv, &datap->participant_guid, timestamp, 1); + /* If we just discovered DDSI2, make sure any existing + participants served by it are made dependent on it */ + make_participants_dependent_on_ddsi2 (gv, &datap->participant_guid, timestamp); } + else if (privileged_pp_guid.prefix.u[0] || privileged_pp_guid.prefix.u[1] || privileged_pp_guid.prefix.u[2]) + { + /* If we just created a participant dependent on DDSI2, make sure + DDSI2 still exists. There is a risk of racing the lease expiry + of DDSI2. */ + if (entidx_lookup_proxy_participant_guid (gv->entity_index, &privileged_pp_guid) == NULL) + { + GVLOGDISC ("make_participants_dependent_on_ddsi2: ddsi2 "PGUIDFMT" is no more, delete "PGUIDFMT"\n", + PGUID (privileged_pp_guid), PGUID (datap->participant_guid)); + delete_proxy_participant_by_guid (gv, &datap->participant_guid, timestamp, 1); + } + } + return 1; } - return 1; } static void handle_SPDP (const struct receiver_state *rst, ddsi_entityid_t pwr_entityid, seqno_t seq, const struct ddsi_serdata *serdata) @@ -1102,7 +1091,7 @@ static struct proxy_participant *implicitly_create_proxypp (struct ddsi_domaingv doing anything about (1). That means we fall back to the legacy mode of locally generating GIDs but leaving the system id unchanged if the remote is OSPL. */ actual_vendorid = (datap->present & PP_VENDORID) ? datap->vendorid : vendorid; - new_proxy_participant(gv, ppguid, 0, &privguid, new_addrset(), new_addrset(), &pp_plist, DDS_INFINITY, actual_vendorid, CF_IMPLICITLY_CREATED_PROXYPP, timestamp, seq); + (void) new_proxy_participant(gv, ppguid, 0, &privguid, new_addrset(), new_addrset(), &pp_plist, DDS_INFINITY, actual_vendorid, CF_IMPLICITLY_CREATED_PROXYPP, timestamp, seq); } else if (ppguid->prefix.u[0] == src_guid_prefix->u[0] && vendor_is_eclipse_or_opensplice (vendorid)) { diff --git a/src/core/ddsi/src/q_entity.c b/src/core/ddsi/src/q_entity.c index 9a8b5ef..5ea7def 100644 --- a/src/core/ddsi/src/q_entity.c +++ b/src/core/ddsi/src/q_entity.c @@ -4872,7 +4872,7 @@ static void free_proxy_participant(struct proxy_participant *proxypp) ddsrt_free (proxypp); } -void new_proxy_participant (struct ddsi_domaingv *gv, const struct ddsi_guid *ppguid, uint32_t bes, const struct ddsi_guid *privileged_pp_guid, struct addrset *as_default, struct addrset *as_meta, const ddsi_plist_t *plist, dds_duration_t tlease_dur, nn_vendorid_t vendor, unsigned custom_flags, ddsrt_wctime_t timestamp, seqno_t seq) +bool new_proxy_participant (struct ddsi_domaingv *gv, const struct ddsi_guid *ppguid, uint32_t bes, const struct ddsi_guid *privileged_pp_guid, struct addrset *as_default, struct addrset *as_meta, const ddsi_plist_t *plist, dds_duration_t tlease_dur, nn_vendorid_t vendor, unsigned custom_flags, ddsrt_wctime_t timestamp, seqno_t seq) { /* No locking => iff all participants use unique guids, and sedp runs on a single thread, it can't go wrong. FIXME, maybe? The @@ -4978,7 +4978,7 @@ void new_proxy_participant (struct ddsi_domaingv *gv, const struct ddsi_guid *pp { GVWARNING ("Remote secure participant "PGUIDFMT" not allowed\n", PGUID (*ppguid)); free_proxy_participant (proxypp); - return; + return false; } } #endif @@ -5004,6 +5004,7 @@ void new_proxy_participant (struct ddsi_domaingv *gv, const struct ddsi_guid *pp proxy_participant_create_handshakes (gv, proxypp); } #endif + return true; } int update_proxy_participant_plist_locked (struct proxy_participant *proxypp, seqno_t seq, const struct ddsi_plist *datap, ddsrt_wctime_t timestamp)