On Wed, Jan 20, 2016 at 11:31:08AM -0500, Russell Bryant wrote:
> Multiple logical ports on the same chassis that were connected to the
> same physical network via localnet ports were not able to send packets
> to each other. This was because ovn-controller created a single patch
> port between br-int and the physical network access bridge and used it
> for all localnet ports.
>
> The fix implemented here is to create a separate patch port for every
> logical port of type=localnet. An optimization is included where these
> ports are only created if the localnet port is on a logical switch with
> another logical port with an associated local VIF.
>
> A nice side effect of this fix is that the code in physical.c got a lot
> simpler, as localnet ports are now handled mostly like local VIFs.
>
> Fixes: c02819293d52 ("ovn: Add "localnet" logical port type.")
> Reported-by: Han Zhou <zhou...@gmail.com>
> Reported-at: http://openvswitch.org/pipermail/dev/2016-January/064413.html
> Signed-off-by: Russell Bryant <russ...@ovn.org>
> Tested-by: Kyle Mestery <mest...@mestery.com
> Acked-By: Kyle Mestery <mest...@mestery.com>
> ---
>
> v1->v2:
> - updated for feedback from Han Zhou.
I guess I reviewed v1 accidentally. Here's a copy of my feedback from
that version.
The new logic here in physical_run() looks wrong to me. I think that it
strips a vlan if it didn't match on one.
if (tag) {
match_set_dl_vlan(&match, htons(tag));
} else if (!strcmp(binding->type, "localnet")) {
/* Match priority-tagged frames, e.g. VLAN ID 0.
*
* We'll add a second flow for frames that lack any 802.1Q
* header later. */
match_set_dl_tci_masked(&match, htons(VLAN_CFI),
htons(VLAN_VID_MASK | VLAN_CFI));
}
/* Strip vlans. */
if (tag) {
ofpact_put_STRIP_VLAN(&ofpacts);
}
I believe that we can simplify it to the following, while fixing the bug
at the same time:
/* Match a VLAN tag and strip it, including stripping priority tags
* (e.g. VLAN ID 0). In the latter case we'll add a second flow
* for frames that lack any 802.1Q header later. */
if (tag || !strcmp(binding->type, "localnet")) {
match_set_dl_vlan(&match, htons(tag));
ofpact_put_STRIP_VLAN(&ofpacts);
}
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
