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:     Vinicius Costa Gomes <vinicius.gomes@intel.com>
CC:     "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Xiaoliang Yang <xiaoliang.yang_1@nxp.com>,
        Rui Sousa <rui.sousa@nxp.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        "UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Michael Walle <michael@walle.cc>,
        Maxim Kochetkov <fido_max@inbox.ru>,
        Colin Foster <colin.foster@in-advantage.com>,
        Richie Pearn <richard.pearn@nxp.com>,
        Kurt Kanzenbach <kurt@linutronix.de>,
        Vladimir Oltean <olteanv@gmail.com>,
        Jesse Brandeburg <jesse.brandeburg@intel.com>,
        Tony Nguyen <anthony.l.nguyen@intel.com>,
        Giuseppe Cavallaro <peppe.cavallaro@st.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        Jose Abreu <joabreu@synopsys.com>,
        Jamal Hadi Salim <jhs@mojatatu.com>,
        Cong Wang <xiyou.wangcong@gmail.com>,
        Jiri Pirko <jiri@resnulli.us>,
        Gerhard Engleder <gerhard@engleder-embedded.com>,
        Grygorii Strashko <grygorii.strashko@ti.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next 04/13] net/sched: taprio: allow user input of
 per-tc max SDU
Thread-Topic: [PATCH net-next 04/13] net/sched: taprio: allow user input of
 per-tc max SDU
Thread-Index: AQHYyE9ZfT5bo6uCkEmd6s08JFlY763fdU4AgAAHuwA=
Date:   Wed, 14 Sep 2022 22:10:43 +0000
Message-ID: <20220914221042.oenxhxacgt2xsb2k@skbuf>
References: <20220914153303.1792444-1-vladimir.oltean@nxp.com>
 <20220914153303.1792444-5-vladimir.oltean@nxp.com> <87k065iqe1.fsf@intel.com>
In-Reply-To: <87k065iqe1.fsf@intel.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?w0Kw4YLgHi9DFx3MahcW7IiHVDWFlFZ65wlTGOqgRiAHSVHYO5UO1mExZRZ5?=
 =?us-ascii?Q?9qirMI3SIXbHeMlnj6iOGSkm/tyNwv+JbJj8SW7oS6xtanITbiqD4BRIKe5Z?=
 =?us-ascii?Q?T5DufAcQ8Da4hlauorXm7UtikvEBy4twbD+iw67+yiv9fyu0q5IeC83Vy/I7?=
 =?us-ascii?Q?ZCVRWpw+BSZtgx83GIILz/fVLaMOVJessZ5xcQfpPSgP2W0+wVaFHuQg13+G?=
 =?us-ascii?Q?NDCGDWSQsQnVzGHk4Tlwxj3nGwkDbzs0BUlY8Vr2+3nlAYNvMn3uYthc2fwM?=
 =?us-ascii?Q?sZdUzvPiSjFwnNSj4zbAOchkdufxjeVT1QfAuSP6tgG4F9DEJSMWx3RPKZ5J?=
 =?us-ascii?Q?I3IHT+Xov2jN5bqNiQiJd8rEYFsb5ojiE7K580Hp61uCNLzvUZbb45TME3ze?=
 =?us-ascii?Q?mEzHxeOlH82XTEzcXCxJJ9JyqRYej8vcg2YDSCOFLK1T7efp5jK3YbH40VOM?=
 =?us-ascii?Q?EXkwCdg0T5q1m6kjMGoKVaNhHJUK5VPhw4sR3SK4HVSCZ+HNVInXqV3icGdB?=
 =?us-ascii?Q?k2hbtSpNFhHWSvyQQfRyLWmt5rEJYU8oY+FtX9wvMTG2HzJTfUq81O8g4UYS?=
 =?us-ascii?Q?k/7wl9SAeIn3pmxCelFhSeobXRW5kZl6Ku/jsaSpKR3NJWdwBKXon/cNs8vE?=
 =?us-ascii?Q?UMlMG6wNESysPr/og5JUu9OfLTsPWeKiREDVKkfvEa+lV9HAcjYBFBWr2quX?=
 =?us-ascii?Q?tvix4sAKC1Y/JK+9nMyTd3vzCEtuTTZ1JYSZ/A8WI6koCSsqQ10Hik1me5iw?=
 =?us-ascii?Q?+dE0qsQi61YAmQuxKOt2qoiDNvhjc+CV/wcycs6bOxZkJoSPV3WQwjDKTcGQ?=
 =?us-ascii?Q?HpZsLTeLTmXLuC6+DNmhawpOZDAyA/DHSjvT6k7Bau48dCamEKSmWdc+NTYI?=
 =?us-ascii?Q?gFYg5HdsG7OuYMV4ZJWjeZolgRFCCVzbwEgiyw01Fo50RLajnRUSyTzwe3mM?=
 =?us-ascii?Q?H/8Xx6AdS51Vz5AXOlxJsMOMeh9tYAUta+k+jNu1famSDey9AnmfHbZJAIJ2?=
 =?us-ascii?Q?Pq4DhmtSt6Z7tIg1m7cj28kbv+UTlKf2UOZcSKXovOnZawkUeBvUysjA9LkM?=
 =?us-ascii?Q?C+nreAhiYY8g4WzHmI51SWSAq1uoL2bCFmgdrE3oU+MnyY280mEhaNmRKed9?=
 =?us-ascii?Q?cbLcmxWyeQa1Mb/ifwB+Kxnv+UWu7+OOLOCM61e8U6OHpfpE7fGe8HWvSCiM?=
 =?us-ascii?Q?9ejjgsU9Eu87hkNLncrDxEXBhrtP49dV+3whXGdDGwwIJTu8t12Z1Xja859A?=
 =?us-ascii?Q?fMViExJOPQVcAYrKjuzfpHYhc/y3mRGdOOT8gI5n1jlXfqtB+N6ceoTFN5Ej?=
 =?us-ascii?Q?I7cWs+UZ43b9BJ/qJ0guCWxEZdYzl3T6NkAsn+x448QLZcSZv2G8XXr+wnQz?=
 =?us-ascii?Q?pkSsP4IfRfISMyeGFNHwraJiLzcB7gyNt8JmNQaflKLcMfsdsigiwQd/NC5c?=
 =?us-ascii?Q?XHsNrSU8aeZyAm2Y3HwTmOq2GQ+COMVkqEy1z4K+TZKDf4AJ0zCjK8AjhbO/?=
 =?us-ascii?Q?ACCxvrP8b+75NxDWVK0YGS9zEp9B3UzyXyu0sSg4VCrNjhSyHNEvvq/sOhhk?=
 =?us-ascii?Q?bwP4Z45d1Z9Wu35fY8IFiHfdAoOWxVaizzkZ1IdTSDzTrwwEh4vE06sftSG8?=
 =?us-ascii?Q?eQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <258B0D48A865A04F99C494AF343C9F6C@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: 769e1f9e-c247-4aa1-4411-08da969df77a
