Re: [PATCH net-next v2 2/4] net: add IPv4 routing FIB support for switchdev

[roopa <roopa@cumulusnetworks.com>]

On 3/2/15, 9:10 AM, Scott Feldman wrote:
> On Mon, Mar 2, 2015 at 6:30 AM, roopa <roopa@cumulusnetworks.com> wrote:
>> On 3/2/15, 2:06 AM, sfeldma@gmail.com wrote:
>>> From: Scott Feldman <sfeldma@gmail.com>
>>>    +                     err = netdev_switch_fib_ipv4_add(key, plen, fi,
>>> +                                                        new_fa->fa_tos,
>>> +                                                        cfg->fc_type,
>>> +                                                        tb->tb_id);
>>> +                       if (err && err != -EOPNOTSUPP) {
>>> +                               kmem_cache_free(fn_alias_kmem, new_fa);
>>> +                               goto out;
>>> +                       }
>>> +
>>>                          hlist_replace_rcu(&fa->fa_list, &new_fa->fa_list);
>>> +
>>>                          alias_free_mem_rcu(fa);
>>>                          fib_release_info(fi_drop);
>>
>> This looks like the replace case: It will need a netdev_switch_fib_ipv4_del
>> for fi_drop ?
> It is the replace case.  No del needed.  Failure to replace in HW
> results in replace not proceeding in SW and err return to user.  So HW
> and SW stay coherent and user can react to failure case.
sure, If your driver can handle it. Am not sure if always doing a 
replace in hw
will work best in all cases. I cant think of a definite testcase right now.

AFAICT it becomes necessary to indicate a replace
for sure in the IPv6 multipath case.

>
>>> @@ -1197,12 +1208,18 @@ int fib_table_insert(struct fib_table *tb, struct
>>> fib_config *cfg)
>>>          new_fa->fa_state = 0;
>>>          new_fa->fa_slen = slen;
>>>    +     /* (Optionally) offload fib entry to switch hardware. */
>>> +       err = netdev_switch_fib_ipv4_add(key, plen, fi, tos,
>>> +                                        cfg->fc_type, tb->tb_id);
>>
>> This could be an NLM_F_APPEND case. Would be better for the switchdev API to
>> also take
>> nlflags as argument, to inform the switch driver of replace and append
>> cases.
> No, we can keep it simple.  The ndo add op is used for adds or mods.
> The driver/device will know if operation is a mod vs. add if object
> exists (it's an add if object doesn't exist).

Again, this will become apparent in the ipv6 multipath route case when 
you come to it.
The append flag is required to insert at tail.

Thanks,
Roopa