On Thu, Jul 17, 2014 at 4:08 PM, Jesse Gross <je...@nicira.com> wrote: > On Thu, Jul 17, 2014 at 3:46 PM, Ansis Atteka <aatt...@nicira.com> wrote: >> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c >> index 5f975a1..a4108c0 100644 >> --- a/datapath/flow_netlink.c >> +++ b/datapath/flow_netlink.c >> @@ -1034,7 +1034,7 @@ int ovs_nla_put_flow(struct datapath *dp, const struct >> sw_flow_key *swkey, >> struct vport *in_port; >> >> in_port = ovs_vport_ovsl_rcu(dp, swkey->phy.in_port); >> - if (in_port->ops->type == OVS_VPORT_TYPE_GENEVE) >> + if (in_port && in_port->ops->type == >> OVS_VPORT_TYPE_GENEVE) > > I think is probably not only an issue for Geneve ports (since the > crash happens before we get to check the type) but for tunnel ports in > general. I also guess that the scenario is not that the port hasn't > been created yet but that the port has been deleted and the flow still > exists. I somehow remember reproducing this issue only when switching from gre to geneve. Either, that implies: 1. issue with my test; or 2. when switching from gre to geneve, then the window during which vport could be NULL is longer.
Wouldn't be surprised that it is #1. > > One thing that I worry about is that this has the possibility to > change how the flow is reported - before the flow deletion it has > Geneve options but immediately after the flow still exists but without > options. OVS will likely deal with this but it doesn't seem like a > great thing. I talked with Pravin on how to solve this "flow changing while vport gets added" issue and he seemed to prefer the idea where we simply look into tun_opts_len to figure out whether to append Geneve attributes. Basically, the new patch makes assumption that if tun_opts_len>0 then that is Geneve port that could potentially have Geneve options. Also, I remember you mentioning that it might be good to distinguish between the case when there are no tunnel options at all and the case when Geneve attributes contain an empty set. Patch v2 does not address that. How important is that in your opinion? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
