RE: [EXTERNAL] Re: [net-next PATCH v11 10/11] octeontx2-pf: Add devlink port support

-----Original Message-----
From: Jiri Pirko <jiri@resnulli.us>
Sent: Thursday, August 22, 2024 8:08 PM
To: Geethasowjanya Akula <gakula@marvell.com>
Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; kuba@kernel.org;
davem@davemloft.net; pabeni@redhat.com; edumazet@google.com; Sunil
Kovvuri Goutham [off-list ref]; Subbaraya Sundeep Bhatta
[off-list ref]; Hariprasad Kelam [off-list ref]
Subject: [EXTERNAL] Re: [net-next PATCH v11 10/11] octeontx2-pf: Add devlink
port support

Thu, Aug 22, 2024 at 03:20:30PM CEST, gakula@marvell.com wrote:

quoted

Register devlink port for the rvu representors.

Signed-off-by: Geetha sowjanya <gakula@marvell.com>
Reviewed-by: Simon Horman <horms@kernel.org>
---
.../ethernet/marvell/octeontx2.rst            | 39 ++++++++
.../net/ethernet/marvell/octeontx2/nic/rep.c  | 91 +++++++++++++++++++
.../net/ethernet/marvell/octeontx2/nic/rep.h  |  2 +
3 files changed, 132 insertions(+)

diff --git
a/Documentation/networking/device_drivers/ethernet/marvell/octeontx2.rs
t
b/Documentation/networking/device_drivers/ethernet/marvell/octeontx2.rs
t index 1132ae2d007c..33258cc18f45 100644
---
a/Documentation/networking/device_drivers/ethernet/marvell/octeontx2.rs
t

+++ b/Documentation/networking/device_drivers/ethernet/marvell/octeontx
+++ 2.rst

@@ -393,3 +393,42 @@ To remove the representors devices from the

system. Change the device to legacy
 - Change device to legacy mode::

	# devlink dev eswitch set pci/0002:1c:00.0 mode legacy
+
+
+RVU representors can be managed using devlink ports (see
+:ref:`Documentation/networking/devlink/devlink-port.rst <devlink_port>`)

interface.

quoted

+
+ - Show devlink ports of representors::
+
+	# devlink port
+
+Sample output::
+
+	# devlink port
+	pci/0002:1c:00.0/0: type eth netdev pf1vf0rep flavour physical port 1

splittable false

quoted

+	pci/0002:1c:00.0/1: type eth netdev pf1vf1rep flavour pcivf controller 0

pfnum 1 vfnum 1 external false splittable false

quoted

+	pci/0002:1c:00.0/2: type eth netdev pf1vf2rep flavour pcivf controller 0

pfnum 1 vfnum 2 external false splittable false

quoted

+	pci/0002:1c:00.0/3: type eth netdev pf1vf3rep flavour pcivf
+controller 0 pfnum 1 vfnum 3 external false splittable false
+
+Function attributes
+===================
+
+The RVU representor support function attributes for representors.
+Port function configuration of the representors are supported through

devlink eswitch port.

quoted

+
+MAC address setup
+-----------------
+
+RVU representor driver support devlink port function attr mechanism to
+setup MAC address. (refer to
+Documentation/networking/devlink/devlink-port.rst)
+
+ - To setup MAC address for port 2::
+
+	# devlink port function set pci/0002:1c:00.0/2 hw_addr
+5c:a1:1b:5e:43:11
+
+Sample output::
+
+	# devlink port show pci/0002:1c:00.0/2
+	pci/0002:1c:00.0/2: type eth netdev pf1vf2rep flavour pcivf controller 0

pfnum 1 vfnum 2 external false splittable false

quoted

+	function:
+		hw_addr 5c:a1:1b:5e:43:11

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c

b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
index 5f767b6e79c3..aab1784b5134 100644

--- a/drivers/net/ethernet/marvell/octeontx2/nic/rep.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/rep.c

@@ -28,6 +28,90 @@ MODULE_DESCRIPTION(DRV_STRING);

MODULE_LICENSE("GPL");  MODULE_DEVICE_TABLE(pci, rvu_rep_id_table);

