Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 30 Jun 2023 15:35:23 +0200
From:   Simon Horman <simon.horman@corigine.com>
To:     Vladimir Oltean <vladimir.oltean@nxp.com>
Cc:     netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net 2/2] net: dsa: sja1105: always enable the send_meta
 options
Message-ID: <ZJ7aG/tzAxutRoeo@corigine.com>
References: <20230629141453.1112919-1-vladimir.oltean@nxp.com>
 <20230629141453.1112919-3-vladimir.oltean@nxp.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230629141453.1112919-3-vladimir.oltean@nxp.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?vwzhsLP+DVhVYrokX9gV06BEXnjto5pIpi4L6XBZ+5Qx7ue6+d7zhexYIXaU?=
 =?us-ascii?Q?fkDh1aH3LFBRAgtjm91QW9wFbww7ZYUBuvyCvfSxzJDnCUxfJwgeyAgkVWCS?=
 =?us-ascii?Q?J97mQ73ZnUwm0MrjKX5D1qOS7U93QuoYNg8hKlRPOp0NI+BFeO28TzbI5Fus?=
 =?us-ascii?Q?6qhqZfVuMW0KElWG/FErS/c3alim6Rhe0u8/giT/E5zO4qLn4GJPd3XUTNyg?=
 =?us-ascii?Q?1wb26wm/FtCBTeHoCBGtg0O9/ubtCSK52daOK8u99O1nk8z+8F1kIWDkwCxe?=
 =?us-ascii?Q?yItFlvmYR8KC1Yfi7rz1PBpr9YLYarWFLn1pSmOrbqIBEDMAeSvhBGbNByJP?=
 =?us-ascii?Q?IR1UXFgQe0rnfNf/5AyiGW2jmvgzYCM9DxA2acgN+HzeECfXuvCx9dri3Oja?=
 =?us-ascii?Q?Nr94CW/FsaDQbRlfT1BL8a3vtoK0iUrmcrx7kUfl0AK20Muv3iVHLze7BW6i?=
 =?us-ascii?Q?HNsWAGmJlk01bEgDJCXbXaKl2AwayqtqH4Cn2D/oblmUfOvpMuIt3C7MDr+x?=
 =?us-ascii?Q?1jGVNSXDosThgeFVX9xMsZhlLIAX8YrtQfyUU4+LCFIjScmLpuwGEr9WywmV?=
 =?us-ascii?Q?OgvwxDH5pZM9BpXuv19Nx30Gs5UZyzGOd3b2FhAfAJkjWhiffaSnU0ZbQHp9?=
 =?us-ascii?Q?4WiLGEQ8mDwfN/qY+E09GpQM8/mKlmBS37qTpefyE5stp09M8J/hw2qk8+HQ?=
 =?us-ascii?Q?N76dPMwayV1kMZL6oaTDpuehVadxr4TSmjXcaZ11BjnMyXm9dJfDrHWIUWZ3?=
 =?us-ascii?Q?vLt93RYt3RcCvtb9o8YhTbWTCKMjqf43iabmZGqDoWR8dj1DY5GfsjVPsp/e?=
 =?us-ascii?Q?JmbBAVRh3jurFIIFB8Gs067Ba/tXKw+o0UXNiRsnd491/97PAv1vYgZezw3m?=
 =?us-ascii?Q?tnF0mnzp0fejClVjkxE2GvhwA0onAi+t7WeFG0fKb+KNtJl9nJdG3Sscmmgq?=
 =?us-ascii?Q?f75v5oEKg3P9IL0rJ9e1GwCDqM+Cd9N40BE8/R8+gE4PSPfAsTibSjHtErLP?=
 =?us-ascii?Q?paddVK9tJmcrd8vYu/jyhBnrVEnwvw+oZ1MSPF0E1trYyewu+Na63EsOReLY?=
 =?us-ascii?Q?3oe+LyyYLBW4kaiYTTlPrcJKCeQIBKANxx3He03DFxkJ5DNcCM3rie50fXJ8?=
 =?us-ascii?Q?7URtiHiqiqvNKH18/KiWBJb5YO4CWSsosgjyhB8Wehzz1vF/SUfWhB1Auu3h?=
 =?us-ascii?Q?80AaiKKHgkDkdXkRhOrvUnShi+HlNb7fCwWV6xKZVzPMSL/WRWUuuX/tcFus?=
 =?us-ascii?Q?NxAciRNzNjVUan5YDeeEVQndwPbLBEVraRAGUxZm5SB42v55wIYu5PbWp646?=
 =?us-ascii?Q?KywwbeYqo25XBZvyLeIvkfDr4NR5phcsZr96eVngQi7B/DAZRyiK89D2TQYz?=
 =?us-ascii?Q?l3OklFMzU03wgeOjSzCiB0TAmPKzckKY9IZSRaynRjlDeqp2wgQamAByMtCv?=
 =?us-ascii?Q?JK4+PmWjdKc2oSGmepCv32FQXxEdOV+IUutXh6psHtYyPaKpekLN1IKDylrf?=
 =?us-ascii?Q?hLwLiknwXrckCPWd43mmqX/LkZnhFH+lzQSh5Lz9hefJUAauY1N1C10BTKd0?=
 =?us-ascii?Q?OX0unnblymIYKQnaAJLIGUJssxwJSXeSqRPeep/VZ9qU4iD62A8MDTlY6FUz?=
 =?us-ascii?Q?V4U72gTn4bHMKxLEOLMhyc03leFJrqdhN/iwUb6oGpHGpIU+li6nn/Puze0R?=
 =?us-ascii?Q?fM5w+Q=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cde2cf67-5adc-4a1e-b9a5-08db796ee5f6
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jun 2023 13:35:41.5837
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 3X22N0I1+RTDzBJ9ILdhqQuJEdT7cPgIqEWAXv6C6aMuXwpw4FIQJGOfcgyxjygsez1bCcwdCh9EiZoAhOqLzujm3sqLNJMyuD6UVlCzsvQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR13MB5619
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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@nxp.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@corigine.com>

...