From: Vlad Yasevich <vyasevic@redhat.com>
To: "Michał Mirosław" <mirqus@gmail.com>
Cc: shemminger@vyatta.com, bridge@lists.linux-foundation.org,
davem@davemloft.net, netdev@vger.kernel.org
Subject: Re: [PATCH v9 net-next 05/12] bridge: Add the ability to configure pvid
Date: Fri, 01 Feb 2013 21:22:38 -0500 [thread overview]
Message-ID: <510C786E.5040406@redhat.com> (raw)
In-Reply-To: <CAHXqBFJ3FutyBGNAXPueWLqJncvzNqd69nTJc0nLgUqzFMTWjw@mail.gmail.com>
On 02/01/2013 08:15 PM, Michał Mirosław wrote:
> 2013/2/2 Michał Mirosław <mirqus@gmail.com>:
>> 2013/2/1 Vlad Yasevich <vyasevic@redhat.com>:
>>> A user may designate a certain vlan as PVID. This means that
>>> any ingress frame that does not contain a vlan tag is assigned to
>>> this vlan and any forwarding decisions are made with this vlan in mind.
>> [...]
>>> struct net_port_vlans {
>>> u16 port_idx;
>>> + u16 pvid;
>>
>> I'm confused about the implementation. I would expect pvid field in
>> net_bridge_port and adding a tag if it isn't there on ingress path.
>> The rest would be just like without PVIDs. But here you pvid field to
>> net_port_vlans, and don't do anything with it in receive nor transmit
>> path. Does it work? What am I missing?
>
> Found the answer in next patch (you should merge #5 and #6).
It was split for incremental testing. #5 added the ability to set and
delete it without impacting anything. #6 added the actual work that
pvid does.
> Still,
> the implementation looks overly complicated. If you force the packet
> to canonical form on ingress (keeping outer tag in skb->vlan_tci, and
> setting skb->vlan_tci = pvid if there is no tag) the code should get
> simpler.
What if there is no outer tag? That's what the ingress code is doing.
If there is no outer tag, pvid is written to vlan_tci. If there was
outer tag in vlan_tci, it's left alone. This way at the end of ingress
vlan_tci is always set.
At egress, we grab that tag and compare it against pvid if any. If it
matches, it's stripped. If it doesn't, we output with the tag thus
adding the header.
The only thing I can simplify is grab the tci directly at egress, but
that's what the code will do anyway.
-vlad
>
> Best Regards,
> Michał Mirosław
>
next prev parent reply other threads:[~2013-02-02 2:22 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-01 20:01 [PATCH v9 net-next 00/12] Add basic VLAN support to bridges Vlad Yasevich
2013-02-01 20:01 ` [PATCH v9 net-next 01/12] bridge: Add vlan filtering infrastructure Vlad Yasevich
2013-02-02 1:04 ` Michał Mirosław
2013-02-02 2:11 ` Vlad Yasevich
2013-02-01 20:02 ` [PATCH v9 net-next 02/12] bridge: Validate that vlan is permitted on ingress Vlad Yasevich
2013-02-02 1:04 ` Michał Mirosław
2013-02-02 2:13 ` Vlad Yasevich
2013-02-01 20:02 ` [PATCH v9 net-next 03/12] bridge: Verify that a vlan is allowed to egress on give port Vlad Yasevich
2013-02-01 20:02 ` [PATCH v9 net-next 04/12] bridge: Add netlink interface to configure vlans on bridge ports Vlad Yasevich
2013-02-01 20:02 ` [PATCH v9 net-next 05/12] bridge: Add the ability to configure pvid Vlad Yasevich
2013-02-02 1:04 ` Michał Mirosław
2013-02-02 1:15 ` Michał Mirosław
2013-02-02 2:22 ` Vlad Yasevich [this message]
2013-02-03 14:49 ` Michał Mirosław
2013-02-03 15:20 ` Michał Mirosław
2013-02-04 16:59 ` Vlad Yasevich
2013-02-01 20:02 ` [PATCH v9 net-next 06/12] bridge: Implement vlan ingress/egress policy Vlad Yasevich
2013-02-01 20:02 ` [PATCH v9 net-next 07/12] bridge: Add vlan to unicast fdb entries Vlad Yasevich
2013-02-01 20:02 ` [PATCH v9 net-next 08/12] bridge: Add vlan id to multicast groups Vlad Yasevich
2013-02-01 20:02 ` [PATCH v9 net-next 09/12] bridge: Add vlan support to static neighbors Vlad Yasevich
2013-02-01 20:02 ` [PATCH v9 net-next 10/12] bridge: Add vlan support for local fdb entries Vlad Yasevich
2013-02-01 20:02 ` [PATCH v9 net-next 11/12] bridge: Dump vlan information from a bridge port Vlad Yasevich
2013-02-01 20:02 ` [PATCH v9 net-next 12/12] bridge: Separate egress policy bitmap Vlad Yasevich
2013-02-04 16:24 ` [PATCH v9 net-next 00/12] Add basic VLAN support to bridges Stephen Hemminger
2013-02-04 16:58 ` Vlad Yasevich
2013-02-07 22:48 ` Vlad Yasevich
2013-02-07 22:57 ` Stephen Hemminger
2013-02-07 23:00 ` Vlad Yasevich
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=510C786E.5040406@redhat.com \
--to=vyasevic@redhat.com \
--cc=bridge@lists.linux-foundation.org \
--cc=davem@davemloft.net \
--cc=mirqus@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=shemminger@vyatta.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* 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).