DFv2 sdk libraries API Reviews

[Novin Changizi]

Hello!

I have a number of libraries that I want to get reviewed so that we can put them in the partner sdk. I've chunked them up into separate smaller reviews.

There are two cl stacks, one is related to driver unit testing, the other is related to writing drivers themselves.

Stack 1 is related to writing driver components (DFv2): fxr/861331

Stack 2 is for driver unit testing (both DFv2 and DFv1): fxr/859970

Let me know if you have any questions!

Thanks,

-novin

[Suraj Malhotra]

It may be helpful for the API council to have a little bit more context about each library:

• The driver_{incoming,outgoing}_cpp libraries are mirrors of the `component_{incoming,outgoing}_cpp` libraries already in the SDK and exist to add support for driver transport FIDL protocols/services, which don't make sense in the context of the component version of the library.
• The driver_logging library exists to work around limitations in shared process namespace which precludes us from using the normal syslog C++ APIs in drivers.
• Lastly, the driver_component library exists to simplify the API presented to drivers, taking care of common boilerplate logic such as setting up the logger and namespace (which normally is done pre-main via libc hooks in a typical ELF component).
• The driver_runtime_testing library provides additional driver runtime APIs that allow a greater degree of control than normally needed by the driver for a test context. Rather than provide a fake driver runtime with a bespoke implementation, we reuse the driver runtime implementation as is (same shared library), and make use of an expanded API set such as the ability to control dispatcher execution from the test.
• The driver testing library provides helpers to simplify writing unit tests in a way that somewhat mirrors the driver component library, setting up the environment and namespace provided to the driver.

We could probably go into additional depth via an addendum doc if it is helpful.

-Suraj

[Novin Changizi]

Thanks for adding more context, I'll get started on an addendum doc too that I'll send out later today

[Hunter Freyer]

Hey Novin! 

This is a pretty hefty set of APIs, so I think it's worth doing an "in-person" calibration so you can walk us through it. In fact, we may need more than one.

Before that, though, for the APIs that mirror other APIs that are used in other contexts, the main thing I'd like to take a look at is whether we're sure there needs to be a "mirror API" at all. For example, for incoming/outgoing directory, have we attempted to make the CF libraries more extensible and support other transports? For logging, have we determined that it's not viable for the logging folks to own the definition of the Logger interface, and the driver folks to own an implementation of that interface and a factory for getting one in the "writing a driver" context? At the moment, we're looking at a lot of interfaces that are liable to diverge gradually from their non-driver counterparts.

For the incoming/outgoing libraries, I'd sync up with +Gary Bressler and +Yegor Pomortsev. For logging, +Christopher Johnson and +Miguel Flores.

Assuming we determine that code sharing isn't viable here, then I imagine the council's advice for these APIs would be rather straightforward: match the existing APIs as closely as possible.

For the APIs that aren't mirroring other functionality, would you mind moving them into a smaller number of CLs? Maybe one for testing and one for not? It's just a lot to take in right now @_@.

It'd also be helpful to have a companion doc that points out some of the more interesting design choices, and any questions you'd like to ask the council during the calibration. It doesn't need to be fancy, it's just to structure the conversation.

Should I try to book some time next week?

Thanks,

Hunter

--
You received this message because you are subscribed to the Google Groups "api-council" group.
To unsubscribe from this group and stop receiving emails from it, send an email to api-council...@fuchsia.dev.
To view this discussion on the web visit https://groups.google.com/a/fuchsia.dev/d/msgid/api-council/CADUkUJ4KUUPviDzzJnFKjXNx1j4EdRkdLfphGq8OOK7wRDHOGA%40mail.gmail.com.

[Novin Changizi]

Created a doc for the unit testing related libraries

https://docs.google.com/document/d/1Rijv-1tkjeYhoqg4M6oNpFDMSdMSY7yncIMpqChUK5M

[Suraj Malhotra]

Diverging on logging is temporary and should go away by EOY. We have unique constraints in drivers which don't make sense outside of the context of drivers (we cannot use globals, at least not when they exist in shared libraries). Once we solve this problem with namespaced linking we will deprecate the library in favor of using the typical logging libraries in the SDK. For the most part the actual exposed API surface of this library (logging macros) tries to match up with existing syslog libraries. The primary difference is in initialization and the prefix (we use FDF rather than FX or FUCHSIA)

The CF libraries for incoming and outgoing namespace were already modified to allow maximum reuse between our library and theirs. The primary dependency which they cannot add which we do is on the FIDL driver transport. Pulling this into their library would cause all users of that library to link in the driver runtime unnecessarily, and the APIs simply would not work nor make sense for non driver components. Having a common base interface we implement is not an option because a lot of the important bits are done via templates.

I imagine we can add some annotations into the CF libraries to make it clear that changing it should result in similar changes in the DF library.

-Suraj

[Novin Changizi]

