[14/40] net/virtio: pack virtio HW struct

  This patch improves the virtio_hw struct packing,
going from 88 down to 80 bytes with a 6 bytes hole in
the end of the first cacheline. Fields only used in the
slow path are placed in the end, so that hot path only
uses the first cacheline.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 drivers/net/virtio/virtio_pci.h | 45 ++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 23 deletions(-)
  

> -----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 14/40] net/virtio: pack virtio HW struct
> 
> This patch improves the virtio_hw struct packing,
> going from 88 down to 80 bytes with a 6 bytes hole in
> the end of the first cacheline. Fields only used in the
> slow path are placed in the end, so that hot path only
> uses the first cacheline.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/net/virtio/virtio_pci.h | 45 ++++++++++++++++-----------------
>  1 file changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index 5629b37050..15f68f141c 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -246,26 +246,25 @@ struct virtio_pci_ops {
>  struct virtio_net_config;
> 
>  struct virtio_hw {
> -	struct virtnet_ctl *cvq;
> -	uint64_t    req_guest_features;
> -	uint64_t    guest_features;
> -	uint32_t    max_queue_pairs;
> -	bool        started;
> -	uint16_t	max_mtu;
> -	uint16_t    vtnet_hdr_size;
> -	uint8_t	    vlan_strip;
> -	uint8_t	    use_msix;
> -	uint8_t     use_vec_rx;
> -	uint8_t     use_vec_tx;
> -	uint8_t     use_inorder_rx;
> -	uint8_t     use_inorder_tx;
> -	uint8_t     weak_barriers;
> -	bool        has_tx_offload;
> -	bool        has_rx_offload;
> -	uint16_t    port_id;
> -	uint8_t     mac_addr[RTE_ETHER_ADDR_LEN];
> -	uint32_t    speed;  /* link speed in MB */
> -	uint8_t     duplex;
> +	struct virtqueue **vqs;
> +	uint64_t guest_features;
> +	uint16_t vtnet_hdr_size;
> +	uint8_t started;
> +	uint8_t weak_barriers;
> +	uint8_t vlan_strip;
> +	uint8_t has_tx_offload;
> +	uint8_t has_rx_offload;
> +	uint8_t use_vec_rx;
> +	uint8_t use_vec_tx;
> +	uint8_t use_inorder_rx;
> +	uint8_t use_inorder_tx;
> +	uint8_t opened;
> +	uint16_t port_id;
> +	uint8_t mac_addr[RTE_ETHER_ADDR_LEN];
> +	uint32_t speed;  /* link speed in MB */
> +	uint8_t duplex;
> +	uint8_t use_msix;
> +	uint16_t max_mtu;
>  	/*
>  	 * App management thread and virtio interrupt handler thread
>  	 * both can change device state, this lock is meant to avoid
> @@ -273,9 +272,9 @@ struct virtio_hw {
>  	 */
>  	rte_spinlock_t state_lock;
>  	struct rte_mbuf **inject_pkts;
> -	bool        opened;
> -
> -	struct virtqueue **vqs;
> +	uint16_t max_queue_pairs;
> +	uint64_t req_guest_features;
> +	struct virtnet_ctl *cvq;
>  };
> 
>  struct virtio_pci_dev {
> --
> 2.29.2

Reviewed-by: Chenbo Xia <chenbo.xia@intel.com>
  

On Sun, Dec 20, 2020 at 10:15 PM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> This patch improves the virtio_hw struct packing,
> going from 88 down to 80 bytes with a 6 bytes hole in
> the end of the first cacheline. Fields only used in the
> slow path are placed in the end, so that hot path only
> uses the first cacheline.

This also changes some boolean fields to uint8_t but we still assign
the true/false values to them (in some cases).
This works, but I would align those fields' assignments to this type change.
  

On 1/6/21 10:58 AM, David Marchand wrote:
> On Sun, Dec 20, 2020 at 10:15 PM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> This patch improves the virtio_hw struct packing,
>> going from 88 down to 80 bytes with a 6 bytes hole in
>> the end of the first cacheline. Fields only used in the
>> slow path are placed in the end, so that hot path only
>> uses the first cacheline.
> 
> This also changes some boolean fields to uint8_t but we still assign
> the true/false values to them (in some cases).
> This works, but I would align those fields' assignments to this type change.
> 

Fixed, note it was already inconsistent for some.

Thanks,
Maxime