netdev - Re: [PATCH net] net: dsa: mv88e6xxx: override existent unicast portvec in port_fdb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+h21hrn6q8NAdma3Djov82sNzHTz_tF480Nqpw-A+JLv_TYcQ@mail.gmail.com>
Date:   Mon, 1 Feb 2021 00:20:53 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     DENG Qingfang <dqfext@...il.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        netdev <netdev@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Tobias Waldekranz <tobias@...dekranz.com>
Subject: Re: [PATCH net] net: dsa: mv88e6xxx: override existent unicast
 portvec in port_fdb_add

On Sun, Jan 31, 2021 at 09:13:15AM +0800, DENG Qingfang wrote:
> This bug is exposed when I try your patch series on kernel 5.4
> https://lore.kernel.org/netdev/20210106095136.224739-1-olteanv@gmail.com/
> https://lore.kernel.org/netdev/20210116012515.3152-1-tobias@waldekranz.com/
>
> Without this patch, DSA will add a new port bit to the existing
> portvec when a client moves to the software part of a bridge. When it
> moves away, DSA will clear the port bit but the existing one will
> remain static. This results in connection issues when the client moves
> to a different port of the switch, and the kernel log below.
>
> mv88e6085 f1072004.mdio-mii:00: ATU member violation for
> xx:xx:xx:xx:xx:xx portvec dc00 spid 0

Ah, ok, DSA adds an FDB entry behind the user's back and it relies upon
the driver behavior being 'override'. A bit subtle, though it gives one
good reason against someone suggesting "why don't you just refuse adding
the new entry instead of overriding, like the software bridge does".
Probably the refusal of overwriting an entry is what needs to be handled
at upper layers, we do need to be able to override from DSA.
I had a quick look through our other drivers and it seems that all of
them are happy to override an existing FDB entry (or at least the
software part is).