Date:	Tue, 01 Oct 2013 20:56:42 +0900
From:	Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
To:	vyasevic@...hat.com
Cc:	Toshiaki Makita <toshiaki.makita1@...il.com>,
	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	Fernando Luis Vazquez Cao <fernando_b1@....ntt.co.jp>,
	Patrick McHardy <kaber@...sh.net>
Subject: Re: [PATCH net 0/4] bridge: Fix problems around the PVID

On Mon, 2013-09-30 at 12:01 -0400, Vlad Yasevich wrote:
> On 09/30/2013 07:46 AM, Toshiaki Makita wrote:
> > On Fri, 2013-09-27 at 14:10 -0400, Vlad Yasevich wrote:
> >> On 09/27/2013 01:11 PM, Toshiaki Makita wrote:
> >>> On Thu, 2013-09-26 at 10:22 -0400, Vlad Yasevich wrote:
> >>>> On 09/26/2013 06:38 AM, Toshiaki Makita wrote:
> >>>>> On Tue, 2013-09-24 at 13:55 -0400, Vlad Yasevich wrote:
> >>>>>> On 09/24/2013 01:30 PM, Toshiaki Makita wrote:
> >>>>>>> On Tue, 2013-09-24 at 09:35 -0400, Vlad Yasevich wrote:
> >>>>>>>> On 09/24/2013 07:45 AM, Toshiaki Makita wrote:
> >>>>>>>>> On Mon, 2013-09-23 at 10:41 -0400, Vlad Yasevich
> >>>>>>>>> wrote:
> >>>>>>>>>> On 09/17/2013 04:12 AM, Toshiaki Makita wrote:
> >>>>>>>>>>> On Mon, 2013-09-16 at 13:49 -0400, Vlad Yasevich
> >>>>>>>>>>> wrote:
> >>>>>>>>>>>> On 09/13/2013 08:06 AM, Toshiaki Makita wrote:
> >>>>>>>>>>>>> On Thu, 2013-09-12 at 16:00 -0400, David
> >>>>>>>>>>>>> Miller wrote:
> >>>>>>>>>>>>>> From: Toshiaki Makita
> >>>>>>>>>>>>>> <makita.toshiaki@....ntt.co.jp> Date: Tue,
> >>>>>>>>>>>>>> 10 Sep 2013 19:27:54 +0900
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> There seem to be some undesirable
> >>>>>>>>>>>>>>> behaviors related with PVID. 1. It has no
> >>>>>>>>>>>>>>> effect assigning PVID to a port. PVID
> >>>>>>>>>>>>>>> cannot be applied to any frame regardless
> >>>>>>>>>>>>>>> of whether we set it or not. 2. FDB
> >>>>>>>>>>>>>>> entries learned via frames applied PVID
> >>>>>>>>>>>>>>> are registered with VID 0 rather than VID
> >>>>>>>>>>>>>>> value of PVID. 3. We can set 0 or 4095 as
> >>>>>>>>>>>>>>> a PVID that are not allowed in IEEE
> >>>>>>>>>>>>>>> 802.1Q. This leads interoperational
> >>>>>>>>>>>>>>> problems such as sending frames with VID
> >>>>>>>>>>>>>>> 4095, which is not allowed in IEEE
> >>>>>>>>>>>>>>> 802.1Q, and treating frames with VID 0 as
> >>>>>>>>>>>>>>> they belong to VLAN 0, which is expected
> >>>>>>>>>>>>>>> to be handled as they have no VID
> >>>>>>>>>>>>>>> according to IEEE 802.1Q.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Note: 2nd and 3rd problems are potential
> >>>>>>>>>>>>>>> and not exposed unless 1st problem is
> >>>>>>>>>>>>>>> fixed, because we cannot activate PVID
> >>>>>>>>>>>>>>> due to it.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Please work out the issues in patch #2 with
> >>>>>>>>>>>>>> Vlad and resubmit this series.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Thank you.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I'm hovering between whether we should fix
> >>>>>>>>>>>>> the issue by changing vlan 0 interface
> >>>>>>>>>>>>> behavior in 8021q module or enabling a bridge
> >>>>>>>>>>>>> port to sending priority-tagged frames, or
> >>>>>>>>>>>>> another better way.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> If you could comment it, I'd appreciate it
> >>>>>>>>>>>>> :)
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> BTW, I think what is discussed in patch #2 is
> >>>>>>>>>>>>> another problem about handling priority-tags,
> >>>>>>>>>>>>> and it exists without this patch set
> >>>>>>>>>>>>> applied. It looks like that we should prepare
> >>>>>>>>>>>>> another patch set than this to fix that
> >>>>>>>>>>>>> problem.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Should I include patches that fix the
> >>>>>>>>>>>>> priority-tags problem in this patch set and
> >>>>>>>>>>>>> resubmit them all together?
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> I am thinking that we might need to do it in
> >>>>>>>>>>>> bridge and it looks like the simplest way to do
> >>>>>>>>>>>> it is to have default priority regeneration
> >>>>>>>>>>>> table (table 6-5 from 802.1Q doc).
> >>>>>>>>>>>>
> >>>>>>>>>>>> That way I think we would conform to the spec.
> >>>>>>>>>>>>
> >>>>>>>>>>>> -vlad
> >>>>>>>>>>>
> >>>>>>>>>>> Unfortunately I don't think the default priority
> >>>>>>>>>>> regeneration table resolves the problem because
> >>>>>>>>>>> IEEE 802.1Q says that a VLAN-aware bridge can
> >>>>>>>>>>> transmit untagged or VLAN-tagged frames only (the
> >>>>>>>>>>> end of section 7.5 and 8.1.7).
> >>>>>>>>>>>
> >>>>>>>>>>> No mechanism to send priority-tagged frames is
> >>>>>>>>>>> found as far as I can see the standard. I think
> >>>>>>>>>>> the regenerated priority is used for outgoing
> >>>>>>>>>>> PCP field only if egress policy is not untagged
> >>>>>>>>>>> (i.e. transmitting as VLAN-tagged), and unused if
> >>>>>>>>>>> untagged (Section 6.9.2 3rd/4th Paragraph).
> >>>>>>>>>>>
> >>>>>>>>>>> If we want to transmit priority-tagged frames
> >>>>>>>>>>> from a bridge port, I think we need to implement
> >>>>>>>>>>> a new (optional) feature that is above the
> >>>>>>>>>>> standard, as I stated previously.
> >>>>>>>>>>>
> >>>>>>>>>>> How do you feel about adding a per-port policy
> >>>>>>>>>>> that enables a bridge to send priority-tagged
> >>>>>>>>>>> frames instead of untagged frames when egress
> >>>>>>>>>>> policy for the port is untagged? With this
> >>>>>>>>>>> change, we can transmit frames for a given vlan
> >>>>>>>>>>> as either all untagged, all priority-tagged or
> >>>>>>>>>>> all VLAN-tagged.
> >>>>>>>>>>
> >>>>>>>>>> That would work.  What I am thinking is that we do
> >>>>>>>>>> it by special casing the vid 0 egress policy
> >>>>>>>>>> specification.  Let it be untagged by default and
> >>>>>>>>>> if it is tagged, then we preserve the priority
> >>>>>>>>>> field and forward it on.
> >>>>>>>>>>
> >>>>>>>>>> This keeps the API stable and doesn't require
> >>>>>>>>>> user/admin from knowing exactly what happens.
> >>>>>>>>>> Default operation conforms to the spec and allows
> >>>>>>>>>> simple change to make it backward-compatible.
> >>>>>>>>>>
> >>>>>>>>>> What do you think.  I've done a simple prototype of
> >>>>>>>>>> this an it seems to work with the VMs I am testing
> >>>>>>>>>> with.
> >>>>>>>>>
> >>>>>>>>> Are you saying that - by default, set the 0th bit of
> >>>>>>>>> untagged_bitmap; and - if we unset the 0th bit and
> >>>>>>>>> set the "vid"th bit, we transmit frames classified as
> >>>>>>>>> belonging to VLAN "vid" as priority-tagged?
> >>>>>>>>>
> >>>>>>>>> If so, though it's attractive to keep current API,
> >>>>>>>>> I'm worried about if it could be a bit confusing and
> >>>>>>>>> not intuitive for kernel/iproute2 developers that VID
> >>>>>>>>> 0 has a special meaning only in the egress policy.
> >>>>>>>>> Wouldn't it be better to adding a new member to
> >>>>>>>>> struct net_port_vlans instead of using VID 0 of
> >>>>>>>>> untagged_bitmap?
> >>>>>>>>>
> >>>>>>>>> Or are you saying that we use a new flag in struct
> >>>>>>>>> net_port_vlans but use the BRIDGE_VLAN_INFO_UNTAGGED
> >>>>>>>>> bit with VID 0 in netlink to set the flag?
> >>>>>>>>>
> >>>>>>>>> Even in that case, I'm afraid that it might be
> >>>>>>>>> confusing for developers for the same reason. We are
> >>>>>>>>> going to prohibit to specify VID with 0 (and 4095) in
> >>>>>>>>> adding/deleting a FDB entry or a vlan filtering
> >>>>>>>>> entry, but it would allow us to use VID 0 only when a
> >>>>>>>>> vlan filtering entry is configured. I am thinking a
> >>>>>>>>> new nlattr is a straightforward approach to
> >>>>>>>>> configure it.
> >>>>>>>>
> >>>>>>>> By making this an explicit attribute it makes vid 0 a
> >>>>>>>> special case for any automatic tool that would
> >>>>>>>> provision such filtering.  Seeing vid 0 would mean that
> >>>>>>>> these tools would have to know that this would have to
> >>>>>>>> be translated to a different attribute instead of
> >>>>>>>> setting the policy values.
> >>>>>>>
> >>>>>>> Yes, I agree with you that we can do it by the way you
> >>>>>>> explained. What I don't understand is the advantage of
> >>>>>>> using vid 0 over another way such as adding a new
> >>>>>>> nlattr. I think we can indicate transmitting
> >>>>>>> priority-tags explicitly by such a nlattr. Using vid 0
> >>>>>>> seems to be easier to implement than a new nlattr, but,
> >>>>>>> for me, it looks less intuitive and more difficult to
> >>>>>>> maintain because we have to care about vid 0 instead of
> >>>>>>> simply ignoring it.
> >>>>>>>
> >>>>>>
> >>>>>> The point I am trying to make is that regardless of the
> >>>>>> approach someone has to know what to do when enabling
> >>>>>> priority tagged frames.  You proposal would require the
> >>>>>> administrator or config tool to have that knowledge.
> >>>>>> Example is: Admin does: bridge vlan set priority on dev
> >>>>>> eth0 Automated app: if (vid == 0) /* Turn on priority
> >>>>>> tagged frame support */
> >>>>>>
> >>>>>> My proposal would require the bridge filtering
> >>>>>> implementation to have it. user tool: bridge vlan add vid 0
> >>>>>> tagged Automated app:  No special case.
> >>>>>>
> >>>>>> IMO its better to have 1 piece code handling the special
> >>>>>> case then putting it multiple places.
> >>>>>
> >>>>> Thank you for the detailed explanation. Now I understand your
> >>>>> intention.
> >>>>>
> >>>>> I have one question about your proposal. I guess the way to
> >>>>> enable priority-tagged is something like bridge vlan add vid
> >>>>> 10 dev eth0 bridge vlan add vid 10 dev vnet0 pvid untagged
> >>>>> bridge vlan add vid 0 dev vnet0 tagged where vnet0 has sub
> >>>>> interface vnet0.0.
> >>>>>
> >>>>> Here the admin have to know the egress policy is applied to a
> >>>>> frame twice in a certain order when it is transmitted from
> >>>>> the port vnet0 attached, that is, first, a frame with vid 10
> >>>>> get untagged, and then, an untagged frame get
> >>>>> priority-tagged.
> >>>>>
> >>>>> This behavior looks difficult to know without previous
> >>>>> knowledge. Any good idea to avoid such a need for the admin's
> >>>>> additional knowledge?
> >>>>
> >>>> To me, the fact that there is vnet0.0 (or typically, there is
> >>>> eth0.0 in the guest or on the remote host) already tells the
> >>>> admin vlan 0 has to be tagged.  The fact that we codify this in
> >>>> the policy makes it explicit.
> >>>
> >>> My worry is that the admin might not be able to guess how to use
> >>> bridge commands to enable priority-tag without any additional
> >>> hint in "man bridge", "bridge vlan help", etc. I actually
> >>> couldn't hit upon such a usage before seeing example commands you
> >>> gave, because I had never think the egress policy could be
> >>> applied twice.
> >>>
> >>>>
> >>>> However, I can see strong argument to be made for an addition
> >>>> egress policy attribute that could be for instance:
> >>>>
> >>>> bridge vlan add vid 10 dev eth0 pvid bridge vlan add vid 10 dev
> >>>> vnet0 pvid untagged prio_tag
> >>>>
> >>>> But this has the same connotations as wrt to egress policy.
> >>>> The 2 policies are applied: (1) untag the frame. (2) add
> >>>> priority_tag.
> >>>>
> >>>> (2) only happens if initial fame received on eth0 was priority
> >>>> tagged.
> >>>
> >>> If we do so, we will not be able to communicate using vlan 0
> >>> interface under a certain circumstance. Eth0 can be receive mixed
> >>> untagged and priority-tagged frames according to the network
> >>> element it is connected to: for example, Open vSwitch can send
> >>> such two kinds of frames from the same port even if original
> >>> incoming frames belong to the same vlan.
> >>
> >> Which priority would you assign to the frame that was received
> >> untagged?
> >
> > Untagged frame's priority is by default 0, so I think 0 is likely.
> >
> > 802.1Q 6.9.1 i) The received priority value and the drop_eligible
> > parameter value are the values in the M_UNITDATA.indication.
> >
> > M_UNITDATA is passed from ISS.
> >
> > 802.1Q 6.7.1 The priority parameter provided in a data indication
> > primitive shall take the value of the Default User Priority parameter
> > for the Port through which the MAC frame was received. The default
> > value of this parameter is 0, it may be set by management in which
> > case the capability to set it to any of the values 0 through 7 shall
> > be provided.
> >
> >>
> >>> In this situation, we can only receive frames that is
> >>> priority-tagged when received on eth0.
> >>
> >> Not sure I understand.  Let's look at this config: bridge vlan add
> >> vid 10 dev eth0 pvid bridge vlan add vid 10 dev vnet0 pvid untagged
> >> prio_tag
> >>
> >> Here, eth0 is allowed to receive vid 10 tagged, untagged, and
> >> prio_tagged (if we look at the patch 2 from this set). Now, frame
> >> is forwarded to vnet0. 1) if the frame had vid 10 in the tag or was
> >> untagged, it should probably be sent untagged. 2) if the frame had
> >> a priority tag, it should probably be sent as such.
> >>
> >> Now, I think a case could be made that if the frame had any
> >> priority markings in the vlan header, we should try to preserve
> >> those markings if prio_tag is turned on.  We can assume value of 0
> >> means not set.
> >
> > If we don't insert prio_tag when PCP is 0, we might receive mixed
> > priority-tagged and untagged frames on eth0.
> 
> Right, and that's what you were trying to handle in your patch:
> 
> > +		/* PVID is set on this port.  Any untagged or priority-tagged +
> > * ingress frame is considered to belong to this vlan. */
> 
> So, in this case we are prepared to handle the "mixed" scenario on ingress.
> 
> > Even if we are sending frames from eth0.0 with some priority other
> > than 0, we could receive frames with priority 0 or untagged on the
> > other side of the bridge.
> > For example, if we receive untagged arp reply on the bridge port, we
> > migit not be able to communicate with such an end station, because
> > untagged reply will not be passed to eth0.0.
> 
> So the ARP request was sent tagged, but the reply came back untagged?

