I agree that the memory handling is too fragile here. Maybe we should just make people attach the dst before calling into tunneling routines though. It might be easier considering the multiple entry points (i.e. GRE calls iptunnel_xmit() directly).
Several callers are definitely expecting iptunnel_xmit() to potentially return an error code. It's probably less confusing to not buck that trend and keep it as is. I guess in theory it could return an error code, it just doesn't right now. On Wed, Oct 1, 2014 at 1:11 PM, Andy Zhou <az...@nicira.com> wrote: > It turns out rout entry will not be attached to skb until > iptunnel_xmit, which does not return negative value. So there is no > bug here. > > I will drop this patch, and modify the upstream vport-geneve.c accordingly. > > It is pretty subtle that the correctness of free depends on the > implementation details of the API. May be it will be better for > vxlan_xmit_skb always > attach the route skb at the very beginning? > > Should we also make iptunnel_xmit() return type as unsigned int? > > On Wed, Oct 1, 2014 at 10:42 AM, Jesse Gross <je...@nicira.com> wrote: >> On Wed, Oct 1, 2014 at 1:02 AM, Andy Zhou <az...@nicira.com> wrote: >>> Route entry will be free on error by vport when freeing skb. >>> additional error check and free after xmit() will cause double free. >>> >>> Signed-off-by: Andy Zhou <az...@nicira.com> >> >> I'm not sure that the route entry is attached to the skb in all cases. >> If it's not then, this patch will cause a leak in those situations. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
