Message-ID: <07a24753-767b-4e1e-2bcf-21ec04bc044a@nvidia.com>
Date:   Sun, 22 Jan 2023 15:47:08 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH 1/4] virtio_net: notify MAC address change on device
 initialization
Content-Language: en-US
To:     Laurent Vivier <lvivier@redhat.com>, linux-kernel@vger.kernel.org
Cc:     "Michael S. Tsirkin" <mst@redhat.com>,
        Parav Pandit <parav@nvidia.com>,
        virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
        Jason Wang <jasowang@redhat.com>,
        Gautam Dawar <gautam.dawar@xilinx.com>,
        Cindy Lu <lulu@redhat.com>,
        "David S. Miller" <davem@davemloft.net>,
        =?UTF-8?Q?Eugenio_P=c3=a9rez?= <eperezma@redhat.com>
References: <20230122100526.2302556-1-lvivier@redhat.com>
 <20230122100526.2302556-2-lvivier@redhat.com>
From:   Eli Cohen <elic@nvidia.com>
In-Reply-To: <20230122100526.2302556-2-lvivier@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MWdZaGdZVEsydUprODRTWDEzb25IZWgrMkhOMmYvcFVVM1puZDdvWllIQ3pa?=
 =?utf-8?B?V1VBeTVGOGsrbVlsbkpkVm9CNkR4VDVGNEZSRHBmcGswMXlySjF0TkNxSE9p?=
 =?utf-8?B?NFhZY1llVUduT2ErWU9SQU12Q3BVdWUvN0ZtUWVCRjdBNm1TczVnZHVoWHBM?=
 =?utf-8?B?RnVIb0dMU2N1SFp2ekNudFVZZ25RZUdxSEdOb09mQ3NJZTMyTlFNemtpVnE5?=
 =?utf-8?B?T2dST2M5U3hXUkFlbW9tbTRpQnA0L0JCRlpuU21RVis1cXJLQ2FkU1V0NGJh?=
 =?utf-8?B?Q2c2bTVQb3dMcEFuTkoyVlFUeENuK3pVaXhRVVNtSGJyQmRBVGJMazdSYmts?=
 =?utf-8?B?V1c4NVp0Q3p3ZkxWWk5wUDAzUk13UmY4TTVQd29BaE4zajVaN2o0VEFyZlpT?=
 =?utf-8?B?L25RM2pnZWg1VUgvZFNxeUhGNWZhcXpQQzk1VUpLM1JkaThqRFROVXR5UXNT?=
 =?utf-8?B?bWk0dktvSFVZOFp4cFBPVGZlcWxtcFkycXNqWnhpa3g1cjZ0bGNTMnFXUlAv?=
 =?utf-8?B?RTl2NldPRFBiaVN0RmVtbTZxTnhHY012RlAvbUdlYzY4SlJCZThMSU8xd1hE?=
 =?utf-8?B?UGZ6R2lCYzVTQ1lSdExPTk9uUlloM3JrQ29YZ3VnZ1lORGEwcWNDM1VIZXZj?=
 =?utf-8?B?bkZ2R2JTNFNlaW1WUGdSUjZiejBqRW1maEcyYnZQMlZEemQxOU5UdSszV214?=
 =?utf-8?B?bW5pS052MElYTFBMUTR1QS9Zb0Q2Wm5EMi9OSk1VckdrUW9VOHFwOXdqSWZ3?=
 =?utf-8?B?dmZscTFUV1pKTURyWndDYkJOUDNITXNja1M4ay8wL2taRkpyaE9KSVhPcHVq?=
 =?utf-8?B?ejJ5NlUvOTRDTXJ4STZ3M2dNa3dxaTJ3SnVvWERJTjI0dmVXK1BvZkkxdGFM?=
 =?utf-8?B?UWZUVkdVNnozWjBCVzlvUlo3MjcwTUtvMURDeUJ3aEN4WG5CMk9US1dqWnZP?=
 =?utf-8?B?Y0cwQWV3eGQrMVk3aXBaemY4UjExUGZyL2Izd3hhTG5BUmowM21VU1JmVVFD?=
 =?utf-8?B?UWZEUU5uRU4vVlo0MHgvanBZZS83QTE1MGcyUnllUzBleWxuS1kvMjlySGVV?=
 =?utf-8?B?RkRhZE40VUpFd2wrYlNRRmRmNDUvdmZSRGsraXhqeXRRQnc5Zk9lNW5wUWp3?=
 =?utf-8?B?SktzWnhJV0JBSVIxdFNjTVphUG5VTFRzSlluNFFnOWw4ZzhIUFVGY01rdTZu?=
 =?utf-8?B?QkVtRy9MWFRHUlcwUE16L2p4ZVdwcnFBc2JUNndLOU1jYTRhNGthbDJBaXFh?=
 =?utf-8?B?MWtZN3NHSkduWHVzL1U2VlQ5cEk1aW9ZTDMvdEVxVFB1aEd1M1R0UmlCcUl6?=
 =?utf-8?B?R3dkc0pQN2IwY0kvYUcva0ZJalprb0QwY2J0dGRRNEM5bFhNaUQrcEhxWHhj?=
 =?utf-8?B?ZVhaTENEQXd6MXRPVDZmaG5hdVNtQVdmYVpaY1dOdC92clhUWWdPYVF6ejN0?=
 =?utf-8?B?VitYcjZEdzFuK0Q0UDJZVU9oc0JmMW5LSjRsd2lpWXlIcVZjR0JxTW9yc3Jp?=
 =?utf-8?B?UzNjTjJNRUlueVpUVGdZaVVST3JsRjlBR1VSNnVIWUgvaHFMMU1XeWpSVlh2?=
 =?utf-8?B?OCtqODJxYzJPLzlPc01kdCs0U3JCYnNjYUlDQ3pRMUNjRDBScEhacHJmcEdo?=
 =?utf-8?B?ZDI5RTdQQmhsdkc5OWppRkhwZ25HTTV1eTY4M1FUSnFidW1QUUhPd1pVMlk2?=
 =?utf-8?B?QXZiNWZDd21iMUg3cHdqRjF3ZmxRSy9yYmlBNThEemlNNVlKMnN3VGRuRTNZ?=
 =?utf-8?B?R01OaldRUmFrWWZyTkRVYnM2QVA0Y1d6aVl0ZjlNQWptdGNIdlVhVml3NWxV?=
 =?utf-8?B?V3ZUMnc3Z1pTbmUrbXRmS2F0aE1HSWFUK1FxbWc3MklhbUc0T0FBNnJFM1ov?=
 =?utf-8?B?QmJGUTZpazBJS0tKOElJT2dzYmFyWWU1UTQ1ZnFFb0hSeUVlajczY1V4WE9k?=
 =?utf-8?B?bWs1ckdqZDczNEpGM3dxNGlWaktuS3c0UXhuR3FML3BtY2ZHWm1GSlJXNDdX?=
 =?utf-8?B?a21qNEJhZ21XSzBPNlczaGpyelVkT0ZmTWxMT2R0SGlMUXJzcmp0eTZxLzdN?=
 =?utf-8?B?S2NJNXdlN2c5VWxvU3doamZCRjYvUkhqMkVleHFocXU5Uk5BbW1vRE1DRHg4?=
 =?utf-8?Q?3TmjvqBBVEclyOe+mIUfQd9ow?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8e460f7b-f3be-4080-921a-08dafc7f2d11
