> From: Jason Wang <jasowang@xxxxxxxxxx>
> Sent: Tuesday, June 22, 2021 1:13 PM
>
> 在 2021/6/17 上午3:11, Parav Pandit 写道:
> > $ vdpa dev add name bar mgmtdev vdpasim_net
> >
> > $ vdpa dev config set bar mac 00:11:22:33:44:55 mtu 9000
> >
> > $ vdpa dev config show
> > bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000 speed
> > 0 duplex 0
> >
> > $ vdpa dev config show -jp
> > {
> > "config": {
> > "bar": {
> > "mac": "00:11:22:33:44:55",
> > "link ": "up",
> > "link_announce ": false,
> > "mtu": 9000,
> > "speed": 0,
> > "duplex": 0
> > }
> > }
> > }
> >
> > Signed-off-by: Parav Pandit <parav@xxxxxxxxxx>
> > Reviewed-by: Eli Cohen <elic@xxxxxxxxxx>
> > ---
> > changelog:
> > v2->v3:
> > - using new setup_config callback to setup device params via mgmt tool
> > to avoid mixing with existing set_config().
> > ---
> > drivers/vdpa/vdpa.c | 91
> ++++++++++++++++++++++++++++++++++++++-
> > include/linux/vdpa.h | 18 ++++++++
> > include/uapi/linux/vdpa.h | 1 +
> > 3 files changed, 109 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index
> > 1295528244c3..40874bd92126 100644
> > --- a/drivers/vdpa/vdpa.c
> > +++ b/drivers/vdpa/vdpa.c
> > @@ -14,7 +14,6 @@
> > #include <uapi/linux/vdpa.h>
> > #include <net/genetlink.h>
> > #include <linux/mod_devicetable.h>
> > -#include <linux/virtio_net.h>
> > #include <linux/virtio_ids.h>
> >
> > static LIST_HEAD(mdev_head);
> > @@ -849,10 +848,94 @@ vdpa_nl_cmd_dev_config_get_dumpit(struct
> sk_buff *msg, struct netlink_callback *
> > return msg->len;
> > }
> >
> > +static int vdpa_dev_net_config_set(struct vdpa_device *vdev,
> > + struct sk_buff *skb, struct genl_info *info) {
> > + struct nlattr **nl_attrs = info->attrs;
> > + struct vdpa_dev_set_config config = {};
> > + const u8 *macaddr;
> > + int err;
> > +
> > + if (!netlink_capable(skb, CAP_NET_ADMIN))
> > + return -EPERM;
>
>
> Interesting, I wonder how cap would be used for other type of devices (e.g
> block).
>
>
> > +
> > + if (!vdev->config->setup_config)
> > + return -EOPNOTSUPP;
> > +
> > + if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
> > + macaddr =
> nla_data(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]);
> > + memcpy(config.net.mac, macaddr, sizeof(config.net.mac));
> > + config.net_mask.mac_valid = true;
> > + }
> > + if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]) {
> > + config.net.mtu =
> > +
> nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]);
> > + config.net_mask.mtu_valid = true;
> > + }
>
>
> Instead of doing memcpy and pass the whole config structure like this. I
> wonder if it's better to switch to use:
>
> vdev->config->setup_config(vdev, offsetof(struct virtio_net_config,
> mtu), &mtu, sizeof(mtu));
>
Well, we need a way to differentiate that the caller is management tool and not the vhost path.
Instead of passing some flag of the caller to setup_config(), a explicitly defined callback served better.
And secondly we need to return the error status. setup_config() cb is void. This is the minor one.
> Then there's no need for the vdpa_dev_set_config structure which will
> became structure virtio_net_config gradually.
>
> The setup_config() can fail if the offset is not at the boundary of a
> specific attribute.
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
