On Sun, 13 Nov 2016 20:43:54 -0800, Pravin B Shelar wrote:
> @@ -1927,13 +1923,13 @@ static void vxlan_xmit_one(struct sk_buff *skb,
> struct net_device *dev,
> struct ip_tunnel_info *info;
> struct vxlan_dev *vxlan = netdev_priv(dev);
> struct sock *sk;
> - struct rtable *rt = NULL;
> const struct iphdr *old_iph;
> union vxlan_addr *dst;
> union vxlan_addr remote_ip, local_ip;
> union vxlan_addr *src;
> struct vxlan_metadata _md;
> struct vxlan_metadata *md = &_md;
> + struct dst_entry *ndst = NULL;
> __be16 src_port = 0, dst_port;
> __be32 vni, label;
> __be16 df = 0;
> @@ -2009,6 +2005,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct
> net_device *dev,
>
> if (dst->sa.sa_family == AF_INET) {
> struct vxlan_sock *sock4 = rcu_dereference(vxlan->vn4_sock);
> + struct rtable *rt;
>
> if (!sock4)
> goto drop;
> @@ -2030,7 +2027,8 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct
> net_device *dev,
> netdev_dbg(dev, "circular route to %pI4\n",
> &dst->sin.sin_addr.s_addr);
> dev->stats.collisions++;
> - goto rt_tx_error;
> + ip_rt_put(rt);
> + goto tx_error;
> }
>
> /* Bypass encapsulation if the destination is local */
> @@ -2053,12 +2051,13 @@ static void vxlan_xmit_one(struct sk_buff *skb,
> struct net_device *dev,
> else if (info->key.tun_flags & TUNNEL_DONT_FRAGMENT)
> df = htons(IP_DF);
>
> + ndst = &rt->dst;
It would be a bit cleaner to do this assignment just after rt is
assigned (but after the IS_ERR(rt) condition), get rid of the added
ip_rt_put call above and move the existing ip_rt_put call in the bypass
case just before the vxlan_encap_bypass call...
> tos = ip_tunnel_ecn_encap(tos, old_iph, skb);
> ttl = ttl ? : ip4_dst_hoplimit(&rt->dst);
> - err = vxlan_build_skb(skb, &rt->dst, sizeof(struct iphdr),
> + err = vxlan_build_skb(skb, ndst, sizeof(struct iphdr),
> vni, md, flags, udp_sum);
> if (err < 0)
> - goto xmit_tx_error;
> + goto tx_error;
>
> udp_tunnel_xmit_skb(rt, sk, skb, src->sin.sin_addr.s_addr,
> dst->sin.sin_addr.s_addr, tos, ttl, df,
> @@ -2066,7 +2065,6 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct
> net_device *dev,
> #if IS_ENABLED(CONFIG_IPV6)
> } else {
> struct vxlan_sock *sock6 = rcu_dereference(vxlan->vn6_sock);
> - struct dst_entry *ndst;
> u32 rt6i_flags;
>
> ndst = vxlan6_get_route(vxlan, sock6, skb,
> @@ -2078,13 +2076,13 @@ static void vxlan_xmit_one(struct sk_buff *skb,
> struct net_device *dev,
> netdev_dbg(dev, "no route to %pI6\n",
> &dst->sin6.sin6_addr);
> dev->stats.tx_carrier_errors++;
> + ndst = NULL;
> goto tx_error;
> }
>
> if (ndst->dev == dev) {
> netdev_dbg(dev, "circular route to %pI6\n",
> &dst->sin6.sin6_addr);
> - dst_release(ndst);
> dev->stats.collisions++;
> goto tx_error;
> }
> @@ -2096,12 +2094,12 @@ static void vxlan_xmit_one(struct sk_buff *skb,
> struct net_device *dev,
> !(rt6i_flags & (RTCF_BROADCAST | RTCF_MULTICAST))) {
> struct vxlan_dev *dst_vxlan;
>
> - dst_release(ndst);
> dst_vxlan = vxlan_find_vni(vxlan->net, vni,
> dst->sa.sa_family, dst_port,
> vxlan->flags);
> if (!dst_vxlan)
> goto tx_error;
> + dst_release(ndst);
> vxlan_encap_bypass(skb, vxlan, dst_vxlan);
> return;
> }
...the same way you have it here, in the IPv6 part. Could you change
the IPv4 part to match it?
Looks good otherwise. Seeing it, I like this version much more than v2.
Thanks!
Jiri
