mbox series

[RFC,0/2] I3C MCTP net driver

Message ID 20230413081139.15673-1-matt@codeconstruct.com.au (mailing list archive)
Headers show
Series I3C MCTP net driver | expand

Message

Matt Johnston April 13, 2023, 8:11 a.m. UTC
This series adds an I3C transport for the kernel's MCTP network
protocol. MCTP is a communication protocol between system components
(BMCs, drives, NICs etc), with higher level protocols such as NVMe-MI or
PLDM built on top of it (in userspace). It runs over various transports
such as I2C, PCIe, or I3C.

The mctp-i3c driver follows a similar approach to the kernel's existing
mctp-i2c driver, creating a "mctpi3cX" network interface for each
numbered I3C bus. Busses opt in to support by adding a "mctp-controller"
property to the devicetree:

&i3c0 {
        mctp-controller;
}

The driver will bind to MCTP class devices (DCR 0xCC) that are on a
supported I3C bus. Each bus is represented by a `struct mctp_i3c_bus`
that keeps state for the network device. An individual I3C device
(struct mctp_i3c_device) performs operations using the "parent"
mctp_i3c_bus object. The I3C notify/enumeration patch is needed so that
the mctp-i3c driver can handle creating/removing mctp_i3c_bus objects as
required.

The mctp-i3c driver is using the Provisioned ID as an identifier for
target I3C devices (the neighbour address), as that will be more stable
than the I3C dynamic address. The driver internally translates that to a
dynamic address for bus operations.

The driver has been tested using an AST2600 platform, with Jeremy's
In-Band Interrupt support patches. A remote endpoint has been tested
against Qemu, as well as using the target mode support in Aspeed's
vendor tree.

I'm sending this as an RFC for linux-i3c, then will submit the mctp-i3c
driver itself to the netdev list after it has had review here.

Cheers,
Matt

Jeremy Kerr (1):
  i3c: Add support for bus enumeration & notification

Matt Johnston (1):
  mctp i3c: MCTP I3C driver

 drivers/i3c/master.c        |  35 ++
 drivers/net/mctp/Kconfig    |   9 +
 drivers/net/mctp/Makefile   |   1 +
 drivers/net/mctp/mctp-i3c.c | 778 ++++++++++++++++++++++++++++++++++++
 include/linux/i3c/master.h  |  11 +
 5 files changed, 834 insertions(+)
 create mode 100644 drivers/net/mctp/mctp-i3c.c

Comments

Winiarska, Iwona May 8, 2023, 6:27 p.m. UTC | #1
On Thu, 2023-04-13 at 16:11 +0800, Matt Johnston wrote:
> This series adds an I3C transport for the kernel's MCTP network
> protocol. MCTP is a communication protocol between system components
> (BMCs, drives, NICs etc), with higher level protocols such as NVMe-MI or
> PLDM built on top of it (in userspace). It runs over various transports
> such as I2C, PCIe, or I3C.
> 
> The mctp-i3c driver follows a similar approach to the kernel's existing
> mctp-i2c driver, creating a "mctpi3cX" network interface for each
> numbered I3C bus. Busses opt in to support by adding a "mctp-controller"
> property to the devicetree:
> 
> &i3c0 {
>         mctp-controller;
> }

Hi,

Wouldn't creating "mctpi3cX" network interface for each I3C device rather than
I3C bus be a better fit to model MCTP over I3C?
Why are we following the mctp-i2c approach rather than mctp-serial?

MCTP over I3C is (like serial) point-to-point - connection between I3C
controller and each I3C device can be treated as separate "logical" bus (since
the only way that those devices can potentially communicate with each other is
using MCTP bridging) and, potentially, a separate MCTP network.

This would also simplify the series, as we would no longer need "mctp-
controller" property, and the driver would just follow the I3C class driver
model without the need for notifiers.

Thanks
-Iwona

