* [PATCH net-next v3 1/7] vxlan: avoid vlan processing in vxlan device.
2016-11-14 4:43 [PATCH net-next v3 0/7] vxlan: xmit improvements Pravin B Shelar
@ 2016-11-14 4:43 ` Pravin B Shelar
2016-11-14 4:43 ` [PATCH net-next v3 2/7] vxlan: avoid checking socket multiple times Pravin B Shelar
` (6 subsequent siblings)
7 siblings, 0 replies; 22+ messages in thread
From: Pravin B Shelar @ 2016-11-14 4:43 UTC (permalink / raw)
To: netdev; +Cc: Pravin B Shelar
VxLan device does not have special handling for vlan taging on egress.
Therefore it does not make sense to expose vlan offloading feature.
This patch does not change vxlan functinality.
Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
Acked-by: Jiri Benc <jbenc@redhat.com>
---
drivers/net/vxlan.c | 9 +--------
include/linux/if_vlan.h | 16 ----------------
2 files changed, 1 insertion(+), 24 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index cb5cc7c..756d826 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1748,18 +1748,13 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
}
min_headroom = LL_RESERVED_SPACE(dst->dev) + dst->header_len
- + VXLAN_HLEN + iphdr_len
- + (skb_vlan_tag_present(skb) ? VLAN_HLEN : 0);
+ + VXLAN_HLEN + iphdr_len;
/* Need space for new headers (invalidates iph ptr) */
err = skb_cow_head(skb, min_headroom);
if (unlikely(err))
goto out_free;
- skb = vlan_hwaccel_push_inside(skb);
- if (WARN_ON(!skb))
- return -ENOMEM;
-
err = iptunnel_handle_offloads(skb, type);
if (err)
goto out_free;
@@ -2527,10 +2522,8 @@ static void vxlan_setup(struct net_device *dev)
dev->features |= NETIF_F_GSO_SOFTWARE;
dev->vlan_features = dev->features;
- dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
dev->hw_features |= NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_RXCSUM;
dev->hw_features |= NETIF_F_GSO_SOFTWARE;
- dev->hw_features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
netif_keep_dst(dev);
dev->priv_flags |= IFF_NO_QUEUE;
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 3319d97..8d5fcd6 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -399,22 +399,6 @@ static inline struct sk_buff *__vlan_hwaccel_push_inside(struct sk_buff *skb)
skb->vlan_tci = 0;
return skb;
}
-/*
- * vlan_hwaccel_push_inside - pushes vlan tag to the payload
- * @skb: skbuff to tag
- *
- * Checks is tag is present in @skb->vlan_tci and if it is, it pushes the
- * VLAN tag from @skb->vlan_tci inside to the payload.
- *
- * Following the skb_unshare() example, in case of error, the calling function
- * doesn't have to worry about freeing the original skb.
- */
-static inline struct sk_buff *vlan_hwaccel_push_inside(struct sk_buff *skb)
-{
- if (skb_vlan_tag_present(skb))
- skb = __vlan_hwaccel_push_inside(skb);
- return skb;
-}
/**
* __vlan_hwaccel_put_tag - hardware accelerated VLAN inserting
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH net-next v3 2/7] vxlan: avoid checking socket multiple times.
2016-11-14 4:43 [PATCH net-next v3 0/7] vxlan: xmit improvements Pravin B Shelar
2016-11-14 4:43 ` [PATCH net-next v3 1/7] vxlan: avoid vlan processing in vxlan device Pravin B Shelar
@ 2016-11-14 4:43 ` Pravin B Shelar
2016-11-15 14:15 ` Jiri Benc
2016-11-14 4:43 ` [PATCH net-next v3 3/7] vxlan: simplify exception handling Pravin B Shelar
` (5 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Pravin B Shelar @ 2016-11-14 4:43 UTC (permalink / raw)
To: netdev; +Cc: Pravin B Shelar
Check the vxlan socket in vxlan6_getroute().
Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
---
drivers/net/vxlan.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 756d826..9adeff9 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1830,6 +1830,7 @@ static struct rtable *vxlan_get_route(struct vxlan_dev *vxlan,
#if IS_ENABLED(CONFIG_IPV6)
static struct dst_entry *vxlan6_get_route(struct vxlan_dev *vxlan,
+ struct vxlan_sock *sock6,
struct sk_buff *skb, int oif, u8 tos,
__be32 label,
const struct in6_addr *daddr,
@@ -1837,7 +1838,6 @@ static struct dst_entry *vxlan6_get_route(struct vxlan_dev *vxlan,
struct dst_cache *dst_cache,
const struct ip_tunnel_info *info)
{
- struct vxlan_sock *sock6 = rcu_dereference(vxlan->vn6_sock);
bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
struct dst_entry *ndst;
struct flowi6 fl6;
@@ -2069,11 +2069,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
struct dst_entry *ndst;
u32 rt6i_flags;
- if (!sock6)
- goto drop;
- sk = sock6->sock->sk;
-
- ndst = vxlan6_get_route(vxlan, skb,
+ ndst = vxlan6_get_route(vxlan, sock6, skb,
rdst ? rdst->remote_ifindex : 0, tos,
label, &dst->sin6.sin6_addr,
&src->sin6.sin6_addr,
@@ -2093,6 +2089,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
goto tx_error;
}
+ sk = sock6->sock->sk;
/* Bypass encapsulation if the destination is local */
rt6i_flags = ((struct rt6_info *)ndst)->rt6i_flags;
if (!info && rt6i_flags & RTF_LOCAL &&
@@ -2432,9 +2429,10 @@ static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
ip_rt_put(rt);
} else {
#if IS_ENABLED(CONFIG_IPV6)
+ struct vxlan_sock *sock6 = rcu_dereference(vxlan->vn6_sock);
struct dst_entry *ndst;
- ndst = vxlan6_get_route(vxlan, skb, 0, info->key.tos,
+ ndst = vxlan6_get_route(vxlan, sock6, skb, 0, info->key.tos,
info->key.label, &info->key.u.ipv6.dst,
&info->key.u.ipv6.src, NULL, info);
if (IS_ERR(ndst))
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH net-next v3 2/7] vxlan: avoid checking socket multiple times.
2016-11-14 4:43 ` [PATCH net-next v3 2/7] vxlan: avoid checking socket multiple times Pravin B Shelar
@ 2016-11-15 14:15 ` Jiri Benc
0 siblings, 0 replies; 22+ messages in thread
From: Jiri Benc @ 2016-11-15 14:15 UTC (permalink / raw)
To: Pravin B Shelar; +Cc: netdev
Pravin,
please CC reviewers of the previous version when submitting a new
version. You'll get faster reviews that way.
On Sun, 13 Nov 2016 20:43:53 -0800, Pravin B Shelar wrote:
> @@ -2069,11 +2069,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
> struct dst_entry *ndst;
> u32 rt6i_flags;
>
> - if (!sock6)
> - goto drop;
> - sk = sock6->sock->sk;
> -
> - ndst = vxlan6_get_route(vxlan, skb,
> + ndst = vxlan6_get_route(vxlan, sock6, skb,
> rdst ? rdst->remote_ifindex : 0, tos,
> label, &dst->sin6.sin6_addr,
> &src->sin6.sin6_addr,
> @@ -2093,6 +2089,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
> goto tx_error;
> }
>
> + sk = sock6->sock->sk;
> /* Bypass encapsulation if the destination is local */
> rt6i_flags = ((struct rt6_info *)ndst)->rt6i_flags;
> if (!info && rt6i_flags & RTF_LOCAL &&
This moves the sk assignment from one arbitrary place to a different
arbitrary place, while it would be best to just remove it and open code
sock6->sock->sk in the call to udp_tunnel6_xmit_skb. But patch 6 does
that later, so whatever.
Acked-by: Jiri Benc <jbenc@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH net-next v3 3/7] vxlan: simplify exception handling
2016-11-14 4:43 [PATCH net-next v3 0/7] vxlan: xmit improvements Pravin B Shelar
2016-11-14 4:43 ` [PATCH net-next v3 1/7] vxlan: avoid vlan processing in vxlan device Pravin B Shelar
2016-11-14 4:43 ` [PATCH net-next v3 2/7] vxlan: avoid checking socket multiple times Pravin B Shelar
@ 2016-11-14 4:43 ` Pravin B Shelar
2016-11-15 14:30 ` Jiri Benc
2016-11-14 4:43 ` [PATCH net-next v3 4/7] vxlan: improve vxlan route lookup checks Pravin B Shelar
` (4 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Pravin B Shelar @ 2016-11-14 4:43 UTC (permalink / raw)
To: netdev; +Cc: Pravin B Shelar
vxlan egress path error handling has became complicated, it
need to handle IPv4 and IPv6 tunnel cases.
Earlier patch removes vlan handling from vxlan_build_skb(), so
vxlan_build_skb does not need to free skb and we can simplify
the xmit path by having single error handling for both type of
tunnels.
Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
---
drivers/net/vxlan.c | 46 +++++++++++++++++++---------------------------
1 file changed, 19 insertions(+), 27 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 9adeff9..8bb58f6 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1753,11 +1753,11 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
/* Need space for new headers (invalidates iph ptr) */
err = skb_cow_head(skb, min_headroom);
if (unlikely(err))
- goto out_free;
+ return err;
err = iptunnel_handle_offloads(skb, type);
if (err)
- goto out_free;
+ return err;
vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
vxh->vx_flags = VXLAN_HF_VNI;
@@ -1781,16 +1781,12 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
if (vxflags & VXLAN_F_GPE) {
err = vxlan_build_gpe_hdr(vxh, vxflags, skb->protocol);
if (err < 0)
- goto out_free;
+ return err;
inner_protocol = skb->protocol;
}
skb_set_inner_protocol(skb, inner_protocol);
return 0;
-
-out_free:
- kfree_skb(skb);
- return err;
}
static struct rtable *vxlan_get_route(struct vxlan_dev *vxlan,
@@ -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;
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;
}
@@ -2114,11 +2112,9 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
skb_scrub_packet(skb, xnet);
err = vxlan_build_skb(skb, ndst, sizeof(struct ipv6hdr),
vni, md, flags, udp_sum);
- if (err < 0) {
- dst_release(ndst);
- dev->stats.tx_errors++;
- return;
- }
+ if (err < 0)
+ goto tx_error;
+
udp_tunnel6_xmit_skb(ndst, sk, skb, dev,
&src->sin6.sin6_addr,
&dst->sin6.sin6_addr, tos, ttl,
@@ -2130,17 +2126,13 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
drop:
dev->stats.tx_dropped++;
- goto tx_free;
+ dev_kfree_skb(skb);
+ return;
-xmit_tx_error:
- /* skb is already freed. */
- skb = NULL;
-rt_tx_error:
- ip_rt_put(rt);
tx_error:
+ dst_release(ndst);
dev->stats.tx_errors++;
-tx_free:
- dev_kfree_skb(skb);
+ kfree_skb(skb);
}
/* Transmit local packets over Vxlan
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH net-next v3 3/7] vxlan: simplify exception handling
2016-11-14 4:43 ` [PATCH net-next v3 3/7] vxlan: simplify exception handling Pravin B Shelar
@ 2016-11-15 14:30 ` Jiri Benc
2016-11-15 16:40 ` Pravin Shelar
0 siblings, 1 reply; 22+ messages in thread
From: Jiri Benc @ 2016-11-15 14:30 UTC (permalink / raw)
To: Pravin B Shelar; +Cc: netdev
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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next v3 3/7] vxlan: simplify exception handling
2016-11-15 14:30 ` Jiri Benc
@ 2016-11-15 16:40 ` Pravin Shelar
2016-11-15 17:04 ` Jiri Benc
0 siblings, 1 reply; 22+ messages in thread
From: Pravin Shelar @ 2016-11-15 16:40 UTC (permalink / raw)
To: Jiri Benc; +Cc: Linux Kernel Network Developers
On Tue, Nov 15, 2016 at 6:30 AM, Jiri Benc <jbenc@redhat.com> wrote:
> 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...
>
Does it really matters given that next patches in this series moves
this duplicate code and does pretty much what you are describing?
>> 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?
>
Patch 5 does this by defining encap_bypass_if_local() for IPv4 and
IPv6 vxlan tunnels.
> Looks good otherwise. Seeing it, I like this version much more than v2.
>
> Thanks!
>
> Jiri
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH net-next v3 3/7] vxlan: simplify exception handling
2016-11-15 16:40 ` Pravin Shelar
@ 2016-11-15 17:04 ` Jiri Benc
2016-11-15 17:15 ` Pravin Shelar
0 siblings, 1 reply; 22+ messages in thread
From: Jiri Benc @ 2016-11-15 17:04 UTC (permalink / raw)
To: Pravin Shelar; +Cc: Linux Kernel Network Developers
On Tue, 15 Nov 2016 08:40:58 -0800, Pravin Shelar wrote:
> On Tue, Nov 15, 2016 at 6:30 AM, Jiri Benc <jbenc@redhat.com> wrote:
> > 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...
> >
> Does it really matters given that next patches in this series moves
> this duplicate code and does pretty much what you are describing?
Okay, right. I tried to look also at patches further in the series but
it seemed to me this will leave an instance of ip_rt_put that could be
avoided. But it will not.
Acked-by: Jiri Benc <jbenc@redhat.com>
(It would make the reviewers' life easier if the individual patches
were more self contained. Ideally, each patch should be able to stand
on its own. This unrelated code shuffling makes it too easy to miss
things...)
Anyway, thanks for the cleanup!
Jiri
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v3 3/7] vxlan: simplify exception handling
2016-11-15 17:04 ` Jiri Benc
@ 2016-11-15 17:15 ` Pravin Shelar
0 siblings, 0 replies; 22+ messages in thread
From: Pravin Shelar @ 2016-11-15 17:15 UTC (permalink / raw)
To: Jiri Benc; +Cc: Linux Kernel Network Developers
On Tue, Nov 15, 2016 at 9:04 AM, Jiri Benc <jbenc@redhat.com> wrote:
> On Tue, 15 Nov 2016 08:40:58 -0800, Pravin Shelar wrote:
>> On Tue, Nov 15, 2016 at 6:30 AM, Jiri Benc <jbenc@redhat.com> wrote:
>> > 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...
>> >
>> Does it really matters given that next patches in this series moves
>> this duplicate code and does pretty much what you are describing?
>
> Okay, right. I tried to look also at patches further in the series but
> it seemed to me this will leave an instance of ip_rt_put that could be
> avoided. But it will not.
>
> Acked-by: Jiri Benc <jbenc@redhat.com>
>
> (It would make the reviewers' life easier if the individual patches
> were more self contained. Ideally, each patch should be able to stand
> on its own. This unrelated code shuffling makes it too easy to miss
> things...)
>
I tried to keep one patch targeting single cleanup. But all patches
targeted same code, that makes it dependent on each other.
> Anyway, thanks for the cleanup!
>
Thanks for all reviews.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH net-next v3 4/7] vxlan: improve vxlan route lookup checks.
2016-11-14 4:43 [PATCH net-next v3 0/7] vxlan: xmit improvements Pravin B Shelar
` (2 preceding siblings ...)
2016-11-14 4:43 ` [PATCH net-next v3 3/7] vxlan: simplify exception handling Pravin B Shelar
@ 2016-11-14 4:43 ` Pravin B Shelar
2016-11-15 14:39 ` Jiri Benc
2016-11-14 4:43 ` [PATCH net-next v3 5/7] vxlan: simplify RTF_LOCAL handling Pravin B Shelar
` (3 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Pravin B Shelar @ 2016-11-14 4:43 UTC (permalink / raw)
To: netdev; +Cc: Pravin B Shelar
Move route sanity check to respective vxlan[4/6]_get_route functions.
This allows us to perform all sanity checks before caching the dst so
that we can avoid these checks on subsequent packets.
This give move accurate metadata information for packet from
fill_metadata_dst().
Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
---
drivers/net/vxlan.c | 77 ++++++++++++++++++++++++++---------------------------
1 file changed, 38 insertions(+), 39 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 8bb58f6..aabb918 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1789,7 +1789,8 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
return 0;
}
-static struct rtable *vxlan_get_route(struct vxlan_dev *vxlan,
+static struct rtable *vxlan_get_route(struct vxlan_dev *vxlan, struct net_device *dev,
+ struct vxlan_sock *sock4,
struct sk_buff *skb, int oif, u8 tos,
__be32 daddr, __be32 *saddr,
struct dst_cache *dst_cache,
@@ -1799,6 +1800,9 @@ static struct rtable *vxlan_get_route(struct vxlan_dev *vxlan,
struct rtable *rt = NULL;
struct flowi4 fl4;
+ if (!sock4)
+ return ERR_PTR(-EIO);
+
if (tos && !info)
use_cache = false;
if (use_cache) {
@@ -1816,16 +1820,26 @@ static struct rtable *vxlan_get_route(struct vxlan_dev *vxlan,
fl4.saddr = *saddr;
rt = ip_route_output_key(vxlan->net, &fl4);
- if (!IS_ERR(rt)) {
+ if (likely(!IS_ERR(rt))) {
+ if (rt->dst.dev == dev) {
+ netdev_dbg(dev, "circular route to %pI4\n", &daddr);
+ ip_rt_put(rt);
+ return ERR_PTR(-ELOOP);
+ }
+
*saddr = fl4.saddr;
if (use_cache)
dst_cache_set_ip4(dst_cache, &rt->dst, fl4.saddr);
+ } else {
+ netdev_dbg(dev, "no route to %pI4\n", &daddr);
+ return ERR_PTR(-ENETUNREACH);
}
return rt;
}
#if IS_ENABLED(CONFIG_IPV6)
static struct dst_entry *vxlan6_get_route(struct vxlan_dev *vxlan,
+ struct net_device *dev,
struct vxlan_sock *sock6,
struct sk_buff *skb, int oif, u8 tos,
__be32 label,
@@ -1861,8 +1875,16 @@ static struct dst_entry *vxlan6_get_route(struct vxlan_dev *vxlan,
err = ipv6_stub->ipv6_dst_lookup(vxlan->net,
sock6->sock->sk,
&ndst, &fl6);
- if (err < 0)
- return ERR_PTR(err);
+ if (unlikely(err < 0)) {
+ netdev_dbg(dev, "no route to %pI6\n", daddr);
+ return ERR_PTR(-ENETUNREACH);
+ }
+
+ if (unlikely(ndst->dev == dev)) {
+ netdev_dbg(dev, "circular route to %pI6\n", daddr);
+ dst_release(ndst);
+ return ERR_PTR(-ELOOP);
+ }
*saddr = fl6.saddr;
if (use_cache)
@@ -1929,8 +1951,8 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
union vxlan_addr *src;
struct vxlan_metadata _md;
struct vxlan_metadata *md = &_md;
- struct dst_entry *ndst = NULL;
__be16 src_port = 0, dst_port;
+ struct dst_entry *ndst = NULL;
__be32 vni, label;
__be16 df = 0;
__u8 tos, ttl;
@@ -2007,29 +2029,14 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
struct vxlan_sock *sock4 = rcu_dereference(vxlan->vn4_sock);
struct rtable *rt;
- if (!sock4)
- goto drop;
- sk = sock4->sock->sk;
-
- rt = vxlan_get_route(vxlan, skb,
+ rt = vxlan_get_route(vxlan, dev, sock4, skb,
rdst ? rdst->remote_ifindex : 0, tos,
dst->sin.sin_addr.s_addr,
&src->sin.sin_addr.s_addr,
dst_cache, info);
- if (IS_ERR(rt)) {
- netdev_dbg(dev, "no route to %pI4\n",
- &dst->sin.sin_addr.s_addr);
- dev->stats.tx_carrier_errors++;
- goto tx_error;
- }
-
- if (rt->dst.dev == dev) {
- netdev_dbg(dev, "circular route to %pI4\n",
- &dst->sin.sin_addr.s_addr);
- dev->stats.collisions++;
- ip_rt_put(rt);
+ if (IS_ERR(rt))
goto tx_error;
- }
+ sk = sock4->sock->sk;
/* Bypass encapsulation if the destination is local */
if (!info && rt->rt_flags & RTCF_LOCAL &&
@@ -2067,27 +2074,17 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
struct vxlan_sock *sock6 = rcu_dereference(vxlan->vn6_sock);
u32 rt6i_flags;
- ndst = vxlan6_get_route(vxlan, sock6, skb,
+ ndst = vxlan6_get_route(vxlan, dev, sock6, skb,
rdst ? rdst->remote_ifindex : 0, tos,
label, &dst->sin6.sin6_addr,
&src->sin6.sin6_addr,
dst_cache, info);
if (IS_ERR(ndst)) {
- 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);
- dev->stats.collisions++;
- goto tx_error;
- }
-
sk = sock6->sock->sk;
+
/* Bypass encapsulation if the destination is local */
rt6i_flags = ((struct rt6_info *)ndst)->rt6i_flags;
if (!info && rt6i_flags & RTF_LOCAL &&
@@ -2130,6 +2127,10 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
return;
tx_error:
+ if (err == -ELOOP)
+ dev->stats.collisions++;
+ else if (err == -ENETUNREACH)
+ dev->stats.tx_carrier_errors++;
dst_release(ndst);
dev->stats.tx_errors++;
kfree_skb(skb);
@@ -2411,9 +2412,7 @@ static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
struct vxlan_sock *sock4 = rcu_dereference(vxlan->vn4_sock);
struct rtable *rt;
- if (!sock4)
- return -EINVAL;
- rt = vxlan_get_route(vxlan, skb, 0, info->key.tos,
+ rt = vxlan_get_route(vxlan, dev, sock4, skb, 0, info->key.tos,
info->key.u.ipv4.dst,
&info->key.u.ipv4.src, NULL, info);
if (IS_ERR(rt))
@@ -2424,7 +2423,7 @@ static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
struct vxlan_sock *sock6 = rcu_dereference(vxlan->vn6_sock);
struct dst_entry *ndst;
- ndst = vxlan6_get_route(vxlan, sock6, skb, 0, info->key.tos,
+ ndst = vxlan6_get_route(vxlan, dev, sock6, skb, 0, info->key.tos,
info->key.label, &info->key.u.ipv6.dst,
&info->key.u.ipv6.src, NULL, info);
if (IS_ERR(ndst))
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH net-next v3 4/7] vxlan: improve vxlan route lookup checks.
2016-11-14 4:43 ` [PATCH net-next v3 4/7] vxlan: improve vxlan route lookup checks Pravin B Shelar
@ 2016-11-15 14:39 ` Jiri Benc
2016-11-17 10:17 ` David Laight
0 siblings, 1 reply; 22+ messages in thread
From: Jiri Benc @ 2016-11-15 14:39 UTC (permalink / raw)
To: Pravin B Shelar; +Cc: netdev
On Sun, 13 Nov 2016 20:43:55 -0800, Pravin B Shelar wrote:
> @@ -1929,8 +1951,8 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
> union vxlan_addr *src;
> struct vxlan_metadata _md;
> struct vxlan_metadata *md = &_md;
> - struct dst_entry *ndst = NULL;
> __be16 src_port = 0, dst_port;
> + struct dst_entry *ndst = NULL;
> __be32 vni, label;
> __be16 df = 0;
> __u8 tos, ttl;
This looks kind of arbitrary. You might want to remove this hunk or
merge it to patch 3.
Other than that,
Acked-by: Jiri Benc <jbenc@redhat.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* RE: [PATCH net-next v3 4/7] vxlan: improve vxlan route lookup checks.
2016-11-15 14:39 ` Jiri Benc
@ 2016-11-17 10:17 ` David Laight
2016-11-17 15:59 ` Jiri Benc
2016-11-18 5:30 ` Pravin Shelar
0 siblings, 2 replies; 22+ messages in thread
From: David Laight @ 2016-11-17 10:17 UTC (permalink / raw)
To: 'Jiri Benc', Pravin B Shelar; +Cc: netdev@vger.kernel.org
From: Of Jiri Benc
> Sent: 15 November 2016 14:40
> On Sun, 13 Nov 2016 20:43:55 -0800, Pravin B Shelar wrote:
> > @@ -1929,8 +1951,8 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
> > union vxlan_addr *src;
> > struct vxlan_metadata _md;
> > struct vxlan_metadata *md = &_md;
> > - struct dst_entry *ndst = NULL;
> > __be16 src_port = 0, dst_port;
> > + struct dst_entry *ndst = NULL;
> > __be32 vni, label;
> > __be16 df = 0;
> > __u8 tos, ttl;
>
> This looks kind of arbitrary. You might want to remove this hunk or
> merge it to patch 3.
Worse than arbitrary, it adds 4 bytes of pad on 64bit systems.
David
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v3 4/7] vxlan: improve vxlan route lookup checks.
2016-11-17 10:17 ` David Laight
@ 2016-11-17 15:59 ` Jiri Benc
2016-11-17 18:11 ` David Miller
2016-11-21 20:34 ` Pravin Shelar
2016-11-18 5:30 ` Pravin Shelar
1 sibling, 2 replies; 22+ messages in thread
From: Jiri Benc @ 2016-11-17 15:59 UTC (permalink / raw)
To: David Laight; +Cc: Pravin B Shelar, netdev@vger.kernel.org
On Thu, 17 Nov 2016 10:17:01 +0000, David Laight wrote:
> Worse than arbitrary, it adds 4 bytes of pad on 64bit systems.
It does not, this is not a struct.
Jiri
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v3 4/7] vxlan: improve vxlan route lookup checks.
2016-11-17 15:59 ` Jiri Benc
@ 2016-11-17 18:11 ` David Miller
2016-11-21 20:34 ` Pravin Shelar
1 sibling, 0 replies; 22+ messages in thread
From: David Miller @ 2016-11-17 18:11 UTC (permalink / raw)
To: jbenc; +Cc: David.Laight, pshelar, netdev
From: Jiri Benc <jbenc@redhat.com>
Date: Thu, 17 Nov 2016 16:59:49 +0100
> On Thu, 17 Nov 2016 10:17:01 +0000, David Laight wrote:
>> Worse than arbitrary, it adds 4 bytes of pad on 64bit systems.
>
> It does not, this is not a struct.
He is talking about on the function stack.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v3 4/7] vxlan: improve vxlan route lookup checks.
2016-11-17 15:59 ` Jiri Benc
2016-11-17 18:11 ` David Miller
@ 2016-11-21 20:34 ` Pravin Shelar
1 sibling, 0 replies; 22+ messages in thread
From: Pravin Shelar @ 2016-11-21 20:34 UTC (permalink / raw)
To: Jiri Benc; +Cc: David Laight, netdev@vger.kernel.org
On Thu, Nov 17, 2016 at 7:59 AM, Jiri Benc <jbenc@redhat.com> wrote:
> On Thu, 17 Nov 2016 10:17:01 +0000, David Laight wrote:
>> Worse than arbitrary, it adds 4 bytes of pad on 64bit systems.
>
> It does not, this is not a struct.
>
right.
After looking at the assembly code, it is clear that GCC and most of
modern compiler can reorder function variables for efficient storage.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net-next v3 4/7] vxlan: improve vxlan route lookup checks.
2016-11-17 10:17 ` David Laight
2016-11-17 15:59 ` Jiri Benc
@ 2016-11-18 5:30 ` Pravin Shelar
1 sibling, 0 replies; 22+ messages in thread
From: Pravin Shelar @ 2016-11-18 5:30 UTC (permalink / raw)
To: David Laight; +Cc: Jiri Benc, netdev@vger.kernel.org
On Thu, Nov 17, 2016 at 2:17 AM, David Laight <David.Laight@aculab.com> wrote:
> From: Of Jiri Benc
>> Sent: 15 November 2016 14:40
>> On Sun, 13 Nov 2016 20:43:55 -0800, Pravin B Shelar wrote:
>> > @@ -1929,8 +1951,8 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
>> > union vxlan_addr *src;
>> > struct vxlan_metadata _md;
>> > struct vxlan_metadata *md = &_md;
>> > - struct dst_entry *ndst = NULL;
>> > __be16 src_port = 0, dst_port;
>> > + struct dst_entry *ndst = NULL;
>> > __be32 vni, label;
>> > __be16 df = 0;
>> > __u8 tos, ttl;
>>
>> This looks kind of arbitrary. You might want to remove this hunk or
>> merge it to patch 3.
>
> Worse than arbitrary, it adds 4 bytes of pad on 64bit systems.
>
OK. I will send out a patch.
But this is not real issue in vxlan module today ;)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH net-next v3 5/7] vxlan: simplify RTF_LOCAL handling.
2016-11-14 4:43 [PATCH net-next v3 0/7] vxlan: xmit improvements Pravin B Shelar
` (3 preceding siblings ...)
2016-11-14 4:43 ` [PATCH net-next v3 4/7] vxlan: improve vxlan route lookup checks Pravin B Shelar
@ 2016-11-14 4:43 ` Pravin B Shelar
2016-11-15 14:44 ` Jiri Benc
2016-11-14 4:43 ` [PATCH net-next v3 6/7] vxlan: simplify vxlan xmit Pravin B Shelar
` (2 subsequent siblings)
7 siblings, 1 reply; 22+ messages in thread
From: Pravin B Shelar @ 2016-11-14 4:43 UTC (permalink / raw)
To: netdev; +Cc: Pravin B Shelar
Avoid code duplicate code for handling RTF_LOCAL routes.
Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
---
drivers/net/vxlan.c | 85 ++++++++++++++++++++++++++++++++---------------------
1 file changed, 51 insertions(+), 34 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index aabb918..0b188d6 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1938,6 +1938,40 @@ static void vxlan_encap_bypass(struct sk_buff *skb, struct vxlan_dev *src_vxlan,
}
}
+static int encap_bypass_if_local(struct sk_buff *skb, struct net_device *dev,
+ struct vxlan_dev *vxlan, union vxlan_addr *daddr,
+ __be32 dst_port, __be32 vni, struct dst_entry *dst,
+ u32 rt_flags)
+{
+#if IS_ENABLED(CONFIG_IPV6)
+ /* IPv6 rt-flags are checked against RTF_LOCAL, but the value of
+ * RTF_LOCAL is equal to RTCF_LOCAL. So to keep code simple
+ * we can use RTCF_LOCAL which works for ipv4 and ipv6 route entry.
+ */
+ BUILD_BUG_ON(RTCF_LOCAL != RTF_LOCAL);
+#endif
+ /* Bypass encapsulation if the destination is local */
+ if (rt_flags & RTCF_LOCAL &&
+ !(rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST))) {
+ struct vxlan_dev *dst_vxlan;
+
+ dst_release(dst);
+ dst_vxlan = vxlan_find_vni(vxlan->net, vni,
+ daddr->sa.sa_family, dst_port,
+ vxlan->flags);
+ if (!dst_vxlan) {
+ dev->stats.tx_errors++;
+ kfree_skb(skb);
+
+ return -ENOENT;
+ }
+ vxlan_encap_bypass(skb, vxlan, dst_vxlan);
+ return 1;
+ }
+
+ return 0;
+}
+
static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
struct vxlan_rdst *rdst, bool did_rsc)
{
@@ -2036,27 +2070,19 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
dst_cache, info);
if (IS_ERR(rt))
goto tx_error;
- sk = sock4->sock->sk;
+ sk = sock4->sock->sk;
/* Bypass encapsulation if the destination is local */
- if (!info && rt->rt_flags & RTCF_LOCAL &&
- !(rt->rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST))) {
- struct vxlan_dev *dst_vxlan;
-
- ip_rt_put(rt);
- dst_vxlan = vxlan_find_vni(vxlan->net, vni,
- dst->sa.sa_family, dst_port,
- vxlan->flags);
- if (!dst_vxlan)
- goto tx_error;
- vxlan_encap_bypass(skb, vxlan, dst_vxlan);
- return;
- }
-
- if (!info)
+ if (!info) {
+ err = encap_bypass_if_local(skb, dev, vxlan, dst,
+ dst_port, vni, &rt->dst,
+ rt->rt_flags);
+ if (err)
+ return;
udp_sum = !(flags & VXLAN_F_UDP_ZERO_CSUM_TX);
- else if (info->key.tun_flags & TUNNEL_DONT_FRAGMENT)
+ } else if (info->key.tun_flags & TUNNEL_DONT_FRAGMENT) {
df = htons(IP_DF);
+ }
ndst = &rt->dst;
tos = ip_tunnel_ecn_encap(tos, old_iph, skb);
@@ -2072,7 +2098,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);
- u32 rt6i_flags;
ndst = vxlan6_get_route(vxlan, dev, sock6, skb,
rdst ? rdst->remote_ifindex : 0, tos,
@@ -2085,24 +2110,16 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
}
sk = sock6->sock->sk;
- /* Bypass encapsulation if the destination is local */
- rt6i_flags = ((struct rt6_info *)ndst)->rt6i_flags;
- if (!info && rt6i_flags & RTF_LOCAL &&
- !(rt6i_flags & (RTCF_BROADCAST | RTCF_MULTICAST))) {
- struct vxlan_dev *dst_vxlan;
-
- 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;
- }
+ if (!info) {
+ u32 rt6i_flags = ((struct rt6_info *)ndst)->rt6i_flags;
- if (!info)
+ err = encap_bypass_if_local(skb, dev, vxlan, dst,
+ dst_port, vni, ndst,
+ rt6i_flags);
+ if (err)
+ return;
udp_sum = !(flags & VXLAN_F_UDP_ZERO_CSUM6_TX);
+ }
tos = ip_tunnel_ecn_encap(tos, old_iph, skb);
ttl = ttl ? : ip6_dst_hoplimit(ndst);
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH net-next v3 6/7] vxlan: simplify vxlan xmit
2016-11-14 4:43 [PATCH net-next v3 0/7] vxlan: xmit improvements Pravin B Shelar
` (4 preceding siblings ...)
2016-11-14 4:43 ` [PATCH net-next v3 5/7] vxlan: simplify RTF_LOCAL handling Pravin B Shelar
@ 2016-11-14 4:43 ` Pravin B Shelar
2016-11-14 4:43 ` [PATCH net-next v3 7/7] vxlan: remove unsed vxlan_dev_dst_port() Pravin B Shelar
2016-11-15 17:16 ` [PATCH net-next v3 0/7] vxlan: xmit improvements David Miller
7 siblings, 0 replies; 22+ messages in thread
From: Pravin B Shelar @ 2016-11-14 4:43 UTC (permalink / raw)
To: netdev; +Cc: Pravin B Shelar
Existing vxlan xmit function handles two distinct cases.
1. vxlan net device
2. vxlan lwt device.
By seperating initialization these two cases the egress path
looks better.
Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
Acked-by: Jiri Benc <jbenc@redhat.com>
---
drivers/net/vxlan.c | 78 +++++++++++++++++++++++------------------------------
1 file changed, 34 insertions(+), 44 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 0b188d6..411534c 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1978,8 +1978,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
struct dst_cache *dst_cache;
struct ip_tunnel_info *info;
struct vxlan_dev *vxlan = netdev_priv(dev);
- struct sock *sk;
- const struct iphdr *old_iph;
+ const struct iphdr *old_iph = ip_hdr(skb);
union vxlan_addr *dst;
union vxlan_addr remote_ip, local_ip;
union vxlan_addr *src;
@@ -1988,7 +1987,6 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
__be16 src_port = 0, dst_port;
struct dst_entry *ndst = NULL;
__be32 vni, label;
- __be16 df = 0;
__u8 tos, ttl;
int err;
u32 flags = vxlan->flags;
@@ -1998,19 +1996,40 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
info = skb_tunnel_info(skb);
if (rdst) {
+ dst = &rdst->remote_ip;
+ if (vxlan_addr_any(dst)) {
+ if (did_rsc) {
+ /* short-circuited back to local bridge */
+ vxlan_encap_bypass(skb, vxlan, vxlan);
+ return;
+ }
+ goto drop;
+ }
+
dst_port = rdst->remote_port ? rdst->remote_port : vxlan->cfg.dst_port;
vni = rdst->remote_vni;
- dst = &rdst->remote_ip;
src = &vxlan->cfg.saddr;
dst_cache = &rdst->dst_cache;
+ md->gbp = skb->mark;
+ ttl = vxlan->cfg.ttl;
+ if (!ttl && vxlan_addr_multicast(dst))
+ ttl = 1;
+
+ tos = vxlan->cfg.tos;
+ if (tos == 1)
+ tos = ip_tunnel_get_dsfield(old_iph, skb);
+
+ if (dst->sa.sa_family == AF_INET)
+ udp_sum = !(flags & VXLAN_F_UDP_ZERO_CSUM_TX);
+ else
+ udp_sum = !(flags & VXLAN_F_UDP_ZERO_CSUM6_TX);
+ label = vxlan->cfg.label;
} else {
if (!info) {
WARN_ONCE(1, "%s: Missing encapsulation instructions\n",
dev->name);
goto drop;
}
- dst_port = info->key.tp_dst ? : vxlan->cfg.dst_port;
- vni = tunnel_id_to_key32(info->key.tun_id);
remote_ip.sa.sa_family = ip_tunnel_info_af(info);
if (remote_ip.sa.sa_family == AF_INET) {
remote_ip.sin.sin_addr.s_addr = info->key.u.ipv4.dst;
@@ -2020,48 +2039,24 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
local_ip.sin6.sin6_addr = info->key.u.ipv6.src;
}
dst = &remote_ip;
+ dst_port = info->key.tp_dst ? : vxlan->cfg.dst_port;
+ vni = tunnel_id_to_key32(info->key.tun_id);
src = &local_ip;
dst_cache = &info->dst_cache;
- }
-
- if (vxlan_addr_any(dst)) {
- if (did_rsc) {
- /* short-circuited back to local bridge */
- vxlan_encap_bypass(skb, vxlan, vxlan);
- return;
- }
- goto drop;
- }
-
- old_iph = ip_hdr(skb);
-
- ttl = vxlan->cfg.ttl;
- if (!ttl && vxlan_addr_multicast(dst))
- ttl = 1;
-
- tos = vxlan->cfg.tos;
- if (tos == 1)
- tos = ip_tunnel_get_dsfield(old_iph, skb);
-
- label = vxlan->cfg.label;
- src_port = udp_flow_src_port(dev_net(dev), skb, vxlan->cfg.port_min,
- vxlan->cfg.port_max, true);
-
- if (info) {
+ if (info->options_len)
+ md = ip_tunnel_info_opts(info);
ttl = info->key.ttl;
tos = info->key.tos;
label = info->key.label;
udp_sum = !!(info->key.tun_flags & TUNNEL_CSUM);
-
- if (info->options_len)
- md = ip_tunnel_info_opts(info);
- } else {
- md->gbp = skb->mark;
}
+ src_port = udp_flow_src_port(dev_net(dev), skb, vxlan->cfg.port_min,
+ vxlan->cfg.port_max, true);
if (dst->sa.sa_family == AF_INET) {
struct vxlan_sock *sock4 = rcu_dereference(vxlan->vn4_sock);
struct rtable *rt;
+ __be16 df = 0;
rt = vxlan_get_route(vxlan, dev, sock4, skb,
rdst ? rdst->remote_ifindex : 0, tos,
@@ -2071,7 +2066,6 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
if (IS_ERR(rt))
goto tx_error;
- sk = sock4->sock->sk;
/* Bypass encapsulation if the destination is local */
if (!info) {
err = encap_bypass_if_local(skb, dev, vxlan, dst,
@@ -2079,7 +2073,6 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
rt->rt_flags);
if (err)
return;
- udp_sum = !(flags & VXLAN_F_UDP_ZERO_CSUM_TX);
} else if (info->key.tun_flags & TUNNEL_DONT_FRAGMENT) {
df = htons(IP_DF);
}
@@ -2092,7 +2085,7 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
if (err < 0)
goto tx_error;
- udp_tunnel_xmit_skb(rt, sk, skb, src->sin.sin_addr.s_addr,
+ udp_tunnel_xmit_skb(rt, sock4->sock->sk, skb, src->sin.sin_addr.s_addr,
dst->sin.sin_addr.s_addr, tos, ttl, df,
src_port, dst_port, xnet, !udp_sum);
#if IS_ENABLED(CONFIG_IPV6)
@@ -2108,7 +2101,6 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
ndst = NULL;
goto tx_error;
}
- sk = sock6->sock->sk;
if (!info) {
u32 rt6i_flags = ((struct rt6_info *)ndst)->rt6i_flags;
@@ -2118,7 +2110,6 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
rt6i_flags);
if (err)
return;
- udp_sum = !(flags & VXLAN_F_UDP_ZERO_CSUM6_TX);
}
tos = ip_tunnel_ecn_encap(tos, old_iph, skb);
@@ -2129,13 +2120,12 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
if (err < 0)
goto tx_error;
- udp_tunnel6_xmit_skb(ndst, sk, skb, dev,
+ udp_tunnel6_xmit_skb(ndst, sock6->sock->sk, skb, dev,
&src->sin6.sin6_addr,
&dst->sin6.sin6_addr, tos, ttl,
label, src_port, dst_port, !udp_sum);
#endif
}
-
return;
drop:
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH net-next v3 7/7] vxlan: remove unsed vxlan_dev_dst_port()
2016-11-14 4:43 [PATCH net-next v3 0/7] vxlan: xmit improvements Pravin B Shelar
` (5 preceding siblings ...)
2016-11-14 4:43 ` [PATCH net-next v3 6/7] vxlan: simplify vxlan xmit Pravin B Shelar
@ 2016-11-14 4:43 ` Pravin B Shelar
2016-11-15 14:56 ` Jiri Benc
2016-11-15 17:16 ` [PATCH net-next v3 0/7] vxlan: xmit improvements David Miller
7 siblings, 1 reply; 22+ messages in thread
From: Pravin B Shelar @ 2016-11-14 4:43 UTC (permalink / raw)
To: netdev; +Cc: Pravin B Shelar
Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
---
include/net/vxlan.h | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 308adc4..49a5920 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -281,16 +281,6 @@ struct vxlan_dev {
struct net_device *vxlan_dev_create(struct net *net, const char *name,
u8 name_assign_type, struct vxlan_config *conf);
-static inline __be16 vxlan_dev_dst_port(struct vxlan_dev *vxlan,
- unsigned short family)
-{
-#if IS_ENABLED(CONFIG_IPV6)
- if (family == AF_INET6)
- return inet_sk(vxlan->vn6_sock->sock->sk)->inet_sport;
-#endif
- return inet_sk(vxlan->vn4_sock->sock->sk)->inet_sport;
-}
-
static inline netdev_features_t vxlan_features_check(struct sk_buff *skb,
netdev_features_t features)
{
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH net-next v3 0/7] vxlan: xmit improvements.
2016-11-14 4:43 [PATCH net-next v3 0/7] vxlan: xmit improvements Pravin B Shelar
` (6 preceding siblings ...)
2016-11-14 4:43 ` [PATCH net-next v3 7/7] vxlan: remove unsed vxlan_dev_dst_port() Pravin B Shelar
@ 2016-11-15 17:16 ` David Miller
7 siblings, 0 replies; 22+ messages in thread
From: David Miller @ 2016-11-15 17:16 UTC (permalink / raw)
To: pshelar; +Cc: netdev
From: Pravin B Shelar <pshelar@ovn.org>
Date: Sun, 13 Nov 2016 20:43:51 -0800
> Following patch series improves vxlan fast path, removes
> duplicate code and simplifies vxlan xmit code path.
>
> v2-v3:
> Removed unrelated warning fix from patch 2.
> rearranged error handling from patch 3
> Fixed stats updates in vxlan route lookup in patch 4
>
> v1-v2:
> Fix compilation error when IPv6 support is not enabled.
Series applied, thanks.
^ permalink raw reply [flat|nested] 22+ messages in thread