On Mon, Nov 19, 2018 at 07:32:46AM -0800, Sim Paul wrote:
> >
> >
> > > I am still trying to understand the test case behavior that i pasted in
> > my
> > > previous email.
> > > In my first test case when vlan-limit=1, the ping worked because
> > > only the outside VLAN tag (36) was inspected ??
> > > But in second case when i set vlan-limit=2, ping stopped working because
> > > both tags 36 and 120 were inspected ?
> > >
> > > Shouldn't the ping work even in second test case ?
> >
> > I'm not sure. Your configuration is a big odd. dot1q-tunnel should only
> > be configured at the ends, but it sounds like you've added it to the
> > patch ports as well.
> >
> > Are you saying you are able to ping a virtual machine sitting on a
> neighboring ovs bridge
> by simply configuring dot1q-tunnel at the end points (VM NICs) ? Plz
> confirm.
> For me, if i don't configure all 4 ports(two VM VNICs and two patch ports)
> as dot1q-tunnel,
> VM1 sitting on ovsbr1 CANNOT ping VM2 sitting on ovsbr2.

Sorry for the delay. I was on holiday and traveling.

I took another look. Using dot1q-tunnel on an already double tagged
packet will not work if vlan-limit == 2 or if vlan-limit == 0.

During the xlate phase the dot1q-tunnel temporarily pushes another VLAN
tag to the internal xlate structures - think of it as an implicit
push_vlan. Because the flow already has two tags it shifts the VLANs to
the right and the right most VLAN is lost. On the other-end the
dot1q-tunnel VLAN is stripped leaving a single VLAN.
i.e.


    ingress input:  [VLAN 36] [VLAN 100]
    ingress output: [VLAN xx] [VLAN 36]
     egress input:  [VLAN xx] [VLAN 36]
     egress output:           [VLAN 36]

where "xx" is your dot1q-tunnel tag.

So why does it work with vlan-limit=1 ?

Recall that with vlan-limit=1 the second VLAN is _not_ parsed as a VLAN
(it'll be the dl_type). The xlate structure has slots for 2 VLANs
regardless of the value of vlan-limit. This means the temporary/internal
shift of the VLAN works as there is room (only one VLAN was parsed).

Possible fix:

I think struct xvlan could be of size FLOW_MAX_VLAN_HEADERS + 1 to allow
the temporary/internal shift caused by dot1q-tunnel. Although I'm not
sure if this would cause a regression elsewhere.

Can you try this untested patch?

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 507e14dd0d00..4f86e7704a50 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -418,7 +418,8 @@ struct xvlan_single {
 };

 struct xvlan {
-    struct xvlan_single v[FLOW_MAX_VLAN_HEADERS];
+   /* Add 1 to the size to allow temporary/internal shift for dot1q-tunnel. */
+    struct xvlan_single v[FLOW_MAX_VLAN_HEADERS + 1];
 };

 const char *xlate_strerror(enum xlate_error error)
_______________________________________________
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss

Reply via email to