> 
> The driver will bind to MCTP class devices (DCR 0xCC) that are on a
> supported I3C bus. Each bus is represented by a `struct mctp_i3c_bus`
> that keeps state for the network device. An individual I3C device
> (struct mctp_i3c_device) performs operations using the "parent"
> mctp_i3c_bus object. The I3C notify/enumeration patch is needed so that
> the mctp-i3c driver can handle creating/removing mctp_i3c_bus objects as
> required.
> 
> The mctp-i3c driver is using the Provisioned ID as an identifier for
> target I3C devices (the neighbour address), as that will be more stable
> than the I3C dynamic address. The driver internally translates that to a
> dynamic address for bus operations.
> 
> The driver has been tested using an AST2600 platform, with Jeremy's
> In-Band Interrupt support patches. A remote endpoint has been tested
> against Qemu, as well as using the target mode support in Aspeed's
> vendor tree.
> 
> I'm sending this as an RFC for linux-i3c, then will submit the mctp-i3c
> driver itself to the netdev list after it has had review here.
> 
> Cheers,
> Matt
> 
> Jeremy Kerr (1):
>   i3c: Add support for bus enumeration & notification
> 
> Matt Johnston (1):
>   mctp i3c: MCTP I3C driver
> 
>  drivers/i3c/master.c        |  35 ++
>  drivers/net/mctp/Kconfig    |   9 +
>  drivers/net/mctp/Makefile   |   1 +
>  drivers/net/mctp/mctp-i3c.c | 778 ++++++++++++++++++++++++++++++++++++
>  include/linux/i3c/master.h  |  11 +
>  5 files changed, 834 insertions(+)
>  create mode 100644 drivers/net/mctp/mctp-i3c.c
> 
> -- 
> 2.37.2
> 
>
Jeremy Kerr May 8, 2023, 11:42 p.m. UTC | #2
Hi Iwona,

> Wouldn't creating "mctpi3cX" network interface for each I3C device rather than
> I3C bus be a better fit to model MCTP over I3C?

Great question! A few reasons for this structure:

 - it is a closer match to existing usage of network interfaces and
   remote endpoints (say, IP, CAN, etc): you have an interface for
   representing the local hardware and stack state, which may provide a
   path for any devices reachable through that hardware.

 - a simpler addressing structure: you only need one *local* MCTP
   address (EID) for the whole bus, rather that one per remote device.

 - the presence/absence of interfaces is not dependent on what's
   connected to the bus. If we used a netdev per remote device, users
   would not be able to set up the local stack until a remote endpoint
   is bound (ie, userspace would need to configure every interface on
   i3c hot join). With this model, that stack state can persist over
   whatever may be happening to remote devices (unplug, reset,
   re-addressing, etc).

 - if there is ever a proposal for broadcast messages in the i3c
   transport binding, we would have to re-write it in this structure
   anyway.

> Why are we following the mctp-i2c approach rather than mctp-serial?

mctp-serial follows the same model: the local device is always present,
the difference here being that no physical addressing is required.
Routes to remote endpoints are added depending on remote endpoint state,
but the local state is preserved regardless of what is (or is not) on
the other end of the link.

> This would also simplify the series, as we would no longer need "mctp-
> controller" property, and the driver would just follow the I3C class driver
> model without the need for notifiers.

It would simplify the series, but we would be punting a lot of those
complexities over to userspace.

Cheers,


Jeremy
Winiarska, Iwona May 9, 2023, 9:14 p.m. UTC | #3
On Tue, 2023-05-09 at 07:42 +0800, Jeremy Kerr wrote:
> Hi Iwona,
> 
> > Wouldn't creating "mctpi3cX" network interface for each I3C device rather
> > than
> > I3C bus be a better fit to model MCTP over I3C?
> 
> Great question! A few reasons for this structure:
> 
>  - it is a closer match to existing usage of network interfaces and
>    remote endpoints (say, IP, CAN, etc): you have an interface for
>    representing the local hardware and stack state, which may provide a
>    path for any devices reachable through that hardware.
> 
>  - a simpler addressing structure: you only need one *local* MCTP
>    address (EID) for the whole bus, rather that one per remote device.
> 
>  - the presence/absence of interfaces is not dependent on what's
>    connected to the bus. If we used a netdev per remote device, users
>    would not be able to set up the local stack until a remote endpoint
>    is bound (ie, userspace would need to configure every interface on
>    i3c hot join). With this model, that stack state can persist over
>    whatever may be happening to remote devices (unplug, reset,
>    re-addressing, etc).
> 
>  - if there is ever a proposal for broadcast messages in the i3c
>    transport binding, we would have to re-write it in this structure
>    anyway.
> 
> > Why are we following the mctp-i2c approach rather than mctp-serial?
> 
> mctp-serial follows the same model: the local device is always present,
> the difference here being that no physical addressing is required.
> Routes to remote endpoints are added depending on remote endpoint state,
> but the local state is preserved regardless of what is (or is not) on
> the other end of the link.
> 
> > This would also simplify the series, as we would no longer need "mctp-
> > controller" property, and the driver would just follow the I3C class driver
> > model without the need for notifiers.
> 
> It would simplify the series, but we would be punting a lot of those
> complexities over to userspace.

