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 v6 4/8] vdpa: Enable user to set mac and mtu of vdpa device
Date: Tue, 26 Oct 2021 09:08:10 -0400 [thread overview]
Message-ID: <20211026090803-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <PH0PR12MB54810B7BE5EDCA0934F88612DC849@PH0PR12MB5481.namprd12.prod.outlook.com>
On Tue, Oct 26, 2021 at 01:03:41PM +0000, Parav Pandit wrote:
>
>
> > From: Stefano Garzarella <sgarzare@redhat.com>
> > Sent: Tuesday, October 26, 2021 6:31 PM
> >
> > On Tue, Oct 26, 2021 at 07:02:39AM +0300, Parav Pandit via Virtualization
> > wrote:
> > >$ 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>
> > >Acked-by: Jason Wang <jasowang@redhat.com>
> > >
> > >---
> > >changelog:
> > >v4->v5:
> > > - added comment for checking device capabilities
> > >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 | 38 ++++++++++++++++++++++++++--
> > > 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, 62 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 b5bd1a553256..6bbdc0ece707 100644
> > >--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > >+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > >@@ -2482,7 +2482,8 @@ static int event_handler(struct notifier_block *nb,
> > unsigned long event, void *p
> > > return ret;
> > > }
> > >
> > >-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
> > >973c56fb60b9..a1168a7fa8b8 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);
> > >@@ -480,9 +479,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;
> > >
> > >@@ -491,6 +496,26 @@ 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);
> > >+ }
> > >+
> > >+ /* Skip checking capability if user didn't prefer to configure any
> > >+ * device networking attributes. It is likely that user might have used
> > >+ * a device specific method to configure such attributes or using device
> > >+ * default attributes.
> > >+ */
> > >+ if ((config.mask & VDPA_DEV_NET_ATTRS_MASK) &&
> > >+ !netlink_capable(skb, CAP_NET_ADMIN))
> > >+ return -EPERM;
> > >+
> > > mutex_lock(&vdpa_dev_mutex);
> > > mdev = vdpa_mgmtdev_get_from_attr(info->attrs);
> > > if (IS_ERR(mdev)) {
> > >@@ -498,8 +523,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;
> > >@@ -835,6 +866,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
> > >fafb7202482c..bf9ddf743e2f 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;
> > >+ u64 mask;
> > >+};
> > >+
> > > /**
> > > * Corresponding file area for device memory mapping
> > > * @file: vma->vm_file for the mapping @@ -397,6 +407,7 @@ void
> > >vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
> > > * @dev_add: Add a vdpa device using alloc and register
> > > * @mdev: parent device to use for device addition
> > > * @name: name of the new vdpa device
> > >+ * @config: config attributes to apply to the device under creation
> > > * Driver need to add a new device using _vdpa_register_device()
> > > * after fully initializing the vdpa device. Driver must return 0
> > > * on success or appropriate error code.
> > >@@ -407,7 +418,8 @@ void vdpa_set_config(struct vdpa_device *dev,
> > unsigned int offset,
> > > * _vdpa_unregister_device().
> > > */
> > > struct vdpa_mgmtdev_ops {
> > >- int (*dev_add)(struct vdpa_mgmt_dev *mdev, const char *name);
> > >+ int (*dev_add)(struct vdpa_mgmt_dev *mdev, const char *name,
> > >+ const struct vdpa_dev_set_config *config);
> > > void (*dev_del)(struct vdpa_mgmt_dev *mdev, struct vdpa_device
> > *dev);
> > > };
> > >
> > >@@ -416,12 +428,15 @@ struct vdpa_mgmtdev_ops {
> > > * @device: Management parent device
> > > * @ops: operations supported by management device
> > > * @id_table: Pointer to device id table of supported ids
> > >+ * @config_attr_mask: bit mask of attributes of type enum vdpa_attr
> > >that
> > >+ * management devie support during dev_add callback
> >
> > s/devie/divice
>
> I ran checkpatch.pl and also codespell for extra check. None catch it. good catch :-)
> Do you use any tool or its your sharp eyes?
OK so v7?
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
next prev parent reply other threads:[~2021-10-26 13:08 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-26 4:02 [PATCH linux-next v6 0/8] vdpa: enable user to set mac, mtu Parav Pandit via Virtualization
2021-10-26 4:02 ` [PATCH linux-next v6 1/8] vdpa: Introduce and use vdpa device get, set config helpers Parav Pandit via Virtualization
2021-10-26 12:58 ` Stefano Garzarella
2021-10-26 4:02 ` [PATCH linux-next v6 2/8] vdpa: Introduce query of device config layout Parav Pandit via Virtualization
2021-10-26 4:02 ` [PATCH linux-next v6 3/8] vdpa: Use kernel coding style for structure comments Parav Pandit via Virtualization
2021-10-26 12:58 ` Stefano Garzarella
2021-10-26 4:02 ` [PATCH linux-next v6 4/8] vdpa: Enable user to set mac and mtu of vdpa device Parav Pandit via Virtualization
2021-10-26 13:01 ` Stefano Garzarella
2021-10-26 13:03 ` Parav Pandit via Virtualization
2021-10-26 13:08 ` Michael S. Tsirkin [this message]
2021-10-26 13:11 ` Parav Pandit via Virtualization
2021-10-26 13:17 ` Stefano Garzarella
2021-10-26 4:02 ` [PATCH linux-next v6 5/8] vdpa_sim_net: Enable user to set mac address and mtu Parav Pandit via Virtualization
2021-10-26 4:09 ` Jason Wang
2021-10-26 13:12 ` Stefano Garzarella
2021-10-26 4:02 ` [PATCH linux-next v6 6/8] vdpa/mlx5: Fix clearing of VIRTIO_NET_F_MAC feature bit Parav Pandit via Virtualization
2021-10-26 4:09 ` Jason Wang
2021-10-26 4:02 ` [PATCH linux-next v6 7/8] vdpa/mlx5: Support configuration of MAC Parav Pandit via Virtualization
2021-10-26 4:02 ` [PATCH linux-next v6 8/8] vdpa/mlx5: Forward only packets with allowed MAC address Parav Pandit via Virtualization
2021-10-26 4:13 ` Jason Wang
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=20211026090803-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.