On 3/16/25 4:27 PM, [email protected] wrote:
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 0dbfe069a6e3..987686e91919 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -30,6 +30,7 @@
> #include <asm/byteorder.h>
> #include <asm/local.h>
>
> +#include <linux/netdev_features.h>
> #include <linux/percpu.h>
> #include <linux/rculist.h>
> #include <linux/workqueue.h>
I guess this chunck is a left-over... in any case it should be dropped.
[...]
> +struct dualpi2_sched_data {
> + struct Qdisc *l_queue; /* The L4S Low latency queue (L-queue) */
> + struct Qdisc *sch; /* The Classic queue (C-queue) */
> +
> + /* Registered tc filters */
> + struct {
> + struct tcf_proto __rcu *filters;
> + struct tcf_block *block;
> + } tcf;
This usage of anonimous struct is quite unconventional, the preferred
way of scoping fields is to add apprepriate prefix to the field name.
[...]
> +static bool must_drop(struct Qdisc *sch, struct dualpi2_sched_data *q,
> + struct sk_buff *skb)
> +{
> + u64 local_l_prob;
> + u32 prob;
> + bool overload;
Please respect the reverse christmas tree order in variable declaration
(many other occurrences below).
[...]
> +static void drop_and_retry(struct dualpi2_sched_data *q, struct sk_buff *skb,
> + struct Qdisc *sch)
> +{
> + ++q->deferred_drops.cnt;
> + q->deferred_drops.len += qdisc_pkt_len(skb);
> + consume_skb(skb);
Possibly you want to use kfree_skb_reason() here.
I think this patch is still too huge to get a reasonable review. You
should try to split it further, e.g. struct definition and parsing in
pX, dump in pX+1, enqueue related ops in p+2, ...
Thanks,
Paolo
