• Vladimir Oltean's avatar
    net: dsa: delete dsa_legacy_fdb_add and dsa_legacy_fdb_del · b117e1e8
    Vladimir Oltean authored
    We want to add reference counting for FDB entries in cross-chip
    topologies, and in order for that to have any chance of working and not
    be unbalanced (leading to entries which are never deleted), we need to
    ensure that higher layers are sane, because if they aren't, it's garbage
    in, garbage out.
    
    For example, if we add a bridge FDB entry twice, the bridge properly
    errors out:
    
    $ bridge fdb add dev swp0 00:01:02:03:04:07 master static
    $ bridge fdb add dev swp0 00:01:02:03:04:07 master static
    RTNETLINK answers: File exists
    
    However, the same thing cannot be said about the bridge bypass
    operations:
    
    $ bridge fdb add dev swp0 00:01:02:03:04:07
    $ bridge fdb add dev swp0 00:01:02:03:04:07
    $ bridge fdb add dev swp0 00:01:02:03:04:07
    $ bridge fdb add dev swp0 00:01:02:03:04:07
    $ echo $?
    0
    
    But one 'bridge fdb del' is enough to remove the entry, no matter how
    many times it was added.
    
    The bridge bypass operations are impossible to maintain in these
    circumstances and lack of support for reference counting the cross-chip
    notifiers is holding us back from making further progress, so just drop
    support for them. The only way left for users to install static bridge
    FDB entries is the proper one, using the "master static" flags.
    
    With this change, rtnl_fdb_add() falls back to calling
    ndo_dflt_fdb_add() which uses the duplicate-exclusive variant of
    dev_uc_add(): dev_uc_add_excl(). Because DSA does not (yet) declare
    IFF_UNICAST_FLT, this results in us going to promiscuous mode:
    
    $ bridge fdb add dev swp0 00:01:02:03:04:05
    [   28.206743] device swp0 entered promiscuous mode
    $ bridge fdb add dev swp0 00:01:02:03:04:05
    RTNETLINK answers: File exists
    
    So even if it does not completely fail, there is at least some indication
    that it is behaving differently from before, and closer to user space
    expectations, I would argue (the lack of a "local|static" specifier
    defaults to "local", or "host-only", so dev_uc_add() is a reasonable
    default implementation). If the generic implementation of .ndo_fdb_add
    provided by Vlad Yasevich is a proof of anything, it only proves that
    the implementation provided by DSA was always wrong, by not looking at
    "ndm->ndm_state & NUD_NOARP" (the "static" flag which means that the FDB
    entry points outwards) and "ndm->ndm_state & NUD_PERMANENT" (the "local"
    flag which means that the FDB entry points towards the host). It all
    used to mean the same thing to DSA.
    
    Update the documentation so that the users are not confused about what's
    going on.
    Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    b117e1e8
slave.c 62.5 KB