niklas/cyclonedds
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

ignore all-zero durability service QoS in SEDP

Browse source 
For compatibility with TwinOaks CoreDX, ignore an all-zero durability
service QoS received over SEDP for volatile and transient-local
endpoints.

Signed-off-by: Erik Boasson <eb@ilities.com>
This commit is contained in:
Erik Boasson 2019-04-02 14:11:21 +02:00 • committed by eboasson
parent bd3188af5b
commit 638cab9291
2 changed files with 100 additions and 22 deletions
Download patch file Download diff file Expand all files Collapse all files

102
src/core/ddsi/src/q_plist.c
View file

@ -593,6 +593,11 @@ static void bswap_history_qospolicy (nn_history_qospolicy_t *q)
q->depth = bswap4 (q->depth);
}
static int history_qospolicy_allzero (const nn_history_qospolicy_t *q)
{
return q->kind == NN_KEEP_LAST_HISTORY_QOS && q->depth == 0;
}
int validate_history_qospolicy (const nn_history_qospolicy_t *q)
{
/* Validity of history setting and of resource limits are dependent,
@ -630,6 +635,11 @@ static void bswap_resource_limits_qospolicy (nn_resource_limits_qospolicy_t *q)
q->max_samples_per_instance = bswap4 (q->max_samples_per_instance);
}
static int resource_limits_qospolicy_allzero (const nn_resource_limits_qospolicy_t *q)
{
return q->max_samples == 0 && q->max_instances == 0 && q->max_samples_per_instance == 0;
}
int validate_resource_limits_qospolicy (const nn_resource_limits_qospolicy_t *q)
{
const int unlimited = NN_DDS_LENGTH_UNLIMITED;
@ -707,9 +717,18 @@ static void bswap_durability_service_qospolicy (nn_durability_service_qospolicy_
bswap_resource_limits_qospolicy (&q->resource_limits);
}
int validate_durability_service_qospolicy (const nn_durability_service_qospolicy_t *q)
static int durability_service_qospolicy_allzero (const nn_durability_service_qospolicy_t *q)
{
return (history_qospolicy_allzero (&q->history) &&
resource_limits_qospolicy_allzero (&q->resource_limits) &&
q->service_cleanup_delay.seconds == 0 && q->service_cleanup_delay.fraction == 0);
}
static int validate_durability_service_qospolicy_acceptzero (const nn_durability_service_qospolicy_t *q, bool acceptzero)
{
int res;
if (acceptzero && durability_service_qospolicy_allzero (q))
return 0;
if ((res = validate_duration (&q->service_cleanup_delay)) < 0)
{
DDS_TRACE("plist/validate_durability_service_qospolicy: duration invalid\n");
@ -723,6 +742,11 @@ int validate_durability_service_qospolicy (const nn_durability_service_qospolicy
return 0;
}
int validate_durability_service_qospolicy (const nn_durability_service_qospolicy_t *q)
{
return validate_durability_service_qospolicy_acceptzero (q, false);
}
static void bswap_liveliness_qospolicy (nn_liveliness_qospolicy_t *q)
{
q->kind = bswap4u (q->kind);
@ -1300,8 +1324,9 @@ static int do_guid (nn_guid_t *dst, uint64_t *present, uint64_t fl, int (*valid)
*dst = nn_ntoh_guid (*dst);
if (valid (dst, dd) < 0)
{
if (fl == PP_PARTICIPANT_GUID && vendor_is_twinoaks (dd->vendorid) &&
dst->entityid.u == 0 && ! NN_STRICT_P)
/* CoreDX once upon a time used to send out PARTICIPANT_GUID parameters with a 0 entity id, but it
that has long since changed (even if I don't know exactly when) */
if (fl == PP_PARTICIPANT_GUID && vendor_is_twinoaks (dd->vendorid) && dst->entityid.u == 0 && ! NN_STRICT_P)
{
DDS_LOG(DDS_LC_DISCOVERY, "plist(vendor %u.%u): rewriting invalid participant guid %x:%x:%x:%x\n",
dd->vendorid.id[0], dd->vendorid.id[1], PGUID (*dst));
@ -1523,7 +1548,6 @@ static int init_one_parameter
} \
return 0
Q (DURABILITY, durability);
Q (DURABILITY_SERVICE, durability_service);
Q (LIVELINESS, liveliness);
Q (DESTINATION_ORDER, destination_order);
Q (HISTORY, history);
@ -1534,6 +1558,27 @@ static int init_one_parameter
Q (TRANSPORT_PRIORITY, transport_priority);
#undef Q
case PID_DURABILITY_SERVICE:
if (dd->bufsz < sizeof (nn_durability_service_qospolicy_t))
{
DDS_TRACE("plist/init_one_parameter[pid=DURABILITY_SERVICE]: buffer too small\n");
return ERR_INVALID;
}
else
{
nn_durability_service_qospolicy_t *q = &dest->qos.durability_service;
/* All-zero durability service is illegal, but at least CoreDX sometimes advertises
it in some harmless cases. So accept all-zero durability service, then handle it
in final_validation, where we can determine whether it really is harmless or not */
memcpy (q, dd->buf, sizeof (*q));
if (dd->bswap)
bswap_durability_service_qospolicy (q);
if ((res = validate_durability_service_qospolicy_acceptzero (q, true)) < 0)
return res;
dest->qos.present |= QP_DURABILITY_SERVICE;
}
return 0;
/* PID_RELIABILITY handled differently because it (formally, for
static typing reasons) has a different type on the network
than internally, with the transformation between the two
@ -2253,9 +2298,9 @@ nn_plist_t *nn_plist_dup (const nn_plist_t *src)
return dst;
}
static int final_validation (nn_plist_t *dest)
static int final_validation (nn_plist_t *dest, nn_protocol_version_t protocol_version, nn_vendorid_t vendorid)
{
/* Resource limits & history are related, so if only one is given,
/* Resource limits & history are related, so if only one is given,
set the other to the default, claim it has been provided &
validate the combination. They can't be changed afterward, so
this is a reasonable interpretation. */
@ -2276,6 +2321,46 @@ static int final_validation (nn_plist_t *dest)
if ((res = validate_history_and_resource_limits (&dest->qos.history, &dest->qos.resource_limits)) < 0)
return res;
}
/* Durability service is sort-of accepted if all zeros, but only
for some protocol versions and vendors. We don't handle want
to deal with that case internally. Now that all QoS have been
parsed we know the setting of the durability QoS (the default
is always VOLATILE), and hence we can verify that the setting
is valid or delete it if irrelevant. */
if (dest->qos.present & QP_DURABILITY_SERVICE)
{
const nn_durability_kind_t durkind = (dest->qos.present & QP_DURABILITY) ? dest->qos.durability.kind : NN_VOLATILE_DURABILITY_QOS;
bool acceptzero;
/* Use a somewhat convoluted rule to decide whether or not to
"accept" an all-zero durability service setting, to find a
reasonable mix of strictness and compatibility */
if (protocol_version_is_newer (protocol_version))
acceptzero = true;
else if (NN_STRICT_P)
acceptzero = vendor_is_twinoaks (vendorid);
else
acceptzero = !vendor_is_eclipse (vendorid);
switch (durkind)
{
case NN_VOLATILE_DURABILITY_QOS:
case NN_TRANSIENT_LOCAL_DURABILITY_QOS:
/* pretend we never saw it if it is all zero */
if (acceptzero && durability_service_qospolicy_allzero (&dest->qos.durability_service))
dest->qos.present &= ~QP_DURABILITY_SERVICE;
break;
case NN_TRANSIENT_DURABILITY_QOS:
case NN_PERSISTENT_DURABILITY_QOS:
break;
}
/* if it is still present, it must be valid */
if (dest->qos.present & QP_DURABILITY_SERVICE)
{
int res;
if ((res = validate_durability_service_qospolicy (&dest->qos.durability_service)) < 0)
return res;
}
}
return 0;
}
@ -2341,10 +2426,9 @@ int nn_plist_init_frommsg
length = (unsigned short) (dd.bswap ? bswap2u (par->length) : par->length);
if (pid == PID_SENTINEL)
{
/* Sentinel terminates list, the length is ignored, DDSI
9.4.2.11. */
/* Sentinel terminates list, the length is ignored, DDSI 9.4.2.11. */
DDS_LOG(DDS_LC_PLIST, "%4x PID %x\n", (unsigned) (pl - src->buf), pid);
if ((res = final_validation (dest)) < 0)
if ((res = final_validation (dest, src->protocol_version, src->vendorid)) < 0)
{
nn_plist_fini (dest);
return ERR_INVALID;

20
src/docs/config.rst
View file

@ -1117,20 +1117,14 @@ OpenSplice.
Compatibility issues with TwinOaks
----------------------------------
Interoperability with TwinOaks CoreDX require (or used to require at some point in the
past):
In the default configuration there should be no interoperability issues with TwinOaks CoreDX,
although there is the aforementioned difference in interpretation of the meaning of the
‘autodispose_unregistered_instances’ QoS on the writer.
Interoperability with very old versions of CoreDX may require setting:
+ ``Compatibility/ManySocketsMode``: *true*
+ ``Compatibility/StandardsConformance``: *lax*
+ ``Compatibility/AckNackNumbitsEmptySet``: *0*
+ ``Compatibility/ExplicitlyPublishQosSetToDefault``: *true*
The ``ManySocketsMode`` option needed to be changed from the default, to ensure that
each domain participant has a unique locator; this was needed because TwinOaks CoreDX
DDS did not include the full GUID of a reader or writer if it needs to address just one,
but this is probably no longer the case. Note that the (old) behaviour of TwinOaks
CoreDX DDS has always been allowed by the specification.
The ``Compatibility/ExplicitlyPublishQosSetToDefault`` settings work around TwinOaks
CoreDX DDS’ use of incorrect default values for some of the QoS settings if they are not
explicitly supplied during discovery. It may be that this is no longer the case.
The exact version number of CoreDX starting with which these settings are no longer needed is
unknown, but it has certainly not been needed for several years.