[PATCH net-next 0/2] ionic: add sriov support

[PATCH net-next 0/2] ionic: add sriov support

[Shannon Nelson]

Set up the basic support for enabling SR-IOV devices in the
ionic driver.  Since most of the management work happens in
the NIC firmware, the driver becomes mostly a pass-through
for the network stack commands that want to control and
configure the VFs.

Shannon Nelson (2):
  ionic: ionic_if bits for sr-iov support
  ionic: support sr-iov operations

 drivers/net/ethernet/pensando/ionic/ionic.h   |  16 +-
 .../ethernet/pensando/ionic/ionic_bus_pci.c   |  75 +++++++
 .../net/ethernet/pensando/ionic/ionic_dev.c   |  61 ++++++
 .../net/ethernet/pensando/ionic/ionic_dev.h   |   7 +
 .../net/ethernet/pensando/ionic/ionic_if.h    |  97 +++++++++
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 188 +++++++++++++++++-
 .../net/ethernet/pensando/ionic/ionic_lif.h   |   6 +
 .../net/ethernet/pensando/ionic/ionic_main.c  |   4 +
 8 files changed, 446 insertions(+), 8 deletions(-)

-- 
2.17.1

[PATCH net-next 1/2] ionic: ionic_if bits for sr-iov support

[Shannon Nelson]

Adds new AdminQ calls and their related structs for
supporting PF controls on VFs:
    CMD_OPCODE_VF_GETATTR
    CMD_OPCODE_VF_SETATTR

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 .../net/ethernet/pensando/ionic/ionic_if.h    | 97 +++++++++++++++++++
 1 file changed, 97 insertions(+)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_if.h b/drivers/net/ethernet/pensando/ionic/ionic_if.h
