[dpdk-dev] [PATCH 11/40] net/virtio: validate features at bus level

Maxime Coquelin maxime.coquelin at redhat.com
Fri Jan 15 10:20:20 CET 2021



On 1/6/21 10:33 AM, David Marchand wrote:
> On Sun, Dec 20, 2020 at 10:15 PM Maxime Coquelin
> <maxime.coquelin at redhat.com> wrote:
>> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
>> index 00aa38e4ef..91a93b2b6e 100644
>> --- a/drivers/net/virtio/virtio_ethdev.c
>> +++ b/drivers/net/virtio/virtio_ethdev.c
>> @@ -1315,17 +1315,16 @@ virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features)
>>         PMD_INIT_LOG(DEBUG, "features after negotiate = %" PRIx64,
>>                 hw->guest_features);
>>
>> -       if (hw->bus_type == VIRTIO_BUS_PCI_MODERN && !vtpci_with_feature(hw, VIRTIO_F_VERSION_1)) {
>> -               PMD_INIT_LOG(ERR,
>> -                       "VIRTIO_F_VERSION_1 features is not enabled.");
>> +       if (VTPCI_OPS(hw)->features_ok(hw) < 0) {
>> +               PMD_INIT_LOG(ERR, "Features not OK at bus level\n");
> 
> We have a log which gives more context in the modern features_ok() callback.
> I don't think we need both log messages.

Yes, maybe overkill. I will remove.

Thanks,
Maxime

>>                 return -1;
>>         }
>>
>> -       if (hw->bus_type == VIRTIO_BUS_PCI_MODERN || hw->bus_type == VIRTIO_BUS_USER) {
>> +       if (vtpci_with_feature(hw, VIRTIO_F_VERSION_1)) {
>>                 vtpci_set_status(hw, VIRTIO_CONFIG_STATUS_FEATURES_OK);
>> +
>>                 if (!(vtpci_get_status(hw) & VIRTIO_CONFIG_STATUS_FEATURES_OK)) {
>> -                       PMD_INIT_LOG(ERR,
>> -                               "failed to set FEATURES_OK status!");
>> +                       PMD_INIT_LOG(ERR, "Failed to set FEATURES_OK status!");
>>                         return -1;
>>                 }
>>         }
>> diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
>> index 599d8afa6b..3de7980b4f 100644
>> --- a/drivers/net/virtio/virtio_pci.c
>> +++ b/drivers/net/virtio/virtio_pci.c
> 
> [snip]
> 
> 
>> @@ -332,6 +339,17 @@ modern_set_features(struct virtio_hw *hw, uint64_t features)
>>                     &hw->common_cfg->guest_feature);
>>  }
>>
>> +static int
>> +modern_features_ok(struct virtio_hw *hw)
>> +{
>> +       if (!vtpci_with_feature(hw, VIRTIO_F_VERSION_1)) {
>> +               PMD_INIT_LOG(ERR, "Version 1+ required with modern devices\n");
>> +               return -1;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>>  static uint8_t
>>  modern_get_status(struct virtio_hw *hw)
>>  {
> 
> 



More information about the dev mailing list