[RFC PATCH V2] ethtool: Add command to configure IOV features

[RFC PATCH V2] ethtool: Add command to configure IOV features

[Greg Rose]

New command to allow configuration of IOV features such as the number of
Virtual Functions to allocate for a given Physical Function interface,
the number of semi-independent net devices to allocate from partitioned
I/O resources in the PF and to set the number of queues per VF.

Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
---

 ethtool.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 93 insertions(+), 1 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 35c3733..1ebf6f3 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -99,6 +99,8 @@ static int do_flash(int fd, struct ifreq *ifr);
 static int do_permaddr(int fd, struct ifreq *ifr);
 static int do_getfwdump(int fd, struct ifreq *ifr);
 static int do_setfwdump(int fd, struct ifreq *ifr);
+static int do_get_iov(int fd, struct ifreq *ifr);
+static int do_set_iov(int fd, struct ifreq *ifr);
 
 static int send_ioctl(int fd, struct ifreq *ifr);
 
@@ -133,6 +135,8 @@ static enum {
 	MODE_PERMADDR,
 	MODE_SET_DUMP,
 	MODE_GET_DUMP,
+	MODE_GET_IOV,
+	MODE_SET_IOV,
 } mode = MODE_GSET;
 
 static struct option {
@@ -266,6 +270,8 @@ static struct option {
     { "-W", "--set-dump", MODE_SET_DUMP,
 		"Set dump flag of the device",
 		"		N\n"},
+    { "-v", "--get-iov", MODE_GET_IOV, "Get IOV parameters", "\n" },
+    { "-V", "--set_iov", MODE_SET_IOV, "Set IOV parameters", "[ N ]\n" },
     { "-h", "--help", MODE_HELP, "Show this help" },
     { NULL, "--version", MODE_VERSION, "Show version number" },
     {}
@@ -392,6 +398,10 @@ static char **rxfhindir_weight = NULL;
 static char *flash_file = NULL;
 static int flash = -1;
 static int flash_region = -1;
+static int iov_changed = 0;
+static int iov_numvfs_wanted = -1;
+static int iov_numnetdevs_wanted = -1;
+static int iov_numvqueues_wanted = -1;
 
 static int msglvl_changed;
 static u32 msglvl_wanted = 0;
@@ -549,6 +559,12 @@ static struct cmdline_info cmdline_msglvl[] = {
 	  NETIF_MSG_WOL, &msglvl_mask },
 };
 
+static struct cmdline_info cmdline_iov[] = {
+	{ "vfs", CMDL_S32, &iov_numvfs_wanted, NULL },
+	{ "netdevs", CMDL_S32, &iov_numnetdevs_wanted, NULL },
+	{ "vqueues", CMDL_S32, &iov_numvqueues_wanted, NULL },
+};
+
 static long long
 get_int_range(char *str, int base, long long min, long long max)
 {
@@ -792,7 +808,9 @@ static void parse_cmdline(int argc, char **argp)
 			    (mode == MODE_FLASHDEV) ||
 			    (mode == MODE_PERMADDR) ||
 			    (mode == MODE_SET_DUMP) ||
-			    (mode == MODE_GET_DUMP)) {
+			    (mode == MODE_GET_DUMP) ||
+			    (mode == MODE_GET_IOV) ||
+			    (mode == MODE_SET_IOV)) {
 				devname = argp[i];
 				break;
 			}
@@ -1007,6 +1025,14 @@ static void parse_cmdline(int argc, char **argp)
 				i = argc;
 				break;
 			}
+			if (mode == MODE_SET_IOV) {
+				parse_generic_cmdline(argc, argp, i,
+					&iov_changed,
+					cmdline_iov,
+					ARRAY_SIZE(cmdline_iov));
+				i = argc;
+				break;
+			}
 			if (mode != MODE_SSET)
 				exit_bad_args();
 			if (!strcmp(argp[i], "speed")) {
@@ -1949,6 +1975,10 @@ static int doit(void)
 		return do_getfwdump(fd, &ifr);
 	} else if (mode == MODE_SET_DUMP) {
 		return do_setfwdump(fd, &ifr);
+	} else if (mode == MODE_GET_IOV) {
+		return do_get_iov(fd, &ifr);
+	} else if (mode == MODE_SET_IOV) {
+		return do_set_iov(fd, &ifr);
 	}
 
 	return 69;
@@ -3338,6 +3368,68 @@ static int do_setfwdump(int fd, struct ifreq *ifr)
 	return 0;
 }
 
