netdev - Re: [PATCH virtio 1/4] pds_vdpa: reset to vdpa specified mac

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <82bf6210-6615-5d33-e3c5-808cfa8cba6d@amd.com>
Date: Mon, 10 Jul 2023 08:42:19 -0700
From: Shannon Nelson <shannon.nelson@....com>
To: Jason Wang <jasowang@...hat.com>
Cc: mst@...hat.com, virtualization@...ts.linux-foundation.org,
 brett.creeley@....com, netdev@...r.kernel.org, drivers@...sando.io,
 Allen Hubbe <allen.hubbe@....com>
Subject: Re: [PATCH virtio 1/4] pds_vdpa: reset to vdpa specified mac

On 7/9/23 8:04 PM, Jason Wang wrote:
> 
> On Sat, Jul 8, 2023 at 4:12 AM Shannon Nelson <shannon.nelson@....com> wrote:
>>
>>
>>
>> On 7/7/23 12:33 AM, Jason Wang wrote:
>>> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>>>
>>>
>>> On Fri, Jun 30, 2023 at 8:36 AM Shannon Nelson <shannon.nelson@....com> wrote:
>>>>
>>>> From: Allen Hubbe <allen.hubbe@....com>
>>>>
>>>> When the vdpa device is reset, also reinitialize it with the mac address
>>>> that was assigned when the device was added.
>>>>
>>>> Fixes: 151cc834f3dd ("pds_vdpa: add support for vdpa and vdpamgmt interfaces")
>>>> Signed-off-by: Allen Hubbe <allen.hubbe@....com>
>>>> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
>>>> Reviewed-by: Brett Creeley <brett.creeley@....com>
>>>> ---
>>>>    drivers/vdpa/pds/vdpa_dev.c | 16 ++++++++--------
>>>>    drivers/vdpa/pds/vdpa_dev.h |  1 +
>>>>    2 files changed, 9 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
>>>> index 5071a4d58f8d..e2e99bb0be2b 100644
>>>> --- a/drivers/vdpa/pds/vdpa_dev.c
>>>> +++ b/drivers/vdpa/pds/vdpa_dev.c
>>>> @@ -409,6 +409,8 @@ static void pds_vdpa_set_status(struct vdpa_device *vdpa_dev, u8 status)
>>>>                           pdsv->vqs[i].avail_idx = 0;
>>>>                           pdsv->vqs[i].used_idx = 0;
>>>>                   }
>>>> +
>>>> +               pds_vdpa_cmd_set_mac(pdsv, pdsv->mac);
>>>
>>> So this is not necessarily called during reset. So I think we need to
>>> move it to pds_vdpa_reset()?
>>
>> pds_vdpa_reset() calls pds_vdpa_set_status() with a status=0, so this is
>> already covered.
> 
> Yes, but pds_vdpa_set_status() will be called when status is not zero?

Yes, but the set_mac is only done with status==0, whether called by 
reset or called when some other thing calls set_status with status==0.

sln


> 
> Thanks
> 
>>
>> sln
>>
>>>
>>> The rest looks good.
>>>
>>> Thanks
>>>
>>>>           }
>>>>
>>>>           if (status & ~old_status & VIRTIO_CONFIG_S_FEATURES_OK) {
>>>> @@ -532,7 +534,6 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>>>>           struct device *dma_dev;
>>>>           struct pci_dev *pdev;
>>>>           struct device *dev;
>>>> -       u8 mac[ETH_ALEN];
>>>>           int err;
>>>>           int i;
>>>>
>>>> @@ -617,19 +618,18 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>>>>            * or set a random mac if default is 00:..:00
>>>>            */
>>>>           if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR)) {
>>>> -               ether_addr_copy(mac, add_config->net.mac);
>>>> -               pds_vdpa_cmd_set_mac(pdsv, mac);
>>>> +               ether_addr_copy(pdsv->mac, add_config->net.mac);
>>>>           } else {
>>>>                   struct virtio_net_config __iomem *vc;
>>>>
>>>>                   vc = pdsv->vdpa_aux->vd_mdev.device;
>>>> -               memcpy_fromio(mac, vc->mac, sizeof(mac));
>>>> -               if (is_zero_ether_addr(mac)) {
>>>> -                       eth_random_addr(mac);
>>>> -                       dev_info(dev, "setting random mac %pM\n", mac);
>>>> -                       pds_vdpa_cmd_set_mac(pdsv, mac);
>>>> +               memcpy_fromio(pdsv->mac, vc->mac, sizeof(pdsv->mac));
>>>> +               if (is_zero_ether_addr(pdsv->mac)) {
>>>> +                       eth_random_addr(pdsv->mac);
>>>> +                       dev_info(dev, "setting random mac %pM\n", pdsv->mac);
>>>>                   }
>>>>           }
>>>> +       pds_vdpa_cmd_set_mac(pdsv, pdsv->mac);
>>>>
>>>>           for (i = 0; i < pdsv->num_vqs; i++) {
>>>>                   pdsv->vqs[i].qid = i;
>>>> diff --git a/drivers/vdpa/pds/vdpa_dev.h b/drivers/vdpa/pds/vdpa_dev.h
>>>> index a1bc37de9537..cf02df287fc4 100644
>>>> --- a/drivers/vdpa/pds/vdpa_dev.h
>>>> +++ b/drivers/vdpa/pds/vdpa_dev.h
>>>> @@ -39,6 +39,7 @@ struct pds_vdpa_device {
>>>>           u64 req_features;               /* features requested by vdpa */
>>>>           u8 vdpa_index;                  /* rsvd for future subdevice use */
>>>>           u8 num_vqs;                     /* num vqs in use */
>>>> +       u8 mac[ETH_ALEN];               /* mac selected when the device was added */
>>>>           struct vdpa_callback config_cb;
>>>>           struct notifier_block nb;
>>>>    };
>>>> --
>>>> 2.17.1
>>>>
>>>
>>
>