+static int rvu_rep_dl_port_fn_hw_addr_get(struct devlink_port *port,
+					  u8 *hw_addr, int *hw_addr_len,
+					  struct netlink_ext_ack *extack) {
+	struct otx2_devlink *otx2_dl = devlink_priv(port->devlink);
+	int rep_id = port->index;
+	struct otx2_nic *priv;
+	struct rep_dev *rep;
+
+	priv = otx2_dl->pfvf;
+	rep = priv->reps[rep_id];

struct rep_dev contains struct devlink_port. Use container_of to get it from
port pointer.

Ok will fix it.

quoted

+	ether_addr_copy(hw_addr, rep->mac);
+	*hw_addr_len = ETH_ALEN;
+	return 0;
+}
+
+static int rvu_rep_dl_port_fn_hw_addr_set(struct devlink_port *port,
+					  const u8 *hw_addr, int hw_addr_len,
+					  struct netlink_ext_ack *extack) {
+	struct otx2_devlink *otx2_dl = devlink_priv(port->devlink);
+	int rep_id = port->index;
+	struct otx2_nic *priv;
+	struct rep_dev *rep;
+
+	priv = otx2_dl->pfvf;
+	rep = priv->reps[rep_id];
+	eth_hw_addr_set(rep->netdev, hw_addr);
+	ether_addr_copy(rep->mac, hw_addr);

You save the mac, yet you never use it. To be clear, this mac should be used for
the actual VF. I believe you are missing that code.

Was planning to implement in follow up patch.  But will add it in this series. 

quoted

+	return 0;
+}
+
+static const struct devlink_port_ops rvu_rep_dl_port_ops = {
+	.port_fn_hw_addr_get = rvu_rep_dl_port_fn_hw_addr_get,
+	.port_fn_hw_addr_set = rvu_rep_dl_port_fn_hw_addr_set, };
+
+static void
+rvu_rep_devlink_set_switch_id(struct otx2_nic *priv,
+			      struct netdev_phys_item_id *ppid) {
+	struct pci_dev *pdev = priv->pdev;
+	u64 id;
+
+	id = pci_get_dsn(pdev);

Is physical port using this switch_id as well? If not, it should.

It is used by the physical port as well.

quoted

+
+	ppid->id_len = sizeof(id);
+	put_unaligned_be64(id, &ppid->id);
+}
+
+static void rvu_rep_devlink_port_unregister(struct rep_dev *rep) {
+	devlink_port_unregister(&rep->dl_port);
+}
+
+static int rvu_rep_devlink_port_register(struct rep_dev *rep) {
+	struct devlink_port_attrs attrs = {};
+	struct otx2_nic *priv = rep->mdev;
+	struct devlink *dl = priv->dl->dl;
+	int err;
+
+	if (!(rep->pcifunc & RVU_PFVF_FUNC_MASK)) {
+		attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
+		attrs.phys.port_number = rvu_get_pf(rep->pcifunc);
+	} else {
+		attrs.flavour = DEVLINK_PORT_FLAVOUR_PCI_VF;
+		attrs.pci_vf.pf = rvu_get_pf(rep->pcifunc);
+		attrs.pci_vf.vf = rep->pcifunc & RVU_PFVF_FUNC_MASK;
+	}
+
+	rvu_rep_devlink_set_switch_id(priv, &attrs.switch_id);
+	devlink_port_attrs_set(&rep->dl_port, &attrs);
+
+	err = devl_port_register_with_ops(dl, &rep->dl_port, rep->rep_id,
+					  &rvu_rep_dl_port_ops);
+	if (err) {
+		dev_err(rep->mdev->dev, "devlink_port_register failed: %d\n",
+			err);
+		return err;
+	}
+	return 0;
+}
+
static int rvu_rep_get_repid(struct otx2_nic *priv, u16 pcifunc)  {
	int rep_id;

@@ -339,6 +423,7 @@ void rvu_rep_destroy(struct otx2_nic *priv)

	for (rep_id = 0; rep_id < priv->rep_cnt; rep_id++) {
		rep = priv->reps[rep_id];
		unregister_netdev(rep->netdev);
+		rvu_rep_devlink_port_unregister(rep);
		free_netdev(rep->netdev);
	}
	kfree(priv->reps);

@@ -381,6 +466,11 @@ int rvu_rep_create(struct otx2_nic *priv, struct

netlink_ext_ack *extack)

quoted

		snprintf(ndev->name, sizeof(ndev->name), "p%dv%drep",
			 rvu_get_pf(pcifunc), (pcifunc &

RVU_PFVF_FUNC_MASK));

quoted

+		err = rvu_rep_devlink_port_register(rep);
+		if (err)
+			goto exit;
+
+		SET_NETDEV_DEVLINK_PORT(ndev, &rep->dl_port);
		eth_hw_addr_random(ndev);
		err = register_netdev(ndev);
		if (err) {

@@ -402,6 +492,7 @@ int rvu_rep_create(struct otx2_nic *priv, struct

netlink_ext_ack *extack)

quoted

	while (--rep_id >= 0) {
		rep = priv->reps[rep_id];
		unregister_netdev(rep->netdev);
+		rvu_rep_devlink_port_unregister(rep);
		free_netdev(rep->netdev);
	}
	kfree(priv->reps);

diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/rep.h

b/drivers/net/ethernet/marvell/octeontx2/nic/rep.h
index 0cefa482f83c..d81af376bf50 100644

--- a/drivers/net/ethernet/marvell/octeontx2/nic/rep.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/rep.h

@@ -34,10 +34,12 @@ struct rep_dev {

	struct net_device *netdev;
	struct rep_stats stats;
	struct delayed_work stats_wrk;
+	struct devlink_port dl_port;
	u16 rep_id;
	u16 pcifunc;
#define RVU_REP_VF_INITIALIZED		BIT_ULL(0)
	u8 flags;
+	u8	mac[ETH_ALEN];
};

static inline bool otx2_rep_dev(struct pci_dev *pdev)
--
2.25.1