netdev - Re: [PATCH net-next v2 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]

Message-ID: <e72df462-3c21-4b57-b7b0-7b71597b97a7@mojatatu.com>
Date: Tue, 5 Dec 2023 13:28:19 -0300
From: Victor Nogueira <victor@...atatu.com>
To: Paolo Abeni <pabeni@...hat.com>, Daniel Borkmann <daniel@...earbox.net>,
 Victor Nogueira <victor@...atatu.com>, jhs@...atatu.com,
 xiyou.wangcong@...il.com, jiri@...nulli.us, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org
Cc: dcaratti@...hat.com, netdev@...r.kernel.org, kernel@...atatu.com
Subject: Re: [PATCH net-next v2 1/3] net: sched: Move drop_reason to struct
 tc_skb_cb

On 05/12/2023 08:32, Paolo Abeni wrote:
> On Tue, 2023-12-05 at 12:06 +0100, Daniel Borkmann wrote:
>> On 12/2/23 12:00 AM, 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>
>>> ---
>>>    include/net/pkt_cls.h     | 14 ++++++++++++--
>>>    include/net/pkt_sched.h   |  1 +
>>>    include/net/sch_generic.h |  1 -
>>>    net/core/dev.c            |  5 +++--
>>>    net/sched/act_api.c       |  2 +-
>>>    net/sched/cls_api.c       | 23 ++++++++---------------
>>>    6 files changed, 25 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
>>> index a76c9171db0e..7bd7ea511100 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);
>>> +
>>> +static inline enum skb_drop_reason
>>> +tc_skb_cb_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
>>> diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
>>> index 9fa1d0794dfa..f09bfa1efed0 100644
>>> --- a/include/net/pkt_sched.h
>>> +++ b/include/net/pkt_sched.h
>>> @@ -277,6 +277,7 @@ static inline void skb_txtime_consumed(struct sk_buff *skb)
>>>    
>>>    struct tc_skb_cb {
>>>    	struct qdisc_skb_cb qdisc_cb;
>>> +	u32 drop_reason;
>>>    
>>>    	u16 mru;
>>
>> Probably also makes sense to reorder zone below mru.
> 
> Or move 'zone' here. It's very minor but I would prefer the latter ;)
> (and leave the hole at the end of the struct)

Yes, this looks better.
Thank you, I'll proceed this way.