From: Andrew Lunn <andrew@lunn•ch>
To: Vivien Didelot <vivien.didelot@savoirfairelinux•com>
Cc: David Miller <davem@davemloft•net>, netdev <netdev@vger•kernel.org>
Subject: Re: [PATCH net] net: dsa: mv88e6xxx: Fix opps when adding vlan bridge
Date: Mon, 12 Dec 2016 14:04:16 +0100 [thread overview]
Message-ID: <20161212130416.GC27057@lunn.ch> (raw)
In-Reply-To: <87a8c2w55e.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>
On Sun, Dec 11, 2016 at 04:02:37PM -0500, Vivien Didelot wrote:
> Hi Andrew,
>
> Andrew Lunn <andrew@lunn•ch> writes:
>
> > @@ -1804,6 +1807,9 @@ static int mv88e6xxx_port_check_hw_vlan(struct dsa_switch *ds, int port,
> > chip->ports[port].bridge_dev)
> > break; /* same bridge, check next VLAN */
> >
> > + if (!chip->ports[i].bridge_dev)
> > + continue;
> > +
>
> The above truncated test:
>
> if (chip->ports[i].bridge_dev ==
> chip->ports[port].bridge_dev)
> break; /* same bridge, check next VLAN */
>
> should handle the case where bridge_dev is NULL, but if you want to
> explicitly test it, I'd move it before this statement.
>
> > netdev_warn(ds->ports[port].netdev,
> > "hardware VLAN %d already used by %s\n",
> > vlan.vid,
Hi Vivien
I don't think you comment is correct. Here is the loop, with my two
additions.
for (i = 0; i < mv88e6xxx_num_ports(chip); ++i) {
if (dsa_is_dsa_port(ds, i) || dsa_is_cpu_port(ds, i))
continue;
if (!ds->ports[port].netdev)
continue;
if (vlan.data[i] ==
GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER)
continue;
if (chip->ports[i].bridge_dev ==
chip->ports[port].bridge_dev)
break; /* same bridge, check next VLAN */
if (!chip->ports[i].bridge_dev)
continue;
netdev_warn(ds->ports[port].netdev,
"hardware VLAN %d already used by %s\n",
vlan.vid,
netdev_name(chip->ports[i].bridge_dev));
err = -EOPNOTSUPP;
goto unlock;
}
The opps was occurring in netdev_name(). I did not check which
one. The obvious one, or the netdev_warn(netdev,... one.
> The above truncated test:
>
> if (chip->ports[i].bridge_dev ==
> chip->ports[port].bridge_dev)
> break; /* same bridge, check next VLAN */
>
> should handle the case where bridge_dev is NULL, but if you want to
> explicitly test it, I'd move it before this statement.
This will not stop chip->ports[i].bridge_dev == NULL from reaching the
netdev_warn().
Andrew
next prev parent reply other threads:[~2016-12-12 13:04 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-11 20:07 [PATCH net] net: dsa: mv88e6xxx: Fix opps when adding vlan bridge Andrew Lunn
2016-12-11 21:02 ` Vivien Didelot
2016-12-12 13:04 ` Andrew Lunn [this message]
2016-12-12 13:15 ` Vivien Didelot
2016-12-16 18:27 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161212130416.GC27057@lunn.ch \
--to=andrew@lunn$(echo .)ch \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=vivien.didelot@savoirfairelinux$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox