On Wed, Mar 03, 2021 at 08:33:50AM +0200, Eli Cohen wrote:
On Wed, Mar 03, 2021 at 05:59:50AM +0200, Parav Pandit wrote:
quoted
Hi Eli,
quoted
From: Eli Cohen <redacted>
Sent: Tuesday, March 2, 2021 11:09 AM
On Mon, Mar 01, 2021 at 11:12:33AM -0500, Michael S. Tsirkin wrote:
quoted
On Mon, Mar 01, 2021 at 03:19:51PM +0200, Eli Cohen wrote:
quoted
On Mon, Mar 01, 2021 at 08:09:48AM -0500, Michael S. Tsirkin wrote:
quoted
On Mon, Mar 01, 2021 at 09:08:28AM +0200, Eli Cohen wrote:
quoted
On Wed, Feb 24, 2021 at 05:11:23PM +0800, Jason Wang wrote:
quoted
On 2021/2/24 2:18 下午, Parav Pandit wrote:
quoted
From: Eli Cohen <redacted>
Use a randomly generated MAC address to be applied in case
it is not configured by management tool.
The value queried through mlx5_query_nic_vport_mac_address()
is not relelavnt to vdpa since it is the mac that should be
used by the regular NIC driver.
Signed-off-by: Eli Cohen <redacted>
Reviewed-by: Parav Pandit <redacted>
Acked-by: Jason Wang <redacted>
quoted
---
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 b67bba581dfd..ece2183e7b20 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -2005,10 +2005,7 @@ static int mlx5_vdpa_dev_add(struct
vdpa_mgmt_dev *v_mdev, const char *name)
quoted
quoted
quoted
quoted
quoted
quoted
if (err)
goto err_mtu;
- err = mlx5_query_nic_vport_mac_address(mdev, 0, 0, config-
mac);
quoted
quoted
quoted
quoted
quoted
- if (err)
- goto err_mtu;
-
+ eth_random_addr(config->mac);
I think this patch is missing setting VIRTIO_NET_F_MTU. I will
post v2 with the other fixes in this series.
I don't really understand why this is a good idea.
If userspace wants a random mac it can set it, with this patch it
is impossible to know whether the mac is a hardware one (which
will be persistent e.g. across reboots) or a random one.
You can still get a persistent MAC set by the vdpa tool. e.g
vdpa dev config set vdpa0 mac 00:11:22:33:44:55
If you don't use vdpa tool, the device assigns a random MAC. This
MAC is used to filter imcoming unicast traffic (done in a subsequent
patch).
quoted
Well previously device learned the MAC from outgoing traffic and used
that for the filter.
No, was is added only in the last series that Parav send. Before that the
device did not filter and forwarded any packet that was forwarded to it buy
the eswitch.
quoted
Is changing that to a random value really all that useful as a
default? Why not do the randomization in userspace?
I think we want management entity to set the MAC, that's the VDPA tool.
So as things are right now (with the last series applied), the vdpa tool is the
tool to assign MAC addresses and if it doesn't, a device randomly generated
MAC applies. Currently MAC addresses set by qemu command line are
ignored (set_config is not implementd). We can allow this but this will
override vdpa tool configuration.
I believe its incorrect to always do random generated mac as of this patch.
This is because, doing so ignores any admin config done by the sysadmin on the switch (ovs side) using [1].
Well, when this patch was sent, we still had thoughts to let mlx5e NIC
and the VDPA to co-exist on the same function (VF or SF). Now that we're
off this idea you can become tempted to use the MAC address configured
for the NIC but I don't think it's a good idea. We already have a
dedicated tool to configure MAC for VDPA so let's use it.
Right. And users can decide to reuse the hardware MAC if they want to.
quoted
So if user choose to configure using eswitch config, mlx5_vnet to honor that.
And if user prefers to override is using vdpa tool or set_config from QEMU side, so be it.
I agree that we should let the user to configure the MAC through qemu
argument when booting the VM. So I'll add this for the next spin of this
series.
OK so
- if MAC is configured it is used
- if not configured 0 is reported to userspace
fair summary?
quoted
Hence, instead of reporting all zeros, mlx5 should query own vport context and publish that mac in the config layout and rx steering side.
If user choose to override it, config layout and rx rules will have to updated on such config.
[1] devlink port function set pci/0000:03:00.0/<port_id_of_sf/vf>/ hw_addr 00:11:22:33:44:55
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
From: Michael S. Tsirkin <mst@redhat.com>
Sent: Wednesday, March 3, 2021 2:59 PM
On Wed, Mar 03, 2021 at 08:33:50AM +0200, Eli Cohen wrote:
quoted
On Wed, Mar 03, 2021 at 05:59:50AM +0200, Parav Pandit wrote:
quoted
Hi Eli,
quoted
From: Eli Cohen <redacted>
Sent: Tuesday, March 2, 2021 11:09 AM
On Mon, Mar 01, 2021 at 11:12:33AM -0500, Michael S. Tsirkin wrote:
quoted
On Mon, Mar 01, 2021 at 03:19:51PM +0200, Eli Cohen wrote:
quoted
On Mon, Mar 01, 2021 at 08:09:48AM -0500, Michael S. Tsirkin wrote:
quoted
On Mon, Mar 01, 2021 at 09:08:28AM +0200, Eli Cohen wrote:
quoted
On Wed, Feb 24, 2021 at 05:11:23PM +0800, Jason Wang wrote:
quoted
On 2021/2/24 2:18 下午, Parav Pandit wrote:
quoted
From: Eli Cohen <redacted>
Use a randomly generated MAC address to be applied in
case it is not configured by management tool.
The value queried through
mlx5_query_nic_vport_mac_address()
is not relelavnt to vdpa since it is the mac that
should be used by the regular NIC driver.
Signed-off-by: Eli Cohen <redacted>
Reviewed-by: Parav Pandit <redacted>
Acked-by: Jason Wang <redacted>
quoted
---
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 b67bba581dfd..ece2183e7b20 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -2005,10 +2005,7 @@ static int
mlx5_vdpa_dev_add(struct
vdpa_mgmt_dev *v_mdev, const char *name)
quoted
quoted
quoted
quoted
quoted
quoted
if (err)
goto err_mtu;
- err = mlx5_query_nic_vport_mac_address(mdev, 0,
0, config-
quoted
quoted
quoted
quoted
mac);
quoted
quoted
quoted
quoted
quoted
- if (err)
- goto err_mtu;
-
+ eth_random_addr(config->mac);
I think this patch is missing setting VIRTIO_NET_F_MTU. I
will post v2 with the other fixes in this series.
I don't really understand why this is a good idea.
If userspace wants a random mac it can set it, with this
patch it is impossible to know whether the mac is a hardware
one (which will be persistent e.g. across reboots) or a random
one.
quoted
quoted
quoted
quoted
quoted
quoted
You can still get a persistent MAC set by the vdpa tool. e.g
vdpa dev config set vdpa0 mac 00:11:22:33:44:55
If you don't use vdpa tool, the device assigns a random MAC.
This MAC is used to filter imcoming unicast traffic (done in a
subsequent
patch).
quoted
Well previously device learned the MAC from outgoing traffic and
used that for the filter.
No, was is added only in the last series that Parav send. Before
that the device did not filter and forwarded any packet that was
forwarded to it buy the eswitch.
quoted
Is changing that to a random value really all that useful as a
default? Why not do the randomization in userspace?
I think we want management entity to set the MAC, that's the VDPA
tool.
quoted
quoted
quoted
So as things are right now (with the last series applied), the
vdpa tool is the tool to assign MAC addresses and if it doesn't, a
device randomly generated MAC applies. Currently MAC addresses set
by qemu command line are ignored (set_config is not implementd).
We can allow this but this will override vdpa tool configuration.
I believe its incorrect to always do random generated mac as of this
patch.
quoted
quoted
This is because, doing so ignores any admin config done by the sysadmin
on the switch (ovs side) using [1].
quoted
quoted
Well, when this patch was sent, we still had thoughts to let mlx5e NIC
and the VDPA to co-exist on the same function (VF or SF). Now that
we're off this idea you can become tempted to use the MAC address
configured for the NIC but I don't think it's a good idea. We already
have a dedicated tool to configure MAC for VDPA so let's use it.
Right. And users can decide to reuse the hardware MAC if they want to.
Right.
If user chose to reuse the hw mac set by the eswitch, so let them use it.
When smartnic is used, some users do not trust compute side for network attributes configuration.
So those users to configure the MAC from eswitch along with ovs policy.
I think Sean gave one example of it.
And user can choose to override it via vdpa tool/qemu.
I am not sure doing only one_way fits all.
Switch/ovs side configuring the mac along with policy seems fine to me.
But this may not fit the case for those who doesn't have eswitch.
Jason offline or in previous thread mentioned a use case to create multiple vdpa device from single PF/VF.
At Mellanox at least we don't see that use case at moment given its attached to eswitch.
quoted
quoted
So if user choose to configure using eswitch config, mlx5_vnet to honor
that.
quoted
quoted
And if user prefers to override is using vdpa tool or set_config from
QEMU side, so be it.
quoted
I agree that we should let the user to configure the MAC through qemu
argument when booting the VM. So I'll add this for the next spin of
this series.
OK so
- if MAC is configured it is used
- if not configured 0 is reported to userspace
fair summary?
LGTM.
Eli?
quoted
quoted
Hence, instead of reporting all zeros, mlx5 should query own vport
context and publish that mac in the config layout and rx steering side.
quoted
quoted
If user choose to override it, config layout and rx rules will have to
updated on such config.
quoted
quoted
[1] devlink port function set pci/0000:03:00.0/<port_id_of_sf/vf>/
hw_addr 00:11:22:33:44:55
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization