[Intel-wired-lan] [PATCH S23 v2 14/15] ice: Change type for queue counts

[Paul Menzel]

Dear Tony, dear Pawel,


On 06/28/19 17:03, Tony Nguyen wrote:
> From: Pawel Kaminski <pawel.kaminski at intel.com>
> 
> A negative queue count should never exist, change the type to unsigned
> to convey that.
> 
> Signed-off-by: Pawel Kaminski <pawel.kaminski at intel.com>
> ---
> v2: Change type from u16 to unsigned int
>     Reword commit message

Thank you.

Also suggestion for a more specific commit message summary.

> ice: Use unsigned types for queue counts

>  drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
> index f2ebbe83ae4c..b8fc08c1d365 100644
> --- a/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
> +++ b/drivers/net/ethernet/intel/ice/ice_virtchnl_pf.c
> @@ -2339,11 +2339,11 @@ static int ice_vc_request_qs_msg(struct ice_vf *vf, u8 *msg)
>  	enum virtchnl_status_code v_ret = VIRTCHNL_STATUS_SUCCESS;
>  	struct virtchnl_vf_res_request *vfres =
>  		(struct virtchnl_vf_res_request *)msg;
> -	int req_queues = vfres->num_queue_pairs;
> +	unsigned int req_queues = vfres->num_queue_pairs;

Looking at the struct definition again, there actually u16 is used.

    /* VF resource request */
    struct virtchnl_vf_res_request {
            u16 num_queue_pairs;
    };

So only for `req_queues` it might be indeed good to match the type.
Sorry for not checking that before, and I do not know if it is
important, as I am not a C expert.

> +	unsigned int max_allowed_vf_queues;
> +	unsigned int tx_rx_queue_left;
>  	struct ice_pf *pf = vf->pf;
> -	int max_allowed_vf_queues;
> -	int tx_rx_queue_left;
> -	int cur_queues;
> +	unsigned int cur_queues;
>  
>  	if (!test_bit(ICE_VF_STATE_ACTIVE, vf->vf_states)) {
>  		v_ret = VIRTCHNL_STATUS_ERR_PARAM;
> @@ -2353,10 +2353,10 @@ static int ice_vc_request_qs_msg(struct ice_vf *vf, u8 *msg)
>  	cur_queues = vf->num_vf_qs;
>  	tx_rx_queue_left = min_t(int, pf->q_left_tx, pf->q_left_rx);
>  	max_allowed_vf_queues = tx_rx_queue_left + cur_queues;
> -	if (req_queues <= 0) {
> +	if (!req_queues) {
>  		dev_err(&pf->pdev->dev,
> -			"VF %d tried to request %d queues. Ignoring.\n",
> -			vf->vf_id, req_queues);
> +			"VF %d tried to request 0 queues. Ignoring.\n",
> +			vf->vf_id);
>  	} else if (req_queues > ICE_MAX_BASE_QS_PER_VF) {
>  		dev_err(&pf->pdev->dev,
>  			"VF %d tried to request more than %d queues.\n",

There are more messages where the value of `req_queues` is printed.
Do the format specifiers need to be changed from %d to %u?


Kind regards,

Paul

> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5174 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20190704/48a90efc/attachment.p7s>