| Message ID | 20211217080827.266799-5-parav@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | David Ahern |
| Headers | show |
| Series | vdpa tool to query and set config layout | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On 12/17/21 1:08 AM, Parav Pandit wrote: > @@ -204,6 +217,8 @@ static void vdpa_opts_put(struct nlmsghdr *nlh, struct vdpa *vdpa) > if (opts->present & VDPA_OPT_VDEV_MAC) > mnl_attr_put(nlh, VDPA_ATTR_DEV_NET_CFG_MACADDR, > sizeof(opts->mac), opts->mac); > + if (opts->present & VDPA_OPT_VDEV_MTU) > + mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MTU, opts->mtu); Why limit the MTU to a u16? Eric for example is working on "Big TCP" where IPv6 can work with Jumbograms where mtu can be > 64k. https://datatracker.ietf.org/doc/html/rfc2675
On Sat, Dec 18, 2021 at 01:53:01PM -0700, David Ahern wrote: > On 12/17/21 1:08 AM, Parav Pandit wrote: > > @@ -204,6 +217,8 @@ static void vdpa_opts_put(struct nlmsghdr *nlh, struct vdpa *vdpa) > > if (opts->present & VDPA_OPT_VDEV_MAC) > > mnl_attr_put(nlh, VDPA_ATTR_DEV_NET_CFG_MACADDR, > > sizeof(opts->mac), opts->mac); > > + if (opts->present & VDPA_OPT_VDEV_MTU) > > + mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MTU, opts->mtu); > > Why limit the MTU to a u16? Eric for example is working on "Big TCP" > where IPv6 can work with Jumbograms where mtu can be > 64k. > > https://datatracker.ietf.org/doc/html/rfc2675 Well it's 16 bit at the virtio level, though we can extend that of course. Making it match for now removes need for validation.
> From: Michael S. Tsirkin <mst@redhat.com> > Sent: Sunday, December 19, 2021 3:37 AM > > On Sat, Dec 18, 2021 at 01:53:01PM -0700, David Ahern wrote: > > On 12/17/21 1:08 AM, Parav Pandit wrote: > > > @@ -204,6 +217,8 @@ static void vdpa_opts_put(struct nlmsghdr *nlh, > struct vdpa *vdpa) > > > if (opts->present & VDPA_OPT_VDEV_MAC) > > > mnl_attr_put(nlh, VDPA_ATTR_DEV_NET_CFG_MACADDR, > > > sizeof(opts->mac), opts->mac); > > > + if (opts->present & VDPA_OPT_VDEV_MTU) > > > + mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MTU, > opts->mtu); > > > > Why limit the MTU to a u16? Eric for example is working on "Big TCP" > > where IPv6 can work with Jumbograms where mtu can be > 64k. > > > > https://datatracker.ietf.org/doc/html/rfc2675 > > Well it's 16 bit at the virtio level, though we can extend that of course. Making > it match for now removes need for validation. > -- As Michael mentioned virtio specification limits the mtu to 64k-1. Hence 16-bit. First we need to update the virtio spec to support > 64K mtu. However, when/if (I don't know when) that happens, we need to make this also u32. So may be we can make it u32 now, but still restrict the mtu value to 64k-1 in kernel, until the spec is updated. Let me know, if you think that's future proofing is better, I first need to update the kernel to take nla u32. > MST
On Mon, Dec 20, 2021 at 03:49:21AM +0000, Parav Pandit wrote: > > > > From: Michael S. Tsirkin <mst@redhat.com> > > Sent: Sunday, December 19, 2021 3:37 AM > > > > On Sat, Dec 18, 2021 at 01:53:01PM -0700, David Ahern wrote: > > > On 12/17/21 1:08 AM, Parav Pandit wrote: > > > > @@ -204,6 +217,8 @@ static void vdpa_opts_put(struct nlmsghdr *nlh, > > struct vdpa *vdpa) > > > > if (opts->present & VDPA_OPT_VDEV_MAC) > > > > mnl_attr_put(nlh, VDPA_ATTR_DEV_NET_CFG_MACADDR, > > > > sizeof(opts->mac), opts->mac); > > > > + if (opts->present & VDPA_OPT_VDEV_MTU) > > > > + mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MTU, > > opts->mtu); > > > > > > Why limit the MTU to a u16? Eric for example is working on "Big TCP" > > > where IPv6 can work with Jumbograms where mtu can be > 64k. > > > > > > https://datatracker.ietf.org/doc/html/rfc2675 > > > > Well it's 16 bit at the virtio level, though we can extend that of course. Making > > it match for now removes need for validation. > > -- > As Michael mentioned virtio specification limits the mtu to 64k-1. Hence 16-bit. > First we need to update the virtio spec to support > 64K mtu. > However, when/if (I don't know when) that happens, we need to make this also u32. > So may be we can make it u32 now, but still restrict the mtu value to 64k-1 in kernel, until the spec is updated. > > Let me know, if you think that's future proofing is better, I first need to update the kernel to take nla u32. > > > MST After consideration, this future proofing seems like a good thing to have.
> From: Michael S. Tsirkin <mst@redhat.com> > Sent: Monday, December 20, 2021 5:33 PM > > On Mon, Dec 20, 2021 at 03:49:21AM +0000, Parav Pandit wrote: > > > > > > > From: Michael S. Tsirkin <mst@redhat.com> > > > Sent: Sunday, December 19, 2021 3:37 AM > > > > > > On Sat, Dec 18, 2021 at 01:53:01PM -0700, David Ahern wrote: > > > > On 12/17/21 1:08 AM, Parav Pandit wrote: > > > > > @@ -204,6 +217,8 @@ static void vdpa_opts_put(struct nlmsghdr > > > > > *nlh, > > > struct vdpa *vdpa) > > > > > if (opts->present & VDPA_OPT_VDEV_MAC) > > > > > mnl_attr_put(nlh, VDPA_ATTR_DEV_NET_CFG_MACADDR, > > > > > sizeof(opts->mac), opts->mac); > > > > > + if (opts->present & VDPA_OPT_VDEV_MTU) > > > > > + mnl_attr_put_u16(nlh, > VDPA_ATTR_DEV_NET_CFG_MTU, > > > opts->mtu); > > > > > > > > Why limit the MTU to a u16? Eric for example is working on "Big TCP" > > > > where IPv6 can work with Jumbograms where mtu can be > 64k. > > > > > > > > https://datatracker.ietf.org/doc/html/rfc2675 > > > > > > Well it's 16 bit at the virtio level, though we can extend that of > > > course. Making it match for now removes need for validation. > > > -- > > As Michael mentioned virtio specification limits the mtu to 64k-1. Hence 16- > bit. > > First we need to update the virtio spec to support > 64K mtu. > > However, when/if (I don't know when) that happens, we need to make this > also u32. > > So may be we can make it u32 now, but still restrict the mtu value to 64k-1 in > kernel, until the spec is updated. > > > > Let me know, if you think that's future proofing is better, I first need to > update the kernel to take nla u32. > > > > > MST > > After consideration, this future proofing seems like a good thing to have. Ok. I will first get kernel change merged, after which will send v3 for iproute2.
On 12/20/21 12:11 PM, Parav Pandit wrote: >> After consideration, this future proofing seems like a good thing to have. > Ok. I will first get kernel change merged, after which will send v3 for iproute2. this set has been committed; not sure what happened to the notification from patchworks bot.
On Mon, Dec 20, 2021 at 02:11:44PM -0700, David Ahern wrote: > On 12/20/21 12:11 PM, Parav Pandit wrote: > >> After consideration, this future proofing seems like a good thing to have. > > Ok. I will first get kernel change merged, after which will send v3 for iproute2. > > this set has been committed; not sure what happened to the notification > from patchworks bot. OK in that case it's too late, so maybe let's worry about supporting extensions later when we actually need them, in particular when linux better supports mtu > 64k.
diff --git a/man/man8/vdpa-dev.8 b/man/man8/vdpa-dev.8 index 5c5ac469..aa21ae3a 100644 --- a/man/man8/vdpa-dev.8 +++ b/man/man8/vdpa-dev.8 @@ -32,6 +32,7 @@ vdpa-dev \- vdpa device configuration .B mgmtdev .I MGMTDEV .RI "[ mac " MACADDR " ]" +.RI "[ mtu " MTU " ]" .ti -8 .B vdpa dev del @@ -69,6 +70,10 @@ Name of the management device to use for device addition. - specifies the mac address for the new vdpa device. This is applicable only for the network type of vdpa device. This is optional. +.BI mtu " MTU" +- specifies the mtu for the new vdpa device. +This is applicable only for the network type of vdpa device. This is optional. + .SS vdpa dev del - Delete the vdpa device. .PP @@ -109,6 +114,11 @@ vdpa dev add name foo mgmtdev vdpa_sim_net mac 00:11:22:33:44:55 Add the vdpa device named foo on the management device vdpa_sim_net with mac address of 00:11:22:33:44:55. .RE .PP +vdpa dev add name foo mgmtdev vdpa_sim_net mac 00:11:22:33:44:55 mtu 9000 +.RS 4 +Add the vdpa device named foo on the management device vdpa_sim_net with mac address of 00:11:22:33:44:55 and mtu of 9000 bytes. +.RE +.PP vdpa dev del foo .RS 4 Delete the vdpa device named foo which was previously created. diff --git a/vdpa/vdpa.c b/vdpa/vdpa.c index 63d464d1..f048e470 100644 --- a/vdpa/vdpa.c +++ b/vdpa/vdpa.c @@ -22,6 +22,7 @@ #define VDPA_OPT_VDEV_NAME BIT(2) #define VDPA_OPT_VDEV_HANDLE BIT(3) #define VDPA_OPT_VDEV_MAC BIT(4) +#define VDPA_OPT_VDEV_MTU BIT(5) struct vdpa_opts { uint64_t present; /* flags of present items */ @@ -30,6 +31,7 @@ struct vdpa_opts { const char *vdev_name; unsigned int device_id; char mac[ETH_ALEN]; + uint16_t mtu; }; struct vdpa { @@ -154,6 +156,17 @@ static int vdpa_argv_mac(struct vdpa *vdpa, int argc, char **argv, char *mac) return 0; } +static int vdpa_argv_u16(struct vdpa *vdpa, int argc, char **argv, + uint16_t *result) +{ + if (argc <= 0 || *argv == NULL) { + fprintf(stderr, "number expected\n"); + return -EINVAL; + } + + return get_u16(result, *argv, 10); +} + struct vdpa_args_metadata { uint64_t o_flag; const char *err_msg; @@ -204,6 +217,8 @@ static void vdpa_opts_put(struct nlmsghdr *nlh, struct vdpa *vdpa) if (opts->present & VDPA_OPT_VDEV_MAC) mnl_attr_put(nlh, VDPA_ATTR_DEV_NET_CFG_MACADDR, sizeof(opts->mac), opts->mac); + if (opts->present & VDPA_OPT_VDEV_MTU) + mnl_attr_put_u16(nlh, VDPA_ATTR_DEV_NET_CFG_MTU, opts->mtu); } static int vdpa_argv_parse(struct vdpa *vdpa, int argc, char **argv, @@ -263,6 +278,15 @@ static int vdpa_argv_parse(struct vdpa *vdpa, int argc, char **argv, NEXT_ARG_FWD(); o_found |= VDPA_OPT_VDEV_MAC; + } else if ((strcmp(*argv, "mtu") == 0) && + (o_all & VDPA_OPT_VDEV_MTU)) { + NEXT_ARG_FWD(); + err = vdpa_argv_u16(vdpa, argc, argv, &opts->mtu); + if (err) + return err; + + NEXT_ARG_FWD(); + o_found |= VDPA_OPT_VDEV_MTU; } else { fprintf(stderr, "Unknown option \"%s\"\n", *argv); return -EINVAL; @@ -443,7 +467,7 @@ static int cmd_mgmtdev(struct vdpa *vdpa, int argc, char **argv) static void cmd_dev_help(void) { fprintf(stderr, "Usage: vdpa dev show [ DEV ]\n"); - fprintf(stderr, " vdpa dev add name NAME mgmtdev MANAGEMENTDEV [ mac MACADDR ]\n"); + fprintf(stderr, " vdpa dev add name NAME mgmtdev MANAGEMENTDEV [ mac MACADDR ] [ mtu MTU ]\n"); fprintf(stderr, " vdpa dev del DEV\n"); fprintf(stderr, "Usage: vdpa dev config COMMAND [ OPTIONS ]\n"); } @@ -533,7 +557,7 @@ static int cmd_dev_add(struct vdpa *vdpa, int argc, char **argv) NLM_F_REQUEST | NLM_F_ACK); err = vdpa_argv_parse_put(nlh, vdpa, argc, argv, VDPA_OPT_VDEV_MGMTDEV_HANDLE | VDPA_OPT_VDEV_NAME, - VDPA_OPT_VDEV_MAC); + VDPA_OPT_VDEV_MAC | VDPA_OPT_VDEV_MTU); if (err) return err;
Implement mtu setting for vdpa device. $ vdpa mgmtdev show vdpasim_net: supported_classes net Add the device with mac address and mtu: $ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:55 mtu 9000 In above command only mac address or only mtu can also be set. View the config after setting: $ vdpa dev config show bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000 Signed-off-by: Parav Pandit <parav@nvidia.com> --- changelog: v1->v2: - use get_u16 - use strcmp() instead of matches() - added man page --- man/man8/vdpa-dev.8 | 10 ++++++++++ vdpa/vdpa.c | 28 ++++++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 2 deletions(-)