From:   <Daniel.Machon@microchip.com>
To:     <simon.horman@corigine.com>
CC:     <netdev@vger.kernel.org>, <davem@davemloft.net>,
        <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
        <Lars.Povlsen@microchip.com>, <Steen.Hegelund@microchip.com>,
        <UNGLinuxDriver@microchip.com>, <joe@perches.com>,
        <richardcochran@gmail.com>, <casper.casan@gmail.com>,
        <Horatiu.Vultur@microchip.com>, <shangxiaojing@huawei.com>,
        <rmk+kernel@armlinux.org.uk>, <nhuck@google.com>,
        <error27@gmail.com>, <linux-kernel@vger.kernel.org>,
        <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH net-next 03/10] net: microchip: sparx5: add support for
 Service Dual Leacky Buckets
Thread-Topic: [PATCH net-next 03/10] net: microchip: sparx5: add support for
 Service Dual Leacky Buckets
Thread-Index: AQHZNvNw/NXbR7cUWEuZ5T9YllIEGa6+wW4AgAIMmoA=
Date:   Sun, 5 Feb 2023 20:11:02 +0000
Message-ID: <Y+ANVTMT7jgV0i0l@DEN-LT-70577>
References: <20230202104355.1612823-1-daniel.machon@microchip.com>
 <20230202104355.1612823-4-daniel.machon@microchip.com>
 <Y95VRJWV4NfSDYUR@corigine.com>
In-Reply-To: <Y95VRJWV4NfSDYUR@corigine.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?texSofwS+vWX9slHWWPBfHf/EVujxFHNvtXMxmH4Wj4/j3T4rrd73fTS73nJ?=
 =?us-ascii?Q?9PL0zArdyeDmdCZthax30JAmBNGQTznP9f17WQn0ywqsckHQzGbFFzrjMmnP?=
 =?us-ascii?Q?GlFt6lb4WMAY5XvTCVTj8QsoXIT+7uLSls4gNxE6ToqU/UpJH/6f3AVSWkIM?=
 =?us-ascii?Q?+YaTixb6BNkYUnHkQjU23GTHQkN3duokU3zwAbzQVMqq80HSnCr2z9UTAQCx?=
 =?us-ascii?Q?e/2dup76H32l6JoMsPEVXgxkecDSU6h7yhzvOSUFJUrzPCyjnmL998VA+xop?=
 =?us-ascii?Q?KkaDsoeIbHRlR875a/t71LFliltlGNxM+gLlh+LCTMlru2t09yRGm9Q1qrCw?=
 =?us-ascii?Q?mGmmE8K5IC0Df3OAXw1Kd0bm78MxQ/4yelh2OvCyNTUn9F/OjHuRnYGubbEP?=
 =?us-ascii?Q?WBdUKRJtmj2UsSMSWnTyZahrC7cu527uknrpsNuHx6V8kiRCGGhS17jPEByz?=
 =?us-ascii?Q?M1KpYtMQft+xYLhVs5v8qaQgo6Yz6jmKKsWqgf2TjWTghjJUbnkQPBWo7Vln?=
 =?us-ascii?Q?OAnwN2u8ZWiLma6e8vkAm6mndfv55r4XrrkDL/fdTKgvD3LEXDoIQfTgD/rz?=
 =?us-ascii?Q?Auit8IWEwHd9Pv0OxrwPV14CiwMFJr9AV2T//3XFKId5e3eqGTraohcDjO4e?=
 =?us-ascii?Q?tBnfg0z/RlEVWK+VqdMETNXBNBsTnmxYeavTLDckfeEKs1uOJNpOB1x5jkun?=
 =?us-ascii?Q?bhlqW5jUfGGoekjzk37uxUkOhKMq7loslSsOFeoDU2PAW1cnZI+aAZynUMLx?=
 =?us-ascii?Q?O1vFheqNlOdKCGHGwGyLiSNJXQn9duekIReEMMlJ+Vh+5gh7m5nloj7MEfSx?=
 =?us-ascii?Q?xy5vgxTK2qsiMTzWDchPpnRbj2zksiRjOhOu8H2oO2RVkyuh72v0//RCUxF8?=
 =?us-ascii?Q?BINtgtAHOxnn1QlYiAc54Dp+cGWg+9bpHp/rS/l3U0kUeU9FnHHGt4fc+kxR?=
 =?us-ascii?Q?Jp8GVjjE9i6S4OHm0rigBDzy1mnlf1gdEMzyTm2wZw94cfffCWZNzTzJGTzM?=
 =?us-ascii?Q?F0NY3dip6W2bxRFX21wUbCqjKoPHY+Zs9PBJk6dIqbYHNjvCBDpDHEyrOdmC?=
 =?us-ascii?Q?K8ZPGAqaqhkpKtWdrvcVXdpx+EEaGDqqUKnjdc7k/Zd2Qwzk8skVd2bAnIiW?=
 =?us-ascii?Q?lz6jmu9xh+7lAYowhMJZszK6khnT0z+Q72yk1NFtOIMmYmSciUlO6cpKMZcc?=
 =?us-ascii?Q?go0wZbRsBimvXk1BbypWhJz84T+o4080Ttjw1YtNqU3YlL54FP451uR9hhiy?=
 =?us-ascii?Q?79GZS37F27DFlMSnFZDec4WVMsqeqWYZ4v92pDZGO+3iR70aBxicgeDqihpa?=
 =?us-ascii?Q?dUhpAI0h2NtrP5gL0/JG7l4x+XFt7Q0DtTswe46eS0Zm41nZ6Ro1xgFtcUu1?=
 =?us-ascii?Q?/+kMgXVnGfhIqVn3RuOS3VUgZVuznnz5efExNo9ovBsJyN9aoONmsWmuGtaW?=
 =?us-ascii?Q?6R4thODsw/H2Mkh+AsIDObUqkuSelv6eqSi4r+RNnzm586TsJQyITwID5hlB?=
 =?us-ascii?Q?pEqPHkr3N/QFtIZ+2naZ2I1LEo1zVJ18hmIrUCnBeJKOdBVMup1dMyD/gd0A?=
 =?us-ascii?Q?V+ZQeqXHL5A/cEnSH2BQAnFLt2rIH88MeNo/qCNm+fyVYLL8E313fkruitxU?=
 =?us-ascii?Q?cDTuXYbvWPE0jKSGZy8xGKA=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <8E80A4D018B8964B9F698349CCF54D11@namprd11.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CO6PR11MB5569.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 34ce8725-3e75-4f80-8a7c-08db07b51abd
