On 24/09/15 13:59, sfel...@gmail.com wrote: > From: Scott Feldman <sfel...@gmail.com> > > Push bridge-level attributes down to switchdev drivers. This patchset > adds the infrastructure and then pushes, as an example, ageing_time attribute > down from bridge to switchdev (rocker) driver. Add some range-checking > for ageing_time. > > # ip link set dev br0 type bridge ageing_time 1000 > > # ip link set dev br0 type bridge ageing_time 999 > RTNETLINK answers: Numerical result out of range > > Up until now, switchdev attrs where port-level attrs, so the netdev used in > switchdev_attr_set() would be a switch port or bond of switch ports. With > bridge-level attrs, the netdev passed to switchdev_attr_set() is the bridge > netdev. The same recusive algo is used to visit the leaves of the stacked > drivers to set the attr, it's just in this case we start one layer higher in > the stack. One note is not all ports in the bridge may support setting a > bridge-level attribute, so rather than failing the entire set, we'll skip over > those ports returning -EOPNOTSUPP.
So, without a better device to hold that kind of information (in the future it could be a global, switch-specific device holding that information), I agree with your decision to take the bridge device to hold that attribute, it still feels a bit uncomfortable to have switchdev_attr_port() take a bridge device parameter, but whatever, here is a scenario I am wondering how we would want to proceed with: - suppose we have a switch which is only able to control ageing globally, not per port or any other kind of logical domain - we have enabled two software bridges on the same physical switch, with different ageing timeouts It does not seem to me like it hurts ageing the other bridge faster than expected (even though that could be expensive for MDIO devices), but we would need to have consistent reporting here for the other bridge. We could therefore have the driver return different things: - < 0: error, value is too low or too high for the hardware to support that - == 0: supports ageing in a more fine-grained way that globally - > 0 (== ageing): supports ageing globally and switchdev/bridge also needs to update the other bridge devices with the same ageing parameters Alternatively, as soon as we have more bridges than supported ageing control knobs, we could have the driver just return -EPERM or something like that, but that means keeping track of bridges attached to the switch's ports. Other than that, this looks good to me, thanks! > > Scott Feldman (4): > switchdev: add bridge attributes > switchdev: skip over ports returning -EOPNOTSUPP when recursing ports > bridge: push bridge setting ageing_time down to switchdev > rocker: handle setting bridge ageing_time > > drivers/net/ethernet/rocker/rocker.c | 22 ++++++++++++++++++++++ > include/net/switchdev.h | 6 ++++++ > include/uapi/linux/if_link.h | 2 +- > net/bridge/br_ioctl.c | 3 +-- > net/bridge/br_netlink.c | 6 +++--- > net/bridge/br_private.h | 1 + > net/bridge/br_stp.c | 24 ++++++++++++++++++++++++ > net/bridge/br_sysfs_br.c | 3 +-- > net/switchdev/switchdev.c | 9 ++++++++- > 9 files changed, 67 insertions(+), 9 deletions(-) > -- Florian -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
