[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5b69b639df7dc88103f5d8c6fbeaa00b62c27c6d.camel@nvidia.com>
Date: Mon, 24 Jun 2024 11:18:31 +0000
From: Cosmin Ratiu <cratiu@...dia.com>
To: Tariq Toukan <tariqt@...dia.com>, "dan.carpenter@...aro.org"
<dan.carpenter@...aro.org>, "leon@...nel.org" <leon@...nel.org>
CC: Paul Blakey <paulb@...dia.com>, Vlad Buslov <vladbu@...dia.com>,
"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>, Chris Mi
<cmi@...dia.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [bug report] net/mlx5e: Implement CT entry update
On Mon, 2024-06-24 at 14:09 +0300, Leon Romanovsky wrote:
> On Thu, Jun 20, 2024 at 11:50:33AM +0300, Dan Carpenter wrote:
> > Hello Vlad Buslov,
> >
> > Commit 94ceffb48eac ("net/mlx5e: Implement CT entry update") from Dec
> > 1, 2022 (linux-next), leads to the following Smatch static checker
> > warning:
> >
> > drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c:1163 mlx5_tc_ct_entry_replace_rules()
> > error: uninitialized symbol 'err'.
>
> This error was introduced by the patch 49d37d05f216 ("net/mlx5: CT: Separate CT and CT-NAT tuple entries")
> https://lore.kernel.org/all/20240613210036.1125203-3-tariqt@nvidia.com/
Yes, Leon is correct.
We also caught the error in our static checker runs but the patch was
already sent to net-next before we could alter it with the fix.
> > drivers/net/ethernet/mellanox/mlx5/core/en/tc_ct.c
> > 1142 static int
> > 1143 mlx5_tc_ct_entry_replace_rules(struct mlx5_tc_ct_priv *ct_priv,
> > 1144 struct flow_rule *flow_rule,
> > 1145 struct mlx5_ct_entry *entry,
> > 1146 u8 zone_restore_id)
> > 1147 {
> > 1148 int err;
> > 1149
> > 1150 if (mlx5_tc_ct_entry_in_ct_table(entry)) {
> > 1151 err = mlx5_tc_ct_entry_replace_rule(ct_priv, flow_rule, entry, false,
> > 1152 zone_restore_id);
> > 1153 if (err)
> > 1154 return err;
> > 1155 }
> > 1156
> > 1157 if (mlx5_tc_ct_entry_in_ct_nat_table(entry)) {
> > 1158 err = mlx5_tc_ct_entry_replace_rule(ct_priv, flow_rule, entry, true,
> > 1159 zone_restore_id);
> > 1160 if (err && mlx5_tc_ct_entry_in_ct_table(entry))
> > 1161 mlx5_tc_ct_entry_del_rule(ct_priv, entry, false);
> > 1162 }
> >
> > Can the entry not be in either table?
> >
> > --> 1163 return err;
> >
> > If so then err is uninitialized.
In practice, the entry will definitely be in at least one of the tables
(if not both), so the uninitialized err will never happen in practice.
Regardless, to fix the noise, there's a patch already pending
upstreaming, about to be sent by Tariq soon.
Cosmin.
Powered by blists - more mailing lists