X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Feb 2023 20:11:02.1537
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3f4057f3-b418-4d4e-ba84-d55b4e897d88
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: yN72ACxeQBu2m2I9gSAoO2WkVxQFuk8zs6iz/QoKlVfQbXDZmGE1PjAMTeTFBCBP7bVjNIog9CF9UexxTfHJrhzOWw1S+9UCyJ282Iwa3/g=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB7087
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Simon,

Thanks for reviewing my patches, appreciate it!

> > +static u32 sparx5_sdlb_group_get_last(struct sparx5 *sparx5, u32 group=
)
> > +{
> > +     u32 itr, next;
> > +
> > +     itr =3D sparx5_sdlb_group_get_first(sparx5, group);
> > +
> > +     for (;;) {
>=20
> Unbounded loops like this give me some apprehension.
> Will they always terminate?

Yes, it will always terminate - unless the add() del() functions are
buggy to begin with.

The end of the leak chain is marked by an index pointing to itself, and
this is the exit condition I am looking for in the unbounded loop.

>=20
> > +             next =3D sparx5_sdlb_group_get_next(sparx5, group, itr);
> > +             if (itr =3D=3D next)
> > +                     return itr;
> > +
> > +             itr =3D next;
> > +     }
> > +}
>=20
> ...
>=20
> > +static int sparx5_sdlb_group_get_adjacent(struct sparx5 *sparx5, u32 g=
roup,
> > +                                       u32 idx, u32 *prev, u32 *next,
> > +                                       u32 *first)
> > +{
> > +     u32 itr;
> > +
> > +     *first =3D sparx5_sdlb_group_get_first(sparx5, group);
> > +     *prev =3D *first;
> > +     *next =3D *first;
> > +     itr =3D *first;
> > +
> > +     for (;;) {
> > +             *next =3D sparx5_sdlb_group_get_next(sparx5, group, itr);
> > +
> > +             if (itr =3D=3D idx)
> > +                     return 0; /* Found it */
> > +
> > +             if (itr =3D=3D *next)
> > +                     return -EINVAL; /* Was not found */
> > +
> > +             *prev =3D itr;
> > +             itr =3D *next;
> > +     }
> > +}
> > +
> > +static int sparx5_sdlb_group_get_count(struct sparx5 *sparx5, u32 grou=
p)
> > +{
> > +     u32 itr, next;
> > +     int count =3D 0;
> > +
> > +     itr =3D sparx5_sdlb_group_get_first(sparx5, group);
> > +
> > +     for (;;) {
> > +             next =3D sparx5_sdlb_group_get_next(sparx5, group, itr);
> > +             if (itr =3D=3D next)
> > +                     return count;
> > +
> > +             itr =3D next;
> > +             count++;
> > +     }
> > +}
>=20
> ...
>=20
> > +int sparx5_sdlb_group_get_by_index(struct sparx5 *sparx5, u32 idx, u32=
 *group)
> > +{
> > +     u32 itr, next;
> > +     int i;
> > +
> > +     for (i =3D 0; i < SPX5_SDLB_GROUP_CNT; i++) {
> > +             if (sparx5_sdlb_group_is_empty(sparx5, i))
> > +                     continue;
> > +
> > +             itr =3D sparx5_sdlb_group_get_first(sparx5, i);
> > +
> > +             for (;;) {
> > +                     next =3D sparx5_sdlb_group_get_next(sparx5, i, it=
r);
> > +
> > +                     if (itr =3D=3D idx) {
> > +                             *group =3D i;
> > +                             return 0; /* Found it */
> > +                     }
> > +                     if (itr =3D=3D next)
> > +                             break; /* Was not found */
> > +
> > +                     itr =3D next;
> > +             }
> > +     }
> > +
> > +     return -EINVAL;
> > +}
>=20
> ...=