On Wed, Jul 29, 2015 at 12:14 PM, Vivien Didelot <vivien.dide...@savoirfairelinux.com> wrote: > Hi Scott, David, > > On Jul 29, 2015, at 2:28 PM, David da...@davemloft.net wrote: > >> From: Scott Feldman <sfel...@gmail.com> >> Date: Wed, 29 Jul 2015 00:31:44 -0700 >> >>> Since the netlink request (for example vlan add) includes the range, >>> I'm not seeing how we can response with success for the satisfied >>> vlans in the range, and also respond with an error for the unsatisfied >>> vlans in the range. In other words, from the netlink msgs >>> perspective, we need to treat a vlan range as all-or-nothing. So in >>> your example, if hw can't add vlan 2, we fail the entire request to >>> add range 2-5. This is where the prepare phase checks to make sure >>> the entire request can be satisfied before committing to hw. > > I made this change in order to start restricting the bridge abstraction > to switchdev, since IMHO its info flags do not add much value to the > switch chip drivers perspective. > > While a range might be convenient to a user, exposing it to drivers is > likely to end up writing the same vid_begin to vid_end for loop. > >> This was my concern with the change as well. >> >> The user asked for the range to be installed, so if any portion >> of it cannot be done we must not make any changes to the HW >> configuration and fail the entire request. > > I understand the concern with the netlink request. > > However, this can be confusing to someone. With the previous example: > > bridge vlan add dev port0 vid 2-5 master > > must fail for the entire range (due to the single netlink request). But: > > bridge vlan add dev port0 vid 2 master > > will silently fallback to software VLAN (assuming that the driver > correctly returned -EOPNOTSUPP in the prepare phase). In other words, no > changes has been committed to the hardware.
I see your concern now, I think. net/bridge/br_netlink.c:br_afspec() does the range loop but doesn't rewind if something goes wrong with one of the vlans in the range. The call into switchdev is one-at-a-time at that point. If br_afspec() handled the rewind, would this address your concern? We can keep the range support in the switchdev vlan obj, so 'self' can use it. -scott -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
