Re: [PATCH v16 net-next 2/5] sched: Dump configuration and statistics of dualpi2 qdisc

[Paolo Abeni <pabeni@redhat.com>]

On 5/16/25 2:01 AM, chia-yu.chang@nokia-bell-labs.com wrote:
> From: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
> 
> The configuration and statistics dump of the DualPI2 Qdisc provides
> information related to both queues, such as packet numbers and queuing
> delays in the L-queue and C-queue, as well as general information such as
> probability value, WRR credits, memory usage, packet marking counters, max
> queue size, etc.
> 
> The following patch includes enqueue/dequeue for DualPI2.
> 
> v16:
> - Update convert_ns_to_usec() to avoid overflow

The changelog should come after the SoB and a '---' separator.
> 
> Signed-off-by: Chia-Yu Chang <chia-yu.chang@nokia-bell-labs.com>
> ---
>  include/uapi/linux/pkt_sched.h | 15 ++++++
>  net/sched/sch_dualpi2.c        | 89 ++++++++++++++++++++++++++++++++++
>  2 files changed, 104 insertions(+)
> 
> diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
> index ae8af0e8d479..a7243f32ff0f 100644
> --- a/include/uapi/linux/pkt_sched.h
> +++ b/include/uapi/linux/pkt_sched.h
> @@ -1264,4 +1264,19 @@ enum {
>  
>  #define TCA_DUALPI2_MAX   (__TCA_DUALPI2_MAX - 1)
>  
> +struct tc_dualpi2_xstats {
> +	__u32 prob;		/* current probability */
> +	__u32 delay_c;		/* current delay in C queue */
> +	__u32 delay_l;		/* current delay in L queue */
> +	__u32 packets_in_c;	/* number of packets enqueued in C queue */
> +	__u32 packets_in_l;	/* number of packets enqueued in L queue */
> +	__u32 maxq;		/* maximum queue size */
> +	__u32 ecn_mark;		/* packets marked with ecn*/
> +	__u32 step_marks;	/* ECN marks due to the step AQM */
> +	__s32 credit;		/* current c_protection credit */
> +	__u32 memory_used;	/* Memory used of both queues */
> +	__u32 max_memory_used;	/* Maximum used memory */
> +	__u32 memory_limit;	/* Memory limit of both queues */
> +};
> +
>  #endif
> diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c
> index ffdfb7803e1f..97986c754e47 100644
> --- a/net/sched/sch_dualpi2.c
> +++ b/net/sched/sch_dualpi2.c
> @@ -123,6 +123,14 @@ static u32 dualpi2_scale_alpha_beta(u32 param)
>  	return tmp;
>  }
>  
> +static u32 dualpi2_unscale_alpha_beta(u32 param)
> +{
> +	u64 tmp = ((u64)param * NSEC_PER_SEC << ALPHA_BETA_SCALING);
> +
> +	do_div(tmp, MAX_PROB);
> +	return tmp;
> +}
> +
>  static ktime_t next_pi2_timeout(struct dualpi2_sched_data *q)
>  {
>  	return ktime_add_ns(ktime_get_ns(), q->pi2_tupdate);
> @@ -223,6 +231,15 @@ static u32 convert_us_to_nsec(u32 us)
>  		return lower_32_bits(ns);
>  }
>  
> +static u32 convert_ns_to_usec(u64 ns)
> +{
> +	do_div(ns, NSEC_PER_USEC);
> +	if (upper_32_bits(ns))
> +		return 0xffffffff;

U32_MAX

> +	else
> +		return lower_32_bits(ns);
> +}
> +
>  static enum hrtimer_restart dualpi2_timer(struct hrtimer *timer)
>  {
>  	struct dualpi2_sched_data *q = from_timer(q, timer, pi2_timer);
> @@ -458,6 +475,76 @@ static int dualpi2_init(struct Qdisc *sch, struct nlattr *opt,
>  	return 0;
>  }
>  
> +static int dualpi2_dump(struct Qdisc *sch, struct sk_buff *skb)
> +{
> +	struct dualpi2_sched_data *q = qdisc_priv(sch);
> +	struct nlattr *opts;
> +
> +	opts = nla_nest_start_noflag(skb, TCA_OPTIONS);
> +	if (!opts)
> +		goto nla_put_failure;
> +
> +	if (nla_put_u32(skb, TCA_DUALPI2_LIMIT, READ_ONCE(sch->limit)) ||
> +	    nla_put_u32(skb, TCA_DUALPI2_MEMORY_LIMIT,
> +			READ_ONCE(q->memory_limit)) ||
> +	    nla_put_u32(skb, TCA_DUALPI2_TARGET,
> +			convert_ns_to_usec(READ_ONCE(q->pi2_target))) ||
> +	    nla_put_u32(skb, TCA_DUALPI2_TUPDATE,
> +			convert_ns_to_usec(READ_ONCE(q->pi2_tupdate))) ||
> +	    nla_put_u32(skb, TCA_DUALPI2_ALPHA,
> +			dualpi2_unscale_alpha_beta(READ_ONCE(q->pi2_alpha))) ||
> +	    nla_put_u32(skb, TCA_DUALPI2_BETA,
> +			dualpi2_unscale_alpha_beta(READ_ONCE(q->pi2_beta))) ||
> +	    nla_put_u32(skb, TCA_DUALPI2_STEP_THRESH,
> +			READ_ONCE(q->step_in_packets) ?
> +			READ_ONCE(q->step_thresh) :
> +			convert_ns_to_usec(READ_ONCE(q->step_thresh))) ||
> +	    nla_put_u32(skb, TCA_DUALPI2_MIN_QLEN_STEP,
> +			READ_ONCE(q->min_qlen_step)) ||
> +	    nla_put_u8(skb, TCA_DUALPI2_COUPLING,
> +		       READ_ONCE(q->coupling_factor)) ||
> +	    nla_put_u8(skb, TCA_DUALPI2_DROP_OVERLOAD,
> +		       READ_ONCE(q->drop_overload)) ||
> +	    (READ_ONCE(q->step_in_packets) &&
> +	     nla_put_flag(skb, TCA_DUALPI2_STEP_PACKETS)) ||
> +	    nla_put_u8(skb, TCA_DUALPI2_DROP_EARLY,
> +		       READ_ONCE(q->drop_early)) ||
> +	    nla_put_u8(skb, TCA_DUALPI2_C_PROTECTION,
> +		       READ_ONCE(q->c_protection_wc)) ||
> +	    nla_put_u8(skb, TCA_DUALPI2_ECN_MASK, READ_ONCE(q->ecn_mask)) ||
> +	    nla_put_u8(skb, TCA_DUALPI2_SPLIT_GSO, READ_ONCE(q->split_gso)))
> +		goto nla_put_failure;
> +
> +	return nla_nest_end(skb, opts);
> +
> +nla_put_failure:
> +	nla_nest_cancel(skb, opts);
> +	return -1;
> +}
> +
> +static int dualpi2_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
> +{
> +	struct dualpi2_sched_data *q = qdisc_priv(sch);
> +	struct tc_dualpi2_xstats st = {
> +		.prob			= READ_ONCE(q->pi2_prob),
> +		.packets_in_c		= q->packets_in_c,
> +		.packets_in_l		= q->packets_in_l,
> +		.maxq			= q->maxq,
> +		.ecn_mark		= q->ecn_mark,
> +		.credit			= q->c_protection_credit,
> +		.step_marks		= q->step_marks,
> +		.memory_used		= q->memory_used,
> +		.max_memory_used	= q->max_memory_used,
> +		.memory_limit		= q->memory_limit,
> +	};

I *think* you either need READ_ONCE() annotation for the above lockless
read, or add a  sch_tree_lock(sch)/sch_tree_unlock(sch) pair.

/P