From: "Michael S. Tsirkin" <mst@redhat.com>
To: Parav Pandit <parav@nvidia.com>
Cc: Eli Cohen <elic@nvidia.com>,
"virtualization@lists.linux-foundation.org"
<virtualization@lists.linux-foundation.org>
Subject: Re: [PATCH linux-next v4 4/8] vdpa: Enable user to set mac and mtu of vdpa device
Date: Mon, 25 Oct 2021 04:08:24 -0400 [thread overview]
Message-ID: <20211025040202-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <PH0PR12MB54817652E2E49FD6A4F58F1BDC839@PH0PR12MB5481.namprd12.prod.outlook.com>
On Mon, Oct 25, 2021 at 07:06:42AM +0000, Parav Pandit wrote:
>
>
> > From: Jason Wang <jasowang@redhat.com>
> > 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@nvidia.com>
> > > Reviewed-by: Eli Cohen <elic@nvidia.com>
> > > ---
> > > 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@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
next prev parent reply other threads:[~2021-10-25 8:08 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-21 16:35 [PATCH linux-next v4 0/8] vdpa: enable user to set mac, mtu Parav Pandit via Virtualization
2021-10-21 16:35 ` [PATCH linux-next v4 1/8] vdpa: Introduce and use vdpa device get, set config helpers Parav Pandit via Virtualization
2021-10-25 6:03 ` Jason Wang
2021-10-25 11:24 ` Parav Pandit via Virtualization
2021-10-21 16:35 ` [PATCH linux-next v4 2/8] vdpa: Introduce query of device config layout Parav Pandit via Virtualization
2021-10-25 6:05 ` Jason Wang
2021-10-25 6:59 ` Parav Pandit via Virtualization
2021-10-25 8:10 ` Michael S. Tsirkin
2021-10-26 2:45 ` Jason Wang
2021-10-25 11:43 ` Parav Pandit via Virtualization
2021-10-21 16:35 ` [PATCH linux-next v4 3/8] vdpa: Use kernel coding style for structure comments Parav Pandit via Virtualization
2021-10-25 6:06 ` Jason Wang
2021-10-21 16:35 ` [PATCH linux-next v4 4/8] vdpa: Enable user to set mac and mtu of vdpa device Parav Pandit via Virtualization
2021-10-25 7:01 ` Jason Wang
2021-10-25 7:06 ` Parav Pandit via Virtualization
2021-10-25 8:08 ` Michael S. Tsirkin [this message]
2021-10-25 8:26 ` Parav Pandit via Virtualization
2021-10-26 2:40 ` Jason Wang
2021-10-21 16:35 ` [PATCH linux-next v4 5/8] vdpa_sim_net: Enable user to set mac address and mtu Parav Pandit via Virtualization
2021-10-25 7:02 ` Jason Wang
2021-10-25 7:11 ` Parav Pandit via Virtualization
2021-10-25 8:09 ` Michael S. Tsirkin
2021-10-25 8:10 ` Parav Pandit via Virtualization
2021-10-25 8:16 ` Michael S. Tsirkin
2021-10-21 16:35 ` [PATCH linux-next v4 6/8] vdpa/mlx5: Fix clearing of VIRTIO_NET_F_MAC feature bit Parav Pandit via Virtualization
2021-10-25 7:05 ` Jason Wang
2021-10-25 7:08 ` Parav Pandit via Virtualization
2021-10-26 2:42 ` Jason Wang
2021-10-21 16:35 ` [PATCH linux-next v4 7/8] vdpa/mlx5: Support configuration of MAC Parav Pandit via Virtualization
2021-10-25 7:07 ` Jason Wang
2021-10-21 16:35 ` [PATCH linux-next v4 8/8] vdpa/mlx5: Forward only packets with allowed MAC address Parav Pandit via Virtualization
2021-10-22 10:41 ` [PATCH linux-next v4 0/8] vdpa: enable user to set mac, mtu Michael S. Tsirkin
2021-10-22 15:07 ` Parav Pandit via Virtualization
2021-10-23 20:03 ` Michael S. Tsirkin
2021-10-23 21:15 ` Michael S. Tsirkin
2021-10-25 3:43 ` Parav Pandit via Virtualization
2021-10-25 7:27 ` Parav Pandit via Virtualization
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211025040202-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=elic@nvidia.com \
--cc=parav@nvidia.com \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).