Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1332929 > unrolled thread

[PATCH net-next 3/4] net: dsa: mv88e6xxx: check hardware VLAN in use

Started byVivien Didelot <vivien.didelot@savoirfairelinux.com>
First post2016-02-12 18:20 +0100
Last post2016-02-13 21:10 +0100
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH net-next 3/4] net: dsa: mv88e6xxx: check hardware VLAN in use Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-02-12 18:20 +0100
    Re: [PATCH net-next 3/4] net: dsa: mv88e6xxx: check hardware VLAN in  use Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-02-13 19:20 +0100
      Re: [PATCH net-next 3/4] net: dsa: mv88e6xxx: check hardware VLAN in use Vivien Didelot <vivien.didelot@savoirfairelinux.com> - 2016-02-13 21:00 +0100
        Re: [PATCH net-next 3/4] net: dsa: mv88e6xxx: check hardware VLAN in  use Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2016-02-13 21:10 +0100

#1332929 — [PATCH net-next 3/4] net: dsa: mv88e6xxx: check hardware VLAN in use

FromVivien Didelot <vivien.didelot@savoirfairelinux.com>
Date2016-02-12 18:20 +0100
Subject[PATCH net-next 3/4] net: dsa: mv88e6xxx: check hardware VLAN in use
Message-ID<r1pxT-1vA-1@gated-at.bofh.it>
The DSA drivers now have access to the VLAN prepare phase and the bridge
net_device. It is easier to check for overlapping bridges from within
the driver. Thus add such check in mv88e6xxx_port_vlan_prepare.

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 2e515e8..685dcb0 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1471,14 +1471,78 @@ static int _mv88e6xxx_vlan_init(struct dsa_switch *ds, u16 vid,
 	return 0;
 }
 
+static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
+					u16 vid_begin, u16 vid_end)
+{
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	struct mv88e6xxx_vtu_stu_entry vlan;
+	int i, err;
+
+	if (!vid_begin)
+		return -EOPNOTSUPP;
+
+	mutex_lock(&ps->smi_mutex);
+
+	err = _mv88e6xxx_vtu_vid_write(ds, vid_begin - 1);
+	if (err)
+		goto unlock;
+
+	do {
+		err = _mv88e6xxx_vtu_getnext(ds, &vlan);
+		if (err)
+			goto unlock;
+
+		if (!vlan.valid)
+			break;
+
+		if (vlan.vid > vid_end)
+			break;
+
+		for (i = 0; i < ps->num_ports; ++i) {
+			if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
+				continue;
+
+			if (vlan.data[i] ==
+			    GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER)
+				continue;
+
+			if (ps->ports[i].bridge_dev ==
+			    ps->ports[port].bridge_dev)
+				break; /* same bridge, check next VLAN */
+
+			netdev_warn(ds->ports[port],
+				    "hardware VLAN %d already used by %s\n",
+				    vlan.vid,
+				    netdev_name(ps->ports[i].bridge_dev));
+			err = -EOPNOTSUPP;
+			goto unlock;
+		}
+	} while (vlan.vid < vid_end);
+
+unlock:
+	mutex_unlock(&ps->smi_mutex);
+
+	return err;
+}
+
 int mv88e6xxx_port_vlan_prepare(struct dsa_switch *ds, int port,
 				const struct switchdev_obj_port_vlan *vlan,
 				struct switchdev_trans *trans)
 {
+	int err;
+
 	/* We reserve a few VLANs to isolate unbridged ports */
 	if (vlan->vid_end >= 4000)
 		return -EOPNOTSUPP;
 
+	/* If the requested port doesn't belong to the same bridge as the VLAN
+	 * members, do not support it (yet) and fallback to software VLAN.
+	 */
+	err = mv88e6xxx_port_check_hw_vlan(ds, port, vlan->vid_begin,
+					   vlan->vid_end);
+	if (err)
+		return err;
+
 	/* We don't need any dynamic resource from the kernel (yet),
 	 * so skip the prepare phase.
 	 */
-- 
2.7.1

[toc] | [next] | [standalone]

#1333347 — Re: [PATCH net-next 3/4] net: dsa: mv88e6xxx: check hardware VLAN in use

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2016-02-13 19:20 +0100
SubjectRe: [PATCH net-next 3/4] net: dsa: mv88e6xxx: check hardware VLAN in use
Message-ID<r1MXw-8rP-13@gated-at.bofh.it>
In reply to#1332929
Hello.

On 2/12/2016 8:09 PM, Vivien Didelot wrote:

> The DSA drivers now have access to the VLAN prepare phase and the bridge
> net_device. It is easier to check for overlapping bridges from within
> the driver. Thus add such check in mv88e6xxx_port_vlan_prepare.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>   drivers/net/dsa/mv88e6xxx.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 64 insertions(+)
>
> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
> index 2e515e8..685dcb0 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -1471,14 +1471,78 @@ static int _mv88e6xxx_vlan_init(struct dsa_switch *ds, u16 vid,
>   	return 0;
>   }
>
> +static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
> +					u16 vid_begin, u16 vid_end)
> +{
> +	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> +	struct mv88e6xxx_vtu_stu_entry vlan;
> +	int i, err;
> +
> +	if (!vid_begin)
> +		return -EOPNOTSUPP;
> +
> +	mutex_lock(&ps->smi_mutex);
> +
> +	err = _mv88e6xxx_vtu_vid_write(ds, vid_begin - 1);
> +	if (err)
> +		goto unlock;
> +
> +	do {
> +		err = _mv88e6xxx_vtu_getnext(ds, &vlan);
> +		if (err)
> +			goto unlock;

    Why are you not using *break*?

> +
> +		if (!vlan.valid)
> +			break;
> +
> +		if (vlan.vid > vid_end)
> +			break;
> +
> +		for (i = 0; i < ps->num_ports; ++i) {
> +			if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
> +				continue;
> +
> +			if (vlan.data[i] ==
> +			    GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER)
> +				continue;
> +
> +			if (ps->ports[i].bridge_dev ==
> +			    ps->ports[port].bridge_dev)
> +				break; /* same bridge, check next VLAN */
> +
> +			netdev_warn(ds->ports[port],
> +				    "hardware VLAN %d already used by %s\n",
> +				    vlan.vid,
> +				    netdev_name(ps->ports[i].bridge_dev));
> +			err = -EOPNOTSUPP;
> +			goto unlock;
> +		}

    Why not *break*?

> +	} while (vlan.vid < vid_end);
> +
> +unlock:
> +	mutex_unlock(&ps->smi_mutex);
> +
> +	return err;
> +}
> +
[...]

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1333355

