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

Accept and generate ACKNACK/GAP with empty bitset

Browse source 
The DDSI spec version 2.1 forbade the use of ACKNACK/GAP messages with
an empty bitset, but most vendors used these forms anyway.  The DDSI
stack of Cyclone had code to avoid generating these (though with a bug
where it could generate an invalid GAP), but for no real benefit.
Because the other vendors used them anyway, the stack has always been
perfectly capable of handling them.

DDSI spec version 2.3 allows these forms, and so there's no value in
maintaining the old complications.  This also eliminates the invalid GAP
messages that could be generated at times.

Signed-off-by: Erik Boasson <eb@ilities.com>
This commit is contained in:
Erik Boasson 2019-06-06 10:22:37 +02:00 • committed by eboasson
parent 8a1980faa6
commit 4a4f092f7b
1 changed files with 4 additions and 41 deletions
Download patch file Download diff file Expand all files Collapse all files

41
src/core/ddsi/src/q_receive.c
View file

@ -104,20 +104,12 @@ static void maybe_set_reader_in_sync (struct proxy_writer *pwr, struct pwr_rd_ma
static int valid_sequence_number_set (const nn_sequence_number_set_t *snset)
{
if (fromSN (snset->bitmap_base) <= 0)
return 0;
if (snset->numbits <= 0 || snset->numbits > 256)
return 0;
return 1;
return (fromSN (snset->bitmap_base) > 0 && snset->numbits <= 256);
}
static int valid_fragment_number_set (const nn_fragment_number_set_t *fnset)
{
if (fnset->bitmap_base <= 0)
return 0;
if (fnset->numbits <= 0 || fnset->numbits > 256)
return 0;
return 1;
return (fnset->bitmap_base > 0 && fnset->numbits <= 256);
}
static int valid_AckNack (AckNack_t *msg, size_t size, int byteswap)
@ -136,25 +128,7 @@ static int valid_AckNack (AckNack_t *msg, size_t size, int byteswap)
msg->writerId = nn_ntoh_entityid (msg->writerId);
/* Validation following 8.3.7.1.3 + 8.3.5.5 */
if (!valid_sequence_number_set (&msg->readerSNState))
{
if (NN_STRICT_P)
return 0;
else
{
/* RTI generates AckNacks with bitmapBase = 0 and numBits = 0
(and possibly others that I don't know about) - their
Wireshark RTPS dissector says that such a message has a
length-0 bitmap, which is to expected given the way the
length is computed from numbits */
if (fromSN (msg->readerSNState.bitmap_base) == 0 &&
msg->readerSNState.numbits == 0)
; /* accept this one known case */
else if (msg->readerSNState.numbits == 0)
; /* maybe RTI, definitely Twinoaks */
else
return 0;
}
}
/* Given the number of bits, we can compute the size of the AckNack
submessage, and verify that the submessage is large enough */
if (size < ACKNACK_SIZE (msg->readerSNState.numbits))
@ -182,10 +156,7 @@ static int valid_Gap (Gap_t *msg, size_t size, int byteswap)
if (fromSN (msg->gapStart) <= 0)
return 0;
if (!valid_sequence_number_set (&msg->gapList))
{
if (NN_STRICT_P || msg->gapList.numbits != 0)
return 0;
}
/* One would expect gapStart < gapList.base, but it is not required by
the spec for the GAP to valid. */
if (size < GAP_SIZE (msg->gapList.numbits))
@ -564,7 +535,6 @@ static int add_Gap (struct nn_xmsg *msg, struct writer *wr, struct proxy_reader
struct nn_xmsg_marker sm_marker;
Gap_t *gap;
ASSERT_MUTEX_HELD (wr->e.lock);
assert (numbits > 0);
gap = nn_xmsg_append (msg, &sm_marker, GAP_SIZE (numbits));
nn_xmsg_submsg_init (msg, sm_marker, SMID_GAP);
gap->readerId = nn_hton_entityid (prd->e.guid.entityid);
@ -973,13 +943,6 @@ static int handle_AckNack (struct receiver_state *rst, nn_etime_t tnow, const Ac
to the remote reader. */
gapend = grow_gap_to_next_seq (wr, gapend);
}
if (gapnumbits == 0)
{
/* Avoid sending an invalid bitset */
gapnumbits = 1;
nn_bitset_set (gapnumbits, gapbits, 0);
gapend--;
}
/* The non-bitmap part of a gap message says everything <=
gapend-1 is no more (so the maximum sequence number it informs
the peer of is gapend-1); each bit adds one sequence number to