Re: [PATCH net-next v3 3/3] net: ti: icssg-prueth: Add support for ICSSG switch firmware

[MD Danish Anwar]

Hi Andrew,

On 27/03/24 6:05 pm, Andrew Lunn wrote:
> On Wed, Mar 27, 2024 at 05:10:54PM +0530, MD Danish Anwar wrote:
>> Add support for ICSSG switch firmware using existing Dual EMAC driver
>> with switchdev.
>>
>> Limitations:
>> VLAN offloading is limited to 0-256 IDs.
>> MDB/FDB static entries are limited to 511 entries and different FDBs can
>> hash to same bucket and thus may not completely offloaded
>>
>> Switch mode requires loading of new firmware into ICSSG cores. This
>> means interfaces have to taken down and then reconfigured to switch
>> mode.
> 
> Patch 0/3 does not say this. It just shows the interfaces being added

I will modify the cover letter to state that.

> to the bridge. There should not be any need to down the interfaces.
> 

The interfaces needs to be turned down for switching between dual emac
and switch mode.

Dual Emac mode runs with ICSSG Dual Emac firmware where as Switch mode
works with ICSSG Switch firmware. These firmware are running on the
dedicated PRU RPROC cores (pru0, rtu0, txpru0). When switch mode is
enabled, these pru cores need to be stopped and then Switch firmware is
loaded on these cores and then the cores are started again.

We stop the cores when interfaces are down and start the cores when
interfaces are up.

In short, Dual EMAC firmware runs on pru cores, we put down the
interface, stop pru cores, load switch firmware on the cores, bring the
interface up and start the pru cores and now Switch mode is enabled.


>> Example assuming ETH1 and ETH2 as ICSSG2 interfaces:
>>
>> Switch to ICSSG Switch mode:
>>  ip link set dev eth1 down
>>  ip link set dev eth2 down
>>  ip link add name br0 type bridge
>>  ip link set dev eth1 master br0
>>  ip link set dev eth2 master br0
>>  ip link set dev br0 up
>>  ip link set dev eth1 up
>>  ip link set dev eth2 up
>>  bridge vlan add dev br0 vid 1 pvid untagged self
>>
>> Going back to Dual EMAC mode:
>>
>>  ip link set dev br0 down
>>  ip link set dev eth1 nomaster
>>  ip link set dev eth2 nomaster
>>  ip link set dev eth1 down
>>  ip link set dev eth2 down
>>  ip link del name br0 type bridge
>>  ip link set dev eth1 up
>>  ip link set dev eth2 up
>>
>> By default, Dual EMAC firmware is loaded, and can be changed to switch
>> mode by above steps
> 
> I keep asking this, so it would be good to explain it in the commit
> message. What configuration is preserved over a firmware reload, and
> what is lost?
> 
> Can i add VLAN in duel MAC mode and then swap into the switch firmware
> and all the VLANs are preserved? Can i add fdb entries to a port in
> dual MAC mode, and then swap into the swtich firmware and the FDB
> table is preserved? What about STP port state? What about ... ?
> 

When ports are brought up (firmware reload) we do a full cleaning of all
the shared memories i.e. SMEM (shared RAM). [1]

Vlan table and FDB table are stored in SMEM so all the configuration
done to VLAN / FDB tables will be lost.

We don't clear DRAM. DRAM is used for sending r30 commands [see
emac_r30_cmd_init()], configure half duplex [see
icssg_config_half_duplex()] and configure link speed [see
icssg_config_set_speed()]. r30 commands are used to set port state (stp).

Now when the interfaces are brought up (firmware reload) r30 command is
reconfigured as a result any changes done to port state (stp) will be
lost. But the duplex and speed settings will be preserved.

To summarize,
VLAN table / FDB table and port states are lost during a firmware reload.

[1]
https://elixir.bootlin.com/linux/latest/source/drivers/net/ethernet/ti/icssg/icssg_prueth.c#L1321
> 
>> +bool prueth_dev_check(const struct net_device *ndev)
>> +{
>> +	if (ndev->netdev_ops == &emac_netdev_ops && netif_running(ndev)) {
>> +		struct prueth_emac *emac = netdev_priv(ndev);
>> +
>> +		return emac->prueth->is_switch_mode;
>> +	}
>> +
>> +	return false;
>> +}
> 
> This does not appear to be used anywhere?
> 
>      Andrew

-- 
Thanks and Regards,
Danish