Re: [PATCH 1/3] IB/core: Add support of checksum capability reporting in ib verbs

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2547 bytes --]

On 09/16/2015 11:56 AM, Bodong Wang wrote:
> A new filed csum_cap is added to both ib_query_device. It contains two members:
> eth_csum_cap and ib_csum_cap, indicates checksum capability of Ethernet and
> Infiniband link layer respectively for different QP types.
> 
> Current checksum caps use the following enum members:
> - IB_CSUM_SUPPORT_UD: device supports validation/calculation of csum for UD QP.
> - IB_CSUM_SUPPORT_RAW: device supports validation/calculation of csum for raw QP.
> 
> Signed-off-by: Bodong Wang <bodong-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> ---
>  include/rdma/ib_verbs.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
> index b0f898e..94dbaee 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -183,6 +183,11 @@ struct ib_cq_init_attr {
>  	u32		flags;
>  };
>  
> +struct ib_csum_cap_per_link {
> +	uint32_t  eth_csum_cap;
> +	uint32_t  ib_csum_cap;
> +};
> +

I generally don't like to waste this many bits on this little
information.  64 bits total for what only uses 4 bits right now, and
even on the high side would probably only ever use 8 or 12 bits, is
excessive.

That said, it's cleaner and easier to read than something like a double
shift where ib is lower 16 bits, eth is upper 16 bits, so I won't
request you change it, just register my eyebrow raise over the number of
bits used to record so little information.  (In fairness, I thought
about make you shrink it down, but the area of the struct you are adding
this to is currently 64bit aligned and it is reasonably likely that the
next item will need 64bit alignment, so saving bits only to possibly
loose them to alignment is a exercise in futility)

>  struct ib_device_attr {
>  	u64			fw_ver;
>  	__be64			sys_image_guid;
> @@ -229,6 +234,7 @@ struct ib_device_attr {
>  	struct ib_odp_caps	odp_caps;
>  	uint64_t		timestamp_mask;
>  	uint64_t		hca_core_clock; /* in KHZ */
> +	struct ib_csum_cap_per_link csum_cap;
>  };
>  
>  enum ib_mtu {
> @@ -868,6 +874,10 @@ enum ib_qp_create_flags {
>  	IB_QP_CREATE_RESERVED_END		= 1 << 31,
>  };
>  
> +enum ib_csum_cap_flags {
> +	IB_CSUM_SUPPORT_UD	= (1 << IB_QPT_UD),
> +	IB_CSUM_SUPPORT_RAW	= (1 << IB_QPT_RAW_PACKET),
> +};
>  
>  /*
>   * Note: users may not call ib_close_qp or ib_destroy_qp from the event_handler
> 


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]