Re: [PATCH v3 3/4] vdpa: show dev config as-is in "vdpa dev show" output

[Si-Wei Liu <si-wei.liu@oracle.com>]

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@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@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
>>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization