[PATCH net-next] octeon_ep: add ndo ops for VFs in PF driver

Shinas Rasheed posted 1 patch 2 weeks, 2 days ago
There is a newer version of this series
.../ethernet/marvell/octeon_ep/octep_main.c   | 98 +++++++++++++++++++
.../ethernet/marvell/octeon_ep/octep_main.h   |  1 +
.../marvell/octeon_ep/octep_pfvf_mbox.c       | 22 ++++-
.../marvell/octeon_ep/octep_pfvf_mbox.h       |  3 +
4 files changed, 122 insertions(+), 2 deletions(-)
[PATCH net-next] octeon_ep: add ndo ops for VFs in PF driver
Posted by Shinas Rasheed 2 weeks, 2 days ago
These APIs are needed to support applicaitons that use netlink to get VF
information from a PF driver.

Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
---
 .../ethernet/marvell/octeon_ep/octep_main.c   | 98 +++++++++++++++++++
 .../ethernet/marvell/octeon_ep/octep_main.h   |  1 +
 .../marvell/octeon_ep/octep_pfvf_mbox.c       | 22 ++++-
 .../marvell/octeon_ep/octep_pfvf_mbox.h       |  3 +
 4 files changed, 122 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
index 549436efc204..129c68f5a4ba 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
@@ -1137,6 +1137,95 @@ static int octep_set_features(struct net_device *dev, netdev_features_t features
 	return err;
 }
 
