| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <86529a16-6358-ad9e-7ae3-ea1580db015c@oracle.com>
Date: Thu, 20 Oct 2022 11:12:25 -0700
From: Si-Wei Liu <si-wei.liu@...cle.com>
To: Jason Wang <jasowang@...hat.com>, Parav Pandit <parav@...dia.com>
Cc: mst@...hat.com, virtualization@...ts.linux-foundation.org,
linux-kernel@...r.kernel.org
Subject: Re: [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@...cle.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@...cle.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
>>
Powered by blists - more mailing lists