Jason Wang
2022-Oct-20 05:25 UTC
[PATCH v2 3/4] vdpa: show dev config as-is in "vdpa dev show" output
On Wed, Oct 19, 2022 at 8:56 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 from > which vdpa device was created as-is. The "vdpa dev show" command > seems to be the right vehicle for that. It is unlike the "vdpa dev > config show" command output that usually goes with the live value > in the device config space, which is not quite reliable subject to > the dynamics of feature negotiation and possible change in device > 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 > virtio_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, > "virtio_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 566c1c6..91eca6d 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_cfgattrs_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 device_id) > +{ > + struct vdpa_dev_set_config *cfg = &vdev->vdev_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;This makes me think if we can reuse the virtio_net_config structure other than duplicate it slowly with a dedicated nested structure inside vdpa_dev_set_config then we can reuse the vdpa_dev_net_config_fill(). Thanks> + 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; > + > + 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_cfgattrs_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-20 18:12 UTC
[PATCH v2 3/4] vdpa: show dev config as-is in "vdpa dev show" output
On 10/19/2022 10:25 PM, Jason Wang wrote:> On Wed, Oct 19, 2022 at 8:56 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 from >> which vdpa device was created as-is. The "vdpa dev show" command >> seems to be the right vehicle for that. It is unlike the "vdpa dev >> config show" command output that usually goes with the live value >> in the device config space, which is not quite reliable subject to >> the dynamics of feature negotiation and possible change in device >> 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 >> virtio_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, >> "virtio_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 566c1c6..91eca6d 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_cfgattrs_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 device_id) >> +{ >> + struct vdpa_dev_set_config *cfg = &vdev->vdev_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; > This makes me think if we can reuse the virtio_net_config structure > other than duplicate it slowly with a dedicated nested structure > inside vdpa_dev_set_config then we can reuse the > vdpa_dev_net_config_fill().Adding Parav. I think for now the struct vdpa_dev_set_config has just a few fields, so it's not very obvious. But from what I understand, the vdpa_dev_set_config struct is designed to be built around vdpa configurables, without getting it limited by what's exposed by the virtio device config structure, such as virtio_net_config. For instance, there could be possibility for vdpa user to specify the size of MAC unicast or multicast address table, which is not defined anywhere in the virtio_net_config. I think it's important to match such configuration (which may not even be defined in spec) for src&dst vdpa devices involving the live migration. -Siwei> > Thanks > >> + 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; >> + >> + 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_cfgattrs_fill(vdev, msg, device_id); >> + if (err) >> + goto msg_err; >> + >> genlmsg_end(msg, hdr); >> return 0; >> >> -- >> 1.8.3.1 >>