On Sun, Feb 02, 2014 at 10:45:29PM -0800, Jesse Gross wrote:
> On Fri, Jan 31, 2014 at 4:59 PM, Ben Pfaff <b...@nicira.com> wrote:
> > On Fri, Jan 31, 2014 at 10:38:20AM -0800, Jesse Gross wrote:
> >> On Thu, Jan 30, 2014 at 1:31 PM, Thomas Morin <thomas.mo...@orange.com>
> >> wrote:
> >> > Hi,
> >> >
> >> > I ran into the same issue as the one described below (post on
> >> > ovs-discuss a
> >> > few months back).
> >> >
> >> > The goal is to use a vport tunnel configured to use a flow-based remote
> >> > IP,
> >> > e.g:
> >> > ovs-vscl add-port br-vnet tun-port -- set Interface tun-port type=gre
> >> > options:remote_ip=flow
> >> >
> >> > This works fine on the sending OVS, but on the receiving OVS the same
> >> > vport
> >> > won't receive any traffic (packets received trigger a "receive tunnel
> >> > port
> >> > not found" warning, and are not further processed).
> >> >
> >> > The original poster had found a workaround relying on also using
> >> > options:key=flow, but as far as I understand, this work around should
> >> > not be
> >> > necessary: it just seems that the tunnel matching code is missing a
> >> > match.
> >> >
> >> > --- a/ofproto/tunnel.c
> >> > +++ b/ofproto/tunnel.c
> >> > @@ -427,8 +427,9 @@ tnl_find(const struct flow *flow)
> >> > OVS_REQ_RDLOCK(rwlock)
> >> > { false, false, IP_SRC_ANY }, /* remote_ip, in_key. */
> >> > { true, false, IP_SRC_CFG }, /* remote_ip, local_ip. */
> >> > { true, false, IP_SRC_ANY }, /* remote_ip. */
> >> > - { true, true, IP_SRC_ANY }, /* Flow-based remote. */
> >> > + { true, true, IP_SRC_ANY }, /* Flow-based remote and
> >> > flow-based
> >> > key. */
> >> > { true, true, IP_SRC_FLOW }, /* Flow-based everything. */
> >> > + { false, true, IP_SRC_ANY }, /* Flow-based remote */
> >> > };
> >> >
> >> > const struct tnl_match_pattern *p;
> >> >
> >> > With this patch, a tunnel configured with just remote_ip=flow will work
> >> > fine
> >> > on the receiving end.
> >> > One thing that I'm not sure is at which position this pattern should
> >> > appear
> >> > in the list.
> >> > The patch also changes the comment for true/true/IP_SRC_ANY.
> >>
> >> This looks fine to me although I think that it the new entry belongs
> >> first in the list of flow-based types. Can you submit a formal patch
> >> with this change?
> >
> > By my count there are 12 possible entries in the list of
> > possibilities. I don't know if there's any point in adding them one
> > by one. I guess we might as well add all of them at one
> > systematically.
> >
> > How about this, then? It's not correct--I haven't debugged it--but it
> > gives the flavor of the idea.
>
> I think it is a reasonable thing to do although I think it might
> change the priority order that we have now - should we switch the two
> outer loops when we do the lookup?
I'm fairly sure that the order here is correct because this program:
#include <stdio.h>
int
main(void)
{
int in_key_flow, ip_dst_flow, ip_src;
for (in_key_flow = 0; in_key_flow < 2; in_key_flow++) {
for (ip_dst_flow = 0; ip_dst_flow < 2; ip_dst_flow++) {
for (ip_src = 0; ip_src < 3; ip_src++) {
printf("%d %d %d\n", in_key_flow, ip_dst_flow, ip_src);
}
}
}
return 0;
}
prints an order that matches the patterns[] array (with lines
deleted):
0 0 0
0 0 1
1 0 0
1 0 1
1 1 1
1 1 2
I'll look this over a bit and properly submit it.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
