On Thu, 04 Aug 2016 11:27:25 -0700
Roopa Prabhu [off-list ref] wrote:

On 8/4/16, 1:15 AM, Toshiaki Makita wrote:

quoted

On 2016/08/04 16:24, Roopa Prabhu wrote:  

quoted

On 8/3/16, 7:11 PM, Toshiaki Makita wrote:  

quoted

Adding fdb entries pointing to the bridge device uses fdb_insert(),
which lacks various checks and does not respect added_by_user flag.

As a result, some inconsistent behavior can happen:
* Adding temporary entries succeeds but results in permanent entries.  

IIRC this is not specific to fdb entries on the bridge device. all temp
fdb entries via iproute2 result in permanent entries. thats why 'dynamic'
was added.  

Sorry for confusing you, I meant "temp" (i.e., "static") of bridge fdb
command.
"temp", "dynamic" and "use" should not result in "permanent".
 

quoted

quoted

* Same goes for "dynamic" and "use".  

This patch seems to not allow dynamic and use entries
on the bridge device. I don't see a strong use-case to
allow them, but any reason you want to add the restriction now ?  

Because dynamic fdb entries pointing the bridge device cannot be
created. So it is prohibited. I cannot find other appropriate behavior
about this.
Or are you suggesting local entries with aging supported or something
like that?  

no, i am ok with prohibiting it, just was wondering if this is necessary.

quoted

quoted

quoted

* Changing mac address of the bridge device causes deletion of
  user-added entries.  

unless I am missing something, this does not seem to be related to the
external fdb entry on the bridge device.  

Yes this is related to manually-added fdb entries on the bridge device.
When manual addition of fdb pointing the bridge device was introduced,
we should have set added_by_user on adding the entry and modify
br_fdb_change_mac_address() to respect the flag, but both were not done.
 

quoted

quoted

* Replacing existing entries looks successful from userspace but actually
  not, regardless of NLM_F_EXCL flag.  

curious about this one. I will try it, but if you have an example that
will help.  

Before:
# bridge fdb add 12:34:56:78:90:ab dev enp3s0 master
# bridge fdb add 12:34:56:78:90:ab dev br0; echo $?
0
# bridge fdb show
...
12:34:56:78:90:ab dev enp3s0 master br0 permanent

# bridge fdb replace 12:34:56:78:90:ab dev br0; echo $?
0
# bridge fdb show
...
12:34:56:78:90:ab dev enp3s0 master br0 permanent

After:
# bridge fdb add 12:34:56:78:90:ab dev enp3s0 master
# bridge fdb add 12:34:56:78:90:ab dev br0; echo $?
RTNETLINK answers: File exists
255

# bridge fdb replace 12:34:56:78:90:ab dev br0; echo $?
0
# bridge fdb show
...
12:34:56:78:90:ab dev br0 master br0 permanent
 

ok, thanks for the example.

Acked-by: Roopa Prabhu <redacted>

It should be possible to manually add fdb entries with any combination
of valid flags. That is it should be possible to add temporary as well as permanent
entries. There are people that use this facility to do long and short ageing temporary
entries.