Subject: Re: [PATCH net-next v3 1/3] net: sched: Move drop_reason to struct tc_skb_cb

From: Simon Horman <[email-masked]>
Date: Mon, 11 Dec 2023 13:37:38 +0000

On Tue, Dec 05, 2023 at 05:50:28PM -0300, Victor Nogueira wrote:
> Move drop_reason from struct tcf_result to skb cb - more specifically to
> struct tc_skb_cb. With that, we'll be able to also set the drop reason for
> the remaining qdiscs (aside from clsact) that do not have access to
> tcf_result when time comes to set the skb drop reason.
> 
> Signed-off-by: Victor Nogueira <[email-masked]>

Hi Victor,

The nit below notwithstanding, this looks good to me.

Reviewed-by: Simon Horman <[email-masked]>

> ---
>  include/net/pkt_cls.h     | 14 ++++++++++++--
>  include/net/pkt_sched.h   |  3 ++-
>  include/net/sch_generic.h |  1 -
>  net/core/dev.c            |  4 ++--
>  net/sched/act_api.c       |  2 +-
>  net/sched/cls_api.c       | 23 ++++++++---------------
>  6 files changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
> index a76c9171db0e..761e4500cca0 100644
> --- a/include/net/pkt_cls.h
> +++ b/include/net/pkt_cls.h
> @@ -154,10 +154,20 @@ __cls_set_class(unsigned long *clp, unsigned long cl)
>  	return xchg(clp, cl);
>  }
>  
> -static inline void tcf_set_drop_reason(struct tcf_result *res,
> +struct tc_skb_cb;
> +
> +static inline struct tc_skb_cb *tc_skb_cb(const struct sk_buff *skb);
> +

nit: It's not very important, as this code disappears in the next patch,
     but FWIIW I don't think the forward declarations of
     tc_skb_cb (struct or function) are needed.

> +static inline enum skb_drop_reason
> +tcf_get_drop_reason(const struct sk_buff *skb)
> +{
> +	return tc_skb_cb(skb)->drop_reason;
> +}
> +
> +static inline void tcf_set_drop_reason(const struct sk_buff *skb,
>  				       enum skb_drop_reason reason)
>  {
> -	res->drop_reason = reason;
> +	tc_skb_cb(skb)->drop_reason = reason;
>  }
>  
>  static inline void

...