Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   "Keller, Jacob E" <jacob.e.keller@intel.com>
To:     ivecera <ivecera@redhat.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>
CC:     poros <poros@redhat.com>, mschmidt <mschmidt@redhat.com>,
        "Brandeburg, Jesse" <jesse.brandeburg@intel.com>,
        "Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
        "David S. Miller" <davem@davemloft.net>,
        "Jakub Kicinski" <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        "moderated list:INTEL ETHERNET DRIVERS" 
        <intel-wired-lan@lists.osuosl.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH net v2] ice: Fix MAC address setting
Thread-Topic: [PATCH net v2] ice: Fix MAC address setting
Thread-Index: AQHYQEvWKQE7B4Gt4EeDyRfKN/Jn4KzVGJjQ
Date:   Mon, 28 Mar 2022 17:53:19 +0000
Message-ID: <CO1PR11MB508954503C974FD6D9E162FCD61D9@CO1PR11MB5089.namprd11.prod.outlook.com>
References: <20220325132524.1765342-1-ivecera@redhat.com>
In-Reply-To: <20220325132524.1765342-1-ivecera@redhat.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?dyKd8rZ4PpzD+Jc/2mJyd7S5nV4Fkrcgmyd0XD/V0bx3UjMP64lWwKES8TLE?=
 =?us-ascii?Q?u5voF71XLv8wVtGblKswq/QHkjkFZ8ZdpfM4l8lx9PIRQ4/M2u81l6UMOy+i?=
 =?us-ascii?Q?a9seQN6INxev/Y1N96mL+ts4JhQsGXNVQZUDjss2aAzRBbavLhpCwY8KL0TQ?=
 =?us-ascii?Q?Rr8Q7xanfdyIrR0JcHKKvOY5R8hgmoZbhy2nzzbYk4ty6J2uPFOOkbAxwn7f?=
 =?us-ascii?Q?dg5fe1ofbeW4vQMC0TRj/VNZaRF2GA9y5USiZTJCQ5qsYLIev1J890KZ+rKV?=
 =?us-ascii?Q?/DAkn7zN/QPwkZziQoyqlaK+sLt/Z/PDO4Ih4h+VuEhvlKVO54Zop+GyVpdB?=
 =?us-ascii?Q?R7jlebsKW5RJlbJdWWTcDysWTAgbg/dVxaqNUPJwSIFj5nCrDkeUkJ83f5oH?=
 =?us-ascii?Q?xv19QLS02xbVLnZaTHLUDvrR6439xM87NJw7Ndbxbw/2Q8ouEpUcmaHT0kXs?=
 =?us-ascii?Q?2dVvUJYbJQNnvvSE4pBQHBLmpA2JQ+4gdEpOx4azo6xCfDRD8mPEcD+2d4Zq?=
 =?us-ascii?Q?Q9VR9F9Mea0cST7fb+XusundGza7dJtIuBYZFmHGmRqHbxfCCkdfb0snyHTE?=
 =?us-ascii?Q?SSFtMkwEYii7N88dyTWDbh2EFywvNApn87/1/iXSJ6VA9IW3iLGAqLlhP9EH?=
 =?us-ascii?Q?o+dRRffdibLc0+k1uR1nfEEWdtSH2elBmOhd5O1ilH7klG21jA9ranogQx+2?=
 =?us-ascii?Q?3jzyTmiXWMfd4hwSqdvELwltNO5A/CHMrG6kzh6h0sG2A37htgUKJmrABAp9?=
 =?us-ascii?Q?Fts1yS0lDWq5aUqiD+xuPsCIUK6QyIj7tHfN2I8Om9cuYv2zgh1uTH6WxtzP?=
 =?us-ascii?Q?d5h2o9/iBQn/p6xSuorL1hKVN0PhJkFmMk4kxrjlzgnYmngsu8xkYt+0sB3x?=
 =?us-ascii?Q?V15eWyUU88Fxrb1Oqt59LV6GM1VF1/VFhjiSizmpujkeZQk4qzLx8OPGO7LK?=
 =?us-ascii?Q?W47kYQ3xgkc/qVSEa7BqPCAFSS8YCHggd8/9szr/82tufdACtu6DacAxT2bh?=
 =?us-ascii?Q?Ne9OWc/aAOJ3hWEF59Mdi3MBcQnClgc1QFl3YGlYNExU+faCQvDKSuhjhSU7?=
 =?us-ascii?Q?hK7gcpeDPMSQUbah5tdviqnHDO3/fMkaYlom8+sbzMfOqb9nYW4SYBtEfGkl?=
 =?us-ascii?Q?baiqYk4ovNNqs2GR2um75y7IfxJF9KB1Qj3nNP62jRM8fZnkF4DVL8C+TN+C?=
 =?us-ascii?Q?W4uvqAOUfCCQ4yiuXx+LVyMFcSGvn3lEoIHeLQlhEds5EhuxQ3xZF8IvWNsi?=
 =?us-ascii?Q?r4m1/CPiOrGdRVPtB4a1s4Zlcx+6Y5PAOfguow54I1nwDkmoWv8f+jyXe2TO?=
 =?us-ascii?Q?3ZGw3BlFUYHr8kSoFogu7kUxQ3OiTx67AZ9UmwEHnht2OksPhoqwDhh1hLdh?=
 =?us-ascii?Q?WkqaPE7t6I8Z5HZ3ZqRAfAsyC1dg8HPifkoXADI2cPtwxAX8Dp7+QYRzsAof?=
 =?us-ascii?Q?xAwZfr7jmq+m+dOmBSO/ltL5r9rFnw9jBV5ymV/9DpcQtVBoiBetKGqGCPs7?=
 =?us-ascii?Q?TGPVDBCAje0S/9lqpzA2GtnsdlGXDUXZoHnvyu9Qgo/HiycfXhJY7NgWVktG?=
 =?us-ascii?Q?wwTrP8kx/+IKTzPW9sFMtGVT2inF2btEfHb57QZlpoa1VyIMdr33urK6+vHX?=
 =?us-ascii?Q?4iu4nGSwkwS4x0DC4oUIaUGPrpHlirEE43zPXh6lLnw924FbjPm+ATSmxABK?=
 =?us-ascii?Q?1qeBsS5DIXbngMlUQPDx+WGFvWYHiMH035yi1Y0qrirWq3bA4avEw9KfIHBH?=
 =?us-ascii?Q?hIcPOMOGBg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0f423707-53ac-4be0-2c2e-08da10e3d81e
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Mar 2022 17:53:19.4417
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: s6eP65fEiSnuZWsBuiz19RmzGafI3GSv0J+7fvBRmQQBBCNje7SfE9YwhoQ2DhevzRQLcriu9+Ll/aC1xdFAAnYQ68/AGCw2vBncJ2gj/Kk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR11MB2062
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE
	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


