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: <30bc72f5-b21b-4846-b97b-1251b3dc3d4c@mojatatu.com>
Date: Tue, 5 Dec 2023 13:27:56 -0300
From: Victor Nogueira <victor@...atatu.com>
To: 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, pabeni@...hat.com
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:06, Daniel Borkmann wrote:
>> [...]
>> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
>> index dcb9160e6467..c499b56bb215 100644
>> --- a/include/net/sch_generic.h
>> +++ b/include/net/sch_generic.h
>> @@ -332,7 +332,6 @@ struct tcf_result {
>>           };
>>           const struct tcf_proto *goto_tp;
>>       };
>> -    enum skb_drop_reason        drop_reason;
>>   };
>>   struct tcf_chain;
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 3950ced396b5..323496ca0dc3 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -3924,14 +3924,15 @@ static int tc_run(struct tcx_entry *entry, 
>> struct sk_buff *skb,
>>       tc_skb_cb(skb)->mru = 0;
>>       tc_skb_cb(skb)->post_ct = false;
>> -    res.drop_reason = *drop_reason;
>> +    tc_skb_cb(skb)->post_ct = false;
> 
> Why the double assignment ?

Sigh, sorry will change that in v3.

> 
>> +    tcf_set_drop_reason(skb, *drop_reason);
>>       mini_qdisc_bstats_cpu_update(miniq, skb);
>>       ret = tcf_classify(skb, miniq->block, miniq->filter_list, &res, 
>> false);
>>       /* Only tcf related quirks below. */
>>       switch (ret) {
>>       case TC_ACT_SHOT:
>> -        *drop_reason = res.drop_reason;
>> +        *drop_reason = tc_skb_cb_drop_reason(skb);
> 
> nit: I'd rename into tcf_get_drop_reason() so it aligns with the 
> tcf_set_drop_reason().
> It's weird to name the getter tc_skb_cb_drop_reason() instead.

Seems more consistent, will do.

>> [...]