index dbdb7c5ae8f1..ea5c379824c1 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_if.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_if.h
@@ -51,6 +51,10 @@ enum ionic_cmd_opcode {
 	IONIC_CMD_RDMA_CREATE_CQ		= 52,
 	IONIC_CMD_RDMA_CREATE_ADMINQ		= 53,
 
+	/* SR/IOV commands */
+	IONIC_CMD_VF_GETATTR			= 60,
+	IONIC_CMD_VF_SETATTR			= 61,
+
 	/* QoS commands */
 	IONIC_CMD_QOS_CLASS_IDENTIFY		= 240,
 	IONIC_CMD_QOS_CLASS_INIT		= 241,
@@ -1639,6 +1643,93 @@ enum ionic_qos_sched_type {
 	IONIC_QOS_SCHED_TYPE_DWRR	= 1,	/* Deficit weighted round-robin */
 };
 
+enum ionic_vf_attr {
+	IONIC_VF_ATTR_SPOOFCHK	= 1,
+	IONIC_VF_ATTR_TRUST	= 2,
+	IONIC_VF_ATTR_MAC	= 3,
+	IONIC_VF_ATTR_LINKSTATE	= 4,
+	IONIC_VF_ATTR_VLAN	= 5,
+	IONIC_VF_ATTR_RATE	= 6,
+	IONIC_VF_ATTR_STATSADDR	= 7,
+};
+
+/**
+ * VF link status
+ */
+enum ionic_vf_link_status {
+	IONIC_VF_LINK_STATUS_AUTO = 0,	/* link state of the uplink */
+	IONIC_VF_LINK_STATUS_UP   = 1,	/* link is always up */
+	IONIC_VF_LINK_STATUS_DOWN = 2,	/* link is always down */
+};
+
+/**
+ * struct ionic_vf_setattr_cmd - Set VF attributes on the NIC
+ * @opcode:     Opcode
+ * @index:      VF index
+ * @attr:       Attribute type (enum ionic_vf_attr)
+ *	macaddr		mac address
+ *	vlanid		vlan ID
+ *	maxrate		max Tx rate in Mbps
+ *	spoofchk	enable address spoof checking
+ *	trust		enable VF trust
+ *	linkstate	set link up or down
+ *	stats_pa	set DMA address for VF stats
+ */
+struct ionic_vf_setattr_cmd {
+	u8     opcode;
+	u8     attr;
+	__le16 vf_index;
+	union {
+		u8     macaddr[6];
+		__le16 vlanid;
+		__le32 maxrate;
+		u8     spoofchk;
+		u8     trust;
+		u8     linkstate;
+		__le64 stats_pa;
+		u8     pad[60];
+	};
+};
+
+struct ionic_vf_setattr_comp {
+	u8     status;
+	u8     attr;
+	__le16 vf_index;
+	__le16 comp_index;
+	u8     rsvd[9];
+	u8     color;
+};
+
+/**
+ * struct ionic_vf_getattr_cmd - Get VF attributes from the NIC
+ * @opcode:     Opcode
+ * @index:      VF index
+ * @attr:       Attribute type (enum ionic_vf_attr)
+ */
+struct ionic_vf_getattr_cmd {
+	u8     opcode;
+	u8     attr;
+	__le16 vf_index;
+	u8     rsvd[60];
+};
+
+struct ionic_vf_getattr_comp {
+	u8     status;
+	u8     attr;
+	__le16 vf_index;
+	union {
+		u8     macaddr[6];
+		__le16 vlanid;
+		__le32 maxrate;
+		u8     spoofchk;
+		u8     trust;
+		u8     linkstate;
+		__le64 stats_pa;
+		u8     pad[11];
+	};
+	u8     color;
+};
+
 /**
  * union ionic_qos_config - Qos configuration structure
  * @flags:		Configuration flags
@@ -2289,6 +2380,9 @@ union ionic_dev_cmd {
 	struct ionic_port_getattr_cmd port_getattr;
 	struct ionic_port_setattr_cmd port_setattr;
 
+	struct ionic_vf_setattr_cmd vf_setattr;
+	struct ionic_vf_getattr_cmd vf_getattr;
+
 	struct ionic_lif_identify_cmd lif_identify;
 	struct ionic_lif_init_cmd lif_init;
 	struct ionic_lif_reset_cmd lif_reset;
@@ -2318,6 +2412,9 @@ union ionic_dev_cmd_comp {
 	struct ionic_port_getattr_comp port_getattr;
 	struct ionic_port_setattr_comp port_setattr;
 
+	struct ionic_vf_setattr_comp vf_setattr;
+	struct ionic_vf_getattr_comp vf_getattr;
+
 	struct ionic_lif_identify_comp lif_identify;
 	struct ionic_lif_init_comp lif_init;
 	ionic_lif_reset_comp lif_reset;
-- 
2.17.1

[PATCH net-next 2/2] ionic: support sr-iov operations

[Shannon Nelson]

Add the netdev ops for managing VFs.  Since most of the
management work happens in the NIC firmware, the driver becomes
mostly a pass-through for the network stack commands that want
to control and configure the VFs.

We also tweak ionic_station_set() a little to allow for
the VFs that start off with a zero'd mac address.

Signed-off-by: Shannon Nelson <snelson@pensando.io>
---
 drivers/net/ethernet/pensando/ionic/ionic.h   |  16 +-
 .../ethernet/pensando/ionic/ionic_bus_pci.c   |  75 +++++++
 .../net/ethernet/pensando/ionic/ionic_dev.c   |  61 ++++++
 .../net/ethernet/pensando/ionic/ionic_dev.h   |   7 +
 .../net/ethernet/pensando/ionic/ionic_lif.c   | 188 +++++++++++++++++-
 .../net/ethernet/pensando/ionic/ionic_lif.h   |   6 +
 .../net/ethernet/pensando/ionic/ionic_main.c  |   4 +
 7 files changed, 349 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
index 98e102af7756..d4cf58da8d13 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic.h
@@ -12,7 +12,7 @@ struct ionic_lif;
 
 #define IONIC_DRV_NAME		"ionic"
 #define IONIC_DRV_DESCRIPTION	"Pensando Ethernet NIC Driver"
-#define IONIC_DRV_VERSION	"0.18.0-k"
+#define IONIC_DRV_VERSION	"0.20.0-k"
 
 #define PCI_VENDOR_ID_PENSANDO			0x1dd8
 
@@ -25,6 +25,18 @@ struct ionic_lif;
 
 #define DEVCMD_TIMEOUT  10
 
+struct ionic_vf {
+	u16	 index;
+	u8	 macaddr[6];
+	__le32	 maxrate;
+	__le16	 vlanid;
+	u8	 spoofchk;
+	u8	 trusted;
+	u8	 linkstate;
+	dma_addr_t       stats_pa;
+	struct ionic_lif_stats stats;
+};
+
 struct ionic {
 	struct pci_dev *pdev;
 	struct device *dev;
@@ -46,6 +58,8 @@ struct ionic {
 	DECLARE_BITMAP(intrs, IONIC_INTR_CTRL_REGS_MAX);
 	struct work_struct nb_work;
 	struct notifier_block nb;
+	int num_vfs;
+	struct ionic_vf **vf;
 	struct timer_list watchdog_timer;
 	int watchdog_period;
 };
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
index 9a9ab8cb2cb3..fe4efe12b50b 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
@@ -274,11 +274,86 @@ static void ionic_remove(struct pci_dev *pdev)
 	ionic_devlink_free(ionic);
 }
 
+static int ionic_sriov_configure(struct pci_dev *pdev, int num_vfs)
+{
+	struct ionic *ionic = pci_get_drvdata(pdev);
+	struct device *dev = ionic->dev;
+	unsigned int size;
+	int i, err = 0;
+
+	if (!ionic_is_pf(ionic))
+		return -ENODEV;
+
+	if (num_vfs > 0) {
+		err = pci_enable_sriov(pdev, num_vfs);
+		if (err) {
+			dev_err(&pdev->dev, "Cannot enable SRIOV: %d\n", err);
+			return err;
+		}
+
+		size = sizeof(struct ionic_vf *) * num_vfs;
+		ionic->vf = kzalloc(size, GFP_KERNEL);
+		if (!ionic->vf) {
+			pci_disable_sriov(pdev);
+			return -ENOMEM;
+		}
+
+		for (i = 0; i < num_vfs; i++) {
+			struct ionic_vf *v;
+
+			v = kzalloc(sizeof(*v), GFP_KERNEL);
+			if (!v) {
+				err = -ENOMEM;
+				num_vfs = 0;
+				goto remove_vfs;
+			}
+
+			v->stats_pa = dma_map_single(dev, &v->stats,
+						     sizeof(v->stats),
+						     DMA_FROM_DEVICE);
+			if (dma_mapping_error(dev, v->stats_pa)) {
+				err = -ENODEV;
+				kfree(v);
+				ionic->vf[i] = NULL;
+				num_vfs = 0;
+				goto remove_vfs;
+			}
+
+			ionic->vf[i] = v;
+			ionic->num_vfs++;
+		}
+
+		return num_vfs;
+	}
+
+remove_vfs:
+	if (num_vfs == 0) {
+		if (err)
+			dev_err(&pdev->dev, "SRIOV setup failed: %d\n", err);
+
+		pci_disable_sriov(pdev);
+
+		for (i = 0; i < ionic->num_vfs; i++) {
+			dma_unmap_single(dev, ionic->vf[i]->stats_pa,
+					 sizeof(struct ionic_lif_stats),
+					 DMA_FROM_DEVICE);
+			kfree(ionic->vf[i]);
+		}
+
+		kfree(ionic->vf);
+		ionic->vf = NULL;
+		ionic->num_vfs = 0;
+	}
+
+	return err;
+}
+
 static struct pci_driver ionic_driver = {
 	.name = IONIC_DRV_NAME,
 	.id_table = ionic_id_table,
 	.probe = ionic_probe,
 	.remove = ionic_remove,
+	.sriov_configure = ionic_sriov_configure,
 };
 
 int ionic_bus_register_driver(void)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
index 5f9d2ec70446..8f0ab5838667 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
@@ -286,6 +286,67 @@ void ionic_dev_cmd_port_pause(struct ionic_dev *idev, u8 pause_type)
 	ionic_dev_cmd_go(idev, &cmd);
 }
 
+/* VF commands */
+int ionic_set_vf_config(struct ionic *ionic, int vf, u8 attr, u8 *data)
+{
+	union ionic_dev_cmd cmd = {
+		.vf_setattr.opcode = IONIC_CMD_VF_SETATTR,
+		.vf_setattr.attr = attr,
+		.vf_setattr.vf_index = vf,
+	};
+	int err;
+
+	if (vf >= ionic->num_vfs)
+		return -EINVAL;
+
+	switch (attr) {
+	case IONIC_VF_ATTR_SPOOFCHK:
+		cmd.vf_setattr.spoofchk = *data;
+		dev_dbg(ionic->dev, "%s: vf %d spoof %d\n",
+			__func__, vf, *data);
+		break;
+	case IONIC_VF_ATTR_TRUST:
+		cmd.vf_setattr.trust = *data;
+		dev_dbg(ionic->dev, "%s: vf %d trust %d\n",
+			__func__, vf, *data);
+		break;
+	case IONIC_VF_ATTR_LINKSTATE:
+		cmd.vf_setattr.linkstate = *data;
+		dev_dbg(ionic->dev, "%s: vf %d linkstate %d\n",
+			__func__, vf, *data);
+		break;
+	case IONIC_VF_ATTR_MAC:
+		ether_addr_copy(cmd.vf_setattr.macaddr, data);
+		dev_dbg(ionic->dev, "%s: vf %d macaddr %pM\n",
+			__func__, vf, data);
+		break;
+	case IONIC_VF_ATTR_VLAN:
+		cmd.vf_setattr.vlanid = cpu_to_le16(*(u16 *)data);
+		dev_dbg(ionic->dev, "%s: vf %d vlan %d\n",
+			__func__, vf, *(u16 *)data);
+		break;
+	case IONIC_VF_ATTR_RATE:
+		cmd.vf_setattr.maxrate = cpu_to_le32(*(u32 *)data);
+		dev_dbg(ionic->dev, "%s: vf %d maxrate %d\n",
+			__func__, vf, *(u32 *)data);
+		break;
+	case IONIC_VF_ATTR_STATSADDR:
+		cmd.vf_setattr.stats_pa = cpu_to_le64(*(u64 *)data);
+		dev_dbg(ionic->dev, "%s: vf %d stats_pa 0x%08llx\n",
+			__func__, vf, *(u64 *)data);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	mutex_lock(&ionic->dev_cmd_lock);
+	ionic_dev_cmd_go(&ionic->idev, &cmd);
+	err = ionic_dev_cmd_wait(ionic, DEVCMD_TIMEOUT);
+	mutex_unlock(&ionic->dev_cmd_lock);
+
+	return err;
+}
+
 /* LIF commands */
 void ionic_dev_cmd_lif_identify(struct ionic_dev *idev, u8 type, u8 ver)
 {
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index 4665c5dc5324..7838e342c4fd 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -113,6 +113,12 @@ static_assert(sizeof(struct ionic_rxq_desc) == 16);
 static_assert(sizeof(struct ionic_rxq_sg_desc) == 128);
 static_assert(sizeof(struct ionic_rxq_comp) == 16);
 
+/* SR/IOV */
+static_assert(sizeof(struct ionic_vf_setattr_cmd) == 64);
+static_assert(sizeof(struct ionic_vf_setattr_comp) == 16);
+static_assert(sizeof(struct ionic_vf_getattr_cmd) == 64);
+static_assert(sizeof(struct ionic_vf_getattr_comp) == 16);
+
 struct ionic_devinfo {
 	u8 asic_type;
 	u8 asic_rev;
@@ -275,6 +281,7 @@ void ionic_dev_cmd_port_autoneg(struct ionic_dev *idev, u8 an_enable);
 void ionic_dev_cmd_port_fec(struct ionic_dev *idev, u8 fec_type);
 void ionic_dev_cmd_port_pause(struct ionic_dev *idev, u8 pause_type);
 
+int ionic_set_vf_config(struct ionic *ionic, int vf, u8 attr, u8 *data);
 void ionic_dev_cmd_lif_identify(struct ionic_dev *idev, u8 type, u8 ver);
 void ionic_dev_cmd_lif_init(struct ionic_dev *idev, u16 lif_index,
 			    dma_addr_t addr);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 60fd14df49d7..9eb3ce057aa8 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -1616,6 +1616,168 @@ int ionic_stop(struct net_device *netdev)
 	return err;
 }
 
+static int ionic_get_vf_config(struct net_device *netdev,
+			       int vf, struct ifla_vf_info *ivf)
+{
+	struct ionic_lif *lif = netdev_priv(netdev);
+	struct ionic *ionic = lif->ionic;
+
+	if (vf >= ionic->num_vfs)
+		return -EINVAL;
+
+	ivf->vf           = vf;
+	ivf->vlan         = ionic->vf[vf]->vlanid;
+	ivf->qos	  = 0;
+	ivf->spoofchk     = ionic->vf[vf]->spoofchk;
+	ivf->linkstate    = ionic->vf[vf]->linkstate;
+	ivf->max_tx_rate  = ionic->vf[vf]->maxrate;
+	ivf->trusted      = ionic->vf[vf]->trusted;
+	ether_addr_copy(ivf->mac, ionic->vf[vf]->macaddr);
+
+	return 0;
+}
+
+static int ionic_get_vf_stats(struct net_device *netdev, int vf,
+			      struct ifla_vf_stats *vf_stats)
+{
+	struct ionic_lif *lif = netdev_priv(netdev);
+	struct ionic *ionic = lif->ionic;
+	struct ionic_lif_stats *vs;
+
+	if (vf >= ionic->num_vfs)
+		return -EINVAL;
+
+	memset(vf_stats, 0, sizeof(*vf_stats));
+	vs = &ionic->vf[vf]->stats;
+
+	vf_stats->rx_packets = le64_to_cpu(vs->rx_ucast_packets);
+	vf_stats->tx_packets = le64_to_cpu(vs->tx_ucast_packets);
+	vf_stats->rx_bytes   = le64_to_cpu(vs->rx_ucast_bytes);
+	vf_stats->tx_bytes   = le64_to_cpu(vs->tx_ucast_bytes);
+	vf_stats->broadcast  = le64_to_cpu(vs->rx_bcast_packets);
+	vf_stats->multicast  = le64_to_cpu(vs->rx_mcast_packets);
+	vf_stats->rx_dropped = le64_to_cpu(vs->rx_ucast_drop_packets) +
+			       le64_to_cpu(vs->rx_mcast_drop_packets) +
+			       le64_to_cpu(vs->rx_bcast_drop_packets);
+	vf_stats->tx_dropped = le64_to_cpu(vs->tx_ucast_drop_packets) +
+			       le64_to_cpu(vs->tx_mcast_drop_packets) +
+			       le64_to_cpu(vs->tx_bcast_drop_packets);
+
+	return 0;
+}
+
+static int ionic_set_vf_mac(struct net_device *netdev, int vf, u8 *mac)
+{
+	struct ionic_lif *lif = netdev_priv(netdev);
+	int ret;
+
+	if (!(is_zero_ether_addr(mac) || is_valid_ether_addr(mac)))
+		return -EINVAL;
+
+	ret = ionic_set_vf_config(lif->ionic, vf,
+				  IONIC_VF_ATTR_MAC, mac);
+	if (!ret)
+		ether_addr_copy(lif->ionic->vf[vf]->macaddr, mac);
+
+	return ret;
+}
+
+static int ionic_set_vf_vlan(struct net_device *netdev, int vf, u16 vlan,
+			     u8 qos, __be16 proto)
+{
+	struct ionic_lif *lif = netdev_priv(netdev);
+	int ret;
+
+	/* until someday when we support qos */
+	if (qos)
+		return -EINVAL;
+
+	if (vlan > 4095)
+		return -EINVAL;
+
+	if (proto != htons(ETH_P_8021Q))
+		return -EPROTONOSUPPORT;
+
+	ret = ionic_set_vf_config(lif->ionic, vf,
+				  IONIC_VF_ATTR_VLAN, (u8 *)&vlan);
+	if (!ret)
+		lif->ionic->vf[vf]->vlanid = vlan;
+
+	return ret;
+}
+
+static int ionic_set_vf_rate(struct net_device *netdev, int vf,
+			     int tx_min, int tx_max)
+{
+	struct ionic_lif *lif = netdev_priv(netdev);
+	int ret;
+
+	/* setting the min just seems silly */
+	if (tx_min)
+		return -EINVAL;
+
+	ret = ionic_set_vf_config(lif->ionic, vf,
+				  IONIC_VF_ATTR_RATE, (u8 *)&tx_max);
+	if (!ret)
+		lif->ionic->vf[vf]->maxrate = tx_max;
+
+	return ret;
+}
+
+static int ionic_set_vf_spoofchk(struct net_device *netdev, int vf, bool set)
+{
+	struct ionic_lif *lif = netdev_priv(netdev);
+	u8 data = set;  /* convert to u8 for config */
+	int ret;
+
+	ret = ionic_set_vf_config(lif->ionic, vf,
+				  IONIC_VF_ATTR_SPOOFCHK, &data);
+	if (!ret)
+		lif->ionic->vf[vf]->spoofchk = data;
+
+	return ret;
+}
+
+static int ionic_set_vf_trust(struct net_device *netdev, int vf, bool set)
+{
+	struct ionic_lif *lif = netdev_priv(netdev);
+	u8 data = set;  /* convert to u8 for config */
+	int ret;
+
+	ret = ionic_set_vf_config(lif->ionic, vf,
+				  IONIC_VF_ATTR_TRUST, &data);
+	if (!ret)
+		lif->ionic->vf[vf]->trusted = data;
+
+	return ret;
+}
+
+static int ionic_set_vf_link_state(struct net_device *netdev, int vf, int set)
+{
+	struct ionic_lif *lif = netdev_priv(netdev);
+	u8 data;
+	int ret;
+
+	switch (set) {
+	case IFLA_VF_LINK_STATE_AUTO:
+		data = IONIC_VF_LINK_STATUS_AUTO;
+		break;
+	case IFLA_VF_LINK_STATE_ENABLE:
+		data = IONIC_VF_LINK_STATUS_UP;
+		break;
+	case IFLA_VF_LINK_STATE_DISABLE:
+		data = IONIC_VF_LINK_STATUS_DOWN;
+		break;
+	}
+
+	ret = ionic_set_vf_config(lif->ionic, vf,
+				  IONIC_VF_ATTR_LINKSTATE, &data);
+	if (!ret)
+		lif->ionic->vf[vf]->linkstate = set;
+
+	return ret;
+}
+
 static const struct net_device_ops ionic_netdev_ops = {
 	.ndo_open               = ionic_open,
 	.ndo_stop               = ionic_stop,
@@ -1629,6 +1791,14 @@ static const struct net_device_ops ionic_netdev_ops = {
 	.ndo_change_mtu         = ionic_change_mtu,
 	.ndo_vlan_rx_add_vid    = ionic_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid   = ionic_vlan_rx_kill_vid,
+	.ndo_set_vf_vlan	= ionic_set_vf_vlan,
+	.ndo_set_vf_trust	= ionic_set_vf_trust,
+	.ndo_set_vf_mac		= ionic_set_vf_mac,
+	.ndo_set_vf_rate	= ionic_set_vf_rate,
+	.ndo_set_vf_spoofchk	= ionic_set_vf_spoofchk,
+	.ndo_get_vf_config	= ionic_get_vf_config,
+	.ndo_set_vf_link_state	= ionic_set_vf_link_state,
+	.ndo_get_vf_stats       = ionic_get_vf_stats,
 };
 
 int ionic_reset_queues(struct ionic_lif *lif)
@@ -1961,18 +2131,22 @@ static int ionic_station_set(struct ionic_lif *lif)
 	if (err)
 		return err;
 
+	if (is_zero_ether_addr(ctx.comp.lif_getattr.mac))
+		return 0;
+
 	memcpy(addr.sa_data, ctx.comp.lif_getattr.mac, netdev->addr_len);
 	addr.sa_family = AF_INET;
 	err = eth_prepare_mac_addr_change(netdev, &addr);
-	if (err)
-		return err;
-
-	if (!is_zero_ether_addr(netdev->dev_addr)) {
-		netdev_dbg(lif->netdev, "deleting station MAC addr %pM\n",
-			   netdev->dev_addr);
-		ionic_lif_addr(lif, netdev->dev_addr, false);
+	if (err) {
+		netdev_warn(lif->netdev, "ignoring bad MAC addr from NIC %pM\n",
+			    addr.sa_data);
+		return 0;
 	}
 
+	netdev_dbg(lif->netdev, "deleting station MAC addr %pM\n",
+		   netdev->dev_addr);
+	ionic_lif_addr(lif, netdev->dev_addr, false);
+
 	eth_commit_mac_addr_change(netdev, &addr);
 	netdev_dbg(lif->netdev, "adding station MAC addr %pM\n",
 		   netdev->dev_addr);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
index a55fd1f8c31b..03b8bd7e3730 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
@@ -195,6 +195,12 @@ static inline int ionic_wait_for_bit(struct ionic_lif *lif, int bitname)
 	return wait_on_bit_lock(lif->state, bitname, TASK_INTERRUPTIBLE);
 }
 
+static inline bool ionic_is_pf(struct ionic *ionic)
+{
+	return ionic->pdev &&
+	       ionic->pdev->device == PCI_DEVICE_ID_PENSANDO_IONIC_ETH_PF;
+}
+
 static inline u32 ionic_coal_usec_to_hw(struct ionic *ionic, u32 usecs)
 {
 	u32 mult = le32_to_cpu(ionic->ident.dev.intr_coal_mult);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 3590ea7fd88a..837b85f2fed9 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -165,6 +165,10 @@ static const char *ionic_opcode_to_str(enum ionic_cmd_opcode opcode)
 		return "IONIC_CMD_FW_DOWNLOAD";
 	case IONIC_CMD_FW_CONTROL:
 		return "IONIC_CMD_FW_CONTROL";
+	case IONIC_CMD_VF_GETATTR:
+		return "IONIC_CMD_VF_GETATTR";
+	case IONIC_CMD_VF_SETATTR:
+		return "IONIC_CMD_VF_SETATTR";
 	default:
 		return "DEVCMD_UNKNOWN";
 	}
-- 
2.17.1

Re: [PATCH net-next 2/2] ionic: support sr-iov operations

[Parav Pandit]

On 12/10/2019 4:54 PM, Shannon Nelson wrote:
> Add the netdev ops for managing VFs.  Since most of the
> management work happens in the NIC firmware, the driver becomes
> mostly a pass-through for the network stack commands that want
> to control and configure the VFs.
> 
> We also tweak ionic_station_set() a little to allow for
> the VFs that start off with a zero'd mac address.
> 
> Signed-off-by: Shannon Nelson <snelson@pensando.io>
> ---
>  drivers/net/ethernet/pensando/ionic/ionic.h   |  16 +-
>  .../ethernet/pensando/ionic/ionic_bus_pci.c   |  75 +++++++
>  .../net/ethernet/pensando/ionic/ionic_dev.c   |  61 ++++++
>  .../net/ethernet/pensando/ionic/ionic_dev.h   |   7 +
>  .../net/ethernet/pensando/ionic/ionic_lif.c   | 188 +++++++++++++++++-
>  .../net/ethernet/pensando/ionic/ionic_lif.h   |   6 +
>  .../net/ethernet/pensando/ionic/ionic_main.c  |   4 +
>  7 files changed, 349 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
> index 98e102af7756..d4cf58da8d13 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic.h
> @@ -12,7 +12,7 @@ struct ionic_lif;
>  
>  #define IONIC_DRV_NAME		"ionic"
>  #define IONIC_DRV_DESCRIPTION	"Pensando Ethernet NIC Driver"
> -#define IONIC_DRV_VERSION	"0.18.0-k"
> +#define IONIC_DRV_VERSION	"0.20.0-k"
>  
>  #define PCI_VENDOR_ID_PENSANDO			0x1dd8
>  
> @@ -25,6 +25,18 @@ struct ionic_lif;
>  
>  #define DEVCMD_TIMEOUT  10
>  
> +struct ionic_vf {
> +	u16	 index;
> +	u8	 macaddr[6];
> +	__le32	 maxrate;
> +	__le16	 vlanid;
> +	u8	 spoofchk;
> +	u8	 trusted;
> +	u8	 linkstate;
> +	dma_addr_t       stats_pa;
> +	struct ionic_lif_stats stats;
> +};
> +
>  struct ionic {
>  	struct pci_dev *pdev;
>  	struct device *dev;
> @@ -46,6 +58,8 @@ struct ionic {
>  	DECLARE_BITMAP(intrs, IONIC_INTR_CTRL_REGS_MAX);
>  	struct work_struct nb_work;
>  	struct notifier_block nb;
> +	int num_vfs;
Please drop num_vfs and use pci_num_vf() in ionic_get_vf_config() and
other friend functions.

> +	struct ionic_vf **vf;
>  	struct timer_list watchdog_timer;
>  	int watchdog_period;
>  };
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> index 9a9ab8cb2cb3..fe4efe12b50b 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> @@ -274,11 +274,86 @@ static void ionic_remove(struct pci_dev *pdev)
>  	ionic_devlink_free(ionic);
>  }
>  
> +static int ionic_sriov_configure(struct pci_dev *pdev, int num_vfs)
> +{
> +	struct ionic *ionic = pci_get_drvdata(pdev);
> +	struct device *dev = ionic->dev;
> +	unsigned int size;
> +	int i, err = 0;
> +
> +	if (!ionic_is_pf(ionic))
> +		return -ENODEV;
> +
This check is already done by pci core kernel. No need for ionic driver
to do this. If SR-IOV capability is not enabled, it won't reach here.

> +	if (num_vfs > 0) {
> +		err = pci_enable_sriov(pdev, num_vfs);
> +		if (err) {
> +			dev_err(&pdev->dev, "Cannot enable SRIOV: %d\n", err);
> +			return err;
> +		}
> +
> +		size = sizeof(struct ionic_vf *) * num_vfs;
> +		ionic->vf = kzalloc(size, GFP_KERNEL);
Please use kcalloc()

> +		if (!ionic->vf) {
> +			pci_disable_sriov(pdev);
> +			return -ENOMEM;
> +		}
> +
> +		for (i = 0; i < num_vfs; i++) {
> +			struct ionic_vf *v;
> +
> +			v = kzalloc(sizeof(*v), GFP_KERNEL);
> +			if (!v) {
> +				err = -ENOMEM;
> +				num_vfs = 0;
> +				goto remove_vfs;
> +			}
> +
> +			v->stats_pa = dma_map_single(dev, &v->stats,
> +						     sizeof(v->stats),
> +						     DMA_FROM_DEVICE);
> +			if (dma_mapping_error(dev, v->stats_pa)) {
> +				err = -ENODEV;
> +				kfree(v);
> +				ionic->vf[i] = NULL;
> +				num_vfs = 0;
> +				goto remove_vfs;
> +			}
> +
> +			ionic->vf[i] = v;
> +			ionic->num_vfs++;
> +		}
> +
> +		return num_vfs;
> +	}
> +
> +remove_vfs:
> +	if (num_vfs == 0) {
> +		if (err)
> +			dev_err(&pdev->dev, "SRIOV setup failed: %d\n", err);
> +
> +		pci_disable_sriov(pdev);
> +
> +		for (i = 0; i < ionic->num_vfs; i++) {
> +			dma_unmap_single(dev, ionic->vf[i]->stats_pa,
> +					 sizeof(struct ionic_lif_stats),
> +					 DMA_FROM_DEVICE);
> +			kfree(ionic->vf[i]);
> +		}
> +
> +		kfree(ionic->vf);
> +		ionic->vf = NULL;
> +		ionic->num_vfs = 0;
> +	}
> +
> +	return err;
> +}
> +
>  static struct pci_driver ionic_driver = {
>  	.name = IONIC_DRV_NAME,
>  	.id_table = ionic_id_table,
>  	.probe = ionic_probe,
>  	.remove = ionic_remove,
> +	.sriov_configure = ionic_sriov_configure,
>  };

Re: [PATCH net-next 2/2] ionic: support sr-iov operations

[Shannon Nelson]

On 12/10/19 3:25 PM, Parav Pandit wrote:
> On 12/10/2019 4:54 PM, Shannon Nelson wrote:
>> Add the netdev ops for managing VFs.  Since most of the
>> management work happens in the NIC firmware, the driver becomes
>> mostly a pass-through for the network stack commands that want
>> to control and configure the VFs.
>>
>> We also tweak ionic_station_set() a little to allow for
>> the VFs that start off with a zero'd mac address.
>>
>> Signed-off-by: Shannon Nelson <snelson@pensando.io>
>> ---
>>   drivers/net/ethernet/pensando/ionic/ionic.h   |  16 +-
>>   .../ethernet/pensando/ionic/ionic_bus_pci.c   |  75 +++++++
>>   .../net/ethernet/pensando/ionic/ionic_dev.c   |  61 ++++++
>>   .../net/ethernet/pensando/ionic/ionic_dev.h   |   7 +
>>   .../net/ethernet/pensando/ionic/ionic_lif.c   | 188 +++++++++++++++++-
>>   .../net/ethernet/pensando/ionic/ionic_lif.h   |   6 +
>>   .../net/ethernet/pensando/ionic/ionic_main.c  |   4 +
>>   7 files changed, 349 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
>> index 98e102af7756..d4cf58da8d13 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic.h
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic.h
>> @@ -12,7 +12,7 @@ struct ionic_lif;
>>   
>>   #define IONIC_DRV_NAME		"ionic"
>>   #define IONIC_DRV_DESCRIPTION	"Pensando Ethernet NIC Driver"
>> -#define IONIC_DRV_VERSION	"0.18.0-k"
>> +#define IONIC_DRV_VERSION	"0.20.0-k"
>>   
>>   #define PCI_VENDOR_ID_PENSANDO			0x1dd8
>>   
>> @@ -25,6 +25,18 @@ struct ionic_lif;
>>   
>>   #define DEVCMD_TIMEOUT  10
>>   
>> +struct ionic_vf {
>> +	u16	 index;
>> +	u8	 macaddr[6];
>> +	__le32	 maxrate;
>> +	__le16	 vlanid;
>> +	u8	 spoofchk;
>> +	u8	 trusted;
>> +	u8	 linkstate;
>> +	dma_addr_t       stats_pa;
>> +	struct ionic_lif_stats stats;
>> +};
>> +
>>   struct ionic {
>>   	struct pci_dev *pdev;
>>   	struct device *dev;
>> @@ -46,6 +58,8 @@ struct ionic {
>>   	DECLARE_BITMAP(intrs, IONIC_INTR_CTRL_REGS_MAX);
>>   	struct work_struct nb_work;
>>   	struct notifier_block nb;
>> +	int num_vfs;
> Please drop num_vfs and use pci_num_vf() in ionic_get_vf_config() and
> other friend functions.

Sure.

>> +	struct ionic_vf **vf;
>>   	struct timer_list watchdog_timer;
>>   	int watchdog_period;
>>   };
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> index 9a9ab8cb2cb3..fe4efe12b50b 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> @@ -274,11 +274,86 @@ static void ionic_remove(struct pci_dev *pdev)
>>   	ionic_devlink_free(ionic);
>>   }
>>   
>> +static int ionic_sriov_configure(struct pci_dev *pdev, int num_vfs)
>> +{
>> +	struct ionic *ionic = pci_get_drvdata(pdev);
>> +	struct device *dev = ionic->dev;
>> +	unsigned int size;
>> +	int i, err = 0;
>> +
>> +	if (!ionic_is_pf(ionic))
>> +		return -ENODEV;
>> +
> This check is already done by pci core kernel. No need for ionic driver
> to do this. If SR-IOV capability is not enabled, it won't reach here.

Got it

>
>> +	if (num_vfs > 0) {
>> +		err = pci_enable_sriov(pdev, num_vfs);
>> +		if (err) {
>> +			dev_err(&pdev->dev, "Cannot enable SRIOV: %d\n", err);
>> +			return err;
>> +		}
>> +
>> +		size = sizeof(struct ionic_vf *) * num_vfs;
>> +		ionic->vf = kzalloc(size, GFP_KERNEL);
> Please use kcalloc()

Sure

Thanks for the comments,
sln

Re: [PATCH net-next 2/2] ionic: support sr-iov operations

[Parav Pandit]

On 12/10/2019 4:54 PM, Shannon Nelson wrote:
> Add the netdev ops for managing VFs.  Since most of the
> management work happens in the NIC firmware, the driver becomes
> mostly a pass-through for the network stack commands that want
> to control and configure the VFs.
> 
> We also tweak ionic_station_set() a little to allow for
> the VFs that start off with a zero'd mac address.
> 
> Signed-off-by: Shannon Nelson <snelson@pensando.io>
> ---
>  drivers/net/ethernet/pensando/ionic/ionic.h   |  16 +-
>  .../ethernet/pensando/ionic/ionic_bus_pci.c   |  75 +++++++
>  .../net/ethernet/pensando/ionic/ionic_dev.c   |  61 ++++++
>  .../net/ethernet/pensando/ionic/ionic_dev.h   |   7 +
>  .../net/ethernet/pensando/ionic/ionic_lif.c   | 188 +++++++++++++++++-
>  .../net/ethernet/pensando/ionic/ionic_lif.h   |   6 +
>  .../net/ethernet/pensando/ionic/ionic_main.c  |   4 +
>  7 files changed, 349 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
> index 98e102af7756..d4cf58da8d13 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic.h
> @@ -12,7 +12,7 @@ struct ionic_lif;
>  
>  #define IONIC_DRV_NAME		"ionic"
>  #define IONIC_DRV_DESCRIPTION	"Pensando Ethernet NIC Driver"
> -#define IONIC_DRV_VERSION	"0.18.0-k"
> +#define IONIC_DRV_VERSION	"0.20.0-k"
>  
>  #define PCI_VENDOR_ID_PENSANDO			0x1dd8
>  
> @@ -25,6 +25,18 @@ struct ionic_lif;
>  
>  #define DEVCMD_TIMEOUT  10
>  
> +struct ionic_vf {
> +	u16	 index;
> +	u8	 macaddr[6];
> +	__le32	 maxrate;
> +	__le16	 vlanid;
> +	u8	 spoofchk;
> +	u8	 trusted;
> +	u8	 linkstate;
> +	dma_addr_t       stats_pa;
> +	struct ionic_lif_stats stats;
> +};
> +
>  struct ionic {
>  	struct pci_dev *pdev;
>  	struct device *dev;
> @@ -46,6 +58,8 @@ struct ionic {
>  	DECLARE_BITMAP(intrs, IONIC_INTR_CTRL_REGS_MAX);
>  	struct work_struct nb_work;
>  	struct notifier_block nb;
> +	int num_vfs;
> +	struct ionic_vf **vf;
>  	struct timer_list watchdog_timer;
>  	int watchdog_period;
>  };
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> index 9a9ab8cb2cb3..fe4efe12b50b 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
> @@ -274,11 +274,86 @@ static void ionic_remove(struct pci_dev *pdev)
>  	ionic_devlink_free(ionic);
>  }
>  
> +static int ionic_sriov_configure(struct pci_dev *pdev, int num_vfs)
> +{
> +	struct ionic *ionic = pci_get_drvdata(pdev);
> +	struct device *dev = ionic->dev;
> +	unsigned int size;
> +	int i, err = 0;
> +
> +	if (!ionic_is_pf(ionic))
> +		return -ENODEV;
> +
> +	if (num_vfs > 0) {
> +		err = pci_enable_sriov(pdev, num_vfs);
> +		if (err) {
> +			dev_err(&pdev->dev, "Cannot enable SRIOV: %d\n", err);
> +			return err;
> +		}
> +
> +		size = sizeof(struct ionic_vf *) * num_vfs;
> +		ionic->vf = kzalloc(size, GFP_KERNEL);
> +		if (!ionic->vf) {
> +			pci_disable_sriov(pdev);
> +			return -ENOMEM;
> +		}
> +
> +		for (i = 0; i < num_vfs; i++) {
> +			struct ionic_vf *v;
> +
> +			v = kzalloc(sizeof(*v), GFP_KERNEL);
> +			if (!v) {
> +				err = -ENOMEM;
> +				num_vfs = 0;
> +				goto remove_vfs;
> +			}
> +
> +			v->stats_pa = dma_map_single(dev, &v->stats,
> +						     sizeof(v->stats),
> +						     DMA_FROM_DEVICE);
> +			if (dma_mapping_error(dev, v->stats_pa)) {
> +				err = -ENODEV;
> +				kfree(v);
> +				ionic->vf[i] = NULL;
> +				num_vfs = 0;
> +				goto remove_vfs;
> +			}
> +
> +			ionic->vf[i] = v;
> +			ionic->num_vfs++;
> +		}
> +
> +		return num_vfs;
> +	}
> +
> +remove_vfs:
> +	if (num_vfs == 0) {
> +		if (err)
> +			dev_err(&pdev->dev, "SRIOV setup failed: %d\n", err);
> +
> +		pci_disable_sriov(pdev);
> +
> +		for (i = 0; i < ionic->num_vfs; i++) {
> +			dma_unmap_single(dev, ionic->vf[i]->stats_pa,
> +					 sizeof(struct ionic_lif_stats),
> +					 DMA_FROM_DEVICE);
> +			kfree(ionic->vf[i]);
> +		}
> +
> +		kfree(ionic->vf);
> +		ionic->vf = NULL;
> +		ionic->num_vfs = 0;
> +	}
> +
> +	return err;
> +}
> +
>  static struct pci_driver ionic_driver = {
>  	.name = IONIC_DRV_NAME,
>  	.id_table = ionic_id_table,
>  	.probe = ionic_probe,
>  	.remove = ionic_remove,
> +	.sriov_configure = ionic_sriov_configure,
>  };
>  
>  int ionic_bus_register_driver(void)
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.c b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
> index 5f9d2ec70446..8f0ab5838667 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.c
> @@ -286,6 +286,67 @@ void ionic_dev_cmd_port_pause(struct ionic_dev *idev, u8 pause_type)
>  	ionic_dev_cmd_go(idev, &cmd);
>  }
>  
> +/* VF commands */
> +int ionic_set_vf_config(struct ionic *ionic, int vf, u8 attr, u8 *data)
> +{
I forgot to mention in my previous review comment that set_vf_config()
and other VF config friend callback functions can race with
ionic_sriov_configure().

Former is called from netlink context, later is called from sysfs.
Its not too hard to crash the system both racing with each other.

Hence protect them using rwsem, where set_vf_() and sriov_configure()
does down/up_write() and get_vf_config() and get_vf_stat() does
down_up/read().

> +	union ionic_dev_cmd cmd = {
> +		.vf_setattr.opcode = IONIC_CMD_VF_SETATTR,
> +		.vf_setattr.attr = attr,
> +		.vf_setattr.vf_index = vf,
> +	};
> +	int err;
> +
> +	if (vf >= ionic->num_vfs)
> +		return -EINVAL;
> +
> +	switch (attr) {
> +	case IONIC_VF_ATTR_SPOOFCHK:
> +		cmd.vf_setattr.spoofchk = *data;
> +		dev_dbg(ionic->dev, "%s: vf %d spoof %d\n",
> +			__func__, vf, *data);
> +		break;
> +	case IONIC_VF_ATTR_TRUST:
> +		cmd.vf_setattr.trust = *data;
> +		dev_dbg(ionic->dev, "%s: vf %d trust %d\n",
> +			__func__, vf, *data);
> +		break;
> +	case IONIC_VF_ATTR_LINKSTATE:
> +		cmd.vf_setattr.linkstate = *data;
> +		dev_dbg(ionic->dev, "%s: vf %d linkstate %d\n",
> +			__func__, vf, *data);
> +		break;
> +	case IONIC_VF_ATTR_MAC:
> +		ether_addr_copy(cmd.vf_setattr.macaddr, data);
> +		dev_dbg(ionic->dev, "%s: vf %d macaddr %pM\n",
> +			__func__, vf, data);
> +		break;
> +	case IONIC_VF_ATTR_VLAN:
> +		cmd.vf_setattr.vlanid = cpu_to_le16(*(u16 *)data);
> +		dev_dbg(ionic->dev, "%s: vf %d vlan %d\n",
> +			__func__, vf, *(u16 *)data);
> +		break;
> +	case IONIC_VF_ATTR_RATE:
> +		cmd.vf_setattr.maxrate = cpu_to_le32(*(u32 *)data);
> +		dev_dbg(ionic->dev, "%s: vf %d maxrate %d\n",
> +			__func__, vf, *(u32 *)data);
> +		break;
> +	case IONIC_VF_ATTR_STATSADDR:
> +		cmd.vf_setattr.stats_pa = cpu_to_le64(*(u64 *)data);
> +		dev_dbg(ionic->dev, "%s: vf %d stats_pa 0x%08llx\n",
> +			__func__, vf, *(u64 *)data);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	mutex_lock(&ionic->dev_cmd_lock);
> +	ionic_dev_cmd_go(&ionic->idev, &cmd);
> +	err = ionic_dev_cmd_wait(ionic, DEVCMD_TIMEOUT);
> +	mutex_unlock(&ionic->dev_cmd_lock);
> +
> +	return err;
> +}
> +
>  /* LIF commands */
>  void ionic_dev_cmd_lif_identify(struct ionic_dev *idev, u8 type, u8 ver)
>  {
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> index 4665c5dc5324..7838e342c4fd 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> @@ -113,6 +113,12 @@ static_assert(sizeof(struct ionic_rxq_desc) == 16);
>  static_assert(sizeof(struct ionic_rxq_sg_desc) == 128);
>  static_assert(sizeof(struct ionic_rxq_comp) == 16);
>  
> +/* SR/IOV */
> +static_assert(sizeof(struct ionic_vf_setattr_cmd) == 64);
> +static_assert(sizeof(struct ionic_vf_setattr_comp) == 16);
> +static_assert(sizeof(struct ionic_vf_getattr_cmd) == 64);
> +static_assert(sizeof(struct ionic_vf_getattr_comp) == 16);
> +
>  struct ionic_devinfo {
>  	u8 asic_type;
>  	u8 asic_rev;
> @@ -275,6 +281,7 @@ void ionic_dev_cmd_port_autoneg(struct ionic_dev *idev, u8 an_enable);
>  void ionic_dev_cmd_port_fec(struct ionic_dev *idev, u8 fec_type);
>  void ionic_dev_cmd_port_pause(struct ionic_dev *idev, u8 pause_type);
>  
> +int ionic_set_vf_config(struct ionic *ionic, int vf, u8 attr, u8 *data);
>  void ionic_dev_cmd_lif_identify(struct ionic_dev *idev, u8 type, u8 ver);
>  void ionic_dev_cmd_lif_init(struct ionic_dev *idev, u16 lif_index,
>  			    dma_addr_t addr);
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index 60fd14df49d7..9eb3ce057aa8 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -1616,6 +1616,168 @@ int ionic_stop(struct net_device *netdev)
>  	return err;
>  }
>  
> +static int ionic_get_vf_config(struct net_device *netdev,
> +			       int vf, struct ifla_vf_info *ivf)
> +{
> +	struct ionic_lif *lif = netdev_priv(netdev);
> +	struct ionic *ionic = lif->ionic;
> +
> +	if (vf >= ionic->num_vfs)
> +		return -EINVAL;
> +
> +	ivf->vf           = vf;
> +	ivf->vlan         = ionic->vf[vf]->vlanid;
> +	ivf->qos	  = 0;
> +	ivf->spoofchk     = ionic->vf[vf]->spoofchk;
> +	ivf->linkstate    = ionic->vf[vf]->linkstate;
> +	ivf->max_tx_rate  = ionic->vf[vf]->maxrate;
> +	ivf->trusted      = ionic->vf[vf]->trusted;
> +	ether_addr_copy(ivf->mac, ionic->vf[vf]->macaddr);
> +
> +	return 0;
> +}
> +
> +static int ionic_get_vf_stats(struct net_device *netdev, int vf,
> +			      struct ifla_vf_stats *vf_stats)
> +{
> +	struct ionic_lif *lif = netdev_priv(netdev);
> +	struct ionic *ionic = lif->ionic;
> +	struct ionic_lif_stats *vs;
> +
> +	if (vf >= ionic->num_vfs)
> +		return -EINVAL;
> +
> +	memset(vf_stats, 0, sizeof(*vf_stats));
> +	vs = &ionic->vf[vf]->stats;
> +
> +	vf_stats->rx_packets = le64_to_cpu(vs->rx_ucast_packets);
> +	vf_stats->tx_packets = le64_to_cpu(vs->tx_ucast_packets);
> +	vf_stats->rx_bytes   = le64_to_cpu(vs->rx_ucast_bytes);
> +	vf_stats->tx_bytes   = le64_to_cpu(vs->tx_ucast_bytes);
> +	vf_stats->broadcast  = le64_to_cpu(vs->rx_bcast_packets);
> +	vf_stats->multicast  = le64_to_cpu(vs->rx_mcast_packets);
> +	vf_stats->rx_dropped = le64_to_cpu(vs->rx_ucast_drop_packets) +
> +			       le64_to_cpu(vs->rx_mcast_drop_packets) +
> +			       le64_to_cpu(vs->rx_bcast_drop_packets);
> +	vf_stats->tx_dropped = le64_to_cpu(vs->tx_ucast_drop_packets) +
> +			       le64_to_cpu(vs->tx_mcast_drop_packets) +
> +			       le64_to_cpu(vs->tx_bcast_drop_packets);
> +
> +	return 0;
> +}
> +
> +static int ionic_set_vf_mac(struct net_device *netdev, int vf, u8 *mac)
> +{
> +	struct ionic_lif *lif = netdev_priv(netdev);
> +	int ret;
> +
> +	if (!(is_zero_ether_addr(mac) || is_valid_ether_addr(mac)))
> +		return -EINVAL;
> +
> +	ret = ionic_set_vf_config(lif->ionic, vf,
> +				  IONIC_VF_ATTR_MAC, mac);
> +	if (!ret)
> +		ether_addr_copy(lif->ionic->vf[vf]->macaddr, mac);
> +
> +	return ret;
> +}
> +
> +static int ionic_set_vf_vlan(struct net_device *netdev, int vf, u16 vlan,
> +			     u8 qos, __be16 proto)
> +{
> +	struct ionic_lif *lif = netdev_priv(netdev);
> +	int ret;
> +
> +	/* until someday when we support qos */
> +	if (qos)
> +		return -EINVAL;
> +
> +	if (vlan > 4095)
> +		return -EINVAL;
> +
> +	if (proto != htons(ETH_P_8021Q))
> +		return -EPROTONOSUPPORT;
> +
> +	ret = ionic_set_vf_config(lif->ionic, vf,
> +				  IONIC_VF_ATTR_VLAN, (u8 *)&vlan);
> +	if (!ret)
> +		lif->ionic->vf[vf]->vlanid = vlan;
> +
> +	return ret;
> +}
> +
> +static int ionic_set_vf_rate(struct net_device *netdev, int vf,
> +			     int tx_min, int tx_max)
> +{
> +	struct ionic_lif *lif = netdev_priv(netdev);
> +	int ret;
> +
> +	/* setting the min just seems silly */
> +	if (tx_min)
> +		return -EINVAL;
> +
> +	ret = ionic_set_vf_config(lif->ionic, vf,
> +				  IONIC_VF_ATTR_RATE, (u8 *)&tx_max);
> +	if (!ret)
> +		lif->ionic->vf[vf]->maxrate = tx_max;
> +
> +	return ret;
> +}
> +
> +static int ionic_set_vf_spoofchk(struct net_device *netdev, int vf, bool set)
> +{
> +	struct ionic_lif *lif = netdev_priv(netdev);
> +	u8 data = set;  /* convert to u8 for config */
> +	int ret;
> +
> +	ret = ionic_set_vf_config(lif->ionic, vf,
> +				  IONIC_VF_ATTR_SPOOFCHK, &data);
> +	if (!ret)
> +		lif->ionic->vf[vf]->spoofchk = data;
> +
> +	return ret;
> +}
> +
> +static int ionic_set_vf_trust(struct net_device *netdev, int vf, bool set)
> +{
> +	struct ionic_lif *lif = netdev_priv(netdev);
> +	u8 data = set;  /* convert to u8 for config */
> +	int ret;
> +
> +	ret = ionic_set_vf_config(lif->ionic, vf,
> +				  IONIC_VF_ATTR_TRUST, &data);
> +	if (!ret)
> +		lif->ionic->vf[vf]->trusted = data;
> +
> +	return ret;
> +}
> +
> +static int ionic_set_vf_link_state(struct net_device *netdev, int vf, int set)
> +{
> +	struct ionic_lif *lif = netdev_priv(netdev);
> +	u8 data;
> +	int ret;
> +
> +	switch (set) {
> +	case IFLA_VF_LINK_STATE_AUTO:
> +		data = IONIC_VF_LINK_STATUS_AUTO;
> +		break;
> +	case IFLA_VF_LINK_STATE_ENABLE:
> +		data = IONIC_VF_LINK_STATUS_UP;
> +		break;
> +	case IFLA_VF_LINK_STATE_DISABLE:
> +		data = IONIC_VF_LINK_STATUS_DOWN;
> +		break;
> +	}
> +
> +	ret = ionic_set_vf_config(lif->ionic, vf,
> +				  IONIC_VF_ATTR_LINKSTATE, &data);
> +	if (!ret)
> +		lif->ionic->vf[vf]->linkstate = set;
> +
> +	return ret;
> +}
> +
>  static const struct net_device_ops ionic_netdev_ops = {
>  	.ndo_open               = ionic_open,
>  	.ndo_stop               = ionic_stop,
> @@ -1629,6 +1791,14 @@ static const struct net_device_ops ionic_netdev_ops = {
>  	.ndo_change_mtu         = ionic_change_mtu,
>  	.ndo_vlan_rx_add_vid    = ionic_vlan_rx_add_vid,
>  	.ndo_vlan_rx_kill_vid   = ionic_vlan_rx_kill_vid,
> +	.ndo_set_vf_vlan	= ionic_set_vf_vlan,
> +	.ndo_set_vf_trust	= ionic_set_vf_trust,
> +	.ndo_set_vf_mac		= ionic_set_vf_mac,
> +	.ndo_set_vf_rate	= ionic_set_vf_rate,
> +	.ndo_set_vf_spoofchk	= ionic_set_vf_spoofchk,
> +	.ndo_get_vf_config	= ionic_get_vf_config,
> +	.ndo_set_vf_link_state	= ionic_set_vf_link_state,
> +	.ndo_get_vf_stats       = ionic_get_vf_stats,
>  };
>  
>  int ionic_reset_queues(struct ionic_lif *lif)
> @@ -1961,18 +2131,22 @@ static int ionic_station_set(struct ionic_lif *lif)
>  	if (err)
>  		return err;
>  
> +	if (is_zero_ether_addr(ctx.comp.lif_getattr.mac))
> +		return 0;
> +
>  	memcpy(addr.sa_data, ctx.comp.lif_getattr.mac, netdev->addr_len);
>  	addr.sa_family = AF_INET;
>  	err = eth_prepare_mac_addr_change(netdev, &addr);
> -	if (err)
> -		return err;
> -
> -	if (!is_zero_ether_addr(netdev->dev_addr)) {
> -		netdev_dbg(lif->netdev, "deleting station MAC addr %pM\n",
> -			   netdev->dev_addr);
> -		ionic_lif_addr(lif, netdev->dev_addr, false);
> +	if (err) {
> +		netdev_warn(lif->netdev, "ignoring bad MAC addr from NIC %pM\n",
> +			    addr.sa_data);
> +		return 0;
>  	}
>  
> +	netdev_dbg(lif->netdev, "deleting station MAC addr %pM\n",
> +		   netdev->dev_addr);
> +	ionic_lif_addr(lif, netdev->dev_addr, false);
> +
>  	eth_commit_mac_addr_change(netdev, &addr);
>  	netdev_dbg(lif->netdev, "adding station MAC addr %pM\n",
>  		   netdev->dev_addr);
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.h b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> index a55fd1f8c31b..03b8bd7e3730 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.h
> @@ -195,6 +195,12 @@ static inline int ionic_wait_for_bit(struct ionic_lif *lif, int bitname)
>  	return wait_on_bit_lock(lif->state, bitname, TASK_INTERRUPTIBLE);
>  }
>  
> +static inline bool ionic_is_pf(struct ionic *ionic)
> +{
> +	return ionic->pdev &&
> +	       ionic->pdev->device == PCI_DEVICE_ID_PENSANDO_IONIC_ETH_PF;
> +}
> +
>  static inline u32 ionic_coal_usec_to_hw(struct ionic *ionic, u32 usecs)
>  {
>  	u32 mult = le32_to_cpu(ionic->ident.dev.intr_coal_mult);
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> index 3590ea7fd88a..837b85f2fed9 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
> @@ -165,6 +165,10 @@ static const char *ionic_opcode_to_str(enum ionic_cmd_opcode opcode)
>  		return "IONIC_CMD_FW_DOWNLOAD";
>  	case IONIC_CMD_FW_CONTROL:
>  		return "IONIC_CMD_FW_CONTROL";
> +	case IONIC_CMD_VF_GETATTR:
> +		return "IONIC_CMD_VF_GETATTR";
> +	case IONIC_CMD_VF_SETATTR:
> +		return "IONIC_CMD_VF_SETATTR";
>  	default:
>  		return "DEVCMD_UNKNOWN";
>  	}
> 

Re: [PATCH net-next 2/2] ionic: support sr-iov operations

[Shannon Nelson]

On 12/10/19 5:39 PM, Parav Pandit wrote:
> On 12/10/2019 4:54 PM, Shannon Nelson wrote:
>> Add the netdev ops for managing VFs.  Since most of the
>> management work happens in the NIC firmware, the driver becomes
>> mostly a pass-through for the network stack commands that want
>> to control and configure the VFs.
>>
>> We also tweak ionic_station_set() a little to allow for
>> the VFs that start off with a zero'd mac address.
>>
>> Signed-off-by: Shannon Nelson <snelson@pensando.io>
>> ---
[...]
>>   
>> +/* VF commands */
>> +int ionic_set_vf_config(struct ionic *ionic, int vf, u8 attr, u8 *data)
>> +{
> I forgot to mention in my previous review comment that set_vf_config()
> and other VF config friend callback functions can race with
> ionic_sriov_configure().
>
> Former is called from netlink context, later is called from sysfs.
> Its not too hard to crash the system both racing with each other.
>
> Hence protect them using rwsem, where set_vf_() and sriov_configure()
> does down/up_write() and get_vf_config() and get_vf_stat() does
> down_up/read().
>
>

Ah, good catch.  That seems to be relatively a new thing and with a 
quick look around it seems not many drivers deal with that yet. Thanks 
for pointing it out.

sln

Re: [PATCH net-next 2/2] ionic: support sr-iov operations

[Parav Pandit]

On 12/10/2019 9:20 PM, Shannon Nelson wrote:
> On 12/10/19 5:39 PM, Parav Pandit wrote:
>> On 12/10/2019 4:54 PM, Shannon Nelson wrote:
>>> Add the netdev ops for managing VFs.  Since most of the
>>> management work happens in the NIC firmware, the driver becomes
>>> mostly a pass-through for the network stack commands that want
>>> to control and configure the VFs.
>>>
>>> We also tweak ionic_station_set() a little to allow for
>>> the VFs that start off with a zero'd mac address.
>>>
>>> Signed-off-by: Shannon Nelson <snelson@pensando.io>
>>> ---
> [...]
>>>   +/* VF commands */
>>> +int ionic_set_vf_config(struct ionic *ionic, int vf, u8 attr, u8 *data)
>>> +{
>> I forgot to mention in my previous review comment that set_vf_config()
>> and other VF config friend callback functions can race with
>> ionic_sriov_configure().
>>
>> Former is called from netlink context, later is called from sysfs.
>> Its not too hard to crash the system both racing with each other.
>>
>> Hence protect them using rwsem, where set_vf_() and sriov_configure()
>> does down/up_write() and get_vf_config() and get_vf_stat() does
>> down_up/read().
>>
>>
> 
> Ah, good catch.  That seems to be relatively a new thing and with a
> quick look around it seems not many drivers deal with that yet. Thanks
> for pointing it out.
>
Yes. I am aware of it for a while. I am fixing mlx5 driver currently.

> sln
>