From:   Hariprasad Kelam <hkelam@marvell.com>
To:     Maxim Mikityanskiy <maxtram95@gmail.com>,
        Jakub Kicinski <kuba@kernel.org>
CC:     "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "edumazet@google.com" <edumazet@google.com>,
        Sunil Kovvuri Goutham <sgoutham@marvell.com>,
        Linu Cherian <lcherian@marvell.com>,
        Geethasowjanya Akula <gakula@marvell.com>,
        Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
        Subbaraya Sundeep Bhatta <sbhatta@marvell.com>,
        "jhs@mojatatu.com" <jhs@mojatatu.com>,
        "xiyou.wangcong@gmail.com" <xiyou.wangcong@gmail.com>,
        "jiri@resnulli.us" <jiri@resnulli.us>,
        "saeedm@nvidia.com" <saeedm@nvidia.com>,
        "richardcochran@gmail.com" <richardcochran@gmail.com>,
        "tariqt@nvidia.com" <tariqt@nvidia.com>,
        "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
        "hariprasad.netdev@gmail.com" <hariprasad.netdev@gmail.com>,
        Naveen Mamindlapalli <naveenm@marvell.com>
Subject: Re: [net-next Patch v2 4/5] octeontx2-pf: Add devlink support to
 configure TL1 RR_PRIO
Thread-Topic: [net-next Patch v2 4/5] octeontx2-pf: Add devlink support to
 configure TL1 RR_PRIO
Thread-Index: AQHZL+n1iE8p6ldbf0mFARjxWm2brg==
Date:   Tue, 24 Jan 2023 11:49:48 +0000
Message-ID: <PH0PR18MB44748DFCCABCDC2806A2DC2CDEC99@PH0PR18MB4474.namprd18.prod.outlook.com>
References: <20230118105107.9516-1-hkelam@marvell.com>
 <20230118105107.9516-5-hkelam@marvell.com> <Y8hYlYk/7FfGdfy8@mail.gmail.com>
 <PH0PR18MB4474FCEAC4FA5907CAC17011DEC59@PH0PR18MB4474.namprd18.prod.outlook.com>
 <Y8qZNhUgsdOMavC4@mail.gmail.com>
 <PH0PR18MB4474DBEF155EFA4DA6BA5B10DEC59@PH0PR18MB4474.namprd18.prod.outlook.com>
 <Y803rePcLc97CGik@mail.gmail.com>
 <PH0PR18MB44741D5EBBD7B4010C78C7DFDEC89@PH0PR18MB4474.namprd18.prod.outlook.com>
 <Y87onaDuo8NkFNqC@mail.gmail.com> <20230123144548.4a2c06ae@kernel.org>
 <Y88Rug7iaC0nOGvu@mail.gmail.com>
