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;
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Maxime Chevallier <maxime.chevallier@bootlin.com>
CC:     "davem@davemloft.net" <davem@davemloft.net>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "thomas.petazzoni@bootlin.com" <thomas.petazzoni@bootlin.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        Luka Perkov <luka.perkov@sartura.hr>,
        Robert Marko <robert.marko@sartura.hr>
Subject: Re: [PATCH net-next v4 2/5] net: dsa: add out-of-band tagging
 protocol
Thread-Topic: [PATCH net-next v4 2/5] net: dsa: add out-of-band tagging
 protocol
Thread-Index: AQHYxGBYKUV8EatjeEynFu0Ch2i3Kq3izGWA
Date:   Sat, 17 Sep 2022 00:15:22 +0000
Message-ID: <20220917001521.wskocisy53vozska@skbuf>
References: <20220909152454.7462-1-maxime.chevallier@bootlin.com>
 <20220909152454.7462-3-maxime.chevallier@bootlin.com>
In-Reply-To: <20220909152454.7462-3-maxime.chevallier@bootlin.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?XgvaS/fagbDA1i7+t/5um5Jf/imWnGKgJtfN/NMTJQw+cZ1nh0eVAWgkarkF?=
 =?us-ascii?Q?Kgk7W2uqhlc9G9hJvV7ftYSIHCzlZDYcnbYCvg/dWFBAmrUdrxQHBrJLtQod?=
 =?us-ascii?Q?BjvfiW7MQVezsfMu8Q/dm7zJ3U+MUinyKhDTQArxfRze/Kjk8NHA1k1EySwS?=
 =?us-ascii?Q?37+LYrMIE3RV3gttoMF2/ijO5XeKfEzTw1aQF3b0EL5ZO8WDuz9XbcQX3bmA?=
 =?us-ascii?Q?hNe9OrUmX/fiiJXRdUaiIM6xxmqESUgAcXw9rqVbtSSSWSq6jY7RjfzJaH2D?=
 =?us-ascii?Q?T8RmkSQwxHW/sjG9/GtBOjcu6jYU6tovnpmEGVg8rUS1PQudZ8ZGdws+NCBZ?=
 =?us-ascii?Q?4eRX89QDDlMPGHQWUVj3WfJwReaJhZ7sUTXBuJRIt7cYswsFhb9FFy6VhRzK?=
 =?us-ascii?Q?2QOqcPfeiAlB6wDJdb8xdwrXuwQ5FxB6A5vjeicCkzxezjbsmco3BRyVbz7+?=
 =?us-ascii?Q?RoOyazHtLg8IMbKyibJn2kdMNlI/RTjRYJ6QZAoaPZfP8o2ydtJAMB7bgafY?=
 =?us-ascii?Q?LdJryrXZl4+ykO9FunOF7itIEQAicv0FHXWEQvEik6c9ke2hqWNqnPhM3xSj?=
 =?us-ascii?Q?dUMRs3m2svtUrbjeFQ4cgkJdp/pB2J318GcRV/5Qcf6SMX4117jcKrioXLM4?=
 =?us-ascii?Q?CzpoIfDAaY0/oqxehb5qsAYVedekXsLd0SIR2dWls/jDB7tqWaFdXYmMk+C2?=
 =?us-ascii?Q?RRzw7Hte/DDWe4afwXhOfaWyhYNQEzg0wWOEwT2299ftocEd2crprl3hLxoT?=
 =?us-ascii?Q?LP9evNDo3wnAQLtvxV1tukRQq5+aJ1ssNZf4ujUWtyg70X3Rz9ggxQGlV8Ms?=
 =?us-ascii?Q?nyhnSnq2+NQtahcmNacuMPBDDk3HN5/YQiVBxdj2SyXRhZJ7NnpX6SezhLDx?=
 =?us-ascii?Q?PBhZRoNxxlWkeq+Xf4SggtqcYqERxiqlGWhn6ne0C+EG1rtho90nTQ+asI9J?=
 =?us-ascii?Q?Yx6tKH602brlM9b2eux9tX5pdBfWdL3bFleay7LCpJMk/ZjSmwR6yOVSjhm9?=
 =?us-ascii?Q?4TPHu1OK8A8gLTDINuQZ0Kr43kG8B082tPfCyQDLNtLhmGRyoYDGLJMVA33e?=
 =?us-ascii?Q?aApobqePGZlEQHspvN8nK5wjcJwOY4JUj+yZexxGR4fEmhdMr1le0hkoUosb?=
 =?us-ascii?Q?WO7pYD1XABm7ykAPn3JgJ1zO7i4mndxHwjQlrLiwb+QIi35cPnwsIoW2OWeU?=
 =?us-ascii?Q?vKa2iiU/JdUMtYgEeNl5WP9WySqHZGzbbTlPMZz+6A3QjNkjWmurfeecZQFL?=
 =?us-ascii?Q?MK8NKJrbG1u7oFwCl+9xUiEnFsuq7z84QA4A29KiMwbSwfEJs5k6l1cC6Z0l?=
 =?us-ascii?Q?xekeSTfLl25mSEPskjpXlJazGECMKWx3vFIszcnER/Pj4MQNHFOb7EQAZhmG?=
 =?us-ascii?Q?gosFHoEgFrgjNonqUHrg+YpUNIlE1VGE01vuApewm2K2IbdN2Nf7H1NU/bnO?=
 =?us-ascii?Q?4BGKBzAHxuFamzXjlgKOb3tOOQAahxK+eF+ffwUNXpjraW4KZ+4eWtW++RnU?=
 =?us-ascii?Q?7Lspby4Sh30OvhUyQ7KQcYY1IR547P85WtGGHvDVOXwWyZiRZtVQqZ8pocJo?=
 =?us-ascii?Q?5TppjbQTrwKT/Co6CjnrUO9u7GU1xuavmJMcqpsZZJwTqxhuVAS+sNlj4wz7?=
 =?us-ascii?Q?GQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <645D74E6B5A3C84480CB1A6114E4F10D@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5136.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: eecae46b-b5fb-4769-bb81-08da9841b68a
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Sep 2022 00:15:22.8563
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: j+XQ2JJnHpm7JFO4UEF4ATYVZvhAcsCVwOm7VqFSOb/ESE63hYc8hShGLoJ7fEJ8dSqnMIYjB/09n+kr2khw1w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB7815
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS 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

