On Sat Sep 05 2020, Vladimir Oltean wrote:
> On Fri, Sep 04, 2020 at 08:27:34AM +0200, Kurt Kanzenbach wrote:
>> Add a basic DSA driver for Hirschmann Hellcreek switches. Those switches are
>> implementing features needed for Time Sensitive Networking (TSN) such as 
>> support
>> for the Time Precision Protocol and various shapers like the Time Aware 
>> Shaper.
>> 
>> This driver includes basic support for networking:
>> 
>>  * VLAN handling
>>  * FDB handling
>>  * Port statistics
>>  * STP
>>  * Phylink
>> 
>> Signed-off-by: Kurt Kanzenbach <k...@linutronix.de>
>> ---
>> +
>> +/* Default setup for DSA:
>> + *  VLAN 2: CPU and Port 1 egress untagged.
>> + *  VLAN 3: CPU and Port 2 egress untagged.
>> + */
>> +static int hellcreek_setup_vlan_membership(struct dsa_switch *ds, int port,
>> +                                       bool enabled)
>
> If you use VLAN 2 and 3 for port separation, then how does the driver
> deal with the following:
>
> ip link add link swp1 name swp1.100 type vlan id 100
> ip link add link swp2 name swp2.100 type vlan id 100
>
> In this case, frames with VLAN 100 shouldn't leak from one port to the
> other, will they?

Well, that depends on whether hellcreek_vlan_add() is called for
creating that vlan interfaces. In general: As soon as both ports are
members of the same vlan that traffic is switched.

>
>> +{
>> +    int upstream = dsa_upstream_port(ds, port);
>> +    struct switchdev_obj_port_vlan vlan = {
>> +            .vid_begin = port,
>> +            .vid_end = port,
>> +    };
>> +    int err = 0;
>> +
>> +    /* The CPU port is implicitly configured by
>> +     * configuring the front-panel ports
>> +     */
>> +    if (!dsa_is_user_port(ds, port))
>> +            return 0;
>> +
>> +    /* Apply vid to port as egress untagged and port vlan id */
>> +    vlan.flags = BRIDGE_VLAN_INFO_UNTAGGED | BRIDGE_VLAN_INFO_PVID;
>> +    if (enabled)
>> +            hellcreek_vlan_add(ds, port, &vlan);
>> +    else
>> +            err = hellcreek_vlan_del(ds, port, &vlan);
>> +    if (err) {
>> +            dev_err(ds->dev, "Failed to apply VID %d to port %d: %d\n",
>> +                    port, port, err);
>> +            return err;
>> +    }
>> +
>> +    /* Apply vid to cpu port as well */
>> +    vlan.flags = BRIDGE_VLAN_INFO_UNTAGGED;
>> +    if (enabled)
>> +            hellcreek_vlan_add(ds, upstream, &vlan);
>> +    else
>> +            err = hellcreek_vlan_del(ds, upstream, &vlan);
>> +    if (err) {
>> +            dev_err(ds->dev, "Failed to apply VID %d to port %d: %d\n",
>> +                    port, port, err);
>> +            return err;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void hellcreek_setup_ingressflt(struct hellcreek *hellcreek, int 
>> port,
>> +                                   bool enable)
>> +{
>> +    struct hellcreek_port *hellcreek_port = &hellcreek->ports[port];
>> +    u16 ptcfg;
>> +
>> +    mutex_lock(&hellcreek->reg_lock);
>> +
>> +    ptcfg = hellcreek_port->ptcfg;
>> +
>> +    if (enable)
>> +            ptcfg |= HR_PTCFG_INGRESSFLT;
>> +    else
>> +            ptcfg &= ~HR_PTCFG_INGRESSFLT;
>> +
>> +    hellcreek_select_port(hellcreek, port);
>> +    hellcreek_write(hellcreek, ptcfg, HR_PTCFG);
>> +    hellcreek_port->ptcfg = ptcfg;
>> +
>> +    mutex_unlock(&hellcreek->reg_lock);
>> +}
>> +
>> +static int hellcreek_vlan_filtering(struct dsa_switch *ds, int port,
>> +                                bool vlan_filtering)
>
> I remember asking in Message-ID: <20200716082935.snokd33kn52ixk5h@skbuf>
> whether it would be possible for you to set
> ds->configure_vlan_while_not_filtering = true during hellcreek_setup.
> Did anything unexpected happen while trying that?

No, that comment got lost.

So looking at the flag: Does it mean the driver can receive vlan
configurations when a bridge without vlan filtering is used? That might
be problematic as this driver uses vlans for the port separation by
default. This is undone when vlan filtering is set to 1 meaning vlan
configurations can be received without any problems.

Thanks,
Kurt

Attachment: signature.asc
Description: PGP signature

Reply via email to