On Wed, Oct 1, 2014 at 1:38 PM, Jesse Gross <je...@nicira.com> wrote: > 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).
Yes, this is a better approach. I will fix up the Geneve driver before upstream. I will come up with separate patch for Vxlan. > > 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. Good point. However, if iptunnel_xmit() could return error code, then above fix is a must. > > 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
