linux-kernel - Re: [External] : [PATCH net-next v4 1/3] net: ti: icssm-prueth: Adds helper functions to configure and maintain FDB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <482294584.44920.1762952879480.JavaMail.zimbra@couthit.local>
Date: Wed, 12 Nov 2025 18:37:59 +0530 (IST)
From: Parvathi Pudi <parvathi@...thit.com>
To: ALOK TIWARI <alok.a.tiwari@...cle.com>
Cc: Parvathi Pudi <parvathi@...thit.com>, andrew+netdev <andrew+netdev@...n.ch>, 
	davem <davem@...emloft.net>, edumazet <edumazet@...gle.com>, 
	kuba <kuba@...nel.org>, pabeni <pabeni@...hat.com>, 
	danishanwar <danishanwar@...com>, rogerq <rogerq@...nel.org>, 
	pmohan <pmohan@...thit.com>, basharath <basharath@...thit.com>, 
	afd <afd@...com>, linux-kernel <linux-kernel@...r.kernel.org>, 
	netdev <netdev@...r.kernel.org>, 
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>, 
	pratheesh <pratheesh@...com>, j-rameshbabu <j-rameshbabu@...com>, 
	Vignesh Raghavendra <vigneshr@...com>, praneeth <praneeth@...com>, 
	srk <srk@...com>, rogerq <rogerq@...com>, 
	krishna <krishna@...thit.com>, mohan <mohan@...thit.com>
Subject: Re: [External] : [PATCH net-next v4 1/3] net: ti: icssm-prueth:
 Adds helper functions to configure and maintain FDB

Hi,

>> +static int icssm_prueth_sw_insert_fdb_entry(struct prueth_emac *emac,
>> +					    const u8 *mac, u8 is_static)
>> +{
>> +	struct fdb_index_tbl_entry __iomem *bucket_info;
>> +	struct fdb_mac_tbl_entry __iomem *mac_info;
>> +	struct prueth *prueth = emac->prueth;
>> +	struct prueth_emac *other_emac;
>> +	enum prueth_port other_port_id;
>> +	u8 hash_val, mac_tbl_idx;
>> +	struct fdb_tbl *fdb;
>> +	u8 flags;
>> +	u16 val;
>> +	s16 ret;
>> +	int err;
>> +
>> +	fdb = prueth->fdb_tbl;
>> +	other_port_id = (emac->port_id == PRUETH_PORT_MII0) ?
>> +			 PRUETH_PORT_MII1 : PRUETH_PORT_MII0;
>> +
>> +	other_emac = prueth->emac[other_port_id - 1];
>> +
>> +	if (fdb->total_entries == FDB_MAC_TBL_MAX_ENTRIES)
>> +		return -ENOMEM;
>> +
>> +	if (ether_addr_equal(mac, emac->mac_addr) ||
>> +	    ether_addr_equal(mac, other_emac->mac_addr)) {
>> +		/* Don't insert fdb of own mac addr */
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Get the bucket that the mac belongs to */
>> +	hash_val = icssm_prueth_sw_fdb_hash(mac);
>> +	bucket_info = FDB_IDX_TBL_ENTRY(hash_val);
>> +
>> +	if (!readw(&bucket_info->bucket_entries)) {
>> +		mac_tbl_idx = icssm_prueth_sw_fdb_find_open_slot(fdb);
>> +		writew(mac_tbl_idx, &bucket_info->bucket_idx);
>> +	}
>> +
>> +	ret = icssm_prueth_sw_find_fdb_insert(fdb, prueth, bucket_info, mac,
>> +					      emac->port_id - 1);
>> +	if (ret < 0)
>> +		/* mac is already in fdb table */
>> +		return 0;
>> +
>> +	mac_tbl_idx = ret;
> 
> If ret == -1 mac_tbl_idx wraps to 255 silently.
> 

Thanks. Theoretically, assigning -1 to an unsigned variable would
result in 255 due to signed-to-unsigned conversion. However, in
this case, the above code already checks if (ret < 0) and returns early,
so the assignment to mac_tbl_idx never happens when ret is negative. 

>> +
>> +	err = icssm_prueth_sw_fdb_spin_lock(fdb);
>> +	if (err) {
>> +		dev_err(prueth->dev, "PRU lock timeout %d\n", ret);
> 
> wrong var ret print.
> return ret or err here ?
> 

We will address this in the next version.


Thanks and Regards,
Parvathi.