X-MS-Exchange-CrossTenant-AuthSource: DM8PR12MB5400.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Jan 2023 13:47:17.6487
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: cs4KAtReTbgR+o5iZKOHXLphsvxbHhp5B7ADcsNTmKa5F9mKYqvxNnfXyR+Hk2Vb
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR12MB6274
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 22/01/2023 12:05, Laurent Vivier wrote:
> In virtnet_probe(), if the device doesn't provide a MAC address the
> driver assigns a random one.
> As we modify the MAC address we need to notify the device to allow it
> to update all the related information.
>
> The problem can be seen with vDPA and mlx5_vdpa driver as it doesn't
> assign a MAC address by default. The virtio_net device uses a random
> MAC address (we can see it with "ip link"), but we can't ping a net
> namespace from another one using the virtio-vdpa device because the
> new MAC address has not been provided to the hardware.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   drivers/net/virtio_net.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 7723b2a49d8e..25511a86590e 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3800,6 +3800,8 @@ static int virtnet_probe(struct virtio_device *vdev)
>   		eth_hw_addr_set(dev, addr);
>   	} else {
>   		eth_hw_addr_random(dev);
> +		dev_info(&vdev->dev, "Assigned random MAC address %pM\n",
> +			 dev->dev_addr);
>   	}
>   
>   	/* Set up our device-specific information */
> @@ -3956,6 +3958,18 @@ static int virtnet_probe(struct virtio_device *vdev)
>   	pr_debug("virtnet: registered device %s with %d RX and TX vq's\n",
>   		 dev->name, max_queue_pairs);
>   
> +	/* a random MAC address has been assigned, notify the device */
> +	if (dev->addr_assign_type == NET_ADDR_RANDOM &&
Maybe it's better to not count on addr_assign_type and use a local 
variable to indicate that virtnet_probe assigned random MAC. The reason 
is that the hardware driver might have done that as well and does not 
need notification.
> +	    virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
> +		struct scatterlist sg;
> +
> +		sg_init_one(&sg, dev->dev_addr, dev->addr_len);
> +		if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
> +					  VIRTIO_NET_CTRL_MAC_ADDR_SET, &sg)) {
> +			dev_warn(&vdev->dev, "Failed to update MAC address.\n");
> +		}
> +	}
> +
>   	return 0;
>   
>   free_unregister_netdev: