netdev - Re: [PATCH net-next RFC 17/20] p4tc: add table entry create, update, get, delete, flush and dump

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:   Thu, 26 Jan 2023 10:45:15 -0500
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     Vlad Buslov <vladbu@...dia.com>
Cc:     netdev@...r.kernel.org, kernel@...atatu.com,
        deb.chatterjee@...el.com, anjali.singhai@...el.com,
        namrata.limaye@...el.com, khalidm@...dia.com, tom@...anda.io,
        pratyush@...anda.io, jiri@...nulli.us, xiyou.wangcong@...il.com,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, simon.horman@...igine.com
Subject: Re: [PATCH net-next RFC 17/20] p4tc: add table entry create, update,
 get, delete, flush and dump

On Wed, Jan 25, 2023 at 4:25 PM Vlad Buslov <vladbu@...dia.com> wrote:
>
> On Tue 24 Jan 2023 at 12:05, Jamal Hadi Salim <jhs@...atatu.com> wrote:
> > +     int                              num_acts;
> > +     struct rhlist_head               ht_node;
> > +     struct list_head                 list;

[..]

> > +static int __tcf_table_entry_del(struct p4tc_pipeline *pipeline,
> > +                              struct p4tc_table *table,
> > +                              struct p4tc_table_entry_key *key,
> > +                              struct p4tc_table_entry_mask *mask, u32 prio,
> > +                              struct netlink_ext_ack *extack)
>
> This seems to be an infrastructure function implemented for future
> usage, but since it is static and not called form anywhere it causes
> compilation failure on this and following patches.
>

Ah - you must be compiling with -Werror.
There's some other code we didnt include for statefulness (what they
call "add on miss" in P4 PNA)
that would use this. We should have taken this out. We will take it
out in the next update if add-on-miss is
not included.

> > +{
> > +     struct p4tc_table_entry *entry;
> > +     int ret;
> > +
> > +     tcf_table_entry_build_key(key, mask);
> > +
> > +     entry = p4tc_entry_lookup(table, key, prio);
> > +     if (!entry) {
> > +             rcu_read_unlock();
>
> Where is the dual rcu_read_lock() for this?
>

Shouldnt be there at all. Speaks to the state of completion for add-on-miss
which is still in process of being implemented to meet the spec.

BTW:  I apologize if i skipped some of your comments - i am having a bit of a
hard time with a web based agent (I am giving up on thunderbird since it cant
seem to handle well the amount of emails i have using IMAP).
So if i skipped something it is not intentional, just point it out
again. I am using
old skule hand editing of the email when responding.

cheers,
jamal