| 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
| ||
|
Message-ID: <CACGkMEsOK24Hi-qEkTHzM55tye12cp3Xu2i9fyz--L=kYZCE-g@mail.gmail.com>
Date: Thu, 13 Jul 2023 13:07:55 +0800
From: Jason Wang <jasowang@...hat.com>
To: Shannon Nelson <shannon.nelson@....com>
Cc: mst@...hat.com, virtualization@...ts.linux-foundation.org,
brett.creeley@....com, netdev@...r.kernel.org, drivers@...sando.io
Subject: Re: [PATCH v2 virtio 2/5] pds_vdpa: always allow offering VIRTIO_NET_F_MAC
On Tue, Jul 11, 2023 at 12:25 PM Shannon Nelson <shannon.nelson@....com> wrote:
>
> Our driver sets a mac if the HW is 00:..:00 so we need to be sure to
> advertise VIRTIO_NET_F_MAC even if the HW doesn't. We also need to be
> sure that virtio_net sees the VIRTIO_NET_F_MAC and doesn't rewrite the
> mac address that a user may have set with the vdpa utility.
>
> After reading the hw_feature bits, add the VIRTIO_NET_F_MAC to the driver's
> supported_features and use that for reporting what is available. If the
> HW is not advertising it, be sure to strip the VIRTIO_NET_F_MAC before
> finishing the feature negotiation. If the user specifies a device_features
> bitpattern in the vdpa utility without the VIRTIO_NET_F_MAC set, then
> don't set the mac.
>
> Fixes: 151cc834f3dd ("pds_vdpa: add support for vdpa and vdpamgmt interfaces")
> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
Acked-by: Jason Wang <jasowang@...hat.com>
Thanks
> ---
> drivers/vdpa/pds/debugfs.c | 2 --
> drivers/vdpa/pds/vdpa_dev.c | 30 +++++++++++++++++++++---------
> drivers/vdpa/pds/vdpa_dev.h | 4 ++--
> 3 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/vdpa/pds/debugfs.c b/drivers/vdpa/pds/debugfs.c
> index 21a0dc0cb607..754ccb7a6666 100644
> --- a/drivers/vdpa/pds/debugfs.c
> +++ b/drivers/vdpa/pds/debugfs.c
> @@ -224,8 +224,6 @@ static int config_show(struct seq_file *seq, void *v)
> seq_printf(seq, "dev_status: %#x\n", status);
> print_status_bits(seq, status);
>
> - seq_printf(seq, "req_features: %#llx\n", pdsv->req_features);
> - print_feature_bits_all(seq, pdsv->req_features);
> driver_features = vp_modern_get_driver_features(&pdsv->vdpa_aux->vd_mdev);
> seq_printf(seq, "driver_features: %#llx\n", driver_features);
> print_feature_bits_all(seq, driver_features);
> diff --git a/drivers/vdpa/pds/vdpa_dev.c b/drivers/vdpa/pds/vdpa_dev.c
> index e2e99bb0be2b..5b566e0eef0a 100644
> --- a/drivers/vdpa/pds/vdpa_dev.c
> +++ b/drivers/vdpa/pds/vdpa_dev.c
> @@ -318,6 +318,7 @@ static int pds_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 featur
> struct device *dev = &pdsv->vdpa_dev.dev;
> u64 driver_features;
> u64 nego_features;
> + u64 hw_features;
> u64 missing;
>
> if (!(features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM)) && features) {
> @@ -325,21 +326,26 @@ static int pds_vdpa_set_driver_features(struct vdpa_device *vdpa_dev, u64 featur
> return -EOPNOTSUPP;
> }
>
> - pdsv->req_features = features;
> -
> /* Check for valid feature bits */
> - nego_features = features & le64_to_cpu(pdsv->vdpa_aux->ident.hw_features);
> - missing = pdsv->req_features & ~nego_features;
> + nego_features = features & pdsv->supported_features;
> + missing = features & ~nego_features;
> if (missing) {
> dev_err(dev, "Can't support all requested features in %#llx, missing %#llx features\n",
> - pdsv->req_features, missing);
> + features, missing);
> return -EOPNOTSUPP;
> }
>
> + pdsv->negotiated_features = nego_features;
> +
> driver_features = pds_vdpa_get_driver_features(vdpa_dev);
> dev_dbg(dev, "%s: %#llx => %#llx\n",
> __func__, driver_features, nego_features);
>
> + /* if we're faking the F_MAC, strip it before writing to device */
> + hw_features = le64_to_cpu(pdsv->vdpa_aux->ident.hw_features);
> + if (!(hw_features & BIT_ULL(VIRTIO_NET_F_MAC)))
> + nego_features &= ~BIT_ULL(VIRTIO_NET_F_MAC);
> +
> if (driver_features == nego_features)
> return 0;
>
> @@ -352,7 +358,7 @@ static u64 pds_vdpa_get_driver_features(struct vdpa_device *vdpa_dev)
> {
> struct pds_vdpa_device *pdsv = vdpa_to_pdsv(vdpa_dev);
>
> - return vp_modern_get_driver_features(&pdsv->vdpa_aux->vd_mdev);
> + return pdsv->negotiated_features;
> }
>
> static void pds_vdpa_set_config_cb(struct vdpa_device *vdpa_dev,
> @@ -564,7 +570,7 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>
> if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) {
> u64 unsupp_features =
> - add_config->device_features & ~mgmt->supported_features;
> + add_config->device_features & ~pdsv->supported_features;
>
> if (unsupp_features) {
> dev_err(dev, "Unsupported features: %#llx\n", unsupp_features);
> @@ -615,7 +621,8 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
> }
>
> /* Set a mac, either from the user config if provided
> - * or set a random mac if default is 00:..:00
> + * or use the device's mac if not 00:..:00
> + * or set a random mac
> */
> if (add_config->mask & BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR)) {
> ether_addr_copy(pdsv->mac, add_config->net.mac);
> @@ -624,7 +631,8 @@ static int pds_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
>
> vc = pdsv->vdpa_aux->vd_mdev.device;
> memcpy_fromio(pdsv->mac, vc->mac, sizeof(pdsv->mac));
> - if (is_zero_ether_addr(pdsv->mac)) {
> + if (is_zero_ether_addr(pdsv->mac) &&
> + (pdsv->supported_features & BIT_ULL(VIRTIO_NET_F_MAC))) {
> eth_random_addr(pdsv->mac);
> dev_info(dev, "setting random mac %pM\n", pdsv->mac);
> }
> @@ -752,6 +760,10 @@ int pds_vdpa_get_mgmt_info(struct pds_vdpa_aux *vdpa_aux)
> mgmt->id_table = pds_vdpa_id_table;
> mgmt->device = dev;
> mgmt->supported_features = le64_to_cpu(vdpa_aux->ident.hw_features);
> +
> + /* advertise F_MAC even if the device doesn't */
> + mgmt->supported_features |= BIT_ULL(VIRTIO_NET_F_MAC);
> +
> mgmt->config_attr_mask = BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MACADDR);
> mgmt->config_attr_mask |= BIT_ULL(VDPA_ATTR_DEV_NET_CFG_MAX_VQP);
> mgmt->config_attr_mask |= BIT_ULL(VDPA_ATTR_DEV_FEATURES);
> diff --git a/drivers/vdpa/pds/vdpa_dev.h b/drivers/vdpa/pds/vdpa_dev.h
> index cf02df287fc4..d984ba24a7da 100644
> --- a/drivers/vdpa/pds/vdpa_dev.h
> +++ b/drivers/vdpa/pds/vdpa_dev.h
> @@ -35,8 +35,8 @@ struct pds_vdpa_device {
> struct pds_vdpa_aux *vdpa_aux;
>
> struct pds_vdpa_vq_info vqs[PDS_VDPA_MAX_QUEUES];
> - u64 supported_features; /* specified device features */
> - u64 req_features; /* features requested by vdpa */
> + u64 supported_features; /* supported device features */
> + u64 negotiated_features; /* negotiated features */
> 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 */
> --
> 2.17.1
>
Powered by blists - more mailing lists