netdev - Re: [PATCH net-next 1/4] net: dsa: sja1105: unconditionally set DESTMETA and SRCMETA in AVB table

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] [day] [month] [year] [list]

Date:   Tue, 24 Mar 2020 11:07:29 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Christian Herber <christian.herber@....com>
Cc:     "davem@...emloft.net" <davem@...emloft.net>,
        "richardcochran@...il.com" <richardcochran@...il.com>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "vivien.didelot@...il.com" <vivien.didelot@...il.com>,
        "andrew@...n.ch" <andrew@...n.ch>, "Y.b. Lu" <yangbo.lu@....com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 1/4] net: dsa: sja1105: unconditionally set
 DESTMETA and SRCMETA in AVB table

Hi Christian,

On Tue, 24 Mar 2020 at 09:39, Christian Herber <christian.herber@....com> wrote:
>
> >+static int sja1105_init_avb_params(struct sja1105_private *priv)
> >+{
> >+       struct sja1105_avb_params_entry *avb;
> >+       struct sja1105_table *table;
> >+
> >+       table = &priv->static_config.tables[BLK_IDX_AVB_PARAMS];
> >+
> >+       /* Discard previous AVB Parameters Table */
> >+       if (table->entry_count) {
> >+               kfree(table->entries);
> >+               table->entry_count = 0;
> >+       }
> >+
> >+       table->entries = kcalloc(SJA1105_MAX_AVB_PARAMS_COUNT,
> >+                                table->ops->unpacked_entry_size, GFP_KERNEL);
> >+       if (!table->entries)
> >+               return -ENOMEM;
> >+
> >+       table->entry_count = SJA1105_MAX_AVB_PARAMS_COUNT;
> >+
> >+       avb = table->entries;
> >+
> >+       /* Configure the MAC addresses for meta frames */
> >+       avb->destmeta = SJA1105_META_DMAC;
> >+       avb->srcmeta  = SJA1105_META_SMAC;
> >+
> >+       return 0;
> >+}
> >+
>
> Would it be possible to use the MAC address of the connected eth as destination? This is nicer also when going over multiple cascaded switches.
>
> Christian

The current destination MAC is 01:80:C2:00:00:0E, which is already a
bridge link-local multicast address that switches are supposed to trap
towards the CPU and not forward.
The way things work right now is that the driver assumes user space
will add the multicast MAC addresses required for PTP operation on the
slave net device, and DSA will propagate those addresses to the master
port as well.
I think your suggestion might make a difference when the PTP user
space stack exits, and the slave ports are operating in standalone
(non-bridged) mode. There is an issue in that case, where the switch
still sends RX timestamps (since nobody disabled RX timestamping via
the ioctl), but the multicast MAC address is no longer in the DSA
master's RX filter. So the kernel log is spammed by the state machine
in net/dsa/tag_sja1105.c getting upset about meta frames being
expected but not received.
If anything, I'm a bit worried about the source MAC address (currently
22:22:22:22:22:22) which is potentially non-unique, although I need to
do more testing to see if this is actually a problem. I have a board
with a DSA switch acting as 3 DSA masters on 3 ports, on each port
having cascaded a sja1105 switch, and source address learning (of
22:22:22:22:22:22) on the top-level DSA switch is what concerns me.

Thanks,
-Vladimir