I totally agree with these being hefty Hunter no worries, we can do an in-person calibration and next week works for me.

For the non-mirror libraries I have design docs, they might have become a bit outdated as far as the details go compared with what we have settled on, but would probably give some more context to the council:

unit testing design doc: go/dfv2-unit-testing

driver library design doc: https://docs.google.com/document/d/1Lvn4qGkwDHppRCw_tW_hsTZF1-hrH9B2agCZ0gTa9BE

Regarding the mirror libraries, as suraj said we are limited a bit at the moment. Drivers can share a process, so globals (which is how the component libraries do it today) don't work for us yet. The driver transport fidl dependency is also a concern with the incoming/outgoing libraries. We have tried to mirror those as much as possible.

I'll go ahead and rework the cls to mirror/non-mirror libraries if that is more helpful than each cl being one library.

Thanks,

-novin

[Gary Bressler]

Disregard this idea if it doesn't make sense, but would it be possible for the driver transport to be transparently handled by the shared library backend (libsvc), rather than the client library .h files? Optimally, both libraries could share the same source set, the only difference being the dynamic library they link in.

On Fri, May 26, 2023 at 2:04 PM Novin Changizi <nov...@google.com> wrote:

Hello!

Just a quick update, I have reworked the cls into just 3 cls now:

mirror libraries: https://fuchsia-review.git.corp.google.com/c/fuchsia/+/861332

driver component library: https://fuchsia-review.git.corp.google.com/c/fuchsia/+/861335

driver testing libraries: https://fuchsia-review.git.corp.google.com/c/fuchsia/+/859970

I also created the second addendum doc for the driver component library:

https://docs.google.com/document/d/1TzobRA6FVDa7v9I4bkDickT3SrT3p99ejdwoE8Gf8PM

And so all the links are in one place, this is the doc for the testing libraries:

https://docs.google.com/document/d/1Rijv-1tkjeYhoqg4M6oNpFDMSdMSY7yncIMpqChUK5M

Happy long weekend!

-novin

[Gary Bressler]

Oh, that idea is probably only relevant for the outgoing library because that's the one that uses libsvc, incoming just uses fdio, unless we find some way to put the non-common bits of incoming into a shared lib.

[Gary Bressler]

Is that something that could be templatized? 

On Fri, May 26, 2023 at 3:40 PM Suraj Malhotra <surajm...@google.com> wrote:

No, I don't think it's possible because the client handle types returned by the APIs are strongly typed eg `fdf::ClientEnd` vs `fidl::ClientEnd`. It's not a goal to hide that sort of detail from the driver author.

[Suraj Malhotra]

I guess that's possible (although it would make it more confusing for someone looking at the header). Looking at outgoing_directory, I can't really think of a way to move this logic into libsvc. It might be possible, but I think it would require moving a lot of service handling logic which lives in the fidl/component library layers into libsvc.

[Suraj Malhotra]

No, I don't think it's possible because the client handle types returned by the APIs are strongly typed eg `fdf::ClientEnd` vs `fidl::ClientEnd`. It's not a goal to hide that sort of detail from the driver author.

[Novin Changizi]

Hello!

Just a quick update, I have reworked the cls into just 3 cls now:

mirror libraries: https://fuchsia-review.git.corp.google.com/c/fuchsia/+/861332

driver component library: https://fuchsia-review.git.corp.google.com/c/fuchsia/+/861335

driver testing libraries: https://fuchsia-review.git.corp.google.com/c/fuchsia/+/859970

I also created the second addendum doc for the driver component library:

https://docs.google.com/document/d/1TzobRA6FVDa7v9I4bkDickT3SrT3p99ejdwoE8Gf8PM

And so all the links are in one place, this is the doc for the testing libraries:

https://docs.google.com/document/d/1Rijv-1tkjeYhoqg4M6oNpFDMSdMSY7yncIMpqChUK5M

Happy long weekend!

-novin

On Thu, May 25, 2023 at 11:37 AM Novin Changizi <nov...@google.com> wrote:

[Hunter Freyer]

Okay, here's how I think we ought to do this: 

I've booked two "in-person" calibration sessions: one for driver_component_cpp and the other for the driver testing libraries. If anyone wants to provide feedback but can't make it to one of the sessions, please add comments to the driver_component_cpp api review doc or the CL it links to. Ditto with the testing doc.

For the "mirror libraries", we'll do an asynchronous calibration via gerrit. Anyone can provide feedback on the CL itself, but +Gary Bressler has volunteered in particular to look at the incoming/outgoing directory mirror libraries, and +Christopher Johnson will take a look at the logging mirror library. Everyone: please provide any feedback on this CL before next Wednesday!

Note that any feedback provided as part of this calibration is not blocking; these are suggestions for potential improvements. The decision for whether to implement those suggestions or not lies with +Novin Changizi, +Jocelyn Dang, and +Christopher Anderson. 

Thanks all!

Hunter