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

[Vlad Buslov <vladbu@nvidia.com>]

On Sun 08 Oct 2023 at 18:53, Andrew Lunn <andrew@lunn.ch> wrote:
>> > +/* Invoked from both control and data path  */
>> > +static int __p4tc_table_entry_update(struct p4tc_pipeline *pipeline,
>> > +				     struct p4tc_table *table,
>> > +				     struct p4tc_table_entry *entry,
>> > +				     struct p4tc_table_entry_mask *mask,
>> > +				     u16 whodunnit, bool from_control)
>> > +__must_hold(RCU)
>> > +{
>> > +	struct p4tc_table_entry_mask *mask_found = NULL;
>> > +	struct p4tc_table_entry_work *entry_work;
>> > +	struct p4tc_table_entry_value *value_old;
>> > +	struct p4tc_table_entry_value *value;
>> > +	struct p4tc_table_entry *entry_old;
>> > +	struct p4tc_table_entry_tm *tm_old;
>> > +	struct p4tc_table_entry_tm *tm;
>> > +	int ret;
>> > +
>> > +	value = p4tc_table_entry_value(entry);
>> > +	/* We set it to zero on create an update to avoid having entry
>> > +	 * deletion in parallel before we report to user space.
>> > +	 */
>> > +	refcount_set(&value->entries_ref, 0);
>> 
>> TBH I already commented on one of the previous versions of this series
>> that it is very hard to understand and review tons of different atomic
>> reference counters, especially when they are modified with functions
>> like refcount_dec_not_one() or unconditional set like in this place.
>
> Hi Vlad
>
> Please always trim replies to just the relevant text. This is a 3000
> line patch, which i think you made one comment in, but maybe i missed
> others will paging down again, again and again....
>
>        Andrew

Hi Andrew,

I considered eliding the rest of the code (like I did in another reply
to this series), but assumed that any further discussion regarding my
question might involve other atomic ops spread around the change.

Regards,
Vlad