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:41:00Z

On Tue, Nov 25, 2014 at 4:34 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 25/11/14 18:03, Scott Feldman wrote:
>> 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.
>
> Thanks, makes sense now. This is probably obvious in this context, but
> maybe it would not hurt to come up with a documentation that describe
> the offload API, FDB entry lifetime and HW/SW ownership etc...

I have an updated Documentation/networking/switchdev.txt that covers
the swdev APIs and usage and notes, but Jiri is being stingy with it.
Will get this out, either in v4 or follow-on patches. There is enough
going on just with L2 offload that we're going to need some good
documentation to guide implementers.