Thanks for the explanation - and it makes perfect sense, I'm just worried that
it might be difficult to model certain use cases with this structure.

Specifically: 

1) This is a simplified example - the EID collision can happen further down the
network (with both I3C devices acting as bridges and having multiple MCTP
endpoints behind them), bottom line is - we're dealing two separate networks.
Since the network is a link property - do we have a way forward if we go with a
single network interface for I3C controller if we need to work in an environment
like this?

                             Network 1
                         +------------------+
                         |                  |
   Network ?             | device eid 0x12  |
+------------+     +-----+                  |
|  eid 0x11  +-----+     +------------------+
| controller |
|  eid 0x10  +-----+     +------------------+
+------------+     +-----+                  |
                         | device eid 0x12  |
                         |                  |
                         +------------------+
                             Network 2

2) This is the "normal" case. 0x11 sends MCTP message to 0x12 (by issuing IBI to
the controller). How will the routing table need to look like in order for the
message to be retransmited on the same network interface? What if we do not want
to forward? What would be the difference?

                             Network 1
                         +------------------+
                         |                  |
   Network 1             | device eid 0x11  |
+------------+     +-----+                  |
|            +-----+     +------------------+
| controller |
|  eid 0x10  +-----+     +------------------+
+------------+     +-----+                  |
                         | device eid 0x12  |
                         |                  |
                         +------------------+
                             Network 1

Thanks
-Iwona


> 
> Cheers,
> 
> 
> Jeremy
Jeremy Kerr May 10, 2023, 3:28 a.m. UTC | #4
Hi Iwona,

> Since the network is a link property - do we have a way forward if we go with a
> single network interface for I3C controller if we need to work in an environment
> like this?
> 
>                              Network 1
>                          +------------------+
>                          |                  |
>    Network ?             | device eid 0x12  |
> +------------+     +-----+                  |
> >  eid 0x11  +-----+     +------------------+
> > controller |
> >  eid 0x10  +-----+     +------------------+
> +------------+     +-----+                  |
>                          | device eid 0x12  |
>                          |                  |
>                          +------------------+
>                              Network 2

So I went digging around to see how this configuration fits with the
MCTP standards. A bit of background, which I'm sure you're already
familiar with, but just so we have a shared reference:

The MCTP Base spec (DSP0236) has a definition for a "MCTP bus":

    3.2.7
    bus
    a physical addressing domain shared between one or more platform
    components that share a common physical layer address space

Which would mean that your two devices there are on the same MCTP bus,
given we have shared physical address space. Our approach for the kernel
MCTP stack is for a netdev (ie, the interface) to represent the system's
connection to a bus.

While there's nothing explicitly defining whether a MCTP bus is required
to only host one MCTP network, all of the example topologies in DSP0236
seem to assume so.

Then: the MCTP I3C Transport Binding spec (DSP0233) introduces a
"logical bus" term, but with no formal definition. This would cover your
model above, where the two devices are on different logical busses, but
the same MCTP bus. There's an example of that in Figure 3.

However, there's still no clarification on whether these logical buses
can be on separate networks (and so operate within a separate EID
space). In fact, the Figure 3 example *requires* the logical busses to
be on the same network, as the Top Level Bus Owner exists on a separate
logical bus to its owned devices.

[There are also a couple of inconsistencies in this part of DSP0233;
there's a reference to "bridging between networks", which isn't possible
with the definitions of "bridging" and "network", as well as a footnote
that contradicts the definition of a MCTP bus. I'll see if I can chase
up some clarifications to those]

Anyhow: it doesn't seem to be prohibited by the standard, but I'm not
seeing much in the way of allowing it either. I'm happy to go by the
assumption that it's allowed, but it does seem unnecessarily eclectic to
me.

