| Message ID | 20230419134329.346825-2-maxime.coquelin@redhat.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | vduse: add support for networking devices | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Wed, Apr 19, 2023 at 9:43 PM Maxime Coquelin <maxime.coquelin@redhat.com> wrote: > > This patch is preliminary work to enable network device > type support to VDUSE. > > As VIRTIO_BLK_F_CONFIG_WCE shares the same value as > VIRTIO_NET_F_HOST_TSO4, we need to restrict its check > to Virtio-blk device type. > > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> > --- > drivers/vdpa/vdpa_user/vduse_dev.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c > index 0c3b48616a9f..6fa598a03d8e 100644 > --- a/drivers/vdpa/vdpa_user/vduse_dev.c > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c > @@ -1416,13 +1416,14 @@ static bool device_is_allowed(u32 device_id) > return false; > } > > -static bool features_is_valid(u64 features) > +static bool features_is_valid(struct vduse_dev_config *config) > { > - if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) > + if (!(config->features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) > return false; > > /* Now we only support read-only configuration space */ > - if (features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE)) > + if ((config->device_id == VIRTIO_ID_BLOCK) && > + (config->features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE))) The reason we filter WCE out is to avoid writable config space which might block the driver with a buggy userspace. For networking, I guess we should fail if VERSION_1 is not negotiated, then we can avoid setting mac addresses via the config space. Thanks > return false; > > return true; > @@ -1446,7 +1447,7 @@ static bool vduse_validate_config(struct vduse_dev_config *config) > if (!device_is_allowed(config->device_id)) > return false; > > - if (!features_is_valid(config->features)) > + if (!features_is_valid(config)) > return false; > > return true; > -- > 2.39.2 >
On 4/20/23 06:06, Jason Wang wrote: > On Wed, Apr 19, 2023 at 9:43 PM Maxime Coquelin > <maxime.coquelin@redhat.com> wrote: >> >> This patch is preliminary work to enable network device >> type support to VDUSE. >> >> As VIRTIO_BLK_F_CONFIG_WCE shares the same value as >> VIRTIO_NET_F_HOST_TSO4, we need to restrict its check >> to Virtio-blk device type. >> >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> >> --- >> drivers/vdpa/vdpa_user/vduse_dev.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c >> index 0c3b48616a9f..6fa598a03d8e 100644 >> --- a/drivers/vdpa/vdpa_user/vduse_dev.c >> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c >> @@ -1416,13 +1416,14 @@ static bool device_is_allowed(u32 device_id) >> return false; >> } >> >> -static bool features_is_valid(u64 features) >> +static bool features_is_valid(struct vduse_dev_config *config) >> { >> - if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) >> + if (!(config->features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) >> return false; >> >> /* Now we only support read-only configuration space */ >> - if (features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE)) >> + if ((config->device_id == VIRTIO_ID_BLOCK) && >> + (config->features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE))) > > The reason we filter WCE out is to avoid writable config space which > might block the driver with a buggy userspace. > > For networking, I guess we should fail if VERSION_1 is not negotiated, > then we can avoid setting mac addresses via the config space. Ok, I will add it to patch 2 in V1. Thanks, Maxime > > Thanks > >> return false; >> >> return true; >> @@ -1446,7 +1447,7 @@ static bool vduse_validate_config(struct vduse_dev_config *config) >> if (!device_is_allowed(config->device_id)) >> return false; >> >> - if (!features_is_valid(config->features)) >> + if (!features_is_valid(config)) >> return false; >> >> return true; >> -- >> 2.39.2 >> >
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c index 0c3b48616a9f..6fa598a03d8e 100644 --- a/drivers/vdpa/vdpa_user/vduse_dev.c +++ b/drivers/vdpa/vdpa_user/vduse_dev.c @@ -1416,13 +1416,14 @@ static bool device_is_allowed(u32 device_id) return false; } -static bool features_is_valid(u64 features) +static bool features_is_valid(struct vduse_dev_config *config) { - if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) + if (!(config->features & (1ULL << VIRTIO_F_ACCESS_PLATFORM))) return false; /* Now we only support read-only configuration space */ - if (features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE)) + if ((config->device_id == VIRTIO_ID_BLOCK) && + (config->features & (1ULL << VIRTIO_BLK_F_CONFIG_WCE))) return false; return true; @@ -1446,7 +1447,7 @@ static bool vduse_validate_config(struct vduse_dev_config *config) if (!device_is_allowed(config->device_id)) return false; - if (!features_is_valid(config->features)) + if (!features_is_valid(config)) return false; return true;
This patch is preliminary work to enable network device type support to VDUSE. As VIRTIO_BLK_F_CONFIG_WCE shares the same value as VIRTIO_NET_F_HOST_TSO4, we need to restrict its check to Virtio-blk device type. Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- drivers/vdpa/vdpa_user/vduse_dev.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)