mctpd: Send Discovery Notify on Endpoint role set

[khangng-ampere]

Implement the first step of partial discovery, which is notifying the bus owner of our presence on the bus.

Tested: Discovery Notify message is sent when setting role on interface.

Jul 15 08:33:42 evb-endpoint mctpd[3322]: mctpd: read_message got from sockaddr_mctp_ext eid 8 net 1 type 0x00 if 2 hw len 2 0x80:08 len 3
Jul 15 08:33:42 evb-endpoint mctpd[3322]: mctpd: Failure completion code 0x05 from physaddr if 2 hw len 2 0x00:00
Jul 15 08:33:42 evb-endpoint mctpd[3322]: mctpd: Warning: discovery notify on interface 'mctppcie0' failed: Connection refused

(My Root Complex does not handle Discovery Notify message yet, but can confirm that the message is sent)

[jk-ozlabs]

The code looks fine, but why is this triggered by a dbus method? Shouldn't mctpd be in control of when the Discovery Notify messages are sent in the first place?

Even if not, this seems like the wrong abstraction to be exposing over dbus. What's the actual state/event that we want to expose in this scenario?

(also, can you share the PCIe binding driver?)

[khangng-ampere]

The code looks fine, but why is this triggered by a dbus method? Shouldn't mctpd be in control of when the Discovery Notify messages are sent in the first place?

Even if not, this seems like the wrong abstraction to be exposing over dbus. What's the actual state/event that we want to expose in this scenario?

Yeah, I agree, at the time, I wasn't thinking much about when to send the Discovery Notify messages, so I left that to the method caller to decide. I suppose we could do this inside mctpd anyway.

After giving it some thought, I think my main blocker to rework this PR is how to know when the PCI address is gone. This seems like something that netlink would provide, but atm I couldn't figure out how to tell apart media types of a link.

For example, is there a way to know where a link comes from, is it a mctp-i2c link or mctp-serial link?

EDIT: for the PCIe binding driver, I'm afraid it is not something I have the permission to share just yet.

[ThuBaNguyen]

The code looks fine, but why is this triggered by a dbus method? Shouldn't mctpd be in control of when the Discovery Notify messages are sent in the first place?

Even if not, this seems like the wrong abstraction to be exposing over dbus. What's the actual state/event that we want to expose in this scenario?

(also, can you share the PCIe binding driver?)

The D-Bus method provides the method consumer ability to decide the time which the BO can start initialize/discovery the BMC thru PCIe medium interface. Example, when the BMC boot up with the host is On, if BMC is set as Endpoint the other service can detect the available of the host then call Discovery Notify to BO to setEID for BMC. In theory, BMC does not need to do that but I don't think propose the Discovery Notify will cause any potential problem.

[jk-ozlabs]

In theory, BMC does not need to do that but I don't think propose the Discovery Notify will cause any potential problem.

My concern is allowing callers to drive details of the MCTP Control Protocol implementation, rather than having those decisions made by mctpd internally.

However, that could be matter of making this call more of a "notify the bus owner of our presence" operation rather than "send a Notify Discovery" message. For PCIe links, that would be equivalent, but other links (ie SMBus) may be a no-op (or could return an error?).

But still: DSP0238 defines the specific enumeration / discovery process, and section 6.9.1 gives specific events which would trigger the control protocol implementation to send a Notify Discovery message. I suspect that only mctpd would have full knowledge of when these events would occur. Are you proposing external components are tracking the MCTP enumeration state? Otherwise, how do they know when it is suitable to send a Notify Discovery message?

Some knowledge of the interactions to the PCIe binding driver might be helpful here. Can you share that?

Example, when the BMC boot up with the host is On, if BMC is set as Endpoint the other service can detect the available of the host then call Discovery Notify to BO to setEID for BMC.

Right, and that is best handled by mctpd automatically sending a Notify Discovery message when the "discovered" flag is not set, as per 6.9.1.

[ThuBaNguyen]

In theory, BMC does not need to do that but I don't think propose the Discovery Notify will cause any potential problem.

My concern is allowing callers to drive details of the MCTP Control Protocol implementation, rather than having those decisions made by mctpd internally.

However, that could be matter of making this call more of a "notify the bus owner of our presence" operation rather than "send a Notify Discovery" message. For PCIe links, that would be equivalent, but other links (ie SMBus) may be a no-op (or could return an error?).

But still: DSP0238 defines the specific enumeration / discovery process, and section 6.9.1 gives specific events which would trigger the control protocol implementation to send a Notify Discovery message. I suspect that only mctpd would have full knowledge of when these events would occur. Are you proposing external components are tracking the MCTP enumeration state? Otherwise, how do they know when it is suitable to send a Notify Discovery message?

I agree with above point, maybe we don't need public the Notify Discovery method to D-Bus.

Some knowledge of the interactions to the PCIe binding driver might be helpful here. Can you share that?

I'm asking my manager for this. Currently, we do internal review to make the clean code before create MR to kernel.

Example, when the BMC boot up with the host is On, if BMC is set as Endpoint the other service can detect the available of the host then call Discovery Notify to BO to setEID for BMC.

Right, and that is best handled by mctpd automatically sending a Notify Discovery message when the "discovered" flag is not set, as per 6.9.1.

Agree.

So We don't need support Notify Discovery to D-Bus method.

[khangng-ampere]

Piggyback on recent changes, I think the most suitable time to send Discovery Notify now is when user set link role to Endpoint. I updated my PR.

[jk-ozlabs]

Nice one. I'll take a look shortly.

[khangng-ampere]

Hi @jk-ozlabs, do you have any concerns regarding this?

[jk-ozlabs]

Ah, no concerns, just travelling at the moment. I'll look at merging when I'm back at the deck next week.

I have a v2.0 release queued though, any preference on whether this is merged before or after that?

[khangng-ampere]

Ah, no concerns, just travelling at the moment. I'll look at merging when I'm back at the deck next week.

I have a v2.0 release queued though, any preference on whether this is merged before or after that?

Let's do it after. I have a few more PRs to support the rest of the discovery process for endpoints, so they should probably be batched in one release.