Back to the figure 3 example, and assuming we want to preserve the
existing concept of the netdev being the connection to a bus: this
*requires* us to implement the netdev-per-bus (rather than
netdev-per-remote-endpoint) model as proposed in this series, as there
is the shared logical bus between multiple remote endpoints.

In order to represent your use-case of multiple logical busses, I would
propose that we use a similar model to how logical ethernet interfaces
(ie, VLANs) are already implemented in Linux. Under that model, we could
allow new (logical) interfaces to be created as subordinate to the
existing (actual) interfaces, which would represent the system's
connection to a *logical* bus, and provide the mapping to a MCTP network.

For that, we would need a bit of netlink definition to allow
construction of the logical-bus netdev. I *think* we could then use the
existing neighbour table definitions to provide the partitioning of
devices over logical busses.

However: I would think it would be much simpler to just put the two
devices on the same i3c bus on the same MCTP network, and assign
distinct EIDs. Whether those devices could communicate can be defined by
bridging policy.

On to that, while slightly re-ordering your questions:

> 2) This is the "normal" case.
> 
>                              Network 1
>                          +------------------+
>                          |                  |
>    Network 1             | device eid 0x11  |
> +------------+     +-----+                  |
> >            +-----+     +------------------+
> > controller |
> >  eid 0x10  +-----+     +------------------+
> +------------+     +-----+                  |
>                          | device eid 0x12  |
>                          |                  |
>                          +------------------+
>                              Network 1
> 
> How will the routing table need to look like in order for the message
> to be retransmited on the same network interface? What if we do not
> want to forward? What would be the difference?

There are a couple of options here:

 * we set a "forwarding" boolean flag on the network, which specifies
   whether or not packets would be forwarded by the controller (acting
   as a bridge)

 * individual routes could be configured with bridging policy: in this
   case, the input interface and/or the source EIDs.

The former is definitely simpler, and was what I had originally
intended, but may not cover all use-cases. I'd be interested in your
thoughts there.

Cheers,


Jeremy
Winiarska, Iwona May 13, 2023, 8:21 p.m. UTC | #5
On Wed, 2023-05-10 at 11:28 +0800, Jeremy Kerr wrote:
> Hi Iwona,
> 
> > Since the network is a link property - do we have a way forward if we go
> > with a
> > single network interface for I3C controller if we need to work in an
> > environment
> > like this?
> > 
> >                              Network 1
> >                          +------------------+
> >                          |                  |
> >    Network ?             | device eid 0x12  |
> > +------------+     +-----+                  |
> > >  eid 0x11  +-----+     +------------------+
> > > controller |
> > >  eid 0x10  +-----+     +------------------+
> > +------------+     +-----+                  |
> >                          | device eid 0x12  |
> >                          |                  |
> >                          +------------------+
> >                              Network 2
> 
> So I went digging around to see how this configuration fits with the
> MCTP standards. A bit of background, which I'm sure you're already
> familiar with, but just so we have a shared reference:
> 
> The MCTP Base spec (DSP0236) has a definition for a "MCTP bus":
> 
>     3.2.7
>     bus
>     a physical addressing domain shared between one or more platform
>     components that share a common physical layer address space
> 
> Which would mean that your two devices there are on the same MCTP bus,
> given we have shared physical address space. Our approach for the kernel
> MCTP stack is for a netdev (ie, the interface) to represent the system's
> connection to a bus.
> 
> While there's nothing explicitly defining whether a MCTP bus is required
> to only host one MCTP network, all of the example topologies in DSP0236
> seem to assume so.
> 
> Then: the MCTP I3C Transport Binding spec (DSP0233) introduces a
> "logical bus" term, but with no formal definition. This would cover your
> model above, where the two devices are on different logical busses, but
> the same MCTP bus. There's an example of that in Figure 3.
> 
> However, there's still no clarification on whether these logical buses
> can be on separate networks (and so operate within a separate EID
> space). In fact, the Figure 3 example *requires* the logical busses to
> be on the same network, as the Top Level Bus Owner exists on a separate
> logical bus to its owned devices.
> 
> [There are also a couple of inconsistencies in this part of DSP0233;
> there's a reference to "bridging between networks", which isn't possible
> with the definitions of "bridging" and "network", as well as a footnote
> that contradicts the definition of a MCTP bus. I'll see if I can chase
> up some clarifications to those]
> 
> Anyhow: it doesn't seem to be prohibited by the standard, but I'm not
> seeing much in the way of allowing it either. I'm happy to go by the
> assumption that it's allowed, but it does seem unnecessarily eclectic to
> me.
> 
> Back to the figure 3 example, and assuming we want to preserve the
> existing concept of the netdev being the connection to a bus: this
> *requires* us to implement the netdev-per-bus (rather than
> netdev-per-remote-endpoint) model as proposed in this series, as there
> is the shared logical bus between multiple remote endpoints.
> 
> In order to represent your use-case of multiple logical busses, I would
> propose that we use a similar model to how logical ethernet interfaces
> (ie, VLANs) are already implemented in Linux. Under that model, we could
> allow new (logical) interfaces to be created as subordinate to the
> existing (actual) interfaces, which would represent the system's
> connection to a *logical* bus, and provide the mapping to a MCTP network.
> 
> For that, we would need a bit of netlink definition to allow
> construction of the logical-bus netdev. I *think* we could then use the
> existing neighbour table definitions to provide the partitioning of
> devices over logical busses.
> 
> However: I would think it would be much simpler to just put the two
> devices on the same i3c bus on the same MCTP network, and assign
> distinct EIDs. Whether those devices could communicate can be defined by
> bridging policy.