Yes, it can happen.
These are problematic cases.

Example 1:
            prio_tagged         prio_tagged
+------------+ ---> +------------+ ---> +----------+
|guest eth0.0|------|host1 Bridge|------|host2 eth0|
+------------+ <--- +------------+ <--- +----------+
             untagged            untagged

Note: Host2 eth0, which is an interface on Linux, can receive
priority-tagged frames if it doesn't have vlan 0 interface (eth0.0).


Example 2:

            prio_tagged         prio_tagged       untagged
+------------+ ---> +------------+ ---> +---------+ ---> +-----+
|guest eth0.0|------|host1 Bridge|------|HW switch|------|host2|
+------------+ <--- +------------+ <--- +---------+ <--- +-----+
             untagged            untagged       prio/untagged

Note: 802.1Q conformed HW switch transmits only untagged or VLAN-tagged
frames.

> 
> How does that work when the end station is attached directly to the
> HW switch instead of a linux bridge?
> 
> The station configures eth0.0 and sends priority-tagged traffic to
> the HW switch.  If the HW switch sends back untagged traffic, then
> the untagged traffic will never reach eth0.0.

Currently we cannot communicate using eth0.0 via directly connected
802.1Q conformed switch, because we never receive priority-tagged frames
from the switch.
It is not a problem of Linux bridge and is why I wondered whether it
should be fixed by bridge or vlan 0 interface.

