netdev - Re: [PATCH net-next v2 2/3] net/sched: cls_api: Expose tc block ports to the datapath

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]

Message-ID: <CALnP8Zbq4puMU4-9EgTbJYn2MPiBc3Ygpaxnhmh-pqmDX7rXDA@mail.gmail.com>
Date: Wed, 23 Aug 2023 10:33:48 -0700
From: Marcelo Ricardo Leitner <mleitner@...hat.com>
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, 
	netdev@...r.kernel.org, vladbu@...dia.com, horms@...nel.org, 
	pctammela@...atatu.com, kernel@...atatu.com
Subject: Re: [PATCH net-next v2 2/3] net/sched: cls_api: Expose tc block ports
 to the datapath

On Sat, Aug 19, 2023 at 01:35:13PM -0300, Victor Nogueira wrote:
> The datapath can now find the block of the port in which the packet arrived
> at. It can then use it for various activities.

I think $subject needs a s/ports//. Because, well, the patch is
exposing the block, which contains the ports.. The first sentence here
goes along with this rationale.

more below

>
> In the next patch we show a simple action that multicasts to all ports
> excep for the port in which the packet arrived on.

"except"

>
> Co-developed-by: Jamal Hadi Salim <jhs@...atatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@...atatu.com>
> Co-developed-by: Pedro Tammela <pctammela@...atatu.com>
> Signed-off-by: Pedro Tammela <pctammela@...atatu.com>
> Signed-off-by: Victor Nogueira <victor@...atatu.com>
> ---
>  include/net/sch_generic.h |  4 ++++
>  net/sched/cls_api.c       | 10 +++++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index 824a0ecb5afc..c5defb166ef6 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -440,6 +440,8 @@ struct qdisc_skb_cb {
>  	};
>  #define QDISC_CB_PRIV_LEN 20
>  	unsigned char		data[QDISC_CB_PRIV_LEN];
> +	/* This should allow eBPF to continue to align */

Not sure if this comment really belongs in here. Up to you but it
seems better suited in the patch description. Hopefully the next one
won't do something like:

 	/* This should allow eBPF to continue to align */
 	u32                     block_index;
+	/* This one too */
+	u32                     my_var;

:-)

> +	u32                     block_index;
>  };
>
>  typedef void tcf_chain_head_change_t(struct tcf_proto *tp_head, void *priv);