[18/40] net/virtio: move virtqueue defines in generic header

Message ID 20201220211405.313012-19-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series net/virtio: Virtio PMD rework |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Maxime Coquelin Dec. 20, 2020, 9:13 p.m. UTC
  This patch moves the virtqueues defines from PCI header
to the genreric one.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio.h                    | 18 ++++++++++++++++++
 drivers/net/virtio/virtio_ethdev.h             |  3 ++-
 drivers/net/virtio/virtio_pci.h                | 17 -----------------
 .../net/virtio/virtio_user/virtio_user_dev.h   |  2 +-
 4 files changed, 21 insertions(+), 19 deletions(-)
  

Comments

Chenbo Xia Dec. 30, 2020, 3:14 a.m. UTC | #1
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Monday, December 21, 2020 5:14 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; olivier.matz@6wind.com;
> amorenoz@redhat.com; david.marchand@redhat.com
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [PATCH 18/40] net/virtio: move virtqueue defines in generic header
> 
> This patch moves the virtqueues defines from PCI header
> to the genreric one.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/net/virtio/virtio.h                    | 18 ++++++++++++++++++
>  drivers/net/virtio/virtio_ethdev.h             |  3 ++-
>  drivers/net/virtio/virtio_pci.h                | 17 -----------------
>  .../net/virtio/virtio_user/virtio_user_dev.h   |  2 +-
>  4 files changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio.h b/drivers/net/virtio/virtio.h
> index 9e787803a4..eeeb5dba4f 100644
> --- a/drivers/net/virtio/virtio.h
> +++ b/drivers/net/virtio/virtio.h
> @@ -88,6 +88,24 @@
>  #define VIRTIO_NET_S_LINK_UP	1	/* Link is up */
>  #define VIRTIO_NET_S_ANNOUNCE	2	/* Announcement is needed */
> 
> +/*
> + * Each virtqueue indirect descriptor list must be physically contiguous.
> + * To allow us to malloc(9) each list individually, limit the number
> + * supported to what will fit in one page. With 4KB pages, this is a limit
> + * of 256 descriptors. If there is ever a need for more, we can switch to
> + * contigmalloc(9) for the larger allocations, similar to what
> + * bus_dmamem_alloc(9) does.
> + *
> + * Note the sizeof(struct vring_desc) is 16 bytes.
> + */
> +#define VIRTIO_MAX_INDIRECT ((int)(PAGE_SIZE / 16))
> +
> +/*
> + * Maximum number of virtqueues per device.
> + */
> +#define VIRTIO_MAX_VIRTQUEUE_PAIRS 8
> +#define VIRTIO_MAX_VIRTQUEUES (VIRTIO_MAX_VIRTQUEUE_PAIRS * 2 + 1)
> +
>  struct virtio_hw {
>  	struct virtqueue **vqs;
>  	uint64_t guest_features;
> diff --git a/drivers/net/virtio/virtio_ethdev.h
> b/drivers/net/virtio/virtio_ethdev.h
> index 13395937c8..6fc373f484 100644
> --- a/drivers/net/virtio/virtio_ethdev.h
> +++ b/drivers/net/virtio/virtio_ethdev.h
> @@ -7,7 +7,8 @@
> 
>  #include <stdint.h>
> 
> -#include "virtio_pci.h"
> +#include "virtio.h"
> +#include <rte_ethdev_driver.h>
> 
>  #ifndef PAGE_SIZE
>  #define PAGE_SIZE 4096
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index b02e5c15f5..249f9754cc 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -67,23 +67,6 @@ struct virtnet_ctl;
>  #define VIRTIO_CONFIG_STATUS_DEV_NEED_RESET	0x40
>  #define VIRTIO_CONFIG_STATUS_FAILED		0x80
> 
> -/*
> - * Each virtqueue indirect descriptor list must be physically contiguous.
> - * To allow us to malloc(9) each list individually, limit the number
> - * supported to what will fit in one page. With 4KB pages, this is a limit
> - * of 256 descriptors. If there is ever a need for more, we can switch to
> - * contigmalloc(9) for the larger allocations, similar to what
> - * bus_dmamem_alloc(9) does.
> - *
> - * Note the sizeof(struct vring_desc) is 16 bytes.
> - */
> -#define VIRTIO_MAX_INDIRECT ((int) (PAGE_SIZE / 16))
> -
> -/*
> - * Maximum number of virtqueues per device.
> - */
> -#define VIRTIO_MAX_VIRTQUEUE_PAIRS 8
> -#define VIRTIO_MAX_VIRTQUEUES (VIRTIO_MAX_VIRTQUEUE_PAIRS * 2 + 1)
> 
>  /* Common configuration */
>  #define VIRTIO_PCI_CAP_COMMON_CFG	1
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index 59f4dd1f24..0eb481ae21 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -7,7 +7,7 @@
> 
>  #include <limits.h>
>  #include <stdbool.h>
> -#include "../virtio_pci.h"
> +#include "../virtio.h"
>  #include "../virtio_ring.h"
> 
>  enum virtio_user_backend_type {
> --
> 2.29.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
  
David Marchand Jan. 6, 2021, 3:53 p.m. UTC | #2
On Sun, Dec 20, 2020 at 10:15 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> This patch moves the virtqueues defines from PCI header
> to the genreric one.


generic*

>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/net/virtio/virtio.h                    | 18 ++++++++++++++++++
>  drivers/net/virtio/virtio_ethdev.h             |  3 ++-
>  drivers/net/virtio/virtio_pci.h                | 17 -----------------
>  .../net/virtio/virtio_user/virtio_user_dev.h   |  2 +-
>  4 files changed, 21 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio.h b/drivers/net/virtio/virtio.h
> index 9e787803a4..eeeb5dba4f 100644
> --- a/drivers/net/virtio/virtio.h
> +++ b/drivers/net/virtio/virtio.h
> @@ -88,6 +88,24 @@
>  #define VIRTIO_NET_S_LINK_UP   1       /* Link is up */
>  #define VIRTIO_NET_S_ANNOUNCE  2       /* Announcement is needed */
>
> +/*
> + * Each virtqueue indirect descriptor list must be physically contiguous.
> + * To allow us to malloc(9) each list individually, limit the number
> + * supported to what will fit in one page. With 4KB pages, this is a limit
> + * of 256 descriptors. If there is ever a need for more, we can switch to
> + * contigmalloc(9) for the larger allocations, similar to what
> + * bus_dmamem_alloc(9) does.
> + *
> + * Note the sizeof(struct vring_desc) is 16 bytes.
> + */
> +#define VIRTIO_MAX_INDIRECT ((int)(PAGE_SIZE / 16))
> +
> +/*
> + * Maximum number of virtqueues per device.
> + */
> +#define VIRTIO_MAX_VIRTQUEUE_PAIRS 8
> +#define VIRTIO_MAX_VIRTQUEUES (VIRTIO_MAX_VIRTQUEUE_PAIRS * 2 + 1)
> +
>  struct virtio_hw {
>         struct virtqueue **vqs;
>         uint64_t guest_features;
> diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
> index 13395937c8..6fc373f484 100644
> --- a/drivers/net/virtio/virtio_ethdev.h
> +++ b/drivers/net/virtio/virtio_ethdev.h
> @@ -7,7 +7,8 @@
>
>  #include <stdint.h>
>
> -#include "virtio_pci.h"
> +#include "virtio.h"
> +#include <rte_ethdev_driver.h>

The coding style recommends group headers inclusion: first system
headers, then dpdk shared headers and finally "local" headers.
https://doc.dpdk.org/guides/contributing/coding_style.html#header-includes


>
>  #ifndef PAGE_SIZE
>  #define PAGE_SIZE 4096
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index b02e5c15f5..249f9754cc 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -67,23 +67,6 @@ struct virtnet_ctl;
>  #define VIRTIO_CONFIG_STATUS_DEV_NEED_RESET    0x40
>  #define VIRTIO_CONFIG_STATUS_FAILED            0x80
>
> -/*
> - * Each virtqueue indirect descriptor list must be physically contiguous.
> - * To allow us to malloc(9) each list individually, limit the number
> - * supported to what will fit in one page. With 4KB pages, this is a limit
> - * of 256 descriptors. If there is ever a need for more, we can switch to
> - * contigmalloc(9) for the larger allocations, similar to what
> - * bus_dmamem_alloc(9) does.
> - *
> - * Note the sizeof(struct vring_desc) is 16 bytes.
> - */
> -#define VIRTIO_MAX_INDIRECT ((int) (PAGE_SIZE / 16))
> -
> -/*
> - * Maximum number of virtqueues per device.
> - */
> -#define VIRTIO_MAX_VIRTQUEUE_PAIRS 8
> -#define VIRTIO_MAX_VIRTQUEUES (VIRTIO_MAX_VIRTQUEUE_PAIRS * 2 + 1)
>
>  /* Common configuration */
>  #define VIRTIO_PCI_CAP_COMMON_CFG      1
> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> index 59f4dd1f24..0eb481ae21 100644
> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
> @@ -7,7 +7,7 @@
>
>  #include <limits.h>
>  #include <stdbool.h>

Same comment, for readability, add an empty line here.

> -#include "../virtio_pci.h"
> +#include "../virtio.h"
>  #include "../virtio_ring.h"
>
>  enum virtio_user_backend_type {
> --
> 2.29.2
>
  
Maxime Coquelin Jan. 15, 2021, 10:55 a.m. UTC | #3
On 1/6/21 4:53 PM, David Marchand wrote:
> On Sun, Dec 20, 2020 at 10:15 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> This patch moves the virtqueues defines from PCI header
>> to the genreric one.
> 
> 
> generic*
> 
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>  drivers/net/virtio/virtio.h                    | 18 ++++++++++++++++++
>>  drivers/net/virtio/virtio_ethdev.h             |  3 ++-
>>  drivers/net/virtio/virtio_pci.h                | 17 -----------------
>>  .../net/virtio/virtio_user/virtio_user_dev.h   |  2 +-
>>  4 files changed, 21 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/net/virtio/virtio.h b/drivers/net/virtio/virtio.h
>> index 9e787803a4..eeeb5dba4f 100644
>> --- a/drivers/net/virtio/virtio.h
>> +++ b/drivers/net/virtio/virtio.h
>> @@ -88,6 +88,24 @@
>>  #define VIRTIO_NET_S_LINK_UP   1       /* Link is up */
>>  #define VIRTIO_NET_S_ANNOUNCE  2       /* Announcement is needed */
>>
>> +/*
>> + * Each virtqueue indirect descriptor list must be physically contiguous.
>> + * To allow us to malloc(9) each list individually, limit the number
>> + * supported to what will fit in one page. With 4KB pages, this is a limit
>> + * of 256 descriptors. If there is ever a need for more, we can switch to
>> + * contigmalloc(9) for the larger allocations, similar to what
>> + * bus_dmamem_alloc(9) does.
>> + *
>> + * Note the sizeof(struct vring_desc) is 16 bytes.
>> + */
>> +#define VIRTIO_MAX_INDIRECT ((int)(PAGE_SIZE / 16))
>> +
>> +/*
>> + * Maximum number of virtqueues per device.
>> + */
>> +#define VIRTIO_MAX_VIRTQUEUE_PAIRS 8
>> +#define VIRTIO_MAX_VIRTQUEUES (VIRTIO_MAX_VIRTQUEUE_PAIRS * 2 + 1)
>> +
>>  struct virtio_hw {
>>         struct virtqueue **vqs;
>>         uint64_t guest_features;
>> diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
>> index 13395937c8..6fc373f484 100644
>> --- a/drivers/net/virtio/virtio_ethdev.h
>> +++ b/drivers/net/virtio/virtio_ethdev.h
>> @@ -7,7 +7,8 @@
>>
>>  #include <stdint.h>
>>
>> -#include "virtio_pci.h"
>> +#include "virtio.h"
>> +#include <rte_ethdev_driver.h>
> 
> The coding style recommends group headers inclusion: first system
> headers, then dpdk shared headers and finally "local" headers.
> https://doc.dpdk.org/guides/contributing/coding_style.html#header-includes
> 
> 
>>
>>  #ifndef PAGE_SIZE
>>  #define PAGE_SIZE 4096
>> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
>> index b02e5c15f5..249f9754cc 100644
>> --- a/drivers/net/virtio/virtio_pci.h
>> +++ b/drivers/net/virtio/virtio_pci.h
>> @@ -67,23 +67,6 @@ struct virtnet_ctl;
>>  #define VIRTIO_CONFIG_STATUS_DEV_NEED_RESET    0x40
>>  #define VIRTIO_CONFIG_STATUS_FAILED            0x80
>>
>> -/*
>> - * Each virtqueue indirect descriptor list must be physically contiguous.
>> - * To allow us to malloc(9) each list individually, limit the number
>> - * supported to what will fit in one page. With 4KB pages, this is a limit
>> - * of 256 descriptors. If there is ever a need for more, we can switch to
>> - * contigmalloc(9) for the larger allocations, similar to what
>> - * bus_dmamem_alloc(9) does.
>> - *
>> - * Note the sizeof(struct vring_desc) is 16 bytes.
>> - */
>> -#define VIRTIO_MAX_INDIRECT ((int) (PAGE_SIZE / 16))
>> -
>> -/*
>> - * Maximum number of virtqueues per device.
>> - */
>> -#define VIRTIO_MAX_VIRTQUEUE_PAIRS 8
>> -#define VIRTIO_MAX_VIRTQUEUES (VIRTIO_MAX_VIRTQUEUE_PAIRS * 2 + 1)
>>
>>  /* Common configuration */
>>  #define VIRTIO_PCI_CAP_COMMON_CFG      1
>> diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>> index 59f4dd1f24..0eb481ae21 100644
>> --- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
>> +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
>> @@ -7,7 +7,7 @@
>>
>>  #include <limits.h>
>>  #include <stdbool.h>
> 
> Same comment, for readability, add an empty line here.

Fixed it, and also above one.

Thanks,
Maxime

>> -#include "../virtio_pci.h"
>> +#include "../virtio.h"
>>  #include "../virtio_ring.h"
>>
>>  enum virtio_user_backend_type {
>> --
>> 2.29.2
>>
> 
> 
>
  

Patch

diff --git a/drivers/net/virtio/virtio.h b/drivers/net/virtio/virtio.h
index 9e787803a4..eeeb5dba4f 100644
--- a/drivers/net/virtio/virtio.h
+++ b/drivers/net/virtio/virtio.h
@@ -88,6 +88,24 @@ 
 #define VIRTIO_NET_S_LINK_UP	1	/* Link is up */
 #define VIRTIO_NET_S_ANNOUNCE	2	/* Announcement is needed */
 
+/*
+ * Each virtqueue indirect descriptor list must be physically contiguous.
+ * To allow us to malloc(9) each list individually, limit the number
+ * supported to what will fit in one page. With 4KB pages, this is a limit
+ * of 256 descriptors. If there is ever a need for more, we can switch to
+ * contigmalloc(9) for the larger allocations, similar to what
+ * bus_dmamem_alloc(9) does.
+ *
+ * Note the sizeof(struct vring_desc) is 16 bytes.
+ */
+#define VIRTIO_MAX_INDIRECT ((int)(PAGE_SIZE / 16))
+
+/*
+ * Maximum number of virtqueues per device.
+ */
+#define VIRTIO_MAX_VIRTQUEUE_PAIRS 8
+#define VIRTIO_MAX_VIRTQUEUES (VIRTIO_MAX_VIRTQUEUE_PAIRS * 2 + 1)
+
 struct virtio_hw {
 	struct virtqueue **vqs;
 	uint64_t guest_features;
diff --git a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h
index 13395937c8..6fc373f484 100644
--- a/drivers/net/virtio/virtio_ethdev.h
+++ b/drivers/net/virtio/virtio_ethdev.h
@@ -7,7 +7,8 @@ 
 
 #include <stdint.h>
 
-#include "virtio_pci.h"
+#include "virtio.h"
+#include <rte_ethdev_driver.h>
 
 #ifndef PAGE_SIZE
 #define PAGE_SIZE 4096
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index b02e5c15f5..249f9754cc 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -67,23 +67,6 @@  struct virtnet_ctl;
 #define VIRTIO_CONFIG_STATUS_DEV_NEED_RESET	0x40
 #define VIRTIO_CONFIG_STATUS_FAILED		0x80
 
-/*
- * Each virtqueue indirect descriptor list must be physically contiguous.
- * To allow us to malloc(9) each list individually, limit the number
- * supported to what will fit in one page. With 4KB pages, this is a limit
- * of 256 descriptors. If there is ever a need for more, we can switch to
- * contigmalloc(9) for the larger allocations, similar to what
- * bus_dmamem_alloc(9) does.
- *
- * Note the sizeof(struct vring_desc) is 16 bytes.
- */
-#define VIRTIO_MAX_INDIRECT ((int) (PAGE_SIZE / 16))
-
-/*
- * Maximum number of virtqueues per device.
- */
-#define VIRTIO_MAX_VIRTQUEUE_PAIRS 8
-#define VIRTIO_MAX_VIRTQUEUES (VIRTIO_MAX_VIRTQUEUE_PAIRS * 2 + 1)
 
 /* Common configuration */
 #define VIRTIO_PCI_CAP_COMMON_CFG	1
diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.h b/drivers/net/virtio/virtio_user/virtio_user_dev.h
index 59f4dd1f24..0eb481ae21 100644
--- a/drivers/net/virtio/virtio_user/virtio_user_dev.h
+++ b/drivers/net/virtio/virtio_user/virtio_user_dev.h
@@ -7,7 +7,7 @@ 
 
 #include <limits.h>
 #include <stdbool.h>
-#include "../virtio_pci.h"
+#include "../virtio.h"
 #include "../virtio_ring.h"
 
 enum virtio_user_backend_type {