Jason Wang
2022-Oct-24 08:40 UTC
[PATCH v3 3/4] vdpa: show dev config as-is in "vdpa dev show" output
On Sat, Oct 22, 2022 at 7:49 AM Si-Wei Liu <si-wei.liu at oracle.com> wrote:> > Live migration of vdpa would typically require re-instate vdpa > device with an idential set of configs on the destination node, > same way as how source node created the device in the first > place. In order to save orchestration software from memorizing > and keeping track of vdpa config, it will be helpful if the vdpa > tool provides the aids for exporting the initial configs as-is, > the way how vdpa device was created. The "vdpa dev show" command > seems to be the right vehicle for that. It is unlike the "vdpa dev > config show" command output which usually goes with the live value > in the device config space, and is not quite reliable subject to > the dynamics of feature negotiation or possible change by the > driver to the config space. > > Examples: > > 1) Create vDPA by default without any config attribute > > $ vdpa dev add mgmtdev pci/0000:41:04.2 name vdpa0 > $ vdpa dev show vdpa0 > vdpa0: type network mgmtdev pci/0000:41:04.2 vendor_id 5555 max_vqs 9 max_vq_size 256 > $ vdpa dev -jp show vdpa0 > { > "dev": { > "vdpa0": { > "type": "network", > "mgmtdev": "pci/0000:41:04.2", > "vendor_id": 5555, > "max_vqs": 9, > "max_vq_size": 256, > } > } > } > > 2) Create vDPA with config attribute(s) specified > > $ vdpa dev add mgmtdev pci/0000:41:04.2 name vdpa0 \ > mac e4:11:c6:d3:45:f0 max_vq_pairs 4 > $ vdpa dev show > vdpa0: type network mgmtdev pci/0000:41:04.2 vendor_id 5555 max_vqs 9 max_vq_size 256 > initial_config: mac e4:11:c6:d3:45:f0 max_vq_pairs 4 > $ vdpa dev -jp show > { > "dev": { > "vdpa0": { > "type": "network", > "mgmtdev": "pci/0000:41:04.2", > "vendor_id": 5555, > "max_vqs": 9, > "max_vq_size": 256, > "initial_config": { > "mac": "e4:11:c6:d3:45:f0", > "max_vq_pairs": 4 > } > } > } > } > > Signed-off-by: Si-Wei Liu <si-wei.liu at oracle.com> > --- > drivers/vdpa/vdpa.c | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > index bebded6..bfb8f54 100644 > --- a/drivers/vdpa/vdpa.c > +++ b/drivers/vdpa/vdpa.c > @@ -677,6 +677,41 @@ static int vdpa_nl_cmd_dev_del_set_doit(struct sk_buff *skb, struct genl_info *i > } > > static int > +vdpa_dev_initcfg_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 device_id) > +{ > + struct vdpa_dev_set_config *cfg = &vdev->init_cfg; > + int err = -EMSGSIZE; > + > + if (!cfg->mask) > + return 0; > + > + switch (device_id) { > + case VIRTIO_ID_NET: > + if ((cfg->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR)) != 0 && > + nla_put(msg, VDPA_ATTR_DEV_NET_CFG_MACADDR, > + sizeof(cfg->net.mac), cfg->net.mac)) > + return err; > + if ((cfg->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU)) != 0 && > + nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, cfg->net.mtu)) > + return err; > + if ((cfg->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP)) != 0 && > + nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, > + cfg->net.max_vq_pairs)) > + return err; > + break; > + default: > + break; > + } > + > + if ((cfg->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) != 0 && > + nla_put_u64_64bit(msg, VDPA_ATTR_DEV_FEATURES, > + cfg->device_features, VDPA_ATTR_PAD)) > + return err;A question: If any of those above attributes were not provisioned, should we show the ones that are inherited from the parent? Thanks> + > + return 0; > +} > + > +static int > vdpa_dev_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 portid, u32 seq, > int flags, struct netlink_ext_ack *extack) > { > @@ -715,6 +750,10 @@ static int vdpa_nl_cmd_dev_del_set_doit(struct sk_buff *skb, struct genl_info *i > if (nla_put_u16(msg, VDPA_ATTR_DEV_MIN_VQ_SIZE, min_vq_size)) > goto msg_err; > > + err = vdpa_dev_initcfg_fill(vdev, msg, device_id); > + if (err) > + goto msg_err; > + > genlmsg_end(msg, hdr); > return 0; > > -- > 1.8.3.1 >
Si-Wei Liu
2022-Oct-24 19:14 UTC
[PATCH v3 3/4] vdpa: show dev config as-is in "vdpa dev show" output
On 10/24/2022 1:40 AM, Jason Wang wrote:> On Sat, Oct 22, 2022 at 7:49 AM Si-Wei Liu <si-wei.liu at oracle.com> wrote: >> Live migration of vdpa would typically require re-instate vdpa >> device with an idential set of configs on the destination node, >> same way as how source node created the device in the first >> place. In order to save orchestration software from memorizing >> and keeping track of vdpa config, it will be helpful if the vdpa >> tool provides the aids for exporting the initial configs as-is, >> the way how vdpa device was created. The "vdpa dev show" command >> seems to be the right vehicle for that. It is unlike the "vdpa dev >> config show" command output which usually goes with the live value >> in the device config space, and is not quite reliable subject to >> the dynamics of feature negotiation or possible change by the >> driver to the config space. >> >> Examples: >> >> 1) Create vDPA by default without any config attribute >> >> $ vdpa dev add mgmtdev pci/0000:41:04.2 name vdpa0 >> $ vdpa dev show vdpa0 >> vdpa0: type network mgmtdev pci/0000:41:04.2 vendor_id 5555 max_vqs 9 max_vq_size 256 >> $ vdpa dev -jp show vdpa0 >> { >> "dev": { >> "vdpa0": { >> "type": "network", >> "mgmtdev": "pci/0000:41:04.2", >> "vendor_id": 5555, >> "max_vqs": 9, >> "max_vq_size": 256, >> } >> } >> } >> >> 2) Create vDPA with config attribute(s) specified >> >> $ vdpa dev add mgmtdev pci/0000:41:04.2 name vdpa0 \ >> mac e4:11:c6:d3:45:f0 max_vq_pairs 4 >> $ vdpa dev show >> vdpa0: type network mgmtdev pci/0000:41:04.2 vendor_id 5555 max_vqs 9 max_vq_size 256 >> initial_config: mac e4:11:c6:d3:45:f0 max_vq_pairs 4 >> $ vdpa dev -jp show >> { >> "dev": { >> "vdpa0": { >> "type": "network", >> "mgmtdev": "pci/0000:41:04.2", >> "vendor_id": 5555, >> "max_vqs": 9, >> "max_vq_size": 256, >> "initial_config": { >> "mac": "e4:11:c6:d3:45:f0", >> "max_vq_pairs": 4 >> } >> } >> } >> } >> >> Signed-off-by: Si-Wei Liu <si-wei.liu at oracle.com> >> --- >> drivers/vdpa/vdpa.c | 39 +++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 39 insertions(+) >> >> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c >> index bebded6..bfb8f54 100644 >> --- a/drivers/vdpa/vdpa.c >> +++ b/drivers/vdpa/vdpa.c >> @@ -677,6 +677,41 @@ static int vdpa_nl_cmd_dev_del_set_doit(struct sk_buff *skb, struct genl_info *i >> } >> >> static int >> +vdpa_dev_initcfg_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 device_id) >> +{ >> + struct vdpa_dev_set_config *cfg = &vdev->init_cfg; >> + int err = -EMSGSIZE; >> + >> + if (!cfg->mask) >> + return 0; >> + >> + switch (device_id) { >> + case VIRTIO_ID_NET: >> + if ((cfg->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR)) != 0 && >> + nla_put(msg, VDPA_ATTR_DEV_NET_CFG_MACADDR, >> + sizeof(cfg->net.mac), cfg->net.mac)) >> + return err; >> + if ((cfg->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MTU)) != 0 && >> + nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, cfg->net.mtu)) >> + return err; >> + if ((cfg->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP)) != 0 && >> + nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, >> + cfg->net.max_vq_pairs)) >> + return err; >> + break; >> + default: >> + break; >> + } >> + >> + if ((cfg->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) != 0 && >> + nla_put_u64_64bit(msg, VDPA_ATTR_DEV_FEATURES, >> + cfg->device_features, VDPA_ATTR_PAD)) >> + return err; > A question: If any of those above attributes were not provisioned, > should we show the ones that are inherited from the parent?A simple answer would be yes, but the long answer is that I am not sure if there's any for the moment - there's no? default value for mtu, mac, and max_vqp that can be inherited from the parent (max_vqp by default being 1 is spec defined, not something inherited from the parent). And the device_features if inherited is displayed at 'vdpa dev config show' output. Can you remind me of a good example for inherited value that we may want to show here? Thanks, -Siwei> > Thanks > >> + >> + return 0; >> +} >> + >> +static int >> vdpa_dev_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 portid, u32 seq, >> int flags, struct netlink_ext_ack *extack) >> { >> @@ -715,6 +750,10 @@ static int vdpa_nl_cmd_dev_del_set_doit(struct sk_buff *skb, struct genl_info *i >> if (nla_put_u16(msg, VDPA_ATTR_DEV_MIN_VQ_SIZE, min_vq_size)) >> goto msg_err; >> >> + err = vdpa_dev_initcfg_fill(vdev, msg, device_id); >> + if (err) >> + goto msg_err; >> + >> genlmsg_end(msg, hdr); >> return 0; >> >> -- >> 1.8.3.1 >>