| Message ID | 20110922213543.26713.23525.stgit@gitlad.jf.intel.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
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.
> -----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
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);
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(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html