Re: [PATCH net-next] mctp: Add MCTP-over-serial transport binding

Hi Greg,

Thanks for the review, I'll get a v2 done. Some replies inline.

quoted

This change adds a MCTP Serial transport binding, as per DMTF
DSP0253.

What is "DMTF DSP0253"?  Can you provide a link to this or more
information that explains why this has to be a serial thing?

Sure, can do!

[it doesn't *have* to be a serial thing - MCTP supports multiple
physical layers as "transports" - current specs define transports for
serial, i2c and PCIe. The choice of transport will be dictated by
however you've connected your remote MCTP device(s). In this case
though, it's also handy for emulation, where we can transport MCTP
packets between virtual machines by connecting VMs' pty channels]

quoted

The 'mctp' utility provides the ldisc magic to set up the serial
link:

  # mctp link serial /dev/ttyS0 &
  # mctp link
  dev lo index 1 address 0x00:00:00:00:00:00 net 1 mtu 65536 up
  dev mctpserial0 index 5 address 0x(no-addr) net 1 mtu 68 down

Where is this magic mctp application?  I can't find it in my distro
packages anywhere.

The MCTP support is pretty new, and possibly a bit eclectic for general
distro inclusion at this stage. I'll include a ref to the tools.

quoted

+         Say y here if you need to connect to MCTP devices over serial.

Module name?

Ack.

quoted

+#define MCTP_SERIAL_VERSION    0x1

Where does this number come from?

Defined by the current spec; I'll add a comment.

quoted

+static DEFINE_IDA(mctp_serial_ida);

I think you forgot to clean this up when the module is removed.

Would it be possible to have the module exit called while we still have
ida bitmaps still allocated? It looks like a ldisc being open will
require a reference on the module; so a module remove will mean we have
no ldiscs in use, and therefore an empty ida, so the ida_destroy() will
always be a no-op.

Is there a path I'm missing here? Or is this more of a completeness
thing?

Cheers,


Jeremy