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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <123a5491b8485f42c9279d397cdeb6358c610f6c.camel@codeconstruct.com.au>
Date:   Mon, 22 Nov 2021 15:16:55 +0800
From:   Jeremy Kerr <jk@...econstruct.com.au>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     netdev@...r.kernel.org, Matt Johnston <matt@...econstruct.com.au>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jiri Slaby <jirislaby@...nel.org>
Subject: 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.

> > 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]

> > 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.

> > +         Say y here if you need to connect to MCTP devices over serial.
> 
> Module name?

Ack.

> > +#define MCTP_SERIAL_VERSION    0x1
> 
> Where does this number come from?

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

> > +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