> -----Original Message-----
> From: Ivan Vecera <ivecera@redhat.com>
> Sent: Friday, March 25, 2022 6:25 AM
> To: netdev@vger.kernel.org
> Cc: poros <poros@redhat.com>; mschmidt <mschmidt@redhat.com>;
> Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; David S. Miller <davem@davemloft.net>; Jaku=
b
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; moderated
> list:INTEL ETHERNET DRIVERS <intel-wired-lan@lists.osuosl.org>; open list=
 <linux-
> kernel@vger.kernel.org>
> Subject: [PATCH net v2] ice: Fix MAC address setting
>=20
> Commit 2ccc1c1ccc671b ("ice: Remove excess error variables") merged
> the usage of 'status' and 'err' variables into single one in
> function ice_set_mac_address(). Unfortunately this causes
> a regression when call of ice_fltr_add_mac() returns -EEXIST because
> this return value does not indicate an error in this case but
> value of 'err' remains to be -EEXIST till the end of the function
> and is returned to caller.
>=20
> Prior mentioned commit this does not happen because return value of
> ice_fltr_add_mac() was stored to 'status' variable first and
> if it was -EEXIST then 'err' remains to be zero.
>=20
> Fix the problem by reset 'err' to zero when ice_fltr_add_mac()
> returns -EEXIST.
>=20
> Fixes: 2ccc1c1ccc671b ("ice: Remove excess error variables")
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>
> ---

Thanks for the v2. This looks great. Good analysis of how this happened in =
the commit message, I appreciate that.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  drivers/net/ethernet/intel/ice/ice_main.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>=20
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index b588d7995631..d755ce07869f 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -5475,16 +5475,19 @@ static int ice_set_mac_address(struct net_device
> *netdev, void *pi)
>=20
>  	/* Add filter for new MAC. If filter exists, return success */
>  	err =3D ice_fltr_add_mac(vsi, mac, ICE_FWD_TO_VSI);
> -	if (err =3D=3D -EEXIST)
> +	if (err =3D=3D -EEXIST) {
>  		/* Although this MAC filter is already present in hardware it's
>  		 * possible in some cases (e.g. bonding) that dev_addr was
>  		 * modified outside of the driver and needs to be restored back
>  		 * to this value.
>  		 */
>  		netdev_dbg(netdev, "filter for MAC %pM already exists\n", mac);
> -	else if (err)
> +
> +		return 0;
> +	} else if (err) {
>  		/* error if the new filter addition failed */
>  		err =3D -EADDRNOTAVAIL;
> +	}
>=20
>  err_update_filters:
>  	if (err) {
> --
> 2.34.1