> 
> >
> >>
> >>> IMO, if prio_tag is configured, the bridge should send any
> >>> untagged frame as priority-tagged regardless of whatever it is on
> >>> eth0.
> >>
> >> Which priority would you use, 0?  You are not guaranteed to
> >> properly deliver the traffic then for a configuration such as: VM:
> >> eth0: 10.0.0.1/24 eth0.0: 10.0.1.1/24
> >
> > I'd like to use priority 0 for untagged frames.
> >
> > I am assuming that one of our goals is at least that eth0.0 comes to
> > be able to communicate with another end station. It seems to be hard
> > to use both eth0 and eth0.0 simultaneously.
> 
> I understand, but I don't agree that we should always tag.
> 
> Consider config:
> 
>     hw switch <---> (eth0: Linux Bridge: eth1) <--- (em1.0:end station)
> 
> If the end station sends priority-tagged traffic it should receive
> priority tagged traffic back.  Otherwise, untagged traffic may be 
> dropped by the end station.  This is true whether it is connected to
> the hw switch or Linux bridge.

Though such a behavior is generally not necessary as far as I can read
802.1Q spec, it is essential for vlan 0 interface on Linux, I think.
My proposal aims to resolve it at least when we use Linux bridge.

Example configuration:
	bridge vlan add vid 10 dev eth1 pvid untagged
	bridge vlan add vid 10 dev eth0
	bridge vlan set prio_tag on dev eth1

