lkml.org 
[lkml]   [2020]   [Dec]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [RFC PATCH 2/3] net: sparx5: Add Sparx5 switchdev driver
Date

Andrew Lunn writes:

>> +static void sparx5_attr_stp_state_set(struct sparx5_port *port,
>> + struct switchdev_trans *trans,
>> + u8 state)
>> +{
>> + struct sparx5 *sparx5 = port->sparx5;
>> +
>> + if (!test_bit(port->portno, sparx5->bridge_mask)) {
>> + netdev_err(port->ndev,
>> + "Controlling non-bridged port %d?\n", port->portno);
>> + return;
>> + }
>> +
>> + switch (state) {
>> + case BR_STATE_FORWARDING:
>> + set_bit(port->portno, sparx5->bridge_fwd_mask);
>> + break;
>> + default:
>> + clear_bit(port->portno, sparx5->bridge_fwd_mask);
>> + break;
>> + }
>
> That is pretty odd. What about listening, learning, blocking?
>

This only handles simple forward/block. We'll add the learning state as
well.

>> +static int sparx5_port_bridge_join(struct sparx5_port *port,
>> + struct net_device *bridge)
>> +{
>> + struct sparx5 *sparx5 = port->sparx5;
>> +
>> + if (bitmap_empty(sparx5->bridge_mask, SPX5_PORTS))
>> + /* First bridged port */
>> + sparx5->hw_bridge_dev = bridge;
>> + else
>> + if (sparx5->hw_bridge_dev != bridge)
>> + /* This is adding the port to a second bridge, this is
>> + * unsupported
>> + */
>> + return -ENODEV;
>> +
>> + set_bit(port->portno, sparx5->bridge_mask);
>> +
>> + /* Port enters in bridge mode therefor don't need to copy to CPU
>> + * frames for multicast in case the bridge is not requesting them
>> + */
>> + __dev_mc_unsync(port->ndev, sparx5_mc_unsync);
>> +
>> + return 0;
>> +}
>
> This looks suspiciously empty? Don't you need to tell the hardware
> which ports this port is bridges to? Normally you see some code which
> walks all the ports and finds those in the same bridge, and sets a bit
> which allows these ports to talk to each other. Is that code somewhere
> else?
>

This is applied when the STP state is handled - sparx5_update_fwd().

This is pretty much as in the ocelot driver, which can a somewhat
similar switch - and driver - architecture.

> Andrew

Thank you for your comments,

---Lars

--
Lars Povlsen,
Microchip

\
 
 \ /
  Last update: 2020-12-01 12:13    [W:0.245 / U:4.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site