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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 11 Dec 2023 13:37:38 +0000
From: Simon Horman <horms@...nel.org>
To: Victor Nogueira <victor@...atatu.com>
Cc: jhs@...atatu.com, xiyou.wangcong@...il.com, jiri@...nulli.us,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, daniel@...earbox.net, dcaratti@...hat.com,
	netdev@...r.kernel.org, kernel@...atatu.com
Subject: Re: [PATCH net-next v3 1/3] net: sched: Move drop_reason to struct
 tc_skb_cb

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 <victor@...atatu.com>

Hi Victor,

The nit below notwithstanding, this looks good to me.

Reviewed-by: Simon Horman <horms@...nel.org>

> ---
>  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

...