On 5/10/15, 4:55 PM, Scott Feldman wrote:

On Sun, May 10, 2015 at 9:10 AM, roopa [off-list ref] wrote:

quoted

On 5/9/15, 12:00 PM, Jiri Pirko wrote:

quoted

Sat, May 09, 2015 at 07:40:16PM CEST, sfeldma@gmail.com wrote:

quoted

From: Scott Feldman <redacted>

Restore br_setlink back to original and don't call into SELF port driver.
rtnetlink.c:bridge_setlink() already does a call into port driver for
SELF.

bridge set link cmd defaults to MASTER.  From man page for bridge link
set
cmd:

        self   link setting is configured on specified physical device

        master link setting is configured on the software bridge (default)

The link setting has two values: the device-side value and the software
bridge-side value.  These are independent and settable using the bridge
link set cmd by specifying some combination of [master] | [self].
Futhermore, the device-side and bridge-side settings have their own
initial
value, viewable from bridge -d link show cmd.

Restoring br_setlink back to original makes rocker (the only in-kernel
user
of SELF link settings) work as first implement: two-sided values.

It's true that when both MASTER and SELF are specified from the command,
two netlink notifications are generated, one for each side of the
settings.
The user-space app can distiquish between the two notifications by
observing the MASTER or SELF flag.

This is revert of:

commit 68e331c785b85b78f4155e2ab6f90e976b609dc1
Author: Roopa Prabhu [off-list ref]
Date:   Thu Jan 29 22:40:14 2015 -0800

      bridge: offload bridge port attributes to switch asic if feature flag
set

Noting that because I want to make sure everybody is ok with new
behaviour. I tend to like it more.

I am not ok with it. I have raised this earlier. same argument as the fib
code, app now has to remember to call with both master and self. I do
however feel that this code needs some rework..,.add to hardware first and
then software
just like fib and rollback hardware on failure. In which case, i am ok with
submitting a new patch to do it differently.

The problem with your patch to br_setlink/br_dellink is it hard-coded
a default in the kernel bridge driver, inconsistent with the default
of the application (iproute2/bridge).  Reverting it keeps the kernel
out of the default decision and lets the application define a default
that suits it.

sorry, I am not understanding how this is different from how we do 
offload for fib and stp.
just like stp offload from the bridge driver, i am hoping we can also 
offload vlans (current patch under discussion)
and fdb entries. The switch driver can decide if it is only interested 
in calls with flags set to 'self' (rocker for example).

another example: mstp daemon running in userspace will use bridge 
setlink to propagate stp states to the in-kernel bridge driver. If the 
current patch is removed, mstp daemon will now have to make sure it 
calls bridge setlink with self and master flags to hit both the bridge 
driver and hardware.