The only reason why I believe it would be possible for someone to come up with
such network layout would be the limited amount of EIDs available in MCTP 1.x,
in which case putting everything in the same MCTP network won't be an option.

I'm bringing this up, since I originally considered device-driver model that
goes with network controller per I3C device.
I think this is the only case in which it can be more complicated to model.
I just want to avoid a situation where we paint ourselves into a corner, and the
VLAN-like approach sounds good to me (if we ever end up needing it).

> 
> On to that, while slightly re-ordering your questions:
> 
> > 2) This is the "normal" case.
> > 
> >                              Network 1
> >                          +------------------+
> >                          |                  |
> >    Network 1             | device eid 0x11  |
> > +------------+     +-----+                  |
> > >            +-----+     +------------------+
> > > controller |
> > >  eid 0x10  +-----+     +------------------+
> > +------------+     +-----+                  |
> >                          | device eid 0x12  |
> >                          |                  |
> >                          +------------------+
> >                              Network 1
> > 
> > How will the routing table need to look like in order for the message
> > to be retransmited on the same network interface? What if we do not
> > want to forward? What would be the difference?
> 
> There are a couple of options here:
> 
>  * we set a "forwarding" boolean flag on the network, which specifies
>    whether or not packets would be forwarded by the controller (acting
>    as a bridge)
> 
>  * individual routes could be configured with bridging policy: in this
>    case, the input interface and/or the source EIDs.
> 
> The former is definitely simpler, and was what I had originally
> intended, but may not cover all use-cases. I'd be interested in your
> thoughts there.

Currently, I don't see any use cases where a "forwarding" boolean flag wouldn't
be enough, but we can go back to this in the future when we add bridging
support. 

Thanks
-Iwona

> 
> Cheers,
> 
> 
> Jeremy
Jeremy Kerr May 15, 2023, 2:29 a.m. UTC | #6
Hi Iwona,

> > However: I would think it would be much simpler to just put the two
> > devices on the same i3c bus on the same MCTP network, and assign
> > distinct EIDs. Whether those devices could communicate can be defined by
> > bridging policy.
> 
> The only reason why I believe it would be possible for someone to come up with
> such network layout would be the limited amount of EIDs available in MCTP 1.x,
> in which case putting everything in the same MCTP network won't be an option.

Yep, that's the reason why we implemented networks in the initial core
code - it's likely that we'll hit the EID limits at some point.

> I'm bringing this up, since I originally considered device-driver model that
> goes with network controller per I3C device.
> I think this is the only case in which it can be more complicated to model.
> I just want to avoid a situation where we paint ourselves into a corner, and the
> VLAN-like approach sounds good to me (if we ever end up needing it).

OK, neat - I think we have a plan should we need to represent the i3c
"logical bus" in future then.

> > The former is definitely simpler, and was what I had originally
> > intended, but may not cover all use-cases. I'd be interested in your
> > thoughts there.
> 
> Currently, I don't see any use cases where a "forwarding" boolean flag wouldn't
> be enough, but we can go back to this in the future when we add bridging
> support.

Sounds good then, thanks for the input!

Cheers,


Jeremy