Clarification regarding "asCapable" and IEEE 802.1AS-2011 time-aware bridge

[spanceac]

Hi,

I'm trying the "IEEE 802.1AS-2011 time-aware bridge" functionality added with this commit 1486660e959f728e3a032cb832ee8b8ea9755ef3. In the profiles I set my bridge ports with "asCapable" to "true".

I see these two checks in the code for _if (port_isieee8021as(p)) https://github.com/openil/linuxptp/blob/master/port.c#L1306 https://github.com/openil/linuxptp/blob/master/port.c#L1332

If the ports have "asCapable" set to true, the _port_isieee8021as(p) function will return 0 and the code will the take the else paths, calling _clock_disable_syfurelay(p->clock); https://github.com/openil/linuxptp/blob/master/port.c#L1309 https://github.com/openil/linuxptp/blob/master/port.c#L1335

Is this intended behavior? I'm thinking that when the code was written, the developer wrongly though that _port_isieee8021as(p) will return 1 instead of 0, if the port has "asCapable" set to true.

Thank you and regards!

[vladimiroltean]

You may have found a bug, but not for the code path you think. The "asCapable" config option was introduced as part of the automotive profiles:

commit 3f764aec6a10bdff0c28a68f28fac06d5755c07f
Author: Vedang Patel <vedang.patel@intel.com>
Date:   Wed Oct 3 09:41:49 2018 -0700

    port: Add configurable option to set asCapable.

    If set to 'true', this unconditionally sets the asCapable variable. The
    usual checks will be applied to asCapable if it is set to 'auto'. The
    default value is 'auto'.

    This config option is needed by the Automotive Profile. The master will
    be able to send out Sync Message as soon as the daemon is started.

    Signed-off-by: Vedang Patel <vedang.patel@intel.com>

In my opinion it's a nasty hack to do it like this:

static int port_is_ieee8021as(struct port *p)
{
    if (p->asCapable == ALWAYS_CAPABLE) {
        return 0;
    }
    return p->follow_up_info ? 1 : 0;
}

    if (port_is_ieee8021as(p))
        do_checks_that_must_be_bypassed_on_automotive_profile();

and not like this:

static int port_is_ieee8021as(struct port *p)
{
    return p->follow_up_info ? 1 : 0;
}

    if (port_is_ieee8021as(p) && p->asCapable != ALWAYS_CAPABLE)
        do_checks_that_must_be_bypassed_on_automotive_profile();

The way I would fix things is I would remove the ALWAYS_CAPABLE check from port_is_ieee8021as. This would make the code you pointed out as buggy correct.

By the way, if you're not using the automotive profile you don't need to set asCapable in your config, and therefore you won't hit the bug. Needless to say, the gPTP bridge was not tested in combination with the automotive profile.

[spanceac]

Hi Vladimir,

I'm indeed using the automotive profile. I want to avoid changing the contents of _port_isieee8021as function since I don't know how that will propagate through all the code that calls this function. Instead, I think it's best to rewrite this:

            if (port_is_ieee8021as(p))
                clock_prepare_syfu_relay(p->clock, syn, m);
            else
                clock_disable_syfu_relay(p->clock);

as this:

            if (p->follow_up_info || p->asCapable == ALWAYS_CAPABLE)
                clock_prepare_syfu_relay(p->clock, syn, m);
            else
                clock_disable_syfu_relay(p->clock);

This way, the "if" condition will be true for:

• default gPTP.cfg profile where ports have _follow_upinfo set to 1
• automotive profile that has both _follow_upinfo set to 1 and asCapable set to "true"

Thank you and regards!