On 07/07/2017 12:36 AM, Florian Fainelli wrote: > On 07/06/2017 04:00 AM, Arkadi Sharshevsky wrote: >> >> >> On 07/05/2017 10:45 PM, Florian Fainelli wrote: >>> On 07/05/2017 08:36 AM, Arkadi Sharshevsky wrote: >>>> The bridge port attributes/vlan for DSA devices should be set only >>>> from bridge code. Furthermore, The vlans are synced totally with the >>>> bridge so there is no need for special dump support. >>>> >>>> Signed-off-by: Arkadi Sharshevsky <arka...@mellanox.com> >>> >>> I would be more comfortable if we still had a way to dump HW entries by >>> calling into drivers, because it's useful for debugging, and doing that >>> using standard tools plus an additional flag for instance: >>> >>> bridge fdb show >>> >>> would dump the SW-maintained VLANs, but: >>> >>> bridge fdb show hw >>> >> >> I agree that in your case, the FDBs are exception because they cannot be >> synced with the sw bridge due to lack of interrupt per learned entry, >> but note that this ability of bypass dump should not be supported by >> switchdev code because its contradicts the model. That is why I moved it >> inside DSA in one of the following patches (another reason is that >> nobody is using it beside you so there is no reason to leave it there). > > Sure, I agree with that, no need to clutter switchdev and other > switchdev only drivers like mlxsw with FDB dumping code that they don't > need. > >> >> This is especially true for vlans which are synced completely with the >> software bridge. >> >> Regarding debugging, if you want to see the hardware tables you can >> easily use dpipe for that. For example you can add table which does >> match on vlan and mac and set the output port, or some table that >> matches port and incoming packets vlan into internal metadata (FID) >> that can be used for lookup. >> >> I just don't think it is good idea for mixing this tools together. > > Well, that is once again debatable, ethtool for instance contains a > collection of debug and functional information about your hardware. > Having the ability to dump the VLAN tables using the standard bridge > vlan output has a lot of value in that you can easily correlated whether > the SW representation matches the HW representation (as fed by the > driver back into bridge's netlink messages). >
In my opinion all of this is very confusing. When the user does: $bridge vlan show dev sw1p5 port vlan ids sw1p5 1 PVID Egress Untagged sw1p5 1 PVID Egress Untagged and sees two set of identical vlans its just confusing, Furthermore if the hw/sw are different it even more weird. For example in what scenario would a user want to configure different PVID only for hw? I think that for switchdev device, in case the bridge wants to add a vlan and hardware fails, the operation should fail as well (done under rtnl so it can sleep). By this the user gains: 1. No confusion due to mismatched behavior hw/sw. Vlans should be synced!. 2. Indication about successful offload. So user knows if its in hardware or not. It seems that you already discussed it: https://www.mail-archive.com/netdev@vger.kernel.org/msg147067.html It even seems you have a bug here so deleting this bypass add/del /dump makes the API more consistent and fixes a bug. >> >>> would dump the HW-maintained VLANs, or any flag name really, but the >>> point is that we can keep using a standard tool to expose debugging >>> features. debugfs (or something else) could be used, but was rejected by >>> David. >>> >>> Also, technically, it ought to be possible to configure a port's VLAN >>> membership (although using poor semantics) through e.g: vconfig add >>> sw0p0 1, which would call ndo_vlan_rx_add_vid() (DSA does not support it >>> but mlxsw does), and so being able to dump that configuration would be >> >> Yeah, in mlxsw the vlan devices are used mainly for vlan unaware bridge, >> so the vlans will not be visible by the bridge, but the correct API to >> see this is via: >> >> $ip -d link show sw1p7.8 > > Since the VLAN-aware device is visible to user-space, my comment does > not really make sense in that this very interface existence indicates > that the port has been properly set up with a VLAN tag >
