mctpd: keep UUID on static-EID re-enrollment

[Ryan-CJ-Huang]

An endpoint that answers Set Endpoint ID with a different (its own
static) EID takes the assign-mismatch path, where change_peer_eid()
publishes the D-Bus object before query_peer_properties() has run.
publish_peer() registers the Common.UUID vtable only when peer->uuid
is already known, and is a no-op once the peer is published, so such
endpoints never expose their UUID on D-Bus even though the Get
Endpoint UUID query succeeds.

Observed on real hardware: an SMBus endpoint holding a static EID
enrolled via AssignEndpoint had no xyz.openbmc_project.Common.UUID
interface, while the same device enrolled via AssignEndpointStatic
(no EID mismatch) exposed it. The missing UUID prevents consumers
from correlating a dual-medium device across its endpoints.

Fix: publish from change_peer_eid() only when the peer was already
published (an EID change moves the object path); leave the first
publish to setup_added_peer(), which runs after the property queries.

Tests: added a regression test enrolling a static-EID endpoint via
AssignEndpoint and asserting the UUID interface is present. It fails
without the src change and passes with it; full test_mctpd.py suite
passes (79 tests).

[jk-ozlabs]

Hi Ryan,

Thanks for the PR!

An endpoint that answers Set Endpoint ID with a different (its own
static) EID takes the assign-mismatch path, where change_peer_eid()
publishes the D-Bus object before query_peer_properties() has run.

That sounds like the underlying issue though, perhaps we should address that instead,
and not publish as part of change_peer_eid()?

More out of completeness, could you let me know what behaviour you're seeing with the Set Endpoint ID response, and the response data from the prior Get Endpoint ID, and the behaviour you're expecting from mctpd?

(The UUID property should definitely be addressed, but I am more checking that we have the EID handling correct for this case)

[Ryan-CJ-Huang]

Hi Jeremy, thanks for the review!

Raw behaviour captured on hardware -- the endpoint is an Intel E810
NIC's SMBus/I2C sideband (it also exposes a PCIe VDM endpoint with the
same UUID):

• Get Endpoint ID response: 0a 02 00 2c 00 01
  (cc SUCCESS, EID 0x2c/44, simple endpoint, dynamic-EID capable per
  the EID-type bits)
• Set Endpoint ID requesting EID 10: response 0b 01 00 10 2c 00
  (cc SUCCESS, assignment status bits[5:4] = 01 "rejected", EID
  setting reports 0x2c/44, no pool)
• A subsequent Get Endpoint ID still reports 44.

So despite advertising a dynamic EID, the device rejects reassignment
and holds EID 44. mctpd's current handling -- adopting the EID the
device reports, per the existing
test_setup_endpoint_vary_set_eid_response behaviour -- seems right for
this case, and this PR proposes no change there. It only fixes the
D-Bus publish ordering so the Common.UUID interface is not silently
lost on that path (the same device enrolled via AssignEndpointStatic
with EID 44 exposes its UUID fine).

On the design point: agreed, dropping the publish from
change_peer_eid() entirely is cleaner. Looking at the call sites:

• both assign-mismatch paths and get_endpoint_peer() are followed by
  setup_added_peer(), which publishes after the property queries, so
  they need no change;
• the endpoint recovery path (peer_endpoint_recover()) relied on
  change_peer_eid() re-creating peer->path for the Connectivity
  PropertiesChanged emit, so it gains an explicit publish there.

I've reworked the branch that way and amended the commit accordingly.

[amboar]

By 'static' do you mean in the spec-sense, or the linux-stack sense?

In the case of the former, I'm wondering whether this is reasonable behaviour. DSP0236 v1.3.3 § 8.18.3 has the statement on 1097-1098:

Endpoints that support the option of being configured for one or more static EIDs shall also
support being configured to be dynamically assigned EIDs.

If we're sending a dynamic configuration, should it not accept that for conformance?

[Ryan-CJ-Huang]

Neither spec-static nor linux-stack static. The device (Intel E810 SMBus sideband) advertises dynamic-EID support in Get Endpoint ID, yet answers Set Endpoint ID with status "rejected" and keeps its current EID -- so agreed, per DSP0236 v1.3.3 § 8.18.3 that looks non-conformant on the device's part.

The PR doesn't try to bless that behaviour: mctpd already tolerates a device reporting a different EID than assigned (per the existing test_setup_endpoint_vary_set_eid_response), and the fix is only about keeping the D-Bus object intact on that tolerated path. I've reworded the test docstring to say "holds its current EID" instead of "static" to avoid implying the spec term.