[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <a5349550f7b66ff53c0875b6bcefd20dcd165711.camel@codeconstruct.com.au>
Date: Thu, 07 Nov 2024 11:05:08 +0800
From: Matt Johnston <matt@...econstruct.com.au>
To: Khang Nguyen <khangng@...amperecomputing.com>, Jeremy Kerr
<jk@...econstruct.com.au>, Andrew Lunn <andrew+netdev@...n.ch>, "David S.
Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman
<horms@...nel.org>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: ampere-linux-kernel@...ts.amperecomputing.com, Phong Vo
<phong@...amperecomputing.com>, Thang Nguyen
<thang@...amperecomputing.com>, Khanh Pham <khpham@...erecomputing.com>,
Phong Vo <pvo@...erecomputing.com>, Quan Nguyen
<quan@...amperecomputing.com>, Chanh Nguyen <chanh@...amperecomputing.com>,
Thu Nguyen <thu@...amperecomputing.com>, Hieu Le
<hieul@...erecomputing.com>, openbmc@...ts.ozlabs.org,
patches@...erecomputing.com
Subject: Re: [PATCH net-next] net: mctp: Expose transport binding identifier
via IFLA attribute
Thanks Khang, this looks good.
On Tue, 2024-11-05 at 14:19 +0700, Khang Nguyen wrote:
> MCTP control protocol implementations are transport binding dependent.
> Endpoint discovery is mandatory based on transport binding.
> Message timing requirements are specified in each respective transport
> binding specification.
>
> However, we currently have no means to get this information from MCTP
> links.
>
> Add a IFLA_MCTP_PHYS_BINDING netlink link attribute, which represents
> the transport type using the DMTF DSP0239-defined type numbers, returned
> as part of RTM_GETLINK data.
>
> We get an IFLA_MCTP_PHYS_BINDING attribute for each MCTP link, for
> example:
>
> - 0x00 (unspec) for loopback interface;
> - 0x01 (SMBus/I2C) for mctpi2c%d interfaces; and
> - 0x05 (serial) for mctpserial%d interfaces.
>
> Signed-off-by: Khang Nguyen <khangng@...amperecomputing.com>
> ---
> drivers/net/mctp/mctp-i2c.c | 3 ++-
> drivers/net/mctp/mctp-i3c.c | 2 +-
> drivers/net/mctp/mctp-serial.c | 5 +++--
> include/net/mctp.h | 18 ++++++++++++++++++
> include/net/mctpdevice.h | 4 +++-
> include/uapi/linux/if_link.h | 1 +
> net/mctp/device.c | 12 +++++++++---
> 7 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/mctp/mctp-i2c.c b/drivers/net/mctp/mctp-i2c.c
> index 4dc057c121f5..86151a03570e 100644
> --- a/drivers/net/mctp/mctp-i2c.c
> +++ b/drivers/net/mctp/mctp-i2c.c
> @@ -877,7 +877,8 @@ static int mctp_i2c_add_netdev(struct mctp_i2c_client *mcli,
> goto err;
> }
>
> - rc = mctp_register_netdev(ndev, &mctp_i2c_mctp_ops);
> + rc = mctp_register_netdev(ndev, &mctp_i2c_mctp_ops,
> + MCTP_PHYS_BINDING_SMBUS);
> if (rc < 0) {
> dev_err(&mcli->client->dev,
> "register netdev \"%s\" failed %d\n",
> diff --git a/drivers/net/mctp/mctp-i3c.c b/drivers/net/mctp/mctp-i3c.c
> index 1bc87a062686..9adad59b8676 100644
> --- a/drivers/net/mctp/mctp-i3c.c
> +++ b/drivers/net/mctp/mctp-i3c.c
> @@ -607,7 +607,7 @@ __must_hold(&busdevs_lock)
> goto err_free_uninit;
> }
>
> - rc = mctp_register_netdev(ndev, NULL);
> + rc = mctp_register_netdev(ndev, NULL, MCTP_PHYS_BINDING_I3C);
> if (rc < 0) {
> dev_warn(&ndev->dev, "netdev register failed: %d\n", rc);
> goto err_free_netdev;
> diff --git a/drivers/net/mctp/mctp-serial.c b/drivers/net/mctp/mctp-serial.c
> index e63720ec3238..26c9a33fd636 100644
> --- a/drivers/net/mctp/mctp-serial.c
> +++ b/drivers/net/mctp/mctp-serial.c
> @@ -23,6 +23,7 @@
>
> #include <linux/mctp.h>
> #include <net/mctp.h>
> +#include <net/mctpdevice.h>
> #include <net/pkt_sched.h>
>
> #define MCTP_SERIAL_MTU 68 /* base mtu (64) + mctp header */
> @@ -470,7 +471,7 @@ static int mctp_serial_open(struct tty_struct *tty)
> spin_lock_init(&dev->lock);
> INIT_WORK(&dev->tx_work, mctp_serial_tx_work);
>
> - rc = register_netdev(ndev);
> + rc = mctp_register_netdev(ndev, NULL, MCTP_PHYS_BINDING_SERIAL);
> if (rc)
> goto free_netdev;
>
> @@ -492,7 +493,7 @@ static void mctp_serial_close(struct tty_struct *tty)
> struct mctp_serial *dev = tty->disc_data;
> int idx = dev->idx;
>
> - unregister_netdev(dev->netdev);
> + mctp_unregister_netdev(dev->netdev);
> ida_free(&mctp_serial_ida, idx);
> }
>
> diff --git a/include/net/mctp.h b/include/net/mctp.h
> index 28d59ae94ca3..1ecbff7116f6 100644
> --- a/include/net/mctp.h
> +++ b/include/net/mctp.h
> @@ -298,4 +298,22 @@ void mctp_routes_exit(void);
> int mctp_device_init(void);
> void mctp_device_exit(void);
>
> +/* MCTP IDs and Codes from DMTF specification
> + * "DSP0239 Management Component Transport Protocol (MCTP) IDs and Codes"
> + * https://www.dmtf.org/sites/default/files/standards/documents/DSP0239_1.11.1.pdf
> + */
> +enum mctp_phys_binding {
> + MCTP_PHYS_BINDING_UNSPEC = 0x00,
> + MCTP_PHYS_BINDING_SMBUS = 0x01,
> + MCTP_PHYS_BINDING_PCIE_VDM = 0x02,
> + MCTP_PHYS_BINDING_USB = 0x03,
> + MCTP_PHYS_BINDING_KCS = 0x04,
> + MCTP_PHYS_BINDING_SERIAL = 0x05,
> + MCTP_PHYS_BINDING_I3C = 0x06,
> + MCTP_PHYS_BINDING_MMBI = 0x07,
> + MCTP_PHYS_BINDING_PCC = 0x08,
> + MCTP_PHYS_BINDING_UCIE = 0x09,
> + MCTP_PHYS_BINDING_VENDOR = 0xFF,
> +};
> +
> #endif /* __NET_MCTP_H */
> diff --git a/include/net/mctpdevice.h b/include/net/mctpdevice.h
> index 5c0d04b5c12c..957d9ef924c5 100644
> --- a/include/net/mctpdevice.h
> +++ b/include/net/mctpdevice.h
> @@ -22,6 +22,7 @@ struct mctp_dev {
> refcount_t refs;
>
> unsigned int net;
> + enum mctp_phys_binding binding;
>
> const struct mctp_netdev_ops *ops;
>
> @@ -44,7 +45,8 @@ struct mctp_dev *mctp_dev_get_rtnl(const struct net_device *dev);
> struct mctp_dev *__mctp_dev_get(const struct net_device *dev);
>
> int mctp_register_netdev(struct net_device *dev,
> - const struct mctp_netdev_ops *ops);
> + const struct mctp_netdev_ops *ops,
> + enum mctp_phys_binding binding);
> void mctp_unregister_netdev(struct net_device *dev);
>
> void mctp_dev_hold(struct mctp_dev *mdev);
> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
> index 8516c1ccd57a..2575e0cd9b48 100644
> --- a/include/uapi/linux/if_link.h
> +++ b/include/uapi/linux/if_link.h
> @@ -1958,6 +1958,7 @@ struct ifla_rmnet_flags {
> enum {
> IFLA_MCTP_UNSPEC,
> IFLA_MCTP_NET,
> + IFLA_MCTP_PHYS_BINDING,
> __IFLA_MCTP_MAX,
> };
>
> diff --git a/net/mctp/device.c b/net/mctp/device.c
> index 3d75b919995d..26ce34b7e88e 100644
> --- a/net/mctp/device.c
> +++ b/net/mctp/device.c
> @@ -371,6 +371,8 @@ static int mctp_fill_link_af(struct sk_buff *skb,
> return -ENODATA;
> if (nla_put_u32(skb, IFLA_MCTP_NET, mdev->net))
> return -EMSGSIZE;
> + if (nla_put_u8(skb, IFLA_MCTP_PHYS_BINDING, mdev->binding))
> + return -EMSGSIZE;
> return 0;
> }
>
> @@ -385,6 +387,7 @@ static size_t mctp_get_link_af_size(const struct net_device *dev,
> if (!mdev)
> return 0;
> ret = nla_total_size(4); /* IFLA_MCTP_NET */
> + ret += nla_total_size(1); /* IFLA_MCTP_PHYS_BINDING */
> mctp_dev_put(mdev);
> return ret;
> }
> @@ -480,7 +483,8 @@ static int mctp_dev_notify(struct notifier_block *this, unsigned long event,
> }
>
> static int mctp_register_netdevice(struct net_device *dev,
> - const struct mctp_netdev_ops *ops)
> + const struct mctp_netdev_ops *ops,
> + enum mctp_phys_binding binding)
> {
> struct mctp_dev *mdev;
>
> @@ -489,17 +493,19 @@ static int mctp_register_netdevice(struct net_device *dev,
> return PTR_ERR(mdev);
>
> mdev->ops = ops;
> + mdev->binding = binding;
>
> return register_netdevice(dev);
> }
>
> int mctp_register_netdev(struct net_device *dev,
> - const struct mctp_netdev_ops *ops)
> + const struct mctp_netdev_ops *ops,
> + enum mctp_phys_binding binding)
> {
> int rc;
>
> rtnl_lock();
> - rc = mctp_register_netdevice(dev, ops);
> + rc = mctp_register_netdevice(dev, ops, binding);
> rtnl_unlock();
>
> return rc;
Reviewed-by: Matt Johnston <matt@...econstruct.com.au>
Powered by blists - more mailing lists