Intended behavior:

        VID10-tagged                     prio_tagged
+---------+ <--- +------------------------+ <--- +-----------------+
|hw switch|------|eth0: Linux Bridge: eth1|------|em1.0:end station|
+---------+ ---> +------------------------+ ---> +-----------------+
        VID10-tagged                     prio_tagged
                              (always if egress policy untagged)

Thanks,

Toshiaki Makita

> 
> -vlad
> 
> >
> > Thanks,
> >
> > Toshiaki Makita
> >
> >>
> >> -vlad
> >>
> >>>
> >>>>
> >>>> I think I am ok with either approach.  Explicit vid 0 policy is
> >>>> easier for automatic provisioning.   The flag based one is
> >>>> easier for admin/ manual provisioning.
> >>>
> >>> Supposing we have to add something to help or man in any case, I
> >>> think flag based is better. The format below seems to suitable
> >>> for a per-port policy. bridge vlan set prio_tag on dev vnet0
> >>>
> >>> Thanks,
> >>>
> >>> Toshiaki Makita
> >>>
> >>>>
> >>>> -vlad.
> >>>>
> >>>> -vlad
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>
> >>>>>>
> >>>>>> Thanks -vlad
> >>>>>>
> >>>>>>> Thanks,
> >>>>>>>
> >>>>>>> Toshiaki Makita
> >>>>>>>
> >>>>>>>>
> >>>>>>>> How it is implemented internally in the kernel isn't as
> >>>>>>>> big of an issue. We can do it as a separate flag or as
> >>>>>>>> part of existing policy.
> >>>>>>>>
> >>>>>>>> -vlad
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>>
> >>>>>>>>> Toshiaki Makita
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> -vlad
> >>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> Thanks,
> >>>>>>>>>>>
> >>>>>>>>>>> Toshiaki Makita
> >>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Thanks,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Toshiaki Makita
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> -- To unsubscribe from this list: send the
> >>>>>>>>>>>>>> line "unsubscribe netdev" in the body of a
> >>>>>>>>>>>>>> message to majordomo@...r.kernel.org More
> >>>>>>>>>>>>>> majordomo info at
> >>>>>>>>>>>>>> http://vger.kernel.org/majordomo-info.html
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> -- To unsubscribe from this list: send the line
> >>>>>>>>>>> "unsubscribe netdev" in the body of a message to
> >>>>>>>>>>> majordomo@...r.kernel.org More majordomo info at
> >>>>>>>>>>> http://vger.kernel.org/majordomo-info.html
> >>>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>
> >>>
> >
> >


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists