netdev - Re: [PATCH 2/5] net: dsa: lantiq: Add VLAN unaware bridge offloading

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: <59b838d4-d6a7-2f40-f121-3a71872c4886@hauke-m.de>
Date:   Fri, 3 May 2019 00:19:08 +0200
From:   Hauke Mehrtens <hauke@...ke-m.de>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     davem@...emloft.net, vivien.didelot@...il.com,
        f.fainelli@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH 2/5] net: dsa: lantiq: Add VLAN unaware bridge offloading

On 5/2/19 12:34 AM, Andrew Lunn wrote:
> On Wed, May 01, 2019 at 10:45:03PM +0200, Hauke Mehrtens wrote:
> 
> Hi Hauke
> 
> /* Add the LAN port into a bridge with the CPU port by
>> + * default. This prevents automatic forwarding of
>> + * packages between the LAN ports when no explicit
>> + * bridge is configured.
>> + */
>> +static int gswip_add_signle_port_br(struct gswip_priv *priv, int port, bool add)
> 
> single ?

I will fix this.

>> +{
>> +	struct gswip_pce_table_entry vlan_active = {0,};
>> +	struct gswip_pce_table_entry vlan_mapping = {0,};
>> +	unsigned int cpu_port = priv->hw_info->cpu_port;
>> +	unsigned int max_ports = priv->hw_info->max_ports;
>> +	int err;
>> +
>> +	if (port >= max_ports) {
>> +		dev_err(priv->dev, "single port for %i supported\n", port);
>> +		return -EIO;
>> +	}
>> +
>> +	vlan_active.index = port + 1;
> 
>>  
>> +static int gswip_vlan_active_create(struct gswip_priv *priv,
>> +				    struct net_device *bridge,
>> +				    int fid, u16 vid)
>> +{
>> +	struct gswip_pce_table_entry vlan_active = {0,};
>> +	unsigned int max_ports = priv->hw_info->max_ports;
>> +	int idx = -1;
>> +	int err;
>> +	int i;
>> +
>> +	/* Look for a free slot */
>> +	for (i = max_ports; i < ARRAY_SIZE(priv->vlans); i++) {
>> +		if (!priv->vlans[i].bridge) {
>> +			idx = i;
>> +			break;
>> +		}
>> +	}
> 
>> +static int gswip_vlan_add_unaware(struct gswip_priv *priv,
>> +				  struct net_device *bridge, int port)
>> +{
>> +	struct gswip_pce_table_entry vlan_mapping = {0,};
>> +	unsigned int max_ports = priv->hw_info->max_ports;
>> +	unsigned int cpu_port = priv->hw_info->cpu_port;
>> +	bool active_vlan_created = false;
>> +	int idx = -1;
>> +	int i;
>> +	int err;
>> +
>> +	/* Check if there is already a page for this bridge */
>> +	for (i = max_ports; i < ARRAY_SIZE(priv->vlans); i++) {
>> +		if (priv->vlans[i].bridge == bridge) {
>> +			idx = i;
>> +			break;
>> +		}
>> +	}
> 
> If i understand this correctly, VLANs 1 to max_ports are used for when
> a port is not a member of a bridge. When a port is added to a bridge,
> an unused vlan is allocated to the bridge.

Yes that is correct.

> You could however reuse the port VLANs.  When the 1st port joins a
> bridge, it keeps its VLAN ID, but the bridge is associated to the
> port. When the 2nd, 3rd, 4rd port joins the bridge, use the VLAN from
> the 1st port.
> 
> It gets messy when ports leave. If the 1st port is not the last to
> leave, you need to modify the VLAN ID to a port which is still a
> member of the bridge.

The VLAN ID does not matter, I use the same value for these single port
bridges.

This entry contains the following values for the single bridge:
Table Index, 0...63: I use port number + 1
VLAN ID, 0...4095: (fixed set to 0)
flow ID, 0..63: I use the same as the table index
port map, one bit for each port number: The selected port + CPU port
tagged port map, one bit for each port number: 0

This table index 0 is behaving somehow special I just excluded it from
normal operations for now.

If two ports are in the same bridge and are allowing VLAN ID 123, only
one entry is needed with the VLAN id 123 and the bits in the port map
set for both ports.

When I would reuse the old entries I can have a problem when ports are
leaving a bridge. For example I have a bridge with 62 VLANs and all
ports in it, now I remove the ports one after the other. In the end I
have one port in this bridge with 62 VLANs and 4 ports not in any
bridge. In this situation I do not have enough bridge table entries any
more.

> What you have here is simple, but if you think VLANs are valuable,
> this scheme can save you some VLANS, but at the expense of a bit of
> extra code complexity.

Yes the number of possible bridge entries / VLANs is a limiting factor
which people can hit pretty fast.
My first approach was to get something which works and is simple. I can
still improve it later. ;-)

Hauke