FromVivien Didelot <vivien.didelot@savoirfairelinux.com>
Date2016-02-13 21:00 +0100
Message-ID<r1Owi-Og-9@gated-at.bofh.it>
In reply to#1333347
Hi Sergei,

Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> writes:

> Hello.
>
> On 2/12/2016 8:09 PM, Vivien Didelot wrote:
>
>> The DSA drivers now have access to the VLAN prepare phase and the bridge
>> net_device. It is easier to check for overlapping bridges from within
>> the driver. Thus add such check in mv88e6xxx_port_vlan_prepare.
>>
>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> ---
>>   drivers/net/dsa/mv88e6xxx.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 64 insertions(+)
>>
>> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
>> index 2e515e8..685dcb0 100644
>> --- a/drivers/net/dsa/mv88e6xxx.c
>> +++ b/drivers/net/dsa/mv88e6xxx.c
>> @@ -1471,14 +1471,78 @@ static int _mv88e6xxx_vlan_init(struct dsa_switch *ds, u16 vid,
>>   	return 0;
>>   }
>>
>> +static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
>> +					u16 vid_begin, u16 vid_end)
>> +{
>> +	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
>> +	struct mv88e6xxx_vtu_stu_entry vlan;
>> +	int i, err;
>> +
>> +	if (!vid_begin)
>> +		return -EOPNOTSUPP;
>> +
>> +	mutex_lock(&ps->smi_mutex);
>> +
>> +	err = _mv88e6xxx_vtu_vid_write(ds, vid_begin - 1);
>> +	if (err)
>> +		goto unlock;
>> +
>> +	do {
>> +		err = _mv88e6xxx_vtu_getnext(ds, &vlan);
>> +		if (err)
>> +			goto unlock;
>
>     Why are you not using *break*?

I use goto for explicit error handling, and break for expected flow.

>> +
>> +		if (!vlan.valid)
>> +			break;
>> +
>> +		if (vlan.vid > vid_end)
>> +			break;
>> +
>> +		for (i = 0; i < ps->num_ports; ++i) {
>> +			if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
>> +				continue;
>> +
>> +			if (vlan.data[i] ==
>> +			    GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER)
>> +				continue;
>> +
>> +			if (ps->ports[i].bridge_dev ==
>> +			    ps->ports[port].bridge_dev)
>> +				break; /* same bridge, check next VLAN */
>> +
>> +			netdev_warn(ds->ports[port],
>> +				    "hardware VLAN %d already used by %s\n",
>> +				    vlan.vid,
>> +				    netdev_name(ps->ports[i].bridge_dev));
>> +			err = -EOPNOTSUPP;
>> +			goto unlock;
>> +		}
>
>     Why not *break*?

Because here it would only break the for loop, and not the while loop.

>
>> +	} while (vlan.vid < vid_end);
>> +
>> +unlock:
>> +	mutex_unlock(&ps->smi_mutex);
>> +
>> +	return err;
>> +}
>> +
> [...]
>
> MBR, Sergei

Thanks,
-v

[toc] | [prev] | [next] | [standalone]

#1333356 — Re: [PATCH net-next 3/4] net: dsa: mv88e6xxx: check hardware VLAN in use

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2016-02-13 21:10 +0100
SubjectRe: [PATCH net-next 3/4] net: dsa: mv88e6xxx: check hardware VLAN in use
Message-ID<r1OFY-16I-7@gated-at.bofh.it>
In reply to#1333355
On 02/13/2016 10:53 PM, Vivien Didelot wrote:

>>> The DSA drivers now have access to the VLAN prepare phase and the bridge
>>> net_device. It is easier to check for overlapping bridges from within
>>> the driver. Thus add such check in mv88e6xxx_port_vlan_prepare.
>>>
>>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>>> ---
>>>    drivers/net/dsa/mv88e6xxx.c | 64 +++++++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 64 insertions(+)
>>>
>>> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
>>> index 2e515e8..685dcb0 100644
>>> --- a/drivers/net/dsa/mv88e6xxx.c
>>> +++ b/drivers/net/dsa/mv88e6xxx.c
>>> @@ -1471,14 +1471,78 @@ static int _mv88e6xxx_vlan_init(struct dsa_switch *ds, u16 vid,
>>>    	return 0;
>>>    }
>>>
>>> +static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
>>> +					u16 vid_begin, u16 vid_end)
>>> +{
>>> +	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
>>> +	struct mv88e6xxx_vtu_stu_entry vlan;
>>> +	int i, err;
>>> +
>>> +	if (!vid_begin)
>>> +		return -EOPNOTSUPP;
>>> +
>>> +	mutex_lock(&ps->smi_mutex);
>>> +
>>> +	err = _mv88e6xxx_vtu_vid_write(ds, vid_begin - 1);
>>> +	if (err)
>>> +		goto unlock;
>>> +
>>> +	do {
>>> +		err = _mv88e6xxx_vtu_getnext(ds, &vlan);
>>> +		if (err)
>>> +			goto unlock;
>>
>>      Why are you not using *break*?
>
> I use goto for explicit error handling, and break for expected flow.

    Thought you'd say so. :-)
    I still think *break* is preferable...

>>> +
>>> +		if (!vlan.valid)
>>> +			break;
>>> +
>>> +		if (vlan.vid > vid_end)
>>> +			break;
>>> +
>>> +		for (i = 0; i < ps->num_ports; ++i) {
>>> +			if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
>>> +				continue;
>>> +
>>> +			if (vlan.data[i] ==
>>> +			    GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER)
>>> +				continue;
>>> +
>>> +			if (ps->ports[i].bridge_dev ==
>>> +			    ps->ports[port].bridge_dev)
>>> +				break; /* same bridge, check next VLAN */
>>> +
>>> +			netdev_warn(ds->ports[port],
>>> +				    "hardware VLAN %d already used by %s\n",
>>> +				    vlan.vid,
>>> +				    netdev_name(ps->ports[i].bridge_dev));
>>> +			err = -EOPNOTSUPP;
>>> +			goto unlock;
>>> +		}
>>
>>      Why not *break*?
>
> Because here it would only break the for loop, and not the while loop.

    Oops, I overlooked the *for* loop. Sorry about that.

>>
>>> +	} while (vlan.vid < vid_end);
>>> +
>>> +unlock:
>>> +	mutex_unlock(&ps->smi_mutex);
>>> +
>>> +	return err;
>>> +}
>>> +
>> [...]
>
> Thanks,
> -v

MBR, Sergei

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web