Re: [patch net-next v3 09/17] bridge: add API to notify bridge driver of learned FBD on offloaded device

Scott Feldman
2014-11-26T02:03:32Z

On Tue, Nov 25, 2014 at 12:44 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 25/11/14 02:28, Jiri Pirko wrote:
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> When the swdev device learns a new mac/vlan on a port, it sends some async
>> notification to the driver and the driver installs an FDB in the device.
>> To give a holistic system view, the learned mac/vlan should be reflected
>> in the bridge's FBD table, so the user, using normal iproute2 cmds, can view
>> what is currently learned by the device. This API on the bridge driver gives
>> a way for the swdev driver to install an FBD entry in the bridge FBD table.
>> (And remove one).
>>
>> This is equivalent to the device running these cmds:
>>
>> bridge fdb [add|del] <mac> dev <dev> vid <vlan id> master
>>
>> This patch needs some extra eyeballs for review, in paricular around the
>> locking and contexts.
>>
>> Signed-off-by: Scott Feldman <sfeldma@gmail.com>
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>
> [snip]
>
>> + head = &br->hash[br_mac_hash(addr, vid)];
>> + fdb = fdb_find(head, addr, vid);
>> + if (!fdb) {
>> + fdb = fdb_create(head, p, addr, vid);
>> + if (!fdb) {
>> + err = -ENOMEM;
>> + goto err_unlock;
>> + }
>> + fdb->added_by_external_learn = 1;
>> + fdb_notify(br, fdb, RTM_NEWNEIGH);
>> + } else if (fdb->added_by_external_learn) {
>> + /* Refresh entry */
>> + fdb->updated = fdb->used = jiffies;
>> + } else if (!fdb->added_by_user) {
>> + /* Take over SW learned entry */
>> + fdb->added_by_external_learn = 1;
>> + fdb->updated = jiffies;
>> + fdb_notify(br, fdb, RTM_NEWNEIGH);
>> + }
>
> Is there any case where this fdb entry gets re-used and is no longer
> added by an external learning? Should we clear this flag somewhere?

Once the FDB entry is marked "added_by_external_learn" it stays marked
as such until removed by aging cleanup process (or flushed due to
interface down, etc). If aged out (and now deleted), the FDB entry
may come back either by SW learn or by HW learn. If SW learn comes
first, and then HW learn, HW learn will override and mark the existing
FDB entry "added_by_external_learn". So there is take-over by HW but
no give-back to SW. And there is no explicit clearing of the mark
short of deleting the FDB entry. The mark is mostly for letting
user's know which FDB entries where learned by HW and synced to the
bridge's FDB.

> [snip]
>
>> +EXPORT_SYMBOL(br_fdb_external_learn_del);
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 4f577c4..02cd63b 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -101,6 +101,7 @@ struct net_bridge_fdb_entry
>> unsigned char is_local;
>> unsigned char is_static;
>> unsigned char added_by_user;
>> + unsigned char added_by_external_learn;
>
> Pheww, we could be saving some memory footprint here by using different
> types here ;)
> --
> Florian