+static int octep_get_vf_config(struct net_device *dev, int vf, struct ifla_vf_info *ivi)
+{
+	struct octep_device *oct = netdev_priv(dev);
+
+	ivi->vf = vf;
+	ether_addr_copy(ivi->mac, oct->vf_info[vf].mac_addr);
+	ivi->vlan = 0;
+	ivi->qos = 0;
+	ivi->spoofchk = 0;
+	ivi->linkstate = IFLA_VF_LINK_STATE_ENABLE;
+	ivi->trusted = true;
+	ivi->max_tx_rate = 10000;
+	ivi->min_tx_rate = 0;
+
+	return 0;
+}
+
+static int octep_set_vf_mac(struct net_device *dev, int vf, u8 *mac)
+{
+	struct octep_device *oct = netdev_priv(dev);
+	int i, err;
+
+	if (!is_valid_ether_addr(mac)) {
+		dev_err(&oct->pdev->dev, "Invalid  MAC Address %pM\n", mac);
+		return -EADDRNOTAVAIL;
+	}
+
+	dev_dbg(&oct->pdev->dev, "set vf-%d mac to %pM\n", vf, mac);
+	for (i = 0; i < ETH_ALEN; i++)
+		oct->vf_info[vf].mac_addr[i] = mac[i];
+	oct->vf_info[vf].flags |=  OCTEON_PFVF_FLAG_MAC_SET_BY_PF;
+
+	err = octep_ctrl_net_set_mac_addr(oct, vf, mac, true);
+	if (err) {
+		dev_err(&oct->pdev->dev, "Set VF%d MAC address failed via host control Mbox\n", vf);
+		return err;
+	}
+
+	return 0;
+}
+
+static int octep_set_vf_vlan(struct net_device *dev, int vf, u16 vlan, u8 qos, __be16 vlan_proto)
+{
+	struct octep_device *oct = netdev_priv(dev);
+
+	dev_err(&oct->pdev->dev, "Setting VF VLAN not supported\n");
+	return 0;
+}
+
+static int octep_set_vf_spoofchk(struct net_device *dev, int vf, bool setting)
+{
+	struct octep_device *oct = netdev_priv(dev);
+
+	dev_err(&oct->pdev->dev, "Setting VF spoof check not supported\n");
+	return 0;
+}
+
+static int octep_set_vf_trust(struct net_device *dev, int vf, bool setting)
+{
+	struct octep_device *oct = netdev_priv(dev);
+
+	dev_err(&oct->pdev->dev, "Setting VF trust not supported\n");
+	return 0;
+}
+
+static int octep_set_vf_rate(struct net_device *dev, int vf, int min_tx_rate, int max_tx_rate)
+{
+	struct octep_device *oct = netdev_priv(dev);
+
+	dev_err(&oct->pdev->dev, "Setting VF rate not supported\n");
+	return 0;
+}
+
+static int octep_set_vf_link_state(struct net_device *dev, int vf, int link_state)
+{
+	struct octep_device *oct = netdev_priv(dev);
+
+	dev_err(&oct->pdev->dev, "Setting VF link state not supported\n");
+	return 0;
+}
+
+static int octep_get_vf_stats(struct net_device *dev, int vf, struct ifla_vf_stats *vf_stats)
+{
+	struct octep_device *oct = netdev_priv(dev);
+
+	dev_err(&oct->pdev->dev, "Getting VF stats not supported\n");
+	return 0;
+}
+
 static const struct net_device_ops octep_netdev_ops = {
 	.ndo_open                = octep_open,
 	.ndo_stop                = octep_stop,
@@ -1146,6 +1235,15 @@ static const struct net_device_ops octep_netdev_ops = {
 	.ndo_set_mac_address     = octep_set_mac,
 	.ndo_change_mtu          = octep_change_mtu,
 	.ndo_set_features        = octep_set_features,
+	/* for VFs */
+	.ndo_get_vf_config       = octep_get_vf_config,
+	.ndo_set_vf_mac          = octep_set_vf_mac,
+	.ndo_set_vf_vlan         = octep_set_vf_vlan,
+	.ndo_set_vf_spoofchk     = octep_set_vf_spoofchk,
+	.ndo_set_vf_trust        = octep_set_vf_trust,
+	.ndo_set_vf_rate         = octep_set_vf_rate,
+	.ndo_set_vf_link_state   = octep_set_vf_link_state,
+	.ndo_get_vf_stats        = octep_get_vf_stats,
 };
 
 /**
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.h b/drivers/net/ethernet/marvell/octeon_ep/octep_main.h
index fee59e0e0138..3b56916af468 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.h
@@ -220,6 +220,7 @@ struct octep_iface_link_info {
 /* The Octeon VF device specific info data structure.*/
 struct octep_pfvf_info {
 	u8 mac_addr[ETH_ALEN];
+	u32 flags;
 	u32 mbox_version;
 };
 
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.c b/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.c
index e6eb98d70f3c..be21ad5ec75e 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.c
@@ -156,12 +156,23 @@ static void octep_pfvf_set_mac_addr(struct octep_device *oct,  u32 vf_id,
 {
 	int err;
 
+	if (oct->vf_info[vf_id].flags & OCTEON_PFVF_FLAG_MAC_SET_BY_PF) {
+		dev_err(&oct->pdev->dev,
+			"VF%d attampted to override administrative set MAC address\n",
+			vf_id);
+		rsp->s_set_mac.type = OCTEP_PFVF_MBOX_TYPE_RSP_NACK;
+		return;
+	}
+
 	err = octep_ctrl_net_set_mac_addr(oct, vf_id, cmd.s_set_mac.mac_addr, true);
 	if (err) {
 		rsp->s_set_mac.type = OCTEP_PFVF_MBOX_TYPE_RSP_NACK;
-		dev_err(&oct->pdev->dev, "Set VF MAC address failed via host control Mbox\n");
+		dev_err(&oct->pdev->dev, "Set VF%d MAC address failed via host control Mbox\n",
+			vf_id);
 		return;
 	}
+
+	ether_addr_copy(oct->vf_info[vf_id].mac_addr, cmd.s_set_mac.mac_addr);
 	rsp->s_set_mac.type = OCTEP_PFVF_MBOX_TYPE_RSP_ACK;
 }
 
@@ -171,10 +182,17 @@ static void octep_pfvf_get_mac_addr(struct octep_device *oct,  u32 vf_id,
 {
 	int err;
 
+	if (oct->vf_info[vf_id].flags & OCTEON_PFVF_FLAG_MAC_SET_BY_PF) {
+		dev_dbg(&oct->pdev->dev, "VF%d MAC address set by PF\n", vf_id);
+		ether_addr_copy(rsp->s_set_mac.mac_addr, oct->vf_info[vf_id].mac_addr);
+		rsp->s_set_mac.type = OCTEP_PFVF_MBOX_TYPE_RSP_ACK;
+		return;
+	}
 	err = octep_ctrl_net_get_mac_addr(oct, vf_id, rsp->s_set_mac.mac_addr);
 	if (err) {
 		rsp->s_set_mac.type = OCTEP_PFVF_MBOX_TYPE_RSP_NACK;
-		dev_err(&oct->pdev->dev, "Get VF MAC address failed via host control Mbox\n");
+		dev_err(&oct->pdev->dev, "Get VF%d MAC address failed via host control Mbox\n",
+			vf_id);
 		return;
 	}
 	rsp->s_set_mac.type = OCTEP_PFVF_MBOX_TYPE_RSP_ACK;
diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.h b/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.h
index 0dc6eead292a..339977c7131a 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.h
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.h
@@ -23,6 +23,9 @@ enum octep_pfvf_mbox_version {
 
 #define OCTEP_PFVF_MBOX_VERSION_CURRENT	OCTEP_PFVF_MBOX_VERSION_V2
 
+/* VF flags */
+#define OCTEON_PFVF_FLAG_MAC_SET_BY_PF  BIT_ULL(0) /* PF has set VF MAC address */
+
 enum octep_pfvf_mbox_opcode {
 	OCTEP_PFVF_MBOX_CMD_VERSION,
 	OCTEP_PFVF_MBOX_CMD_SET_MTU,
-- 
2.25.1
Re: [PATCH net-next] octeon_ep: add ndo ops for VFs in PF driver
Posted by Kalesh Anakkur Purayil 1 week, 5 days ago
Hi Shinas,

On Thu, Nov 7, 2024 at 5:47 PM Shinas Rasheed <srasheed@marvell.com> wrote:
>
> These APIs are needed to support applicaitons that use netlink to get VF
[d] typo in applications
> information from a PF driver.
>
> Signed-off-by: Shinas Rasheed <srasheed@marvell.com>
> ---
>  .../ethernet/marvell/octeon_ep/octep_main.c   | 98 +++++++++++++++++++
>  .../ethernet/marvell/octeon_ep/octep_main.h   |  1 +
>  .../marvell/octeon_ep/octep_pfvf_mbox.c       | 22 ++++-
>  .../marvell/octeon_ep/octep_pfvf_mbox.h       |  3 +
>  4 files changed, 122 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> index 549436efc204..129c68f5a4ba 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.c
> @@ -1137,6 +1137,95 @@ static int octep_set_features(struct net_device *dev, netdev_features_t features
>         return err;
>  }
>
> +static int octep_get_vf_config(struct net_device *dev, int vf, struct ifla_vf_info *ivi)
> +{
> +       struct octep_device *oct = netdev_priv(dev);
> +
> +       ivi->vf = vf;
> +       ether_addr_copy(ivi->mac, oct->vf_info[vf].mac_addr);
> +       ivi->vlan = 0;
> +       ivi->qos = 0;
> +       ivi->spoofchk = 0;
> +       ivi->linkstate = IFLA_VF_LINK_STATE_ENABLE;
> +       ivi->trusted = true;
> +       ivi->max_tx_rate = 10000;
> +       ivi->min_tx_rate = 0;
> +
> +       return 0;
> +}
> +
> +static int octep_set_vf_mac(struct net_device *dev, int vf, u8 *mac)
> +{
> +       struct octep_device *oct = netdev_priv(dev);
> +       int i, err;
> +
> +       if (!is_valid_ether_addr(mac)) {
> +               dev_err(&oct->pdev->dev, "Invalid  MAC Address %pM\n", mac);
> +               return -EADDRNOTAVAIL;
> +       }
> +
> +       dev_dbg(&oct->pdev->dev, "set vf-%d mac to %pM\n", vf, mac);
> +       for (i = 0; i < ETH_ALEN; i++)
> +               oct->vf_info[vf].mac_addr[i] = mac[i];
[Kalesh] Is there any reason to no do a memcpy here or a ether_addr_copy()?
> +       oct->vf_info[vf].flags |=  OCTEON_PFVF_FLAG_MAC_SET_BY_PF;
> +
> +       err = octep_ctrl_net_set_mac_addr(oct, vf, mac, true);
> +       if (err) {
> +               dev_err(&oct->pdev->dev, "Set VF%d MAC address failed via host control Mbox\n", vf);
[d] looks like this return is unnecessary. You can "return rc" at the
end of the function.
> +               return err;
> +       }
> +
> +       return 0;
> +}
> +
> +static int octep_set_vf_vlan(struct net_device *dev, int vf, u16 vlan, u8 qos, __be16 vlan_proto)
> +{
> +       struct octep_device *oct = netdev_priv(dev);
> +
> +       dev_err(&oct->pdev->dev, "Setting VF VLAN not supported\n");
> +       return 0;
> +}
> +
> +static int octep_set_vf_spoofchk(struct net_device *dev, int vf, bool setting)
> +{
> +       struct octep_device *oct = netdev_priv(dev);
> +
> +       dev_err(&oct->pdev->dev, "Setting VF spoof check not supported\n");
> +       return 0;
> +}
> +
> +static int octep_set_vf_trust(struct net_device *dev, int vf, bool setting)
> +{
> +       struct octep_device *oct = netdev_priv(dev);
> +
> +       dev_err(&oct->pdev->dev, "Setting VF trust not supported\n");
> +       return 0;
> +}
> +
> +static int octep_set_vf_rate(struct net_device *dev, int vf, int min_tx_rate, int max_tx_rate)
> +{
> +       struct octep_device *oct = netdev_priv(dev);
> +
> +       dev_err(&oct->pdev->dev, "Setting VF rate not supported\n");
> +       return 0;
> +}
> +
> +static int octep_set_vf_link_state(struct net_device *dev, int vf, int link_state)
> +{
> +       struct octep_device *oct = netdev_priv(dev);
> +
> +       dev_err(&oct->pdev->dev, "Setting VF link state not supported\n");
> +       return 0;
> +}
> +
> +static int octep_get_vf_stats(struct net_device *dev, int vf, struct ifla_vf_stats *vf_stats)
> +{
> +       struct octep_device *oct = netdev_priv(dev);
> +
> +       dev_err(&oct->pdev->dev, "Getting VF stats not supported\n");
> +       return 0;
> +}
[Kalesh] Do not expose the support for these unsupported hooks in
struct net_device_ops. Stack has a check for the support before
invoking the callback.
> +
>  static const struct net_device_ops octep_netdev_ops = {
>         .ndo_open                = octep_open,
>         .ndo_stop                = octep_stop,
> @@ -1146,6 +1235,15 @@ static const struct net_device_ops octep_netdev_ops = {
>         .ndo_set_mac_address     = octep_set_mac,
>         .ndo_change_mtu          = octep_change_mtu,
>         .ndo_set_features        = octep_set_features,
> +       /* for VFs */
> +       .ndo_get_vf_config       = octep_get_vf_config,
> +       .ndo_set_vf_mac          = octep_set_vf_mac,
> +       .ndo_set_vf_vlan         = octep_set_vf_vlan,
> +       .ndo_set_vf_spoofchk     = octep_set_vf_spoofchk,
> +       .ndo_set_vf_trust        = octep_set_vf_trust,
> +       .ndo_set_vf_rate         = octep_set_vf_rate,
> +       .ndo_set_vf_link_state   = octep_set_vf_link_state,
> +       .ndo_get_vf_stats        = octep_get_vf_stats,
>  };
>
>  /**
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_main.h b/drivers/net/ethernet/marvell/octeon_ep/octep_main.h
> index fee59e0e0138..3b56916af468 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_main.h
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_main.h
> @@ -220,6 +220,7 @@ struct octep_iface_link_info {
>  /* The Octeon VF device specific info data structure.*/
>  struct octep_pfvf_info {
>         u8 mac_addr[ETH_ALEN];
> +       u32 flags;
>         u32 mbox_version;
>  };
>
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.c b/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.c
> index e6eb98d70f3c..be21ad5ec75e 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.c
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.c
> @@ -156,12 +156,23 @@ static void octep_pfvf_set_mac_addr(struct octep_device *oct,  u32 vf_id,
>  {
>         int err;
>
> +       if (oct->vf_info[vf_id].flags & OCTEON_PFVF_FLAG_MAC_SET_BY_PF) {
> +               dev_err(&oct->pdev->dev,
> +                       "VF%d attampted to override administrative set MAC address\n",
[d] typo in "attempted"
> +                       vf_id);
> +               rsp->s_set_mac.type = OCTEP_PFVF_MBOX_TYPE_RSP_NACK;
> +               return;
> +       }
> +
>         err = octep_ctrl_net_set_mac_addr(oct, vf_id, cmd.s_set_mac.mac_addr, true);
>         if (err) {
>                 rsp->s_set_mac.type = OCTEP_PFVF_MBOX_TYPE_RSP_NACK;
> -               dev_err(&oct->pdev->dev, "Set VF MAC address failed via host control Mbox\n");
> +               dev_err(&oct->pdev->dev, "Set VF%d MAC address failed via host control Mbox\n",
> +                       vf_id);
>                 return;
>         }
> +
> +       ether_addr_copy(oct->vf_info[vf_id].mac_addr, cmd.s_set_mac.mac_addr);
>         rsp->s_set_mac.type = OCTEP_PFVF_MBOX_TYPE_RSP_ACK;
>  }
>
> @@ -171,10 +182,17 @@ static void octep_pfvf_get_mac_addr(struct octep_device *oct,  u32 vf_id,
>  {
>         int err;
>
> +       if (oct->vf_info[vf_id].flags & OCTEON_PFVF_FLAG_MAC_SET_BY_PF) {
> +               dev_dbg(&oct->pdev->dev, "VF%d MAC address set by PF\n", vf_id);
> +               ether_addr_copy(rsp->s_set_mac.mac_addr, oct->vf_info[vf_id].mac_addr);
> +               rsp->s_set_mac.type = OCTEP_PFVF_MBOX_TYPE_RSP_ACK;
> +               return;
> +       }
>         err = octep_ctrl_net_get_mac_addr(oct, vf_id, rsp->s_set_mac.mac_addr);
>         if (err) {
>                 rsp->s_set_mac.type = OCTEP_PFVF_MBOX_TYPE_RSP_NACK;
> -               dev_err(&oct->pdev->dev, "Get VF MAC address failed via host control Mbox\n");
> +               dev_err(&oct->pdev->dev, "Get VF%d MAC address failed via host control Mbox\n",
> +                       vf_id);
>                 return;
>         }
>         rsp->s_set_mac.type = OCTEP_PFVF_MBOX_TYPE_RSP_ACK;
> diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.h b/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.h
> index 0dc6eead292a..339977c7131a 100644
> --- a/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.h
> +++ b/drivers/net/ethernet/marvell/octeon_ep/octep_pfvf_mbox.h
> @@ -23,6 +23,9 @@ enum octep_pfvf_mbox_version {
>
>  #define OCTEP_PFVF_MBOX_VERSION_CURRENT        OCTEP_PFVF_MBOX_VERSION_V2
>
> +/* VF flags */
> +#define OCTEON_PFVF_FLAG_MAC_SET_BY_PF  BIT_ULL(0) /* PF has set VF MAC address */
> +
>  enum octep_pfvf_mbox_opcode {
>         OCTEP_PFVF_MBOX_CMD_VERSION,
>         OCTEP_PFVF_MBOX_CMD_SET_MTU,
> --
> 2.25.1
>
>


-- 
Regards,
Kalesh A P
Re: [PATCH net-next] octeon_ep: add ndo ops for VFs in PF driver
Posted by Simon Horman 1 week, 6 days ago
On Thu, Nov 07, 2024 at 04:16:37AM -0800, Shinas Rasheed wrote:
> These APIs are needed to support applicaitons that use netlink to get VF
> information from a PF driver.
> 
> Signed-off-by: Shinas Rasheed <srasheed@marvell.com>

...

> +static int octep_set_vf_vlan(struct net_device *dev, int vf, u16 vlan, u8 qos, __be16 vlan_proto)
> +{
> +	struct octep_device *oct = netdev_priv(dev);
> +
> +	dev_err(&oct->pdev->dev, "Setting VF VLAN not supported\n");
> +	return 0;
> +}

Hi Shinas,

Given that the operation is not supported I think it would
be more appropriate to return -EOPNOTSUPP. And moreover, given
that this is a noop, I think it would be yet more appropriate
not to implement it at all and let the core treat it as not supported.

Likewise for other NDOs implemented as noops in this patch.

...