Re: [net-next,v3] net: dsa: mv88e6xxx: Add partial support for TCAM entries

[Cedric Jehasse]

Hi,

>> diff --git a/drivers/net/dsa/mv88e6xxx/tcam.c b/drivers/net/dsa/mv88e6xxx/tcam.c
>> new file mode 100644
>> index 000000000000..ed02c8798200
>> --- /dev/null
>> +++ b/drivers/net/dsa/mv88e6xxx/tcam.c
>
>[ ... ]
>
>> +/* insert tcam entry in ordered list and move existing entries if necessary */
>> +static int mv88e6xxx_tcam_insert_entry(struct mv88e6xxx_chip *chip,
>> +                                    struct mv88e6xxx_tcam_entry *entry)
>> +{
>> +     struct mv88e6xxx_tcam_entry *elem;
>> +     struct list_head *hpos;
>> +     int err;
>> +
>> +     list_for_each_prev(hpos, &chip->tcam.entries) {
>> +             u8 move_idx;
>> +
>> +             elem = list_entry(hpos, struct mv88e6xxx_tcam_entry, list);
>> +             if (entry->prio >= elem->prio)
>> +                     break;
>> +
>> +             move_idx = elem->hw_idx + 1;
>> +
>> +             err = mv88e6xxx_tcam_flush_entry(chip, move_idx);
>> +             if (err)
>> +                     return err;
>> +
>> +             err = chip->info->ops->tcam_ops->entry_add(chip, elem,
>> +                                                        move_idx);
>> +             if (err)
>> +                     return err;
>> +
>> +             elem->hw_idx = move_idx;
>> +     }
>
>If entry_add() fails after some entries have already been moved to new
>hardware positions (with their hw_idx updated), doesn't the function
>return without rolling back those partial moves? The software list would
>then have some entries pointing to their new positions while the gap for
>the new entry is only partially created, leaving the TCAM in an
>inconsistent state for subsequent operations.

I don't think this is an issue. For entries that are successfully moved the
hw_idx points to the new index. And for entries that haven't successfully
moved it's still pointing to the old index. Hence the hw_idx field for every
list entry should still point to the correct TCAM index. There will be a gap
in the TCAM entries in the hardware, but that's not an issue.
If moving an existing entry returns an error. The rollback moving of entries
is as likely to return an error, and potentially leave the TCAM in an
inconsistent state.

Thanks,
Cedric