netdev - Re: [PATCH net 2/2] net: dsa: sja1105: always enable the send

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: <ZJ7aG/tzAxutRoeo@corigine.com>
Date: Fri, 30 Jun 2023 15:35:23 +0200
From: Simon Horman <simon.horman@...igine.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
	Florian Fainelli <f.fainelli@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net 2/2] net: dsa: sja1105: always enable the send_meta
 options

On Thu, Jun 29, 2023 at 05:14:53PM +0300, Vladimir Oltean wrote:
> incl_srcpt has the limitation, mentioned in commit b4638af8885a ("net:
> dsa: sja1105: always enable the INCL_SRCPT option"), that frames with a
> MAC DA of 01:80:c2:xx:yy:zz will be received as 01:80:c2:00:00:zz unless
> PTP RX timestamping is enabled.
> 
> The incl_srcpt option was initially unconditionally enabled, then that
> changed with commit 42824463d38d ("net: dsa: sja1105: Limit use of
> incl_srcpt to bridge+vlan mode"), then again with b4638af8885a ("net:
> dsa: sja1105: always enable the INCL_SRCPT option"). Bottom line is that
> it now needs to be always enabled, otherwise the driver does not have a
> reliable source of information regarding source_port and switch_id for
> link-local traffic (tag_8021q VLANs may be imprecise since now they
> identify an entire bridging domain when ports are not standalone).
> 
> If we accept that PTP RX timestamping (and therefore, meta frame
> generation) is always enabled in hardware, then that limitation could be
> avoided and packets with any MAC DA can be properly received, because
> meta frames do contain the original bytes from the MAC DA of their
> associated link-local packet.
> 
> This change enables meta frame generation unconditionally, which also
> has the nice side effects of simplifying the switch control path
> (a switch reset is no longer required on hwtstamping settings change)
> and the tagger data path (it no longer needs to be informed whether to
> expect meta frames or not - it always does).
> 
> Fixes: 227d07a07ef1 ("net: dsa: sja1105: Add support for traffic through standalone ports")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
>  drivers/net/dsa/sja1105/sja1105.h      |  2 +-
>  drivers/net/dsa/sja1105/sja1105_main.c |  5 ++-
>  drivers/net/dsa/sja1105/sja1105_ptp.c  | 48 +++-----------------------
>  include/linux/dsa/sja1105.h            |  4 ---
>  net/dsa/tag_sja1105.c                  | 45 ------------------------
>  5 files changed, 7 insertions(+), 97 deletions(-)

Hi Vladimir,

this patch isn't that big, so I'm ok with it.  But it also isn't that
small, so I'd just like to mention that a different approach might be a
small patch that enables meta frame generation unconditionally, as a fix.
And then, later, some cleanup, which seems to comprise most of this patch.

I do admit that I didn't try this. So it might not be sensible.  And as I
said, I am ok with this patch. But I did think it was worth mentioning.

Reviewed-by: Simon Horman <simon.horman@...igine.com>

...