Re: [dpdk-dev] [PATCH v4 06/11] sched: improve doxygen comments

["Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>]

> -----Original Message-----
> From: Singh, Jasvinder
> Sent: Friday, July 12, 2019 11:57 AM
> To: dev@dpdk.org
> Cc: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Tovar, AbrahamX
> <abrahamx.tovar@intel.com>; Krakowiak, LukaszX
> <lukaszx.krakowiak@intel.com>
> Subject: [PATCH v4 06/11] sched: improve doxygen comments
> 
> Improve doxygen comments.
> 
> Signed-off-by: Jasvinder Singh <jasvinder.singh@intel.com>
> Signed-off-by: Abraham Tovar <abrahamx.tovar@intel.com>
> Signed-off-by: Lukasz Krakowiak <lukaszx.krakowiak@intel.com>
> ---
>  lib/librte_sched/rte_sched.h | 145 ++++++++++++++++++++++-------------
>  1 file changed, 93 insertions(+), 52 deletions(-)
> 
> diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
> index 9cccdda41..e8aa63301 100644
> --- a/lib/librte_sched/rte_sched.h
> +++ b/lib/librte_sched/rte_sched.h
> @@ -52,7 +52,7 @@ extern "C" {
>   *	    multiple connections of same traffic class belonging to
>   *	    the same user;
>   *           - Weighted Round Robin (WRR) is used to service the
> - *	    queues within same pipe traffic class.
> + *	    queues within same pipe lowest priority traffic class (best-effort).
>   *
>   */
> 
> @@ -83,6 +83,8 @@ extern "C" {
>  #define RTE_SCHED_BE_QUEUES_PER_PIPE    4
> 
>  /** Number of traffic classes per pipe (as well as subport).
> + * @see struct rte_sched_subport_params
> + * @see struct rte_sched_pipe_params
>   */
>  #define RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE    \
>  (RTE_SCHED_QUEUES_PER_PIPE - RTE_SCHED_BE_QUEUES_PER_PIPE + 1)
> @@ -106,6 +108,8 @@ extern "C" {
>   *
>   * The FCS is considered overhead only if not included in the packet
>   * length (field pkt_len of struct rte_mbuf).
> + *
> + * @see struct rte_sched_port_params
>   */
>  #ifndef RTE_SCHED_FRAME_OVERHEAD_DEFAULT
>  #define RTE_SCHED_FRAME_OVERHEAD_DEFAULT      24
> @@ -121,34 +125,39 @@ extern "C" {
>   * byte.
>   */
>  struct rte_sched_subport_params {
> -	/* Subport token bucket */
> -	uint32_t tb_rate;                /**< Rate (measured in bytes per second)
> */
> -	uint32_t tb_size;                /**< Size (measured in credits) */
> +	/** Token bucket rate (measured in bytes per second) */
> +	uint32_t tb_rate;
> +
> +	/** Token bucket size (measured in credits) */
> +	uint32_t tb_size;
> 
> -	/* Subport traffic classes */
> +	/** Traffic class rates (measured in bytes per second) */
>  	uint32_t tc_rate[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> -	/**< Traffic class rates (measured in bytes per second) */
> +
> +	/** Enforcement period for rates (measured in milliseconds) */
>  	uint32_t tc_period;
> -	/**< Enforcement period for rates (measured in milliseconds) */
> +
> +	/** Number of subport_pipes */
> +	uint32_t n_pipes_per_subport;
>  };
> 
>  /** Subport statistics */
>  struct rte_sched_subport_stats {
> -	/* Packets */
> +	/** Number of packets successfully written */
>  	uint32_t n_pkts_tc[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> -	/**< Number of packets successfully written */
> +
> +	/** Number of packets dropped */
>  	uint32_t
> n_pkts_tc_dropped[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> -	/**< Number of packets dropped */
> 
> -	/* Bytes */
> +	/** Number of bytes successfully written for each traffic class */
>  	uint32_t n_bytes_tc[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> -	/**< Number of bytes successfully written for each traffic class */
> +
> +	/** Number of bytes dropped for each traffic class */
>  	uint32_t
> n_bytes_tc_dropped[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> -	/**< Number of bytes dropped for each traffic class */
> 
>  #ifdef RTE_SCHED_RED
> +	/** Number of packets dropped by red */
>  	uint32_t
> n_pkts_red_dropped[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> -	/**< Number of packets dropped by red */
>  #endif
>  };
> 
> @@ -162,59 +171,91 @@ struct rte_sched_subport_stats {
>   * byte.
>   */
>  struct rte_sched_pipe_params {
> -	/* Pipe token bucket */
> -	uint32_t tb_rate;                /**< Rate (measured in bytes per second)
> */
> -	uint32_t tb_size;                /**< Size (measured in credits) */
> +	/** Token bucket rate (measured in bytes per second) */
> +	uint32_t tb_rate;
> +
> +	/** Token bucket size (measured in credits) */
> +	uint32_t tb_size;
> 
> -	/* Pipe traffic classes */
> +	/** Traffic class rates (measured in bytes per second) */
>  	uint32_t tc_rate[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> -	/**< Traffic class rates (measured in bytes per second) */
> +
> +	/** Enforcement period (measured in milliseconds) */
>  	uint32_t tc_period;
> -	/**< Enforcement period (measured in milliseconds) */
> -	uint8_t tc_ov_weight;		 /**< Weight Traffic class 3
> oversubscription */
> 
> -	/* Pipe queues */
> -	uint8_t  wrr_weights[RTE_SCHED_BE_QUEUES_PER_PIPE]; /**<
> WRR weights */
> +	/** Best-effort traffic class oversubscription weight */
> +	uint8_t tc_ov_weight;
> +
> +	/** WRR weights of best-effort traffic class queues */
> +	uint8_t wrr_weights[RTE_SCHED_BE_QUEUES_PER_PIPE];
>  };
> 
>  /** Queue statistics */
>  struct rte_sched_queue_stats {
> -	/* Packets */
> -	uint32_t n_pkts;                 /**< Packets successfully written */
> -	uint32_t n_pkts_dropped;         /**< Packets dropped */
> +	/** Packets successfully written */
> +	uint32_t n_pkts;
> +
> +	/** Packets dropped */
> +	uint32_t n_pkts_dropped;
> +
>  #ifdef RTE_SCHED_RED
> -	uint32_t n_pkts_red_dropped;	 /**< Packets dropped by RED */
> +	/** Packets dropped by RED */
> +	uint32_t n_pkts_red_dropped;
>  #endif
> 
> -	/* Bytes */
> -	uint32_t n_bytes;                /**< Bytes successfully written */
> -	uint32_t n_bytes_dropped;        /**< Bytes dropped */
> +	/** Bytes successfully written */
> +	uint32_t n_bytes;
> +
> +	/** Bytes dropped */
> +	uint32_t n_bytes_dropped;
>  };
> 
>  /** Port configuration parameters. */
>  struct rte_sched_port_params {
> -	const char *name;                /**< String to be associated */
> -	int socket;                      /**< CPU socket ID */
> -	uint32_t rate;                   /**< Output port rate
> -					  * (measured in bytes per second) */
> -	uint32_t mtu;                    /**< Maximum Ethernet frame size
> -					  * (measured in bytes).
> -					  * Should not include the framing
> overhead. */
> -	uint32_t frame_overhead;         /**< Framing overhead per packet
> -					  * (measured in bytes) */
> -	uint32_t n_subports_per_port;    /**< Number of subports */
> -	uint32_t n_pipes_per_subport;    /**< Number of pipes per subport
> */
> +	/** Name of the port to be associated */
> +	const char *name;
> +
> +	/** CPU socket ID */
> +	int socket;
> +
> +	/** Output port rate (measured in bytes per second) */
> +	uint32_t rate;
> +
> +	/** Maximum Ethernet frame size (measured in bytes).
> +	 * Should not include the framing overhead.
> +	 */
> +	uint32_t mtu;
> +
> +	/** Framing overhead per packet (measured in bytes) */
> +	uint32_t frame_overhead;
> +
> +	/** Number of subports */
> +	uint32_t n_subports_per_port;
> +
> +	/** Number of subport_pipes */
> +	uint32_t n_pipes_per_subport;
> +
> +	/** Packet queue size for each traffic class.
> +	 * All the pipes within the same subport share the similar
> +	 * configuration for the queues. Queues which are not needed, have
> +	 * zero size.
> +	 */
>  	uint16_t qsize[RTE_SCHED_QUEUES_PER_PIPE];
> -	/**< Packet queue size for each traffic class.
> -	 * Queues which are not needed are allowed to have zero size. */
> +
> +	/** Pipe profile table.
> +	 * Every pipe is configured using one of the profiles from this table.
> +	 */
>  	struct rte_sched_pipe_params *pipe_profiles;
> -	/**< Pipe profile table.
> -	 * Every pipe is configured using one of the profiles from this table.
> */
> -	uint32_t n_pipe_profiles;        /**< Profiles in the pipe profile table */
> +
> +	/** Profiles in the pipe profile table */
> +	uint32_t n_pipe_profiles;
> +
> +	/** Max profiles allowed in the pipe profile table */
>  	uint32_t n_max_pipe_profiles;
> -	/**< Max profiles allowed in the pipe profile table */
> +
>  #ifdef RTE_SCHED_RED
> -	struct rte_red_params
> red_params[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE][RTE_COLORS]; /**<
> RED parameters */
> +	/** RED parameters */
> +	struct rte_red_params
> red_params[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE][RTE_COLORS];
>  #endif
>  };
> 
> @@ -328,8 +369,8 @@ rte_sched_port_get_memory_footprint(struct
> rte_sched_port_params *params);
>   *   Pointer to pre-allocated subport statistics structure where the statistics
>   *   counters should be stored
>   * @param tc_ov
> - *   Pointer to pre-allocated 4-entry array where the oversubscription status
> for
> - *   each of the 4 subport traffic classes should be stored.
> + *   Pointer to pre-allocated 13-entry array where the oversubscription
> status for
> + *   each of the subport traffic classes should be stored.
>   * @return
>   *   0 upon success, error code otherwise
>   */
> @@ -374,7 +415,7 @@ rte_sched_queue_read_stats(struct rte_sched_port
> *port,
>   * @param pipe
>   *   Pipe ID within subport
>   * @param traffic_class
> - *   Traffic class ID within pipe (0 .. 3)
> + *   Traffic class ID within pipe (0 .. 12)
>   * @param queue
>   *   Queue ID within pipe traffic class (0 .. 3)
>   * @param color
> @@ -401,7 +442,7 @@ rte_sched_port_pkt_write(struct rte_sched_port
> *port,
>   * @param pipe
>   *   Pipe ID within subport
>   * @param traffic_class
> - *   Traffic class ID within pipe (0 .. 3)
> + *   Traffic class ID within pipe (0 .. 12)
>   * @param queue
>   *   Queue ID within pipe traffic class (0 .. 3)
>   *

It would be great if we could replace these magic numbers of 3, 12, etc with the corresponding macros (in the API file at least, but it would be great to do it in the .c file as well).
> --
> 2.21.0