In-Reply-To: <Y88Rug7iaC0nOGvu@mail.gmail.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?xydTHhS781fsIBoSGnWlYSVIbkbIcKYWbd+nOJAOJCYzmLu7D/2jo65YYSMa?=
 =?us-ascii?Q?gJB8aogBp4nJypHmwyV0J3An1KcN0hZp0P0ed5aYCdWgphgfNsxSs2zmQOjB?=
 =?us-ascii?Q?hdda+rxyoTXy9m6txthS89loZ9RLhs5irZxjGifx7VEL/y46jDhYF959pcae?=
 =?us-ascii?Q?8TzswoFGs0ylNIz+FIuN92oynPLo/0RpiXmyOfXyTN7cTmZT+88OnGY0Ov1A?=
 =?us-ascii?Q?3i7EngRSNBw+hDxMRnXymBaTmcaIWqczilKharnOZO0BdHe4k+GBMaJmMg3T?=
 =?us-ascii?Q?x/iZ8i+UCeueMYn9zwUNiG5QUt90/Zd1gVWBm0bIwOwe9QDwa4FT3v5efCnl?=
 =?us-ascii?Q?MKMKwvo/I9wY4dfWvcjSyLvTqPXz98jCWHK3spcW2XK77Bu2efMPeLa5ANNY?=
 =?us-ascii?Q?aThlFwS/MK8VF/8f4+UiUsahQAXHJQPJ9dKbUbC27AQyn3cRvvGkbUAjAGRI?=
 =?us-ascii?Q?RcS7ltIF48hhUjfbcxYbM5gbHsmXTM1PQw7HnZZFuB9CZ7mMGD9vyGLZw5J3?=
 =?us-ascii?Q?gq9bI/LYuQJzx8V5Z0Nt1rwtcOiz69WzIzD1C+6joZm6RADKscRQxJIMOEXn?=
 =?us-ascii?Q?UgSS/7VL/AiETlwcztczoN4ei6NLV7xzFSawux8/meqW3FGBYSMnUj3opkLP?=
 =?us-ascii?Q?4ZM9MyGFp0jskhwm/p0BuAq7fKYlndLvmAHVvDfFwXhZXiPt+F6YpjmpVZr9?=
 =?us-ascii?Q?DqhtA6a9t5XCPKPG2D6ipDgjkvmJ58Sj19nsUvh5JGoh1IigfWLIbqOGTA2m?=
 =?us-ascii?Q?dmhQ16YG8SStS4RGoKZAeJKgYuAezZZfGmct5qmuqtwwzc4Vp/LP+9jK5mDh?=
 =?us-ascii?Q?GNGpXqH6YRd2VD0OSDHtu9AbRkX9e8e0SWM2BRMuC6QGRgvLQuqL+E5/Bl5g?=
 =?us-ascii?Q?6e49zVNl4NE4RANnmpBH9rRwRmw4M1Jyq/dviJqH1ODSjpNh+YaVc7cOfW2J?=
 =?us-ascii?Q?PM/qemwMPmIcL14BGBlHlan0QDspVsmjnk5xVezrrMYdeEz+NEUjg7cYmBaj?=
 =?us-ascii?Q?qqYQW9g2aX3e7gULqHTYTp+0nCt4qA1OwVyBGm2OCziAzO28v+PplaKQv1Dc?=
 =?us-ascii?Q?FdnQZiEjtOgYfF618xUwGGF0R2FXakJVqPa/qUBIXzLgNCz1kUEpL1tvlgdV?=
 =?us-ascii?Q?+lN5i/boHdKw4l16AdyvnmNqxQMh8ZH1I5PjPi/oU/keQeck1Yucun+UiUzW?=
 =?us-ascii?Q?iBgZK7rWNkF/kvoaTjVyJDyZsaA0JAHxPuyczM7LIAH5Jdh90Wz0JC3aN4yY?=
 =?us-ascii?Q?7WxQZVktMSiPFTZ0652VzTsl4r2nUOhN24naDpkEto8ptCPF5B4mN252hWMu?=
 =?us-ascii?Q?M5XpZX+6MHfsr3Jd7DNJe1dEYtfQTMROSZZEWAnocE14eSsat7D1Z9AsrW0N?=
 =?us-ascii?Q?wzljoWZHfJf02vXLdhcAEybxdICPY6SBXQjvv/G0ueIB9RdfisrxJt4V9Hru?=
 =?us-ascii?Q?AGaPiF/UB13rhBgjj+fYebylPuYxyRTBlG5MhdyPJXKhmi2PNCssyysw5J8E?=
 =?us-ascii?Q?tPGQhedvv5BE6MJMCXCY+LKC+tqtjuMnTJD7p63GlEiWNkK6tnRuVxuhdIir?=
 =?us-ascii?Q?dFI+LMmVZe+yD3DA0ncSEtrgJ6X7RP1p75b8cnoTvdlEi4X/J/WyOR66l+AM?=
 =?us-ascii?Q?/bj9rXQdQ2zkYrz+aPWHg9GJsjMBPCwlF9F5/pHtZ64E?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR18MB4474.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d9964c2f-947a-40a4-ef8c-08dafe011855
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Jan 2023 11:49:48.2429
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: R3Qwsok0PGAe1QC6dUIVl/XVx5pSOSIgUsPutPum2JwipAE6vuEiNHuk106qUOXOA3UMz/9R0g5uG8dOPAkHOw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR18MB4222
X-Proofpoint-GUID: TDe45cSI4Wv05FTrXBf7ptq5rkHTeXWf
X-Proofpoint-ORIG-GUID: TDe45cSI4Wv05FTrXBf7ptq5rkHTeXWf
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.930,Hydra:6.0.562,FMLib:17.11.122.1
 definitions=2023-01-23_12,2023-01-24_01,2022-06-22_01
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Mon, Jan 23, 2023 at 02:45:48PM -0800, Jakub Kicinski wrote:
> > On Mon, 23 Jan 2023 22:05:58 +0200 Maxim Mikityanskiy wrote:
> > > OK, I seem to get it now, thanks for the explanation!
> > >
> > > How do you set the priority for HTB, though? You mentioned this
> > > command to set priority of unclassified traffic:
> > >
> > > devlink -p dev param set pci/0002:04:00.0 name tl1_rr_prio value 6 \
> > > cmode runtime
> > >
> > > But what is the command to change priority for HTB?
> > >
> > > What bothers me about using devlink to configure HTB priority is:
> > >
> > > 1. Software HTB implementation doesn't have this functionality, and
> > > it always prioritizes unclassified traffic. As far as I understand,
> > > the rule for tc stuff is that all features must have a reference
> > > implementation in software.
> > >
> > > 2. Adding a flag (prefer unclassified vs prefer classified) to HTB
> > > itself may be not straightforward, because your devlink command has
> > > a second purpose of setting priorities between PFs/VFs, and it may
> > > conflict with the HTB flag.
> >
> > If there is a two-stage hierarchy the lower level should be controlled
> > by devlink-rate, no?
>=20
> From the last picture by Hariprasad, I understood that the user sets all
> priorities (for unclassified traffic per PF and VF, and for HTB traffic) =
on the
> same TL2 level, i.e. it's not two-stage. (Maybe I got it all wrong again?=
)
>=20
> I asked about the command to change the HTB priority, cause the
> parameters aren't easily guessed, but I assume it's also devlink (i.e.
> driver-specific).
>=20
Currently, we don't support changing HTB priority since TC_HTB_MODIFY is no=
t yet supported.
The driver implementation is inline with htb tc framework, below are comman=
ds we use for setting htb priority

ethtool -K eth0 hw-tc-offload on
tc qdisc replace dev eth0 root handle 1: htb offload
tc class add dev eth0 parent 1: classid 1:1 htb rate 10Gbit prio 2
tc class add dev eth0 parent 1: classid 1:1 htb rate 10Gbit prio 3


> If there were two levels (the upper level chooses who goes first: HTB or
> unclassified, and the lower level sets priorities per PF and VF for uncla=
ssified
> traffic), that would be more straightforward to solve: the upper level sh=
ould
> be controlled by a new HTB parameter, and the lower level is device-speci=
fic,
> thus it goes to devlink.

The PF netdev and VFs share the same physical link and share the same TL1 n=
ode.
Hardware supports one DWRR group and the rest are strict priority nodes. Dr=
iver configures
the priority set by devlink to PF and VF traffic TL2 nodes such that traffi=
c is forwarded
to TL1 using DWRR algorithm.

Now that if we add htb command for unclassified traffic, at any given point=
 in time HTB
rule only applies to a single interface, since we require to set DWRR prior=
ity at TL1,=20
which applies to both PF/VFs, we feel it's a different use case altogether.