+static int do_get_iov(int fd, struct ifreq *ifr)
+{
+	int err;
+	struct ethtool_iov_get_cmd iov_cmd;
+
+	iov_cmd.cmd = ETHTOOL_IOV_GET_CMD;
+	ifr->ifr_data = (caddr_t)&iov_cmd;
+	err = send_ioctl(fd, ifr);
+	if (err < 0) {
+		perror("Can not get current IOV mode\n");
+		return 1;
+	}
+
+	memcpy(&iov_cmd, ifr->ifr_data, sizeof(iov_cmd));
+
+	switch(iov_cmd.mode) {
+	case ETHTOOL_IOV_MODE_SRIOV:
+		printf("Device %s is configured for SR-IOV mode\n", devname);
+		break;
+	case ETHTOOL_IOV_MODE_VM_QUEUES:
+		printf("Device %s is configured for VM Queues mode\n", devname);
+		break;
+	case ETHTOOL_IOV_MODE_NONE:
+	default:
+		printf("Device %s is not configured for IO Virtualization\n",
+				devname);
+		break;
+	}
+
+	return 0;
+}
+
+static int do_set_iov(int fd, struct ifreq *ifr)
+{
+	int err;
+	struct ethtool_iov_set_cmd iov_cmd;
+
+	if (iov_changed) {
+		iov_cmd.cmd = ETHTOOL_IOV_SET_CMD;
+		if (iov_numvfs_wanted >= 0) {
+			iov_cmd.set_cmd = ETHTOOL_IOV_CMD_CONFIGURE_SRIOV;
+			iov_cmd.cmd_param = iov_numvfs_wanted;
+		} else if (iov_numnetdevs_wanted >= 0) {
+			iov_cmd.set_cmd = ETHTOOL_IOV_CMD_CONFIGURE_NETDEVS;
+			iov_cmd.cmd_param = iov_numnetdevs_wanted;
+		} else if (iov_numvqueues_wanted >= 0) {
+			iov_cmd.set_cmd = ETHTOOL_IOV_CMD_CONFIGURE_VF_QUEUES;
+			iov_cmd.cmd_param = iov_numvqueues_wanted;
+		} else {
+			return -EINVAL;
+		}
+		ifr->ifr_data = (caddr_t)&iov_cmd;
+		err = send_ioctl(fd, ifr);
+		if (err < 0) {
+			perror("Can not set new IOV mode\n");
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
 static int send_ioctl(int fd, struct ifreq *ifr)
 {
 	return ioctl(fd, SIOCETHTOOL, ifr);

Re: [RFC PATCH V2] ethtool: Add command to configure IOV features

[Ben Hutchings]

On Thu, 2011-09-22 at 14:35 -0700, Greg Rose wrote:
> New command to allow configuration of IOV features such as the number of
> Virtual Functions to allocate for a given Physical Function interface,
> the number of semi-independent net devices to allocate from partitioned
> I/O resources in the PF and to set the number of queues per VF.
[...]
> @@ -266,6 +270,8 @@ static struct option {
>      { "-W", "--set-dump", MODE_SET_DUMP,
>  		"Set dump flag of the device",
>  		"		N\n"},
> +    { "-v", "--get-iov", MODE_GET_IOV, "Get IOV parameters", "\n" },
> +    { "-V", "--set_iov", MODE_SET_IOV, "Set IOV parameters", "[ N ]\n" },

Doesn't match the implementation.

[...]
> +static int do_get_iov(int fd, struct ifreq *ifr)
> +{
> +	int err;
> +	struct ethtool_iov_get_cmd iov_cmd;
> +
> +	iov_cmd.cmd = ETHTOOL_IOV_GET_CMD;
> +	ifr->ifr_data = (caddr_t)&iov_cmd;
> +	err = send_ioctl(fd, ifr);
> +	if (err < 0) {
> +		perror("Can not get current IOV mode\n");
> +		return 1;
> +	}
> +
> +	memcpy(&iov_cmd, ifr->ifr_data, sizeof(iov_cmd));

But ifr->ifr_data == &iov_cmd.  So this is both pointless and dangerous
(as memcpy() doesn't handle overlapping source and destination).

[...]
> +static int do_set_iov(int fd, struct ifreq *ifr)
> +{
> +	int err;
> +	struct ethtool_iov_set_cmd iov_cmd;
> +
> +	if (iov_changed) {
> +		iov_cmd.cmd = ETHTOOL_IOV_SET_CMD;
> +		if (iov_numvfs_wanted >= 0) {
> +			iov_cmd.set_cmd = ETHTOOL_IOV_CMD_CONFIGURE_SRIOV;
> +			iov_cmd.cmd_param = iov_numvfs_wanted;
> +		} else if (iov_numnetdevs_wanted >= 0) {
> +			iov_cmd.set_cmd = ETHTOOL_IOV_CMD_CONFIGURE_NETDEVS;
> +			iov_cmd.cmd_param = iov_numnetdevs_wanted;
> +		} else if (iov_numvqueues_wanted >= 0) {
> +			iov_cmd.set_cmd = ETHTOOL_IOV_CMD_CONFIGURE_VF_QUEUES;
> +			iov_cmd.cmd_param = iov_numvqueues_wanted;
> +		} else {
> +			return -EINVAL;
> +		}
[...]

So what if the user specifies multiple keywords?

Also missing an update to the manual page.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

RE: [RFC PATCH V2] ethtool: Add command to configure IOV features

[Rose, Gregory V]

> -----Original Message-----
> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Sent: Friday, October 07, 2011 5:41 PM
> To: Rose, Gregory V
> Cc: netdev@vger.kernel.org
> Subject: Re: [RFC PATCH V2] ethtool: Add command to configure IOV features
> 
> On Thu, 2011-09-22 at 14:35 -0700, Greg Rose wrote:
> > New command to allow configuration of IOV features such as the number of
> > Virtual Functions to allocate for a given Physical Function interface,
> > the number of semi-independent net devices to allocate from partitioned
> > I/O resources in the PF and to set the number of queues per VF.
> [...]
> > @@ -266,6 +270,8 @@ static struct option {
> >      { "-W", "--set-dump", MODE_SET_DUMP,
> >  		"Set dump flag of the device",
> >  		"		N\n"},
> > +    { "-v", "--get-iov", MODE_GET_IOV, "Get IOV parameters", "\n" },
> > +    { "-V", "--set_iov", MODE_SET_IOV, "Set IOV parameters", "[ N ]\n"
> },
> 
> Doesn't match the implementation.

Whoops... OK.

> 
> [...]
> > +static int do_get_iov(int fd, struct ifreq *ifr)
> > +{
> > +	int err;
> > +	struct ethtool_iov_get_cmd iov_cmd;
> > +
> > +	iov_cmd.cmd = ETHTOOL_IOV_GET_CMD;
> > +	ifr->ifr_data = (caddr_t)&iov_cmd;
> > +	err = send_ioctl(fd, ifr);
> > +	if (err < 0) {
> > +		perror("Can not get current IOV mode\n");
> > +		return 1;
> > +	}
> > +
> > +	memcpy(&iov_cmd, ifr->ifr_data, sizeof(iov_cmd));
> 
> But ifr->ifr_data == &iov_cmd.  So this is both pointless and dangerous
> (as memcpy() doesn't handle overlapping source and destination).

Huh... what was I thinking?  Yeah, I'll fix that.

> 
> [...]
> > +static int do_set_iov(int fd, struct ifreq *ifr)
> > +{
> > +	int err;
> > +	struct ethtool_iov_set_cmd iov_cmd;
> > +
> > +	if (iov_changed) {
> > +		iov_cmd.cmd = ETHTOOL_IOV_SET_CMD;
> > +		if (iov_numvfs_wanted >= 0) {
> > +			iov_cmd.set_cmd = ETHTOOL_IOV_CMD_CONFIGURE_SRIOV;
> > +			iov_cmd.cmd_param = iov_numvfs_wanted;
> > +		} else if (iov_numnetdevs_wanted >= 0) {
> > +			iov_cmd.set_cmd = ETHTOOL_IOV_CMD_CONFIGURE_NETDEVS;
> > +			iov_cmd.cmd_param = iov_numnetdevs_wanted;
> > +		} else if (iov_numvqueues_wanted >= 0) {
> > +			iov_cmd.set_cmd = ETHTOOL_IOV_CMD_CONFIGURE_VF_QUEUES;
> > +			iov_cmd.cmd_param = iov_numvqueues_wanted;
> > +		} else {
> > +			return -EINVAL;
> > +		}
> [...]
> 
> So what if the user specifies multiple keywords?

I hadn't contemplated that.

> 
> Also missing an update to the manual page.

When I get through the RFC process and we've settled on something acceptable to the community I'll update the man page.

Thanks for the review Ben.

- Greg