Received-SPF: pass (google.com: domain of linux-kernel+bounces-98962-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <10dcb1bd-1a61-46e8-81c3-bc87542b72c4@intel.com>
Date: Mon, 11 Mar 2024 15:29:51 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 3/3] net: marvell: prestera: force good base mac
Content-Language: en-US
To: Elad Nachman <enachman@marvell.com>, <taras.chornyi@plvision.eu>,
	<davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
	<pabeni@redhat.com>, <andrew@lunn.ch>, <kory.maincent@bootlin.com>,
	<thomas.petazzoni@bootlin.com>, <miquel.raynal@bootlin.com>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20240311135112.2642491-1-enachman@marvell.com>
 <20240311135112.2642491-4-enachman@marvell.com>
From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
In-Reply-To: <20240311135112.2642491-4-enachman@marvell.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a2oxVkdTV2Q1UGpMSkxnYVFIQmM4QkZpbUU4bjBWSXNDb1FVeFhHK2hJR2Zp?=
 =?utf-8?B?Wm4rMWNjZlh1SWlRRnY5VGlZRzNqcEVVVVRKVkxQYnp2bWhzTTYzRW4zNG5Q?=
 =?utf-8?B?QmVveTczT3pPbXRROTVkUU1aOUJnYnByNnk0TlVrS0hIQmVWazdkUGh0aE0x?=
 =?utf-8?B?M0QrVjNIam5yTzlRZE1VaTNESzVJZStlYUJIYkIrb3RLZmlROGloeWJCVnJm?=
 =?utf-8?B?b3RnRlFjUzhRcEhTb0Roa1F2SXJ0RzVOemQ3TlBYNzZrdllJcC9HeWxGL2pM?=
 =?utf-8?B?ODN6UnNsOHJFUm5XREh1Q2tSeVNnd2tMTzI3cXVNdWZjZkc0N0lQenJKRlRk?=
 =?utf-8?B?aW9BamJGMHRERUUvUEFlakFyOWR1bDhHZ1pXMXdqQnF1cXkzdjhuL1c3MXVC?=
 =?utf-8?B?UjV2N3lIZzJTb0Y2blkxaDRnaktJdmhIVFV4ZDg2bmY2MGE2bHUzV0ZHWnZ5?=
 =?utf-8?B?S05FK2piZUpPeExZd3krS1lNVGNBM1ZpYVdpanFHblAwK2JWOU1kSWMxYmpz?=
 =?utf-8?B?Y3BjdFA2N2pkaFFOOWt1bm5MdWVSak51d2pqTm90RGQzM3JUcy9iMXg4NlNi?=
 =?utf-8?B?Z3ZIYWdhcm5obStYOXFyaXlmRnEwSGFZaktrUHkyVnZERVJ5NVBPQzdLZjlL?=
 =?utf-8?B?TFhaVEdxU0ljT2VBSEFuWVdjMnhodHlISEYzcURSRkw0d3pEWVRLQnUwTVdJ?=
 =?utf-8?B?VVgxd3hQbXY3dThCQWptM3NpdlRRK25zckZyblVQUUpiTkJQajJpUVdzbWpJ?=
 =?utf-8?B?OWhrZVFDdHgrUkNGWjJpcHkwMHVjdTZndk5rQWlnRk82dGhwVkNQNkJrQXRE?=
 =?utf-8?B?eGxIVGhjbU8zcHpPUDNSYUpWRHZEQVNkVE95N29EbDhia1QvRHNkZjV3T0xy?=
 =?utf-8?B?UkRudVhSNzY4aWgzSXorZVczZmhFcTZDaUFOWkg5dmJWVjdMeUlZOTNyU1lC?=
 =?utf-8?B?SU1VdjV1V2grUXhOUUpLS3R3ZUZ6cTBXWWNmN2s4enptZ3pDbEZmaHFicS9a?=
 =?utf-8?B?Uy9vYTEybGN0RHlVeTVuSVFrUUJzZnRwQmpRZW80QXF4SmdKNWlMRWExQmkz?=
 =?utf-8?B?UzBuR0xZb0xFNlhyUklLSWJlZHIvVHA2OWI5MWsvd1hmejdOSmQwLzMxZDhv?=
 =?utf-8?B?c2d2bHNreFdPTnpidzlreFZ3K29QRC92VzdMcFlaYkRTa3ZlaE4wTXArZldk?=
 =?utf-8?B?bDVYZm1QVFQrT3A2clQ3ZzQzWEhNSFUybGZ6elIxMENjaEpDdmJRMGtSTUFw?=
 =?utf-8?B?RGNlaVB0K0Z0NmZKOWc5MERiT2pyTmFYMUdocURuMzRuMEtveWR5NFNuSTBL?=
 =?utf-8?B?YkJxcTlUcFdiVTVQaVJqSlVzR2l1TWdoazRXNHFObng3WGs4dk5BeURkOEtr?=
 =?utf-8?B?QmxZbnFyYnpIa2p4T1RVSjQzZzRqd2RydGVnYzR2b2RoMTVybVJ1L2FCVzJW?=
 =?utf-8?B?MnBReCtoVHVYRmVvVW9tckxzYzY2MHVBLzZKUExQM2JkREFCOVQzUHZNRURF?=
 =?utf-8?B?N25CdGFscHk5MVQ4QlIwZXcvRXpFV2Vhb1BXamF3OVFrRmRZdEg1WUxpV2dn?=
 =?utf-8?B?c1FHS3JXTkN4UTQrV0tzT0ttVUo3QzcxTDcvdXBHaVp2M1IwSWJCNGRTWTBP?=
 =?utf-8?B?dTJYeHNWaDMyYXBJenkxNUlyOWlrSGtTeFczTWpJa1lHQVpTcmlkVkp2V0dP?=
 =?utf-8?B?SERkeGp1eW8vcTVTN1dUdXAyWFdBUUtIQ0d6Zm5DZ2FvWHBwdDFQL3NCdDhx?=
 =?utf-8?B?MGI4c0Mxa1Z6YUowemsrNHRjVkt2RDQ0Q0o3UjlNMjFQM09xY0MwaXNGNnpo?=
 =?utf-8?B?b2dkVEVtWVNLSjhDOWt6OGhHS2FHWHl2cW55RVBNc3llTVdhL21zc0YvN2dO?=
 =?utf-8?B?aS82ZzBqUUtiNDIzekpLdnhIa0ZmNkxHMUN0dkxKemlPM3VRekF6dE1VVlpH?=
 =?utf-8?B?NWdnVlZ0VEljM1p2eFFTYzdQVDMxZGxaczZHQnFDSXZLbURXbUJSRzhaeTdp?=
 =?utf-8?B?M2prSWU3SDViZVg2ck11RnBHYlE3T3p2OGVTeUZCSXNnNFJzbHNvZTVjajZi?=
 =?utf-8?B?RkVockdqc2VUb0NqRFZzNkNvc3c2akc0cEplb0pCZWN3YVgwUXlQc08rcWNp?=
 =?utf-8?B?eVpDbTRxZlZLdStGUjAyWUI5N0xONEYvc1hOY0FPeFZRUmVCUWxndjRRTktt?=
 =?utf-8?Q?p3ujlPqHBj3KaZKpBwmKRz4=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3f4b0e6f-e98c-496b-ad16-08dc41d7baf9