Hi Maxime,

On Fri, Sep 09, 2022 at 05:24:51PM +0200, Maxime Chevallier wrote:
> +int dsa_oob_tag_push(struct sk_buff *skb, struct dsa_oob_tag_info *ti)
> +{
> +	struct dsa_oob_tag_info *tag_info;
> +
> +	tag_info =3D (struct dsa_oob_tag_info *)skb->head;
> +
> +	tag_info->proto =3D ti->proto;
> +	tag_info->dp =3D ti->dp;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(dsa_oob_tag_push);
> +
> +int dsa_oob_tag_pop(struct sk_buff *skb, struct dsa_oob_tag_info *ti)
> +{
> +	struct dsa_oob_tag_info *tag_info;
> +
> +	tag_info =3D (struct dsa_oob_tag_info *)skb->head;
> +
> +	if (tag_info->proto !=3D DSA_TAG_PROTO_OOB)
> +		return -EINVAL;
> +
> +	ti->proto =3D tag_info->proto;
> +	ti->dp =3D tag_info->dp;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(dsa_oob_tag_pop);
> +
> +static struct sk_buff *oob_tag_xmit(struct sk_buff *skb,
> +				    struct net_device *dev)
> +{
> +	struct dsa_port *dp =3D dsa_slave_to_port(dev);
> +	struct dsa_oob_tag_info tag_info;
> +
> +	tag_info.dp =3D dp->index;
> +	tag_info.proto =3D DSA_TAG_PROTO_OOB;
> +
> +	if (dsa_oob_tag_push(skb, &tag_info))
> +		return NULL;
> +
> +	return skb;
> +}

I don't have too many comments on this patch set, except for a very
fundamental one. It is impossible to pass a DSA out of band header
between the switch tagging protocol driver and the host Ethernet
controller via the beginning of skb->head, and just putting some magic
bytes there and hoping that no random junk in the buffer will have the
same value (and that skb_push() calls will not eat into your tag_info
structure which isn't accounted for in any way by skb->data).

Please create an skb extension for this, it is the only unambiguous way
to deal with the given hardware, which will not give lots of headaches
in the future.=