Messages in this thread | | | Subject | Re: [PATCH net-next 3/4] net: dsa: mv88e6xxx: check hardware VLAN in use | From | Sergei Shtylyov <> | Date | Sat, 13 Feb 2016 23:08:49 +0300 |
| |
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
| |