X-MS-Exchange-CrossTenant-AuthSource: MN6PR11MB8102.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Mar 2024 14:29:59.2169
 (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: suvDM5FjFzoR2kI69/5Kdd/U760SeZQpCfjReZS6pv36jcLiAIQ3CmPLWfBclAxvno8y6WZYtjYXxpdudWj+/GmJlNnPcG5FTrUIAob1pVA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4548
X-OriginatorOrg: intel.com

On 3/11/24 14:51, Elad Nachman wrote:
> From: Elad Nachman <enachman@marvell.com>
> 
> Since each switchport MAC address uses the switch base mac address
> and adds the physical port number to it,
> Force the last byte of the switch base mac address to be at
> least 128, so when adding to it, we will not wrap around the

to be at *most* 128

> previous (more significant) mac address byte, resulting in a
> warning message.
> 
> Signed-off-by: Elad Nachman <enachman@marvell.com>
> ---
>   drivers/net/ethernet/marvell/prestera/prestera_main.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> index bcaa8ea27b49..e17b1a24fe18 100644
> --- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
> +++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
> @@ -859,7 +859,9 @@ static int prestera_switch_set_base_mac_addr(struct prestera_switch *sw)
>   	if (sw->np)
>   		ret = of_get_mac_address(sw->np, sw->base_mac);
>   	if (!is_valid_ether_addr(sw->base_mac) || ret) {
> -		eth_random_addr(sw->base_mac);
> +		do {
> +			eth_random_addr(sw->base_mac);
> +		} while (sw->base_mac[5] > 0x80);

Instead of this loop, that uses 6 bytes of random data at each step,
I would just fix the last byte.

Either by calling get_random_u8() in a loop, or perhaps better, just
toggle of MSB unconditionally:
sw->base_mac[5] &= ~0x80; // or '&= 127'

what would change your condition in commit message to "to be at most
127", but I think that should be fine, granted simpler code.

>   		dev_info(prestera_dev(sw), "using random base mac address\n");
>   	}
>