X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Sep 2022 22:10:43.2051
 (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: h+m/atCdWOEpz+I6qVurK4AV1UGCQ5U8nFj4W/k/9cLy5kA4eoI9TvYuBGpfLdfsf0HQqySZ4aHSMM+nEr8qZQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB8836
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_NONE,SPF_HELO_PASS,T_SCC_BODY_TEXT_LINE,T_SPF_PERMERROR
        autolearn=no 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 Wed, Sep 14, 2022 at 02:43:02PM -0700, Vinicius Costa Gomes wrote:
> > @@ -416,6 +417,9 @@ static int taprio_enqueue_one(struct sk_buff *skb, =
struct Qdisc *sch,
> >  			      struct Qdisc *child, struct sk_buff **to_free)
> >  {
> >  	struct taprio_sched *q =3D qdisc_priv(sch);
> > +	struct net_device *dev =3D qdisc_dev(sch);
> > +	int prio =3D skb->priority;
> > +	u8 tc;
> > =20
> >  	/* sk_flags are only safe to use on full sockets. */
> >  	if (skb->sk && sk_fullsock(skb->sk) && sock_flag(skb->sk, SOCK_TXTIME=
)) {
> > @@ -427,6 +431,12 @@ static int taprio_enqueue_one(struct sk_buff *skb,=
 struct Qdisc *sch,
> >  			return qdisc_drop(skb, sch, to_free);
> >  	}
> > =20
> > +	/* Devices with full offload are expected to honor this in hardware *=
/
> > +	tc =3D netdev_get_prio_tc_map(dev, prio);
> > +	if (q->max_sdu[tc] &&
> > +	    q->max_sdu[tc] < max_t(int, 0, skb->len - skb_mac_header_len(skb)=
))
> > +		return qdisc_drop(skb, sch, to_free);
> > +
>=20
> One minor idea, perhaps if you initialize q->max_sdu[] with a value that
> you could use to compare here (2^32 - 1), this comparison could be
> simplified. The issue is that that value would become invalid for a
> maximum SDU, not a problem for ethernet.

Could do (and the fact that U32_MAX becomes a reserved value shouldn't
be a problem for any linklayer), but if I optimize the code for this one
place, I need, in turn, to increase the complexity in the netlink dump
and in the offload procedures, to hide what I've done.

If I look at the difference in generated code, maybe it's worth it
(I get rid of a "cbz" instruction). Maybe it's worth simply creating a
shadow array of q->max_sdu[], but which is also adjusted for something
like dev->hard_header_len (also a fast path invariant)? This way, we
could only check for q->max_frm_len[tc] > skb->len and save even more
checks in the fast path.=