RE: [PATCH net-next V2 1/9] liquidio CN23XX: HW config for VF support

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Thursday, October 20, 2016 1:57 PM
To: Vatsavayi, Raghu
Cc: netdev@vger.kernel.org; Chickles, Derek; Burla, Satananda; Manlunas,
Felix
Subject: Re: [PATCH net-next V2 1/9] liquidio CN23XX: HW config for VF
support

From: "Vatsavayi, Raghu" <redacted>
Date: Thu, 20 Oct 2016 20:01:37 +0000

quoted

quoted

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Thursday, October 20, 2016 11:13 AM
To: Vatsavayi, Raghu
Cc: netdev@vger.kernel.org; Vatsavayi, Raghu; Chickles, Derek; Burla,
Satananda; Manlunas, Felix
Subject: Re: [PATCH net-next V2 1/9] liquidio CN23XX: HW config for
VF support

From: Raghu Vatsavayi <redacted>
Date: Wed, 19 Oct 2016 22:40:38 -0700

quoted

+/* Default behaviour of Liquidio is to provide one queue per VF.
+But Liquidio
+ * can also provide multiple queues to each VF. If user wants to
+change the
+ * default behaviour HW should be provided configuration info at
+init time,
+ * based on which it will create control queues for communicating
+with

FW.

quoted

+ */
+static u32 max_vfs[2] = { 0, 0 };
+module_param_array(max_vfs, int, NULL, 0444);
+MODULE_PARM_DESC(max_vfs, "Assign two comma-separated

unsigned

quoted

quoted

quoted

+integers that specify max number of VFs for PF0 (left of the
+comma) and PF1 (right of the comma); for 23xx only. By default HW
+will configure as many VFs as queues after allocating PF queues.To
+increase queues for VF use this parameter. Use sysfs to create
+these VFs.");
+
+static unsigned int num_queues_per_pf[2] = { 0, 0 };
+module_param_array(num_queues_per_pf, uint, NULL, 0444);
+MODULE_PARM_DESC(num_queues_per_pf, "two comma-separated

unsigned

quoted

+integers that specify number of queues per PF0 (left of the comma)
+and PF1 (right of the comma); for 23xx only");
+
 static int ptp_enable = 1;

We cannot continue to allow drivers to add custom module parameters
to control this.  It is the worst user experience possible.

We need a tree-wide generic, consistent, manner in which to configure
and control this kind of thing.

Sure Dave, I will remove max_vfs module parameter and will use tree
wide generic sysfs interface to enable VFs.

That's not what I meant.

I mean there needs to be a generic mechanism that isn't a per-device knob
(be it a module parameter or a sysctl, to me these are identical functionality
and user experience wise).

Something like ethtool or netlink.

Dave, I will remove max_vfs module parameter and will just use the generic mechanism that all drivers do like:

" echo 10 > /sys/devices/pci0000:00/0000:00:03.0/0000:03:00.1/sriov_numvfs"

Regarding other module parameters, in non-default case if user wants to have multiple queues then because of the way Liquidio HW works we need num_queues_per_pf and num_queues_per_vf module parameters at HW/module init time. This is because in multi-queues per VF scenario, HW has to carve these queues before FW can start communicating with PF/VF host drivers, so we must include these two parameters. 

Please confirm that having these two module parameters is fine for non-default case. I will soon forward you the patches with these changes that you have recommended.

Thanks Much
Raghu.