The struct is not in network order because it is already converted to
cpu's endianess before to call the bmca.
The patch works in my setup.
Ciao
Luigi
Il gio 17 nov 2022, 19:04 Geva, Erez <erez.geva....@siemens.com> ha scritto:
> On Mon, 2022-11-14 at 14:25 +0100, Luigi Mantellini wrote:
> > The PortId is defined as a couple of ClockId (an 8-bytes opaque) and
> > the PortNumber (UInterger16).
> > In order to correctly handle the PortId endianess, the comparison has
> > been split in the ClockIds comparison (using memcmp) and PortNumber
> > comparison.
>
> You code do not solve any endianess issue.
> The clock ID is byte stream.
> And port number is 16 bits, it require converting from network order to
> host order, which you do not need here. Becuase port ID are already in
> the same endian here.
>
> As struct PortIdentity is defined as "PACKED", it should not have any
> pad that may cause the memory compare to fail. Of having different
> values in the pad.
>
>
> So memcmp and your "portid_cmp" should give the same result.
>
> I would suggest to replace the memcmp and "portid_cmp" with existing
> pid_eq(const struct PortIdentity *a, const struct PortIdentity *b)
>
>
> And debug and find the real reason you have the bug.
> And than send a real bug fix :-)
>
>
> Erez
>
>
>
>
> > ---
> > bmc.c | 17 ++++++++++++++---
> > 1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/bmc.c b/bmc.c
> > index 6ac7aa0..ebc0789 100644
> > --- a/bmc.c
> > +++ b/bmc.c
> > @@ -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;
> > @@ -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)
> > @@ -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)
> > @@ -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)
>
>
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
