Hi Alban, On Mon, Jan 18, 2021 at 05:03:17PM +0100, Alban Bedel wrote: > Multicast entries in the MAC table use the high bits of the MAC > address to encode the ports that should get the packets. But this port > mask does not work for the CPU port, to receive these packets on the > CPU port the MAC_CPU_COPY flag must be set. > > Because of this IPv6 was effectively not working because neighbor > solicitations were never received. This was not apparent before commit > 9403c158 (net: mscc: ocelot: support IPv4, IPv6 and plain Ethernet mdb > entries) as the IPv6 entries were broken so all incoming IPv6 > multicast was then treated as unknown and flooded on all ports. > > To fix this problem add a new `flags` parameter to ocelot_mact_learn() > and set MAC_CPU_COPY when the CPU port is in the port set. We still > leave the CPU port in the bitfield as it doesn't seems to hurt. > > Signed-off-by: Alban Bedel <alban.be...@aerq.com> > Fixes: 9403c158 (net: mscc: ocelot: support IPv4, IPv6 and plain Ethernet mdb > entries) > ---
Good catch, it seems that I really did not test that patch with multicast traffic received on the CPU (and not only that patch, but ever since, in fact), shame on me. What I don't like your patch is how it spills over the entire ocelot driver, yet still fails to compile. You missed a bunch of ocelot_mact_learn calls from ocelot_net.c (8 of them, in fact). I don't know which kernel tree you applied this patch to, but clearly not "net"/master: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git I would prefer to see a more self-contained bug fix, such as potentially this one: -----------------------------[cut here]----------------------------- diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c index a560d6be2a44..4d7443b123bd 100644 --- a/drivers/net/ethernet/mscc/ocelot.c +++ b/drivers/net/ethernet/mscc/ocelot.c @@ -56,18 +56,46 @@ static void ocelot_mact_select(struct ocelot *ocelot, } +static unsigned long +ocelot_decode_ports_from_mdb(const unsigned char *addr, + enum macaccess_entry_type entry_type) +{ + unsigned long ports = 0; + + if (entry_type == ENTRYTYPE_MACv4) { + ports = addr[2]; + ports |= addr[1] << 8; + } else if (entry_type == ENTRYTYPE_MACv6) { + ports = addr[1]; + ports |= addr[0] << 8; + } + + return ports; +} + int ocelot_mact_learn(struct ocelot *ocelot, int port, const unsigned char mac[ETH_ALEN], unsigned int vid, enum macaccess_entry_type type) { + u32 flags = ANA_TABLES_MACACCESS_VALID | + ANA_TABLES_MACACCESS_DEST_IDX(port) | + ANA_TABLES_MACACCESS_ENTRYTYPE(type) | + ANA_TABLES_MACACCESS_MAC_TABLE_CMD(MACACCESS_CMD_LEARN); + ocelot_mact_select(ocelot, mac, vid); + /* Little API trickery to make this function "just work" when the CPU + * port module is included in the port mask for multicast IP entries. + */ + if (type == ENTRYTYPE_MACv4 || type == ENTRYTYPE_MACv6) { + unsigned long ports = ocelot_decode_ports_from_mdb(mac, type); + + if (ports & BIT(ocelot->num_phys_ports)) + flags |= ANA_TABLES_MACACCESS_MAC_CPU_COPY; + } + /* Issue a write command */ - ocelot_write(ocelot, ANA_TABLES_MACACCESS_VALID | - ANA_TABLES_MACACCESS_DEST_IDX(port) | - ANA_TABLES_MACACCESS_ENTRYTYPE(type) | - ANA_TABLES_MACACCESS_MAC_TABLE_CMD(MACACCESS_CMD_LEARN), - ANA_TABLES_MACACCESS); + ocelot_write(ocelot, flags, ANA_TABLES_MACACCESS); return ocelot_mact_wait_for_completion(ocelot); } -----------------------------[cut here]----------------------------- It has the advantage of actually compiling, plus it should be easier to backport because the changes are all in one place. Please make sure to read: Documentation/process/submitting-patches.rst (this will tell you what is wrong with your Fixes: tag) Documentation/networking/netdev-FAQ.rst (this will tell you what is wrong with this patch's --subject-prefix, and why the patch does not build on the trees it is supposed to be applied to): https://patchwork.kernel.org/project/netdevbpf/patch/20210118160317.554018-1-alban.be...@aerq.com/
