On Mon, Oct 25, 2021 at 07:06:42AM +0000, Parav Pandit wrote:
>
>
> > From: Jason Wang <jasowang@xxxxxxxxxx>
> > Sent: Monday, October 25, 2021 12:31 PM
> >
> > 在 2021/10/22 上午12:35, Parav Pandit 写道:
> > > $ vdpa dev add name bar mgmtdev vdpasim_net 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
> > >
> > > $ vdpa dev config show -jp
> > > {
> > > "config": {
> > > "bar": {
> > > "mac": "00:11:22:33:44:55",
> > > "link ": "up",
> > > "link_announce ": false,
> > > "mtu": 9000,
> > > }
> > > }
> > > }
> > >
> > > Signed-off-by: Parav Pandit <parav@xxxxxxxxxx>
> > > Reviewed-by: Eli Cohen <elic@xxxxxxxxxx>
> > > ---
> > > changelog:
> > > v3->v4:
> > > - provide config attributes during device addition time
> > > ---
> > > drivers/vdpa/ifcvf/ifcvf_main.c | 3 ++-
> > > drivers/vdpa/mlx5/net/mlx5_vnet.c | 3 ++-
> > > drivers/vdpa/vdpa.c | 33 ++++++++++++++++++++++++++--
> > > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 3 ++-
> > > drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 3 ++-
> > > drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++-
> > > include/linux/vdpa.h | 17 +++++++++++++-
> > > 7 files changed, 57 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c
> > > b/drivers/vdpa/ifcvf/ifcvf_main.c index dcd648e1f7e7..6dc75ca70b37
> > > 100644
> > > --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> > > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> > > @@ -499,7 +499,8 @@ static u32 get_dev_type(struct pci_dev *pdev)
> > > return dev_type;
> > > }
> > >
> > > -static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char
> > > *name)
> > > +static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char
> > *name,
> > > + const struct vdpa_dev_set_config *config)
> > > {
> > > struct ifcvf_vdpa_mgmt_dev *ifcvf_mgmt_dev;
> > > struct ifcvf_adapter *adapter;
> > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > index bd56de7484dc..ca05f69054b6 100644
> > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > @@ -2404,7 +2404,8 @@ struct mlx5_vdpa_mgmtdev {
> > > struct mlx5_vdpa_net *ndev;
> > > };
> > >
> > > -static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char
> > > *name)
> > > +static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char
> > *name,
> > > + const struct vdpa_dev_set_config *add_config)
> > > {
> > > struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct
> > mlx5_vdpa_mgmtdev, mgtdev);
> > > struct virtio_net_config *config;
> > > diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index
> > > a50a6aa1cfc4..daa34a61c898 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);
> > > @@ -472,9 +471,15 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff
> > *msg, struct netlink_callback *cb)
> > > return msg->len;
> > > }
> > >
> > > +#define VDPA_DEV_NET_ATTRS_MASK ((1 <<
> > VDPA_ATTR_DEV_NET_CFG_MACADDR) | \
> > > + (1 << VDPA_ATTR_DEV_NET_CFG_MTU))
> > > +
> > > static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct
> > genl_info *info)
> > > {
> > > + struct vdpa_dev_set_config config = {};
> > > + struct nlattr **nl_attrs = info->attrs;
> > > struct vdpa_mgmt_dev *mdev;
> > > + const u8 *macaddr;
> > > const char *name;
> > > int err = 0;
> > >
> > > @@ -483,6 +488,21 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct
> > > sk_buff *skb, struct genl_info *i
> > >
> > > name = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);
> > >
> > > + 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.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR);
> > > + }
> > > + if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]) {
> > > + config.net.mtu =
> > > +
> > nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]);
> > > + config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MTU);
> > > + }
> > > +
> > > + if ((config.mask & VDPA_DEV_NET_ATTRS_MASK) &&
> > > + !netlink_capable(skb, CAP_NET_ADMIN))
> > > + return -EPERM;
> >
> >
> > This deserves a independent patch. And do we need backport it to stable?
> This patch is adding the ability to configure mac and mtu. Hence, it is in this patch.
> It cannot be a different patch after this.
>
> >
> > Another question is that, do need the cap if not attrs were specified?
> I am not sure. A user is adding the vpda device anchored on the bus.
> We likely need different capability check than the NET_ADMIN.
It depends on what will the user be able to do then.
Inject packets? Affect RX routing? Use up networking resources?
NET_ADMIN is a safe choice but we didn't check any capability
in the past so it seems reasonable to keep not
checking it for the time being unless we see an actual
security issue.
> >
> >
> > > +
> > > mutex_lock(&vdpa_dev_mutex);
> > > mdev = vdpa_mgmtdev_get_from_attr(info->attrs);
> > > if (IS_ERR(mdev)) {
> > > @@ -490,8 +510,14 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct
> > sk_buff *skb, struct genl_info *i
> > > err = PTR_ERR(mdev);
> > > goto err;
> > > }
> > > + if ((config.mask & mdev->config_attr_mask) != config.mask) {
> > > + NL_SET_ERR_MSG_MOD(info->extack,
> > > + "All provided attributes are not supported");
> > > + err = -EOPNOTSUPP;
> > > + goto err;
> > > + }
> > >
> > > - err = mdev->ops->dev_add(mdev, name);
> > > + err = mdev->ops->dev_add(mdev, name, &config);
> > > err:
> > > mutex_unlock(&vdpa_dev_mutex);
> > > return err;
> > > @@ -822,6 +848,9 @@ static const struct nla_policy
> > vdpa_nl_policy[VDPA_ATTR_MAX + 1] = {
> > > [VDPA_ATTR_MGMTDEV_BUS_NAME] = { .type = NLA_NUL_STRING },
> > > [VDPA_ATTR_MGMTDEV_DEV_NAME] = { .type = NLA_STRING },
> > > [VDPA_ATTR_DEV_NAME] = { .type = NLA_STRING },
> > > + [VDPA_ATTR_DEV_NET_CFG_MACADDR] = NLA_POLICY_ETH_ADDR,
> > > + /* virtio spec 1.1 section 5.1.4.1 for valid MTU range */
> > > + [VDPA_ATTR_DEV_NET_CFG_MTU] = NLA_POLICY_MIN(NLA_U16, 68),
> > > };
> > >
> > > static const struct genl_ops vdpa_nl_ops[] = { diff --git
> > > a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> > > b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> > > index a790903f243e..42d401d43911 100644
> > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> > > @@ -248,7 +248,8 @@ static struct device vdpasim_blk_mgmtdev = {
> > > .release = vdpasim_blk_mgmtdev_release,
> > > };
> > >
> > > -static int vdpasim_blk_dev_add(struct vdpa_mgmt_dev *mdev, const char
> > > *name)
> > > +static int vdpasim_blk_dev_add(struct vdpa_mgmt_dev *mdev, const char
> > *name,
> > > + const struct vdpa_dev_set_config *config)
> > > {
> > > struct vdpasim_dev_attr dev_attr = {};
> > > struct vdpasim *simdev;
> > > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> > > b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> > > index a1ab6163f7d1..d681e423e64f 100644
> > > --- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> > > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> > > @@ -126,7 +126,8 @@ static struct device vdpasim_net_mgmtdev = {
> > > .release = vdpasim_net_mgmtdev_release,
> > > };
> > >
> > > -static int vdpasim_net_dev_add(struct vdpa_mgmt_dev *mdev, const char
> > > *name)
> > > +static int vdpasim_net_dev_add(struct vdpa_mgmt_dev *mdev, const char
> > *name,
> > > + const struct vdpa_dev_set_config *config)
> > > {
> > > struct vdpasim_dev_attr dev_attr = {};
> > > struct vdpasim *simdev;
> > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > index 841667a896dd..c9204c62f339 100644
> > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > @@ -1503,7 +1503,8 @@ static int vduse_dev_init_vdpa(struct vduse_dev
> > *dev, const char *name)
> > > return 0;
> > > }
> > >
> > > -static int vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name)
> > > +static int vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
> > > + const struct vdpa_dev_set_config *config)
> > > {
> > > struct vduse_dev *dev;
> > > int ret;
> > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h index
> > > 111153c9ee71..315da5f918dc 100644
> > > --- a/include/linux/vdpa.h
> > > +++ b/include/linux/vdpa.h
> > > @@ -6,6 +6,8 @@
> > > #include <linux/device.h>
> > > #include <linux/interrupt.h>
> > > #include <linux/vhost_iotlb.h>
> > > +#include <linux/virtio_net.h>
> > > +#include <linux/if_ether.h>
> > >
> > > /**
> > > * struct vdpa_calllback - vDPA callback definition.
> > > @@ -93,6 +95,14 @@ struct vdpa_iova_range {
> > > u64 last;
> > > };
> > >
> > > +struct vdpa_dev_set_config {
> > > + struct {
> > > + u8 mac[ETH_ALEN];
> > > + u16 mtu;
> > > + } net;
> >
> >
> > If we want to add block device, I guess we need a union as a container?
> Right. When that occurs in future, there will be union to contain both.
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
