On Fri, Aug 09, 2024 at 01:16:28PM +0200, Jiri Pirko wrote:

Thu, Aug 08, 2024 at 07:30:49PM CEST, anthony.l.nguyen@intel.com wrote:

quoted

From: Piotr Raczynski <redacted>

[...]

quoted

+static int
+ice_devlink_port_new_check_attr(struct ice_pf *pf,
+				const struct devlink_port_new_attrs *new_attr,
+				struct netlink_ext_ack *extack)
+{
+	if (new_attr->flavour != DEVLINK_PORT_FLAVOUR_PCI_SF) {
+		NL_SET_ERR_MSG_MOD(extack, "Flavour other than pcisf is not supported");
+		return -EOPNOTSUPP;
+	}
+
+	if (new_attr->controller_valid) {
+		NL_SET_ERR_MSG_MOD(extack, "Setting controller is not supported");
+		return -EOPNOTSUPP;
+	}
+
+	if (new_attr->port_index_valid) {
+		NL_SET_ERR_MSG_MOD(extack, "Port index is invalid");

Nope, it is actually valid, but your driver does not support that.
Please fix the message.

Ok

quoted

+		return -EOPNOTSUPP;
+	}
+
+	if (new_attr->pfnum != pf->hw.bus.func) {

hw.bus.func, hmm. How about if you pass-through the PF to VM, can't the
func be anything? Will this still make sense? I don't think so. If you
get the PF number like this in the rest of the driver, you need to fix
this.

I can change it to our internal value. I wonder if it will be better. If
I understand correctly, now:
PF0 - xx.xx.0; PF1 - xx.xx.1

I am doing pass-through PF1
PF0 (on host) - xx.xx.0

PF1 (on VM) - xx.xx.0 (there is one PF on VM, so for me it is more
intuitive to have func 0)

after I change:
PF0 - xx.xx.0; PF1 - xx.xx.1

pass-through PF1
PF0 (on host) - xx.xx.0

PF1 (on VM) - xx.xx.0, but user will have to use 1 as pf num

Correct me if I am wrong.

quoted

+		NL_SET_ERR_MSG_MOD(extack, "Incorrect pfnum supplied");
+		return -EINVAL;
+	}
+
+	if (!pci_msix_can_alloc_dyn(pf->pdev)) {
+		NL_SET_ERR_MSG_MOD(extack, "Dynamic MSIX-X interrupt allocation is not supported");
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}

[...]

quoted

+int ice_devlink_create_sf_port(struct ice_dynamic_port *dyn_port)
+{
+	struct devlink_port_attrs attrs = {};
+	struct devlink_port *devlink_port;
+	struct devlink *devlink;
+	struct ice_vsi *vsi;
+	struct ice_pf *pf;
+
+	vsi = dyn_port->vsi;
+	pf = dyn_port->pf;
+
+	devlink_port = &dyn_port->devlink_port;
+
+	attrs.flavour = DEVLINK_PORT_FLAVOUR_PCI_SF;
+	attrs.pci_sf.pf = pf->hw.bus.func;

Same here.

quoted

+	attrs.pci_sf.sf = dyn_port->sfnum;
+
+	devlink_port_attrs_set(devlink_port, &attrs);
+	devlink = priv_to_devlink(pf);
+
+	return devl_port_register_with_ops(devlink, devlink_port, vsi->idx,
+					   &ice_devlink_port_sf_ops);
+}
+

[...]