On Wed, Mar 03, 2021 at 03:59:50AM +0000, Parav Pandit wrote: > Hi Eli, > > > From: Eli Cohen <elic@xxxxxxxxxx> > > Sent: Tuesday, March 2, 2021 11:09 AM > > > > On Mon, Mar 01, 2021 at 11:12:33AM -0500, Michael S. Tsirkin wrote: > > > On Mon, Mar 01, 2021 at 03:19:51PM +0200, Eli Cohen wrote: > > > > On Mon, Mar 01, 2021 at 08:09:48AM -0500, Michael S. Tsirkin wrote: > > > > > On Mon, Mar 01, 2021 at 09:08:28AM +0200, Eli Cohen wrote: > > > > > > On Wed, Feb 24, 2021 at 05:11:23PM +0800, Jason Wang wrote: > > > > > > > > > > > > > > On 2021/2/24 2:18 下午, Parav Pandit wrote: > > > > > > > > From: Eli Cohen <elic@xxxxxxxxxx> > > > > > > > > > > > > > > > > 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 <elic@xxxxxxxxxx> > > > > > > > > Reviewed-by: Parav Pandit <parav@xxxxxxxxxx> > > > > > > > > > > > > > > > > > > > > > Acked-by: Jason Wang <jasowang@xxxxxxxxxx> > > > > > > > > > > > > > > > > > > > > > > --- > > > > > > > > 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) > > > > > > > > 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); > > > > > > > > > > > > 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). > > > > > > 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. > > > > > 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]. > > 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. > 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 well it has reported all zeros to mean "learning bridge" for a so while I suspect changing that is an ABI change, though a minor one ... If you do change it please add some other way to find out whether it still learns from outgoing traffic (ie whether mac spoofing is enabled). -- MST _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
