From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751309AbcBMTxN (ORCPT ); Sat, 13 Feb 2016 14:53:13 -0500 Received: from mail.savoirfairelinux.com ([208.88.110.44]:42066 "EHLO mail.savoirfairelinux.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750902AbcBMTxL (ORCPT ); Sat, 13 Feb 2016 14:53:11 -0500 From: Vivien Didelot To: Sergei Shtylyov , netdev@vger.kernel.org Cc: linux-kernel@vger.kernel.org, kernel@savoirfairelinux.com, "David S. Miller" , Florian Fainelli , Andrew Lunn Subject: Re: [PATCH net-next 3/4] net: dsa: mv88e6xxx: check hardware VLAN in use In-Reply-To: <56BF731F.8000008@cogentembedded.com> References: <1455296981-27998-1-git-send-email-vivien.didelot@savoirfairelinux.com> <1455296981-27998-4-git-send-email-vivien.didelot@savoirfairelinux.com> <56BF731F.8000008@cogentembedded.com> User-Agent: Notmuch/0.21 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-unknown-linux-gnu) Date: Sat, 13 Feb 2016 14:53:06 -0500 Message-ID: <87mvr44cbh.fsf@ketchup.mtl.sfl> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Sergei, Sergei Shtylyov 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 >> --- >> 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