Skip to content

Commit

Permalink
The PortId is defined as a couple of ClockId (an 8-bytes opaque) and …
Browse files Browse the repository at this point in the history
…the PortNumber (UInterger16).

The current implementation compares the entire PortId, consisting of ClockId and PortNumber, using a byte-to-byte comparison (memcmp).
This way doesn't consider the HOST's endianess on PortId part, giving wrong results in some cases.

Consider the following setup (assuming the same ClockId)

PortId_a = (ClockId, PortNumber=0x1234)
PortId_b = (ClockId, PortNumber=0x4512)

On a LittleEndian HOST, we obtain the following memory setup:

PortId_a = ... ClockId ... 0x34 0x12
PortId_b = ... ClockId ... 0x12 0x45

Using just a memcmp() call we obtain the WRONG result PortId_a > PortId_b, because the byte-to-byte comparison will check the PortNumber Lower-Byte (0x34 vs 0x12) first.

To fix the BCMA implementation we need to split the PortId comparison in:

 - ClockId comparison, still using memcmp() on a opaque byte stream
 - PortNumber, using a natural semantic comparison between integer numbers.

In other hands, following the ITU-T G.8275.2 Figure 4, we need to ensure the same ordering criteria on all places:

 a Compare portIdentities of receiver of A and sender of A
 b Compare portIdentities of receiver of B and sender of B
 c Compare portIdentities of sender of A and sender of B
 d Compare PortNumbers of receiver of A and receiver B

Signed-off-by: Luigi Mantellini <[email protected]>
Signed-off-by: Luigi Mantellini <[email protected]>
  • Loading branch information
comio authored and richardcochran committed Dec 5, 2022
1 parent 57d5c6a commit 02bc393
Showing 1 changed file with 14 additions and 3 deletions.
17 changes: 14 additions & 3 deletions bmc.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@
#include "bmc.h"
#include "ds.h"

static int portid_cmp(struct PortIdentity *a, struct PortIdentity *b)
{
int diff = memcmp(&a->clockIdentity, &b->clockIdentity, sizeof(a->clockIdentity));

if (diff == 0) {
diff = a->portNumber - b->portNumber;
}

return diff;
}

int dscmp2(struct dataset *a, struct dataset *b)
{
int diff;
Expand All @@ -35,7 +46,7 @@ int dscmp2(struct dataset *a, struct dataset *b)
* standard, since there is nothing we can do about it anyway.
*/
if (A < B) {
diff = memcmp(&b->receiver, &b->sender, sizeof(b->receiver));
diff = portid_cmp(&b->receiver, &b->sender);
if (diff < 0)
return A_BETTER;
if (diff > 0)
Expand All @@ -44,7 +55,7 @@ int dscmp2(struct dataset *a, struct dataset *b)
return 0;
}
if (A > B) {
diff = memcmp(&a->receiver, &a->sender, sizeof(a->receiver));
diff = portid_cmp(&a->receiver, &a->sender);
if (diff < 0)
return B_BETTER;
if (diff > 0)
Expand All @@ -53,7 +64,7 @@ int dscmp2(struct dataset *a, struct dataset *b)
return 0;
}

diff = memcmp(&a->sender, &b->sender, sizeof(a->sender));
diff = portid_cmp(&a->sender, &b->sender);
if (diff < 0)
return A_BETTER_TOPO;
if (diff > 0)
Expand Down

0 comments on commit 02bc393

Please sign in to comment.