At first glance I would try to skip or enhance the port and bridge lookup in upcall_receive() or xlate_lookup() if flow->recirc_id != 0.
The code in xlate_actions() actually already today restores bridge and flow.in_port from the thawed frozen_state. We need to make sure that prior to reaching that point code does not stop working if the upcall bridge and in_port are not populated properly. If there is code that requires this, we may want to move the lookup of the recirc node and frozen state a little bit up the call stack so that we can initialize it from the frozen state. I wonder if recirc_id and frozen bridge and in_port are only relevant in the context of MISS upcalls or also for SFLOW, IPFIX and FLOW_SAMPLE upcalls. There seems to be some "magic" also in those parts of the code that guess the in_port for ingress tunneling, so they might also benefit. Regards, Jan > -----Original Message----- > From: Jan Scheurich > Sent: Friday, 08 December, 2017 11:30 > To: Zoltán Balogh <zoltan.bal...@ericsson.com>; d...@openvswitch.org > Cc: Ben Pfaff <b...@ovn.org> > Subject: RE: [PATCH] tunnel: fix tnl_find() after packet_type changed > > Hi Zoltan, > > My feeling here is that it is conceptually wrong to do another tunnel lookup > if the packet is recirculated after it has already been de- > tunneled. You are trying to fix the symptoms and I am not sure that the more > liberal check on packet type won't lead to incorrect proper > tunnel port lookups. > > The actual bridge is stored in the frozen state of the recirculation context, > and the OF in_port should be stored there as well (if it is not > yet). I believe a recirculation should never change the bridge and the OF > in_port. @Ben: Can you confirm? > > I think the order of things is wrong in an upcall. First ofproto should check > if it is a recirculation upcall, and only if it is not, it should derive > the bridge and the in_port from the flow. Today the first recirculation check > happens later in upcall_xlate(). > > Can you look into that option and propose a better way to solve the issue? > > Thanks, Jan > > > -----Original Message----- > > From: Zoltán Balogh > > Sent: Friday, 08 December, 2017 10:56 > > To: d...@openvswitch.org > > Cc: Zoltán Balogh <zoltan.bal...@ericsson.com>; Jan Scheurich > > <jan.scheur...@ericsson.com>; Ben Pfaff <b...@ovn.org> > > Subject: [PATCH] tunnel: fix tnl_find() after packet_type changed > > > > During xlate, it can happen that tnl_find() is invoked when flow > > packet_type has been already changed. For instance, pop_mpls and > > resubmit actions should be applied to the packet in overlay bridge after > > packet was received on a legacy_l3 tunnel port. > > In this case, packet is recirculated after pop_mpls, a new tunnel lookup > > is performed in order to find the proper ofproto, however packet_type of > > flow is already PT_ETHERNET while the tunnel port mode is > > NETDEV_PT_LEGACY_L3. So, no tunnel port is found and the packet is > > dropped. > > > > This fix does an additional tnl_find() if no port is found. It looks for > > L3 tunnel port in case of L2 packet and vice versa. > > > > Signed-off-by: Zoltan Balogh <zoltan.bal...@ericsson.com> > > CC: Jan Scheurich <jan.scheur...@ericsson.com> > > CC: Ben Pfaff <b...@ovn.org> > > Fixes: 875ab13020b1 ("userspace: Handling of versatile tunnel ports") > > --- > > ofproto/tunnel.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c > > index 1676f4d46..d497f026b 100644 > > --- a/ofproto/tunnel.c > > +++ b/ofproto/tunnel.c > > @@ -585,6 +585,19 @@ tnl_find(const struct flow *flow) > > OVS_REQ_RDLOCK(rwlock) > > if (tnl_port) { > > return tnl_port; > > } > > + > > + /* Maybe flow->packet_type has been already changed > > during > > + * xlate, so let's try to find L2 port for L3 packet or > > + * L3 port for L2 packet. */ > > + if (pt_ns(flow->packet_type) == OFPHTN_ETHERTYPE) { > > + match.pt_mode = NETDEV_PT_LEGACY_L2; > > + } else { > > + match.pt_mode = NETDEV_PT_LEGACY_L3; > > + } > > + tnl_port = tnl_find_exact(&match, map); > > + if (tnl_port) { > > + return tnl_port; > > + } > > } > > > > i++; > > -- > > 2.14.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev