Netdev Archive on
help / color / mirror / Atom feed
From: Vladimir Oltean <>
To: Kurt Kanzenbach <>
Cc: Andrew Lunn <>,
	Vivien Didelot <>,
	Florian Fainelli <>,
	"David S. Miller" <>,
	Jakub Kicinski <>,, Rob Herring <>,,
	Sebastian Andrzej Siewior <>,
	Richard Cochran <>,
	Kamil Alkhouri <>,
Subject: Re: [PATCH v5 2/7] net: dsa: Add DSA driver for Hirschmann Hellcreek switches
Date: Mon, 7 Sep 2020 13:48:21 +0300	[thread overview]
Message-ID: <20200907104821.kvu7bxvzwazzg7cv@skbuf> (raw)
In-Reply-To: <875z8qazq2.fsf@kurt>

On Mon, Sep 07, 2020 at 08:05:25AM +0200, Kurt Kanzenbach wrote:
> Well, that depends on whether hellcreek_vlan_add() is called for
> creating that vlan interfaces. In general: As soon as both ports are
> members of the same vlan that traffic is switched.

That's indeed what I would expect.
Not only that, but with your pvid-based setup, you only ensure port
separation for untagged traffic anyway. I don't think you even need to
call hellcreek_vlan_add() for VID 100 to be switched between ports,
because your .port_vlan_filtering callback does not in fact disable VLAN
awareness, it just configures the ports to not drop unknown VLANs. So,
arguably, VLAN classification is still performed. An untagged packet is
classified to the PVID, a tagged packet is classified to the VID in the
packet. So tagged packets bypass the separation.

So, I think that's not ok. I think the only proper way to solve this is
to inform the IP designers that VLANs are no substitute for a port
forwarding matrix (a lookup table that answers the question "can port i
forward to port j"). Switch ports that are individually addressable by
the network stack are a fundamental assumption of the switchdev

> > I remember asking in Message-ID: <20200716082935.snokd33kn52ixk5h@skbuf>
> > whether it would be possible for you to set
> > ds->configure_vlan_while_not_filtering = true during hellcreek_setup.
> > Did anything unexpected happen while trying that?
> No, that comment got lost.
> So looking at the flag: Does it mean the driver can receive vlan
> configurations when a bridge without vlan filtering is used? That might
> be problematic as this driver uses vlans for the port separation by
> default. This is undone when vlan filtering is set to 1 meaning vlan
> configurations can be received without any problems.

Generally speaking, the old DSA behavior is something that we're trying
to get rid of, once all drivers set the option to true. So a new driver
should not rely on it even if it needs something like that. If you need
caching of VLANs installed by the bridge and/or by the 8021q module,
then you can add those to a list, and restore them in the
.port_vlan_filtering callback by yourself. You can look at how sja1105
does that.


  reply	other threads:[~2020-09-07 10:48 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-04  6:27 [PATCH v5 0/7] Hirschmann Hellcreek DSA driver Kurt Kanzenbach
2020-09-04  6:27 ` [PATCH v5 1/7] net: dsa: Add tag handling for Hirschmann Hellcreek switches Kurt Kanzenbach
2020-09-04  6:27 ` [PATCH v5 2/7] net: dsa: Add DSA driver " Kurt Kanzenbach
2020-09-05 20:42   ` Vladimir Oltean
2020-09-07  6:05     ` Kurt Kanzenbach
2020-09-07 10:48       ` Vladimir Oltean [this message]
2020-09-07 12:49         ` Kurt Kanzenbach
2020-09-07 13:21           ` Vladimir Oltean
2020-09-07 14:01             ` Kurt Kanzenbach
2020-09-07 15:17         ` Richard Cochran
2020-09-04  6:27 ` [PATCH v5 3/7] net: dsa: hellcreek: Add PTP clock support Kurt Kanzenbach
2020-09-04  6:27 ` [PATCH v5 4/7] net: dsa: hellcreek: Add support for hardware timestamping Kurt Kanzenbach
2020-09-04  6:27 ` [PATCH v5 5/7] net: dsa: hellcreek: Add PTP status LEDs Kurt Kanzenbach
2020-09-04  6:27 ` [PATCH v5 6/7] dt-bindings: Add vendor prefix for Hirschmann Kurt Kanzenbach
2020-09-04  6:27 ` [PATCH v5 7/7] dt-bindings: net: dsa: Add documentation for Hellcreek switches Kurt Kanzenbach

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200907104821.kvu7bxvzwazzg7cv@skbuf \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).