Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mctpd: Get Routing Table Entries method implementation #37

Open
khangng-ampere opened this issue Mar 22, 2024 · 3 comments
Open

mctpd: Get Routing Table Entries method implementation #37

khangng-ampere opened this issue Mar 22, 2024 · 3 comments

Comments

@khangng-ampere
Copy link

khangng-ampere commented Mar 22, 2024

#11 already mentioned this, however, a large part of the issue is more of static EIDs management. I'm creating this one for the Get Routing Table implementation.

I would like to discuss some of the side effects that we want this method to do, other than returning the routing table to D-Bus:

  1. Add the routes to kernel: The received table should be added to the kernel routing table (RTM_NEWROUTE).
  2. Add to D-Bus: mctpd should create the D-Bus objects for the peers, set .Degraded to True and request recovery to query the peers' properties.

Those effect might not be desirable for some applications that just want to know the routing table, like the one to only print the whole topology of the network. We might want to add a Publish argument to the method, to toggle the behaviour.

When Publish is true, I think we might only want to wait for the routing table response itself and the result of adding the endpoint to D-Bus. If adding to the kernel routing table or querying the properties fails, the D-Bus object will be removed and the application interested should listen to the remove signal, similar to how the health polling system works.

mctpd will also need to track the origin of the added objects, so as to remove the peers when the origin is removed.

I would like to know what everyone thinks on this, and also if I missed any considerations for the implementation.

@amboar
Copy link
Contributor

amboar commented Mar 26, 2024

  1. Add to D-Bus: mctpd should create the D-Bus objects for the peers, set .Degraded to True and request recovery to query the peers' properties

I'm not following why it's necessary to invoke the endpoint recovery machinery. Isn't it the case that we should query the peers based on the received routing information, and, once having done so, publish the peers on DBus? From there the lifetimes of endpoint objects on DBus are bounded by updates to the routing table as published by the upstream bus owner.

@khangng-ampere
Copy link
Author

khangng-ampere commented Mar 26, 2024

  1. Add to D-Bus: mctpd should create the D-Bus objects for the peers, set .Degraded to True and request recovery to query the peers' properties

I'm not following why it's necessary to invoke the endpoint recovery machinery. Isn't it the case that we should query the peers based on the received routing information, and, once having done so, publish the peers on DBus? From there the lifetimes of endpoint objects on DBus are bounded by updates to the routing table as published by the upstream bus owner.

Yes you're correct, on my second thought, we don't need to publish it while querying - just publish it after querying. The reason I mentioned the endpoint recovery mainly was that I was thinking about reusing the async aspect of the implementation - the properties queries should not block mctpd.

(Should all request/response pairs be async by default in mctpd? Would be quite a restructure, so I'm not so sure about that)

@amboar
Copy link
Contributor

amboar commented Mar 27, 2024

(Should all request/response pairs be async by default in mctpd? Would be quite a restructure, so I'm not so sure about that)

I'll defer to @jk-ozlabs and @mkj on that :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants