Re: [External] Re: [PATCH 0/7] Introduce vdpa management tool

[Jason Wang]

On 2020/11/30 下午3:07, Yongji Xie wrote:

Thanks for adding me, Jason!

Now I'm working on a v2 patchset for VDUSE (vDPA Device in Userspace)
[1]. This tool is very useful for the vduse device. So I'm considering
integrating this into my v2 patchset. But there is one problem：

In this tool, vdpa device config action and enable action are combined
into one netlink msg: VDPA_CMD_DEV_NEW. But in vduse case, it needs to
be splitted because a chardev should be created and opened by a
userspace process before we enable the vdpa device (call
vdpa_register_device()).

So I'd like to know whether it's possible (or have some plans) to add
two new netlink msgs something like: VDPA_CMD_DEV_ENABLE and
VDPA_CMD_DEV_DISABLE to make the config path more flexible.


Actually, we've discussed such intermediate step in some early
discussion. It looks to me VDUSE could be one of the users of this.

Or I wonder whether we can switch to use anonymous inode(fd) for VDUSE
then fetching it via an VDUSE_GET_DEVICE_FD ioctl?


Yes, we can. Actually the current implementation in VDUSE is like
this.  But seems like this is still a intermediate step. The fd should
be binded to a name or something else which need to be configured
before.



The name could be specified via the netlink. It looks to me the real 
issue is that until the device is connected with a userspace, it can't 
be used. So we also need to fail the enabling if it doesn't opened.


Thanks




Thanks,
Yongji



___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 000/141] Fix fall-through warnings for Clang

[Martin K. Petersen]

Gustavo,

> This series aims to fix almost all remaining fall-through warnings in
> order to enable -Wimplicit-fallthrough for Clang.

Applied 20-22,54,120-124 to 5.11/scsi-staging, thanks.

-- 
Martin K. Petersen  Oracle Linux Engineering
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 12/17] vdpa_sim: add get_config callback in vdpasim_dev_attr

[Jason Wang]

On 2020/11/30 下午10:14, Stefano Garzarella wrote:

On Mon, Nov 30, 2020 at 11:25:31AM +0800, Jason Wang wrote:


On 2020/11/26 下午10:49, Stefano Garzarella wrote:

The get_config callback can be used by the device to fill the
config structure.
The callback will be invoked in vdpasim_get_config() before copying
bytes into caller buffer.

Move vDPA-net config updates from vdpasim_set_features() in the
new vdpasim_net_get_config() callback.

Signed-off-by: Stefano Garzarella 
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 33 +++-
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c 
b/drivers/vdpa/vdpa_sim/vdpa_sim.c

index c07ddf6af720..8b87ce0485b6 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -58,6 +58,8 @@ struct vdpasim_virtqueue {
 #define VDPASIM_NET_FEATURES    (VDPASIM_FEATURES | \
  (1ULL << VIRTIO_NET_F_MAC))
+struct vdpasim;
+
 struct vdpasim_dev_attr {
 u64 supported_features;
 size_t config_size;
@@ -65,6 +67,7 @@ struct vdpasim_dev_attr {
 u32 id;
 work_func_t work_fn;
+    void (*get_config)(struct vdpasim *vdpasim, void *config);
 };
 /* State of each vdpasim device */
@@ -520,8 +523,6 @@ static u64 vdpasim_get_features(struct 
vdpa_device *vdpa)
 static int vdpasim_set_features(struct vdpa_device *vdpa, u64 
features)

 {
 struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
-    struct virtio_net_config *config =
-    (struct virtio_net_config *)vdpasim->config;
 /* DMA mapping must be done by driver */
 if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
@@ -529,15 +530,6 @@ static int vdpasim_set_features(struct 
vdpa_device *vdpa, u64 features)
 vdpasim->features = features & 
vdpasim->dev_attr.supported_features;
-    /* We generally only know whether guest is using the legacy 
interface
- * here, so generally that's the earliest we can set config 
fields.

- * Note: We actually require VIRTIO_F_ACCESS_PLATFORM above which
- * implies VIRTIO_F_VERSION_1, but let's not try to be clever 
here.

- */
-
-    config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
-    config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
-    memcpy(config->mac, macaddr_buf, ETH_ALEN);
 return 0;
 }
@@ -593,8 +585,12 @@ static void vdpasim_get_config(struct 
vdpa_device *vdpa, unsigned int offset,

 {
 struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
-    if (offset + len < vdpasim->dev_attr.config_size)
-    memcpy(buf, vdpasim->config + offset, len);
+    if (offset + len > vdpasim->dev_attr.config_size)
+    return;
+
+    vdpasim->dev_attr.get_config(vdpasim, vdpasim->config);
+
+    memcpy(buf, vdpasim->config + offset, len);
 }



I wonder how much value we can get from vdpasim->config consider 
we've already had get_config() method.


Is it possible to copy to the buffer directly here?


I had thought of eliminating it too, but then I wanted to do something 
similar to what we do in QEMU (hw/virtio/virtio.c), leaving in the 
simulator core the buffer, the memory copy (handling offset and len), 
and the boundary checks.


In this way each device should simply fill the entire configuration 
and we can avoid code duplication.


Storing the configuration in the core may also be useful if some 
device needs to support config writes.



I think in that way we need should provide config_write().




Do you think it makes sense, or is it better to move everything in the 
device?



I prefer to do that in the device but it's also fine keep what the patch 
has done.


Thanks




Thanks,
Stefano



___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper

[Sasha Levin]

On Mon, Nov 30, 2020 at 09:29:02PM +0100, Paolo Bonzini wrote:

On 30/11/20 20:44, Mike Christie wrote:

I have never seen a public/open-source vhost-scsi testsuite.

For patch 23 (the one that adds the lun reset support which is built on
patch 22), we can't add it to stable right now if you wanted to, because
it has a bug in it. Michael T, sent the fix:

https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/commit/?h=linux-next=b4fffc177fad3c99ee049611a508ca9561bb6871

to Linus today.


Ok, so at least it was only a close call and anyway not for something 
that most people would be running on their machines.  But it still 
seems to me that the state of CI in Linux is abysmal compared to what 
is needed to arbitrarily(*) pick up patches and commit them to 
"stable" trees.


Paolo

(*) A ML bot is an arbitrary choice as far as we are concerned since 
we cannot know how it makes a decision.


The choice of patches is "arbitrary", but the decision is human. The
patches are reviewed coming out of the AI, sent to public mailing
list(s) for review, followed by 2 reminders asking for reviews.

The process for AUTOSEL patches generally takes longer than most patches
do for upstream.

It's quite easy to NAK a patch too, just reply saying "no" and it'll be
dropped (just like this patch was dropped right after your first reply)
so the burden on maintainers is minimal.

--
Thanks,
Sasha
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper

[Paolo Bonzini]

On 30/11/20 20:44, Mike Christie wrote:

I have never seen a public/open-source vhost-scsi testsuite.

For patch 23 (the one that adds the lun reset support which is built on
patch 22), we can't add it to stable right now if you wanted to, because
it has a bug in it. Michael T, sent the fix:

https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/commit/?h=linux-next=b4fffc177fad3c99ee049611a508ca9561bb6871

to Linus today.


Ok, so at least it was only a close call and anyway not for something 
that most people would be running on their machines.  But it still seems 
to me that the state of CI in Linux is abysmal compared to what is 
needed to arbitrarily(*) pick up patches and commit them to "stable" trees.


Paolo

(*) A ML bot is an arbitrary choice as far as we are concerned since we 
cannot know how it makes a decision.


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper

[Paolo Bonzini]

On 30/11/20 18:38, Sasha Levin wrote:
I am not aware of any public CI being done _at all_ done on 
vhost-scsi, by CKI or everyone else.  So autoselection should be done 
only on subsystems that have very high coverage in CI.


Where can I find a testsuite for virtio/vhost? I see one for KVM, but
where is the one that the maintainers of virtio/vhost run on patches
that come in?


I don't know of any, especially for vhost-scsi.  MikeC?

Paolo

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper

[Sasha Levin]

On Mon, Nov 30, 2020 at 09:33:46AM +0100, Paolo Bonzini wrote:

On 29/11/20 22:06, Sasha Levin wrote:

Plus all the testing we have for the stable trees, yes. It goes beyond
just compiling at this point.

Your very own co-workers (https://cki-project.org/) are pushing hard on
this effort around stable kernel testing, and statements like these
aren't helping anyone.


I am not aware of any public CI being done _at all_ done on 
vhost-scsi, by CKI or everyone else.  So autoselection should be done 
only on subsystems that have very high coverage in CI.


Where can I find a testsuite for virtio/vhost? I see one for KVM, but
where is the one that the maintainers of virtio/vhost run on patches
that come in?

--
Thanks,
Sasha
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper

[Sasha Levin]

On Mon, Nov 30, 2020 at 03:00:13PM +0100, Paolo Bonzini wrote:

On 30/11/20 14:57, Greg KH wrote:

Every patch should be "fixing a real issue"---even a new feature.  But the
larger the patch, the more the submitters and maintainers should be trusted
rather than a bot.  The line between feature and bugfix_sometimes_  is
blurry, I would say that in this case it's not, and it makes me question how
the bot decided that this patch would be acceptable for stable (which AFAIK
is not something that can be answered).

I thought that earlier Sasha said that this patch was needed as a
prerequisite patch for a later fix, right?  If not, sorry, I've lost the
train of thought in this thread...


Yeah---sorry I am replying to 22/33 but referring to 23/33, which is 
the one that in my opinion should not be blindly accepted for stable 
kernels without the agreement of the submitter or maintainer.


But it's not "blindly", right? I've sent this review mail over a week
ago, and if it goes into the queue there will be at least two more
emails going out to the author/maintainers.

During all this time it gets tested by various entities who do things
that go beyond simple boot testing.

I'd argue that the backports we push in the stable tree sometimes get
tested and reviewed better than the commits that land upstream.

--
Thanks,
Sasha
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] vdpa/mlx5: Use random MAC for the vdpa net instance

[Michael S. Tsirkin]

On Mon, Nov 30, 2020 at 06:41:45PM +0800, Cindy Lu wrote:
> On Mon, Nov 30, 2020 at 5:33 PM Michael S. Tsirkin  wrote:
> >
> > On Mon, Nov 30, 2020 at 11:27:59AM +0200, Eli Cohen wrote:
> > > On Mon, Nov 30, 2020 at 04:00:51AM -0500, Michael S. Tsirkin wrote:
> > > > On Mon, Nov 30, 2020 at 08:27:46AM +0200, Eli Cohen wrote:
> > > > > On Sun, Nov 29, 2020 at 03:08:22PM -0500, Michael S. Tsirkin wrote:
> > > > > > On Sun, Nov 29, 2020 at 08:43:51AM +0200, Eli Cohen wrote:
> > > > > > > We should not try to use the VF MAC address as that is used by the
> > > > > > > regular (e.g. mlx5_core) NIC implementation. Instead, use a random
> > > > > > > generated MAC address.
> > > > > > >
> > > > > > > Suggested by: Cindy Lu 
> > > > > > > Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported 
> > > > > > > mlx5 devices")
> > > > > > > Signed-off-by: Eli Cohen 
> > > > > >
> > > > > > I didn't realise it's possible to use VF in two ways
> > > > > > with and without vdpa.
> > > > >
> > > > > Using a VF you can create quite a few resources, e.g. send queues
> > > > > recieve queues, virtio_net queues etc. So you can possibly create
> > > > > several instances of vdpa net devices and nic net devices.
> > > > >
> > > > > > Could you include a bit more description on the failure
> > > > > > mode?
> > > > >
> > > > > Well, using the MAC address of the nic vport is wrong since that is 
> > > > > the
> > > > > MAC of the regular NIC implementation of mlx5_core.
> > > >
> > > > Right but ATM it doesn't coexist with vdpa so what's the problem?
> > > >
> > >
> > > This call is wrong:  mlx5_query_nic_vport_mac_address()
> > >
> > > > > > Is switching to a random mac for such an unusual
> > > > > > configuration really justified?
> > > > >
> > > > > Since I can't use the NIC's MAC address, I have two options:
> > > > > 1. To get the MAC address as was chosen by the user administering the
> > > > >NIC. This should invoke the set_config callback. Unfortunately this
> > > > >is not implemented yet.
> > > > >
> > > > > 2. Use a random MAC address. This is OK since if (1) is implemented it
> > > > >can always override this random configuration.
> > > > >
> > > > > > It looks like changing a MAC could break some guests,
> > > > > > can it not?
> > > > > >
> > > > >
> > > > > No, it will not. The current version of mlx5 VDPA does not allow 
> > > > > regular
> > > > > NIC driver and VDPA to co-exist. I have patches ready that enable that
> > > > > from steering point of view. I will post them here once other patches 
> > > > > on
> > > > > which they depend will be merged.
> > > > >
> > > > > https://patchwork.ozlabs.org/project/netdev/patch/20201120230339.651609-12-sae...@nvidia.com/
> > > >
> > > > Could you be more explicit on the following points:
> > > > - which configuration is broken ATM (as in, two device have identical
> > > >   macs? any other issues)?
> > >
> > > The only wrong thing is the call to  mlx5_query_nic_vport_mac_address().
> > > It's not breaking anything yet is wrong. The random MAC address setting
> > > is required for the steering patches.
> >
> > Okay so I'm not sure the Fixes tag at least is appropriate if it's a
> > dependency of a new feature.
> >
> > > > - why won't device MAC change from guest point of view?
> > > >
> > >
> > > It's lack of implementation in qemu as far as I know.
> >
> > Sorry not sure I understand. What's not implemented in QEMU?
> >
> HI Michael, there are some bug in qemu to set_config, this will fix in future,
> But this patch is still needed, because without this patch the mlx
> driver will give an 0 mac address to qemu
> and qemu will overwrite the default mac address.  This will cause traffic 
> down.

Hmm the patch description says VF mac address, not 0 address. Confused.
If there's no mac we can clear VIRTIO_NET_F_MAC and have guest
use a random value ...

> > > >
> > > > > > > ---
> > > > > > >  drivers/vdpa/mlx5/net/mlx5_vnet.c | 5 +
> > > > > > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
> > > > > > > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > > > index 1fa6fcac8299..80d06d958b8b 100644
> > > > > > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > > > @@ -1955,10 +1955,7 @@ void *mlx5_vdpa_add_dev(struct 
> > > > > > > mlx5_core_dev *mdev)
> > > > > > >   if (err)
> > > > > > >   goto err_mtu;
> > > > > > >
> > > > > > > - err = mlx5_query_nic_vport_mac_address(mdev, 0, 0, 
> > > > > > > config->mac);
> > > > > > > - if (err)
> > > > > > > - goto err_mtu;
> > > > > > > -
> > > > > > > + eth_random_addr(config->mac);
> > > > > > >   mvdev->vdev.dma_dev = mdev->device;
> > > > > > >   err = mlx5_vdpa_alloc_resources(>mvdev);
> > > > > > >   if (err)
> > > > > > > --
> > > > > > > 2.26.2
> > > > > >
> > > >
> >

___

Re: [PATCH] vdpa/mlx5: Use random MAC for the vdpa net instance

[Michael S. Tsirkin]

On Mon, Nov 30, 2020 at 01:51:06PM +0200, Eli Cohen wrote:
> On Mon, Nov 30, 2020 at 04:33:09AM -0500, Michael S. Tsirkin wrote:
> > On Mon, Nov 30, 2020 at 11:27:59AM +0200, Eli Cohen wrote:
> > > On Mon, Nov 30, 2020 at 04:00:51AM -0500, Michael S. Tsirkin wrote:
> > > > On Mon, Nov 30, 2020 at 08:27:46AM +0200, Eli Cohen wrote:
> > > > > On Sun, Nov 29, 2020 at 03:08:22PM -0500, Michael S. Tsirkin wrote:
> > > > > > On Sun, Nov 29, 2020 at 08:43:51AM +0200, Eli Cohen wrote:
> > > > > > > We should not try to use the VF MAC address as that is used by the
> > > > > > > regular (e.g. mlx5_core) NIC implementation. Instead, use a random
> > > > > > > generated MAC address.
> > > > > > > 
> > > > > > > Suggested by: Cindy Lu 
> > > > > > > Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported 
> > > > > > > mlx5 devices")
> > > > > > > Signed-off-by: Eli Cohen 
> > > > > > 
> > > > > > I didn't realise it's possible to use VF in two ways
> > > > > > with and without vdpa.
> > > > > 
> > > > > Using a VF you can create quite a few resources, e.g. send queues
> > > > > recieve queues, virtio_net queues etc. So you can possibly create
> > > > > several instances of vdpa net devices and nic net devices.
> > > > > 
> > > > > > Could you include a bit more description on the failure
> > > > > > mode?
> > > > > 
> > > > > Well, using the MAC address of the nic vport is wrong since that is 
> > > > > the
> > > > > MAC of the regular NIC implementation of mlx5_core.
> > > > 
> > > > Right but ATM it doesn't coexist with vdpa so what's the problem?
> > > > 
> > > 
> > > This call is wrong:  mlx5_query_nic_vport_mac_address()
> > > 
> > > > > > Is switching to a random mac for such an unusual
> > > > > > configuration really justified?
> > > > > 
> > > > > Since I can't use the NIC's MAC address, I have two options:
> > > > > 1. To get the MAC address as was chosen by the user administering the
> > > > >NIC. This should invoke the set_config callback. Unfortunately this
> > > > >is not implemented yet.
> > > > > 
> > > > > 2. Use a random MAC address. This is OK since if (1) is implemented it
> > > > >can always override this random configuration.
> > > > > 
> > > > > > It looks like changing a MAC could break some guests,
> > > > > > can it not?
> > > > > >
> > > > > 
> > > > > No, it will not. The current version of mlx5 VDPA does not allow 
> > > > > regular
> > > > > NIC driver and VDPA to co-exist. I have patches ready that enable that
> > > > > from steering point of view. I will post them here once other patches 
> > > > > on
> > > > > which they depend will be merged.
> > > > > 
> > > > > https://patchwork.ozlabs.org/project/netdev/patch/20201120230339.651609-12-sae...@nvidia.com/
> > > > 
> > > > Could you be more explicit on the following points:
> > > > - which configuration is broken ATM (as in, two device have identical
> > > >   macs? any other issues)?
> > > 
> > > The only wrong thing is the call to  mlx5_query_nic_vport_mac_address().
> > > It's not breaking anything yet is wrong. The random MAC address setting
> > > is required for the steering patches.
> > 
> > Okay so I'm not sure the Fixes tag at least is appropriate if it's a
> > dependency of a new feature.
> > 
> 
> OK, let's leave it for now. I will push along with the steering patches.
> The meaning is the the VDPA net device instance is create with a MAC of
> all zeros which also mean the link is down. You can set a MAC which will
> let the link come up.
> The vdpa driver will not get a callback as I
> stated but since current mode of steering directs all traffic to the
> vdpa instance it will work. In the future this must be fixed.

So at the moment the MAC is in the config space and that is read during
probe.  So I guess userspace will do that before passing device to
guest.

> 
> > > > - why won't device MAC change from guest point of view?
> > > > 
> > > 
> > > It's lack of implementation in qemu as far as I know.
> > 
> > Sorry not sure I understand. What's not implemented in QEMU?
> > 
> 
> vdpa config operation set_config() should be called whenever the MAC is
> changed, e.g. when administrator of the vdpa net device changes the mac.
> This does not happen which is a bug.

I am not sure that's a good interface for that, I think set_config
is for guest writes into config space.
Will let Jason comment once patches are posted.

> > > > 
> > > > > > > ---
> > > > > > >  drivers/vdpa/mlx5/net/mlx5_vnet.c | 5 +
> > > > > > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
> > > > > > > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > > > index 1fa6fcac8299..80d06d958b8b 100644
> > > > > > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > > > @@ -1955,10 +1955,7 @@ void *mlx5_vdpa_add_dev(struct 
> > > > > > > mlx5_core_dev *mdev)
> > > > > > >   if (err)
> > > > > > >   goto 

Re: [PATCH v2 17/17] vdpa: split vdpasim to core and net modules

[Stefano Garzarella]

On Mon, Nov 30, 2020 at 11:31:43AM +0800, Jason Wang wrote:


On 2020/11/26 下午10:49, Stefano Garzarella wrote:

From: Max Gurtovoy

Introduce new vdpa_sim_net and vdpa_sim (core) drivers. This is a
preparation for adding a vdpa simulator module for block devices.

Signed-off-by: Max Gurtovoy
[sgarzare: various cleanups/fixes]
Signed-off-by: Stefano Garzarella
---
v2:
- Fixed "warning: variable 'dev' is used uninitialized" reported by
  'kernel test robot' and Dan Carpenter
- rebased on top of other changes (dev_attr, get_config(), notify(), etc.)
- left batch_mapping module parameter in the core [Jason]

v1:
- Removed unused headers
- Removed empty module_init() module_exit()
- Moved vdpasim_is_little_endian() in vdpa_sim.h
- Moved vdpasim16_to_cpu/cpu_to_vdpasim16() in vdpa_sim.h
- Added vdpasim*_to_cpu/cpu_to_vdpasim*() also for 32 and 64
- Replaced 'select VDPA_SIM' with 'depends on VDPA_SIM' since selected
  option can not depend on other [Jason]
---
 drivers/vdpa/vdpa_sim/vdpa_sim.h | 103 +
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 222 +--
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 171 +
 drivers/vdpa/Kconfig |  13 +-
 drivers/vdpa/vdpa_sim/Makefile   |   1 +
 5 files changed, 290 insertions(+), 220 deletions(-)
 create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim.h
 create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim_net.c



Looks good, consider there are some still some questions left. I will 
probably ack for the next version.




Sure, thanks for your feedback!

Stefano

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 13/17] vdpa_sim: set vringh notify callback

[Stefano Garzarella]

On Mon, Nov 30, 2020 at 11:27:51AM +0800, Jason Wang wrote:


On 2020/11/26 下午10:49, Stefano Garzarella wrote:

Instead of calling the vq callback directly, we can leverage the
vringh_notify() function, adding vdpasim_vq_notify() and setting it
in the vringh notify callback.

Suggested-by: Jason Wang 
Signed-off-by: Stefano Garzarella 
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 8b87ce0485b6..4327efd6d41e 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -120,6 +120,17 @@ static struct vdpasim *dev_to_sim(struct device *dev)
return vdpa_to_sim(vdpa);
 }
+static void vdpasim_vq_notify(struct vringh *vring)
+{
+   struct vdpasim_virtqueue *vq =
+   container_of(vring, struct vdpasim_virtqueue, vring);
+
+   if (!vq->cb)
+   return;
+
+   vq->cb(vq->private);
+}
+
 static void vdpasim_queue_ready(struct vdpasim *vdpasim, unsigned int idx)
 {
struct vdpasim_virtqueue *vq = >vqs[idx];
@@ -131,6 +142,8 @@ static void vdpasim_queue_ready(struct vdpasim *vdpasim, 
unsigned int idx)
  (uintptr_t)vq->driver_addr,
  (struct vring_used *)
  (uintptr_t)vq->device_addr);
+
+   vq->vring.notify = vdpasim_vq_notify;



Do we need to clear notify during reset?


Right, I'll clear it.



Other looks good.



Thanks,
Stefano

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 12/17] vdpa_sim: add get_config callback in vdpasim_dev_attr

[Stefano Garzarella]

On Mon, Nov 30, 2020 at 11:25:31AM +0800, Jason Wang wrote:


On 2020/11/26 下午10:49, Stefano Garzarella wrote:

The get_config callback can be used by the device to fill the
config structure.
The callback will be invoked in vdpasim_get_config() before copying
bytes into caller buffer.

Move vDPA-net config updates from vdpasim_set_features() in the
new vdpasim_net_get_config() callback.

Signed-off-by: Stefano Garzarella 
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 33 +++-
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index c07ddf6af720..8b87ce0485b6 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -58,6 +58,8 @@ struct vdpasim_virtqueue {
 #define VDPASIM_NET_FEATURES   (VDPASIM_FEATURES | \
 (1ULL << VIRTIO_NET_F_MAC))
+struct vdpasim;
+
 struct vdpasim_dev_attr {
u64 supported_features;
size_t config_size;
@@ -65,6 +67,7 @@ struct vdpasim_dev_attr {
u32 id;
work_func_t work_fn;
+   void (*get_config)(struct vdpasim *vdpasim, void *config);
 };
 /* State of each vdpasim device */
@@ -520,8 +523,6 @@ static u64 vdpasim_get_features(struct vdpa_device *vdpa)
 static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
 {
struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
-   struct virtio_net_config *config =
-   (struct virtio_net_config *)vdpasim->config;
/* DMA mapping must be done by driver */
if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
@@ -529,15 +530,6 @@ static int vdpasim_set_features(struct vdpa_device *vdpa, 
u64 features)
vdpasim->features = features & vdpasim->dev_attr.supported_features;
-   /* We generally only know whether guest is using the legacy interface
-* here, so generally that's the earliest we can set config fields.
-* Note: We actually require VIRTIO_F_ACCESS_PLATFORM above which
-* implies VIRTIO_F_VERSION_1, but let's not try to be clever here.
-*/
-
-   config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
-   config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
-   memcpy(config->mac, macaddr_buf, ETH_ALEN);
return 0;
 }
@@ -593,8 +585,12 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, 
unsigned int offset,
 {
struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
-   if (offset + len < vdpasim->dev_attr.config_size)
-   memcpy(buf, vdpasim->config + offset, len);
+   if (offset + len > vdpasim->dev_attr.config_size)
+   return;
+
+   vdpasim->dev_attr.get_config(vdpasim, vdpasim->config);
+
+   memcpy(buf, vdpasim->config + offset, len);
 }



I wonder how much value we can get from vdpasim->config consider we've 
already had get_config() method.


Is it possible to copy to the buffer directly here?


I had thought of eliminating it too, but then I wanted to do something 
similar to what we do in QEMU (hw/virtio/virtio.c), leaving in the 
simulator core the buffer, the memory copy (handling offset and len), 
and the boundary checks.


In this way each device should simply fill the entire configuration and 
we can avoid code duplication.


Storing the configuration in the core may also be useful if some device 
needs to support config writes.


Do you think it makes sense, or is it better to move everything in the 
device?


Thanks,
Stefano

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper

[Paolo Bonzini]

On 30/11/20 14:57, Greg KH wrote:

Every patch should be "fixing a real issue"---even a new feature.  But the
larger the patch, the more the submitters and maintainers should be trusted
rather than a bot.  The line between feature and bugfix_sometimes_  is
blurry, I would say that in this case it's not, and it makes me question how
the bot decided that this patch would be acceptable for stable (which AFAIK
is not something that can be answered).

I thought that earlier Sasha said that this patch was needed as a
prerequisite patch for a later fix, right?  If not, sorry, I've lost the
train of thought in this thread...


Yeah---sorry I am replying to 22/33 but referring to 23/33, which is the 
one that in my opinion should not be blindly accepted for stable kernels 
without the agreement of the submitter or maintainer.


Paolo

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper

[Greg KH]

On Mon, Nov 30, 2020 at 02:52:11PM +0100, Paolo Bonzini wrote:
> On 30/11/20 14:28, Greg KH wrote:
> > > > Lines of code is not everything. If you think that this needs additional
> > > > testing then that's fine and we can drop it, but not picking up a fix
> > > > just because it's 120 lines is not something we'd do.
> > > Starting with the first two steps in stable-kernel-rules.rst:
> > > 
> > > Rules on what kind of patches are accepted, and which ones are not, into 
> > > the
> > > "-stable" tree:
> > > 
> > >   - It must be obviously correct and tested.
> > >   - It cannot be bigger than 100 lines, with context.
> > We do obviously take patches that are bigger than 100 lines, as there
> > are always exceptions to the rules here.  Look at all of the
> > spectre/meltdown patches as one such example.  Should we refuse a patch
> > just because it fixes a real issue yet is 101 lines long?
> 
> Every patch should be "fixing a real issue"---even a new feature.  But the
> larger the patch, the more the submitters and maintainers should be trusted
> rather than a bot.  The line between feature and bugfix _sometimes_ is
> blurry, I would say that in this case it's not, and it makes me question how
> the bot decided that this patch would be acceptable for stable (which AFAIK
> is not something that can be answered).

I thought that earlier Sasha said that this patch was needed as a
prerequisite patch for a later fix, right?  If not, sorry, I've lost the
train of thought in this thread...

thanks,

greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Doctoral Symposium - CISTI 2021, Chaves, Portugal

[Maria Lemos]

--  --  
--  --
Doctoral Symposium

CISTI 2021 - 16th Iberian Conference on Information Systems and Technologies, 
Chaves, Portugal, 23 - 26 June 2021

http://www.cisti.eu/ 

--  --  
--  -


The purpose of CISTI'2021’s Doctoral Symposium is to provide graduate students 
a setting where they can, informally, expose and discuss their work, collecting 
valuable expert opinions and sharing new ideas, methods and applications. The 
Doctoral Symposium is an excellent opportunity for PhD students to present and 
discuss their work in a Workshop format. Each presentation will be evaluated by 
a panel composed by at least three Information Systems and Technologies experts.



Contributions Submission

The Doctoral Symposium is opened to PhD students whose research area includes 
the themes proposed for this Conference. Submissions must include an extended 
abstract (maximum 4 pages), following the Conference style guide. All selected 
contributions will be published with the Conference Proceedings in electronic 
format with ISBN. These contributions will be available in the IEEE Xplore 
Digital Library and will be sent for indexing in ISI, Scopus, EI-Compendex, 
INSPEC and Google Scholar.

Submissions must include the field, the PhD institution and the number of 
months devoted to the development of the work. Additionally, they should 
include in a clear and succinct manner:

   •The problem approached and its significance or relevance
   •The research objectives and related investigation topics
   •A brief display of what is already known
   •A proposed solution methodology for the problem
   •Expected results



Important Dates

Paper submission: February 14, 2021

Notification of acceptance: March 28, 2021

Submission of accepted papers: April 11, 2021

Payment of registration, to ensure the inclusion of an accepted paper in the 
conference proceedings: April 11, 2021



Organizing Committee

Álvaro Rocha, ISEG, Universidade de Lisboa

Francisco García-Peñalvo, Universidad de Salamanca



Scientific Committee

Francisco García-Peñalvo, Universidad de Salamanca (Chair)

A. Augusto Sousa, FEUP, Universidade do Porto

Adérito Fernandes-Marcos, Universidade Aberta

Adolfo Lozano Tello, Universidad de Extremadura

Alicia García Holgado, Universidad de Salamanca

Álvaro Rocha, ISEG, Universidade de Lisboa

Ana Amélia Carvalho, Universidade de Coimbra

António Palma do Reis, ISEG, Universidade de Lisboa

Arnaldo Martins, Universidade de Aveiro

Borja Bordel, Universidad Politécnica de Madrid

Bráulio Alturas, ISCTE - Instituto Universitário de Lisboa

Carina Soledad González, Universidad de La Laguna

Carlos Costa, ISEG, Universidade de Lisboa

Carlos Ferrás Sexto, Universidad de Santiago de Compostela

Cesar Collazos, Universidad del Cauca

Daniel Amo, La Salle, Universidad Ramon Llull

David Fonseca, La Salle, Universitat Ramon Llull

Eduardo Sánchez Vila, Universidade de Santiago de Compostela

Fernando Moreira, Universidade Portucalense

Fernando Ramos, Universidade de Aveiro

Francisco Restivo, Universidade Católica Portuguesa

Gonçalo Paiva Dias, Universidade de Aveiro

João Costa, Universidade de Coimbra

João Manuel R.S. Tavares, FEUP, Universidade do Porto

João Pascoal Faria, FEUP, Universidade do Porto

José Machado, Universidade do Minho

Luis Camarinha-Matos, FCT, Universidade NOVA de Lisboa

Luís Paulo Reis, FEUP, Universidade do Porto

Marcelo Marciszack, Universidad Tecnológica Nacional

Marco Painho, NOVA IMS

María J Lado, Universidade de Vigo

María Pilar Mareca Lopez, Universidad Politécnica de Madrid

Mário Piattini, Universidad de Castilla-La Mancha

Martin Llamas Nistal, Universidad de Vigo

Miguel de Castro Neto, NOVA IMS

Miguel Ramón González-Castro, ENCE

Nelson Rocha, Universidade de Aveiro

Óscar Mealha, Universidade de Aveiro

Paulo Pinto, FCT, Universidade Nova de Lisboa

Ramiro Gonçalves, Universidade de Trás-os-Montes e Alto Douro

Tomas San Feliu, Universidad Politécnica de Madrid

Vitor Santos, NOVA IMS



Website of CISTI'2020: http://www.cisti.eu/ 



CISTI 2021 Team

http://www.cisti.eu/ 


--
This email has been checked for viruses by AVG.
https://www.avg.com
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper

[Paolo Bonzini]

On 30/11/20 14:28, Greg KH wrote:

Lines of code is not everything. If you think that this needs additional
testing then that's fine and we can drop it, but not picking up a fix
just because it's 120 lines is not something we'd do.

Starting with the first two steps in stable-kernel-rules.rst:

Rules on what kind of patches are accepted, and which ones are not, into the
"-stable" tree:

  - It must be obviously correct and tested.
  - It cannot be bigger than 100 lines, with context.

We do obviously take patches that are bigger than 100 lines, as there
are always exceptions to the rules here.  Look at all of the
spectre/meltdown patches as one such example.  Should we refuse a patch
just because it fixes a real issue yet is 101 lines long?


Every patch should be "fixing a real issue"---even a new feature.  But 
the larger the patch, the more the submitters and maintainers should be 
trusted rather than a bot.  The line between feature and bugfix 
_sometimes_ is blurry, I would say that in this case it's not, and it 
makes me question how the bot decided that this patch would be 
acceptable for stable (which AFAIK is not something that can be answered).


Paolo

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper

[Greg KH]

On Mon, Nov 30, 2020 at 09:33:46AM +0100, Paolo Bonzini wrote:
> On 29/11/20 22:06, Sasha Levin wrote:
> > On Sun, Nov 29, 2020 at 06:34:01PM +0100, Paolo Bonzini wrote:
> > > On 29/11/20 05:13, Sasha Levin wrote:
> > > > > Which doesn't seem to be suitable for stable either...  Patch 3/5 in
> > > > 
> > > > Why not? It was sent as a fix to Linus.
> > > 
> > > Dunno, 120 lines of new code?  Even if it's okay for an rc, I don't
> > > see why it is would be backported to stable releases and release it
> > > without any kind of testing.  Maybe for 5.9 the chances of breaking
> > 
> > Lines of code is not everything. If you think that this needs additional
> > testing then that's fine and we can drop it, but not picking up a fix
> > just because it's 120 lines is not something we'd do.
> 
> Starting with the first two steps in stable-kernel-rules.rst:
> 
> Rules on what kind of patches are accepted, and which ones are not, into the
> "-stable" tree:
> 
>  - It must be obviously correct and tested.
>  - It cannot be bigger than 100 lines, with context.

We do obviously take patches that are bigger than 100 lines, as there
are always exceptions to the rules here.  Look at all of the
spectre/meltdown patches as one such example.  Should we refuse a patch
just because it fixes a real issue yet is 101 lines long?

thanks,

greg k-h
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 04/17] vdpa_sim: remove the limit of IOTLB entries

[Stefano Garzarella]

On Mon, Nov 30, 2020 at 11:07:08AM +0800, Jason Wang wrote:


On 2020/11/26 下午10:49, Stefano Garzarella wrote:

The simulated devices can support multiple queues, so this limit
should be defined according to the number of queues supported by
the device.

Since we are in a simulator, let's simply remove that limit.

Suggested-by: Jason Wang 
Acked-by: Jason Wang 
Signed-off-by: Stefano Garzarella 
---
v2:
- added VDPASIM_IOTLB_LIMIT macro [Jason]



Sorry for being unclear. I meant adding a macro like

VHOST_IOTLB_UNLIMITED 0 in vhost_iotlb.h.

And use that in vdpa_sim.


Got it :-) I'll fix adding the macro in another patch and using it in 
this one.


Thanks,
Stefano

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 02/17] vdpa_sim: remove unnecessary headers inclusion

[Stefano Garzarella]

On Mon, Nov 30, 2020 at 11:04:49AM +0800, Jason Wang wrote:


On 2020/11/26 下午10:49, Stefano Garzarella wrote:

Some headers are not necessary, so let's remove them to do
some cleaning.

Signed-off-by: Stefano Garzarella 
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 6a90fdb9cbfc..c6eaf62df8ec 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -7,24 +7,11 @@
  *
  */
-#include 
 #include 
-#include 



I think the rule is to make sure e.g the structure definition can be 
via direct inclusion. E.g struct device {} is defined in this file.




-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
 #include 
-#include 
-#include 
 #include 
 #include 
 #include 
-#include 



And the  __cpu_to_virtio16 is defined in this file.



Okay, I'll keep this two includes and check better the others.

Thanks,
Stefano

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] vdpa/mlx5: Use random MAC for the vdpa net instance

[Michael S. Tsirkin]

On Mon, Nov 30, 2020 at 11:27:59AM +0200, Eli Cohen wrote:
> On Mon, Nov 30, 2020 at 04:00:51AM -0500, Michael S. Tsirkin wrote:
> > On Mon, Nov 30, 2020 at 08:27:46AM +0200, Eli Cohen wrote:
> > > On Sun, Nov 29, 2020 at 03:08:22PM -0500, Michael S. Tsirkin wrote:
> > > > On Sun, Nov 29, 2020 at 08:43:51AM +0200, Eli Cohen wrote:
> > > > > We should not try to use the VF MAC address as that is used by the
> > > > > regular (e.g. mlx5_core) NIC implementation. Instead, use a random
> > > > > generated MAC address.
> > > > > 
> > > > > Suggested by: Cindy Lu 
> > > > > Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 
> > > > > devices")
> > > > > Signed-off-by: Eli Cohen 
> > > > 
> > > > I didn't realise it's possible to use VF in two ways
> > > > with and without vdpa.
> > > 
> > > Using a VF you can create quite a few resources, e.g. send queues
> > > recieve queues, virtio_net queues etc. So you can possibly create
> > > several instances of vdpa net devices and nic net devices.
> > > 
> > > > Could you include a bit more description on the failure
> > > > mode?
> > > 
> > > Well, using the MAC address of the nic vport is wrong since that is the
> > > MAC of the regular NIC implementation of mlx5_core.
> > 
> > Right but ATM it doesn't coexist with vdpa so what's the problem?
> > 
> 
> This call is wrong:  mlx5_query_nic_vport_mac_address()
> 
> > > > Is switching to a random mac for such an unusual
> > > > configuration really justified?
> > > 
> > > Since I can't use the NIC's MAC address, I have two options:
> > > 1. To get the MAC address as was chosen by the user administering the
> > >NIC. This should invoke the set_config callback. Unfortunately this
> > >is not implemented yet.
> > > 
> > > 2. Use a random MAC address. This is OK since if (1) is implemented it
> > >can always override this random configuration.
> > > 
> > > > It looks like changing a MAC could break some guests,
> > > > can it not?
> > > >
> > > 
> > > No, it will not. The current version of mlx5 VDPA does not allow regular
> > > NIC driver and VDPA to co-exist. I have patches ready that enable that
> > > from steering point of view. I will post them here once other patches on
> > > which they depend will be merged.
> > > 
> > > https://patchwork.ozlabs.org/project/netdev/patch/20201120230339.651609-12-sae...@nvidia.com/
> > 
> > Could you be more explicit on the following points:
> > - which configuration is broken ATM (as in, two device have identical
> >   macs? any other issues)?
> 
> The only wrong thing is the call to  mlx5_query_nic_vport_mac_address().
> It's not breaking anything yet is wrong. The random MAC address setting
> is required for the steering patches.

Okay so I'm not sure the Fixes tag at least is appropriate if it's a
dependency of a new feature.

> > - why won't device MAC change from guest point of view?
> > 
> 
> It's lack of implementation in qemu as far as I know.

Sorry not sure I understand. What's not implemented in QEMU?

> > 
> > > > > ---
> > > > >  drivers/vdpa/mlx5/net/mlx5_vnet.c | 5 +
> > > > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
> > > > > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > index 1fa6fcac8299..80d06d958b8b 100644
> > > > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > > > @@ -1955,10 +1955,7 @@ void *mlx5_vdpa_add_dev(struct mlx5_core_dev 
> > > > > *mdev)
> > > > >   if (err)
> > > > >   goto err_mtu;
> > > > >  
> > > > > - err = mlx5_query_nic_vport_mac_address(mdev, 0, 0, config->mac);
> > > > > - if (err)
> > > > > - goto err_mtu;
> > > > > -
> > > > > + eth_random_addr(config->mac);
> > > > >   mvdev->vdev.dma_dev = mdev->device;
> > > > >   err = mlx5_vdpa_alloc_resources(>mvdev);
> > > > >   if (err)
> > > > > -- 
> > > > > 2.26.2
> > > > 
> > 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] vdpa/mlx5: Use random MAC for the vdpa net instance

[Michael S. Tsirkin]

On Mon, Nov 30, 2020 at 08:27:46AM +0200, Eli Cohen wrote:
> On Sun, Nov 29, 2020 at 03:08:22PM -0500, Michael S. Tsirkin wrote:
> > On Sun, Nov 29, 2020 at 08:43:51AM +0200, Eli Cohen wrote:
> > > We should not try to use the VF MAC address as that is used by the
> > > regular (e.g. mlx5_core) NIC implementation. Instead, use a random
> > > generated MAC address.
> > > 
> > > Suggested by: Cindy Lu 
> > > Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 
> > > devices")
> > > Signed-off-by: Eli Cohen 
> > 
> > I didn't realise it's possible to use VF in two ways
> > with and without vdpa.
> 
> Using a VF you can create quite a few resources, e.g. send queues
> recieve queues, virtio_net queues etc. So you can possibly create
> several instances of vdpa net devices and nic net devices.
> 
> > Could you include a bit more description on the failure
> > mode?
> 
> Well, using the MAC address of the nic vport is wrong since that is the
> MAC of the regular NIC implementation of mlx5_core.

Right but ATM it doesn't coexist with vdpa so what's the problem?

> > Is switching to a random mac for such an unusual
> > configuration really justified?
> 
> Since I can't use the NIC's MAC address, I have two options:
> 1. To get the MAC address as was chosen by the user administering the
>NIC. This should invoke the set_config callback. Unfortunately this
>is not implemented yet.
> 
> 2. Use a random MAC address. This is OK since if (1) is implemented it
>can always override this random configuration.
> 
> > It looks like changing a MAC could break some guests,
> > can it not?
> >
> 
> No, it will not. The current version of mlx5 VDPA does not allow regular
> NIC driver and VDPA to co-exist. I have patches ready that enable that
> from steering point of view. I will post them here once other patches on
> which they depend will be merged.
> 
> https://patchwork.ozlabs.org/project/netdev/patch/20201120230339.651609-12-sae...@nvidia.com/

Could you be more explicit on the following points:
- which configuration is broken ATM (as in, two device have identical
  macs? any other issues)?
- why won't device MAC change from guest point of view?


> > > ---
> > >  drivers/vdpa/mlx5/net/mlx5_vnet.c | 5 +
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
> > > b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > index 1fa6fcac8299..80d06d958b8b 100644
> > > --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > @@ -1955,10 +1955,7 @@ void *mlx5_vdpa_add_dev(struct mlx5_core_dev *mdev)
> > >   if (err)
> > >   goto err_mtu;
> > >  
> > > - err = mlx5_query_nic_vport_mac_address(mdev, 0, 0, config->mac);
> > > - if (err)
> > > - goto err_mtu;
> > > -
> > > + eth_random_addr(config->mac);
> > >   mvdev->vdev.dma_dev = mdev->device;
> > >   err = mlx5_vdpa_alloc_resources(>mvdev);
> > >   if (err)
> > > -- 
> > > 2.26.2
> > 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[GIT PULL] vhost,vdpa: bugfixes

[Michael S. Tsirkin]

The following changes since commit 418baf2c28f3473039f2f7377760bd8f6897ae18:

  Linux 5.10-rc5 (2020-11-22 15:36:08 -0800)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

for you to fetch changes up to ad89653f79f1882d55d9df76c9b2b94f008c4e27:

  vhost-vdpa: fix page pinning leakage in error path (rework) (2020-11-25 
04:29:07 -0500)


vhost,vdpa: fixes

A couple of minor fixes.

Signed-off-by: Michael S. Tsirkin 


Mike Christie (1):
  vhost scsi: fix lun reset completion handling

Si-Wei Liu (1):
  vhost-vdpa: fix page pinning leakage in error path (rework)

Stefano Garzarella (1):
  vringh: fix vringh_iov_push_*() documentation

 drivers/vhost/scsi.c   |  4 ++-
 drivers/vhost/vdpa.c   | 80 ++
 drivers/vhost/vringh.c |  6 ++--
 3 files changed, 68 insertions(+), 22 deletions(-)

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH AUTOSEL 5.9 22/33] vhost scsi: add lun parser helper

[Paolo Bonzini]

On 29/11/20 22:06, Sasha Levin wrote:

On Sun, Nov 29, 2020 at 06:34:01PM +0100, Paolo Bonzini wrote:

On 29/11/20 05:13, Sasha Levin wrote:

Which doesn't seem to be suitable for stable either...  Patch 3/5 in


Why not? It was sent as a fix to Linus.


Dunno, 120 lines of new code?  Even if it's okay for an rc, I don't 
see why it is would be backported to stable releases and release it 
without any kind of testing.  Maybe for 5.9 the chances of breaking 


Lines of code is not everything. If you think that this needs additional
testing then that's fine and we can drop it, but not picking up a fix
just because it's 120 lines is not something we'd do.


Starting with the first two steps in stable-kernel-rules.rst:

Rules on what kind of patches are accepted, and which ones are not, into 
the "-stable" tree:


 - It must be obviously correct and tested.
 - It cannot be bigger than 100 lines, with context.


Plus all the testing we have for the stable trees, yes. It goes beyond
just compiling at this point.

Your very own co-workers (https://cki-project.org/) are pushing hard on
this effort around stable kernel testing, and statements like these
aren't helping anyone.


I am not aware of any public CI being done _at all_ done on vhost-scsi, 
by CKI or everyone else.  So autoselection should be done only on 
subsystems that have very high coverage in CI.


Paolo

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization