2024-02-24 12:44:30

by Karthik Sundaravel

[permalink] [raw]
Subject: [PATCH v4] ice: Add get/set hw address for VFs using devlink commands

Changing the MAC address of the VFs are not available
via devlink. Add the function handlers to set and get
the HW address for the VFs.

Signed-off-by: Karthik Sundaravel <[email protected]>
---
drivers/net/ethernet/intel/ice/ice_devlink.c | 88 +++++++++++++++++++-
1 file changed, 87 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index 80dc5445b50d..c3813edd6a76 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -1576,6 +1576,91 @@ void ice_devlink_destroy_pf_port(struct ice_pf *pf)
devlink_port_unregister(&pf->devlink_port);
}

+/**
+ * ice_devlink_port_get_vf_mac_address - .port_fn_hw_addr_get devlink handler
+ * @port: devlink port structure
+ * @hw_addr: MAC address of the port
+ * @hw_addr_len: length of MAC address
+ * @extack: extended netdev ack structure
+ *
+ * Callback for the devlink .port_fn_hw_addr_get operation
+ * Return: zero on success or an error code on failure.
+ */
+
+static int ice_devlink_port_get_vf_mac_address(struct devlink_port *port,
+ u8 *hw_addr, int *hw_addr_len,
+ struct netlink_ext_ack *extack)
+{
+ struct devlink_port_attrs *attrs = &port->attrs;
+ struct devlink_port_pci_vf_attrs *pci_vf;
+ struct devlink *devlink = port->devlink;
+ struct ice_pf *pf;
+ struct ice_vf *vf;
+ int vf_id;
+
+ pf = devlink_priv(devlink);
+ if (attrs->flavour == DEVLINK_PORT_FLAVOUR_PCI_VF) {
+ pci_vf = &attrs->pci_vf;
+ vf_id = pci_vf->vf;
+ } else {
+ NL_SET_ERR_MSG_MOD(extack, "Unable to get the vf id");
+ return -EADDRNOTAVAIL;
+ }
+ vf = ice_get_vf_by_id(pf, vf_id);
+ if (!vf) {
+ NL_SET_ERR_MSG_MOD(extack, "Unable to get the vf");
+ return -EINVAL;
+ }
+ ether_addr_copy(hw_addr, vf->dev_lan_addr);
+ *hw_addr_len = ETH_ALEN;
+
+ ice_put_vf(vf);
+ return 0;
+}
+
+/**
+ * ice_devlink_port_set_vf_mac_address - .port_fn_hw_addr_set devlink handler
+ * @port: devlink port structure
+ * @hw_addr: MAC address of the port
+ * @hw_addr_len: length of MAC address
+ * @extack: extended netdev ack structure
+ *
+ * Callback for the devlink .port_fn_hw_addr_set operation
+ * Return: zero on success or an error code on failure.
+ */
+static int ice_devlink_port_set_vf_mac_address(struct devlink_port *port,
+ const u8 *hw_addr,
+ int hw_addr_len,
+ struct netlink_ext_ack *extack)
+{
+ struct net_device *netdev = port->type_eth.netdev;
+ struct devlink_port_attrs *attrs = &port->attrs;
+ struct devlink_port_pci_vf_attrs *pci_vf;
+ u8 mac[ETH_ALEN];
+ int vf_id;
+
+ if (attrs->flavour == DEVLINK_PORT_FLAVOUR_PCI_VF) {
+ pci_vf = &attrs->pci_vf;
+ vf_id = pci_vf->vf;
+ } else {
+ NL_SET_ERR_MSG_MOD(extack, "Unable to get the vf id");
+ return -EADDRNOTAVAIL;
+ }
+
+ if (!netdev) {
+ NL_SET_ERR_MSG_MOD(extack, "Unable to get the netdev");
+ return -EADDRNOTAVAIL;
+ }
+ ether_addr_copy(mac, hw_addr);
+
+ return ice_set_vf_mac(netdev, vf_id, mac);
+}
+
+static const struct devlink_port_ops ice_devlink_vf_port_ops = {
+ .port_fn_hw_addr_get = ice_devlink_port_get_vf_mac_address,
+ .port_fn_hw_addr_set = ice_devlink_port_set_vf_mac_address,
+};
+
/**
* ice_devlink_create_vf_port - Create a devlink port for this VF
* @vf: the VF to create a port for
@@ -1611,7 +1696,8 @@ int ice_devlink_create_vf_port(struct ice_vf *vf)
devlink_port_attrs_set(devlink_port, &attrs);
devlink = priv_to_devlink(pf);

- err = devlink_port_register(devlink, devlink_port, vsi->idx);
+ err = devlink_port_register_with_ops(devlink, devlink_port,
+ vsi->idx, &ice_devlink_vf_port_ops);
if (err) {
dev_err(dev, "Failed to create devlink port for VF %d, error %d\n",
vf->vf_id, err);
--
2.39.3 (Apple Git-145)



2024-02-27 15:43:27

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH v4] ice: Add get/set hw address for VFs using devlink commands

On Sat, Feb 24, 2024 at 06:14:06PM +0530, Karthik Sundaravel wrote:
> Changing the MAC address of the VFs are not available
> via devlink. Add the function handlers to set and get
> the HW address for the VFs.
>
> Signed-off-by: Karthik Sundaravel <[email protected]>

Reviewed-by: Simon Horman <[email protected]>


2024-02-27 16:13:38

by Jiri Pirko

[permalink] [raw]
Subject: Re: [PATCH v4] ice: Add get/set hw address for VFs using devlink commands

Sat, Feb 24, 2024 at 01:44:06PM CET, [email protected] wrote:
>Changing the MAC address of the VFs are not available
>via devlink. Add the function handlers to set and get
>the HW address for the VFs.
>
>Signed-off-by: Karthik Sundaravel <[email protected]>
>---
> drivers/net/ethernet/intel/ice/ice_devlink.c | 88 +++++++++++++++++++-
> 1 file changed, 87 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
>index 80dc5445b50d..c3813edd6a76 100644
>--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
>+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
>@@ -1576,6 +1576,91 @@ void ice_devlink_destroy_pf_port(struct ice_pf *pf)
> devlink_port_unregister(&pf->devlink_port);
> }
>
>+/**
>+ * ice_devlink_port_get_vf_mac_address - .port_fn_hw_addr_get devlink handler
>+ * @port: devlink port structure
>+ * @hw_addr: MAC address of the port
>+ * @hw_addr_len: length of MAC address
>+ * @extack: extended netdev ack structure
>+ *
>+ * Callback for the devlink .port_fn_hw_addr_get operation
>+ * Return: zero on success or an error code on failure.
>+ */
>+
>+static int ice_devlink_port_get_vf_mac_address(struct devlink_port *port,
>+ u8 *hw_addr, int *hw_addr_len,
>+ struct netlink_ext_ack *extack)
>+{
>+ struct devlink_port_attrs *attrs = &port->attrs;
>+ struct devlink_port_pci_vf_attrs *pci_vf;
>+ struct devlink *devlink = port->devlink;
>+ struct ice_pf *pf;
>+ struct ice_vf *vf;
>+ int vf_id;
>+
>+ pf = devlink_priv(devlink);
>+ if (attrs->flavour == DEVLINK_PORT_FLAVOUR_PCI_VF) {
>+ pci_vf = &attrs->pci_vf;
>+ vf_id = pci_vf->vf;
>+ } else {
>+ NL_SET_ERR_MSG_MOD(extack, "Unable to get the vf id");
>+ return -EADDRNOTAVAIL;
>+ }
>+ vf = ice_get_vf_by_id(pf, vf_id);
>+ if (!vf) {
>+ NL_SET_ERR_MSG_MOD(extack, "Unable to get the vf");
>+ return -EINVAL;
>+ }
>+ ether_addr_copy(hw_addr, vf->dev_lan_addr);
>+ *hw_addr_len = ETH_ALEN;
>+
>+ ice_put_vf(vf);
>+ return 0;
>+}
>+
>+/**
>+ * ice_devlink_port_set_vf_mac_address - .port_fn_hw_addr_set devlink handler
>+ * @port: devlink port structure
>+ * @hw_addr: MAC address of the port
>+ * @hw_addr_len: length of MAC address
>+ * @extack: extended netdev ack structure
>+ *
>+ * Callback for the devlink .port_fn_hw_addr_set operation
>+ * Return: zero on success or an error code on failure.
>+ */
>+static int ice_devlink_port_set_vf_mac_address(struct devlink_port *port,

Better to have "fn" in the name of the function, so it is clear this
sets the mac of function itself.


>+ const u8 *hw_addr,
>+ int hw_addr_len,
>+ struct netlink_ext_ack *extack)
>+{
>+ struct net_device *netdev = port->type_eth.netdev;
>+ struct devlink_port_attrs *attrs = &port->attrs;
>+ struct devlink_port_pci_vf_attrs *pci_vf;
>+ u8 mac[ETH_ALEN];
>+ int vf_id;
>+
>+ if (attrs->flavour == DEVLINK_PORT_FLAVOUR_PCI_VF) {
>+ pci_vf = &attrs->pci_vf;
>+ vf_id = pci_vf->vf;
>+ } else {
>+ NL_SET_ERR_MSG_MOD(extack, "Unable to get the vf id");

How exactly this can happen? I'm pretty sure it can't.


>+ return -EADDRNOTAVAIL;
>+ }
>+
>+ if (!netdev) {

How exactly this can happen? I'm pretty sure it can't.


>+ NL_SET_ERR_MSG_MOD(extack, "Unable to get the netdev");
>+ return -EADDRNOTAVAIL;
>+ }
>+ ether_addr_copy(mac, hw_addr);
>+
>+ return ice_set_vf_mac(netdev, vf_id, mac);

It's misleading to call ice_set_vf_mac like this. It is originally used
by legacy ip vf api, where the netdev is the PF netdev. Here you pass
devlink port representor netdev. Internally ice_set_vf_mac() gets
pointer to struct ice_pf.

Could you please use:
struct ice_pf *pf = devlink_priv(devlink);
and pass it to some variant of ice_set_vf_mac() function?


pw-bot: cr


>+}
>+
>+static const struct devlink_port_ops ice_devlink_vf_port_ops = {
>+ .port_fn_hw_addr_get = ice_devlink_port_get_vf_mac_address,
>+ .port_fn_hw_addr_set = ice_devlink_port_set_vf_mac_address,
>+};
>+
> /**
> * ice_devlink_create_vf_port - Create a devlink port for this VF
> * @vf: the VF to create a port for
>@@ -1611,7 +1696,8 @@ int ice_devlink_create_vf_port(struct ice_vf *vf)
> devlink_port_attrs_set(devlink_port, &attrs);
> devlink = priv_to_devlink(pf);
>
>- err = devlink_port_register(devlink, devlink_port, vsi->idx);
>+ err = devlink_port_register_with_ops(devlink, devlink_port,
>+ vsi->idx, &ice_devlink_vf_port_ops);
> if (err) {
> dev_err(dev, "Failed to create devlink port for VF %d, error %d\n",
> vf->vf_id, err);
>--
>2.39.3 (Apple Git-145)
>